On Mon, Apr 4, 2016 at 9:32 PM, Herbert Xu <herb...@gondor.apana.org.au> wrote: > On Mon, Apr 04, 2016 at 09:26:55PM -0700, Alexander Duyck wrote: >> >> The problem is right now we are mangling the IP ID for outer headers >> on tunnels. We end up totally ignoring the delta between the values >> so if you have two flows that get interleaved over the same tunnel GRO >> will currently mash the IP IDs for the two tunnels so that they end up >> overlapping. > > Then it should be fixed. I never reviewed those patches or I would > have objected at the time.
The problem is what you are proposing is much larger than I am comfortable proposing for a net patch so I will probably go back to targeting net-next. >> The reason why I keep referencing RFC 6864 is because it specifies >> that the IP ID field must not be read if the DF bit is set, and that >> if we are manipulating headers we can handle the IP ID as though we >> are the transmitting station. What this means is that if DF is not >> set we have to have unique values per packet, otherwise we can ignore >> the values if DF is set. > > As I said GRO itself should not be visible. The fact that it is > for tunnels is a bug. Right. But the fact that we are even trying to get reliable data out of IP ID is also a bug. The fact is the IP ID isn't going to be linear in the case of tunnels. There is a good liklihood that the IP ID will jump around if we are doing encapsulation using a third party device such as a switch. That was one of the reasons why my first implementation just completely ignored the IP ID in the case that the DF bit is set. Honestly I am leaning more toward taking the approach of going back to that implementation and adding a sysctl that would let you disable it. Maybe something like net.ipv4.gro.rfc6864 since that is the RFC that spells out that we should treat IP ID as a floating input/output if DF is set. Basically we need to be able to do that if the GSO partial code is going to work. >> The question I would have is what are you really losing with increment >> from 0 versus fixed 0? From what I see it is essentially just garbage >> in/garbage out. > > GRO is meant to be lossless, that is, you should not be able to > detect its presence from the outside. If you lose information then > you're breaking this rule and people will soon start asking for it > to be disabled in various situations. Right. But in this case we technically aren't losing information. That is the thing I have been trying to point out with RFC 6864. It calls out that you MUST not read IP ID if the DF bit is set. > I'm not against doing this per se but it should not be part of the > default configuration. I disagree I think it will have to be part of the default configuration. The problem is the IP ID is quickly becoming meaningless. When you consider that a 40Gb/s link can wrap the IP ID value nearly 50 times a second using a 1500 MTU the IP ID field should just be ignored anyway because you cannot guarantee that it will be unique without limiting the Tx window size. That was the whole point of RFC6864. Basically the IP ID field is so small that as we push into the higher speeds you cannot guarantee that the field will have any meaning so for any case where you don't need to use it you shouldn't because it will likely not provide enough useful data. - Alex
