Hi Olver, > -----Original Message----- > From: Olivier MATZ [mailto:olivier.matz at 6wind.com] > Sent: Friday, November 28, 2014 10:46 AM > To: Ananyev, Konstantin; Liu, Jijiang > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH 1/3] mbuf:add two TX offload flags and change > three fields > > 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.
After another thought, I think that the way you proposed is a better one. I gives us more flexibility: let's say for now we'll keep both l2_len and outer_l2_len as 7 bit fields, and upper layer would have to: mb->l2_len = eth_hdr_in + vxlan_hdr; for VXLAN packets. Then if in the future, we'll realise that 7 bit is not enough we can either: - increase size of l2_len and outer_l2_len - introduce new field (l4tun_len as we planned now) In both cases backward compatibility wouldn't be affected. >From other side - if we'' introduce a new field now (l4tun_len), it would be >very hard to get rid of it in the future. So, I think we'd better follow your approach here. Thanks Konstantin > > Regards, > Olivier
