[dpdk-dev] [PATCH 1/3] mbuf:add two TX offload flags and change three fields
Ananyev, Konstantin
konstantin.ananyev at intel.com
Sun Nov 30 15:50:32 CET 2014
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
More information about the dev
mailing list