[PATCH] ethdev: add roughly match pattern

[PATCH] ethdev: add roughly match pattern

[Qi Zhang]

Add new meta pattern item RTE_FLOW_TYPE_ITEM_ROUGHLY.

This is for device that support no-perfect match option.
Usually a no-perfect match is fast but the cost is accuracy.
i.e. Signature Match only match pattern's hash value, but it is
possible two different patterns have the same hash value.

Matching accuracy level can be configure by subfield threshold.
Driver can divide the range of threshold and map to different
accuracy levels that device support.

Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
---
 app/test-pmd/cmdline_flow.c                 | 24 ++++++++++++++
 app/test-pmd/config.c                       |  1 +
 doc/guides/prog_guide/rte_flow.rst          | 50 +++++++++++++++++++++++++++++
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |  2 ++
 lib/librte_ether/rte_flow.h                 | 30 +++++++++++++++++
 5 files changed, 107 insertions(+)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 0fd69f9..18ffcff 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -107,6 +107,8 @@ enum index {
 	ITEM_END,
 	ITEM_VOID,
 	ITEM_INVERT,
+	ITEM_ROUGHLY,
+	ITEM_ROUGHLY_THRESHOLD,
 	ITEM_ANY,
 	ITEM_ANY_NUM,
 	ITEM_PF,
@@ -426,6 +428,7 @@ static const enum index next_item[] = {
 	ITEM_END,
 	ITEM_VOID,
 	ITEM_INVERT,
+	ITEM_ROUGHLY,
 	ITEM_ANY,
 	ITEM_PF,
 	ITEM_VF,
@@ -447,6 +450,12 @@ static const enum index next_item[] = {
 	ZERO,
 };
 
+static const enum index item_roughly[] = {
+	ITEM_ROUGHLY_THRESHOLD,
+	ITEM_NEXT,
+	ZERO,
+};
+
 static const enum index item_any[] = {
 	ITEM_ANY_NUM,
 	ITEM_NEXT,
@@ -954,6 +963,21 @@ static const struct token token_list[] = {
 		.next = NEXT(NEXT_ENTRY(ITEM_NEXT)),
 		.call = parse_vc,
 	},
+	[ITEM_ROUGHLY] = {
+		.name = "roughly",
+		.help = "match the pattern roughly",
+		.priv = PRIV_ITEM(ROUGHLY,
+				sizeof(struct rte_flow_item_roughly)),
+		.next = NEXT(item_roughly),
+		.call = parse_vc,
+	},
+	[ITEM_ROUGHLY_THRESHOLD] = {
+		.name = "threshold",
+		.help = "match accuracy threshold",
+		.next = NEXT(item_roughly, NEXT_ENTRY(UNSIGNED), item_param),
+		.args = ARGS(ARGS_ENTRY(struct rte_flow_item_roughly,
+					threshold)),
+	},
 	[ITEM_ANY] = {
 		.name = "any",
 		.help = "match any protocol for the current layer",
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 4d873cd..5b0cd4d 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -954,6 +954,7 @@ static const struct {
 	MK_FLOW_ITEM(END, 0),
 	MK_FLOW_ITEM(VOID, 0),
 	MK_FLOW_ITEM(INVERT, 0),
+	MK_FLOW_ITEM(ROUGHLY, sizeof(struct rte_flow_item_roughly)),
 	MK_FLOW_ITEM(ANY, sizeof(struct rte_flow_item_any)),
 	MK_FLOW_ITEM(PF, 0),
 	MK_FLOW_ITEM(VF, sizeof(struct rte_flow_item_vf)),
diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
index b587ba9..4cc1876 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -491,6 +491,7 @@ Usage example, matching non-TCPv4 packets only:
 
    +-------+----------+
    | Index | Item     |
+
    +=======+==========+
    | 0     | INVERT   |
    +-------+----------+
@@ -503,6 +504,55 @@ Usage example, matching non-TCPv4 packets only:
    | 4     | END      |
    +-------+----------+
 
+Item: ``ROUGHLY``
+^^^^^^^^^^^^^^^^^
+
+Roughly matching, not perfect match.
+
+This is for device that support no-perfect match option.
+Usually a no-perfect match is fast but the cost is accuracy.
+i.e. Signature Match only match pattern's hash value, but it is
+possible two different patterns have the same hash value.
+
+Matching accuracy level can be configure by threshold.
+Driver can divide the range of threshold and map to different
+accuracy levels that device support.
+
+.. _table_rte_flow_item_roughly:
+
+.. table:: ROUGHLY
+
+   +----------+---------------+--------------------------------------------------+
+   | Field    |   Subfield    | Value                                            |
+   +==========+===========+======================================================+
+   | ``spec`` | ``threshold`` | 0 as perfect match, 0xffffffff as roughest match |
+   +----------+---------------+--------------------------------------------------+
+   | ``last`` | ``threshold`` | ignored                                          |
+   +----------+-----------+------------------------------------------------------+
+   | ``mask`` | ``threshold`` | ignored                                          |
+   +----------+-----------+------------------------------------------------------+
+
+
+Usage example, roughly match a TCPv4 packets:
+
+.. _table_rte_flow_item_roughly_example:
+
+.. table:: Roughly matching
+
+   +-------+----------+
+   | Index | Item     |
+   +=======+==========+
+   | 0     | ROUGHLY  |
+   +-------+----------+
+   | 1     | Ethernet |
+   +-------+----------+
+   | 2     | IPv4     |
+   +-------+----------+
+   | 3     | TCP      |
+   +-------+----------+
+   | 4     | END      |
+   +-------+----------+
+
 Item: ``PF``
 ^^^^^^^^^^^^
 
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index 0e50c10..08a88f8 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -2513,6 +2513,8 @@ This section lists supported pattern items and their attributes, if any.
 
 - ``invert``: perform actions when pattern does not match.
 
+- ``roughly``: pattern is matched roughly.
+
 - ``any``: match any protocol for the current layer.
 
   - ``num {unsigned}``: number of layers covered.
diff --git a/lib/librte_ether/rte_flow.h b/lib/librte_ether/rte_flow.h
index c47edbc..4921858 100644
--- a/lib/librte_ether/rte_flow.h
+++ b/lib/librte_ether/rte_flow.h
@@ -148,6 +148,18 @@ enum rte_flow_item_type {
 	RTE_FLOW_ITEM_TYPE_INVERT,
 
 	/**
+	 * [META]
+	 *
+	 * Roughly matching, not perfect matching
+	 *
+	 * This is for device that support no-perfect matching option.
+	 * Usually a no-perfect matching is fast but the cost is accuracy.
+	 *
+	 * See struct rte_flow_item_roughly
+	 */
+	RTE_FLOW_ITEM_TYPE_ROUGHLY,
+
+	/**
 	 * Matches any protocol in place of the current layer, a single ANY
 	 * may also stand for several protocol layers.
 	 *
@@ -300,6 +312,24 @@ enum rte_flow_item_type {
 };
 
 /**
+ * RTE_FLOW_ITEM_TYPE_ROUGHLY
+ *
+ * Roughly matching, not perfect match.
+ *
+ * This is for device that support no-perfect match option.
+ * Usually a no-perfect match is fast but the cost is accuracy.
+ * i.e. Signature Match only match pattern's hash value, but it is
+ * possible two different patterns have the same hash value.
+ *
+ * Matching accuracy level can be configure by threshold.
+ * Driver can divide the range of threshold and map to different
+ * accuracy levels that device support.
+ */
+struct rte_flow_item_roughly {
+	uint32_t threshold; /**< accuracy threshold*/
+};
+
+/**
  * RTE_FLOW_ITEM_TYPE_ANY
  *
  * Matches any protocol in place of the current layer, a single ANY may also
-- 
2.7.4

Re: [PATCH] ethdev: add roughly match pattern

[Adrien Mazarguil]

Hi Zhang,

You should cram "flow API" somewhere in the title of such commits.

On Tue, May 23, 2017 at 07:28:54PM -0400, Qi Zhang wrote:
> Add new meta pattern item RTE_FLOW_TYPE_ITEM_ROUGHLY.
> 
> This is for device that support no-perfect match option.
> Usually a no-perfect match is fast but the cost is accuracy.
> i.e. Signature Match only match pattern's hash value, but it is
> possible two different patterns have the same hash value.
> 
> Matching accuracy level can be configure by subfield threshold.
> Driver can divide the range of threshold and map to different
> accuracy levels that device support.
> 
> Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>

While I really like the "roughly" pattern item name since it perfectly
describes its intended purpose in my opinion, perhaps some may not find this
name appropriate. I would like to hear other people's opinion on the matter
and not be the only one to ack this patch.

Several more comments below.

> ---
>  app/test-pmd/cmdline_flow.c                 | 24 ++++++++++++++
>  app/test-pmd/config.c                       |  1 +
>  doc/guides/prog_guide/rte_flow.rst          | 50 +++++++++++++++++++++++++++++
>  doc/guides/testpmd_app_ug/testpmd_funcs.rst |  2 ++
>  lib/librte_ether/rte_flow.h                 | 30 +++++++++++++++++
>  5 files changed, 107 insertions(+)
> 
> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
> index 0fd69f9..18ffcff 100644
> --- a/app/test-pmd/cmdline_flow.c
> +++ b/app/test-pmd/cmdline_flow.c
> @@ -107,6 +107,8 @@ enum index {
>  	ITEM_END,
>  	ITEM_VOID,
>  	ITEM_INVERT,
> +	ITEM_ROUGHLY,
> +	ITEM_ROUGHLY_THRESHOLD,

"Threshold" is commonly abbreviated as "thresh", I think it's fine if you
use this shorter form here and in the structure definition.

There is also an issue with the position of these enum entries. They should
come in the same order as rte_flow.h definitions like you did, but you are
supposed to add new entries at the end of the various lists in that file
instead of in the middle, otherwise you're destroying API/ABI compatibility
which is not supposed to happen. More on that topic below.

>  	ITEM_ANY,
>  	ITEM_ANY_NUM,
>  	ITEM_PF,
> @@ -426,6 +428,7 @@ static const enum index next_item[] = {
>  	ITEM_END,
>  	ITEM_VOID,
>  	ITEM_INVERT,
> +	ITEM_ROUGHLY,

This will have to be moved at the end of the list.

>  	ITEM_ANY,
>  	ITEM_PF,
>  	ITEM_VF,
> @@ -447,6 +450,12 @@ static const enum index next_item[] = {
>  	ZERO,
>  };
>  
> +static const enum index item_roughly[] = {
> +	ITEM_ROUGHLY_THRESHOLD,

I suggest "ITEM_ROUGHLY_THRESH".

> +	ITEM_NEXT,
> +	ZERO,
> +};
> +
>  static const enum index item_any[] = {
>  	ITEM_ANY_NUM,
>  	ITEM_NEXT,
> @@ -954,6 +963,21 @@ static const struct token token_list[] = {
>  		.next = NEXT(NEXT_ENTRY(ITEM_NEXT)),
>  		.call = parse_vc,
>  	},
> +	[ITEM_ROUGHLY] = {

This will have to be moved at the end of the list.

> +		.name = "roughly",
> +		.help = "match the pattern roughly",

Hehe, the question is who will go out of their way to match traffic roughly
instead of perfectly? They need a better incentive to do so, in a very short
sentence.

> +		.priv = PRIV_ITEM(ROUGHLY,
> +				sizeof(struct rte_flow_item_roughly)),
> +		.next = NEXT(item_roughly),
> +		.call = parse_vc,
> +	},
> +	[ITEM_ROUGHLY_THRESHOLD] = {
> +		.name = "threshold",

"thresh" again, I won't comment them all, you get the idea.

> +		.help = "match accuracy threshold",
> +		.next = NEXT(item_roughly, NEXT_ENTRY(UNSIGNED), item_param),
> +		.args = ARGS(ARGS_ENTRY(struct rte_flow_item_roughly,
> +					threshold)),
> +	},
>  	[ITEM_ANY] = {
>  		.name = "any",
>  		.help = "match any protocol for the current layer",
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> index 4d873cd..5b0cd4d 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -954,6 +954,7 @@ static const struct {
>  	MK_FLOW_ITEM(END, 0),
>  	MK_FLOW_ITEM(VOID, 0),
>  	MK_FLOW_ITEM(INVERT, 0),
> +	MK_FLOW_ITEM(ROUGHLY, sizeof(struct rte_flow_item_roughly)),

This will have to be moved at the end of the list.

>  	MK_FLOW_ITEM(ANY, sizeof(struct rte_flow_item_any)),
>  	MK_FLOW_ITEM(PF, 0),
>  	MK_FLOW_ITEM(VF, sizeof(struct rte_flow_item_vf)),
> diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
> index b587ba9..4cc1876 100644
> --- a/doc/guides/prog_guide/rte_flow.rst
> +++ b/doc/guides/prog_guide/rte_flow.rst
> @@ -491,6 +491,7 @@ Usage example, matching non-TCPv4 packets only:
>  
>     +-------+----------+
>     | Index | Item     |
> +

This change looks unnecessary.

>     +=======+==========+
>     | 0     | INVERT   |
>     +-------+----------+
> @@ -503,6 +504,55 @@ Usage example, matching non-TCPv4 packets only:
>     | 4     | END      |
>     +-------+----------+
>  
> +Item: ``ROUGHLY``
> +^^^^^^^^^^^^^^^^^

This will have to be moved at the end of the list (documentation also
follows the same order as rte_flow.h).

> +
> +Roughly matching, not perfect match.
> +
> +This is for device that support no-perfect match option.
> +Usually a no-perfect match is fast but the cost is accuracy.
> +i.e. Signature Match only match pattern's hash value, but it is
> +possible two different patterns have the same hash value.
> +
> +Matching accuracy level can be configure by threshold.
> +Driver can divide the range of threshold and map to different
> +accuracy levels that device support.

Please expand these paragraphs to fit 75-79 columns wide like the rest of
the file.

I think a better wording is necessary to provide incentives for applications
to use this mode. I have a few ideas but I'm not familiar enough with the
original signature mode for that. Perhaps John can help?

> +
> +.. _table_rte_flow_item_roughly:
> +
> +.. table:: ROUGHLY
> +
> +   +----------+---------------+--------------------------------------------------+
> +   | Field    |   Subfield    | Value                                            |
> +   +==========+===========+======================================================+
> +   | ``spec`` | ``threshold`` | 0 as perfect match, 0xffffffff as roughest match |
> +   +----------+---------------+--------------------------------------------------+
> +   | ``last`` | ``threshold`` | ignored                                          |
> +   +----------+-----------+------------------------------------------------------+
> +   | ``mask`` | ``threshold`` | ignored                                          |
> +   +----------+-----------+------------------------------------------------------+

Last and mask cannot be ignored. The only items where they are ignored are
those that do not even take a spec definition.

This means that a mask set to 0 is supposed to be the same as no item
provided. PMDs should retrieve the threshold value using something like:

 thresh = spec->thresh & mask->thresh;
 if (last->thresh && (last->thresh & mask->thresh) < thresh)
     complain_unsupported();

Ranges (last) can be ignored when otherwise valid because what matters is
only the lowest threshold value.

See 8.2.3 Pattern item [1] for more information.

> +

Extra empty line.

> +
> +Usage example, roughly match a TCPv4 packets:
> +
> +.. _table_rte_flow_item_roughly_example:
> +
> +.. table:: Roughly matching

How about "Rough matching".

> +
> +   +-------+----------+
> +   | Index | Item     |
> +   +=======+==========+
> +   | 0     | ROUGHLY  |
> +   +-------+----------+
> +   | 1     | Ethernet |
> +   +-------+----------+
> +   | 2     | IPv4     |
> +   +-------+----------+
> +   | 3     | TCP      |
> +   +-------+----------+
> +   | 4     | END      |
> +   +-------+----------+
> +
>  Item: ``PF``
>  ^^^^^^^^^^^^

There is a missing change in this file. You must modify the following
statement:

 - Signature mode of operation is not defined but could be handled through a
   specific item type if needed.

And mention ROUGHLY in index 3 of the table below:

    +---+-------------------+----------+-----+                       |
    | 3 | VF, PF (optional) | ``spec`` | any |                       |
    |   |                   +----------+-----+                       |
    |   |                   | ``last`` | N/A |                       |
    |   |                   +----------+-----+                       |
    |   |                   | ``mask`` | any |                       |
    +---+-------------------+----------+-----+                       |

> diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> index 0e50c10..08a88f8 100644
> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> @@ -2513,6 +2513,8 @@ This section lists supported pattern items and their attributes, if any.
>  
>  - ``invert``: perform actions when pattern does not match.
>  
> +- ``roughly``: pattern is matched roughly.
> +

How about "match pattern roughly"? (remember to keep this in sync with
testpmd's inline help string though)

This will also have to be moved at the end of the list.

>  - ``any``: match any protocol for the current layer.
>  
>    - ``num {unsigned}``: number of layers covered.
> diff --git a/lib/librte_ether/rte_flow.h b/lib/librte_ether/rte_flow.h
> index c47edbc..4921858 100644
> --- a/lib/librte_ether/rte_flow.h
> +++ b/lib/librte_ether/rte_flow.h
> @@ -148,6 +148,18 @@ enum rte_flow_item_type {
>  	RTE_FLOW_ITEM_TYPE_INVERT,
>  
>  	/**
> +	 * [META]
> +	 *
> +	 * Roughly matching, not perfect matching
> +	 *
> +	 * This is for device that support no-perfect matching option.
> +	 * Usually a no-perfect matching is fast but the cost is accuracy.

Perhaps John can help here as well. This one should be kept mostly in sync
with the full description for struct rte_flow_item_roughly and also the
documentation in rte_flow.rst.

> +	 *
> +	 * See struct rte_flow_item_roughly

Missing colon.

> +	 */
> +	RTE_FLOW_ITEM_TYPE_ROUGHLY,
> +

This new item type *must* be moved at the end of the list to avoid breaking
API/ABI and existing applications. Remember it's append-only.

You then need to update the rest of the patch accordingly.

> +	/**
>  	 * Matches any protocol in place of the current layer, a single ANY
>  	 * may also stand for several protocol layers.
>  	 *
> @@ -300,6 +312,24 @@ enum rte_flow_item_type {
>  };
>  
>  /**
> + * RTE_FLOW_ITEM_TYPE_ROUGHLY
> + *
> + * Roughly matching, not perfect match.
> + *
> + * This is for device that support no-perfect match option.
> + * Usually a no-perfect match is fast but the cost is accuracy.
> + * i.e. Signature Match only match pattern's hash value, but it is
> + * possible two different patterns have the same hash value.
> + *
> + * Matching accuracy level can be configure by threshold.
> + * Driver can divide the range of threshold and map to different
> + * accuracy levels that device support.

John?

> + */
> +struct rte_flow_item_roughly {
> +	uint32_t threshold; /**< accuracy threshold*/

How about:

 uint32_t thresh; /**< Accuracy threshold. */

> +};

Again this structure must be defined after all the others to keep the same
order as enum rte_flow_item_type.

> +
> +/**
>   * RTE_FLOW_ITEM_TYPE_ANY
>   *
>   * Matches any protocol in place of the current layer, a single ANY may also
> -- 
> 2.7.4
> 

[1] http://dpdk.org/doc/guides/prog_guide/rte_flow.html#pattern-item

-- 
Adrien Mazarguil
6WIND

Re: [PATCH] ethdev: add roughly match pattern

[Gaëtan Rivet]

On Tue, May 30, 2017 at 02:46:30PM +0200, Adrien Mazarguil wrote:
>Hi Zhang,
>
>You should cram "flow API" somewhere in the title of such commits.
>
>On Tue, May 23, 2017 at 07:28:54PM -0400, Qi Zhang wrote:
>> Add new meta pattern item RTE_FLOW_TYPE_ITEM_ROUGHLY.
>>
>> This is for device that support no-perfect match option.
>> Usually a no-perfect match is fast but the cost is accuracy.
>> i.e. Signature Match only match pattern's hash value, but it is
>> possible two different patterns have the same hash value.
>>
>> Matching accuracy level can be configure by subfield threshold.
>> Driver can divide the range of threshold and map to different
>> accuracy levels that device support.
>>
>> Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
>
>While I really like the "roughly" pattern item name since it perfectly
>describes its intended purpose in my opinion, perhaps some may not find this
>name appropriate. I would like to hear other people's opinion on the matter
>and not be the only one to ack this patch.

"no-perfect" has been used a few times in the documentation. How about
"IMPERFECT" as item name?

-- 
Gaëtan Rivet
6WIND

Re: [PATCH] ethdev: add roughly match pattern

[Zhang, Qi Z]

> -----Original Message-----
> From: Gaëtan Rivet [mailto:gaetan.rivet@6wind.com]
> Sent: Wednesday, May 31, 2017 3:52 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>; dev@dpdk.org
> Cc: Adrien Mazarguil <adrien.mazarguil@6wind.com>; Mcnamara, John
> <john.mcnamara@intel.com>
> Subject: Re: [dpdk-dev] [PATCH] ethdev: add roughly match pattern
> 
> On Tue, May 30, 2017 at 02:46:30PM +0200, Adrien Mazarguil wrote:
> >Hi Zhang,
> >
> >You should cram "flow API" somewhere in the title of such commits.
> >
> >On Tue, May 23, 2017 at 07:28:54PM -0400, Qi Zhang wrote:
> >> Add new meta pattern item RTE_FLOW_TYPE_ITEM_ROUGHLY.
> >>
> >> This is for device that support no-perfect match option.
> >> Usually a no-perfect match is fast but the cost is accuracy.
> >> i.e. Signature Match only match pattern's hash value, but it is
> >> possible two different patterns have the same hash value.
> >>
> >> Matching accuracy level can be configure by subfield threshold.
> >> Driver can divide the range of threshold and map to different
> >> accuracy levels that device support.
> >>
> >> Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
> >
> >While I really like the "roughly" pattern item name since it perfectly
> >describes its intended purpose in my opinion, perhaps some may not find
> >this name appropriate. I would like to hear other people's opinion on
> >the matter and not be the only one to ack this patch.
> 
> "no-perfect" has been used a few times in the documentation. How about
> "IMPERFECT" as item name?
> 
"Imperfect" looks better for me, 
If no other objection, I will use this in V2.

Thanks
Qi
> --
> Gaëtan Rivet
> 6WIND

Re: [PATCH] ethdev: add roughly match pattern

[Adrien Mazarguil]

On Thu, Jun 01, 2017 at 01:44:56AM +0000, Zhang, Qi Z wrote:
> > -----Original Message-----
> > From: Gaëtan Rivet [mailto:gaetan.rivet@6wind.com]
> > Sent: Wednesday, May 31, 2017 3:52 PM
> > To: Zhang, Qi Z <qi.z.zhang@intel.com>; dev@dpdk.org
> > Cc: Adrien Mazarguil <adrien.mazarguil@6wind.com>; Mcnamara, John
> > <john.mcnamara@intel.com>
> > Subject: Re: [dpdk-dev] [PATCH] ethdev: add roughly match pattern
> > 
> > On Tue, May 30, 2017 at 02:46:30PM +0200, Adrien Mazarguil wrote:
> > >Hi Zhang,
> > >
> > >You should cram "flow API" somewhere in the title of such commits.
> > >
> > >On Tue, May 23, 2017 at 07:28:54PM -0400, Qi Zhang wrote:
> > >> Add new meta pattern item RTE_FLOW_TYPE_ITEM_ROUGHLY.
> > >>
> > >> This is for device that support no-perfect match option.
> > >> Usually a no-perfect match is fast but the cost is accuracy.
> > >> i.e. Signature Match only match pattern's hash value, but it is
> > >> possible two different patterns have the same hash value.
> > >>
> > >> Matching accuracy level can be configure by subfield threshold.
> > >> Driver can divide the range of threshold and map to different
> > >> accuracy levels that device support.
> > >>
> > >> Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
> > >
> > >While I really like the "roughly" pattern item name since it perfectly
> > >describes its intended purpose in my opinion, perhaps some may not find
> > >this name appropriate. I would like to hear other people's opinion on
> > >the matter and not be the only one to ack this patch.
> > 
> > "no-perfect" has been used a few times in the documentation. How about
> > "IMPERFECT" as item name?
> > 
> "Imperfect" looks better for me, 
> If no other objection, I will use this in V2.

An "imperfect threshold" doesn't make a lot more sense than a roughness
one. Who wants to match flows imperfectly by the way?

How about "fuzzy" then? Fuzzy matching, fuzzy thresholds, those are pretty
well understood concepts. And people like fuzzy things.

-- 
Adrien Mazarguil
6WIND

[PATCH v2] ethdev: add fuzzy match pattern

[Qi Zhang]

Add new meta pattern item RTE_FLOW_TYPE_ITEM_FUZZY.

This is for device that support fuzzy  match option.
Usually a fuzzy match is fast but the cost is accuracy.
i.e. Signature Match only match pattern's hash value, but it is
possible two different patterns have the same hash value.

Matching accuracy level can be configure by subfield threshold.
Driver can divide the range of threshold and map to different
accuracy levels that device support.

Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
---

v2:
- replace "Roughly" with "Fuzzy"

 app/test-pmd/cmdline_flow.c                 | 24 ++++++++++++++
 app/test-pmd/config.c                       |  1 +
 doc/guides/prog_guide/rte_flow.rst          | 50 +++++++++++++++++++++++++++++
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |  2 ++
 lib/librte_ether/rte_flow.h                 | 30 +++++++++++++++++
 5 files changed, 107 insertions(+)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 0fd69f9..048d455 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -107,6 +107,8 @@ enum index {
 	ITEM_END,
 	ITEM_VOID,
 	ITEM_INVERT,
+	ITEM_FUZZY,
+	ITEM_FUZZY_THRESHOLD,
 	ITEM_ANY,
 	ITEM_ANY_NUM,
 	ITEM_PF,
@@ -426,6 +428,7 @@ static const enum index next_item[] = {
 	ITEM_END,
 	ITEM_VOID,
 	ITEM_INVERT,
+	ITEM_FUZZY,
 	ITEM_ANY,
 	ITEM_PF,
 	ITEM_VF,
@@ -447,6 +450,12 @@ static const enum index next_item[] = {
 	ZERO,
 };
 
+static const enum index item_fuzzy[] = {
+	ITEM_FUZZY_THRESHOLD,
+	ITEM_NEXT,
+	ZERO,
+};
+
 static const enum index item_any[] = {
 	ITEM_ANY_NUM,
 	ITEM_NEXT,
@@ -954,6 +963,21 @@ static const struct token token_list[] = {
 		.next = NEXT(NEXT_ENTRY(ITEM_NEXT)),
 		.call = parse_vc,
 	},
+	[ITEM_FUZZY] = {
+		.name = "fuzzy",
+		.help = "match the pattern fuzzy",
+		.priv = PRIV_ITEM(FUZZY,
+				sizeof(struct rte_flow_item_fuzzy)),
+		.next = NEXT(item_fuzzy),
+		.call = parse_vc,
+	},
+	[ITEM_FUZZY_THRESHOLD] = {
+		.name = "threshold",
+		.help = "match accuracy threshold",
+		.next = NEXT(item_fuzzy, NEXT_ENTRY(UNSIGNED), item_param),
+		.args = ARGS(ARGS_ENTRY(struct rte_flow_item_fuzzy,
+					threshold)),
+	},
 	[ITEM_ANY] = {
 		.name = "any",
 		.help = "match any protocol for the current layer",
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 4d873cd..db7b740 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -954,6 +954,7 @@ static const struct {
 	MK_FLOW_ITEM(END, 0),
 	MK_FLOW_ITEM(VOID, 0),
 	MK_FLOW_ITEM(INVERT, 0),
+	MK_FLOW_ITEM(FUZZY, sizeof(struct rte_flow_item_fuzzy)),
 	MK_FLOW_ITEM(ANY, sizeof(struct rte_flow_item_any)),
 	MK_FLOW_ITEM(PF, 0),
 	MK_FLOW_ITEM(VF, sizeof(struct rte_flow_item_vf)),
diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
index b587ba9..516a0ac 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -491,6 +491,7 @@ Usage example, matching non-TCPv4 packets only:
 
    +-------+----------+
    | Index | Item     |
+
    +=======+==========+
    | 0     | INVERT   |
    +-------+----------+
@@ -503,6 +504,55 @@ Usage example, matching non-TCPv4 packets only:
    | 4     | END      |
    +-------+----------+
 
+Item: ``FUZZY``
+^^^^^^^^^^^^^^^^^
+
+Fuzzy matching, not perfect match.
+
+This is for device that support fuzzy match option.
+Usually a fuzzy  match is fast but the cost is accuracy.
+i.e. Signature Match only match pattern's hash value, but it is
+possible two different patterns have the same hash value.
+
+Matching accuracy level can be configure by threshold.
+Driver can divide the range of threshold and map to different
+accuracy levels that device support.
+
+.. _table_rte_flow_item_fuzzy:
+
+.. table:: FUZZY
+
+   +----------+---------------+--------------------------------------------------+
+   | Field    |   Subfield    | Value                                            |
+   +==========+===========+======================================================+
+   | ``spec`` | ``threshold`` | 0 as perfect match, 0xffffffff as fuzzies match |
+   +----------+---------------+--------------------------------------------------+
+   | ``last`` | ``threshold`` | ignored                                          |
+   +----------+-----------+------------------------------------------------------+
+   | ``mask`` | ``threshold`` | ignored                                          |
+   +----------+-----------+------------------------------------------------------+
+
+
+Usage example, fuzzy match a TCPv4 packets:
+
+.. _table_rte_flow_item_fuzzy_example:
+
+.. table:: Fuzzy matching
+
+   +-------+----------+
+   | Index | Item     |
+   +=======+==========+
+   | 0     | FUZZY    |
+   +-------+----------+
+   | 1     | Ethernet |
+   +-------+----------+
+   | 2     | IPv4     |
+   +-------+----------+
+   | 3     | TCP      |
+   +-------+----------+
+   | 4     | END      |
+   +-------+----------+
+
 Item: ``PF``
 ^^^^^^^^^^^^
 
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index 0e50c10..0b1d927 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -2513,6 +2513,8 @@ This section lists supported pattern items and their attributes, if any.
 
 - ``invert``: perform actions when pattern does not match.
 
+- ``fuzzy``: this is a fuzzy match..
+
 - ``any``: match any protocol for the current layer.
 
   - ``num {unsigned}``: number of layers covered.
diff --git a/lib/librte_ether/rte_flow.h b/lib/librte_ether/rte_flow.h
index c47edbc..b27e858 100644
--- a/lib/librte_ether/rte_flow.h
+++ b/lib/librte_ether/rte_flow.h
@@ -148,6 +148,18 @@ enum rte_flow_item_type {
 	RTE_FLOW_ITEM_TYPE_INVERT,
 
 	/**
+	 * [META]
+	 *
+	 * Fuzzy matching, not perfect matching
+	 *
+	 * This is for device that support fuzzy matching option.
+	 * Usually a fuzzy matching is fast but the cost is accuracy.
+	 *
+	 * See struct rte_flow_item_fuzzy
+	 */
+	RTE_FLOW_ITEM_TYPE_FUZZY,
+
+	/**
 	 * Matches any protocol in place of the current layer, a single ANY
 	 * may also stand for several protocol layers.
 	 *
@@ -300,6 +312,24 @@ enum rte_flow_item_type {
 };
 
 /**
+ * RTE_FLOW_ITEM_TYPE_FUZZY
+ *
+ * Fuzzy matching, not perfect match.
+ *
+ * This is for device that support fuzzy match option.
+ * Usually a fuzzy match is fast but the cost is accuracy.
+ * i.e. Signature Match only match pattern's hash value, but it is
+ * possible two different patterns have the same hash value.
+ *
+ * Matching accuracy level can be configure by threshold.
+ * Driver can divide the range of threshold and map to different
+ * accuracy levels that device support.
+ */
+struct rte_flow_item_fuzzy {
+	uint32_t threshold; /**< accuracy threshold*/
+};
+
+/**
  * RTE_FLOW_ITEM_TYPE_ANY
  *
  * Matches any protocol in place of the current layer, a single ANY may also
-- 
2.7.4

Re: [PATCH v2] ethdev: add fuzzy match pattern

[Thomas Monjalon]

Hi,

08/06/2017 00:21, Qi Zhang:
> Add new meta pattern item RTE_FLOW_TYPE_ITEM_FUZZY.

I disagree about fuzzy wording, because fuzzy is something different
I think.

> This is for device that support fuzzy  match option.
> Usually a fuzzy match is fast but the cost is accuracy.
> i.e. Signature Match only match pattern's hash value, but it is
> possible two different patterns have the same hash value.

You have described it yourself here: it matches a hash of the signature.
Why not using "hash" as wording?

> Matching accuracy level can be configure by subfield threshold.
> Driver can divide the range of threshold and map to different
> accuracy levels that device support.
> 
> Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>

Re: [PATCH v2] ethdev: add fuzzy match pattern

[Adrien Mazarguil]

On Thu, Jun 08, 2017 at 09:19:24AM +0200, Thomas Monjalon wrote:
> Hi,
> 
> 08/06/2017 00:21, Qi Zhang:
> > Add new meta pattern item RTE_FLOW_TYPE_ITEM_FUZZY.
> 
> I disagree about fuzzy wording, because fuzzy is something different
> I think.

At least it's a generic term, unrelated PMDs could likewise implement
another kind of fuzzy matching for performance reasons (for applications
that really care more about performance than accuracy). The fact this
particular implementation is built on top of something that is internally
known as signature hash should not be relevant.

> > This is for device that support fuzzy  match option.
> > Usually a fuzzy match is fast but the cost is accuracy.
> > i.e. Signature Match only match pattern's hash value, but it is
> > possible two different patterns have the same hash value.
> 
> You have described it yourself here: it matches a hash of the signature.
> Why not using "hash" as wording?

While "hash" would be also correct, in my opinion it would restrict this
pattern item to Intel adapters (ixgbe) with the ability to actually perform
a hash on patterns and use the resulting value to imperfectly match
traffic. This excludes other implementations with similar unpredictable
results.

Also a threshold notion is necessary as far as I understand, the signature
mode has several levels. The higher, the fuzzier it gets. This behavior is
difficult to translate to something generic named "hash", as one would have
to describe how the hash is computed and packets matched according to that
value, which really is ixgbe-specific at the moment.

> > Matching accuracy level can be configure by subfield threshold.
> > Driver can divide the range of threshold and map to different
> > accuracy levels that device support.
> > 
> > Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>

Qi, many of my other comments about v1 still stand, please check my previous
reply in any case.

-- 
Adrien Mazarguil
6WIND

[PATCH v3] ethdev: add fuzzy match in flow API

[Qi Zhang]

Add new meta pattern item RTE_FLOW_TYPE_ITEM_FUZZY in flow API.

This is for device that support fuzzy match option.
Usually a fuzzy match is fast but the cost is accuracy.
i.e. Signature Match only match pattern's hash value, but it is
possible that two different patterns have the same hash value.

Matching accuracy level can be configured by subfield threshold.
Driver can divide the range of threshold and map to different
accuracy levels that device support.

Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
---
v2:
- replace "Roughly" with "Fuzzy"

v3:
- Append the new item to avoid ABI/API break.
- last, mask will take effect.
- "threshold" is abbreviated as "thresh".
- modified couple comments (proberly need to be further polished).

 app/test-pmd/cmdline_flow.c                 | 25 +++++++++++++
 app/test-pmd/config.c                       |  1 +
 doc/guides/prog_guide/rte_flow.rst          | 54 ++++++++++++++++++++++++++---
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |  4 +++
 lib/librte_ether/rte_flow.h                 | 37 ++++++++++++++++++++
 5 files changed, 117 insertions(+), 4 deletions(-)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 0fd69f9..6befcaf 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -167,6 +167,8 @@ enum index {
 	ITEM_MPLS_LABEL,
 	ITEM_GRE,
 	ITEM_GRE_PROTO,
+	ITEM_FUZZY,
+	ITEM_FUZZY_THRESH,
 
 	/* Validate/create actions. */
 	ACTIONS,
@@ -444,6 +446,13 @@ static const enum index next_item[] = {
 	ITEM_NVGRE,
 	ITEM_MPLS,
 	ITEM_GRE,
+	ITEM_FUZZY,
+	ZERO,
+};
+
+static const enum index item_fuzzy[] = {
+	ITEM_FUZZY_THRESH,
+	ITEM_NEXT,
 	ZERO,
 };
 
@@ -1372,6 +1381,22 @@ static const struct token token_list[] = {
 		.args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_item_gre,
 					     protocol)),
 	},
+	[ITEM_FUZZY] = {
+		.name = "fuzzy",
+		.help = "fuzzy pattern match, expect faster than default",
+		.priv = PRIV_ITEM(FUZZY,
+				sizeof(struct rte_flow_item_fuzzy)),
+		.next = NEXT(item_fuzzy),
+		.call = parse_vc,
+	},
+	[ITEM_FUZZY_THRESH] = {
+		.name = "thresh",
+		.help = "match accuracy threshold",
+		.next = NEXT(item_fuzzy, NEXT_ENTRY(UNSIGNED), item_param),
+		.args = ARGS(ARGS_ENTRY(struct rte_flow_item_fuzzy,
+					thresh)),
+	},
+
 	/* Validate/create actions. */
 	[ACTIONS] = {
 		.name = "actions",
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 4d873cd..8846df3 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -970,6 +970,7 @@ static const struct {
 	MK_FLOW_ITEM(VXLAN, sizeof(struct rte_flow_item_vxlan)),
 	MK_FLOW_ITEM(MPLS, sizeof(struct rte_flow_item_mpls)),
 	MK_FLOW_ITEM(GRE, sizeof(struct rte_flow_item_gre)),
+	MK_FLOW_ITEM(FUZZY, sizeof(struct rte_flow_item_fuzzy)),
 };
 
 /** Compute storage space needed by item specification. */
diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
index b587ba9..bf801de 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -906,6 +906,52 @@ Matches a GRE header.
 - ``protocol``: protocol type.
 - Default ``mask`` matches protocol only.
 
+Item: ``FUZZY``
+^^^^^^^^^^^^^^^^^
+
+Fuzzy pattern match, expect faster than default.
+
+This is for device that support fuzzy match option. Usually a fuzzy match is
+fast but the cost is accuracy. i.e. Signature Match only match pattern's hash
+value, but it is possible two different patterns have the same hash value.
+
+Matching accuracy level can be configured by threshold. Driver can divide the
+range of threshold and map to different accuracy levels that device support.
+
+.. _table_rte_flow_item_fuzzy:
+
+.. table:: FUZZY
+
+   +----------+---------------+--------------------------------------------------+
+   | Field    |   Subfield    | Value                                            |
+   +==========+===========+======================================================+
+   | ``spec`` | ``threshold`` | 0 as perfect match, 0xffffffff as fuzziest match |
+   +----------+---------------+--------------------------------------------------+
+   | ``last`` | ``threshold`` | upper range value                                |
+   +----------+-----------+------------------------------------------------------+
+   | ``mask`` | ``threshold`` | bit-mask apply to "spec" and "last"              |
+   +----------+-----------+------------------------------------------------------+
+
+Usage example, fuzzy match a TCPv4 packets:
+
+.. _table_rte_flow_item_fuzzy_example:
+
+.. table:: Fuzzy matching
+
+   +-------+----------+
+   | Index | Item     |
+   +=======+==========+
+   | 0     | FUZZY    |
+   +-------+----------+
+   | 1     | Ethernet |
+   +-------+----------+
+   | 2     | IPv4     |
+   +-------+----------+
+   | 3     | TCP      |
+   +-------+----------+
+   | 4     | END      |
+   +-------+----------+
+
 Actions
 ~~~~~~~
 
@@ -2026,8 +2072,8 @@ A few features are intentionally not supported:
 - "MAC VLAN" or "tunnel" perfect matching modes should be automatically set
   according to the created flow rules.
 
-- Signature mode of operation is not defined but could be handled through a
-  specific item type if needed.
+- Signature mode of operation is not defined but could be handled through
+  "FUZZY" item.
 
 .. _table_rte_flow_migration_fdir:
 
@@ -2054,8 +2100,8 @@ A few features are intentionally not supported:
    |   |                   +----------+-----+                       |
    |   |                   | ``mask`` | any |                       |
    +---+-------------------+----------+-----+                       |
-   | 3 | VF, PF (optional) | ``spec`` | any |                       |
-   |   |                   +----------+-----+                       |
+   | 3 | VF, PF, FUZZY     | ``spec`` | any |                       |
+   |   | (optional)        +----------+-----+                       |
    |   |                   | ``last`` | N/A |                       |
    |   |                   +----------+-----+                       |
    |   |                   | ``mask`` | any |                       |
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index 0e50c10..8437f7a 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -2608,6 +2608,10 @@ This section lists supported pattern items and their attributes, if any.
 
   - ``protocol {unsigned}``: protocol type.
 
+- ``fuzzy``: fuzzy pattern match, expect faster than default.
+
+  - ``thresh {unsigned}``: accuracy threshold.
+
 Actions list
 ^^^^^^^^^^^^
 
diff --git a/lib/librte_ether/rte_flow.h b/lib/librte_ether/rte_flow.h
index c47edbc..90749b1 100644
--- a/lib/librte_ether/rte_flow.h
+++ b/lib/librte_ether/rte_flow.h
@@ -297,6 +297,18 @@ enum rte_flow_item_type {
 	 * See struct rte_flow_item_gre.
 	 */
 	RTE_FLOW_ITEM_TYPE_GRE,
+
+	/**
+	 * [META]
+	 *
+	 * Fuzzy pattern match, expect faster than default.
+	 *
+	 * This is for device that support fuzzy matching option.
+	 * Usually a fuzzy matching is fast but the cost is accuracy.
+	 *
+	 * See struct rte_flow_item_fuzzy.
+	 */
+	RTE_FLOW_ITEM_TYPE_FUZZY,
 };
 
 /**
@@ -701,6 +713,31 @@ static const struct rte_flow_item_gre rte_flow_item_gre_mask = {
 #endif
 
 /**
+ * RTE_FLOW_ITEM_TYPE_FUZZY
+ *
+ * Fuzzy pattern match, expect faster than default.
+ *
+ * This is for device that support fuzzy match option.
+ * Usually a fuzzy match is fast but the cost is accuracy.
+ * i.e. Signature Match only match pattern's hash value, but it is
+ * possible two different patterns have the same hash value.
+ *
+ * Matching accuracy level can be configure by threshold.
+ * Driver can divide the range of threshold and map to different
+ * accuracy levels that device support.
+ */
+struct rte_flow_item_fuzzy {
+	uint32_t thresh; /**< Accuracy threshold*/
+};
+
+/** Default mask for RTE_FLOW_ITEM_TYPE_FUZZY. */
+#ifndef __cplusplus
+static const struct rte_flow_item_fuzzy rte_flow_item_fuzzy_mask = {
+	.thresh = 0xffffffff,
+};
+#endif
+
+/**
  * Matching pattern item definition.
  *
  * A pattern is formed by stacking items starting from the lowest protocol
-- 
2.7.4

Re: [PATCH v2] ethdev: add fuzzy match pattern

[Zhang, Qi Z]

> -----Original Message-----
> From: Adrien Mazarguil [mailto:adrien.mazarguil@6wind.com]
> Sent: Monday, June 12, 2017 11:39 PM
> To: Thomas Monjalon <thomas@monjalon.net>
> Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2] ethdev: add fuzzy match pattern
> 
> On Thu, Jun 08, 2017 at 09:19:24AM +0200, Thomas Monjalon wrote:
> > Hi,
> >
> > 08/06/2017 00:21, Qi Zhang:
> > > Add new meta pattern item RTE_FLOW_TYPE_ITEM_FUZZY.
> >
> > I disagree about fuzzy wording, because fuzzy is something different I
> > think.
> 
> At least it's a generic term, unrelated PMDs could likewise implement
> another kind of fuzzy matching for performance reasons (for applications
> that really care more about performance than accuracy). The fact this
> particular implementation is built on top of something that is internally
> known as signature hash should not be relevant.
> 
> > > This is for device that support fuzzy  match option.
> > > Usually a fuzzy match is fast but the cost is accuracy.
> > > i.e. Signature Match only match pattern's hash value, but it is
> > > possible two different patterns have the same hash value.
> >
> > You have described it yourself here: it matches a hash of the signature.
> > Why not using "hash" as wording?
> 
> While "hash" would be also correct, in my opinion it would restrict this
> pattern item to Intel adapters (ixgbe) with the ability to actually perform a
> hash on patterns and use the resulting value to imperfectly match traffic.
> This excludes other implementations with similar unpredictable results.
> 
> Also a threshold notion is necessary as far as I understand, the signature
> mode has several levels. The higher, the fuzzier it gets. This behavior is
> difficult to translate to something generic named "hash", as one would have
> to describe how the hash is computed and packets matched according to
> that value, which really is ixgbe-specific at the moment.
> 
> > > Matching accuracy level can be configure by subfield threshold.
> > > Driver can divide the range of threshold and map to different
> > > accuracy levels that device support.
> > >
> > > Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
> 
> Qi, many of my other comments about v1 still stand, please check my
> previous reply in any case.

Sorry, seems I skipped that important mail by mistake, just find it, thanks for all the comments. 
> 
> --
> Adrien Mazarguil
> 6WIND

[PATCH v3 0/3] net/ixgbe: enable signature match and ipv6 for consistent API.

[Qi Zhang]

The patchset is based on
http://dpdk.org/dev/patchwork/patch/25292/

v3:
-  will check fuzzy match's last and mask value.

v2:
-  add comment to explain macro replacement
-  fix wrong flow type assignment


Qi Zhang (3):
  net/ixgbe: replace macro with inline function
  net/ixgbe: enable signature match for consistent API
  net/ixgbe: enable IPv6 for consistent API

 drivers/net/ixgbe/ixgbe_flow.c | 397 ++++++++++++++++++++++++++---------------
 1 file changed, 249 insertions(+), 148 deletions(-)

-- 
2.7.4

[PATCH v3 1/3] net/ixgbe: replace macro with inline function

[Qi Zhang]

Code clean with 2 purposes.
1. No variable "index" needed
2. inline function make it easy and safe when be nest into a loop.

Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
---

v2:
- Update the commit log

 drivers/net/ixgbe/ixgbe_flow.c | 208 +++++++++++++++--------------------------
 1 file changed, 73 insertions(+), 135 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_flow.c b/drivers/net/ixgbe/ixgbe_flow.c
index ccc73fa..067252a 100644
--- a/drivers/net/ixgbe/ixgbe_flow.c
+++ b/drivers/net/ixgbe/ixgbe_flow.c
@@ -79,23 +79,39 @@
 #define IXGBE_MIN_N_TUPLE_PRIO 1
 #define IXGBE_MAX_N_TUPLE_PRIO 7
 #define IXGBE_MAX_FLX_SOURCE_OFF 62
-#define NEXT_ITEM_OF_PATTERN(item, pattern, index)\
-	do {		\
-		item = pattern + index;\
-		while (item->type == RTE_FLOW_ITEM_TYPE_VOID) {\
-		index++;				\
-		item = pattern + index;		\
-		}						\
-	} while (0)
-
-#define NEXT_ITEM_OF_ACTION(act, actions, index)\
-	do {								\
-		act = actions + index;					\
-		while (act->type == RTE_FLOW_ACTION_TYPE_VOID) {\
-		index++;					\
-		act = actions + index;				\
-		}							\
-	} while (0)
+
+/**
+ * Endless loop will never happen with below assumption
+ * 1. there is at least one no-void item(END)
+ * 2. cur is before END.
+ */
+static inline
+const struct rte_flow_item *next_no_void_pattern(
+		const struct rte_flow_item pattern[],
+		const struct rte_flow_item *cur)
+{
+	const struct rte_flow_item *next =
+		cur ? cur + 1 : &pattern[0];
+	while (1) {
+		if (next->type != RTE_FLOW_ITEM_TYPE_VOID)
+			return next;
+		next++;
+	}
+}
+
+static inline
+const struct rte_flow_action *next_no_void_action(
+		const struct rte_flow_action actions[],
+		const struct rte_flow_action *cur)
+{
+	const struct rte_flow_action *next =
+		cur ? cur + 1 : &actions[0];
+	while (1) {
+		if (next->type != RTE_FLOW_ACTION_TYPE_VOID)
+			return next;
+		next++;
+	}
+}
 
 /**
  * Please aware there's an asumption for all the parsers.
@@ -145,7 +161,6 @@ cons_parse_ntuple_filter(const struct rte_flow_attr *attr,
 	const struct rte_flow_item_udp *udp_mask;
 	const struct rte_flow_item_sctp *sctp_spec;
 	const struct rte_flow_item_sctp *sctp_mask;
-	uint32_t index;
 
 	if (!pattern) {
 		rte_flow_error_set(error,
@@ -167,11 +182,8 @@ cons_parse_ntuple_filter(const struct rte_flow_attr *attr,
 		return -rte_errno;
 	}
 
-	/* parse pattern */
-	index = 0;
-
 	/* the first not void item can be MAC or IPv4 */
-	NEXT_ITEM_OF_PATTERN(item, pattern, index);
+	item = next_no_void_pattern(pattern, NULL);
 
 	if (item->type != RTE_FLOW_ITEM_TYPE_ETH &&
 	    item->type != RTE_FLOW_ITEM_TYPE_IPV4) {
@@ -199,8 +211,7 @@ cons_parse_ntuple_filter(const struct rte_flow_attr *attr,
 			return -rte_errno;
 		}
 		/* check if the next not void item is IPv4 */
-		index++;
-		NEXT_ITEM_OF_PATTERN(item, pattern, index);
+		item = next_no_void_pattern(pattern, item);
 		if (item->type != RTE_FLOW_ITEM_TYPE_IPV4) {
 			rte_flow_error_set(error,
 			  EINVAL, RTE_FLOW_ERROR_TYPE_ITEM,
@@ -253,8 +264,7 @@ cons_parse_ntuple_filter(const struct rte_flow_attr *attr,
 	filter->proto  = ipv4_spec->hdr.next_proto_id;
 
 	/* check if the next not void item is TCP or UDP */
-	index++;
-	NEXT_ITEM_OF_PATTERN(item, pattern, index);
+	item = next_no_void_pattern(pattern, item);
 	if (item->type != RTE_FLOW_ITEM_TYPE_TCP &&
 	    item->type != RTE_FLOW_ITEM_TYPE_UDP &&
 	    item->type != RTE_FLOW_ITEM_TYPE_SCTP) {
@@ -372,8 +382,7 @@ cons_parse_ntuple_filter(const struct rte_flow_attr *attr,
 	}
 
 	/* check if the next not void item is END */
-	index++;
-	NEXT_ITEM_OF_PATTERN(item, pattern, index);
+	item = next_no_void_pattern(pattern, item);
 	if (item->type != RTE_FLOW_ITEM_TYPE_END) {
 		memset(filter, 0, sizeof(struct rte_eth_ntuple_filter));
 		rte_flow_error_set(error, EINVAL,
@@ -382,14 +391,11 @@ cons_parse_ntuple_filter(const struct rte_flow_attr *attr,
 		return -rte_errno;
 	}
 
-	/* parse action */
-	index = 0;
-
 	/**
 	 * n-tuple only supports forwarding,
 	 * check if the first not void action is QUEUE.
 	 */
-	NEXT_ITEM_OF_ACTION(act, actions, index);
+	act = next_no_void_action(actions, NULL);
 	if (act->type != RTE_FLOW_ACTION_TYPE_QUEUE) {
 		memset(filter, 0, sizeof(struct rte_eth_ntuple_filter));
 		rte_flow_error_set(error, EINVAL,
@@ -401,8 +407,7 @@ cons_parse_ntuple_filter(const struct rte_flow_attr *attr,
 		((const struct rte_flow_action_queue *)act->conf)->index;
 
 	/* check if the next not void item is END */
-	index++;
-	NEXT_ITEM_OF_ACTION(act, actions, index);
+	act = next_no_void_action(actions, act);
 	if (act->type != RTE_FLOW_ACTION_TYPE_END) {
 		memset(filter, 0, sizeof(struct rte_eth_ntuple_filter));
 		rte_flow_error_set(error, EINVAL,
@@ -521,7 +526,6 @@ cons_parse_ethertype_filter(const struct rte_flow_attr *attr,
 	const struct rte_flow_item_eth *eth_spec;
 	const struct rte_flow_item_eth *eth_mask;
 	const struct rte_flow_action_queue *act_q;
-	uint32_t index;
 
 	if (!pattern) {
 		rte_flow_error_set(error, EINVAL,
@@ -544,15 +548,8 @@ cons_parse_ethertype_filter(const struct rte_flow_attr *attr,
 		return -rte_errno;
 	}
 
-	/* Parse pattern */
-	index = 0;
-
+	item = next_no_void_pattern(pattern, NULL);
 	/* The first non-void item should be MAC. */
-	item = pattern + index;
-	while (item->type == RTE_FLOW_ITEM_TYPE_VOID) {
-		index++;
-		item = pattern + index;
-	}
 	if (item->type != RTE_FLOW_ITEM_TYPE_ETH) {
 		rte_flow_error_set(error, EINVAL,
 			RTE_FLOW_ERROR_TYPE_ITEM,
@@ -611,12 +608,7 @@ cons_parse_ethertype_filter(const struct rte_flow_attr *attr,
 	filter->ether_type = rte_be_to_cpu_16(eth_spec->type);
 
 	/* Check if the next non-void item is END. */
-	index++;
-	item = pattern + index;
-	while (item->type == RTE_FLOW_ITEM_TYPE_VOID) {
-		index++;
-		item = pattern + index;
-	}
+	item = next_no_void_pattern(pattern, item);
 	if (item->type != RTE_FLOW_ITEM_TYPE_END) {
 		rte_flow_error_set(error, EINVAL,
 				RTE_FLOW_ERROR_TYPE_ITEM,
@@ -626,13 +618,7 @@ cons_parse_ethertype_filter(const struct rte_flow_attr *attr,
 
 	/* Parse action */
 
-	index = 0;
-	/* Check if the first non-void action is QUEUE or DROP. */
-	act = actions + index;
-	while (act->type == RTE_FLOW_ACTION_TYPE_VOID) {
-		index++;
-		act = actions + index;
-	}
+	act = next_no_void_action(actions, NULL);
 	if (act->type != RTE_FLOW_ACTION_TYPE_QUEUE &&
 	    act->type != RTE_FLOW_ACTION_TYPE_DROP) {
 		rte_flow_error_set(error, EINVAL,
@@ -649,12 +635,7 @@ cons_parse_ethertype_filter(const struct rte_flow_attr *attr,
 	}
 
 	/* Check if the next non-void item is END */
-	index++;
-	act = actions + index;
-	while (act->type == RTE_FLOW_ACTION_TYPE_VOID) {
-		index++;
-		act = actions + index;
-	}
+	act = next_no_void_action(actions, act);
 	if (act->type != RTE_FLOW_ACTION_TYPE_END) {
 		rte_flow_error_set(error, EINVAL,
 				RTE_FLOW_ERROR_TYPE_ACTION,
@@ -794,7 +775,6 @@ cons_parse_syn_filter(const struct rte_flow_attr *attr,
 	const struct rte_flow_item_tcp *tcp_spec;
 	const struct rte_flow_item_tcp *tcp_mask;
 	const struct rte_flow_action_queue *act_q;
-	uint32_t index;
 
 	if (!pattern) {
 		rte_flow_error_set(error, EINVAL,
@@ -817,11 +797,9 @@ cons_parse_syn_filter(const struct rte_flow_attr *attr,
 		return -rte_errno;
 	}
 
-	/* parse pattern */
-	index = 0;
 
 	/* the first not void item should be MAC or IPv4 or IPv6 or TCP */
-	NEXT_ITEM_OF_PATTERN(item, pattern, index);
+	item = next_no_void_pattern(pattern, NULL);
 	if (item->type != RTE_FLOW_ITEM_TYPE_ETH &&
 	    item->type != RTE_FLOW_ITEM_TYPE_IPV4 &&
 	    item->type != RTE_FLOW_ITEM_TYPE_IPV6 &&
@@ -850,8 +828,7 @@ cons_parse_syn_filter(const struct rte_flow_attr *attr,
 		}
 
 		/* check if the next not void item is IPv4 or IPv6 */
-		index++;
-		NEXT_ITEM_OF_PATTERN(item, pattern, index);
+		item = next_no_void_pattern(pattern, item);
 		if (item->type != RTE_FLOW_ITEM_TYPE_IPV4 &&
 		    item->type != RTE_FLOW_ITEM_TYPE_IPV6) {
 			rte_flow_error_set(error, EINVAL,
@@ -873,8 +850,7 @@ cons_parse_syn_filter(const struct rte_flow_attr *attr,
 		}
 
 		/* check if the next not void item is TCP */
-		index++;
-		NEXT_ITEM_OF_PATTERN(item, pattern, index);
+		item = next_no_void_pattern(pattern, item);
 		if (item->type != RTE_FLOW_ITEM_TYPE_TCP) {
 			rte_flow_error_set(error, EINVAL,
 				RTE_FLOW_ERROR_TYPE_ITEM,
@@ -918,8 +894,7 @@ cons_parse_syn_filter(const struct rte_flow_attr *attr,
 	}
 
 	/* check if the next not void item is END */
-	index++;
-	NEXT_ITEM_OF_PATTERN(item, pattern, index);
+	item = next_no_void_pattern(pattern, item);
 	if (item->type != RTE_FLOW_ITEM_TYPE_END) {
 		memset(filter, 0, sizeof(struct rte_eth_syn_filter));
 		rte_flow_error_set(error, EINVAL,
@@ -928,11 +903,8 @@ cons_parse_syn_filter(const struct rte_flow_attr *attr,
 		return -rte_errno;
 	}
 
-	/* parse action */
-	index = 0;
-
 	/* check if the first not void action is QUEUE. */
-	NEXT_ITEM_OF_ACTION(act, actions, index);
+	act = next_no_void_action(actions, NULL);
 	if (act->type != RTE_FLOW_ACTION_TYPE_QUEUE) {
 		memset(filter, 0, sizeof(struct rte_eth_syn_filter));
 		rte_flow_error_set(error, EINVAL,
@@ -952,8 +924,7 @@ cons_parse_syn_filter(const struct rte_flow_attr *attr,
 	}
 
 	/* check if the next not void item is END */
-	index++;
-	NEXT_ITEM_OF_ACTION(act, actions, index);
+	act = next_no_void_action(actions, act);
 	if (act->type != RTE_FLOW_ACTION_TYPE_END) {
 		memset(filter, 0, sizeof(struct rte_eth_syn_filter));
 		rte_flow_error_set(error, EINVAL,
@@ -1049,7 +1020,6 @@ cons_parse_l2_tn_filter(const struct rte_flow_attr *attr,
 	const struct rte_flow_item_e_tag *e_tag_mask;
 	const struct rte_flow_action *act;
 	const struct rte_flow_action_queue *act_q;
-	uint32_t index;
 
 	if (!pattern) {
 		rte_flow_error_set(error, EINVAL,
@@ -1071,11 +1041,9 @@ cons_parse_l2_tn_filter(const struct rte_flow_attr *attr,
 				   NULL, "NULL attribute.");
 		return -rte_errno;
 	}
-	/* parse pattern */
-	index = 0;
 
 	/* The first not void item should be e-tag. */
-	NEXT_ITEM_OF_PATTERN(item, pattern, index);
+	item = next_no_void_pattern(pattern, NULL);
 	if (item->type != RTE_FLOW_ITEM_TYPE_E_TAG) {
 		memset(filter, 0, sizeof(struct rte_eth_l2_tunnel_conf));
 		rte_flow_error_set(error, EINVAL,
@@ -1122,8 +1090,7 @@ cons_parse_l2_tn_filter(const struct rte_flow_attr *attr,
 	filter->tunnel_id = rte_be_to_cpu_16(e_tag_spec->rsvd_grp_ecid_b);
 
 	/* check if the next not void item is END */
-	index++;
-	NEXT_ITEM_OF_PATTERN(item, pattern, index);
+	item = next_no_void_pattern(pattern, item);
 	if (item->type != RTE_FLOW_ITEM_TYPE_END) {
 		memset(filter, 0, sizeof(struct rte_eth_l2_tunnel_conf));
 		rte_flow_error_set(error, EINVAL,
@@ -1160,11 +1127,8 @@ cons_parse_l2_tn_filter(const struct rte_flow_attr *attr,
 		return -rte_errno;
 	}
 
-	/* parse action */
-	index = 0;
-
 	/* check if the first not void action is QUEUE. */
-	NEXT_ITEM_OF_ACTION(act, actions, index);
+	act = next_no_void_action(actions, NULL);
 	if (act->type != RTE_FLOW_ACTION_TYPE_QUEUE) {
 		memset(filter, 0, sizeof(struct rte_eth_l2_tunnel_conf));
 		rte_flow_error_set(error, EINVAL,
@@ -1177,8 +1141,7 @@ cons_parse_l2_tn_filter(const struct rte_flow_attr *attr,
 	filter->pool = act_q->index;
 
 	/* check if the next not void item is END */
-	index++;
-	NEXT_ITEM_OF_ACTION(act, actions, index);
+	act = next_no_void_action(actions, act);
 	if (act->type != RTE_FLOW_ACTION_TYPE_END) {
 		memset(filter, 0, sizeof(struct rte_eth_l2_tunnel_conf));
 		rte_flow_error_set(error, EINVAL,
@@ -1227,7 +1190,6 @@ ixgbe_parse_fdir_act_attr(const struct rte_flow_attr *attr,
 	const struct rte_flow_action *act;
 	const struct rte_flow_action_queue *act_q;
 	const struct rte_flow_action_mark *mark;
-	uint32_t index;
 
 	/* parse attr */
 	/* must be input direction */
@@ -1257,11 +1219,8 @@ ixgbe_parse_fdir_act_attr(const struct rte_flow_attr *attr,
 		return -rte_errno;
 	}
 
-	/* parse action */
-	index = 0;
-
 	/* check if the first not void action is QUEUE or DROP. */
-	NEXT_ITEM_OF_ACTION(act, actions, index);
+	act = next_no_void_action(actions, NULL);
 	if (act->type != RTE_FLOW_ACTION_TYPE_QUEUE &&
 	    act->type != RTE_FLOW_ACTION_TYPE_DROP) {
 		memset(rule, 0, sizeof(struct ixgbe_fdir_rule));
@@ -1279,8 +1238,7 @@ ixgbe_parse_fdir_act_attr(const struct rte_flow_attr *attr,
 	}
 
 	/* check if the next not void item is MARK */
-	index++;
-	NEXT_ITEM_OF_ACTION(act, actions, index);
+	act = next_no_void_action(actions, act);
 	if ((act->type != RTE_FLOW_ACTION_TYPE_MARK) &&
 		(act->type != RTE_FLOW_ACTION_TYPE_END)) {
 		memset(rule, 0, sizeof(struct ixgbe_fdir_rule));
@@ -1295,8 +1253,7 @@ ixgbe_parse_fdir_act_attr(const struct rte_flow_attr *attr,
 	if (act->type == RTE_FLOW_ACTION_TYPE_MARK) {
 		mark = (const struct rte_flow_action_mark *)act->conf;
 		rule->soft_id = mark->id;
-		index++;
-		NEXT_ITEM_OF_ACTION(act, actions, index);
+		act = next_no_void_action(actions, act);
 	}
 
 	/* check if the next not void item is END */
@@ -1378,7 +1335,7 @@ ixgbe_parse_fdir_filter_normal(const struct rte_flow_attr *attr,
 	const struct rte_flow_item_raw *raw_mask;
 	const struct rte_flow_item_raw *raw_spec;
 
-	uint32_t index, j;
+	uint32_t j;
 
 	if (!pattern) {
 		rte_flow_error_set(error, EINVAL,
@@ -1410,14 +1367,11 @@ ixgbe_parse_fdir_filter_normal(const struct rte_flow_attr *attr,
 	rule->mask.vlan_tci_mask = 0;
 	rule->mask.flex_bytes_mask = 0;
 
-	/* parse pattern */
-	index = 0;
-
 	/**
 	 * The first not void item should be
 	 * MAC or IPv4 or TCP or UDP or SCTP.
 	 */
-	NEXT_ITEM_OF_PATTERN(item, pattern, index);
+	item = next_no_void_pattern(pattern, NULL);
 	if (item->type != RTE_FLOW_ITEM_TYPE_ETH &&
 	    item->type != RTE_FLOW_ITEM_TYPE_IPV4 &&
 	    item->type != RTE_FLOW_ITEM_TYPE_TCP &&
@@ -1510,8 +1464,7 @@ ixgbe_parse_fdir_filter_normal(const struct rte_flow_attr *attr,
 		 * Check if the next not void item is vlan or ipv4.
 		 * IPv6 is not supported.
 		 */
-		index++;
-		NEXT_ITEM_OF_PATTERN(item, pattern, index);
+		item = next_no_void_pattern(pattern, item);
 		if (rule->mode == RTE_FDIR_MODE_PERFECT_MAC_VLAN) {
 			if (item->type != RTE_FLOW_ITEM_TYPE_VLAN) {
 				memset(rule, 0, sizeof(struct ixgbe_fdir_rule));
@@ -1558,8 +1511,7 @@ ixgbe_parse_fdir_filter_normal(const struct rte_flow_attr *attr,
 		/* More than one tags are not supported. */
 
 		/* Next not void item must be END */
-		index++;
-		NEXT_ITEM_OF_PATTERN(item, pattern, index);
+		item = next_no_void_pattern(pattern, item);
 		if (item->type != RTE_FLOW_ITEM_TYPE_END) {
 			memset(rule, 0, sizeof(struct ixgbe_fdir_rule));
 			rte_flow_error_set(error, EINVAL,
@@ -1629,8 +1581,7 @@ ixgbe_parse_fdir_filter_normal(const struct rte_flow_attr *attr,
 		 * Check if the next not void item is
 		 * TCP or UDP or SCTP or END.
 		 */
-		index++;
-		NEXT_ITEM_OF_PATTERN(item, pattern, index);
+		item = next_no_void_pattern(pattern, item);
 		if (item->type != RTE_FLOW_ITEM_TYPE_TCP &&
 		    item->type != RTE_FLOW_ITEM_TYPE_UDP &&
 		    item->type != RTE_FLOW_ITEM_TYPE_SCTP &&
@@ -1697,8 +1648,7 @@ ixgbe_parse_fdir_filter_normal(const struct rte_flow_attr *attr,
 				tcp_spec->hdr.dst_port;
 		}
 
-		index++;
-		NEXT_ITEM_OF_PATTERN(item, pattern, index);
+		item = next_no_void_pattern(pattern, item);
 		if (item->type != RTE_FLOW_ITEM_TYPE_RAW &&
 		    item->type != RTE_FLOW_ITEM_TYPE_END) {
 			memset(rule, 0, sizeof(struct ixgbe_fdir_rule));
@@ -1758,8 +1708,7 @@ ixgbe_parse_fdir_filter_normal(const struct rte_flow_attr *attr,
 				udp_spec->hdr.dst_port;
 		}
 
-		index++;
-		NEXT_ITEM_OF_PATTERN(item, pattern, index);
+		item = next_no_void_pattern(pattern, item);
 		if (item->type != RTE_FLOW_ITEM_TYPE_RAW &&
 		    item->type != RTE_FLOW_ITEM_TYPE_END) {
 			memset(rule, 0, sizeof(struct ixgbe_fdir_rule));
@@ -1821,8 +1770,7 @@ ixgbe_parse_fdir_filter_normal(const struct rte_flow_attr *attr,
 				sctp_spec->hdr.dst_port;
 		}
 
-		index++;
-		NEXT_ITEM_OF_PATTERN(item, pattern, index);
+		item = next_no_void_pattern(pattern, item);
 		if (item->type != RTE_FLOW_ITEM_TYPE_RAW &&
 		    item->type != RTE_FLOW_ITEM_TYPE_END) {
 			memset(rule, 0, sizeof(struct ixgbe_fdir_rule));
@@ -1906,8 +1854,7 @@ ixgbe_parse_fdir_filter_normal(const struct rte_flow_attr *attr,
 
 	if (item->type != RTE_FLOW_ITEM_TYPE_END) {
 		/* check if the next not void item is END */
-		index++;
-		NEXT_ITEM_OF_PATTERN(item, pattern, index);
+		item = next_no_void_pattern(pattern, item);
 		if (item->type != RTE_FLOW_ITEM_TYPE_END) {
 			memset(rule, 0, sizeof(struct ixgbe_fdir_rule));
 			rte_flow_error_set(error, EINVAL,
@@ -1975,7 +1922,7 @@ ixgbe_parse_fdir_filter_tunnel(const struct rte_flow_attr *attr,
 	const struct rte_flow_item_eth *eth_mask;
 	const struct rte_flow_item_vlan *vlan_spec;
 	const struct rte_flow_item_vlan *vlan_mask;
-	uint32_t index, j;
+	uint32_t j;
 
 	if (!pattern) {
 		rte_flow_error_set(error, EINVAL,
@@ -2006,14 +1953,11 @@ ixgbe_parse_fdir_filter_tunnel(const struct rte_flow_attr *attr,
 	memset(&rule->mask, 0xFF, sizeof(struct ixgbe_hw_fdir_mask));
 	rule->mask.vlan_tci_mask = 0;
 
-	/* parse pattern */
-	index = 0;
-
 	/**
 	 * The first not void item should be
 	 * MAC or IPv4 or IPv6 or UDP or VxLAN.
 	 */
-	NEXT_ITEM_OF_PATTERN(item, pattern, index);
+	item = next_no_void_pattern(pattern, NULL);
 	if (item->type != RTE_FLOW_ITEM_TYPE_ETH &&
 	    item->type != RTE_FLOW_ITEM_TYPE_IPV4 &&
 	    item->type != RTE_FLOW_ITEM_TYPE_IPV6 &&
@@ -2048,8 +1992,7 @@ ixgbe_parse_fdir_filter_tunnel(const struct rte_flow_attr *attr,
 		}
 
 		/* Check if the next not void item is IPv4 or IPv6. */
-		index++;
-		NEXT_ITEM_OF_PATTERN(item, pattern, index);
+		item = next_no_void_pattern(pattern, item);
 		if (item->type != RTE_FLOW_ITEM_TYPE_IPV4 &&
 		    item->type != RTE_FLOW_ITEM_TYPE_IPV6) {
 			memset(rule, 0, sizeof(struct ixgbe_fdir_rule));
@@ -2080,8 +2023,7 @@ ixgbe_parse_fdir_filter_tunnel(const struct rte_flow_attr *attr,
 		}
 
 		/* Check if the next not void item is UDP or NVGRE. */
-		index++;
-		NEXT_ITEM_OF_PATTERN(item, pattern, index);
+		item = next_no_void_pattern(pattern, item);
 		if (item->type != RTE_FLOW_ITEM_TYPE_UDP &&
 		    item->type != RTE_FLOW_ITEM_TYPE_NVGRE) {
 			memset(rule, 0, sizeof(struct ixgbe_fdir_rule));
@@ -2111,8 +2053,7 @@ ixgbe_parse_fdir_filter_tunnel(const struct rte_flow_attr *attr,
 		}
 
 		/* Check if the next not void item is VxLAN. */
-		index++;
-		NEXT_ITEM_OF_PATTERN(item, pattern, index);
+		item = next_no_void_pattern(pattern, item);
 		if (item->type != RTE_FLOW_ITEM_TYPE_VXLAN) {
 			memset(rule, 0, sizeof(struct ixgbe_fdir_rule));
 			rte_flow_error_set(error, EINVAL,
@@ -2268,8 +2209,7 @@ ixgbe_parse_fdir_filter_tunnel(const struct rte_flow_attr *attr,
 	}
 
 	/* check if the next not void item is MAC */
-	index++;
-	NEXT_ITEM_OF_PATTERN(item, pattern, index);
+	item = next_no_void_pattern(pattern, item);
 	if (item->type != RTE_FLOW_ITEM_TYPE_ETH) {
 		memset(rule, 0, sizeof(struct ixgbe_fdir_rule));
 		rte_flow_error_set(error, EINVAL,
@@ -2352,8 +2292,7 @@ ixgbe_parse_fdir_filter_tunnel(const struct rte_flow_attr *attr,
 	 * Check if the next not void item is vlan or ipv4.
 	 * IPv6 is not supported.
 	 */
-	index++;
-	NEXT_ITEM_OF_PATTERN(item, pattern, index);
+	item = next_no_void_pattern(pattern, item);
 	if ((item->type != RTE_FLOW_ITEM_TYPE_VLAN) &&
 		(item->type != RTE_FLOW_ITEM_TYPE_IPV4)) {
 		memset(rule, 0, sizeof(struct ixgbe_fdir_rule));
@@ -2389,8 +2328,7 @@ ixgbe_parse_fdir_filter_tunnel(const struct rte_flow_attr *attr,
 		/* More than one tags are not supported. */
 
 		/* check if the next not void item is END */
-		index++;
-		NEXT_ITEM_OF_PATTERN(item, pattern, index);
+		item = next_no_void_pattern(pattern, item);
 
 		if (item->type != RTE_FLOW_ITEM_TYPE_END) {
 			memset(rule, 0, sizeof(struct ixgbe_fdir_rule));
-- 
2.7.4

[PATCH v3 2/3] net/ixgbe: enable signature match for consistent API

[Qi Zhang]

Enable signature match for rte_flow API.
RTE_FLOW_ITEM_TYPE_FUZZY specify a signature match.

Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
---

v2:
-  will check fuzzy match's last and mask value.

 drivers/net/ixgbe/ixgbe_flow.c | 91 ++++++++++++++++++++++++++++++++++++------
 1 file changed, 79 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_flow.c b/drivers/net/ixgbe/ixgbe_flow.c
index 067252a..c6653d7 100644
--- a/drivers/net/ixgbe/ixgbe_flow.c
+++ b/drivers/net/ixgbe/ixgbe_flow.c
@@ -1268,6 +1268,67 @@ ixgbe_parse_fdir_act_attr(const struct rte_flow_attr *attr,
 	return 0;
 }
 
+/* search next no void pattern and skip roughly */
+static inline
+const struct rte_flow_item *next_no_roughly_pattern(
+		const struct rte_flow_item pattern[],
+		const struct rte_flow_item *cur)
+{
+	const struct rte_flow_item *next =
+		next_no_void_pattern(pattern, cur);
+	while (1) {
+		if (next->type != RTE_FLOW_ITEM_TYPE_FUZZY)
+			return next;
+		next = next_no_void_pattern(pattern, next);
+	}
+}
+
+static inline uint8_t signature_match(const struct rte_flow_item pattern[])
+{
+	const struct rte_flow_item_fuzzy *spec, *last, *mask;
+	const struct rte_flow_item *item;
+	uint32_t sh, lh, mh;
+	int i = 0;
+
+	while (1) {
+		item = pattern + i;
+		if (item->type == RTE_FLOW_ITEM_TYPE_END)
+			break;
+
+		if (item->type == RTE_FLOW_ITEM_TYPE_FUZZY) {
+			spec =
+			(const struct rte_flow_item_fuzzy *)item->spec;
+			last =
+			(const struct rte_flow_item_fuzzy *)item->last;
+			mask =
+			(const struct rte_flow_item_fuzzy *)item->mask;
+
+			if (!spec || !mask)
+				return 0;
+
+			sh = spec->thresh;
+
+			if (!last)
+				lh = sh;
+			else
+				lh = last->thresh;
+
+			mh = mask->thresh;
+			sh = sh & mh;
+			lh = lh & mh;
+
+			if (!sh || sh > lh)
+				return 0;
+
+			return 1;
+		}
+
+		i++;
+	}
+
+	return 0;
+}
+
 /**
  * Parse the rule to see if it is a IP or MAC VLAN flow director rule.
  * And get the flow director filter info BTW.
@@ -1277,6 +1338,7 @@ ixgbe_parse_fdir_act_attr(const struct rte_flow_attr *attr,
  * The next not void item could be UDP or TCP or SCTP (optional)
  * The next not void item could be RAW (for flexbyte, optional)
  * The next not void item must be END.
+ * A Roughly Match pattern can appear at any place before END (optional)
  * MAC VLAN PATTERN:
  * The first not void item must be ETH.
  * The second not void item must be MAC VLAN.
@@ -1371,7 +1433,7 @@ ixgbe_parse_fdir_filter_normal(const struct rte_flow_attr *attr,
 	 * The first not void item should be
 	 * MAC or IPv4 or TCP or UDP or SCTP.
 	 */
-	item = next_no_void_pattern(pattern, NULL);
+	item = next_no_roughly_pattern(pattern, NULL);
 	if (item->type != RTE_FLOW_ITEM_TYPE_ETH &&
 	    item->type != RTE_FLOW_ITEM_TYPE_IPV4 &&
 	    item->type != RTE_FLOW_ITEM_TYPE_TCP &&
@@ -1384,7 +1446,10 @@ ixgbe_parse_fdir_filter_normal(const struct rte_flow_attr *attr,
 		return -rte_errno;
 	}
 
-	rule->mode = RTE_FDIR_MODE_PERFECT;
+	if (signature_match(pattern))
+		rule->mode = RTE_FDIR_MODE_SIGNATURE;
+	else
+		rule->mode = RTE_FDIR_MODE_PERFECT;
 
 	/*Not supported last point for range*/
 	if (item->last) {
@@ -1421,14 +1486,13 @@ ixgbe_parse_fdir_filter_normal(const struct rte_flow_attr *attr,
 
 
 		if (item->mask) {
-			/* If ethernet has meaning, it means MAC VLAN mode. */
-			rule->mode = RTE_FDIR_MODE_PERFECT_MAC_VLAN;
 
 			rule->b_mask = TRUE;
 			eth_mask = (const struct rte_flow_item_eth *)item->mask;
 
 			/* Ether type should be masked. */
-			if (eth_mask->type) {
+			if (eth_mask->type ||
+			    rule->mode == RTE_FDIR_MODE_SIGNATURE) {
 				memset(rule, 0, sizeof(struct ixgbe_fdir_rule));
 				rte_flow_error_set(error, EINVAL,
 					RTE_FLOW_ERROR_TYPE_ITEM,
@@ -1436,6 +1500,9 @@ ixgbe_parse_fdir_filter_normal(const struct rte_flow_attr *attr,
 				return -rte_errno;
 			}
 
+			/* If ethernet has meaning, it means MAC VLAN mode. */
+			rule->mode = RTE_FDIR_MODE_PERFECT_MAC_VLAN;
+
 			/**
 			 * src MAC address must be masked,
 			 * and don't support dst MAC address mask.
@@ -1464,7 +1531,7 @@ ixgbe_parse_fdir_filter_normal(const struct rte_flow_attr *attr,
 		 * Check if the next not void item is vlan or ipv4.
 		 * IPv6 is not supported.
 		 */
-		item = next_no_void_pattern(pattern, item);
+		item = next_no_roughly_pattern(pattern, item);
 		if (rule->mode == RTE_FDIR_MODE_PERFECT_MAC_VLAN) {
 			if (item->type != RTE_FLOW_ITEM_TYPE_VLAN) {
 				memset(rule, 0, sizeof(struct ixgbe_fdir_rule));
@@ -1511,7 +1578,7 @@ ixgbe_parse_fdir_filter_normal(const struct rte_flow_attr *attr,
 		/* More than one tags are not supported. */
 
 		/* Next not void item must be END */
-		item = next_no_void_pattern(pattern, item);
+		item = next_no_roughly_pattern(pattern, item);
 		if (item->type != RTE_FLOW_ITEM_TYPE_END) {
 			memset(rule, 0, sizeof(struct ixgbe_fdir_rule));
 			rte_flow_error_set(error, EINVAL,
@@ -1581,7 +1648,7 @@ ixgbe_parse_fdir_filter_normal(const struct rte_flow_attr *attr,
 		 * Check if the next not void item is
 		 * TCP or UDP or SCTP or END.
 		 */
-		item = next_no_void_pattern(pattern, item);
+		item = next_no_roughly_pattern(pattern, item);
 		if (item->type != RTE_FLOW_ITEM_TYPE_TCP &&
 		    item->type != RTE_FLOW_ITEM_TYPE_UDP &&
 		    item->type != RTE_FLOW_ITEM_TYPE_SCTP &&
@@ -1648,7 +1715,7 @@ ixgbe_parse_fdir_filter_normal(const struct rte_flow_attr *attr,
 				tcp_spec->hdr.dst_port;
 		}
 
-		item = next_no_void_pattern(pattern, item);
+		item = next_no_roughly_pattern(pattern, item);
 		if (item->type != RTE_FLOW_ITEM_TYPE_RAW &&
 		    item->type != RTE_FLOW_ITEM_TYPE_END) {
 			memset(rule, 0, sizeof(struct ixgbe_fdir_rule));
@@ -1708,7 +1775,7 @@ ixgbe_parse_fdir_filter_normal(const struct rte_flow_attr *attr,
 				udp_spec->hdr.dst_port;
 		}
 
-		item = next_no_void_pattern(pattern, item);
+		item = next_no_roughly_pattern(pattern, item);
 		if (item->type != RTE_FLOW_ITEM_TYPE_RAW &&
 		    item->type != RTE_FLOW_ITEM_TYPE_END) {
 			memset(rule, 0, sizeof(struct ixgbe_fdir_rule));
@@ -1770,7 +1837,7 @@ ixgbe_parse_fdir_filter_normal(const struct rte_flow_attr *attr,
 				sctp_spec->hdr.dst_port;
 		}
 
-		item = next_no_void_pattern(pattern, item);
+		item = next_no_roughly_pattern(pattern, item);
 		if (item->type != RTE_FLOW_ITEM_TYPE_RAW &&
 		    item->type != RTE_FLOW_ITEM_TYPE_END) {
 			memset(rule, 0, sizeof(struct ixgbe_fdir_rule));
@@ -1854,7 +1921,7 @@ ixgbe_parse_fdir_filter_normal(const struct rte_flow_attr *attr,
 
 	if (item->type != RTE_FLOW_ITEM_TYPE_END) {
 		/* check if the next not void item is END */
-		item = next_no_void_pattern(pattern, item);
+		item = next_no_roughly_pattern(pattern, item);
 		if (item->type != RTE_FLOW_ITEM_TYPE_END) {
 			memset(rule, 0, sizeof(struct ixgbe_fdir_rule));
 			rte_flow_error_set(error, EINVAL,
-- 
2.7.4

[PATCH v3 3/3] net/ixgbe: enable IPv6 for consistent API

[Qi Zhang]

Enable IPv6 support with rte_flow API.
Only support Sigature Match.

Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
---

v2:
- fix flow type assignment.

 drivers/net/ixgbe/ixgbe_flow.c | 118 +++++++++++++++++++++++++++++++++++++----
 1 file changed, 107 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_flow.c b/drivers/net/ixgbe/ixgbe_flow.c
index c6653d7..b631d59 100644
--- a/drivers/net/ixgbe/ixgbe_flow.c
+++ b/drivers/net/ixgbe/ixgbe_flow.c
@@ -1333,12 +1333,13 @@ static inline uint8_t signature_match(const struct rte_flow_item pattern[])
  * Parse the rule to see if it is a IP or MAC VLAN flow director rule.
  * And get the flow director filter info BTW.
  * UDP/TCP/SCTP PATTERN:
- * The first not void item can be ETH or IPV4.
- * The second not void item must be IPV4 if the first one is ETH.
+ * The first not void item can be ETH or IPV4 or IPV6
+ * The second not void item must be IPV4 or IPV6 if the first one is ETH.
  * The next not void item could be UDP or TCP or SCTP (optional)
  * The next not void item could be RAW (for flexbyte, optional)
  * The next not void item must be END.
- * A Roughly Match pattern can appear at any place before END (optional)
+ * A Roughly Match pattern can appear at any place before END.
+ * Roughly Match is optional for IPV4 but is required for IPV6
  * MAC VLAN PATTERN:
  * The first not void item must be ETH.
  * The second not void item must be MAC VLAN.
@@ -1386,6 +1387,8 @@ ixgbe_parse_fdir_filter_normal(const struct rte_flow_attr *attr,
 	const struct rte_flow_item_eth *eth_mask;
 	const struct rte_flow_item_ipv4 *ipv4_spec;
 	const struct rte_flow_item_ipv4 *ipv4_mask;
+	const struct rte_flow_item_ipv6 *ipv6_spec;
+	const struct rte_flow_item_ipv6 *ipv6_mask;
 	const struct rte_flow_item_tcp *tcp_spec;
 	const struct rte_flow_item_tcp *tcp_mask;
 	const struct rte_flow_item_udp *udp_spec;
@@ -1397,7 +1400,7 @@ ixgbe_parse_fdir_filter_normal(const struct rte_flow_attr *attr,
 	const struct rte_flow_item_raw *raw_mask;
 	const struct rte_flow_item_raw *raw_spec;
 
-	uint32_t j;
+	uint8_t j;
 
 	if (!pattern) {
 		rte_flow_error_set(error, EINVAL,
@@ -1436,6 +1439,7 @@ ixgbe_parse_fdir_filter_normal(const struct rte_flow_attr *attr,
 	item = next_no_roughly_pattern(pattern, NULL);
 	if (item->type != RTE_FLOW_ITEM_TYPE_ETH &&
 	    item->type != RTE_FLOW_ITEM_TYPE_IPV4 &&
+	    item->type != RTE_FLOW_ITEM_TYPE_IPV6 &&
 	    item->type != RTE_FLOW_ITEM_TYPE_TCP &&
 	    item->type != RTE_FLOW_ITEM_TYPE_UDP &&
 	    item->type != RTE_FLOW_ITEM_TYPE_SCTP) {
@@ -1588,7 +1592,7 @@ ixgbe_parse_fdir_filter_normal(const struct rte_flow_attr *attr,
 		}
 	}
 
-	/* Get the IP info. */
+	/* Get the IPV4 info. */
 	if (item->type == RTE_FLOW_ITEM_TYPE_IPV4) {
 		/**
 		 * Set the flow type even if there's no content
@@ -1662,14 +1666,106 @@ ixgbe_parse_fdir_filter_normal(const struct rte_flow_attr *attr,
 		}
 	}
 
+	/* Get the IPV6 info. */
+	if (item->type == RTE_FLOW_ITEM_TYPE_IPV6) {
+		/**
+		 * Set the flow type even if there's no content
+		 * as we must have a flow type.
+		 */
+		rule->ixgbe_fdir.formatted.flow_type =
+			IXGBE_ATR_FLOW_TYPE_IPV6;
+
+		/**
+		 * 1. must signature match
+		 * 2. not support last
+		 * 3. mask must not null
+		 */
+		if (rule->mode != RTE_FDIR_MODE_SIGNATURE ||
+		    item->last ||
+		    !item->mask) {
+			memset(rule, 0, sizeof(struct ixgbe_fdir_rule));
+			rte_flow_error_set(error, EINVAL,
+				RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
+				item, "Not supported last point for range");
+			return -rte_errno;
+		}
+
+		rule->b_mask = TRUE;
+		ipv6_mask =
+			(const struct rte_flow_item_ipv6 *)item->mask;
+		if (ipv6_mask->hdr.vtc_flow ||
+		    ipv6_mask->hdr.payload_len ||
+		    ipv6_mask->hdr.proto ||
+		    ipv6_mask->hdr.hop_limits) {
+			memset(rule, 0, sizeof(struct ixgbe_fdir_rule));
+			rte_flow_error_set(error, EINVAL,
+				RTE_FLOW_ERROR_TYPE_ITEM,
+				item, "Not supported by fdir filter");
+			return -rte_errno;
+		}
+
+		/* check src addr mask */
+		for (j = 0; j < 16; j++) {
+			if (ipv6_mask->hdr.src_addr[j] == UINT8_MAX) {
+				rule->mask.src_ipv6_mask |= 1 << j;
+			} else if (ipv6_mask->hdr.src_addr[j] != 0) {
+				memset(rule, 0, sizeof(struct ixgbe_fdir_rule));
+				rte_flow_error_set(error, EINVAL,
+					RTE_FLOW_ERROR_TYPE_ITEM,
+					item, "Not supported by fdir filter");
+				return -rte_errno;
+			}
+		}
+
+		/* check dst addr mask */
+		for (j = 0; j < 16; j++) {
+			if (ipv6_mask->hdr.dst_addr[j] == UINT8_MAX) {
+				rule->mask.dst_ipv6_mask |= 1 << j;
+			} else if (ipv6_mask->hdr.dst_addr[j] != 0) {
+				memset(rule, 0, sizeof(struct ixgbe_fdir_rule));
+				rte_flow_error_set(error, EINVAL,
+					RTE_FLOW_ERROR_TYPE_ITEM,
+					item, "Not supported by fdir filter");
+				return -rte_errno;
+			}
+		}
+
+		if (item->spec) {
+			rule->b_spec = TRUE;
+			ipv6_spec =
+				(const struct rte_flow_item_ipv6 *)item->spec;
+			rte_memcpy(rule->ixgbe_fdir.formatted.src_ip,
+				   ipv6_spec->hdr.src_addr, 16);
+			rte_memcpy(rule->ixgbe_fdir.formatted.dst_ip,
+				   ipv6_spec->hdr.dst_addr, 16);
+		}
+
+		/**
+		 * Check if the next not void item is
+		 * TCP or UDP or SCTP or END.
+		 */
+		item = next_no_roughly_pattern(pattern, item);
+		if (item->type != RTE_FLOW_ITEM_TYPE_TCP &&
+		    item->type != RTE_FLOW_ITEM_TYPE_UDP &&
+		    item->type != RTE_FLOW_ITEM_TYPE_SCTP &&
+		    item->type != RTE_FLOW_ITEM_TYPE_END &&
+		    item->type != RTE_FLOW_ITEM_TYPE_RAW) {
+			memset(rule, 0, sizeof(struct ixgbe_fdir_rule));
+			rte_flow_error_set(error, EINVAL,
+				RTE_FLOW_ERROR_TYPE_ITEM,
+				item, "Not supported by fdir filter");
+			return -rte_errno;
+		}
+	}
+
 	/* Get the TCP info. */
 	if (item->type == RTE_FLOW_ITEM_TYPE_TCP) {
 		/**
 		 * Set the flow type even if there's no content
 		 * as we must have a flow type.
 		 */
-		rule->ixgbe_fdir.formatted.flow_type =
-			IXGBE_ATR_FLOW_TYPE_TCPV4;
+		rule->ixgbe_fdir.formatted.flow_type |=
+			IXGBE_ATR_L4TYPE_TCP;
 		/*Not supported last point for range*/
 		if (item->last) {
 			rte_flow_error_set(error, EINVAL,
@@ -1733,8 +1829,8 @@ ixgbe_parse_fdir_filter_normal(const struct rte_flow_attr *attr,
 		 * Set the flow type even if there's no content
 		 * as we must have a flow type.
 		 */
-		rule->ixgbe_fdir.formatted.flow_type =
-			IXGBE_ATR_FLOW_TYPE_UDPV4;
+		rule->ixgbe_fdir.formatted.flow_type |=
+			IXGBE_ATR_L4TYPE_UDP;
 		/*Not supported last point for range*/
 		if (item->last) {
 			rte_flow_error_set(error, EINVAL,
@@ -1793,8 +1889,8 @@ ixgbe_parse_fdir_filter_normal(const struct rte_flow_attr *attr,
 		 * Set the flow type even if there's no content
 		 * as we must have a flow type.
 		 */
-		rule->ixgbe_fdir.formatted.flow_type =
-			IXGBE_ATR_FLOW_TYPE_SCTPV4;
+		rule->ixgbe_fdir.formatted.flow_type |=
+			IXGBE_ATR_L4TYPE_SCTP;
 		/*Not supported last point for range*/
 		if (item->last) {
 			rte_flow_error_set(error, EINVAL,
-- 
2.7.4

Re: [PATCH v3] ethdev: add fuzzy match in flow API

[Ferruh Yigit]

On 6/13/2017 4:07 AM, Qi Zhang wrote:
> Add new meta pattern item RTE_FLOW_TYPE_ITEM_FUZZY in flow API.
> 
> This is for device that support fuzzy match option.
> Usually a fuzzy match is fast but the cost is accuracy.
> i.e. Signature Match only match pattern's hash value, but it is
> possible that two different patterns have the same hash value.
> 
> Matching accuracy level can be configured by subfield threshold.
> Driver can divide the range of threshold and map to different
> accuracy levels that device support.
> 
> Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>

Is there any more review for the patch?

There are some dependent driver patches sitting on my queue.

If there is no objection, I can get this one via next-net?

Thanks,
ferruh

Re: [PATCH v3] ethdev: add fuzzy match in flow API

[Thomas Monjalon]

13/06/2017 05:07, Qi Zhang:
> Add new meta pattern item RTE_FLOW_TYPE_ITEM_FUZZY in flow API.
> 
> This is for device that support fuzzy match option.
> Usually a fuzzy match is fast but the cost is accuracy.
> i.e. Signature Match only match pattern's hash value, but it is
> possible that two different patterns have the same hash value.
> 
> Matching accuracy level can be configured by subfield threshold.
> Driver can divide the range of threshold and map to different
> accuracy levels that device support.
> 
> Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>

Applied, thanks