TSO handling in IPv6 when ip options are present.

[hiren]

Summary

TSO is not feasible if there are ip options present. Which is okay but ip header
flags like DSCP/ECN should be allowed. TSO works fine in v4 case as ip options
and ip header fields are handled separately. In v6 case, we've munged them into
one in6p_outputopts. But we are incorrectly checking for inp6_options
(which is totally unused) to see if ip options are present.

We need to check in6p_outputopts instead of inp6_options. But because
in6p_outputopts combines both ip options and ip header fields, we need to rely
on ip6_optlen() which correctly reports ip options length to decide if ip
options are present and whether we can do TSO or not.

NOTE: This is totally untested. I just want to get the discussion started and get inputs from people who know how IPv6 works. :-)

[kbowling]

I found Microsoft's docs quite good https://msdn.microsoft.com/en-us/windows/hardware/drivers/network/offloading-the-segmentation-of-large-tcp-packets.

• Support replication of the IPv4 options, from the large packet, in each segment packet that the network interface card (NIC) generates.
• Support replication of the IPv6 extension header, from the large TCP packet, in each TCP segment packet.
• Support replication of TCP options in each TCP segment packet that the miniport driver generates.

@kmacy suspects there might be drivers/firmware that cannot handle this but we haven't gone and dug any examples of that up. If that is the case, we should disable TSO on any packet with IP options set as the current code does, but additionally check for a flag on the output interface indicating that incompatibility.

[hiren]

In D8737#180982, @kevin.bowling_kev009.com wrote:

[skip]

@kmacy suspects there might be drivers/firmware that cannot handle this but we haven't gone and dug any examples of that up. If that is the case, we should disable TSO on any packet with IP options set as the current code does, but additionally check for a flag on the output interface indicating that incompatibility.

Thanks for the pointer and this can be a good enhancement to how we do TSO right now. But I'd like to keep this review strictly for fixing the bug that exists.

[ae]

This looks correct to me.
Also, I'd like to note, that in IPSEC case ip6_output will ignore in6p_outputopts, because ip6_ipsec_output() will be called first. But this is not related to this change.

[bz]

Seems like this is a more conservative solution to avoid possible offload problems.

There's a couple of other things the came to my mind or I spotted in that code but for the sake of "just fixing" this looks right.

I think your commit message could point out more clearly that you are fixing two (independent) things.

[hiren]

In D8737#181056, @bz wrote:

Seems like this is a more conservative solution to avoid possible offload problems.

There's a couple of other things the came to my mind or I spotted in that code but for the sake of "just fixing" this looks right.

Right. This is fixing the *obvious* bug. Please let me know what else you have in mind and that way its logged for myself of someone else to pick up.

I think your commit message could point out more clearly that you are fixing two (independent) things.

This is only fixing one thing: incorrectly looking at in6p_options to decide whether to do TSO or not. Instead we should look at just ip options part of in6p_outputopts as done by ip6_optlen().

Let me know if I can make this any more clearer. Or if I am misunderstanding something.

[bz]

I guess it's just my way of seeing things:
(1) fixing the check on options which is wrong
(2) as a consequence fixing possible TSO problems that might currently occur.

They are tight together in that (2) may be a problem because of (1) but the end result is two things observed most likely in entirely different ways being more correct. (1) I can spot by staring at code and reading it, (2) I'd most likely observe due to packet traces, error counters, ... in a totally different (unconnected) way. This is why to me this is solving two problems (you could encounter each individually). Sorry for confusing; just my way of thinking. Go ahead and commit it!