Implement L2 lookup caching for TCP and UDP
ClosedPublic
Actions

Authored by • karels on May 8 2016, 5:49 PM.

Details

Reviewers

jtl
gnn
bz
cem

Group Reviewers

transport

Commits

rS301217: This change re-adds L2 caching for TCP and UDP, as originally added in D4306

Summary

This change re-adds L2 caching for TCP and UDP, as originally added in D4306
but removed due to other changes in the system. Restore the llentry pointer
to the "struct route", and use it to cache the L2 lookup (ARP or ND6) as
appropriate.

Test Plan

This change is based on production code in Sidewinder, aka McAfee Firewall
Enterprise. Testing has included changing of MAC addresses on neighboring
systems and verifying that connections continue to work.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 4005
Build 4048: arc lint + arc unit

Event Timeline

• karels updated this revision to Diff 16032.May 8 2016, 5:49 PM

• karels retitled this revision from to Implement L2 lookup caching for TCP and UDP.

• karels updated this object.

• karels edited the test plan for this revision. (Show Details)

• karels added reviewers: gnn, bz, jtl.

Herald added a subscriber: imp. · View Herald TranscriptMay 8 2016, 5:49 PM

• karels added a reviewer: transport.May 8 2016, 5:53 PM

Hello? Is anyone looking at this?

Update to current HEAD; fix to work with flowtable

I've tested this in a two host test using iperf. https://github.com/gvnn3/netperf/tree/master/VANILLA/Tests/iperf-twohost-nooffload

and not seen any issues on 10G Chelsio hardware.

This revision is now accepted and ready to land.Jun 2 2016, 2:18 AM

Closed by commit rS301217: This change re-adds L2 caching for TCP and UDP, as originally added in D4306 (authored by gnn). · Explain WhyJun 2 2016, 5:51 PM

This revision was automatically updated to reflect the committed changes.

Just came across this commit tracing other problems ...

head/sys/netinet6/in6_pcb.c
836 ↗	(On Diff #17229)	Question (post commit review :) Why is this inp_route and not inp_route6? I know the heads of them are identical and it's a union, yet ... for clarity if nothing else?
853 ↗	(On Diff #17229)	Same here, inp_route6?

Herald added a reviewer: cem. · View Herald TranscriptAug 27 2018, 9:41 PM

Herald added a subscriber: ae. · View Herald Transcript

You are right, inp_route6 would be better. I'm sure I copied and pasted, then didn't make as many changes as I should have. I will happily approve a review to make that change if you test that it compiles :-).

bz mentioned this in rS338448: As discussed in D6262 post-commit review, change inp_route to.Sep 3 2018, 10:13 PM