> -----Original Message----- > From: Ananyev, Konstantin > Sent: Sunday, November 30, 2014 10:51 PM > To: Olivier MATZ; 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 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;
A question, how to fill the mb->l2_len for IP in IP, IP in GRE tunneling packet that is no inner L2 header? What is the rule? Or you think it should be mb->l2_len = 0 + l4_tun_len; ?? > 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
