[Differential] D7415: tcp/lro: If timestamps mismatch or it's a FIN, force flush.

[sepherosa_gmail.com (Sepherosa Ziehau)]

sepherosa_gmail.com created this revision.
sepherosa_gmail.com added reviewers: rrs, gallatin, hselasky, np, glebius.
sepherosa_gmail.com added a subscriber: freebsd-net-list.
Herald added a reviewer: transport.

REVISION SUMMARY
This keeps the segments/ACK/FIN delivery order.

Before this patch, it was observed: if A sent FIN immediately after
an ACK, B would deliver FIN first to the TCP stack, then the ACK.
This out-of-order delivery causes one unnecessary ACK sent from B.

Obtained from: gallatin, rrs

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

AFFECTED FILES
sys/netinet/tcp_lro.c

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

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

[sepherosa_gmail.com (Sepherosa Ziehau)]

sepherosa_gmail.com added reviewers: hiren, bz.

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

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

To: sepherosa_gmail.com, rrs, gallatin, hselasky, np, glebius, #transport, hiren, bz
Cc: freebsd-net-list
_______________________________________________
freeb...@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "freebsd-net...@freebsd.org"

[hselasky (Hans Petter Selasky)]

hselasky added inline comments.

INLINE COMMENTS

> tcp_lro.c:852
> }
>
> /* Try to find an empty slot. */

Should there be added:

if (force_flush)

return (TCP_LRO_CANNOT);

If no match is found in the list?

[sepherosa_gmail.com (Sepherosa Ziehau)]

sepherosa_gmail.com added inline comments.

INLINE COMMENTS

> hselasky wrote in tcp_lro.c:852

> Should there be added:
>
> if (force_flush)
>
> return (TCP_LRO_CANNOT);
>
> If no match is found in the list?

Ah, yeah, it was in the original patch from rrs, got lost by me :P

[sepherosa_gmail.com (Sepherosa Ziehau)]

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

Don't create entry for segments cant be further aggregated/delayed. Noticed-by: hps

CHANGES SINCE LAST UPDATE
https://reviews.freebsd.org/D7415?vs=19031&id=19032

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

AFFECTED FILES
sys/netinet/tcp_lro.c

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

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

[hselasky (Hans Petter Selasky)]

hselasky accepted this revision.
hselasky added a comment.


Looks good to me.

--HPS

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

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

To: sepherosa_gmail.com, rrs, gallatin, np, glebius, hiren, bz, #transport, hselasky

[gallatin (Andrew Gallatin)]

gallatin accepted this revision.

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

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

To: sepherosa_gmail.com, rrs, np, glebius, hiren, bz, #transport, hselasky, gallatin

[Phabricator]

This revision was automatically updated to reflect the committed changes.
Closed by commit rS303766: tcp/lro: If timestamps mismatch or it's a FIN, force flush. (authored by sephe).

CHANGED PRIOR TO COMMIT
https://reviews.freebsd.org/D7415?vs=19032&id=19063#toc

REPOSITORY
rS FreeBSD src repository

CHANGES SINCE LAST UPDATE
https://reviews.freebsd.org/D7415?vs=19032&id=19063

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

AFFECTED FILES
head/sys/netinet/tcp_lro.c