Page MenuHomeFreeBSD

e1000: fixes for lem(4) and em(4) TSO
ClosedPublic

Authored by kbowling on Jul 25 2023, 3:35 AM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Nov 18, 8:41 PM
Unknown Object (File)
Mon, Nov 18, 8:34 PM
Unknown Object (File)
Mon, Nov 18, 8:32 PM
Unknown Object (File)
Wed, Nov 13, 6:46 AM
Unknown Object (File)
Tue, Nov 12, 4:58 AM
Unknown Object (File)
Mon, Nov 11, 9:46 PM
Unknown Object (File)
Fri, Nov 8, 9:21 AM
Unknown Object (File)
Fri, Nov 8, 3:29 AM
Subscribers

Details

Summary

This patch fixes:

  • IPv6 TSO on em(4)
  • Limits TSO to IPv4 on lem(4) based on NetBSD and Linux
Test Plan

Tested on I219, 82541GI, 82546EB, VirtualBox lem(4), qemu lem(4), VMWare lem(4), VMware 82574L

TSO is not enabled on lem(4) or em(4) by default

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

Owners added a reviewer: Restricted Owners Package.Jul 25 2023, 3:35 AM
sys/dev/e1000/em_txrx.c
163 ↗(On Diff #125111)

I moved this here to avoid duplicating conditionals for IPv4 vs IPv6 but am open to style suggestion.

sys/dev/e1000/if_em.c
789

According to Linux lem(4) doesn't support IPv6 TSO, but they do enable TSO for IPv4 by default for (>82544, !82547). I can't see any particular reason while TSOv6 doesn't work here but intel has never been clear on all the bugs on the entire family of chips.

sys/dev/e1000/em_txrx.c
198 ↗(On Diff #125111)

I added this previously based on reading the SDM noting a slightly different behavior.. but I can't think of why it would matter in reality, because you wouldn't be doing a lower (IP) checksum on IPv6 anyway.

Don't try TSO on 10/100 for lem(4)/em(4)

I agree with all of the other changes here.

sys/dev/e1000/em_txrx.c
181 ↗(On Diff #125208)

Why add an extra line here?

183 ↗(On Diff #125208)

I like the addition of the field names to the comment here.

shurd added inline comments.
sys/dev/e1000/if_em.c
1418

Looks like SPEED_1000 (which == 1000 of course).

Also, do any types < igb_mac_min support 2500?

sys/dev/e1000/if_em.c
1418

I think the only device covered by this driver that could possibly support 2500 link speed would be i354, which would be above igb_mac_min.

kbowling edited the test plan for this revision. (Show Details)
kbowling added reviewers: marius, shurd.

@shurd I added some smarts to the TSO automask. Also added a sysctl for people that want to opt out of.

@shurd updated automask to not require the iflib internal change.

This revision was not accepted when it landed; it landed in state Needs Review.Jul 31 2023, 3:22 PM
This revision was automatically updated to reflect the committed changes.

rebase after committed changes