Re: [dpdk-dev] [PATCH v4 5/6] testpmd: add support for MARK flow item

[Adrien Mazarguil <adrien.mazarguil@6wind.com>]

On Wed, Apr 18, 2018 at 10:04:22PM +0100, Declan Doherty wrote:
> Add support for specificaiton of new MARK flow item in testpmd's cli,
> and update testpmd documentation to describe new MARK flow item support.
> 
> Signed-off-by: Declan Doherty <declan.doherty@intel.com>

specificaiton => specification, however you should merge this in the
previous patch ("ethdev: add mark flow item to flow item types"), a few nits
below.

> ---
>  app/test-pmd/cmdline_flow.c                 | 22 ++++++++++++++++++++++
>  doc/guides/testpmd_app_ug/testpmd_funcs.rst |  4 ++++
>  2 files changed, 26 insertions(+)
> 
> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
> index 93e9a240d..e6284ce11 100644
> --- a/app/test-pmd/cmdline_flow.c
> +++ b/app/test-pmd/cmdline_flow.c
> @@ -87,6 +87,8 @@ enum index {
>  	ITEM_VF_ID,
>  	ITEM_PORT,
>  	ITEM_PORT_INDEX,
> +	ITEM_MARK,
> +	ITEM_MARK_ID,
>  	ITEM_RAW,
>  	ITEM_RAW_RELATIVE,
>  	ITEM_RAW_SEARCH,
> @@ -419,6 +421,7 @@ static const enum index next_item[] = {
>  	ITEM_PF,
>  	ITEM_VF,
>  	ITEM_PORT,
> +	ITEM_MARK,
>  	ITEM_RAW,
>  	ITEM_ETH,
>  	ITEM_VLAN,
> @@ -465,6 +468,12 @@ static const enum index item_port[] = {
>  	ZERO,
>  };
>  
> +static const enum index item_mark[] = {
> +	ITEM_MARK_ID,
> +	ITEM_NEXT,
> +	ZERO,
> +};
> +
>  static const enum index item_raw[] = {
>  	ITEM_RAW_RELATIVE,
>  	ITEM_RAW_SEARCH,
> @@ -1041,6 +1050,19 @@ static const struct token token_list[] = {
>  		.next = NEXT(item_port, NEXT_ENTRY(UNSIGNED), item_param),
>  		.args = ARGS(ARGS_ENTRY(struct rte_flow_item_port, index)),
>  	},
> +	[ITEM_MARK] = {
> +		.name = "mark",
> +		.help = "match packets against value set in previous group",

As previously discussed, you should drop the "previous group" part.

> +		.priv = PRIV_ITEM(MARK, sizeof(struct rte_flow_item_mark)),
> +		.next = NEXT(item_mark),
> +		.call = parse_vc,
> +	},
> +	[ITEM_MARK_ID] = {
> +		.name = "id",
> +		.help = "Integer value to match against",
> +		.next = NEXT(item_mark, NEXT_ENTRY(UNSIGNED), item_param),
> +		.args = ARGS(ARGS_ENTRY(struct rte_flow_item_mark, id)),
> +	},
>  	[ITEM_RAW] = {
>  		.name = "raw",
>  		.help = "match an arbitrary byte string",
> diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> index 7ecd602da..2b5ade4a8 100644
> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> @@ -3211,6 +3211,10 @@ This section lists supported pattern items and their attributes, if any.
>  
>    - ``index {unsigned}``: physical port index.
>  
> +- ``mark``: match value set in previously matched flow in higher priority group.
> +
> +  - ``id {unsigned}``: arbitrary integer value.
> +

Ditto.

>  - ``raw``: match an arbitrary byte string.
>  
>    - ``relative {boolean}``: look for pattern after the previous item.
> -- 
> 2.14.3
> 

-- 
Adrien Mazarguil
6WIND