|2769 ↗||(On Diff #42946)|
It looks like these macros are always tested with IS_TSO4()... a combined macro would be better.
IS_TX_OFFLOAD4() or something checking CSUM_IP_TSO and CSUM_IP_TCP together.
|2872 ↗||(On Diff #42946)|
This can be moved inside the previous block so we're not relying on compiler magic to DTRT (same for below).
|2924 ↗||(On Diff #42946)|
This looks like an accident... this bit is already set or we wouldn't be here.
Looks good, but not forcing CSUM_TCP_IPV6 for TSO6 may need a closer look.
Non-TCP TSO won't need the tcp header and sequence number information though. The IS_TSO4() block below fills in the generic information.
Did you confirm that the CSUM_TCP_IPV6 csum flag *is* set on -current? Not forcing CSUM_TCP_IPV6 seems like a significant change here. I'll take a look at what "the rest of the flow requires it" means in this context.
I think my comment there was really asking if iflib is supposed to support non-TCP TSO.
Does FreeBSD mean TSO to be "TCP segmentation offload" like in the ifconfig(8) man page, or "Transmit segmentation offload" like in the Intel datasheets?
Err, looking at this again, I shouldn't have reverted it. I think yesterday I read the flag as being TSO instead of TCP and agreed that it was redundant.
FreeBSD does not support anything but TCP TSO. So checking for IPPROTO_TCP is redundant, and can be skipped.
And setting it is safe, because you've already nuked the checksum that would have been there if it had not been set.
Speaking of that.. It would be nice if FreeBSD handled TSO pseudo hdr checksums based on a length of 0 like windows, since that's what most ASICs want. At Myricom, we had a special mode in the NIC to handle linux/freebsd TSO where we subtracted out the len from the csum we were passed, but most NICs don't have that. I suppose a change like that might be too disruptive, but it would allow cleaning up a ton of places where drivers call in*_cksum_pseudo() to get the checksum in the form they want.