[dpdk-dev] [RFC] ethdev: add fragment attribute to IPv6 item

[dpdk-dev] [RFC] ethdev: add fragment attribute to IPv6 item

[Dekel Peled]

Using the current implementation of DPDK, an application cannot
match on fragmented/non-fragmented IPv6 packets in a simple way.

In current implementation:
IPv6 header doesn't contain information regarding the packet
fragmentation.
Fragmented IPv6 packets contain a dedicated extension header, as
detailed in RFC [1], which is not yet supported in rte_flow.
Non-fragmented packets don't contain the fragment extension header.
For an application to match on non-fragmented IPv6 packets, the
current implementation doesn't provide a suitable solution.
Matching on the Next Header field is not sufficient, since additional
extension headers might be present in the same packet.
To match on fragmented IPv6 packets, the same difficulty exists.

Proposed update:
An additional value will be added to IPv6 header struct.
This value will contain the fragmentation attribute of the packet,
providing simple means for identification of fragmented and
non-fragmented packets.

This update changes ABI, and is proposed for the 20.11 LTS version.

[1] http://mails.dpdk.org/archives/dev/2020-March/160255.html

Signed-off-by: Dekel Peled <dekelp@mellanox.com>
---
 lib/librte_ethdev/rte_flow.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
index b0e4199..3bc8ce1 100644
--- a/lib/librte_ethdev/rte_flow.h
+++ b/lib/librte_ethdev/rte_flow.h
@@ -787,6 +787,8 @@ struct rte_flow_item_ipv4 {
  */
 struct rte_flow_item_ipv6 {
 	struct rte_ipv6_hdr hdr; /**< IPv6 header definition. */
+	uint32_t is_frag:1; /**< Is IPv6 packet fragmented/non-fragmented. */
+	uint32_t reserved:31; /**< Reserved, must be zero. */
 };
 
 /** Default mask for RTE_FLOW_ITEM_TYPE_IPV6. */
-- 
1.8.3.1

Re: [dpdk-dev] [RFC] ethdev: add fragment attribute to IPv6 item

[Stephen Hemminger]

On Sun, 31 May 2020 17:43:29 +0300
Dekel Peled <dekelp@mellanox.com> wrote:

> Using the current implementation of DPDK, an application cannot
> match on fragmented/non-fragmented IPv6 packets in a simple way.
> 
> In current implementation:
> IPv6 header doesn't contain information regarding the packet
> fragmentation.
> Fragmented IPv6 packets contain a dedicated extension header, as
> detailed in RFC [1], which is not yet supported in rte_flow.
> Non-fragmented packets don't contain the fragment extension header.
> For an application to match on non-fragmented IPv6 packets, the
> current implementation doesn't provide a suitable solution.
> Matching on the Next Header field is not sufficient, since additional
> extension headers might be present in the same packet.
> To match on fragmented IPv6 packets, the same difficulty exists.
> 
> Proposed update:
> An additional value will be added to IPv6 header struct.
> This value will contain the fragmentation attribute of the packet,
> providing simple means for identification of fragmented and
> non-fragmented packets.
> 
> This update changes ABI, and is proposed for the 20.11 LTS version.
> 
> [1] http://mails.dpdk.org/archives/dev/2020-March/160255.html
> 
> Signed-off-by: Dekel Peled <dekelp@mellanox.com>
> ---
>  lib/librte_ethdev/rte_flow.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
> index b0e4199..3bc8ce1 100644
> --- a/lib/librte_ethdev/rte_flow.h
> +++ b/lib/librte_ethdev/rte_flow.h
> @@ -787,6 +787,8 @@ struct rte_flow_item_ipv4 {
>   */
>  struct rte_flow_item_ipv6 {
>  	struct rte_ipv6_hdr hdr; /**< IPv6 header definition. */
> +	uint32_t is_frag:1; /**< Is IPv6 packet fragmented/non-fragmented. */
> +	uint32_t reserved:31; /**< Reserved, must be zero. */
>  };

You can't do this in the 20.08 release it would be an ABI breakage.

Re: [dpdk-dev] [RFC] ethdev: add fragment attribute to IPv6 item

[Dekel Peled]

Thanks, PSB.

> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Monday, June 1, 2020 8:39 AM
> To: Dekel Peled <dekelp@mellanox.com>
> Cc: ferruh.yigit@intel.com; arybchenko@solarflare.com; Ori Kam
> <orika@mellanox.com>; john.mcnamara@intel.com;
> marko.kovacevic@intel.com; Asaf Penso <asafp@mellanox.com>; Matan
> Azrad <matan@mellanox.com>; Eli Britstein <elibr@mellanox.com>;
> dev@dpdk.org
> Subject: Re: [dpdk-dev] [RFC] ethdev: add fragment attribute to IPv6 item
> 
> On Sun, 31 May 2020 17:43:29 +0300
> Dekel Peled <dekelp@mellanox.com> wrote:
> 
> > Using the current implementation of DPDK, an application cannot match
> > on fragmented/non-fragmented IPv6 packets in a simple way.
> >
> > In current implementation:
> > IPv6 header doesn't contain information regarding the packet
> > fragmentation.
> > Fragmented IPv6 packets contain a dedicated extension header, as
> > detailed in RFC [1], which is not yet supported in rte_flow.
> > Non-fragmented packets don't contain the fragment extension header.
> > For an application to match on non-fragmented IPv6 packets, the
> > current implementation doesn't provide a suitable solution.
> > Matching on the Next Header field is not sufficient, since additional
> > extension headers might be present in the same packet.
> > To match on fragmented IPv6 packets, the same difficulty exists.
> >
> > Proposed update:
> > An additional value will be added to IPv6 header struct.
> > This value will contain the fragmentation attribute of the packet,
> > providing simple means for identification of fragmented and
> > non-fragmented packets.
> >
> > This update changes ABI, and is proposed for the 20.11 LTS version.
> >
> > [1]
> >
> https://eur03.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmails
> > .dpdk.org%2Farchives%2Fdev%2F2020-
> March%2F160255.html&amp;data=02%7C01
> >
> %7Cdekelp%40mellanox.com%7C9ee87004dc3943b945c908d805ee0bcc%7Ca
> 652971c
> >
> 7d2e4d9ba6a4d149256f461b%7C0%7C0%7C637265867256841029&amp;sdata
> =rf1zYz
> >
> fNLGdqayXLHffO%2FrM%2FeX5op6KO91RDKq%2BYk3Q%3D&amp;reserved
> =0
> >
> > Signed-off-by: Dekel Peled <dekelp@mellanox.com>
> > ---
> >  lib/librte_ethdev/rte_flow.h | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/lib/librte_ethdev/rte_flow.h
> > b/lib/librte_ethdev/rte_flow.h index b0e4199..3bc8ce1 100644
> > --- a/lib/librte_ethdev/rte_flow.h
> > +++ b/lib/librte_ethdev/rte_flow.h
> > @@ -787,6 +787,8 @@ struct rte_flow_item_ipv4 {
> >   */
> >  struct rte_flow_item_ipv6 {
> >  	struct rte_ipv6_hdr hdr; /**< IPv6 header definition. */
> > +	uint32_t is_frag:1; /**< Is IPv6 packet fragmented/non-fragmented.
> */
> > +	uint32_t reserved:31; /**< Reserved, must be zero. */
> >  };
> 
> You can't do this in the 20.08 release it would be an ABI breakage.
Please see above, I noted in the commit log that this is proposed for 20.11.

Re: [dpdk-dev] [RFC] ethdev: add fragment attribute to IPv6 item

[Andrew Rybchenko]

On 5/31/20 5:43 PM, Dekel Peled wrote:
> Using the current implementation of DPDK, an application cannot
> match on fragmented/non-fragmented IPv6 packets in a simple way.
> 
> In current implementation:
> IPv6 header doesn't contain information regarding the packet
> fragmentation.
> Fragmented IPv6 packets contain a dedicated extension header, as
> detailed in RFC [1], which is not yet supported in rte_flow.
> Non-fragmented packets don't contain the fragment extension header.
> For an application to match on non-fragmented IPv6 packets, the
> current implementation doesn't provide a suitable solution.
> Matching on the Next Header field is not sufficient, since additional
> extension headers might be present in the same packet.
> To match on fragmented IPv6 packets, the same difficulty exists.
> 
> Proposed update:
> An additional value will be added to IPv6 header struct.
> This value will contain the fragmentation attribute of the packet,
> providing simple means for identification of fragmented and
> non-fragmented packets.
> 
> This update changes ABI, and is proposed for the 20.11 LTS version.
> 
> [1] http://mails.dpdk.org/archives/dev/2020-March/160255.html
> 
> Signed-off-by: Dekel Peled <dekelp@mellanox.com>
> ---
>  lib/librte_ethdev/rte_flow.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
> index b0e4199..3bc8ce1 100644
> --- a/lib/librte_ethdev/rte_flow.h
> +++ b/lib/librte_ethdev/rte_flow.h
> @@ -787,6 +787,8 @@ struct rte_flow_item_ipv4 {
>   */
>  struct rte_flow_item_ipv6 {
>  	struct rte_ipv6_hdr hdr; /**< IPv6 header definition. */
> +	uint32_t is_frag:1; /**< Is IPv6 packet fragmented/non-fragmented. */
> +	uint32_t reserved:31; /**< Reserved, must be zero. */

The solution is simple, but hardly generic and adds an
example for the future extensions. I doubt that it is a
right way to go.

May be we should add 256-bit string with one bit for each
IP protocol number and apply it to extension headers only?
If bit A is set in the mask:
 - if bit A is set in spec as well, extension header with
   IP protocol (1 << A) number must present
 - if bit A is clear in spec, extension header with
   IP protocol (1 << A) number must absent
If bit is clear in the mask, corresponding extension header
may present and may absent (i.e. don't care).

The RFC indirectly touches IPv6 proto (next header) matching
logic.

If logic used in ETH+VLAN is applied on IPv6 as well, it would
make pattern specification and handling complicated. E.g.:
  eth / ipv6 / udp / end
should match UDP over IPv6 without any extension headers only.

And how to specify UPD over IPv6 regardless extension headers?
  eth / ipv6 / ipv6_ext / udp / end
with a convention that ipv6_ext is optional if spec and mask
are NULL (or mask is empty).

I'm wondering if any driver treats it this way?

I agree that the problem really comes when we'd like match
IPv6 frags or even worse not fragments.

Two patterns for fragments:
  eth / ipv6 (proto=FRAGMENT) / end
  eth / ipv6 / ipv6_ext (next_hdr=FRAGMENT) / end

Any sensible solution for not-fragments with any other
extension headers?

INVERT exists, but hardly useful, since it simply says
that patches which do not match pattern without INVERT
matches the pattern with INVERT and
  invert / eth / ipv6 (proto=FRAGMENT) / end
will match ARP, IPv4, IPv6 with an extension header before
fragment header and so on.

Bit string suggested above will allow to match:
 - UDP over IPv6 with any extension headers:
    eth / ipv6 (ext_hdrs mask empty) / udp / end
 - UDP over IPv6 without any extension headers:
    eth / ipv6 (ext_hdrs mask full, spec empty) / udp / end
 - UDP over IPv6 without fragment header:
    eth / ipv6 (ext.spec & ~FRAGMENT, ext.mask | FRAGMENT) / udp / end
 - UDP over IPv6 with fragment header
    eth / ipv6 (ext.spec | FRAGMENT, ext.mask | FRAGMENT) / udp / end

where FRAGMENT is 1 << IPPROTO_FRAGMENT.

Above I intentionally keep 'proto' unspecified in ipv6
since otherwise it would specify the next header after IPv6
header.

Extension headers mask should be empty by default.

Andrew.

Re: [dpdk-dev] [RFC] ethdev: add fragment attribute to IPv6 item

[Ori Kam]

Hi Andrew,

PSB,

Best,
Ori

> -----Original Message-----
> From: Andrew Rybchenko <arybchenko@solarflare.com>
> Sent: Tuesday, June 2, 2020 5:33 PM
> Subject: Re: [RFC] ethdev: add fragment attribute to IPv6 item
> 
> On 5/31/20 5:43 PM, Dekel Peled wrote:
> > Using the current implementation of DPDK, an application cannot
> > match on fragmented/non-fragmented IPv6 packets in a simple way.
> >
> > In current implementation:
> > IPv6 header doesn't contain information regarding the packet
> > fragmentation.
> > Fragmented IPv6 packets contain a dedicated extension header, as
> > detailed in RFC [1], which is not yet supported in rte_flow.
> > Non-fragmented packets don't contain the fragment extension header.
> > For an application to match on non-fragmented IPv6 packets, the
> > current implementation doesn't provide a suitable solution.
> > Matching on the Next Header field is not sufficient, since additional
> > extension headers might be present in the same packet.
> > To match on fragmented IPv6 packets, the same difficulty exists.
> >
> > Proposed update:
> > An additional value will be added to IPv6 header struct.
> > This value will contain the fragmentation attribute of the packet,
> > providing simple means for identification of fragmented and
> > non-fragmented packets.
> >
> > This update changes ABI, and is proposed for the 20.11 LTS version.
> >
> > [1]
> https://eur03.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmails.dpdk
> .org%2Farchives%2Fdev%2F2020-
> March%2F160255.html&amp;data=02%7C01%7Corika%40mellanox.com%7C42
> 1376a4759b4b9550fd08d80701d24c%7Ca652971c7d2e4d9ba6a4d149256f461b
> %7C0%7C0%7C637267051695278770&amp;sdata=%2F1HKQPZUVwU199ERL2S
> HPFBYj%2BLFmx8%2BtW8ZBiDL%2FTw%3D&amp;reserved=0
> >
> > Signed-off-by: Dekel Peled <dekelp@mellanox.com>
> > ---
> >  lib/librte_ethdev/rte_flow.h | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
> > index b0e4199..3bc8ce1 100644
> > --- a/lib/librte_ethdev/rte_flow.h
> > +++ b/lib/librte_ethdev/rte_flow.h
> > @@ -787,6 +787,8 @@ struct rte_flow_item_ipv4 {
> >   */
> >  struct rte_flow_item_ipv6 {
> >  	struct rte_ipv6_hdr hdr; /**< IPv6 header definition. */
> > +	uint32_t is_frag:1; /**< Is IPv6 packet fragmented/non-fragmented. */
> > +	uint32_t reserved:31; /**< Reserved, must be zero. */
> 
> The solution is simple, but hardly generic and adds an
> example for the future extensions. I doubt that it is a
> right way to go.
> 
I agree with you that this is not the most generic way possible,
but the IPV6 extensions are very unique. So the solution is also unique.
In general, I'm always in favor of finding the most generic way, but sometimes
it is better to keep things simple, and see how it goes.

> May be we should add 256-bit string with one bit for each
> IP protocol number and apply it to extension headers only?
> If bit A is set in the mask:
>  - if bit A is set in spec as well, extension header with
>    IP protocol (1 << A) number must present
>  - if bit A is clear in spec, extension header with
>    IP protocol (1 << A) number must absent
> If bit is clear in the mask, corresponding extension header
> may present and may absent (i.e. don't care).
> 
There are only 12 possible extension headers and currently none of them
are supported in rte_flow. So adding a logic to parse the 256 just to get a max of 12 
possible values is an overkill. Also, if we disregard the case of the extension, 
the application must select only one next proto. For example, the application
can't select udp + tcp. There is the option to add a flag for each of the
possible extensions, does it makes more sense to you?

> The RFC indirectly touches IPv6 proto (next header) matching
> logic.
> 
> If logic used in ETH+VLAN is applied on IPv6 as well, it would
> make pattern specification and handling complicated. E.g.:
>   eth / ipv6 / udp / end
> should match UDP over IPv6 without any extension headers only.
> 
The issue with VLAN I agree is different since by definition VLAN is 
layer 2.5. We can add the same logic also to the VLAN case, maybe it will
be easier. 
In any case, in your example above and according to the RFC we will
get all ipv6 udp traffic with and without extensions.

> And how to specify UPD over IPv6 regardless extension headers?

Please see above the rule will be eth / ipv6 /udp.

>   eth / ipv6 / ipv6_ext / udp / end
> with a convention that ipv6_ext is optional if spec and mask
> are NULL (or mask is empty).
> 
I would guess that this flow should match all ipv6 that has one ext and the next 
proto is udp.

> I'm wondering if any driver treats it this way?
>
I'm not sure, we can support only the frag ext by default, but if required we can support other 
ext.
 
> I agree that the problem really comes when we'd like match
> IPv6 frags or even worse not fragments.
> 
> Two patterns for fragments:
>   eth / ipv6 (proto=FRAGMENT) / end
>   eth / ipv6 / ipv6_ext (next_hdr=FRAGMENT) / end
> 
> Any sensible solution for not-fragments with any other
> extension headers?
> 
The one propose in this mail 😊 

> INVERT exists, but hardly useful, since it simply says
> that patches which do not match pattern without INVERT
> matches the pattern with INVERT and
>   invert / eth / ipv6 (proto=FRAGMENT) / end
> will match ARP, IPv4, IPv6 with an extension header before
> fragment header and so on.
>
I agree with you, INVERT in this doesn’t help.
We were considering adding some kind of not mask / item per item.
some think around this line:
user request ipv6 unfragmented udp packets. The flow would look something
like this:
Eth / ipv6 / Not (Ipv6.proto = frag_proto) / udp
But it makes the rules much harder to use, and I don't think that there
is any HW that support not, and adding such feature to all items is overkill.

 
> Bit string suggested above will allow to match:
>  - UDP over IPv6 with any extension headers:
>     eth / ipv6 (ext_hdrs mask empty) / udp / end
>  - UDP over IPv6 without any extension headers:
>     eth / ipv6 (ext_hdrs mask full, spec empty) / udp / end
>  - UDP over IPv6 without fragment header:
>     eth / ipv6 (ext.spec & ~FRAGMENT, ext.mask | FRAGMENT) / udp / end
>  - UDP over IPv6 with fragment header
>     eth / ipv6 (ext.spec | FRAGMENT, ext.mask | FRAGMENT) / udp / end
> 
> where FRAGMENT is 1 << IPPROTO_FRAGMENT.
> 
Please see my response regarding this above.

> Above I intentionally keep 'proto' unspecified in ipv6
> since otherwise it would specify the next header after IPv6
> header.
> 
> Extension headers mask should be empty by default.
> 
> Andrew.
Ori

Re: [dpdk-dev] [RFC] ethdev: add fragment attribute to IPv6 item

[Adrien Mazarguil]

Hi Ori, Andrew, Delek,

(been a while eh?)

On Tue, Jun 02, 2020 at 06:28:41PM +0000, Ori Kam wrote:
> Hi Andrew,
> 
> PSB,
[...]
> > > diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
> > > index b0e4199..3bc8ce1 100644
> > > --- a/lib/librte_ethdev/rte_flow.h
> > > +++ b/lib/librte_ethdev/rte_flow.h
> > > @@ -787,6 +787,8 @@ struct rte_flow_item_ipv4 {
> > >   */
> > >  struct rte_flow_item_ipv6 {
> > >  	struct rte_ipv6_hdr hdr; /**< IPv6 header definition. */
> > > +	uint32_t is_frag:1; /**< Is IPv6 packet fragmented/non-fragmented. */
> > > +	uint32_t reserved:31; /**< Reserved, must be zero. */
> > 
> > The solution is simple, but hardly generic and adds an
> > example for the future extensions. I doubt that it is a
> > right way to go.
> > 
> I agree with you that this is not the most generic way possible,
> but the IPV6 extensions are very unique. So the solution is also unique.
> In general, I'm always in favor of finding the most generic way, but sometimes
> it is better to keep things simple, and see how it goes.

Same feeling here, it doesn't look right.

> > May be we should add 256-bit string with one bit for each
> > IP protocol number and apply it to extension headers only?
> > If bit A is set in the mask:
> >  - if bit A is set in spec as well, extension header with
> >    IP protocol (1 << A) number must present
> >  - if bit A is clear in spec, extension header with
> >    IP protocol (1 << A) number must absent
> > If bit is clear in the mask, corresponding extension header
> > may present and may absent (i.e. don't care).
> > 
> There are only 12 possible extension headers and currently none of them
> are supported in rte_flow. So adding a logic to parse the 256 just to get a max of 12 
> possible values is an overkill. Also, if we disregard the case of the extension, 
> the application must select only one next proto. For example, the application
> can't select udp + tcp. There is the option to add a flag for each of the
> possible extensions, does it makes more sense to you?

Each of these extension headers has its own structure, we first need the
ability to match them properly by adding the necessary pattern items.

> > The RFC indirectly touches IPv6 proto (next header) matching
> > logic.
> > 
> > If logic used in ETH+VLAN is applied on IPv6 as well, it would
> > make pattern specification and handling complicated. E.g.:
> >   eth / ipv6 / udp / end
> > should match UDP over IPv6 without any extension headers only.
> > 
> The issue with VLAN I agree is different since by definition VLAN is 
> layer 2.5. We can add the same logic also to the VLAN case, maybe it will
> be easier. 
> In any case, in your example above and according to the RFC we will
> get all ipv6 udp traffic with and without extensions.
> 
> > And how to specify UPD over IPv6 regardless extension headers?
> 
> Please see above the rule will be eth / ipv6 /udp.
> 
> >   eth / ipv6 / ipv6_ext / udp / end
> > with a convention that ipv6_ext is optional if spec and mask
> > are NULL (or mask is empty).
> > 
> I would guess that this flow should match all ipv6 that has one ext and the next 
> proto is udp.

In my opinion RTE_FLOW_ITEM_TYPE_IPV6_EXT is a bit useless on its own. It's
only for matching packets that contain some kind of extension header, not a
specific one, more about that below.

> > I'm wondering if any driver treats it this way?
> >
> I'm not sure, we can support only the frag ext by default, but if required we can support other 
> ext.
>  
> > I agree that the problem really comes when we'd like match
> > IPv6 frags or even worse not fragments.
> > 
> > Two patterns for fragments:
> >   eth / ipv6 (proto=FRAGMENT) / end
> >   eth / ipv6 / ipv6_ext (next_hdr=FRAGMENT) / end
> > 
> > Any sensible solution for not-fragments with any other
> > extension headers?
> > 
> The one propose in this mail 😊 
> 
> > INVERT exists, but hardly useful, since it simply says
> > that patches which do not match pattern without INVERT
> > matches the pattern with INVERT and
> >   invert / eth / ipv6 (proto=FRAGMENT) / end
> > will match ARP, IPv4, IPv6 with an extension header before
> > fragment header and so on.
> >
> I agree with you, INVERT in this doesn’t help.
> We were considering adding some kind of not mask / item per item.
> some think around this line:
> user request ipv6 unfragmented udp packets. The flow would look something
> like this:
> Eth / ipv6 / Not (Ipv6.proto = frag_proto) / udp
> But it makes the rules much harder to use, and I don't think that there
> is any HW that support not, and adding such feature to all items is overkill.
> 
>  
> > Bit string suggested above will allow to match:
> >  - UDP over IPv6 with any extension headers:
> >     eth / ipv6 (ext_hdrs mask empty) / udp / end
> >  - UDP over IPv6 without any extension headers:
> >     eth / ipv6 (ext_hdrs mask full, spec empty) / udp / end
> >  - UDP over IPv6 without fragment header:
> >     eth / ipv6 (ext.spec & ~FRAGMENT, ext.mask | FRAGMENT) / udp / end
> >  - UDP over IPv6 with fragment header
> >     eth / ipv6 (ext.spec | FRAGMENT, ext.mask | FRAGMENT) / udp / end
> > 
> > where FRAGMENT is 1 << IPPROTO_FRAGMENT.
> > 
> Please see my response regarding this above.
> 
> > Above I intentionally keep 'proto' unspecified in ipv6
> > since otherwise it would specify the next header after IPv6
> > header.
> > 
> > Extension headers mask should be empty by default.

This is a deliberate design choice/issue with rte_flow: an empty pattern
matches everything; adding items only narrows the selection. As Andrew said
there is currently no way to provide a specific item to reject, it can only
be done globally on a pattern through INVERT that no PMD implements so far.

So we have two requirements here: the ability to specifically match IPv6
fragment headers and the ability to reject them.

To match IPv6 fragment headers, we need a dedicated pattern item. The
generic RTE_FLOW_ITEM_TYPE_IPV6_EXT is useless for that on its own, it must
be completed with RTE_FLOW_ITEM_TYPE_IPV6_EXT_FRAG and associated object
to match individual fields if needed (like all the others
protocols/headers).

Then to reject a pattern item... My preference goes to a new "NOT" meta item
affecting the meaning of the item coming immediately after in the pattern
list. That would be ultra generic, wouldn't break any ABI/API and like
INVERT, wouldn't even require a new object associated with it.

To match UDPv6 traffic when there is no fragment header, one could then do
something like:

 eth / ipv6 / not / ipv6_ext_frag / udp

PMD support would be trivial to implement (I'm sure!)

We may later implement other kinds of "operator" items as Andrew suggested,
for bit-wise stuff and so on. Let's keep adding features on a needed basis
though.

-- 
Adrien Mazarguil
6WIND

Re: [dpdk-dev] [RFC] ethdev: add fragment attribute to IPv6 item

[Ori Kam]

Hi Adrien,

Great to hear from you again.

> -----Original Message-----
> From: Adrien Mazarguil <adrien.mazarguil@6wind.com>
> Sent: Tuesday, June 2, 2020 10:04 PM
> To: Ori Kam <orika@mellanox.com>
> Cc: Andrew Rybchenko <arybchenko@solarflare.com>; Dekel Peled
> <dekelp@mellanox.com>; ferruh.yigit@intel.com; john.mcnamara@intel.com;
> marko.kovacevic@intel.com; Asaf Penso <asafp@mellanox.com>; Matan Azrad
> <matan@mellanox.com>; Eli Britstein <elibr@mellanox.com>; dev@dpdk.org;
> Ivan Malov <Ivan.Malov@oktetlabs.ru>
> Subject: Re: [RFC] ethdev: add fragment attribute to IPv6 item
> 
> Hi Ori, Andrew, Delek,
> 
> (been a while eh?)
> 
> On Tue, Jun 02, 2020 at 06:28:41PM +0000, Ori Kam wrote:
> > Hi Andrew,
> >
> > PSB,
> [...]
> > > > diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
> > > > index b0e4199..3bc8ce1 100644
> > > > --- a/lib/librte_ethdev/rte_flow.h
> > > > +++ b/lib/librte_ethdev/rte_flow.h
> > > > @@ -787,6 +787,8 @@ struct rte_flow_item_ipv4 {
> > > >   */
> > > >  struct rte_flow_item_ipv6 {
> > > >  	struct rte_ipv6_hdr hdr; /**< IPv6 header definition. */
> > > > +	uint32_t is_frag:1; /**< Is IPv6 packet fragmented/non-fragmented. */
> > > > +	uint32_t reserved:31; /**< Reserved, must be zero. */
> > >
> > > The solution is simple, but hardly generic and adds an
> > > example for the future extensions. I doubt that it is a
> > > right way to go.
> > >
> > I agree with you that this is not the most generic way possible,
> > but the IPV6 extensions are very unique. So the solution is also unique.
> > In general, I'm always in favor of finding the most generic way, but
> sometimes
> > it is better to keep things simple, and see how it goes.
> 
> Same feeling here, it doesn't look right.
> 
> > > May be we should add 256-bit string with one bit for each
> > > IP protocol number and apply it to extension headers only?
> > > If bit A is set in the mask:
> > >  - if bit A is set in spec as well, extension header with
> > >    IP protocol (1 << A) number must present
> > >  - if bit A is clear in spec, extension header with
> > >    IP protocol (1 << A) number must absent
> > > If bit is clear in the mask, corresponding extension header
> > > may present and may absent (i.e. don't care).
> > >
> > There are only 12 possible extension headers and currently none of them
> > are supported in rte_flow. So adding a logic to parse the 256 just to get a max
> of 12
> > possible values is an overkill. Also, if we disregard the case of the extension,
> > the application must select only one next proto. For example, the application
> > can't select udp + tcp. There is the option to add a flag for each of the
> > possible extensions, does it makes more sense to you?
> 
> Each of these extension headers has its own structure, we first need the
> ability to match them properly by adding the necessary pattern items.
> 
> > > The RFC indirectly touches IPv6 proto (next header) matching
> > > logic.
> > >
> > > If logic used in ETH+VLAN is applied on IPv6 as well, it would
> > > make pattern specification and handling complicated. E.g.:
> > >   eth / ipv6 / udp / end
> > > should match UDP over IPv6 without any extension headers only.
> > >
> > The issue with VLAN I agree is different since by definition VLAN is
> > layer 2.5. We can add the same logic also to the VLAN case, maybe it will
> > be easier.
> > In any case, in your example above and according to the RFC we will
> > get all ipv6 udp traffic with and without extensions.
> >
> > > And how to specify UPD over IPv6 regardless extension headers?
> >
> > Please see above the rule will be eth / ipv6 /udp.
> >
> > >   eth / ipv6 / ipv6_ext / udp / end
> > > with a convention that ipv6_ext is optional if spec and mask
> > > are NULL (or mask is empty).
> > >
> > I would guess that this flow should match all ipv6 that has one ext and the
> next
> > proto is udp.
> 
> In my opinion RTE_FLOW_ITEM_TYPE_IPV6_EXT is a bit useless on its own. It's
> only for matching packets that contain some kind of extension header, not a
> specific one, more about that below.
> 
> > > I'm wondering if any driver treats it this way?
> > >
> > I'm not sure, we can support only the frag ext by default, but if required we
> can support other
> > ext.
> >
> > > I agree that the problem really comes when we'd like match
> > > IPv6 frags or even worse not fragments.
> > >
> > > Two patterns for fragments:
> > >   eth / ipv6 (proto=FRAGMENT) / end
> > >   eth / ipv6 / ipv6_ext (next_hdr=FRAGMENT) / end
> > >
> > > Any sensible solution for not-fragments with any other
> > > extension headers?
> > >
> > The one propose in this mail 😊
> >
> > > INVERT exists, but hardly useful, since it simply says
> > > that patches which do not match pattern without INVERT
> > > matches the pattern with INVERT and
> > >   invert / eth / ipv6 (proto=FRAGMENT) / end
> > > will match ARP, IPv4, IPv6 with an extension header before
> > > fragment header and so on.
> > >
> > I agree with you, INVERT in this doesn’t help.
> > We were considering adding some kind of not mask / item per item.
> > some think around this line:
> > user request ipv6 unfragmented udp packets. The flow would look something
> > like this:
> > Eth / ipv6 / Not (Ipv6.proto = frag_proto) / udp
> > But it makes the rules much harder to use, and I don't think that there
> > is any HW that support not, and adding such feature to all items is overkill.
> >
> >
> > > Bit string suggested above will allow to match:
> > >  - UDP over IPv6 with any extension headers:
> > >     eth / ipv6 (ext_hdrs mask empty) / udp / end
> > >  - UDP over IPv6 without any extension headers:
> > >     eth / ipv6 (ext_hdrs mask full, spec empty) / udp / end
> > >  - UDP over IPv6 without fragment header:
> > >     eth / ipv6 (ext.spec & ~FRAGMENT, ext.mask | FRAGMENT) / udp / end
> > >  - UDP over IPv6 with fragment header
> > >     eth / ipv6 (ext.spec | FRAGMENT, ext.mask | FRAGMENT) / udp / end
> > >
> > > where FRAGMENT is 1 << IPPROTO_FRAGMENT.
> > >
> > Please see my response regarding this above.
> >
> > > Above I intentionally keep 'proto' unspecified in ipv6
> > > since otherwise it would specify the next header after IPv6
> > > header.
> > >
> > > Extension headers mask should be empty by default.
> 
> This is a deliberate design choice/issue with rte_flow: an empty pattern
> matches everything; adding items only narrows the selection. As Andrew said
> there is currently no way to provide a specific item to reject, it can only
> be done globally on a pattern through INVERT that no PMD implements so far.
> 
> So we have two requirements here: the ability to specifically match IPv6
> fragment headers and the ability to reject them.
> 
> To match IPv6 fragment headers, we need a dedicated pattern item. The
> generic RTE_FLOW_ITEM_TYPE_IPV6_EXT is useless for that on its own, it must
> be completed with RTE_FLOW_ITEM_TYPE_IPV6_EXT_FRAG and associated
> object

Yes, we must add EXT_FRAG to be able to match on the FRAG bits.

> to match individual fields if needed (like all the others
> protocols/headers).
> 
> Then to reject a pattern item... My preference goes to a new "NOT" meta item
> affecting the meaning of the item coming immediately after in the pattern
> list. That would be ultra generic, wouldn't break any ABI/API and like
> INVERT, wouldn't even require a new object associated with it.
> 
> To match UDPv6 traffic when there is no fragment header, one could then do
> something like:
> 
>  eth / ipv6 / not / ipv6_ext_frag / udp
> 
> PMD support would be trivial to implement (I'm sure!)
> 
I agree with you as I said above. The issue is not PMD, the issues are:
1. think about the rule you stated above from logic point there is some contradiction,
you are saying ipv6 next proto udp but you also say not frag, this is logic only for IPV6 ext.
2. HW issue, I don't know of HW that knows how to support not on an item.
So adding something for all items for only one case is overkill.
 


> We may later implement other kinds of "operator" items as Andrew suggested,
> for bit-wise stuff and so on. Let's keep adding features on a needed basis
> though.
> 
> --
> Adrien Mazarguil
> 6WIND

Best,
Ori

Re: [dpdk-dev] [RFC] ethdev: add fragment attribute to IPv6 item

[Dekel Peled]

Hi, PSB.

> -----Original Message-----
> From: Ori Kam <orika@mellanox.com>
> Sent: Wednesday, June 3, 2020 11:16 AM
> To: Adrien Mazarguil <adrien.mazarguil@6wind.com>
> Cc: Andrew Rybchenko <arybchenko@solarflare.com>; Dekel Peled
> <dekelp@mellanox.com>; ferruh.yigit@intel.com;
> john.mcnamara@intel.com; marko.kovacevic@intel.com; Asaf Penso
> <asafp@mellanox.com>; Matan Azrad <matan@mellanox.com>; Eli Britstein
> <elibr@mellanox.com>; dev@dpdk.org; Ivan Malov
> <Ivan.Malov@oktetlabs.ru>
> Subject: RE: [RFC] ethdev: add fragment attribute to IPv6 item
> 
> Hi Adrien,
> 
> Great to hear from you again.
> 
> > -----Original Message-----
> > From: Adrien Mazarguil <adrien.mazarguil@6wind.com>
> > Sent: Tuesday, June 2, 2020 10:04 PM
> > To: Ori Kam <orika@mellanox.com>
> > Cc: Andrew Rybchenko <arybchenko@solarflare.com>; Dekel Peled
> > <dekelp@mellanox.com>; ferruh.yigit@intel.com;
> > john.mcnamara@intel.com; marko.kovacevic@intel.com; Asaf Penso
> > <asafp@mellanox.com>; Matan Azrad <matan@mellanox.com>; Eli
> Britstein
> > <elibr@mellanox.com>; dev@dpdk.org; Ivan Malov
> > <Ivan.Malov@oktetlabs.ru>
> > Subject: Re: [RFC] ethdev: add fragment attribute to IPv6 item
> >
> > Hi Ori, Andrew, Delek,

It's Dekel, not Delek ;-)

> >
> > (been a while eh?)
> >
> > On Tue, Jun 02, 2020 at 06:28:41PM +0000, Ori Kam wrote:
> > > Hi Andrew,
> > >
> > > PSB,
> > [...]
> > > > > diff --git a/lib/librte_ethdev/rte_flow.h
> > > > > b/lib/librte_ethdev/rte_flow.h index b0e4199..3bc8ce1 100644
> > > > > --- a/lib/librte_ethdev/rte_flow.h
> > > > > +++ b/lib/librte_ethdev/rte_flow.h
> > > > > @@ -787,6 +787,8 @@ struct rte_flow_item_ipv4 {
> > > > >   */
> > > > >  struct rte_flow_item_ipv6 {
> > > > >  	struct rte_ipv6_hdr hdr; /**< IPv6 header definition. */
> > > > > +	uint32_t is_frag:1; /**< Is IPv6 packet fragmented/non-
> fragmented. */
> > > > > +	uint32_t reserved:31; /**< Reserved, must be zero. */
> > > >
> > > > The solution is simple, but hardly generic and adds an example for
> > > > the future extensions. I doubt that it is a right way to go.
> > > >
> > > I agree with you that this is not the most generic way possible, but
> > > the IPV6 extensions are very unique. So the solution is also unique.
> > > In general, I'm always in favor of finding the most generic way, but
> > sometimes
> > > it is better to keep things simple, and see how it goes.
> >
> > Same feeling here, it doesn't look right.
> >
> > > > May be we should add 256-bit string with one bit for each IP
> > > > protocol number and apply it to extension headers only?
> > > > If bit A is set in the mask:
> > > >  - if bit A is set in spec as well, extension header with
> > > >    IP protocol (1 << A) number must present
> > > >  - if bit A is clear in spec, extension header with
> > > >    IP protocol (1 << A) number must absent If bit is clear in the
> > > > mask, corresponding extension header may present and may absent
> > > > (i.e. don't care).
> > > >
> > > There are only 12 possible extension headers and currently none of
> > > them are supported in rte_flow. So adding a logic to parse the 256
> > > just to get a max
> > of 12
> > > possible values is an overkill. Also, if we disregard the case of
> > > the extension, the application must select only one next proto. For
> > > example, the application can't select udp + tcp. There is the option
> > > to add a flag for each of the possible extensions, does it makes more
> sense to you?
> >
> > Each of these extension headers has its own structure, we first need
> > the ability to match them properly by adding the necessary pattern items.
> >
> > > > The RFC indirectly touches IPv6 proto (next header) matching
> > > > logic.
> > > >
> > > > If logic used in ETH+VLAN is applied on IPv6 as well, it would
> > > > make pattern specification and handling complicated. E.g.:
> > > >   eth / ipv6 / udp / end
> > > > should match UDP over IPv6 without any extension headers only.
> > > >
> > > The issue with VLAN I agree is different since by definition VLAN is
> > > layer 2.5. We can add the same logic also to the VLAN case, maybe it
> > > will be easier.
> > > In any case, in your example above and according to the RFC we will
> > > get all ipv6 udp traffic with and without extensions.
> > >
> > > > And how to specify UPD over IPv6 regardless extension headers?
> > >
> > > Please see above the rule will be eth / ipv6 /udp.
> > >
> > > >   eth / ipv6 / ipv6_ext / udp / end with a convention that
> > > > ipv6_ext is optional if spec and mask are NULL (or mask is empty).
> > > >
> > > I would guess that this flow should match all ipv6 that has one ext
> > > and the
> > next
> > > proto is udp.
> >
> > In my opinion RTE_FLOW_ITEM_TYPE_IPV6_EXT is a bit useless on its own.
> > It's only for matching packets that contain some kind of extension
> > header, not a specific one, more about that below.
> >
> > > > I'm wondering if any driver treats it this way?
> > > >
> > > I'm not sure, we can support only the frag ext by default, but if
> > > required we
> > can support other
> > > ext.
> > >
> > > > I agree that the problem really comes when we'd like match
> > > > IPv6 frags or even worse not fragments.
> > > >
> > > > Two patterns for fragments:
> > > >   eth / ipv6 (proto=FRAGMENT) / end
> > > >   eth / ipv6 / ipv6_ext (next_hdr=FRAGMENT) / end
> > > >
> > > > Any sensible solution for not-fragments with any other extension
> > > > headers?
> > > >
> > > The one propose in this mail 😊
> > >
> > > > INVERT exists, but hardly useful, since it simply says that
> > > > patches which do not match pattern without INVERT matches the
> > > > pattern with INVERT and
> > > >   invert / eth / ipv6 (proto=FRAGMENT) / end will match ARP, IPv4,
> > > > IPv6 with an extension header before fragment header and so on.
> > > >
> > > I agree with you, INVERT in this doesn’t help.
> > > We were considering adding some kind of not mask / item per item.
> > > some think around this line:
> > > user request ipv6 unfragmented udp packets. The flow would look
> > > something like this:
> > > Eth / ipv6 / Not (Ipv6.proto = frag_proto) / udp But it makes the
> > > rules much harder to use, and I don't think that there is any HW
> > > that support not, and adding such feature to all items is overkill.
> > >
> > >
> > > > Bit string suggested above will allow to match:
> > > >  - UDP over IPv6 with any extension headers:
> > > >     eth / ipv6 (ext_hdrs mask empty) / udp / end
> > > >  - UDP over IPv6 without any extension headers:
> > > >     eth / ipv6 (ext_hdrs mask full, spec empty) / udp / end
> > > >  - UDP over IPv6 without fragment header:
> > > >     eth / ipv6 (ext.spec & ~FRAGMENT, ext.mask | FRAGMENT) / udp /
> > > > end
> > > >  - UDP over IPv6 with fragment header
> > > >     eth / ipv6 (ext.spec | FRAGMENT, ext.mask | FRAGMENT) / udp /
> > > > end
> > > >
> > > > where FRAGMENT is 1 << IPPROTO_FRAGMENT.
> > > >
> > > Please see my response regarding this above.
> > >
> > > > Above I intentionally keep 'proto' unspecified in ipv6 since
> > > > otherwise it would specify the next header after IPv6 header.
> > > >
> > > > Extension headers mask should be empty by default.
> >
> > This is a deliberate design choice/issue with rte_flow: an empty
> > pattern matches everything; adding items only narrows the selection.
> > As Andrew said there is currently no way to provide a specific item to
> > reject, it can only be done globally on a pattern through INVERT that no
> PMD implements so far.
> >
> > So we have two requirements here: the ability to specifically match
> > IPv6 fragment headers and the ability to reject them.
> >
> > To match IPv6 fragment headers, we need a dedicated pattern item. The
> > generic RTE_FLOW_ITEM_TYPE_IPV6_EXT is useless for that on its own, it
> > must be completed with RTE_FLOW_ITEM_TYPE_IPV6_EXT_FRAG and
> associated
> > object
> 
> Yes, we must add EXT_FRAG to be able to match on the FRAG bits.
> 

Please see previous RFC I sent.
[RFC] ethdev: add IPv6 fragment extension header item
http://mails.dpdk.org/archives/dev/2020-March/160255.html
It is complemented by this RFC.

> > to match individual fields if needed (like all the others
> > protocols/headers).
> >
> > Then to reject a pattern item... My preference goes to a new "NOT"
> > meta item affecting the meaning of the item coming immediately after
> > in the pattern list. That would be ultra generic, wouldn't break any
> > ABI/API and like INVERT, wouldn't even require a new object associated
> with it.
> >
> > To match UDPv6 traffic when there is no fragment header, one could
> > then do something like:
> >
> >  eth / ipv6 / not / ipv6_ext_frag / udp
> >
> > PMD support would be trivial to implement (I'm sure!)
> >
> I agree with you as I said above. The issue is not PMD, the issues are:
> 1. think about the rule you stated above from logic point there is some
> contradiction, you are saying ipv6 next proto udp but you also say not frag,
> this is logic only for IPV6 ext.
> 2. HW issue, I don't know of HW that knows how to support not on an item.
> So adding something for all items for only one case is overkill.
> 
> 
> 
> > We may later implement other kinds of "operator" items as Andrew
> > suggested, for bit-wise stuff and so on. Let's keep adding features on
> > a needed basis though.
> >
> > --
> > Adrien Mazarguil
> > 6WIND
> 
> Best,
> Ori

Re: [dpdk-dev] [RFC] ethdev: add fragment attribute to IPv6 item

[Dekel Peled]

Hi,

Kind reminder, please respond on the recent correspondence so we can conclude this issue.

Regards,
Dekel

> -----Original Message-----
> From: Dekel Peled <dekelp@mellanox.com>
> Sent: Wednesday, June 3, 2020 3:11 PM
> To: Ori Kam <orika@mellanox.com>; Adrien Mazarguil
> <adrien.mazarguil@6wind.com>
> Cc: Andrew Rybchenko <arybchenko@solarflare.com>;
> ferruh.yigit@intel.com; john.mcnamara@intel.com;
> marko.kovacevic@intel.com; Asaf Penso <asafp@mellanox.com>; Matan
> Azrad <matan@mellanox.com>; Eli Britstein <elibr@mellanox.com>;
> dev@dpdk.org; Ivan Malov <Ivan.Malov@oktetlabs.ru>
> Subject: RE: [RFC] ethdev: add fragment attribute to IPv6 item
> 
> Hi, PSB.
> 
> > -----Original Message-----
> > From: Ori Kam <orika@mellanox.com>
> > Sent: Wednesday, June 3, 2020 11:16 AM
> > To: Adrien Mazarguil <adrien.mazarguil@6wind.com>
> > Cc: Andrew Rybchenko <arybchenko@solarflare.com>; Dekel Peled
> > <dekelp@mellanox.com>; ferruh.yigit@intel.com;
> > john.mcnamara@intel.com; marko.kovacevic@intel.com; Asaf Penso
> > <asafp@mellanox.com>; Matan Azrad <matan@mellanox.com>; Eli
> Britstein
> > <elibr@mellanox.com>; dev@dpdk.org; Ivan Malov
> > <Ivan.Malov@oktetlabs.ru>
> > Subject: RE: [RFC] ethdev: add fragment attribute to IPv6 item
> >
> > Hi Adrien,
> >
> > Great to hear from you again.
> >
> > > -----Original Message-----
> > > From: Adrien Mazarguil <adrien.mazarguil@6wind.com>
> > > Sent: Tuesday, June 2, 2020 10:04 PM
> > > To: Ori Kam <orika@mellanox.com>
> > > Cc: Andrew Rybchenko <arybchenko@solarflare.com>; Dekel Peled
> > > <dekelp@mellanox.com>; ferruh.yigit@intel.com;
> > > john.mcnamara@intel.com; marko.kovacevic@intel.com; Asaf Penso
> > > <asafp@mellanox.com>; Matan Azrad <matan@mellanox.com>; Eli
> > Britstein
> > > <elibr@mellanox.com>; dev@dpdk.org; Ivan Malov
> > > <Ivan.Malov@oktetlabs.ru>
> > > Subject: Re: [RFC] ethdev: add fragment attribute to IPv6 item
> > >
> > > Hi Ori, Andrew, Delek,
> 
> It's Dekel, not Delek ;-)
> 
> > >
> > > (been a while eh?)
> > >
> > > On Tue, Jun 02, 2020 at 06:28:41PM +0000, Ori Kam wrote:
> > > > Hi Andrew,
> > > >
> > > > PSB,
> > > [...]
> > > > > > diff --git a/lib/librte_ethdev/rte_flow.h
> > > > > > b/lib/librte_ethdev/rte_flow.h index b0e4199..3bc8ce1 100644
> > > > > > --- a/lib/librte_ethdev/rte_flow.h
> > > > > > +++ b/lib/librte_ethdev/rte_flow.h
> > > > > > @@ -787,6 +787,8 @@ struct rte_flow_item_ipv4 {
> > > > > >   */
> > > > > >  struct rte_flow_item_ipv6 {
> > > > > >  	struct rte_ipv6_hdr hdr; /**< IPv6 header definition. */
> > > > > > +	uint32_t is_frag:1; /**< Is IPv6 packet fragmented/non-
> > fragmented. */
> > > > > > +	uint32_t reserved:31; /**< Reserved, must be zero. */
> > > > >
> > > > > The solution is simple, but hardly generic and adds an example
> > > > > for the future extensions. I doubt that it is a right way to go.
> > > > >
> > > > I agree with you that this is not the most generic way possible,
> > > > but the IPV6 extensions are very unique. So the solution is also unique.
> > > > In general, I'm always in favor of finding the most generic way,
> > > > but
> > > sometimes
> > > > it is better to keep things simple, and see how it goes.
> > >
> > > Same feeling here, it doesn't look right.
> > >
> > > > > May be we should add 256-bit string with one bit for each IP
> > > > > protocol number and apply it to extension headers only?
> > > > > If bit A is set in the mask:
> > > > >  - if bit A is set in spec as well, extension header with
> > > > >    IP protocol (1 << A) number must present
> > > > >  - if bit A is clear in spec, extension header with
> > > > >    IP protocol (1 << A) number must absent If bit is clear in
> > > > > the mask, corresponding extension header may present and may
> > > > > absent (i.e. don't care).
> > > > >
> > > > There are only 12 possible extension headers and currently none of
> > > > them are supported in rte_flow. So adding a logic to parse the 256
> > > > just to get a max
> > > of 12
> > > > possible values is an overkill. Also, if we disregard the case of
> > > > the extension, the application must select only one next proto.
> > > > For example, the application can't select udp + tcp. There is the
> > > > option to add a flag for each of the possible extensions, does it
> > > > makes more
> > sense to you?
> > >
> > > Each of these extension headers has its own structure, we first need
> > > the ability to match them properly by adding the necessary pattern items.
> > >
> > > > > The RFC indirectly touches IPv6 proto (next header) matching
> > > > > logic.
> > > > >
> > > > > If logic used in ETH+VLAN is applied on IPv6 as well, it would
> > > > > make pattern specification and handling complicated. E.g.:
> > > > >   eth / ipv6 / udp / end
> > > > > should match UDP over IPv6 without any extension headers only.
> > > > >
> > > > The issue with VLAN I agree is different since by definition VLAN
> > > > is layer 2.5. We can add the same logic also to the VLAN case,
> > > > maybe it will be easier.
> > > > In any case, in your example above and according to the RFC we
> > > > will get all ipv6 udp traffic with and without extensions.
> > > >
> > > > > And how to specify UPD over IPv6 regardless extension headers?
> > > >
> > > > Please see above the rule will be eth / ipv6 /udp.
> > > >
> > > > >   eth / ipv6 / ipv6_ext / udp / end with a convention that
> > > > > ipv6_ext is optional if spec and mask are NULL (or mask is empty).
> > > > >
> > > > I would guess that this flow should match all ipv6 that has one
> > > > ext and the
> > > next
> > > > proto is udp.
> > >
> > > In my opinion RTE_FLOW_ITEM_TYPE_IPV6_EXT is a bit useless on its
> own.
> > > It's only for matching packets that contain some kind of extension
> > > header, not a specific one, more about that below.
> > >
> > > > > I'm wondering if any driver treats it this way?
> > > > >
> > > > I'm not sure, we can support only the frag ext by default, but if
> > > > required we
> > > can support other
> > > > ext.
> > > >
> > > > > I agree that the problem really comes when we'd like match
> > > > > IPv6 frags or even worse not fragments.
> > > > >
> > > > > Two patterns for fragments:
> > > > >   eth / ipv6 (proto=FRAGMENT) / end
> > > > >   eth / ipv6 / ipv6_ext (next_hdr=FRAGMENT) / end
> > > > >
> > > > > Any sensible solution for not-fragments with any other extension
> > > > > headers?
> > > > >
> > > > The one propose in this mail 😊
> > > >
> > > > > INVERT exists, but hardly useful, since it simply says that
> > > > > patches which do not match pattern without INVERT matches the
> > > > > pattern with INVERT and
> > > > >   invert / eth / ipv6 (proto=FRAGMENT) / end will match ARP,
> > > > > IPv4,
> > > > > IPv6 with an extension header before fragment header and so on.
> > > > >
> > > > I agree with you, INVERT in this doesn’t help.
> > > > We were considering adding some kind of not mask / item per item.
> > > > some think around this line:
> > > > user request ipv6 unfragmented udp packets. The flow would look
> > > > something like this:
> > > > Eth / ipv6 / Not (Ipv6.proto = frag_proto) / udp But it makes the
> > > > rules much harder to use, and I don't think that there is any HW
> > > > that support not, and adding such feature to all items is overkill.
> > > >
> > > >
> > > > > Bit string suggested above will allow to match:
> > > > >  - UDP over IPv6 with any extension headers:
> > > > >     eth / ipv6 (ext_hdrs mask empty) / udp / end
> > > > >  - UDP over IPv6 without any extension headers:
> > > > >     eth / ipv6 (ext_hdrs mask full, spec empty) / udp / end
> > > > >  - UDP over IPv6 without fragment header:
> > > > >     eth / ipv6 (ext.spec & ~FRAGMENT, ext.mask | FRAGMENT) / udp
> > > > > / end
> > > > >  - UDP over IPv6 with fragment header
> > > > >     eth / ipv6 (ext.spec | FRAGMENT, ext.mask | FRAGMENT) / udp
> > > > > / end
> > > > >
> > > > > where FRAGMENT is 1 << IPPROTO_FRAGMENT.
> > > > >
> > > > Please see my response regarding this above.
> > > >
> > > > > Above I intentionally keep 'proto' unspecified in ipv6 since
> > > > > otherwise it would specify the next header after IPv6 header.
> > > > >
> > > > > Extension headers mask should be empty by default.
> > >
> > > This is a deliberate design choice/issue with rte_flow: an empty
> > > pattern matches everything; adding items only narrows the selection.
> > > As Andrew said there is currently no way to provide a specific item
> > > to reject, it can only be done globally on a pattern through INVERT
> > > that no
> > PMD implements so far.
> > >
> > > So we have two requirements here: the ability to specifically match
> > > IPv6 fragment headers and the ability to reject them.
> > >
> > > To match IPv6 fragment headers, we need a dedicated pattern item.
> > > The generic RTE_FLOW_ITEM_TYPE_IPV6_EXT is useless for that on its
> > > own, it must be completed with RTE_FLOW_ITEM_TYPE_IPV6_EXT_FRAG
> and
> > associated
> > > object
> >
> > Yes, we must add EXT_FRAG to be able to match on the FRAG bits.
> >
> 
> Please see previous RFC I sent.
> [RFC] ethdev: add IPv6 fragment extension header item
> http://mails.dpdk.org/archives/dev/2020-March/160255.html
> It is complemented by this RFC.
> 
> > > to match individual fields if needed (like all the others
> > > protocols/headers).
> > >
> > > Then to reject a pattern item... My preference goes to a new "NOT"
> > > meta item affecting the meaning of the item coming immediately after
> > > in the pattern list. That would be ultra generic, wouldn't break any
> > > ABI/API and like INVERT, wouldn't even require a new object
> > > associated
> > with it.
> > >
> > > To match UDPv6 traffic when there is no fragment header, one could
> > > then do something like:
> > >
> > >  eth / ipv6 / not / ipv6_ext_frag / udp
> > >
> > > PMD support would be trivial to implement (I'm sure!)
> > >
> > I agree with you as I said above. The issue is not PMD, the issues are:
> > 1. think about the rule you stated above from logic point there is
> > some contradiction, you are saying ipv6 next proto udp but you also
> > say not frag, this is logic only for IPV6 ext.
> > 2. HW issue, I don't know of HW that knows how to support not on an item.
> > So adding something for all items for only one case is overkill.
> >
> >
> >
> > > We may later implement other kinds of "operator" items as Andrew
> > > suggested, for bit-wise stuff and so on. Let's keep adding features
> > > on a needed basis though.
> > >
> > > --
> > > Adrien Mazarguil
> > > 6WIND
> >
> > Best,
> > Ori

Re: [dpdk-dev] [RFC] ethdev: add fragment attribute to IPv6 item

[Dekel Peled]

Hi,

This change is proposed for 20.11.
It is suggested after internal discussions, where multiple suggestions were considered, some of them similar to the ones suggested below. 
Continuing the earlier correspondence in this thread, please send any other comments/suggestions you have.

Regards,
Dekel

> -----Original Message-----
> From: Dekel Peled <dekelp@mellanox.com>
> Sent: Thursday, June 18, 2020 9:59 AM
> To: Adrien Mazarguil <adrien.mazarguil@6wind.com>; Ori Kam
> <orika@mellanox.com>; Andrew Rybchenko <arybchenko@solarflare.com>
> Cc: ferruh.yigit@intel.com; john.mcnamara@intel.com;
> marko.kovacevic@intel.com; Asaf Penso <asafp@mellanox.com>; Matan
> Azrad <matan@mellanox.com>; Eli Britstein <elibr@mellanox.com>;
> dev@dpdk.org; Ivan Malov <Ivan.Malov@oktetlabs.ru>
> Subject: RE: [RFC] ethdev: add fragment attribute to IPv6 item
> 
> Hi,
> 
> Kind reminder, please respond on the recent correspondence so we can
> conclude this issue.
> 
> Regards,
> Dekel
> 
> > -----Original Message-----
> > From: Dekel Peled <dekelp@mellanox.com>
> > Sent: Wednesday, June 3, 2020 3:11 PM
> > To: Ori Kam <orika@mellanox.com>; Adrien Mazarguil
> > <adrien.mazarguil@6wind.com>
> > Cc: Andrew Rybchenko <arybchenko@solarflare.com>;
> > ferruh.yigit@intel.com; john.mcnamara@intel.com;
> > marko.kovacevic@intel.com; Asaf Penso <asafp@mellanox.com>; Matan
> > Azrad <matan@mellanox.com>; Eli Britstein <elibr@mellanox.com>;
> > dev@dpdk.org; Ivan Malov <Ivan.Malov@oktetlabs.ru>
> > Subject: RE: [RFC] ethdev: add fragment attribute to IPv6 item
> >
> > Hi, PSB.
> >
> > > -----Original Message-----
> > > From: Ori Kam <orika@mellanox.com>
> > > Sent: Wednesday, June 3, 2020 11:16 AM
> > > To: Adrien Mazarguil <adrien.mazarguil@6wind.com>
> > > Cc: Andrew Rybchenko <arybchenko@solarflare.com>; Dekel Peled
> > > <dekelp@mellanox.com>; ferruh.yigit@intel.com;
> > > john.mcnamara@intel.com; marko.kovacevic@intel.com; Asaf Penso
> > > <asafp@mellanox.com>; Matan Azrad <matan@mellanox.com>; Eli
> > Britstein
> > > <elibr@mellanox.com>; dev@dpdk.org; Ivan Malov
> > > <Ivan.Malov@oktetlabs.ru>
> > > Subject: RE: [RFC] ethdev: add fragment attribute to IPv6 item
> > >
> > > Hi Adrien,
> > >
> > > Great to hear from you again.
> > >
> > > > -----Original Message-----
> > > > From: Adrien Mazarguil <adrien.mazarguil@6wind.com>
> > > > Sent: Tuesday, June 2, 2020 10:04 PM
> > > > To: Ori Kam <orika@mellanox.com>
> > > > Cc: Andrew Rybchenko <arybchenko@solarflare.com>; Dekel Peled
> > > > <dekelp@mellanox.com>; ferruh.yigit@intel.com;
> > > > john.mcnamara@intel.com; marko.kovacevic@intel.com; Asaf Penso
> > > > <asafp@mellanox.com>; Matan Azrad <matan@mellanox.com>; Eli
> > > Britstein
> > > > <elibr@mellanox.com>; dev@dpdk.org; Ivan Malov
> > > > <Ivan.Malov@oktetlabs.ru>
> > > > Subject: Re: [RFC] ethdev: add fragment attribute to IPv6 item
> > > >
> > > > Hi Ori, Andrew, Delek,
> >
> > It's Dekel, not Delek ;-)
> >
> > > >
> > > > (been a while eh?)
> > > >
> > > > On Tue, Jun 02, 2020 at 06:28:41PM +0000, Ori Kam wrote:
> > > > > Hi Andrew,
> > > > >
> > > > > PSB,
> > > > [...]
> > > > > > > diff --git a/lib/librte_ethdev/rte_flow.h
> > > > > > > b/lib/librte_ethdev/rte_flow.h index b0e4199..3bc8ce1 100644
> > > > > > > --- a/lib/librte_ethdev/rte_flow.h
> > > > > > > +++ b/lib/librte_ethdev/rte_flow.h
> > > > > > > @@ -787,6 +787,8 @@ struct rte_flow_item_ipv4 {
> > > > > > >   */
> > > > > > >  struct rte_flow_item_ipv6 {
> > > > > > >  	struct rte_ipv6_hdr hdr; /**< IPv6 header definition. */
> > > > > > > +	uint32_t is_frag:1; /**< Is IPv6 packet fragmented/non-
> > > fragmented. */
> > > > > > > +	uint32_t reserved:31; /**< Reserved, must be zero. */
> > > > > >
> > > > > > The solution is simple, but hardly generic and adds an example
> > > > > > for the future extensions. I doubt that it is a right way to go.
> > > > > >
> > > > > I agree with you that this is not the most generic way possible,
> > > > > but the IPV6 extensions are very unique. So the solution is also
> unique.
> > > > > In general, I'm always in favor of finding the most generic way,
> > > > > but
> > > > sometimes
> > > > > it is better to keep things simple, and see how it goes.
> > > >
> > > > Same feeling here, it doesn't look right.
> > > >
> > > > > > May be we should add 256-bit string with one bit for each IP
> > > > > > protocol number and apply it to extension headers only?
> > > > > > If bit A is set in the mask:
> > > > > >  - if bit A is set in spec as well, extension header with
> > > > > >    IP protocol (1 << A) number must present
> > > > > >  - if bit A is clear in spec, extension header with
> > > > > >    IP protocol (1 << A) number must absent If bit is clear in
> > > > > > the mask, corresponding extension header may present and may
> > > > > > absent (i.e. don't care).
> > > > > >
> > > > > There are only 12 possible extension headers and currently none
> > > > > of them are supported in rte_flow. So adding a logic to parse
> > > > > the 256 just to get a max
> > > > of 12
> > > > > possible values is an overkill. Also, if we disregard the case
> > > > > of the extension, the application must select only one next proto.
> > > > > For example, the application can't select udp + tcp. There is
> > > > > the option to add a flag for each of the possible extensions,
> > > > > does it makes more
> > > sense to you?
> > > >
> > > > Each of these extension headers has its own structure, we first
> > > > need the ability to match them properly by adding the necessary
> pattern items.
> > > >
> > > > > > The RFC indirectly touches IPv6 proto (next header) matching
> > > > > > logic.
> > > > > >
> > > > > > If logic used in ETH+VLAN is applied on IPv6 as well, it would
> > > > > > make pattern specification and handling complicated. E.g.:
> > > > > >   eth / ipv6 / udp / end
> > > > > > should match UDP over IPv6 without any extension headers only.
> > > > > >
> > > > > The issue with VLAN I agree is different since by definition
> > > > > VLAN is layer 2.5. We can add the same logic also to the VLAN
> > > > > case, maybe it will be easier.
> > > > > In any case, in your example above and according to the RFC we
> > > > > will get all ipv6 udp traffic with and without extensions.
> > > > >
> > > > > > And how to specify UPD over IPv6 regardless extension headers?
> > > > >
> > > > > Please see above the rule will be eth / ipv6 /udp.
> > > > >
> > > > > >   eth / ipv6 / ipv6_ext / udp / end with a convention that
> > > > > > ipv6_ext is optional if spec and mask are NULL (or mask is empty).
> > > > > >
> > > > > I would guess that this flow should match all ipv6 that has one
> > > > > ext and the
> > > > next
> > > > > proto is udp.
> > > >
> > > > In my opinion RTE_FLOW_ITEM_TYPE_IPV6_EXT is a bit useless on its
> > own.
> > > > It's only for matching packets that contain some kind of extension
> > > > header, not a specific one, more about that below.
> > > >
> > > > > > I'm wondering if any driver treats it this way?
> > > > > >
> > > > > I'm not sure, we can support only the frag ext by default, but
> > > > > if required we
> > > > can support other
> > > > > ext.
> > > > >
> > > > > > I agree that the problem really comes when we'd like match
> > > > > > IPv6 frags or even worse not fragments.
> > > > > >
> > > > > > Two patterns for fragments:
> > > > > >   eth / ipv6 (proto=FRAGMENT) / end
> > > > > >   eth / ipv6 / ipv6_ext (next_hdr=FRAGMENT) / end
> > > > > >
> > > > > > Any sensible solution for not-fragments with any other
> > > > > > extension headers?
> > > > > >
> > > > > The one propose in this mail 😊
> > > > >
> > > > > > INVERT exists, but hardly useful, since it simply says that
> > > > > > patches which do not match pattern without INVERT matches the
> > > > > > pattern with INVERT and
> > > > > >   invert / eth / ipv6 (proto=FRAGMENT) / end will match ARP,
> > > > > > IPv4,
> > > > > > IPv6 with an extension header before fragment header and so on.
> > > > > >
> > > > > I agree with you, INVERT in this doesn’t help.
> > > > > We were considering adding some kind of not mask / item per item.
> > > > > some think around this line:
> > > > > user request ipv6 unfragmented udp packets. The flow would look
> > > > > something like this:
> > > > > Eth / ipv6 / Not (Ipv6.proto = frag_proto) / udp But it makes
> > > > > the rules much harder to use, and I don't think that there is
> > > > > any HW that support not, and adding such feature to all items is
> overkill.
> > > > >
> > > > >
> > > > > > Bit string suggested above will allow to match:
> > > > > >  - UDP over IPv6 with any extension headers:
> > > > > >     eth / ipv6 (ext_hdrs mask empty) / udp / end
> > > > > >  - UDP over IPv6 without any extension headers:
> > > > > >     eth / ipv6 (ext_hdrs mask full, spec empty) / udp / end
> > > > > >  - UDP over IPv6 without fragment header:
> > > > > >     eth / ipv6 (ext.spec & ~FRAGMENT, ext.mask | FRAGMENT) /
> > > > > > udp / end
> > > > > >  - UDP over IPv6 with fragment header
> > > > > >     eth / ipv6 (ext.spec | FRAGMENT, ext.mask | FRAGMENT) /
> > > > > > udp / end
> > > > > >
> > > > > > where FRAGMENT is 1 << IPPROTO_FRAGMENT.
> > > > > >
> > > > > Please see my response regarding this above.
> > > > >
> > > > > > Above I intentionally keep 'proto' unspecified in ipv6 since
> > > > > > otherwise it would specify the next header after IPv6 header.
> > > > > >
> > > > > > Extension headers mask should be empty by default.
> > > >
> > > > This is a deliberate design choice/issue with rte_flow: an empty
> > > > pattern matches everything; adding items only narrows the selection.
> > > > As Andrew said there is currently no way to provide a specific
> > > > item to reject, it can only be done globally on a pattern through
> > > > INVERT that no
> > > PMD implements so far.
> > > >
> > > > So we have two requirements here: the ability to specifically
> > > > match
> > > > IPv6 fragment headers and the ability to reject them.
> > > >
> > > > To match IPv6 fragment headers, we need a dedicated pattern item.
> > > > The generic RTE_FLOW_ITEM_TYPE_IPV6_EXT is useless for that on its
> > > > own, it must be completed with
> RTE_FLOW_ITEM_TYPE_IPV6_EXT_FRAG
> > and
> > > associated
> > > > object
> > >
> > > Yes, we must add EXT_FRAG to be able to match on the FRAG bits.
> > >
> >
> > Please see previous RFC I sent.
> > [RFC] ethdev: add IPv6 fragment extension header item
> > http://mails.dpdk.org/archives/dev/2020-March/160255.html
> > It is complemented by this RFC.
> >
> > > > to match individual fields if needed (like all the others
> > > > protocols/headers).
> > > >
> > > > Then to reject a pattern item... My preference goes to a new "NOT"
> > > > meta item affecting the meaning of the item coming immediately
> > > > after in the pattern list. That would be ultra generic, wouldn't
> > > > break any ABI/API and like INVERT, wouldn't even require a new
> > > > object associated
> > > with it.
> > > >
> > > > To match UDPv6 traffic when there is no fragment header, one could
> > > > then do something like:
> > > >
> > > >  eth / ipv6 / not / ipv6_ext_frag / udp
> > > >
> > > > PMD support would be trivial to implement (I'm sure!)
> > > >
> > > I agree with you as I said above. The issue is not PMD, the issues are:
> > > 1. think about the rule you stated above from logic point there is
> > > some contradiction, you are saying ipv6 next proto udp but you also
> > > say not frag, this is logic only for IPV6 ext.
> > > 2. HW issue, I don't know of HW that knows how to support not on an
> item.
> > > So adding something for all items for only one case is overkill.
> > >
> > >
> > >
> > > > We may later implement other kinds of "operator" items as Andrew
> > > > suggested, for bit-wise stuff and so on. Let's keep adding
> > > > features on a needed basis though.
> > > >
> > > > --
> > > > Adrien Mazarguil
> > > > 6WIND
> > >
> > > Best,
> > > Ori

Re: [dpdk-dev] [RFC] ethdev: add fragment attribute to IPv6 item

[Andrew Rybchenko]

On 6/2/20 10:04 PM, Adrien Mazarguil wrote:
> Hi Ori, Andrew, Delek,
> 
> (been a while eh?)
> 
> On Tue, Jun 02, 2020 at 06:28:41PM +0000, Ori Kam wrote:
>> Hi Andrew,
>>
>> PSB,
> [...]
>>>> diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
>>>> index b0e4199..3bc8ce1 100644
>>>> --- a/lib/librte_ethdev/rte_flow.h
>>>> +++ b/lib/librte_ethdev/rte_flow.h
>>>> @@ -787,6 +787,8 @@ struct rte_flow_item_ipv4 {
>>>>   */
>>>>  struct rte_flow_item_ipv6 {
>>>>  	struct rte_ipv6_hdr hdr; /**< IPv6 header definition. */
>>>> +	uint32_t is_frag:1; /**< Is IPv6 packet fragmented/non-fragmented. */
>>>> +	uint32_t reserved:31; /**< Reserved, must be zero. */
>>>
>>> The solution is simple, but hardly generic and adds an
>>> example for the future extensions. I doubt that it is a
>>> right way to go.
>>>
>> I agree with you that this is not the most generic way possible,
>> but the IPV6 extensions are very unique. So the solution is also unique.
>> In general, I'm always in favor of finding the most generic way, but sometimes
>> it is better to keep things simple, and see how it goes.
> 
> Same feeling here, it doesn't look right.
> 
>>> May be we should add 256-bit string with one bit for each
>>> IP protocol number and apply it to extension headers only?
>>> If bit A is set in the mask:
>>>  - if bit A is set in spec as well, extension header with
>>>    IP protocol (1 << A) number must present
>>>  - if bit A is clear in spec, extension header with
>>>    IP protocol (1 << A) number must absent
>>> If bit is clear in the mask, corresponding extension header
>>> may present and may absent (i.e. don't care).
>>>
>> There are only 12 possible extension headers and currently none of them
>> are supported in rte_flow. So adding a logic to parse the 256 just to get a max of 12 
>> possible values is an overkill. Also, if we disregard the case of the extension, 
>> the application must select only one next proto. For example, the application
>> can't select udp + tcp. There is the option to add a flag for each of the
>> possible extensions, does it makes more sense to you?
> 
> Each of these extension headers has its own structure, we first need the
> ability to match them properly by adding the necessary pattern items.
> 
>>> The RFC indirectly touches IPv6 proto (next header) matching
>>> logic.
>>>
>>> If logic used in ETH+VLAN is applied on IPv6 as well, it would
>>> make pattern specification and handling complicated. E.g.:
>>>   eth / ipv6 / udp / end
>>> should match UDP over IPv6 without any extension headers only.
>>>
>> The issue with VLAN I agree is different since by definition VLAN is 
>> layer 2.5. We can add the same logic also to the VLAN case, maybe it will
>> be easier. 
>> In any case, in your example above and according to the RFC we will
>> get all ipv6 udp traffic with and without extensions.
>>
>>> And how to specify UPD over IPv6 regardless extension headers?
>>
>> Please see above the rule will be eth / ipv6 /udp.
>>
>>>   eth / ipv6 / ipv6_ext / udp / end
>>> with a convention that ipv6_ext is optional if spec and mask
>>> are NULL (or mask is empty).
>>>
>> I would guess that this flow should match all ipv6 that has one ext and the next 
>> proto is udp.
> 
> In my opinion RTE_FLOW_ITEM_TYPE_IPV6_EXT is a bit useless on its own. It's
> only for matching packets that contain some kind of extension header, not a
> specific one, more about that below.
> 
>>> I'm wondering if any driver treats it this way?
>>>
>> I'm not sure, we can support only the frag ext by default, but if required we can support other 
>> ext.
>>  
>>> I agree that the problem really comes when we'd like match
>>> IPv6 frags or even worse not fragments.
>>>
>>> Two patterns for fragments:
>>>   eth / ipv6 (proto=FRAGMENT) / end
>>>   eth / ipv6 / ipv6_ext (next_hdr=FRAGMENT) / end
>>>
>>> Any sensible solution for not-fragments with any other
>>> extension headers?
>>>
>> The one propose in this mail 😊 
>>
>>> INVERT exists, but hardly useful, since it simply says
>>> that patches which do not match pattern without INVERT
>>> matches the pattern with INVERT and
>>>   invert / eth / ipv6 (proto=FRAGMENT) / end
>>> will match ARP, IPv4, IPv6 with an extension header before
>>> fragment header and so on.
>>>
>> I agree with you, INVERT in this doesn’t help.
>> We were considering adding some kind of not mask / item per item.
>> some think around this line:
>> user request ipv6 unfragmented udp packets. The flow would look something
>> like this:
>> Eth / ipv6 / Not (Ipv6.proto = frag_proto) / udp
>> But it makes the rules much harder to use, and I don't think that there
>> is any HW that support not, and adding such feature to all items is overkill.
>>
>>  
>>> Bit string suggested above will allow to match:
>>>  - UDP over IPv6 with any extension headers:
>>>     eth / ipv6 (ext_hdrs mask empty) / udp / end
>>>  - UDP over IPv6 without any extension headers:
>>>     eth / ipv6 (ext_hdrs mask full, spec empty) / udp / end
>>>  - UDP over IPv6 without fragment header:
>>>     eth / ipv6 (ext.spec & ~FRAGMENT, ext.mask | FRAGMENT) / udp / end
>>>  - UDP over IPv6 with fragment header
>>>     eth / ipv6 (ext.spec | FRAGMENT, ext.mask | FRAGMENT) / udp / end
>>>
>>> where FRAGMENT is 1 << IPPROTO_FRAGMENT.
>>>
>> Please see my response regarding this above.
>>
>>> Above I intentionally keep 'proto' unspecified in ipv6
>>> since otherwise it would specify the next header after IPv6
>>> header.
>>>
>>> Extension headers mask should be empty by default.
> 
> This is a deliberate design choice/issue with rte_flow: an empty pattern
> matches everything; adding items only narrows the selection. As Andrew said
> there is currently no way to provide a specific item to reject, it can only
> be done globally on a pattern through INVERT that no PMD implements so far.
> 
> So we have two requirements here: the ability to specifically match IPv6
> fragment headers and the ability to reject them.
> 

I think that one of key requirements here is an ability to say
that an extension header may be anywhere (or it must be no
extension header anywhere), since specification using a pattern
item suggests specified order of items, but it could be other
extension headers before frag header and after it before UDP protocol
header.

> To match IPv6 fragment headers, we need a dedicated pattern item. The
> generic RTE_FLOW_ITEM_TYPE_IPV6_EXT is useless for that on its own, it must
> be completed with RTE_FLOW_ITEM_TYPE_IPV6_EXT_FRAG and associated object
> to match individual fields if needed (like all the others
> protocols/headers).
> 

Yes, I agree, but it is strictly required if we want to match
on fragment header content or see it in exact place in next
protocols chain.

> Then to reject a pattern item... My preference goes to a new "NOT" meta item
> affecting the meaning of the item coming immediately after in the pattern
> list. That would be ultra generic, wouldn't break any ABI/API and like
> INVERT, wouldn't even require a new object associated with it.
> 

Yes, that's true, but I'm not sure if it is easy to do in HW.
Also, *NOT* scope could be per item field in fact, not whole
item. It sounds like it is getting more and more complicated.

> To match UDPv6 traffic when there is no fragment header, one could then do
> something like:
> 
>  eth / ipv6 / not / ipv6_ext_frag / udp
> 
> PMD support would be trivial to implement (I'm sure!)
> 

The problem is an interpretation of the above pattern.
Strictly speaking only UDP packets with exactly one
not frag extension header match the pattern.
What about packets without any extension headers?
Or packet with two (more) extension headers when the first
one is not frag header?

> We may later implement other kinds of "operator" items as Andrew suggested,
> for bit-wise stuff and so on. Let's keep adding features on a needed basis
> though.
> 

Thanks,
Andrew.

[dpdk-dev] [RFC v2] ethdev: add extensions attributes to IPv6 item

[Dekel Peled]

Using the current implementation of DPDK, an application cannot match on
IPv6 packets, based on the existing extension headers, in a simple way.

Field 'Next Header' in IPv6 header indicates type of the first extension
header only. Following extension headers can't be identified by
inspecting the IPv6 header.
As a result, the existence or absence of specific extension headers
can't be used for packet matching.

For example, fragmented IPv6 packets contain a dedicated extension header,
as detailed in RFC [1], which is not yet supported in rte_flow.
Non-fragmented packets don't contain the fragment extension header.
For an application to match on non-fragmented IPv6 packets, the current
implementation doesn't provide a suitable solution.
Matching on the Next Header field is not sufficient, since additional
extension headers might be present in the same packet.
To match on fragmented IPv6 packets, the same difficulty exists.

Proposed update:
A set of additional values will be added to IPv6 header struct.
These values will indicate the existence of every defined extension
header type, providing simple means for identification of existing
extensions in the packet header.
Continuing the above example, fragmented packets can be identified using
the specific value indicating existence of fragment extension header.

This update changes ABI, and is proposed for the 20.11 LTS version.

[1] http://mails.dpdk.org/archives/dev/2020-March/160255.html

Signed-off-by: Dekel Peled <dekelp@mellanox.com>
---
 lib/librte_ethdev/rte_flow.h | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
index da8bfa5..8d2073d 100644
--- a/lib/librte_ethdev/rte_flow.h
+++ b/lib/librte_ethdev/rte_flow.h
@@ -792,11 +792,33 @@ struct rte_flow_item_ipv4 {
  *
  * Matches an IPv6 header.
  *
+ * Dedicated flags indicate existence of specific extension headers.
+ *
  * Note: IPv6 options are handled by dedicated pattern items, see
  * RTE_FLOW_ITEM_TYPE_IPV6_EXT.
  */
 struct rte_flow_item_ipv6 {
 	struct rte_ipv6_hdr hdr; /**< IPv6 header definition. */
+	uint64_t hop_ext_exist:1;
+	/**< Hop-by-Hop Options extension header exists. */
+        uint64_t rout_ext_exist:1;
+        /**< Routing extension header exists. */
+        uint64_t frag_ext_exist:1;
+        /**< Fragment extension header exists. */
+        uint64_t auth_ext_exist:1;
+        /**< Authentication extension header exists. */
+        uint64_t esp_ext_exist:1;
+        /**< Encapsulation Security Payload extension header exists. */
+        uint64_t dest_ext_exist:1;
+        /**< Destination Options extension header exists. */
+        uint64_t mobil_ext_exist:1;
+        /**< Mobility extension header exists. */
+        uint64_t hip_ext_exist:1;
+        /**< Host Identity Protocol extension header exists. */
+        uint64_t shim6_ext_exist:1;
+        /**< Shim6 Protocol extension header exists. */
+        uint64_t reserved:55;
+        /**< Reserved for future extension headers, must be zero. */
 };
 
 /** Default mask for RTE_FLOW_ITEM_TYPE_IPV6. */
-- 
1.8.3.1

[dpdk-dev] [RFC v3] ethdev: add extensions attributes to IPv6 item

[Dekel Peled]

Using the current implementation of DPDK, an application cannot match on
IPv6 packets, based on the existing extension headers, in a simple way.

Field 'Next Header' in IPv6 header indicates type of the first extension
header only. Following extension headers can't be identified by
inspecting the IPv6 header.
As a result, the existence or absence of specific extension headers
can't be used for packet matching.

For example, fragmented IPv6 packets contain a dedicated extension header,
as detailed in RFC [1], which is not yet supported in rte_flow.
Non-fragmented packets don't contain the fragment extension header.
For an application to match on non-fragmented IPv6 packets, the current
implementation doesn't provide a suitable solution.
Matching on the Next Header field is not sufficient, since additional
extension headers might be present in the same packet.
To match on fragmented IPv6 packets, the same difficulty exists.

Proposed update:
A set of additional values will be added to IPv6 header struct.
These values will indicate the existence of every defined extension
header type, providing simple means for identification of existing
extensions in the packet header.
Continuing the above example, fragmented packets can be identified using
the specific value indicating existence of fragment extension header.

This update changes ABI, and is proposed for the 20.11 LTS version.

[1] http://mails.dpdk.org/archives/dev/2020-March/160255.html

---
v3: Fix checkpatch comments.
v2: Update from fragment attribute to all extensions attributes.
---

Signed-off-by: Dekel Peled <dekelp@mellanox.com>
---
 lib/librte_ethdev/rte_flow.h | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
index da8bfa5..246918e 100644
--- a/lib/librte_ethdev/rte_flow.h
+++ b/lib/librte_ethdev/rte_flow.h
@@ -792,11 +792,33 @@ struct rte_flow_item_ipv4 {
  *
  * Matches an IPv6 header.
  *
+ * Dedicated flags indicate existence of specific extension headers.
+ *
  * Note: IPv6 options are handled by dedicated pattern items, see
  * RTE_FLOW_ITEM_TYPE_IPV6_EXT.
  */
 struct rte_flow_item_ipv6 {
 	struct rte_ipv6_hdr hdr; /**< IPv6 header definition. */
+	uint64_t hop_ext_exist:1;
+	/**< Hop-by-Hop Options extension header exists. */
+	uint64_t rout_ext_exist:1;
+	/**< Routing extension header exists. */
+	uint64_t frag_ext_exist:1;
+	/**< Fragment extension header exists. */
+	uint64_t auth_ext_exist:1;
+	/**< Authentication extension header exists. */
+	uint64_t esp_ext_exist:1;
+	/**< Encapsulation Security Payload extension header exists. */
+	uint64_t dest_ext_exist:1;
+	/**< Destination Options extension header exists. */
+	uint64_t mobil_ext_exist:1;
+	/**< Mobility extension header exists. */
+	uint64_t hip_ext_exist:1;
+	/**< Host Identity Protocol extension header exists. */
+	uint64_t shim6_ext_exist:1;
+	/**< Shim6 Protocol extension header exists. */
+	uint64_t reserved:55;
+	/**< Reserved for future extension headers, must be zero. */
 };
 
 /** Default mask for RTE_FLOW_ITEM_TYPE_IPV6. */
-- 
1.8.3.1