Page MenuHomeFreeBSD
Differential D29564

TCP LRO support for VLAN and VXLAN
ClosedPublic
Actions

Authored by • hselasky on Apr 2 2021, 10:37 PM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Sep 8, 4:03 PM
Unknown Object (File)
Thu, Aug 20, 7:21 PM
Unknown Object (File)
Wed, Aug 19, 11:56 PM
Unknown Object (File)
Sun, Aug 16, 1:00 AM
Unknown Object (File)
Thu, Aug 13, 4:13 AM
Unknown Object (File)
Wed, Aug 12, 7:33 PM
Unknown Object (File)
Aug 11 2026, 10:20 PM
Unknown Object (File)
Aug 11 2026, 9:17 PM
View All Files
Subscribers
gallatin
kbowling
melifaro
rrs

Details

Reviewers
kib
rrs
Group Reviewers
transport
Commits
rGe31579b8558d: Add TCP LRO support for VLAN and VxLAN.
rGa9b66dbd9169: Allow the tcp_lro_flush_all() function to be called when the control
rG9ca874cf740e: Add TCP LRO support for VLAN and VxLAN.
Summary
  • Speedup header data comparison. Don't compare field by field, but instead use unsigned long.
  • Make smaller functions doing one thing at a time instead of big functions with lots of repeated code.
  • Try to refactor the TCP ACK compression code to be more readable. Predict number of ACKs which needs compression.
  • Use sbintime() for all time-keeping.
  • Try to shrink the size of the LRO entry, because it is frequently zeroed.

MFC after: 1 week
Sponsored by: Mellanox Technologies // NVIDIA Networking

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

• hselasky created this revision.Apr 2 2021, 10:37 PM
Herald added a reviewer: transport. · View Herald TranscriptApr 2 2021, 10:37 PM
Herald added a subscriber: melifaro. · View Herald Transcript
• hselasky requested review of this revision.Apr 2 2021, 10:37 PM
• hselasky updated this revision to Diff 86779.Apr 3 2021, 9:44 AM

Minor bugfix.

• hselasky updated this revision to Diff 86781.Apr 3 2021, 10:18 AM

Fix UDP checksumming, when there is no UDP checksum. Optimise other checksumming.

• hselasky added a comment.Apr 3 2021, 10:35 AM

Test OK: VXLAN
Test OK: NO-VLAN
Test OK: VLAN
TODO: PRIO-TAGGED

• hselasky added a reviewer: kib.Apr 3 2021, 10:40 AM
• hselasky updated this revision to Diff 86866.Apr 5 2021, 8:08 PM

Add fixes for BBR/RACK.

Incorporate a fix for bad assert / panic.

gallatin added a subscriber: gallatin.Apr 7 2021, 3:03 PM
gallatin added a comment.Apr 7 2021, 3:07 PM

I may be reading this wrong, but for mbuf compression, you now seem to be appending a chain of packets, then coalescing them. The original design choice of coalescing as the new packet was encountered was intentional. The idea is that you want to operate on and free the incoming mbuf while it is hot in cache. By chaining mbufs and dealing with them later, they're likely to be cold in cache, so you're likely encounter additional cache misses on every mbuf in the chain as you copy out data and free it.

• hselasky added a comment.Apr 7 2021, 3:19 PM

Hi Drew,

For sorted LRO the mbufs are still hot in the CPU cache. Remember we are flushing all the entries after each new flowid value!

--HPS

gallatin added a comment.Apr 7 2021, 7:26 PM
In D29564#664209, @hselasky wrote:

Hi Drew,

For sorted LRO the mbufs are still hot in the CPU cache. Remember we are flushing all the entries after each new flowid value!

--HPS

Remember that very few drivers use sorted LRO. Iflib based drivers do not, which is the vast majority of network traffic in FreeBSD.

(I tried to add sorted LRO to iflib, but wound up causing a small packet forwarding regression, so I abandoned the effort)

• hselasky added a comment.Apr 8 2021, 7:56 AM

The advantage with my approach is:

  1. Lookup the INP only once.
  2. Less error handling.

Won't the same problem happen looking up INP's from 1000's of connections, that the CPU runs out of cache?

Then it is better to only process packets for one INP at a time?

• hselasky added a comment.Apr 8 2021, 11:46 AM

@gallatin: Who can test this patch with non-sorted-LRO and TCPHPTS, to verify performance improvements or losses?

• hselasky updated this revision to Diff 87022.Apr 8 2021, 12:12 PM
• hselasky retitled this revision from Work in progress TCP LRO support for VLAN and VXLAN. to TCP LRO support for VLAN and VXLAN.

Incorporate changes from Randall.
Some minor style and spelling fixes.

