On Fri, Apr 8, 2016 at 2:40 PM, Jesse Gross <je...@kernel.org> wrote: > On Thu, Apr 7, 2016 at 8:52 PM, Alexander Duyck > <alexander.du...@gmail.com> wrote: >> Just a thought. What if I replaced NETIF_F_TSO_FIXEDID with something >> that meant we could mange the IP ID like a NETIF_F_TSO_IPID_MANGLE >> (advice for better name welcome). Instead of the feature flag meaning >> we are going to transmit packets with a fixed ID it would mean we >> don't care about the ID and are free to mangle it as we see fit. The >> GSO type can retain the same meaning as far as that requiring the same >> ID for all, but the feature would mean we will take fixed and convert >> it to incrementing, or incrementing and convert it to fixed. > > I saw the new version of the code that you posted with this idea and > now that I understand it better, it seems like a reasonable choice to > me - it's nice that it is consistent with GRO and not tunnel specific. > It also makes behavior consistent across drivers in regards to > incrementing IDs in the default case, which was one of my concerns > from before. > > Maybe I missed it but I didn't see any checks for the DF bit being set > when we transmit a packet with NETIF_F_TSO_MANGLEID. Even if I am > comfortable mangling my IDs in the DF case, I don't think this would > ever extend to non-DF packets. In the documentation you noted that it > is the driver's responsibility to do this check but I couldn't find it > in either ixgbe or igb. It would also be nice if the core stack could > enforce it somehow as well rather than each driver.
Yeah I had glossed over that in the igb and ixgbe patches. A check is only really needed for the incrementing to non-incrementing case and I wasn't sure how common it was to have TCP with an IP header that didn't set the DF bit. In the case of the outer headers igb and ixgbe will increment the IP ID always so we don't have to worry about if DF is set of not there. For the inner headers I had fudged it a bit and didn't add the validation. If needed I can see about adding that shortly. - Alex
