[v3,01/12] ethdev: add port representor item to flow API

Message ID 20211010143930.4985-2-ivan.malov@oktetlabs.ru (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series ethdev: rework transfer flow API |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Ivan Malov Oct. 10, 2021, 2:39 p.m. UTC
  For use in "transfer" flows. Supposed to match traffic
entering the embedded switch from the given ethdev.

Must not be combined with direction attributes.

Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
---
 app/test-pmd/cmdline_flow.c                 | 27 ++++++++++
 doc/guides/prog_guide/rte_flow.rst          | 59 +++++++++++++++++++++
 doc/guides/rel_notes/release_21_11.rst      |  2 +
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |  4 ++
 lib/ethdev/rte_flow.c                       |  1 +
 lib/ethdev/rte_flow.h                       | 27 ++++++++++
 6 files changed, 120 insertions(+)
  

Comments

Ori Kam Oct. 11, 2021, 11:49 a.m. UTC | #1
Hi Ivan,

> -----Original Message-----
> From: Ivan Malov <ivan.malov@oktetlabs.ru>
> Sent: Sunday, October 10, 2021 5:39 PM
> Subject: [PATCH v3 01/12] ethdev: add port representor item to flow API
> 
> For use in "transfer" flows. Supposed to match traffic entering the embedded switch from the given
> ethdev.
> 
> Must not be combined with direction attributes.
> 
> Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
> ---

Acked-by: Ori Kam <orika@nvidia.com>
Thanks,
Ori
  
Andrew Rybchenko Oct. 12, 2021, 12:39 p.m. UTC | #2
On 10/11/21 2:49 PM, Ori Kam wrote:
> Hi Ivan,
> 
>> -----Original Message-----
>> From: Ivan Malov <ivan.malov@oktetlabs.ru>
>> Sent: Sunday, October 10, 2021 5:39 PM
>> Subject: [PATCH v3 01/12] ethdev: add port representor item to flow API
>>
>> For use in "transfer" flows. Supposed to match traffic entering the embedded switch from the given
>> ethdev.
>>
>> Must not be combined with direction attributes.
>>
>> Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
>> ---
> 
> Acked-by: Ori Kam <orika@nvidia.com>
> Thanks,
> Ori
> 

Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
  
Slava Ovsiienko Oct. 12, 2021, 8:55 p.m. UTC | #3
Hi,

Please, see below.

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Ivan Malov
> Sent: Sunday, October 10, 2021 17:39
> To: dev@dpdk.org
> Cc: NBU-Contact-Thomas Monjalon <thomas@monjalon.net>; Ori Kam
> <orika@nvidia.com>; Xiaoyun Li <xiaoyun.li@intel.com>; Ferruh Yigit
> <ferruh.yigit@intel.com>; Andrew Rybchenko
> <andrew.rybchenko@oktetlabs.ru>
> Subject: [dpdk-dev] [PATCH v3 01/12] ethdev: add port representor item to
> flow API
> 
> For use in "transfer" flows. Supposed to match traffic entering the embedded
> switch from the given ethdev.
> 
> Must not be combined with direction attributes.
> 
> Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
> ---
>  app/test-pmd/cmdline_flow.c                 | 27 ++++++++++

Should we separate testpmd changes into dedicated commit?
This patch intermixes RTE Flow API update and testpmd.

With best regards,
Slava

>  doc/guides/prog_guide/rte_flow.rst          | 59 +++++++++++++++++++++
>  doc/guides/rel_notes/release_21_11.rst      |  2 +
>  doc/guides/testpmd_app_ug/testpmd_funcs.rst |  4 ++
>  lib/ethdev/rte_flow.c                       |  1 +
>  lib/ethdev/rte_flow.h                       | 27 ++++++++++
>  6 files changed, 120 insertions(+)
> 
> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
> index bb22294dd3..a912a8d815 100644
> --- a/app/test-pmd/cmdline_flow.c
> +++ b/app/test-pmd/cmdline_flow.c
> @@ -306,6 +306,8 @@ enum index {
>  	ITEM_POL_PORT,
>  	ITEM_POL_METER,
>  	ITEM_POL_POLICY,
> +	ITEM_PORT_REPRESENTOR,
> +	ITEM_PORT_REPRESENTOR_PORT_ID,
> 
>  	/* Validate/create actions. */
>  	ACTIONS,
> @@ -1000,6 +1002,7 @@ static const enum index next_item[] = {
>  	ITEM_GENEVE_OPT,
>  	ITEM_INTEGRITY,
>  	ITEM_CONNTRACK,
> +	ITEM_PORT_REPRESENTOR,
>  	END_SET,
>  	ZERO,
>  };
> @@ -1368,6 +1371,12 @@ static const enum index item_integrity_lv[] = {
>  	ZERO,
>  };
> 
> +static const enum index item_port_representor[] = {
> +	ITEM_PORT_REPRESENTOR_PORT_ID,
> +	ITEM_NEXT,
> +	ZERO,
> +};
> +
>  static const enum index next_action[] = {
>  	ACTION_END,
>  	ACTION_VOID,
> @@ -3608,6 +3617,21 @@ static const struct token token_list[] = {
>  			     item_param),
>  		.args = ARGS(ARGS_ENTRY(struct rte_flow_item_conntrack,
> flags)),
>  	},
> +	[ITEM_PORT_REPRESENTOR] = {
> +		.name = "port_representor",
> +		.help = "match traffic entering the embedded switch from the
> given ethdev",
> +		.priv = PRIV_ITEM(PORT_REPRESENTOR,
> +				  sizeof(struct rte_flow_item_ethdev)),
> +		.next = NEXT(item_port_representor),
> +		.call = parse_vc,
> +	},
> +	[ITEM_PORT_REPRESENTOR_PORT_ID] = {
> +		.name = "port_id",
> +		.help = "ethdev port ID",
> +		.next = NEXT(item_port_representor,
> NEXT_ENTRY(COMMON_UNSIGNED),
> +			     item_param),
> +		.args = ARGS(ARGS_ENTRY(struct rte_flow_item_ethdev,
> port_id)),
> +	},
>  	/* Validate/create actions. */
>  	[ACTIONS] = {
>  		.name = "actions",
> @@ -8343,6 +8367,9 @@ flow_item_default_mask(const struct
> rte_flow_item *item)
>  	case RTE_FLOW_ITEM_TYPE_PFCP:
>  		mask = &rte_flow_item_pfcp_mask;
>  		break;
> +	case RTE_FLOW_ITEM_TYPE_PORT_REPRESENTOR:
> +		mask = &rte_flow_item_ethdev_mask;
> +		break;
>  	default:
>  		break;
>  	}
> diff --git a/doc/guides/prog_guide/rte_flow.rst
> b/doc/guides/prog_guide/rte_flow.rst
> index 2b42d5ec8c..91d5bdd712 100644
> --- a/doc/guides/prog_guide/rte_flow.rst
> +++ b/doc/guides/prog_guide/rte_flow.rst
> @@ -1425,6 +1425,65 @@ Matches a conntrack state after conntrack action.
>  - ``flags``: conntrack packet state flags.
>  - Default ``mask`` matches all state bits.
> 
> +Item: ``PORT_REPRESENTOR``
> +^^^^^^^^^^^^^^^^^^^^^^^^^^
> +
> +Matches traffic entering the embedded switch from the given ethdev.
> +
> +Term **ethdev** and the concept of **port representor** are synonymous.
> +The **represented port** is an *entity* plugged to the embedded switch
> +at the opposite end of the "wire" leading to the ethdev.
> +
> +::
> +
> +    .--------------------.
> +    |  PORT_REPRESENTOR  |  Ethdev (Application Port Referred to by its ID)
> +    '--------------------'
> +              ||
> +              \/
> +      .----------------.
> +      |  Logical Port  |
> +      '----------------'
> +              ||
> +              ||
> +              ||
> +              \/
> +         .----------.
> +         |  Switch  |
> +         '----------'
> +              :
> +               :
> +              :
> +               :
> +      .----------------.
> +      |  Logical Port  |
> +      '----------------'
> +              :
> +               :
> +    .--------------------.
> +    |  REPRESENTED_PORT  |  Net / Guest / Another Ethdev (Same
> Application)
> +    '--------------------'
> +
> +
> +- Incompatible with `Attribute: Traffic direction`_.
> +- Requires `Attribute: Transfer`_.
> +
> +.. _table_rte_flow_item_ethdev:
> +
> +.. table:: ``struct rte_flow_item_ethdev``
> +
> +   +----------+-------------+---------------------------+
> +   | Field    | Subfield    | Value                     |
> +   +==========+=============+===========================+
> +   | ``spec`` | ``port_id`` | ethdev port ID            |
> +   +----------+-------------+---------------------------+
> +   | ``last`` | ``port_id`` | upper range value         |
> +   +----------+-------------+---------------------------+
> +   | ``mask`` | ``port_id`` | zeroed for wildcard match |
> +   +----------+-------------+---------------------------+
> +
> +- Default ``mask`` provides exact match behaviour.
> +
>  Actions
>  ~~~~~~~
> 
> diff --git a/doc/guides/rel_notes/release_21_11.rst
> b/doc/guides/rel_notes/release_21_11.rst
> index 89d4b33ef1..1261cb2bf3 100644
> --- a/doc/guides/rel_notes/release_21_11.rst
> +++ b/doc/guides/rel_notes/release_21_11.rst
> @@ -188,6 +188,8 @@ API Changes
>     Also, make sure to start the actual text at the margin.
>     =======================================================
> 
> +* ethdev: Added item ``PORT_REPRESENTOR`` to flow API.
> +
>  * kvargs: The experimental function ``rte_kvargs_strcmp()`` has been
>    removed. Its usages have been replaced by a new function
>    ``rte_kvargs_get_with_value()``.
> diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> index 8ead7a4a71..dcb9f47d98 100644
> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> @@ -3795,6 +3795,10 @@ This section lists supported pattern items and
> their attributes, if any.
> 
>  - ``conntrack``: match conntrack state.
> 
> +- ``port_representor``: match traffic entering the embedded switch from
> +the given ethdev
> +
> +  - ``port_id {unsigned}``: ethdev port ID
> +
>  Actions list
>  ^^^^^^^^^^^^
> 
> diff --git a/lib/ethdev/rte_flow.c b/lib/ethdev/rte_flow.c index
> 8cb7a069c8..5e9317c6d1 100644
> --- a/lib/ethdev/rte_flow.c
> +++ b/lib/ethdev/rte_flow.c
> @@ -100,6 +100,7 @@ static const struct rte_flow_desc_data
> rte_flow_desc_item[] = {
>  	MK_FLOW_ITEM(GENEVE_OPT, sizeof(struct
> rte_flow_item_geneve_opt)),
>  	MK_FLOW_ITEM(INTEGRITY, sizeof(struct rte_flow_item_integrity)),
>  	MK_FLOW_ITEM(CONNTRACK, sizeof(uint32_t)),
> +	MK_FLOW_ITEM(PORT_REPRESENTOR, sizeof(struct
> rte_flow_item_ethdev)),
>  };
> 
>  /** Generate flow_action[] entry. */
> diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h index
> 7b1ed7f110..3625fd2c12 100644
> --- a/lib/ethdev/rte_flow.h
> +++ b/lib/ethdev/rte_flow.h
> @@ -574,6 +574,15 @@ enum rte_flow_item_type {
>  	 * @see struct rte_flow_item_conntrack.
>  	 */
>  	RTE_FLOW_ITEM_TYPE_CONNTRACK,
> +
> +	/**
> +	 * [META]
> +	 *
> +	 * Matches traffic entering the embedded switch from the given
> ethdev.
> +	 *
> +	 * @see struct rte_flow_item_ethdev
> +	 */
> +	RTE_FLOW_ITEM_TYPE_PORT_REPRESENTOR,
>  };
> 
>  /**
> @@ -1799,6 +1808,24 @@ static const struct rte_flow_item_conntrack
> rte_flow_item_conntrack_mask = {  };  #endif
> 
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this structure may change without prior notice
> + *
> + * Provides an ethdev port ID for use with the following items:
> + * RTE_FLOW_ITEM_TYPE_PORT_REPRESENTOR.
> + */
> +struct rte_flow_item_ethdev {
> +	uint16_t port_id; /**< ethdev port ID */ };
> +
> +/** Default mask for items based on struct rte_flow_item_ethdev */
> +#ifndef __cplusplus static const struct rte_flow_item_ethdev
> +rte_flow_item_ethdev_mask = {
> +	.port_id = 0xffff,
> +};
> +#endif
> +
>  /**
>   * Matching pattern item definition.
>   *
> --
> 2.20.1
  
Ivan Malov Oct. 12, 2021, 11:14 p.m. UTC | #4
Hi,

On 12/10/2021 23:55, Slava Ovsiienko wrote:
> Hi,
> 
> Please, see below.
> 
>> -----Original Message-----
>> From: dev <dev-bounces@dpdk.org> On Behalf Of Ivan Malov
>> Sent: Sunday, October 10, 2021 17:39
>> To: dev@dpdk.org
>> Cc: NBU-Contact-Thomas Monjalon <thomas@monjalon.net>; Ori Kam
>> <orika@nvidia.com>; Xiaoyun Li <xiaoyun.li@intel.com>; Ferruh Yigit
>> <ferruh.yigit@intel.com>; Andrew Rybchenko
>> <andrew.rybchenko@oktetlabs.ru>
>> Subject: [dpdk-dev] [PATCH v3 01/12] ethdev: add port representor item to
>> flow API
>>
>> For use in "transfer" flows. Supposed to match traffic entering the embedded
>> switch from the given ethdev.
>>
>> Must not be combined with direction attributes.
>>
>> Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
>> ---
>>   app/test-pmd/cmdline_flow.c                 | 27 ++++++++++
> 
> Should we separate testpmd changes into dedicated commit?
> This patch intermixes RTE Flow API update and testpmd.

I'm no fan of mixing things like that, but doing so appears to be rather 
logical. Each commit should be build testable. If the new defines have 
no users in the same commit, they are dummy. And I believe that readers 
typically want to see the use example in the same commit, too.

> 
> With best regards,
> Slava
> 
>>   doc/guides/prog_guide/rte_flow.rst          | 59 +++++++++++++++++++++
>>   doc/guides/rel_notes/release_21_11.rst      |  2 +
>>   doc/guides/testpmd_app_ug/testpmd_funcs.rst |  4 ++
>>   lib/ethdev/rte_flow.c                       |  1 +
>>   lib/ethdev/rte_flow.h                       | 27 ++++++++++
>>   6 files changed, 120 insertions(+)
>>
>> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
>> index bb22294dd3..a912a8d815 100644
>> --- a/app/test-pmd/cmdline_flow.c
>> +++ b/app/test-pmd/cmdline_flow.c
>> @@ -306,6 +306,8 @@ enum index {
>>   	ITEM_POL_PORT,
>>   	ITEM_POL_METER,
>>   	ITEM_POL_POLICY,
>> +	ITEM_PORT_REPRESENTOR,
>> +	ITEM_PORT_REPRESENTOR_PORT_ID,
>>
>>   	/* Validate/create actions. */
>>   	ACTIONS,
>> @@ -1000,6 +1002,7 @@ static const enum index next_item[] = {
>>   	ITEM_GENEVE_OPT,
>>   	ITEM_INTEGRITY,
>>   	ITEM_CONNTRACK,
>> +	ITEM_PORT_REPRESENTOR,
>>   	END_SET,
>>   	ZERO,
>>   };
>> @@ -1368,6 +1371,12 @@ static const enum index item_integrity_lv[] = {
>>   	ZERO,
>>   };
>>
>> +static const enum index item_port_representor[] = {
>> +	ITEM_PORT_REPRESENTOR_PORT_ID,
>> +	ITEM_NEXT,
>> +	ZERO,
>> +};
>> +
>>   static const enum index next_action[] = {
>>   	ACTION_END,
>>   	ACTION_VOID,
>> @@ -3608,6 +3617,21 @@ static const struct token token_list[] = {
>>   			     item_param),
>>   		.args = ARGS(ARGS_ENTRY(struct rte_flow_item_conntrack,
>> flags)),
>>   	},
>> +	[ITEM_PORT_REPRESENTOR] = {
>> +		.name = "port_representor",
>> +		.help = "match traffic entering the embedded switch from the
>> given ethdev",
>> +		.priv = PRIV_ITEM(PORT_REPRESENTOR,
>> +				  sizeof(struct rte_flow_item_ethdev)),
>> +		.next = NEXT(item_port_representor),
>> +		.call = parse_vc,
>> +	},
>> +	[ITEM_PORT_REPRESENTOR_PORT_ID] = {
>> +		.name = "port_id",
>> +		.help = "ethdev port ID",
>> +		.next = NEXT(item_port_representor,
>> NEXT_ENTRY(COMMON_UNSIGNED),
>> +			     item_param),
>> +		.args = ARGS(ARGS_ENTRY(struct rte_flow_item_ethdev,
>> port_id)),
>> +	},
>>   	/* Validate/create actions. */
>>   	[ACTIONS] = {
>>   		.name = "actions",
>> @@ -8343,6 +8367,9 @@ flow_item_default_mask(const struct
>> rte_flow_item *item)
>>   	case RTE_FLOW_ITEM_TYPE_PFCP:
>>   		mask = &rte_flow_item_pfcp_mask;
>>   		break;
>> +	case RTE_FLOW_ITEM_TYPE_PORT_REPRESENTOR:
>> +		mask = &rte_flow_item_ethdev_mask;
>> +		break;
>>   	default:
>>   		break;
>>   	}
>> diff --git a/doc/guides/prog_guide/rte_flow.rst
>> b/doc/guides/prog_guide/rte_flow.rst
>> index 2b42d5ec8c..91d5bdd712 100644
>> --- a/doc/guides/prog_guide/rte_flow.rst
>> +++ b/doc/guides/prog_guide/rte_flow.rst
>> @@ -1425,6 +1425,65 @@ Matches a conntrack state after conntrack action.
>>   - ``flags``: conntrack packet state flags.
>>   - Default ``mask`` matches all state bits.
>>
>> +Item: ``PORT_REPRESENTOR``
>> +^^^^^^^^^^^^^^^^^^^^^^^^^^
>> +
>> +Matches traffic entering the embedded switch from the given ethdev.
>> +
>> +Term **ethdev** and the concept of **port representor** are synonymous.
>> +The **represented port** is an *entity* plugged to the embedded switch
>> +at the opposite end of the "wire" leading to the ethdev.
>> +
>> +::
>> +
>> +    .--------------------.
>> +    |  PORT_REPRESENTOR  |  Ethdev (Application Port Referred to by its ID)
>> +    '--------------------'
>> +              ||
>> +              \/
>> +      .----------------.
>> +      |  Logical Port  |
>> +      '----------------'
>> +              ||
>> +              ||
>> +              ||
>> +              \/
>> +         .----------.
>> +         |  Switch  |
>> +         '----------'
>> +              :
>> +               :
>> +              :
>> +               :
>> +      .----------------.
>> +      |  Logical Port  |
>> +      '----------------'
>> +              :
>> +               :
>> +    .--------------------.
>> +    |  REPRESENTED_PORT  |  Net / Guest / Another Ethdev (Same
>> Application)
>> +    '--------------------'
>> +
>> +
>> +- Incompatible with `Attribute: Traffic direction`_.
>> +- Requires `Attribute: Transfer`_.
>> +
>> +.. _table_rte_flow_item_ethdev:
>> +
>> +.. table:: ``struct rte_flow_item_ethdev``
>> +
>> +   +----------+-------------+---------------------------+
>> +   | Field    | Subfield    | Value                     |
>> +   +==========+=============+===========================+
>> +   | ``spec`` | ``port_id`` | ethdev port ID            |
>> +   +----------+-------------+---------------------------+
>> +   | ``last`` | ``port_id`` | upper range value         |
>> +   +----------+-------------+---------------------------+
>> +   | ``mask`` | ``port_id`` | zeroed for wildcard match |
>> +   +----------+-------------+---------------------------+
>> +
>> +- Default ``mask`` provides exact match behaviour.
>> +
>>   Actions
>>   ~~~~~~~
>>
>> diff --git a/doc/guides/rel_notes/release_21_11.rst
>> b/doc/guides/rel_notes/release_21_11.rst
>> index 89d4b33ef1..1261cb2bf3 100644
>> --- a/doc/guides/rel_notes/release_21_11.rst
>> +++ b/doc/guides/rel_notes/release_21_11.rst
>> @@ -188,6 +188,8 @@ API Changes
>>      Also, make sure to start the actual text at the margin.
>>      =======================================================
>>
>> +* ethdev: Added item ``PORT_REPRESENTOR`` to flow API.
>> +
>>   * kvargs: The experimental function ``rte_kvargs_strcmp()`` has been
>>     removed. Its usages have been replaced by a new function
>>     ``rte_kvargs_get_with_value()``.
>> diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>> b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>> index 8ead7a4a71..dcb9f47d98 100644
>> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>> @@ -3795,6 +3795,10 @@ This section lists supported pattern items and
>> their attributes, if any.
>>
>>   - ``conntrack``: match conntrack state.
>>
>> +- ``port_representor``: match traffic entering the embedded switch from
>> +the given ethdev
>> +
>> +  - ``port_id {unsigned}``: ethdev port ID
>> +
>>   Actions list
>>   ^^^^^^^^^^^^
>>
>> diff --git a/lib/ethdev/rte_flow.c b/lib/ethdev/rte_flow.c index
>> 8cb7a069c8..5e9317c6d1 100644
>> --- a/lib/ethdev/rte_flow.c
>> +++ b/lib/ethdev/rte_flow.c
>> @@ -100,6 +100,7 @@ static const struct rte_flow_desc_data
>> rte_flow_desc_item[] = {
>>   	MK_FLOW_ITEM(GENEVE_OPT, sizeof(struct
>> rte_flow_item_geneve_opt)),
>>   	MK_FLOW_ITEM(INTEGRITY, sizeof(struct rte_flow_item_integrity)),
>>   	MK_FLOW_ITEM(CONNTRACK, sizeof(uint32_t)),
>> +	MK_FLOW_ITEM(PORT_REPRESENTOR, sizeof(struct
>> rte_flow_item_ethdev)),
>>   };
>>
>>   /** Generate flow_action[] entry. */
>> diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h index
>> 7b1ed7f110..3625fd2c12 100644
>> --- a/lib/ethdev/rte_flow.h
>> +++ b/lib/ethdev/rte_flow.h
>> @@ -574,6 +574,15 @@ enum rte_flow_item_type {
>>   	 * @see struct rte_flow_item_conntrack.
>>   	 */
>>   	RTE_FLOW_ITEM_TYPE_CONNTRACK,
>> +
>> +	/**
>> +	 * [META]
>> +	 *
>> +	 * Matches traffic entering the embedded switch from the given
>> ethdev.
>> +	 *
>> +	 * @see struct rte_flow_item_ethdev
>> +	 */
>> +	RTE_FLOW_ITEM_TYPE_PORT_REPRESENTOR,
>>   };
>>
>>   /**
>> @@ -1799,6 +1808,24 @@ static const struct rte_flow_item_conntrack
>> rte_flow_item_conntrack_mask = {  };  #endif
>>
>> +/**
>> + * @warning
>> + * @b EXPERIMENTAL: this structure may change without prior notice
>> + *
>> + * Provides an ethdev port ID for use with the following items:
>> + * RTE_FLOW_ITEM_TYPE_PORT_REPRESENTOR.
>> + */
>> +struct rte_flow_item_ethdev {
>> +	uint16_t port_id; /**< ethdev port ID */ };
>> +
>> +/** Default mask for items based on struct rte_flow_item_ethdev */
>> +#ifndef __cplusplus static const struct rte_flow_item_ethdev
>> +rte_flow_item_ethdev_mask = {
>> +	.port_id = 0xffff,
>> +};
>> +#endif
>> +
>>   /**
>>    * Matching pattern item definition.
>>    *
>> --
>> 2.20.1
  
Ferruh Yigit Oct. 13, 2021, 11:52 a.m. UTC | #5
On 10/10/2021 3:39 PM, Ivan Malov wrote:
> diff --git a/doc/guides/rel_notes/release_21_11.rst b/doc/guides/rel_notes/release_21_11.rst
> index 89d4b33ef1..1261cb2bf3 100644
> --- a/doc/guides/rel_notes/release_21_11.rst
> +++ b/doc/guides/rel_notes/release_21_11.rst
> @@ -188,6 +188,8 @@ API Changes
>      Also, make sure to start the actual text at the margin.
>      =======================================================
>   
> +* ethdev: Added item ``PORT_REPRESENTOR`` to flow API.
> +

Since there will be a new version, can you please add 'ethdev' update after
core libraries, for existing doc it is after 'cryptodev' changes?
  
Slava Ovsiienko Oct. 13, 2021, 6:26 p.m. UTC | #6
Hi,

> -----Original Message-----
> From: Ivan Malov <Ivan.Malov@oktetlabs.ru>
> Sent: Wednesday, October 13, 2021 2:15
> To: Slava Ovsiienko <viacheslavo@nvidia.com>; dev@dpdk.org
> Cc: NBU-Contact-Thomas Monjalon <thomas@monjalon.net>; Ori Kam
> <orika@nvidia.com>; Xiaoyun Li <xiaoyun.li@intel.com>; Ferruh Yigit
> <ferruh.yigit@intel.com>; Andrew Rybchenko
> <andrew.rybchenko@oktetlabs.ru>
> Subject: Re: [dpdk-dev] [PATCH v3 01/12] ethdev: add port representor item
> to flow API
> 
> Hi,
> 
> On 12/10/2021 23:55, Slava Ovsiienko wrote:
> > Hi,
> >
> > Please, see below.
> >
> >> -----Original Message-----
> >> From: dev <dev-bounces@dpdk.org> On Behalf Of Ivan Malov
> >> Sent: Sunday, October 10, 2021 17:39
> >> To: dev@dpdk.org
> >> Cc: NBU-Contact-Thomas Monjalon <thomas@monjalon.net>; Ori Kam
> >> <orika@nvidia.com>; Xiaoyun Li <xiaoyun.li@intel.com>; Ferruh Yigit
> >> <ferruh.yigit@intel.com>; Andrew Rybchenko
> >> <andrew.rybchenko@oktetlabs.ru>
> >> Subject: [dpdk-dev] [PATCH v3 01/12] ethdev: add port representor
> >> item to flow API
> >>
> >> For use in "transfer" flows. Supposed to match traffic entering the
> >> embedded switch from the given ethdev.
> >>
> >> Must not be combined with direction attributes.
> >>
> >> Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
> >> ---
> >>   app/test-pmd/cmdline_flow.c                 | 27 ++++++++++
> >
> > Should we separate testpmd changes into dedicated commit?
> > This patch intermixes RTE Flow API update and testpmd.
> 
> I'm no fan of mixing things like that, but doing so appears to be rather logical.
> Each commit should be build testable. If the new defines have no users in the
> same commit, they are dummy. And I believe that readers typically want to
> see the use example in the same commit, too.
Sometime the testpmd change is larged and should be reviewed carefully.
But this series is not a case, and I see it is common practice to include
small updates of testpmd in the primary patch. I wonder why I do not follow
this practice 😉
OK, please, disregard my comment in previous mail.

With best regards, Slava

> 
> >
> > With best regards,
> > Slava
> >
> >>   doc/guides/prog_guide/rte_flow.rst          | 59 +++++++++++++++++++++
> >>   doc/guides/rel_notes/release_21_11.rst      |  2 +
> >>   doc/guides/testpmd_app_ug/testpmd_funcs.rst |  4 ++
> >>   lib/ethdev/rte_flow.c                       |  1 +
> >>   lib/ethdev/rte_flow.h                       | 27 ++++++++++
> >>   6 files changed, 120 insertions(+)
> >>
> >> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
> >> index bb22294dd3..a912a8d815 100644
> >> --- a/app/test-pmd/cmdline_flow.c
> >> +++ b/app/test-pmd/cmdline_flow.c
> >> @@ -306,6 +306,8 @@ enum index {
> >>   	ITEM_POL_PORT,
> >>   	ITEM_POL_METER,
> >>   	ITEM_POL_POLICY,
> >> +	ITEM_PORT_REPRESENTOR,
> >> +	ITEM_PORT_REPRESENTOR_PORT_ID,
> >>
> >>   	/* Validate/create actions. */
> >>   	ACTIONS,
> >> @@ -1000,6 +1002,7 @@ static const enum index next_item[] = {
> >>   	ITEM_GENEVE_OPT,
> >>   	ITEM_INTEGRITY,
> >>   	ITEM_CONNTRACK,
> >> +	ITEM_PORT_REPRESENTOR,
> >>   	END_SET,
> >>   	ZERO,
> >>   };
> >> @@ -1368,6 +1371,12 @@ static const enum index item_integrity_lv[] = {
> >>   	ZERO,
> >>   };
> >>
> >> +static const enum index item_port_representor[] = {
> >> +	ITEM_PORT_REPRESENTOR_PORT_ID,
> >> +	ITEM_NEXT,
> >> +	ZERO,
> >> +};
> >> +
> >>   static const enum index next_action[] = {
> >>   	ACTION_END,
> >>   	ACTION_VOID,
> >> @@ -3608,6 +3617,21 @@ static const struct token token_list[] = {
> >>   			     item_param),
> >>   		.args = ARGS(ARGS_ENTRY(struct rte_flow_item_conntrack,
> >> flags)),
> >>   	},
> >> +	[ITEM_PORT_REPRESENTOR] = {
> >> +		.name = "port_representor",
> >> +		.help = "match traffic entering the embedded switch from the
> >> given ethdev",
> >> +		.priv = PRIV_ITEM(PORT_REPRESENTOR,
> >> +				  sizeof(struct rte_flow_item_ethdev)),
> >> +		.next = NEXT(item_port_representor),
> >> +		.call = parse_vc,
> >> +	},
> >> +	[ITEM_PORT_REPRESENTOR_PORT_ID] = {
> >> +		.name = "port_id",
> >> +		.help = "ethdev port ID",
> >> +		.next = NEXT(item_port_representor,
> >> NEXT_ENTRY(COMMON_UNSIGNED),
> >> +			     item_param),
> >> +		.args = ARGS(ARGS_ENTRY(struct rte_flow_item_ethdev,
> >> port_id)),
> >> +	},
> >>   	/* Validate/create actions. */
> >>   	[ACTIONS] = {
> >>   		.name = "actions",
> >> @@ -8343,6 +8367,9 @@ flow_item_default_mask(const struct
> >> rte_flow_item *item)
> >>   	case RTE_FLOW_ITEM_TYPE_PFCP:
> >>   		mask = &rte_flow_item_pfcp_mask;
> >>   		break;
> >> +	case RTE_FLOW_ITEM_TYPE_PORT_REPRESENTOR:
> >> +		mask = &rte_flow_item_ethdev_mask;
> >> +		break;
> >>   	default:
> >>   		break;
> >>   	}
> >> diff --git a/doc/guides/prog_guide/rte_flow.rst
> >> b/doc/guides/prog_guide/rte_flow.rst
> >> index 2b42d5ec8c..91d5bdd712 100644
> >> --- a/doc/guides/prog_guide/rte_flow.rst
> >> +++ b/doc/guides/prog_guide/rte_flow.rst
> >> @@ -1425,6 +1425,65 @@ Matches a conntrack state after conntrack
> action.
> >>   - ``flags``: conntrack packet state flags.
> >>   - Default ``mask`` matches all state bits.
> >>
> >> +Item: ``PORT_REPRESENTOR``
> >> +^^^^^^^^^^^^^^^^^^^^^^^^^^
> >> +
> >> +Matches traffic entering the embedded switch from the given ethdev.
> >> +
> >> +Term **ethdev** and the concept of **port representor** are
> synonymous.
> >> +The **represented port** is an *entity* plugged to the embedded switch
> >> +at the opposite end of the "wire" leading to the ethdev.
> >> +
> >> +::
> >> +
> >> +    .--------------------.
> >> +    |  PORT_REPRESENTOR  |  Ethdev (Application Port Referred to by its
> ID)
> >> +    '--------------------'
> >> +              ||
> >> +              \/
> >> +      .----------------.
> >> +      |  Logical Port  |
> >> +      '----------------'
> >> +              ||
> >> +              ||
> >> +              ||
> >> +              \/
> >> +         .----------.
> >> +         |  Switch  |
> >> +         '----------'
> >> +              :
> >> +               :
> >> +              :
> >> +               :
> >> +      .----------------.
> >> +      |  Logical Port  |
> >> +      '----------------'
> >> +              :
> >> +               :
> >> +    .--------------------.
> >> +    |  REPRESENTED_PORT  |  Net / Guest / Another Ethdev (Same
> >> Application)
> >> +    '--------------------'
> >> +
> >> +
> >> +- Incompatible with `Attribute: Traffic direction`_.
> >> +- Requires `Attribute: Transfer`_.
> >> +
> >> +.. _table_rte_flow_item_ethdev:
> >> +
> >> +.. table:: ``struct rte_flow_item_ethdev``
> >> +
> >> +   +----------+-------------+---------------------------+
> >> +   | Field    | Subfield    | Value                     |
> >> +   +==========+=============+===========================+
> >> +   | ``spec`` | ``port_id`` | ethdev port ID            |
> >> +   +----------+-------------+---------------------------+
> >> +   | ``last`` | ``port_id`` | upper range value         |
> >> +   +----------+-------------+---------------------------+
> >> +   | ``mask`` | ``port_id`` | zeroed for wildcard match |
> >> +   +----------+-------------+---------------------------+
> >> +
> >> +- Default ``mask`` provides exact match behaviour.
> >> +
> >>   Actions
> >>   ~~~~~~~
> >>
> >> diff --git a/doc/guides/rel_notes/release_21_11.rst
> >> b/doc/guides/rel_notes/release_21_11.rst
> >> index 89d4b33ef1..1261cb2bf3 100644
> >> --- a/doc/guides/rel_notes/release_21_11.rst
> >> +++ b/doc/guides/rel_notes/release_21_11.rst
> >> @@ -188,6 +188,8 @@ API Changes
> >>      Also, make sure to start the actual text at the margin.
> >>      =======================================================
> >>
> >> +* ethdev: Added item ``PORT_REPRESENTOR`` to flow API.
> >> +
> >>   * kvargs: The experimental function ``rte_kvargs_strcmp()`` has been
> >>     removed. Its usages have been replaced by a new function
> >>     ``rte_kvargs_get_with_value()``.
> >> diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> >> b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> >> index 8ead7a4a71..dcb9f47d98 100644
> >> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> >> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> >> @@ -3795,6 +3795,10 @@ This section lists supported pattern items and
> >> their attributes, if any.
> >>
> >>   - ``conntrack``: match conntrack state.
> >>
> >> +- ``port_representor``: match traffic entering the embedded switch from
> >> +the given ethdev
> >> +
> >> +  - ``port_id {unsigned}``: ethdev port ID
> >> +
> >>   Actions list
> >>   ^^^^^^^^^^^^
> >>
> >> diff --git a/lib/ethdev/rte_flow.c b/lib/ethdev/rte_flow.c index
> >> 8cb7a069c8..5e9317c6d1 100644
> >> --- a/lib/ethdev/rte_flow.c
> >> +++ b/lib/ethdev/rte_flow.c
> >> @@ -100,6 +100,7 @@ static const struct rte_flow_desc_data
> >> rte_flow_desc_item[] = {
> >>   	MK_FLOW_ITEM(GENEVE_OPT, sizeof(struct
> >> rte_flow_item_geneve_opt)),
> >>   	MK_FLOW_ITEM(INTEGRITY, sizeof(struct rte_flow_item_integrity)),
> >>   	MK_FLOW_ITEM(CONNTRACK, sizeof(uint32_t)),
> >> +	MK_FLOW_ITEM(PORT_REPRESENTOR, sizeof(struct
> >> rte_flow_item_ethdev)),
> >>   };
> >>
> >>   /** Generate flow_action[] entry. */
> >> diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h index
> >> 7b1ed7f110..3625fd2c12 100644
> >> --- a/lib/ethdev/rte_flow.h
> >> +++ b/lib/ethdev/rte_flow.h
> >> @@ -574,6 +574,15 @@ enum rte_flow_item_type {
> >>   	 * @see struct rte_flow_item_conntrack.
> >>   	 */
> >>   	RTE_FLOW_ITEM_TYPE_CONNTRACK,
> >> +
> >> +	/**
> >> +	 * [META]
> >> +	 *
> >> +	 * Matches traffic entering the embedded switch from the given
> >> ethdev.
> >> +	 *
> >> +	 * @see struct rte_flow_item_ethdev
> >> +	 */
> >> +	RTE_FLOW_ITEM_TYPE_PORT_REPRESENTOR,
> >>   };
> >>
> >>   /**
> >> @@ -1799,6 +1808,24 @@ static const struct rte_flow_item_conntrack
> >> rte_flow_item_conntrack_mask = {  };  #endif
> >>
> >> +/**
> >> + * @warning
> >> + * @b EXPERIMENTAL: this structure may change without prior notice
> >> + *
> >> + * Provides an ethdev port ID for use with the following items:
> >> + * RTE_FLOW_ITEM_TYPE_PORT_REPRESENTOR.
> >> + */
> >> +struct rte_flow_item_ethdev {
> >> +	uint16_t port_id; /**< ethdev port ID */ };
> >> +
> >> +/** Default mask for items based on struct rte_flow_item_ethdev */
> >> +#ifndef __cplusplus static const struct rte_flow_item_ethdev
> >> +rte_flow_item_ethdev_mask = {
> >> +	.port_id = 0xffff,
> >> +};
> >> +#endif
> >> +
> >>   /**
> >>    * Matching pattern item definition.
> >>    *
> >> --
> >> 2.20.1
> 
> --
> Ivan M
  

Patch

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index bb22294dd3..a912a8d815 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -306,6 +306,8 @@  enum index {
 	ITEM_POL_PORT,
 	ITEM_POL_METER,
 	ITEM_POL_POLICY,
+	ITEM_PORT_REPRESENTOR,
+	ITEM_PORT_REPRESENTOR_PORT_ID,
 
 	/* Validate/create actions. */
 	ACTIONS,
@@ -1000,6 +1002,7 @@  static const enum index next_item[] = {
 	ITEM_GENEVE_OPT,
 	ITEM_INTEGRITY,
 	ITEM_CONNTRACK,
+	ITEM_PORT_REPRESENTOR,
 	END_SET,
 	ZERO,
 };
@@ -1368,6 +1371,12 @@  static const enum index item_integrity_lv[] = {
 	ZERO,
 };
 
+static const enum index item_port_representor[] = {
+	ITEM_PORT_REPRESENTOR_PORT_ID,
+	ITEM_NEXT,
+	ZERO,
+};
+
 static const enum index next_action[] = {
 	ACTION_END,
 	ACTION_VOID,
@@ -3608,6 +3617,21 @@  static const struct token token_list[] = {
 			     item_param),
 		.args = ARGS(ARGS_ENTRY(struct rte_flow_item_conntrack, flags)),
 	},
+	[ITEM_PORT_REPRESENTOR] = {
+		.name = "port_representor",
+		.help = "match traffic entering the embedded switch from the given ethdev",
+		.priv = PRIV_ITEM(PORT_REPRESENTOR,
+				  sizeof(struct rte_flow_item_ethdev)),
+		.next = NEXT(item_port_representor),
+		.call = parse_vc,
+	},
+	[ITEM_PORT_REPRESENTOR_PORT_ID] = {
+		.name = "port_id",
+		.help = "ethdev port ID",
+		.next = NEXT(item_port_representor, NEXT_ENTRY(COMMON_UNSIGNED),
+			     item_param),
+		.args = ARGS(ARGS_ENTRY(struct rte_flow_item_ethdev, port_id)),
+	},
 	/* Validate/create actions. */
 	[ACTIONS] = {
 		.name = "actions",
@@ -8343,6 +8367,9 @@  flow_item_default_mask(const struct rte_flow_item *item)
 	case RTE_FLOW_ITEM_TYPE_PFCP:
 		mask = &rte_flow_item_pfcp_mask;
 		break;
+	case RTE_FLOW_ITEM_TYPE_PORT_REPRESENTOR:
+		mask = &rte_flow_item_ethdev_mask;
+		break;
 	default:
 		break;
 	}
diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
index 2b42d5ec8c..91d5bdd712 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -1425,6 +1425,65 @@  Matches a conntrack state after conntrack action.
 - ``flags``: conntrack packet state flags.
 - Default ``mask`` matches all state bits.
 
+Item: ``PORT_REPRESENTOR``
+^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+Matches traffic entering the embedded switch from the given ethdev.
+
+Term **ethdev** and the concept of **port representor** are synonymous.
+The **represented port** is an *entity* plugged to the embedded switch
+at the opposite end of the "wire" leading to the ethdev.
+
+::
+
+    .--------------------.
+    |  PORT_REPRESENTOR  |  Ethdev (Application Port Referred to by its ID)
+    '--------------------'
+              ||
+              \/
+      .----------------.
+      |  Logical Port  |
+      '----------------'
+              ||
+              ||
+              ||
+              \/
+         .----------.
+         |  Switch  |
+         '----------'
+              :
+               :
+              :
+               :
+      .----------------.
+      |  Logical Port  |
+      '----------------'
+              :
+               :
+    .--------------------.
+    |  REPRESENTED_PORT  |  Net / Guest / Another Ethdev (Same Application)
+    '--------------------'
+
+
+- Incompatible with `Attribute: Traffic direction`_.
+- Requires `Attribute: Transfer`_.
+
+.. _table_rte_flow_item_ethdev:
+
+.. table:: ``struct rte_flow_item_ethdev``
+
+   +----------+-------------+---------------------------+
+   | Field    | Subfield    | Value                     |
+   +==========+=============+===========================+
+   | ``spec`` | ``port_id`` | ethdev port ID            |
+   +----------+-------------+---------------------------+
+   | ``last`` | ``port_id`` | upper range value         |
+   +----------+-------------+---------------------------+
+   | ``mask`` | ``port_id`` | zeroed for wildcard match |
+   +----------+-------------+---------------------------+
+
+- Default ``mask`` provides exact match behaviour.
+
 Actions
 ~~~~~~~
 
diff --git a/doc/guides/rel_notes/release_21_11.rst b/doc/guides/rel_notes/release_21_11.rst
index 89d4b33ef1..1261cb2bf3 100644
--- a/doc/guides/rel_notes/release_21_11.rst
+++ b/doc/guides/rel_notes/release_21_11.rst
@@ -188,6 +188,8 @@  API Changes
    Also, make sure to start the actual text at the margin.
    =======================================================
 
+* ethdev: Added item ``PORT_REPRESENTOR`` to flow API.
+
 * kvargs: The experimental function ``rte_kvargs_strcmp()`` has been
   removed. Its usages have been replaced by a new function
   ``rte_kvargs_get_with_value()``.
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index 8ead7a4a71..dcb9f47d98 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -3795,6 +3795,10 @@  This section lists supported pattern items and their attributes, if any.
 
 - ``conntrack``: match conntrack state.
 
+- ``port_representor``: match traffic entering the embedded switch from the given ethdev
+
+  - ``port_id {unsigned}``: ethdev port ID
+
 Actions list
 ^^^^^^^^^^^^
 
diff --git a/lib/ethdev/rte_flow.c b/lib/ethdev/rte_flow.c
index 8cb7a069c8..5e9317c6d1 100644
--- a/lib/ethdev/rte_flow.c
+++ b/lib/ethdev/rte_flow.c
@@ -100,6 +100,7 @@  static const struct rte_flow_desc_data rte_flow_desc_item[] = {
 	MK_FLOW_ITEM(GENEVE_OPT, sizeof(struct rte_flow_item_geneve_opt)),
 	MK_FLOW_ITEM(INTEGRITY, sizeof(struct rte_flow_item_integrity)),
 	MK_FLOW_ITEM(CONNTRACK, sizeof(uint32_t)),
+	MK_FLOW_ITEM(PORT_REPRESENTOR, sizeof(struct rte_flow_item_ethdev)),
 };
 
 /** Generate flow_action[] entry. */
diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
index 7b1ed7f110..3625fd2c12 100644
--- a/lib/ethdev/rte_flow.h
+++ b/lib/ethdev/rte_flow.h
@@ -574,6 +574,15 @@  enum rte_flow_item_type {
 	 * @see struct rte_flow_item_conntrack.
 	 */
 	RTE_FLOW_ITEM_TYPE_CONNTRACK,
+
+	/**
+	 * [META]
+	 *
+	 * Matches traffic entering the embedded switch from the given ethdev.
+	 *
+	 * @see struct rte_flow_item_ethdev
+	 */
+	RTE_FLOW_ITEM_TYPE_PORT_REPRESENTOR,
 };
 
 /**
@@ -1799,6 +1808,24 @@  static const struct rte_flow_item_conntrack rte_flow_item_conntrack_mask = {
 };
 #endif
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this structure may change without prior notice
+ *
+ * Provides an ethdev port ID for use with the following items:
+ * RTE_FLOW_ITEM_TYPE_PORT_REPRESENTOR.
+ */
+struct rte_flow_item_ethdev {
+	uint16_t port_id; /**< ethdev port ID */
+};
+
+/** Default mask for items based on struct rte_flow_item_ethdev */
+#ifndef __cplusplus
+static const struct rte_flow_item_ethdev rte_flow_item_ethdev_mask = {
+	.port_id = 0xffff,
+};
+#endif
+
 /**
  * Matching pattern item definition.
  *