The igb driver misses support for SCTP/TCP/UDP checksum offloading when using IPv6

[tuexen]

Summary

In the igb driver the support for checksum offloading for SCTP, TCP and UDP over
IPv6 is missing. The attached patch fixes it.
The code is #ifdef'ed similar to the code in ixgbe/if_ix.c.

Test Plan

Tested with interfaces using the 82576 and the 82580 chip.

[sbruno]

I am interested if any benchmarks via netperf/iperf have been done over ipv6 with this change? If not, lets see if Jeffrey (@intel) has the time to run a with/TSO6 & without/TSO6 test.

[jeffrey.e.pieper_intel.com]

Yes, I can run some tests, but probably not until next week.

[tuexen]

We did performance measurements for SCTP and could verify that SCTP checksum offloading for IPv6 wasn't working on the sending side.
On the receiving side, you can't control them for IPv4 and IPv6 independently. So they were on.

[sbruno]

Ok, this affects TCP as well. So, let's get a performance benchmark if possible via Jeffrey.

[tuexen]

Make sure that you are CPU limited or watch the CPU load. For SCTP the CPU overhead is substantial and we have stats counters for checking CRC offload.

[erj]

A couple notes on the patch inline, and one general comment:

• We're going to drop support for 9, so you don't need to keep those FreeBSD version checks for >=10 (or even >=8). At least for us, our plan is to only MFC things as far back as stable/10.

sys/dev/e1000/if_igb.c

1297 ↗

(On Diff #13013)

Your if statement only checks for 82576 and 82580 while there are other macs like i350, i354, i210, and i211 that support SCTP checksum offload as well. I'm trying to get a complete list, but maybe the check should be changed to just exclude 82575.

sys/dev/e1000/if_igb.h

298 ↗

(On Diff #13013)

You should use the updated CSUM_* flags in sys/mbuf.h instead of the backwards-compatible ones. Our internal version of ixgbe uses these new versions; 3.1.13-k in head is using them, too.

[tuexen]

Using the updated CSUM_* flags as requested

[tuexen]

Regarding the FreeBSD version checks for >=10:
I tried to be consistent with the rest of the file. If you want to get rid of them, that is fine, but it might make more sense to get rid of them consistently in the whole file then.

Regarding the checks for 82576 and 82580:
I tried to be consistent with the rest of the file. I don't have other cards in my lab for testing, so I included only these. I'm fine with adding support also for other cards or change the check to exclude 82575, but that has done consistently in the whole file. So it might be better to have that as a separate commit.

But I'll do what you suggest...

[erj]

In D5193#110543, @tuexen wrote:

Regarding the FreeBSD version checks for >=10:
I tried to be consistent with the rest of the file. If you want to get rid of them, that is fine, but it might make more sense to get rid of them consistently in the whole file then.

But I'll do what you suggest...

It might be a better idea to strip out the FreeBSD version ifdefs in a later commit, then. As it is, you're right, it works and is consistent.

Regarding the checks for 82576 and 82580:
I tried to be consistent with the rest of the file. I don't have other cards in my lab for testing, so I included only these. I'm fine with adding support also for other cards or change the check to exclude 82575, but that has done consistently in the whole file. So it might be better to have that as a separate commit.

I can see other places where the 82576/82580-only checks are made. I guess those could be changed in a separate commit, too.

sys/dev/e1000/if_igb.c

3957 ↗

(On Diff #13023)

These should use the new style CSUM_IP* flags, too.

[tuexen]

Consistently use the new CSUM_IP_* defines on FreeBSD 10 and higher. This still supports building for FreeBSD versions older than 10 for consistency with the rest of the file.

[tuexen]

Any comments?

[tuexen]

Do you have a list of NICs which support SCTP checksum offloading?

[tuexen]

CSUM_IP* flags are now used.

[sbruno]

sys/dev/e1000/if_igb.c

1304 ↗

(On Diff #13044)

Eric:

You commented that we should change the list of supported adapters to a list of unsupported adapters. Do you have a list of adapters that do *not* support this feature or should we "guess" that the 82575 is the only exception here?

[sbruno]

This was discussed in the Intel Community call. We agreed that this should be committed with an exclusion of the 82575. Do you have time to do this?

[tuexen]

Will update the patch and commit it early next week. Thanks for the feedback.

[tuexen]

Now using SCTP offloading on all controllers except 82575 as requested.

[tuexen]

Just base the diff at src/