• hselasky updated this revision to Diff 87035.Apr 8 2021, 2:13 PM

Include more changes from Randall.

• hselasky updated this revision to Diff 87036.Apr 8 2021, 2:18 PM

Fix a typo in comment.

• hselasky updated this revision to Diff 87441.Apr 14 2021, 1:52 PM

Add minor fix from Randall: Don't try to compress TCP packets with data payload.

kib added a comment.Apr 19 2021, 10:45 AM

Isn't it a wrong layering?

Why not add lro hooks to the input processing code in vlan and vxlan interfaces, instead of coding incomplete understanding of the encapsulation formats directly into TCP stack? Let drivers handle all details of decapsulation and then TCP would see inner packets. I do not think that we gain _a lot_ by coalescing encapsulated packets, while this definitely breaks layering and might also break some features like firewalls (not sure about this).

• hselasky added a comment.Apr 19 2021, 10:59 AM

Isn't it a wrong layering?

No, its an acceleration feature like TSO is.

LRO is disabled for routing, when IP forwarding is enabled.

Why not add lro hooks to the input processing code in vlan and vxlan interfaces, instead of coding incomplete understanding of the encapsulation formats directly into TCP stack? Let drivers handle all details of decapsulation and then TCP would see inner packets. I do not think that we gain _a lot_ by coalescing encapsulated packets, while this definitely breaks layering and might also break some features like firewalls (not sure about this).

Because that requires more CPU. Then you have to sort the incoming data multiple times, instead of only once. Also how would you implement lookahead of multiple packets for the same destination?

LRO is not for all encapsulation types. When encryption is used, LRO is not practical, except for TLS over TCP.

--HPS

rrs accepted this revision.Apr 19 2021, 11:35 AM
rrs added a subscriber: rrs.

I have done a complete set of regression tests in the NF lab on this patch. They
now all pass 100%. We have also take this patch to live NF machines and
done some performance analysis on 100G machines running 100% TLS.
The overall result is that you lose about 1% CPU in the old path and
you gain 2% CPU in the compressed ack path. So overall this is
a slight win or push. This is more than acceptable considering the new flexibility it
gives us!

This revision is now accepted and ready to land.Apr 19 2021, 11:35 AM
kbowling added a subscriber: kbowling.Apr 19 2021, 9:59 PM
In D29564#669391, @kib wrote:

Isn't it a wrong layering?

Why not add lro hooks to the input processing code in vlan and vxlan interfaces, instead of coding incomplete understanding of the encapsulation formats directly into TCP stack? Let drivers handle all details of decapsulation and then TCP would see inner packets. I do not think that we gain _a lot_ by coalescing encapsulated packets, while this definitely breaks layering and might also break some features like firewalls (not sure about this).

I don't know the details well enough to form a strong option on this but I am wondering if it would be possible to generalize in the future to a dispatch table or somehow invert control back into the tunnel implementations. In particular I think this would be useful for other lite IP tunnels like GENEVE, gif(4), gre(4), and maybe the unfinished vpc(4) stack. My concern, if any, would be stated as let's not build ourselves into a corner for just VXLAN. If there is an opening for future tunnels, that is good.

• hselasky added a comment.Apr 20 2021, 8:42 AM

would be stated as let's not build ourselves into a corner for just VXLAN

The new code is generic and can easily be extended for GRE simply by adding a new LRO_TYPE_XXX in the parser. Then the rest will be automagic.

Closed by commit rG9ca874cf740e: Add TCP LRO support for VLAN and VxLAN. (authored by • hselasky). · Explain WhyApr 20 2021, 11:42 AM
This revision was automatically updated to reflect the committed changes.
• hselasky added a commit: rG9ca874cf740e: Add TCP LRO support for VLAN and VxLAN..
• hselasky added a commit: rGa9b66dbd9169: Allow the tcp_lro_flush_all() function to be called when the control.Apr 24 2021, 10:28 AM
• hselasky mentioned this in D29509: Remove unused write only "m_prev_last" field in TCP LRO code..Jun 1 2021, 10:56 AM
• hselasky added a commit: rGe31579b8558d: Add TCP LRO support for VLAN and VxLAN..Jun 2 2021, 12:25 PM

Revision Contents
Changeset List

PathSize
sys/
netinet/
124 lines
2282 lines
5 lines
5 lines
sys/
7 lines
2 lines

Diff 87790

View Options

sys/netinet/tcp_lro.h

Loading...
View Options

sys/netinet/tcp_lro.c

Loading...
View Options

sys/netinet/tcp_subr.c

Loading...
View Options

sys/netinet/tcp_var.h

Loading...
View Options

sys/sys/mbuf.h

Loading...
View Options

sys/sys/param.h

Loading...