Re: [dpdk-dev] [PATCH] ethdev: add VLAN attributes to ETH and VLAN items

[Thomas Monjalon <thomas@monjalon.net>]

02/10/2020 14:39, Maxime Leroy:
> Hi Dekel,
> 
> On Thu, Oct 1, 2020 at 8:49 PM Dekel Peled <dekelp@nvidia.com> wrote:
> >
> > From: Dekel Peled <dekelp@mellanox.com>
> >
> > This patch implements the change proposes in RFC [1], adding dedicated
> > fields to ETH and VLAN items structs, to clearly define the required
> > characteristic of a packet, and enable precise match criteria.

Please add more explanations, without relying on what is in RFC.
We need to clearly understand the motivation and why
this implementation is chosen.

If you correctly thread your patch with previous ones,
it should not be needed to mention RFC.

> >
> > [1] https://mails.dpdk.org/archives/dev/2020-August/177536.html
> >
> > Signed-off-by: Dekel Peled <dekelp@mellanox.com>
> 
> I am still wondering, why not using a new item 'NOT' for example to
> match only eth packet not tagged ?
> example: eth / not vlan. It's a more generic solution.
> 
> Here in this commit, we add a reference on VLAN fields on ethernet header.
> But tomorrow, we could do the same for mpls by adding mpls_exists in
> the eth item and so on.
> 
> In fact, we  have the same needs for IPv6 options. To match for
> example, ipv6 packet with no fragment option.
> With a NOT field, it can be easily done: > eth / ipv6 / no ipv6_frag.
> 
> Adding new fields 'item'_exists into eth and ipv6 do the jobs, but
> having a NOT attribute is a more generic solution.
> 
> It could address many other use cases like matching any udp packets
> that are not vxlan ( eth / ipv4 / vxlan / not udp),
> 
> Let me know what you think about that.

You're right Maxime, a NOT operator looks better for the user,
but it is expected to be very hard to implement and offload.