[Differential] D6689: tcp/lro: Implement hash table for LRO entries.

Discussion:

(too old to reply)

hselasky (Hans Petter Selasky)

2016-06-02 07:34:17 UTC

_______________________________________________
freebsd-***@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "freebsd-net-***@freebsd.org"

sepherosa_gmail.com (Sepherosa Ziehau)

2016-06-02 08:02:53 UTC

Permalink

sepherosa_gmail.com (Sepherosa Ziehau)

2016-06-02 16:22:47 UTC

Permalink

hselasky (Hans Petter Selasky)

2016-06-02 08:26:36 UTC

Permalink

sepherosa_gmail.com (Sepherosa Ziehau)

2016-06-02 16:09:29 UTC

Permalink

hselasky (Hans Petter Selasky)

2016-06-02 16:17:00 UTC

Permalink

sepherosa_gmail.com (Sepherosa Ziehau)

2016-06-06 08:40:01 UTC

Permalink

sepherosa_gmail.com updated this revision to Diff 17347.
sepherosa_gmail.com added a comment.

Address hps's cache pollution concern for queue_mbuf path. Similar approach was in the patch obtained from rrs.

CHANGES SINCE LAST UPDATE
https://reviews.freebsd.org/D6689?vs=17214&id=17347

REVISION DETAIL
https://reviews.freebsd.org/D6689

AFFECTED FILES
sys/netinet/tcp_lro.c
sys/netinet/tcp_lro.h

EMAIL PREFERENCES
https://reviews.freebsd.org/settings/panel/emailpreferences/

To: sepherosa_gmail.com, rrs, gallatin, glebius, gnn, bz, rwatson, #transport, hselasky
Cc: freebsd-net-list

sepherosa_gmail.com (Sepherosa Ziehau)

2016-06-06 08:41:30 UTC

Permalink

sepherosa_gmail.com added a comment.

Well, as I said the VM does not have the luxury to do sorting :)

Can you test the existing "tcp_lro_flush_all()" in combination with "tcp_lro_queue_mbuf()" and compare it to your version? Sorting is not as slow as you think.
--HPS

I will use the updated patch to test the hash and the queue_mbuf in Hyper-V. Thanks!

REVISION DETAIL
https://reviews.freebsd.org/D6689

EMAIL PREFERENCES
https://reviews.freebsd.org/settings/panel/emailpreferences/

To: sepherosa_gmail.com, rrs, gallatin, glebius, gnn, bz, rwatson, #transport, hselasky
Cc: freebsd-net-list

gallatin (Andrew Gallatin)

2016-06-07 13:16:18 UTC

Permalink

gallatin accepted this revision.
gallatin added a comment.

Looks good in terms of not killing perf. for the sorted case, and I'm fine with it as-is. However, maybe an else would be better than a goto?

REVISION DETAIL
https://reviews.freebsd.org/D6689

EMAIL PREFERENCES
https://reviews.freebsd.org/settings/panel/emailpreferences/

To: sepherosa_gmail.com, rrs, glebius, gnn, bz, rwatson, #transport, hselasky, gallatin
Cc: freebsd-net-list

sepherosa_gmail.com (Sepherosa Ziehau)

2016-06-08 01:25:42 UTC

Permalink

sepherosa_gmail.com added a comment.

Post by gallatin (Andrew Gallatin)
Looks good in terms of not killing perf. for the sorted case, and I'm fine with it as-is. However, maybe an else would be better than a goto?

Yeah, sure, I can move the bucket assignment into the {}, and use if-elseif-else to avoid goto.

And as for performance, I am running the test using hash table currently. The result of nginx+wrk test is quite promising (web object sizes range from 1KB~40KB; for 4reqs/conn, 4~10K concurrent connection, I am getting 12%~14% performance improvement :). 14reqs/conn nginx+wrk test is still running.

REVISION DETAIL
https://reviews.freebsd.org/D6689

EMAIL PREFERENCES
https://reviews.freebsd.org/settings/panel/emailpreferences/

To: sepherosa_gmail.com, rrs, glebius, gnn, bz, rwatson, #transport, hselasky, gallatin
Cc: freebsd-net-list

hselasky (Hans Petter Selasky)

2016-06-08 12:17:23 UTC

Permalink

hselasky added a comment.

Hi,

Were you able to test the performance using tcp_lro_queue_mbuf() ?

Better name for function?? tcp_lro_rx2() -> tcp_lro_rx_sub()

--HPS

REVISION DETAIL
https://reviews.freebsd.org/D6689

EMAIL PREFERENCES
https://reviews.freebsd.org/settings/panel/emailpreferences/

