Hi Konstantin, On 11/27/2014 06:01 PM, Ananyev, Konstantin wrote: >> Yes, I think it could be done that way too. >> Though I still prefer to keep l4tun_len - it makes things a bit cleaner (at >> least to me). >> After all we do have space for it in mbuf's tx_offload. > > As one more thing in favour of separate l4tun_len field: > l2_len is 7 bit long, so in theory it might be not enough, as for FVL: > 12:18 L4TUNLEN L4 Tunneling Length (Teredo / GRE header / VXLAN header) > defined in Words.
The l2_len field is 7 bits long because it was mapped to ixgbe hardware. If it's not enough (although I'm not sure it's possible to have a header larger than 128 bytes in this case), it's probably because we should not have been doing that. Maybe these generic fields should be generic :) If it's not enough, what about changing l2_len to 8 bits? Often in the recent discussions, I see as an argument "fortville needs this so we need to add it in the mbuf". I agree that currently fortville is the only hardware supported for the new features, so it can make sense to act like this, but we have to accept to come back to this API in the future if it makes less sense with other drivers. Also, application developers can be annoyed to see that the mbuf flags and meta data are just duplicating information that is already present in the packet. About the l4tun_len, it's another field the application has to fill, but it's maybe cleaner. I just wanted to clarify why I'm discussing these points. Regards, Olivier