Page MenuHomeFreeBSD
Differential D12876

Only chain non-LRO mbufs when LRO is not possible
ClosedPublic
Actions

Authored by shurd on Oct 31 2017, 5:35 PM.
Tags
None
Referenced Files
Unknown Object (File)
Oct 7 2024, 5:50 AM
Unknown Object (File)
Oct 5 2024, 4:50 PM
Unknown Object (File)
Oct 2 2024, 5:58 PM
Unknown Object (File)
Oct 2 2024, 5:17 AM
Unknown Object (File)
Oct 2 2024, 1:23 AM
Unknown Object (File)
Sep 30 2024, 1:23 AM
Unknown Object (File)
Sep 27 2024, 4:11 AM
Unknown Object (File)
Sep 23 2024, 9:47 PM
View All 42 Files
Subscribers
ae
imp

Details

Reviewers
sbruno
cem
gallatin
bz
Commits
rS325487: Only chain non-LRO mbufs when LRO is not possible
Summary

Preserve packet order between tcp_lro_rx() and if_input() to avoid
creating extra corner cases. If no packets can be LROed, combine them
into one chain for submission via if_input(). If any packet can
potentially be LROed however, retain old behaviour and call if_input()
for each packet.

This should keep the 12% improvement for small packet forwarding intact,
but mostly avoids impacting the LRO case.

Test Plan

Ensure small packet forwarding case is still improved, check
that LRO performance isn't degraded.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 12338
Build 12626: arc lint + arc unit

Event Timeline

shurd created this revision.Oct 31 2017, 5:35 PM
Herald added subscribers: ae, imp. · View Herald TranscriptOct 31 2017, 5:35 PM
Harbormaster completed remote builds in B12336: Diff 34549.Oct 31 2017, 5:35 PM
shurd edited the summary of this revision. (Show Details)Oct 31 2017, 5:38 PM
sbruno accepted this revision.Oct 31 2017, 5:39 PM

Found while testing the iflib'd version of ixgbe(4).

This revision is now accepted and ready to land.Oct 31 2017, 5:39 PM
cem accepted this revision.Oct 31 2017, 5:45 PM

If no packets can be LROed, combine them into one change

Probably "chain"

sys/net/iflib.c
2476

It seems like a more accurate name might be check_ipforwarding_disabled. No?

2615–2616

Could combine these to drop a level of indentation

shurd updated this revision to Diff 34552.Oct 31 2017, 6:07 PM
shurd marked an inline comment as done.

Incorporate feedback.

This revision now requires review to proceed.Oct 31 2017, 6:07 PM
shurd added inline comments.Oct 31 2017, 6:08 PM
sys/net/iflib.c
2476

That's what it currently does, but the intent is to indicate if it's possible the packet could be LROed. I'm considering copying the TCP header checks into the function as well, but can't make a strong case for them at this point.

Added a comment to clarify.

cem added inline comments.Oct 31 2017, 6:11 PM
sys/net/iflib.c
2613

nit: parens aren't needed for this line

2614–2615

I was suggesting to combine the two lines above, not move the assignment into an if(). Like:

if (lro_possible && mf != NULL) {
    ifp->if_input(ifp, mf);
    ...
}
shurd updated this revision to Diff 34558.Oct 31 2017, 6:57 PM

Much better way of dropping a level of indentation. :-)

Harbormaster completed remote builds in B12344: Diff 34558.Oct 31 2017, 6:57 PM
shurd marked an inline comment as done.Oct 31 2017, 6:57 PM
shurd marked an inline comment as done.
cem accepted this revision.Oct 31 2017, 7:06 PM
This revision is now accepted and ready to land.Oct 31 2017, 7:06 PM
shurd updated this revision to Diff 34689.Nov 2 2017, 6:33 PM

Fix typo

This revision now requires review to proceed.Nov 2 2017, 6:33 PM
sbruno accepted this revision.Nov 6 2017, 3:53 PM
This revision is now accepted and ready to land.Nov 6 2017, 3:53 PM
shurd closed this revision.Nov 6 2017, 9:30 PM

committed as r325487

Revision Contents
Changeset List

PathSize
sys/
net/
70 lines

Diff 34552

View Options

sys/net/iflib.c

Loading...