To: sepherosa_gmail.com, rrs, glebius, gnn, bz, rwatson, #transport, hselasky, gallatin
Cc: freebsd-net-list

hselasky (Hans Petter Selasky)

2016-06-08 13:29:07 UTC

Permalink

hselasky added a comment.

@sepherosa_gmail.com

Regarding performance. Is it possible to get the Hyper-V to sort the IP-packets before they enter the FreeBSD network stack in the VM?

REVISION DETAIL
https://reviews.freebsd.org/D6689

EMAIL PREFERENCES
https://reviews.freebsd.org/settings/panel/emailpreferences/

To: sepherosa_gmail.com, rrs, glebius, gnn, bz, rwatson, #transport, hselasky, gallatin
Cc: freebsd-net-list

sepherosa_gmail.com (Sepherosa Ziehau)

2016-06-13 01:39:43 UTC

Permalink

sepherosa_gmail.com (Sepherosa Ziehau)

2016-06-13 01:41:09 UTC

Permalink

sepherosa_gmail.com (Sepherosa Ziehau)

2016-06-14 02:10:25 UTC

Permalink

rrs (Randall Stewart)

2016-07-05 13:27:50 UTC

Permalink

rrs accepted this revision.
rrs added a comment.

These look fine and familiar ;-)

I think a hash table as an option to sorting is probably a good thing :D

REVISION DETAIL
https://reviews.freebsd.org/D6689

EMAIL PREFERENCES
https://reviews.freebsd.org/settings/panel/emailpreferences/

To: sepherosa_gmail.com, glebius, gnn, bz, rwatson, gallatin, hselasky, #transport, rrs
Cc: freebsd-net-list

sepherosa_gmail.com (Sepherosa Ziehau)

2016-07-06 01:43:49 UTC

Permalink

sepherosa_gmail.com added a comment.

Post by rrs (Randall Stewart)
These look fine and familiar ;-)
I think a hash table as an option to sorting is probably a good thing :D

Yeah, and I got pretty exiting performance improvement and noticeable latency reduction with both methods in Azure for nginx 1KB~40KB web objects and 4/14 reqs/conn :)). I will post the result once 128/256 mbuf queue depth for sorting method is done (512/1024/2048/4096 have been completed so far).

REVISION DETAIL
https://reviews.freebsd.org/D6689

EMAIL PREFERENCES
https://reviews.freebsd.org/settings/panel/emailpreferences/

To: sepherosa_gmail.com, glebius, gnn, bz, rwatson, gallatin, hselasky, #transport, rrs
Cc: freebsd-net-list

sepherosa_gmail.com (Sepherosa Ziehau)

2016-07-08 01:48:21 UTC

Permalink

sepherosa_gmail.com added a comment.

Post by sepherosa_gmail.com (Sepherosa Ziehau)

Post by rrs (Randall Stewart)
These look fine and familiar ;-)
I think a hash table as an option to sorting is probably a good thing :D

If you are interested, I have the measurement result posted here:
results <https://people.freebsd.org/~sephe/lro_pic/>

For nginx workload, both latency and performance are measured. As you can see both performance and latency got improved, almost for all nginx workloads we have tested so far.

Both methods provide significant improvement for some kinds of workload:
1KB obj 14reqs/conn <Loading Image...

>
8KB obj 14reqs/conn <Loading Image...

>

REVISION DETAIL
https://reviews.freebsd.org/D6689

EMAIL PREFERENCES
https://reviews.freebsd.org/settings/panel/emailpreferences/

To: sepherosa_gmail.com, glebius, gnn, bz, rwatson, gallatin, hselasky, #transport, rrs
Cc: freebsd-net-list

Phabricator

2016-08-02 06:37:12 UTC

Permalink

This revision was automatically updated to reflect the committed changes.
Closed by commit rS303656: tcp/lro: Implement hash table for LRO entries. (authored by sephe).

CHANGED PRIOR TO COMMIT
https://reviews.freebsd.org/D6689?vs=17569&id=18953#toc

REPOSITORY
rS FreeBSD src repository

CHANGES SINCE LAST UPDATE
https://reviews.freebsd.org/D6689?vs=17569&id=18953

REVISION DETAIL
https://reviews.freebsd.org/D6689

AFFECTED FILES
head/sys/netinet/tcp_lro.c
head/sys/netinet/tcp_lro.h

EMAIL PREFERENCES
https://reviews.freebsd.org/settings/panel/emailpreferences/

To: sepherosa_gmail.com, glebius, gnn, bz, rwatson, gallatin, hselasky, #transport, rrs
Cc: freebsd-net-list