Page MenuHomeFreeBSD

TSO inherently requires checksum offload. In the case where TSO is in use, force the checksum offload on as well for that packet.
ClosedPublic

Authored by shurd on Nov 1 2018, 1:41 PM.
Tags
None
Referenced Files
F82040165: D17801.id50412.diff
Wed, Apr 24, 9:46 PM
Unknown Object (File)
Mar 12 2024, 2:55 AM
Unknown Object (File)
Jan 21 2024, 2:46 PM
Unknown Object (File)
Jan 11 2024, 10:52 PM
Unknown Object (File)
Dec 22 2023, 11:17 PM
Unknown Object (File)
Jun 20 2023, 11:03 PM
Unknown Object (File)
Jun 20 2023, 9:40 PM
Unknown Object (File)
Jun 14 2023, 4:17 PM
Subscribers

Details

Summary

It appears that for IPv4, checksum offload was always enabled
whenever TSO was enabled. This no longer seems to be the case, so
handle this in iflib.

Test Plan

Intel has a test

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

This doesn't seem like the right fix.

sys/net/iflib.c
2997 ↗(On Diff #49882)

Shouldn't the block go up here in the TSO4 section?

3036 ↗(On Diff #49882)

I don't think this is the right place for this block?

3036 ↗(On Diff #49882)

Also, should we set both CSUM_IP_TCP and CSUM_IP?

So this is a step in the right direction, but isn't a complete fix for the issue. We also need to enable the ip_sum workaround in this case as well. Possibly we can just move that check to after the TX_OFFLOAD4 and TX_OFFLOAD6 checks?

sys/net/iflib.c
2976–2977 ↗(On Diff #49882)

Yea, we need to also fix this work around to zero out the ip_sum when doing CSUM_IP_TSO as well, otherwise hardware which requires this work around to behave properly will create invalid TSOs.

Move setting CSUM_IP_TCP into the correct block. Also set CSUM_IP.

Move zeroing the IP checksum to after the IS_TX_OFFLOAD4() block, since it can change
in there now.

Don't zero the IP checksum because of IFLIB_TSO_INIT_IP, only set IP len. Zeroing
the checksum is handled by the IFLIB_NEED_ZERO_CSUM flag. It looks like e1000 is the
only driver that uses IFLIB_TSO_INIT_IP, and it has IFLIB_NEED_ZERO_CSUM set.

Updating D17801: TSO inherently requires checksum offload. In the case where TSO is

in use, force the checksum offload on as well for that packet.

shurd marked 4 inline comments as done.Nov 2 2018, 9:17 PM

Actually, I think the reason we set NEED_CSUM_ZERO in some hardware is only because they need the IP sum zero'd during TSO, not because they need it for non-TSO packets....

If I fix the driver to enable IP checksums on TSOs even when CSUM_IP isn't set, *and* I enable IFLIB_TSO_INIT_IP, then this patch isn't necessary.

I'm leaning towards "just fix the drivers to enable the correct work arounds", rather than changing iflib.

Additionally, based on some testing, the only reason we were setting NEED_CSUM_ZERO on some devices was because the CSUM needed to be zero for TSOs, but not for regular packets.

Restore setting ip_csum to zero for TSO when IFLIB_TSO_INIT_IP is set.

This is an important part of the IFLIB_TSO_INIT_IP behaviour since some
Intel NICs only require the csum set to zero for TSO.

Updating D17801: TSO inherently requires checksum offload. In the case where TSO is

in use, force the checksum offload on as well for that packet.

This revision was not accepted when it landed; it landed in state Needs Review.Nov 14 2018, 3:23 PM
This revision was automatically updated to reflect the committed changes.