Hi Andrew and Ivan,
> -----Original Message-----
> From: Andrew Rybchenko <andrew.rybche...@oktetlabs.ru>
> Sent: Friday, October 1, 2021 4:47 PM
> Subject: [PATCH v1 02/12] ethdev: add eswitch port item to flow API
>
> From: Ivan Malov <ivan.ma...@oktetlabs.ru>
>
> For use with "transfer" flows. Supposed to match traffic entering the e-switch
> from the external world (network, guests) via the port which is logically
> connected with the given ethdev.
>
> Must not be combined with attributes "ingress" / "egress".
>
> This item is meant to use the same structure as ethdev item.
>
In case the app is not working with representors, meaning
each switch port is mapped to ethdev.
both items (ethdev and eswitch port ) have the same meaning?
> Signed-off-by: Ivan Malov <ivan.ma...@oktetlabs.ru>
> Signed-off-by: Andrew Rybchenko <andrew.rybche...@oktetlabs.ru>
> ---
> app/test-pmd/cmdline_flow.c | 27 +++++++++++++++++++++
> doc/guides/prog_guide/rte_flow.rst | 22 +++++++++++++++++
> 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 | 12 ++++++++-
> 6 files changed, 66 insertions(+), 2 deletions(-)
>
> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
> index e05b0d83d2..188d0ee39d 100644
> --- a/app/test-pmd/cmdline_flow.c
> +++ b/app/test-pmd/cmdline_flow.c
> @@ -308,6 +308,8 @@ enum index {
> ITEM_POL_POLICY,
> ITEM_ETHDEV,
> ITEM_ETHDEV_ID,
> + ITEM_ESWITCH_PORT,
> + ITEM_ESWITCH_PORT_ETHDEV_ID,
Like my comment from previous patch, I'm not sure the correct
term for ETHDEV is ID is should be port.
>
> /* Validate/create actions. */
> ACTIONS,
> @@ -1003,6 +1005,7 @@ static const enum index next_item[] = {
> ITEM_INTEGRITY,
> ITEM_CONNTRACK,
> ITEM_ETHDEV,
> + ITEM_ESWITCH_PORT,
> END_SET,
> ZERO,
> };
> @@ -1377,6 +1380,12 @@ static const enum index item_ethdev[] = {
> ZERO,
> };
>
> +static const enum index item_eswitch_port[] = {
> + ITEM_ESWITCH_PORT_ETHDEV_ID,
> + ITEM_NEXT,
> + ZERO,
> +};
> +
> static const enum index next_action[] = {
> ACTION_END,
> ACTION_VOID,
> @@ -3632,6 +3641,21 @@ static const struct token token_list[] = {
> item_param),
> .args = ARGS(ARGS_ENTRY(struct rte_flow_item_ethdev, id)),
> },
> + [ITEM_ESWITCH_PORT] = {
> + .name = "eswitch_port",
> + .help = "match traffic at e-switch going from the external port
> associated with the given ethdev",
Missing the word logically since if we are talking about representor the
connected port
is the PF while we want to match traffic on one of the FVs.
> + .priv = PRIV_ITEM(ESWITCH_PORT,
> + sizeof(struct rte_flow_item_ethdev)),
> + .next = NEXT(item_eswitch_port),
> + .call = parse_vc,
> + },
> + [ITEM_ESWITCH_PORT_ETHDEV_ID] = {
> + .name = "ethdev_id",
> + .help = "ethdev ID",
> + .next = NEXT(item_eswitch_port,
> NEXT_ENTRY(COMMON_UNSIGNED),
> + item_param),
> + .args = ARGS(ARGS_ENTRY(struct rte_flow_item_ethdev, id)),
> + },
> /* Validate/create actions. */
> [ACTIONS] = {
> .name = "actions",
> @@ -8370,6 +8394,9 @@ flow_item_default_mask(const struct
> rte_flow_item *item)
> case RTE_FLOW_ITEM_TYPE_ETHDEV:
> mask = &rte_flow_item_ethdev_mask;
> break;
> + case RTE_FLOW_ITEM_TYPE_ESWITCH_PORT:
> + mask = &rte_flow_item_ethdev_mask;
> + break;
Not sure maybe merged the two cases?
> default:
> break;
> }
> diff --git a/doc/guides/prog_guide/rte_flow.rst
> b/doc/guides/prog_guide/rte_flow.rst
> index ab628d9139..292bb42410 100644
> --- a/doc/guides/prog_guide/rte_flow.rst
> +++ b/doc/guides/prog_guide/rte_flow.rst
> @@ -1460,6 +1460,28 @@ Use this with attribute **transfer**. Attributes
> **ingress** and
> | ``mask`` | ``id`` | zeroed for wildcard match |
> +----------+----------+---------------------------+
>
> +Item: ``ESWITCH_PORT``
> +^^^^^^^^^^^^^^^^^^^^^^
> +
> +Matches traffic at e-switch going from the external port associated
> +with the given ethdev, for example, traffic from net. port or guest.
Maybe replace external with e-switch?
> +
> +::
> +
> + * (Ethdev) ~~~~~~~~~~~~ (Internal Port) ~~~~ [] <<<< (External Port)
> + * : SW : Logical Net / Guest :
> + * : : :
> + * | ---- PMD Layer ---- | ------------ E-Switch Layer ------------ |
> + *
> + * [] shows the effective ("transfer") standpoint, the match engine;
> + * << shows the traffic flow in question hitting the match engine;
> + * ~~ shows logical interconnects between the endpoints.
> +
I'm not sure I understand this diagram.
> +Use this with attribute **transfer**. Attributes **ingress** and
> +**egress** (`Attribute: Traffic direction`_) must not be used.
> +
> +This item is meant to use the same structure as `Item: ETHDEV`_.
> +
> Actions
> ~~~~~~~
>
> diff --git a/doc/guides/rel_notes/release_21_11.rst
> b/doc/guides/rel_notes/release_21_11.rst
> index 91631adb4e..b2b27de3f0 100644
> --- a/doc/guides/rel_notes/release_21_11.rst
> +++ b/doc/guides/rel_notes/release_21_11.rst
> @@ -167,7 +167,7 @@ API Changes
> Also, make sure to start the actual text at the margin.
> =======================================================
>
> -* ethdev: Added item ``ETHDEV`` to flow API.
> +* ethdev: Added items ``ETHDEV``, ``ESWITCH_PORT`` to flow API.
>
> * cryptodev: The API rte_cryptodev_pmd_is_valid_dev is modified to
> rte_cryptodev_is_valid_dev as it can be used by the application as diff
> --git
> a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> index 6d5de5457c..9a5c2a2d82 100644
> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> @@ -3824,6 +3824,10 @@ This section lists supported pattern items and
> their attributes, if any.
>
> - ``id {unsigned}``: ethdev ID
>
> +- ``eswitch_port``: match traffic at e-switch going from the external
> +port associated with the given ethdev
> +
> + - ``ethdev_id {unsigned}``: ethdev ID
> +
> Actions list
> ^^^^^^^^^^^^
>
> diff --git a/lib/ethdev/rte_flow.c b/lib/ethdev/rte_flow.c index
> 84eb61cb6c..c4aea5625f 100644
> --- a/lib/ethdev/rte_flow.c
> +++ b/lib/ethdev/rte_flow.c
> @@ -101,6 +101,7 @@ static const struct rte_flow_desc_data
> rte_flow_desc_item[] = {
> MK_FLOW_ITEM(INTEGRITY, sizeof(struct rte_flow_item_integrity)),
> MK_FLOW_ITEM(CONNTRACK, sizeof(uint32_t)),
> MK_FLOW_ITEM(ETHDEV, sizeof(struct rte_flow_item_ethdev)),
> + MK_FLOW_ITEM(ESWITCH_PORT, 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
> 880502098e..1a7e4c2e3d 100644
> --- a/lib/ethdev/rte_flow.h
> +++ b/lib/ethdev/rte_flow.h
> @@ -583,6 +583,16 @@ enum rte_flow_item_type {
> * @see struct rte_flow_item_ethdev
> */
> RTE_FLOW_ITEM_TYPE_ETHDEV,
> +
> + /**
> + * [META]
> + *
> + * Matches traffic at e-switch going from the external port associated
> + * with the given ethdev, for example, traffic from net. port or guest.
> + *
> + * @see struct rte_flow_item_ethdev
> + */
> + RTE_FLOW_ITEM_TYPE_ESWITCH_PORT,
> };
>
> /**
> @@ -1813,7 +1823,7 @@ static const struct rte_flow_item_conntrack
> rte_flow_item_conntrack_mask = {
> * @b EXPERIMENTAL: this structure may change without prior notice
> *
> * Provides an ethdev ID for use with items which are as follows:
> - * RTE_FLOW_ITEM_TYPE_ETHDEV.
> + * RTE_FLOW_ITEM_TYPE_ETHDEV,
> RTE_FLOW_ITEM_TYPE_ESWITCH_PORT.
> */
> struct rte_flow_item_ethdev {
> uint16_t id; /**< Ethdev ID */
> --
> 2.30.2
Best,
Ori
