[dpdk-dev] [RFC PATCH] ethdev: support priority based flow control

[dpdk-dev] [RFC PATCH] ethdev: support priority based flow control

[jerinj]

From: Jerin Jacob <jerinj@marvell.com>

rte_eth_dev_priority_flow_ctrl_set() based API is not generic as it
can not support other than VLAN priority mapping to PFC traffic class.

Introducing RTE_FLOW_ACTION_TYPE_PFC_SET_TC rte_flow action to
set the traffic class as per 802.1Qbb specification. This will enable,
Traffic class(8bit) to be selected based on any packet field like DSCP.

Also, making it as rte_flow action will enable fine control on
traffic class selection to a specific queue or VF etc.

Signed-off-by: Jerin Jacob <jerinj@marvell.com>
---

Planning to submit the testpmd and cnxk ethdev driver changes after receiving
the feedback on this.


 doc/guides/prog_guide/rte_flow.rst | 24 ++++++++++++++++++++++++
 lib/ethdev/rte_flow.c              |  1 +
 lib/ethdev/rte_flow.h              | 27 +++++++++++++++++++++++++++
 3 files changed, 52 insertions(+)

diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
index 2b42d5ec8c..e59f8a2902 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -2999,6 +2999,30 @@ which is set in the packet meta-data (i.e. struct ``rte_mbuf::sched::color``)
    | ``meter_color`` | Packet color |
    +-----------------+--------------+

+Action: ``PFC_SET_TC``
+^^^^^^^^^^^^^^^^^^^^^^
+
+Set traffic class as per PFC (802.1Qbb) specification.
+
+This action must be used with any of the following action.
+
+- ``RTE_FLOW_ACTION_TYPE_QUEUE``
+- ``RTE_FLOW_ACTION_TYPE_RSS``
+- ``RTE_FLOW_ACTION_TYPE_PF``
+- ``RTE_FLOW_ACTION_TYPE_VF``
+- ``RTE_FLOW_ACTION_TYPE_PHY_PORT``
+- ``RTE_FLOW_ACTION_TYPE_PORT_ID``
+
+.. _table_rte_flow_action_pfc_set_tc:
+
+.. table:: PFC_SET_PRIORITY
+
+   +-----------------+-------------------------------------+
+   | Field           | Value                               |
+   +=================+=====================================+
+   | ``tc``          | Traffic class as per PFC (802.1Qbb) |
+   +-----------------+-------------------------------------+
+
 Negative types
 ~~~~~~~~~~~~~~

diff --git a/lib/ethdev/rte_flow.c b/lib/ethdev/rte_flow.c
index 8cb7a069c8..75c661159e 100644
--- a/lib/ethdev/rte_flow.c
+++ b/lib/ethdev/rte_flow.c
@@ -189,6 +189,7 @@ static const struct rte_flow_desc_data rte_flow_desc_action[] = {
 	 */
 	MK_FLOW_ACTION(INDIRECT, 0),
 	MK_FLOW_ACTION(CONNTRACK, sizeof(struct rte_flow_action_conntrack)),
+	MK_FLOW_ACTION(PFC_SET_TC, sizeof(struct rte_flow_action_pfc_set_tc)),
 };

 int
diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
index 7b1ed7f110..5298418e9e 100644
--- a/lib/ethdev/rte_flow.h
+++ b/lib/ethdev/rte_flow.h
@@ -2409,6 +2409,13 @@ enum rte_flow_action_type {
 	 * See struct rte_flow_action_meter_color.
 	 */
 	RTE_FLOW_ACTION_TYPE_METER_COLOR,
+
+	/**
+	 * Set traffic class as per PFC (802.1Qbb) specification.
+	 *
+	 * See struct rte_flow_action_pfc_set_tc.
+	 */
+	RTE_FLOW_ACTION_TYPE_PFC_SET_TC,
 };

 /**
@@ -3168,6 +3175,26 @@ struct rte_flow_action_meter_color {
 	enum rte_color color; /**< Packet color. */
 };

+/**
+ * @warning
+ * @b EXPERIMENTAL: this structure may change without prior notice
+ *
+ * RTE_FLOW_ACTION_TYPE_PFC_SET_TC
+ *
+ * Set traffic class as per PFC (802.1Qbb) specification.
+ *
+ * This action must be used any of the following action.
+ * - RTE_FLOW_ACTION_TYPE_QUEUE,
+ * - RTE_FLOW_ACTION_TYPE_RSS,
+ * - RTE_FLOW_ACTION_TYPE_PF,
+ * - RTE_FLOW_ACTION_TYPE_VF,
+ * - RTE_FLOW_ACTION_TYPE_PHY_PORT,
+ * - RTE_FLOW_ACTION_TYPE_PORT_ID
+ */
+struct rte_flow_action_pfc_set_tc {
+	uint8_t tc; /**< Traffic class as per PFC (802.1Qbb) specification */
+};
+
 /**
  * Field IDs for MODIFY_FIELD action.
  */
--
2.33.0

Re: [dpdk-dev] [RFC PATCH] ethdev: support priority based flow control

[Jerin Jacob]

On Tue, Oct 5, 2021 at 6:32 PM <jerinj@marvell.com> wrote:
>
> From: Jerin Jacob <jerinj@marvell.com>
>
> rte_eth_dev_priority_flow_ctrl_set() based API is not generic as it
> can not support other than VLAN priority mapping to PFC traffic class.
>
> Introducing RTE_FLOW_ACTION_TYPE_PFC_SET_TC rte_flow action to
> set the traffic class as per 802.1Qbb specification. This will enable,
> Traffic class(8bit) to be selected based on any packet field like DSCP.
>
> Also, making it as rte_flow action will enable fine control on
> traffic class selection to a specific queue or VF etc.
>
> Signed-off-by: Jerin Jacob <jerinj@marvell.com>

Ping. If there are no comments on RFC, Planning to send v1 for 22.02.

> ---
>
> Planning to submit the testpmd and cnxk ethdev driver changes after receiving
> the feedback on this.
>
>
>  doc/guides/prog_guide/rte_flow.rst | 24 ++++++++++++++++++++++++
>  lib/ethdev/rte_flow.c              |  1 +
>  lib/ethdev/rte_flow.h              | 27 +++++++++++++++++++++++++++
>  3 files changed, 52 insertions(+)
>
> diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
> index 2b42d5ec8c..e59f8a2902 100644
> --- a/doc/guides/prog_guide/rte_flow.rst
> +++ b/doc/guides/prog_guide/rte_flow.rst
> @@ -2999,6 +2999,30 @@ which is set in the packet meta-data (i.e. struct ``rte_mbuf::sched::color``)
>     | ``meter_color`` | Packet color |
>     +-----------------+--------------+
>
> +Action: ``PFC_SET_TC``
> +^^^^^^^^^^^^^^^^^^^^^^
> +
> +Set traffic class as per PFC (802.1Qbb) specification.
> +
> +This action must be used with any of the following action.
> +
> +- ``RTE_FLOW_ACTION_TYPE_QUEUE``
> +- ``RTE_FLOW_ACTION_TYPE_RSS``
> +- ``RTE_FLOW_ACTION_TYPE_PF``
> +- ``RTE_FLOW_ACTION_TYPE_VF``
> +- ``RTE_FLOW_ACTION_TYPE_PHY_PORT``
> +- ``RTE_FLOW_ACTION_TYPE_PORT_ID``
> +
> +.. _table_rte_flow_action_pfc_set_tc:
> +
> +.. table:: PFC_SET_PRIORITY
> +
> +   +-----------------+-------------------------------------+
> +   | Field           | Value                               |
> +   +=================+=====================================+
> +   | ``tc``          | Traffic class as per PFC (802.1Qbb) |
> +   +-----------------+-------------------------------------+
> +
>  Negative types
>  ~~~~~~~~~~~~~~
>
> diff --git a/lib/ethdev/rte_flow.c b/lib/ethdev/rte_flow.c
> index 8cb7a069c8..75c661159e 100644
> --- a/lib/ethdev/rte_flow.c
> +++ b/lib/ethdev/rte_flow.c
> @@ -189,6 +189,7 @@ static const struct rte_flow_desc_data rte_flow_desc_action[] = {
>          */
>         MK_FLOW_ACTION(INDIRECT, 0),
>         MK_FLOW_ACTION(CONNTRACK, sizeof(struct rte_flow_action_conntrack)),
> +       MK_FLOW_ACTION(PFC_SET_TC, sizeof(struct rte_flow_action_pfc_set_tc)),
>  };
>
>  int
> diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
> index 7b1ed7f110..5298418e9e 100644
> --- a/lib/ethdev/rte_flow.h
> +++ b/lib/ethdev/rte_flow.h
> @@ -2409,6 +2409,13 @@ enum rte_flow_action_type {
>          * See struct rte_flow_action_meter_color.
>          */
>         RTE_FLOW_ACTION_TYPE_METER_COLOR,
> +
> +       /**
> +        * Set traffic class as per PFC (802.1Qbb) specification.
> +        *
> +        * See struct rte_flow_action_pfc_set_tc.
> +        */
> +       RTE_FLOW_ACTION_TYPE_PFC_SET_TC,
>  };
>
>  /**
> @@ -3168,6 +3175,26 @@ struct rte_flow_action_meter_color {
>         enum rte_color color; /**< Packet color. */
>  };
>
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this structure may change without prior notice
> + *
> + * RTE_FLOW_ACTION_TYPE_PFC_SET_TC
> + *
> + * Set traffic class as per PFC (802.1Qbb) specification.
> + *
> + * This action must be used any of the following action.
> + * - RTE_FLOW_ACTION_TYPE_QUEUE,
> + * - RTE_FLOW_ACTION_TYPE_RSS,
> + * - RTE_FLOW_ACTION_TYPE_PF,
> + * - RTE_FLOW_ACTION_TYPE_VF,
> + * - RTE_FLOW_ACTION_TYPE_PHY_PORT,
> + * - RTE_FLOW_ACTION_TYPE_PORT_ID
> + */
> +struct rte_flow_action_pfc_set_tc {
> +       uint8_t tc; /**< Traffic class as per PFC (802.1Qbb) specification */
> +};
> +
>  /**
>   * Field IDs for MODIFY_FIELD action.
>   */
> --
> 2.33.0
>

RE: [dpdk-dev] [RFC PATCH] ethdev: support priority based flow control

[Ori Kam]

Hi Jerin,

Sorry for my late response,

> -----Original Message-----
> From: Jerin Jacob <jerinjacobk@gmail.com>
> Sent: Wednesday, November 17, 2021 11:49 AM
> To: Jerin Jacob <jerinj@marvell.com>
> Subject: Re: [dpdk-dev] [RFC PATCH] ethdev: support priority based flow control
> 
> On Tue, Oct 5, 2021 at 6:32 PM <jerinj@marvell.com> wrote:
> >
> > From: Jerin Jacob <jerinj@marvell.com>
> >
> > rte_eth_dev_priority_flow_ctrl_set() based API is not generic as it
> > can not support other than VLAN priority mapping to PFC traffic class.
> >
> > Introducing RTE_FLOW_ACTION_TYPE_PFC_SET_TC rte_flow action to
> > set the traffic class as per 802.1Qbb specification. This will enable,
> > Traffic class(8bit) to be selected based on any packet field like DSCP.
> >
> > Also, making it as rte_flow action will enable fine control on
> > traffic class selection to a specific queue or VF etc.
> >
> > Signed-off-by: Jerin Jacob <jerinj@marvell.com>
> 
> Ping. If there are no comments on RFC, Planning to send v1 for 22.02.

All the set type of functions are going to be deprecated.
you should use RTE_FLOW_ACTION_TYPE_MODIFY_FIELD.

What is the item that you are matching on when using rte_flow? Is it part of the tci in the vlan item?


> 
> > ---
> >
> > Planning to submit the testpmd and cnxk ethdev driver changes after receiving
> > the feedback on this.
> >
> >
> >  doc/guides/prog_guide/rte_flow.rst | 24 ++++++++++++++++++++++++
> >  lib/ethdev/rte_flow.c              |  1 +
> >  lib/ethdev/rte_flow.h              | 27 +++++++++++++++++++++++++++
> >  3 files changed, 52 insertions(+)
> >
> > diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
> > index 2b42d5ec8c..e59f8a2902 100644
> > --- a/doc/guides/prog_guide/rte_flow.rst
> > +++ b/doc/guides/prog_guide/rte_flow.rst
> > @@ -2999,6 +2999,30 @@ which is set in the packet meta-data (i.e. struct ``rte_mbuf::sched::color``)
> >     | ``meter_color`` | Packet color |
> >     +-----------------+--------------+
> >
> > +Action: ``PFC_SET_TC``
> > +^^^^^^^^^^^^^^^^^^^^^^
> > +
> > +Set traffic class as per PFC (802.1Qbb) specification.
> > +
> > +This action must be used with any of the following action.
> > +
> > +- ``RTE_FLOW_ACTION_TYPE_QUEUE``
> > +- ``RTE_FLOW_ACTION_TYPE_RSS``
> > +- ``RTE_FLOW_ACTION_TYPE_PF``
> > +- ``RTE_FLOW_ACTION_TYPE_VF``
> > +- ``RTE_FLOW_ACTION_TYPE_PHY_PORT``
> > +- ``RTE_FLOW_ACTION_TYPE_PORT_ID``
> > +

Why? All the above actions are terminating actions so if I want ot match on the value
it doesn't make sense to have it only on the last rule.

> > +.. _table_rte_flow_action_pfc_set_tc:
> > +
> > +.. table:: PFC_SET_PRIORITY
> > +
> > +   +-----------------+-------------------------------------+
> > +   | Field           | Value                               |
> > +   +=================+=====================================+
> > +   | ``tc``          | Traffic class as per PFC (802.1Qbb) |
> > +   +-----------------+-------------------------------------+
> > +
> >  Negative types
> >  ~~~~~~~~~~~~~~
> >
> > diff --git a/lib/ethdev/rte_flow.c b/lib/ethdev/rte_flow.c
> > index 8cb7a069c8..75c661159e 100644
> > --- a/lib/ethdev/rte_flow.c
> > +++ b/lib/ethdev/rte_flow.c
> > @@ -189,6 +189,7 @@ static const struct rte_flow_desc_data rte_flow_desc_action[] = {
> >          */
> >         MK_FLOW_ACTION(INDIRECT, 0),
> >         MK_FLOW_ACTION(CONNTRACK, sizeof(struct rte_flow_action_conntrack)),
> > +       MK_FLOW_ACTION(PFC_SET_TC, sizeof(struct rte_flow_action_pfc_set_tc)),
> >  };
> >
> >  int
> > diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
> > index 7b1ed7f110..5298418e9e 100644
> > --- a/lib/ethdev/rte_flow.h
> > +++ b/lib/ethdev/rte_flow.h
> > @@ -2409,6 +2409,13 @@ enum rte_flow_action_type {
> >          * See struct rte_flow_action_meter_color.
> >          */
> >         RTE_FLOW_ACTION_TYPE_METER_COLOR,
> > +
> > +       /**
> > +        * Set traffic class as per PFC (802.1Qbb) specification.
> > +        *
> > +        * See struct rte_flow_action_pfc_set_tc.
> > +        */
> > +       RTE_FLOW_ACTION_TYPE_PFC_SET_TC,
> >  };
> >
> >  /**
> > @@ -3168,6 +3175,26 @@ struct rte_flow_action_meter_color {
> >         enum rte_color color; /**< Packet color. */
> >  };
> >
> > +/**
> > + * @warning
> > + * @b EXPERIMENTAL: this structure may change without prior notice
> > + *
> > + * RTE_FLOW_ACTION_TYPE_PFC_SET_TC
> > + *
> > + * Set traffic class as per PFC (802.1Qbb) specification.
> > + *
> > + * This action must be used any of the following action.
> > + * - RTE_FLOW_ACTION_TYPE_QUEUE,
> > + * - RTE_FLOW_ACTION_TYPE_RSS,
> > + * - RTE_FLOW_ACTION_TYPE_PF,
> > + * - RTE_FLOW_ACTION_TYPE_VF,
> > + * - RTE_FLOW_ACTION_TYPE_PHY_PORT,
> > + * - RTE_FLOW_ACTION_TYPE_PORT_ID
> > + */

What does it mean?  I must use it only on rules that have one of the above actions?

> > +struct rte_flow_action_pfc_set_tc {
> > +       uint8_t tc; /**< Traffic class as per PFC (802.1Qbb) specification */
> > +};
> > +
> >  /**
> >   * Field IDs for MODIFY_FIELD action.
> >   */
> > --
> > 2.33.0
> >

Best,
Ori

Re: [dpdk-dev] [RFC PATCH] ethdev: support priority based flow control

[Jerin Jacob]

On Sun, Nov 21, 2021 at 3:20 PM Ori Kam <orika@nvidia.com> wrote:
>
> Hi Jerin,

Hi Ori,

>
> Sorry for my late response,

Thanks for the review.

>
> > -----Original Message-----
> > From: Jerin Jacob <jerinjacobk@gmail.com>
> > Sent: Wednesday, November 17, 2021 11:49 AM
> > To: Jerin Jacob <jerinj@marvell.com>
> > Subject: Re: [dpdk-dev] [RFC PATCH] ethdev: support priority based flow control
> >
> > On Tue, Oct 5, 2021 at 6:32 PM <jerinj@marvell.com> wrote:
> > >
> > > From: Jerin Jacob <jerinj@marvell.com>
> > >
> > > rte_eth_dev_priority_flow_ctrl_set() based API is not generic as it
> > > can not support other than VLAN priority mapping to PFC traffic class.
> > >
> > > Introducing RTE_FLOW_ACTION_TYPE_PFC_SET_TC rte_flow action to
> > > set the traffic class as per 802.1Qbb specification. This will enable,
> > > Traffic class(8bit) to be selected based on any packet field like DSCP.
> > >
> > > Also, making it as rte_flow action will enable fine control on
> > > traffic class selection to a specific queue or VF etc.
> > >
> > > Signed-off-by: Jerin Jacob <jerinj@marvell.com>
> >
> > Ping. If there are no comments on RFC, Planning to send v1 for 22.02.
>
> All the set type of functions are going to be deprecated.
> you should use RTE_FLOW_ACTION_TYPE_MODIFY_FIELD.
>
> What is the item that you are matching on when using rte_flow? Is it part of the tci in the vlan item?

TC can be VLAN TCI field or DSCP field in IP header or any other field
in packet.
We need to set the traffic class as per 802.1Qbb specification, May I
know how the "modify"
attribute helps here. It should be a "set" operation. Right?



>
>
> >
> > > ---
> > >
> > > Planning to submit the testpmd and cnxk ethdev driver changes after receiving
> > > the feedback on this.
> > >
> > >
> > >  doc/guides/prog_guide/rte_flow.rst | 24 ++++++++++++++++++++++++
> > >  lib/ethdev/rte_flow.c              |  1 +
> > >  lib/ethdev/rte_flow.h              | 27 +++++++++++++++++++++++++++
> > >  3 files changed, 52 insertions(+)
> > >
> > > diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
> > > index 2b42d5ec8c..e59f8a2902 100644
> > > --- a/doc/guides/prog_guide/rte_flow.rst
> > > +++ b/doc/guides/prog_guide/rte_flow.rst
> > > @@ -2999,6 +2999,30 @@ which is set in the packet meta-data (i.e. struct ``rte_mbuf::sched::color``)
> > >     | ``meter_color`` | Packet color |
> > >     +-----------------+--------------+
> > >
> > > +Action: ``PFC_SET_TC``
> > > +^^^^^^^^^^^^^^^^^^^^^^
> > > +
> > > +Set traffic class as per PFC (802.1Qbb) specification.
> > > +
> > > +This action must be used with any of the following action.
> > > +
> > > +- ``RTE_FLOW_ACTION_TYPE_QUEUE``
> > > +- ``RTE_FLOW_ACTION_TYPE_RSS``
> > > +- ``RTE_FLOW_ACTION_TYPE_PF``
> > > +- ``RTE_FLOW_ACTION_TYPE_VF``
> > > +- ``RTE_FLOW_ACTION_TYPE_PHY_PORT``
> > > +- ``RTE_FLOW_ACTION_TYPE_PORT_ID``
> > > +
>
> Why? All the above actions are terminating actions so if I want ot match on the value
> it doesn't make sense to have it only on the last rule.

In PFC, we are specifying, Given TC needs to steer to specific Queue,
RSS, PF etc.
Not sure how other actions are relevant for SET_TC action. Do you have any
specific action in mind where SET_TC valid in addition to above actions



>
> > > +.. _table_rte_flow_action_pfc_set_tc:
> > > +
> > > +.. table:: PFC_SET_PRIORITY
> > > +
> > > +   +-----------------+-------------------------------------+
> > > +   | Field           | Value                               |
> > > +   +=================+=====================================+
> > > +   | ``tc``          | Traffic class as per PFC (802.1Qbb) |
> > > +   +-----------------+-------------------------------------+
> > > +
> > >  Negative types
> > >  ~~~~~~~~~~~~~~
> > >
> > > diff --git a/lib/ethdev/rte_flow.c b/lib/ethdev/rte_flow.c
> > > index 8cb7a069c8..75c661159e 100644
> > > --- a/lib/ethdev/rte_flow.c
> > > +++ b/lib/ethdev/rte_flow.c
> > > @@ -189,6 +189,7 @@ static const struct rte_flow_desc_data rte_flow_desc_action[] = {
> > >          */
> > >         MK_FLOW_ACTION(INDIRECT, 0),
> > >         MK_FLOW_ACTION(CONNTRACK, sizeof(struct rte_flow_action_conntrack)),
> > > +       MK_FLOW_ACTION(PFC_SET_TC, sizeof(struct rte_flow_action_pfc_set_tc)),
> > >  };
> > >
> > >  int
> > > diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
> > > index 7b1ed7f110..5298418e9e 100644
> > > --- a/lib/ethdev/rte_flow.h
> > > +++ b/lib/ethdev/rte_flow.h
> > > @@ -2409,6 +2409,13 @@ enum rte_flow_action_type {
> > >          * See struct rte_flow_action_meter_color.
> > >          */
> > >         RTE_FLOW_ACTION_TYPE_METER_COLOR,
> > > +
> > > +       /**
> > > +        * Set traffic class as per PFC (802.1Qbb) specification.
> > > +        *
> > > +        * See struct rte_flow_action_pfc_set_tc.
> > > +        */
> > > +       RTE_FLOW_ACTION_TYPE_PFC_SET_TC,
> > >  };
> > >
> > >  /**
> > > @@ -3168,6 +3175,26 @@ struct rte_flow_action_meter_color {
> > >         enum rte_color color; /**< Packet color. */
> > >  };
> > >
> > > +/**
> > > + * @warning
> > > + * @b EXPERIMENTAL: this structure may change without prior notice
> > > + *
> > > + * RTE_FLOW_ACTION_TYPE_PFC_SET_TC
> > > + *
> > > + * Set traffic class as per PFC (802.1Qbb) specification.
> > > + *
> > > + * This action must be used any of the following action.
> > > + * - RTE_FLOW_ACTION_TYPE_QUEUE,
> > > + * - RTE_FLOW_ACTION_TYPE_RSS,
> > > + * - RTE_FLOW_ACTION_TYPE_PF,
> > > + * - RTE_FLOW_ACTION_TYPE_VF,
> > > + * - RTE_FLOW_ACTION_TYPE_PHY_PORT,
> > > + * - RTE_FLOW_ACTION_TYPE_PORT_ID
> > > + */
>
> What does it mean?  I must use it only on rules that have one of the above actions?

See above.

>
> > > +struct rte_flow_action_pfc_set_tc {
> > > +       uint8_t tc; /**< Traffic class as per PFC (802.1Qbb) specification */
> > > +};
> > > +
> > >  /**
> > >   * Field IDs for MODIFY_FIELD action.
> > >   */
> > > --
> > > 2.33.0
> > >
>
> Best,
> Ori

Re: [dpdk-dev] [RFC PATCH] ethdev: support priority based flow control

[Ajit Khaparde]

On Tue, Nov 23, 2021 at 2:58 AM Jerin Jacob <jerinjacobk@gmail.com> wrote:
>
> On Sun, Nov 21, 2021 at 3:20 PM Ori Kam <orika@nvidia.com> wrote:
> >
> > Hi Jerin,
>
> Hi Ori,
>
> >
> > Sorry for my late response,
>
> Thanks for the review.
>
> >
> > > -----Original Message-----
> > > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > Sent: Wednesday, November 17, 2021 11:49 AM
> > > To: Jerin Jacob <jerinj@marvell.com>
> > > Subject: Re: [dpdk-dev] [RFC PATCH] ethdev: support priority based flow control
> > >
> > > On Tue, Oct 5, 2021 at 6:32 PM <jerinj@marvell.com> wrote:
> > > >
> > > > From: Jerin Jacob <jerinj@marvell.com>
> > > >
> > > > rte_eth_dev_priority_flow_ctrl_set() based API is not generic as it
> > > > can not support other than VLAN priority mapping to PFC traffic class.
> > > >
> > > > Introducing RTE_FLOW_ACTION_TYPE_PFC_SET_TC rte_flow action to
> > > > set the traffic class as per 802.1Qbb specification. This will enable,
> > > > Traffic class(8bit) to be selected based on any packet field like DSCP.
> > > >
> > > > Also, making it as rte_flow action will enable fine control on
> > > > traffic class selection to a specific queue or VF etc.
> > > >
> > > > Signed-off-by: Jerin Jacob <jerinj@marvell.com>
> > >
> > > Ping. If there are no comments on RFC, Planning to send v1 for 22.02.
> >
> > All the set type of functions are going to be deprecated.
> > you should use RTE_FLOW_ACTION_TYPE_MODIFY_FIELD.
> >
> > What is the item that you are matching on when using rte_flow? Is it part of the tci in the vlan item?
>
> TC can be VLAN TCI field or DSCP field in IP header or any other field
> in packet.
> We need to set the traffic class as per 802.1Qbb specification, May I
> know how the "modify"
> attribute helps here. It should be a "set" operation. Right?
I think "set" is appropriate.

RE: [dpdk-dev] [RFC PATCH] ethdev: support priority based flow control

[Ori Kam]

Hi Jerin,

> -----Original Message-----
> From: Jerin Jacob <jerinjacobk@gmail.com>
> Sent: Tuesday, November 23, 2021 12:58 PM
> To: Ori Kam <orika@nvidia.com>
> 
> On Sun, Nov 21, 2021 at 3:20 PM Ori Kam <orika@nvidia.com> wrote:
> >
> > Hi Jerin,
> 
> Hi Ori,
> 
> >
> > Sorry for my late response,
> 
> Thanks for the review.
> 
> >
> > > -----Original Message-----
> > > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > Sent: Wednesday, November 17, 2021 11:49 AM
> > > To: Jerin Jacob <jerinj@marvell.com>
> > > Subject: Re: [dpdk-dev] [RFC PATCH] ethdev: support priority based flow control
> > >
> > > On Tue, Oct 5, 2021 at 6:32 PM <jerinj@marvell.com> wrote:
> > > >
> > > > From: Jerin Jacob <jerinj@marvell.com>
> > > >
> > > > rte_eth_dev_priority_flow_ctrl_set() based API is not generic as it
> > > > can not support other than VLAN priority mapping to PFC traffic class.
> > > >
> > > > Introducing RTE_FLOW_ACTION_TYPE_PFC_SET_TC rte_flow action to
> > > > set the traffic class as per 802.1Qbb specification. This will enable,
> > > > Traffic class(8bit) to be selected based on any packet field like DSCP.
> > > >
> > > > Also, making it as rte_flow action will enable fine control on
> > > > traffic class selection to a specific queue or VF etc.
> > > >
> > > > Signed-off-by: Jerin Jacob <jerinj@marvell.com>
> > >
> > > Ping. If there are no comments on RFC, Planning to send v1 for 22.02.
> >
> > All the set type of functions are going to be deprecated.
> > you should use RTE_FLOW_ACTION_TYPE_MODIFY_FIELD.
> >
> > What is the item that you are matching on when using rte_flow? Is it part of the tci in the vlan item?
> 
> TC can be VLAN TCI field or DSCP field in IP header or any other field
> in packet.
> We need to set the traffic class as per 802.1Qbb specification, May I
> know how the "modify"
> attribute helps here. It should be a "set" operation. Right?
> 

Yes, in the rte_flow_action_modify_field there is what operation you want to do,
in this case the action should be set.

I assume that you are trying to set the VLAN tag priority field right?

> 
> 
> >
> >
> > >
> > > > ---
> > > >
> > > > Planning to submit the testpmd and cnxk ethdev driver changes after receiving
> > > > the feedback on this.
> > > >
> > > >
> > > >  doc/guides/prog_guide/rte_flow.rst | 24 ++++++++++++++++++++++++
> > > >  lib/ethdev/rte_flow.c              |  1 +
> > > >  lib/ethdev/rte_flow.h              | 27 +++++++++++++++++++++++++++
> > > >  3 files changed, 52 insertions(+)
> > > >
> > > > diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
> > > > index 2b42d5ec8c..e59f8a2902 100644
> > > > --- a/doc/guides/prog_guide/rte_flow.rst
> > > > +++ b/doc/guides/prog_guide/rte_flow.rst
> > > > @@ -2999,6 +2999,30 @@ which is set in the packet meta-data (i.e. struct
> ``rte_mbuf::sched::color``)
> > > >     | ``meter_color`` | Packet color |
> > > >     +-----------------+--------------+
> > > >
> > > > +Action: ``PFC_SET_TC``
> > > > +^^^^^^^^^^^^^^^^^^^^^^
> > > > +
> > > > +Set traffic class as per PFC (802.1Qbb) specification.
> > > > +
> > > > +This action must be used with any of the following action.
> > > > +
> > > > +- ``RTE_FLOW_ACTION_TYPE_QUEUE``
> > > > +- ``RTE_FLOW_ACTION_TYPE_RSS``
> > > > +- ``RTE_FLOW_ACTION_TYPE_PF``
> > > > +- ``RTE_FLOW_ACTION_TYPE_VF``
> > > > +- ``RTE_FLOW_ACTION_TYPE_PHY_PORT``
> > > > +- ``RTE_FLOW_ACTION_TYPE_PORT_ID``
> > > > +
> >
> > Why? All the above actions are terminating actions so if I want ot match on the value
> > it doesn't make sense to have it only on the last rule.
> 
> In PFC, we are specifying, Given TC needs to steer to specific Queue,
> RSS, PF etc.
> Not sure how other actions are relevant for SET_TC action. Do you have any
> specific action in mind where SET_TC valid in addition to above actions
> 
First what happens in case of egress traffic? There is no dest action.
Second what happens if for example the priority is based on the outer tunnel
which I want decap and at a latter stage I want to do connection tracking and only
if everything is correct I want to send this packet to a queue?

> 
> 
> >
> > > > +.. _table_rte_flow_action_pfc_set_tc:
> > > > +
> > > > +.. table:: PFC_SET_PRIORITY
> > > > +
> > > > +   +-----------------+-------------------------------------+
> > > > +   | Field           | Value                               |
> > > > +   +=================+=====================================+
> > > > +   | ``tc``          | Traffic class as per PFC (802.1Qbb) |
> > > > +   +-----------------+-------------------------------------+
> > > > +
> > > >  Negative types
> > > >  ~~~~~~~~~~~~~~
> > > >
> > > > diff --git a/lib/ethdev/rte_flow.c b/lib/ethdev/rte_flow.c
> > > > index 8cb7a069c8..75c661159e 100644
> > > > --- a/lib/ethdev/rte_flow.c
> > > > +++ b/lib/ethdev/rte_flow.c
> > > > @@ -189,6 +189,7 @@ static const struct rte_flow_desc_data rte_flow_desc_action[] = {
> > > >          */
> > > >         MK_FLOW_ACTION(INDIRECT, 0),
> > > >         MK_FLOW_ACTION(CONNTRACK, sizeof(struct rte_flow_action_conntrack)),
> > > > +       MK_FLOW_ACTION(PFC_SET_TC, sizeof(struct rte_flow_action_pfc_set_tc)),
> > > >  };
> > > >
> > > >  int
> > > > diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
> > > > index 7b1ed7f110..5298418e9e 100644
> > > > --- a/lib/ethdev/rte_flow.h
> > > > +++ b/lib/ethdev/rte_flow.h
> > > > @@ -2409,6 +2409,13 @@ enum rte_flow_action_type {
> > > >          * See struct rte_flow_action_meter_color.
> > > >          */
> > > >         RTE_FLOW_ACTION_TYPE_METER_COLOR,
> > > > +
> > > > +       /**
> > > > +        * Set traffic class as per PFC (802.1Qbb) specification.
> > > > +        *
> > > > +        * See struct rte_flow_action_pfc_set_tc.
> > > > +        */
> > > > +       RTE_FLOW_ACTION_TYPE_PFC_SET_TC,
> > > >  };
> > > >
> > > >  /**
> > > > @@ -3168,6 +3175,26 @@ struct rte_flow_action_meter_color {
> > > >         enum rte_color color; /**< Packet color. */
> > > >  };
> > > >
> > > > +/**
> > > > + * @warning
> > > > + * @b EXPERIMENTAL: this structure may change without prior notice
> > > > + *
> > > > + * RTE_FLOW_ACTION_TYPE_PFC_SET_TC
> > > > + *
> > > > + * Set traffic class as per PFC (802.1Qbb) specification.
> > > > + *
> > > > + * This action must be used any of the following action.
> > > > + * - RTE_FLOW_ACTION_TYPE_QUEUE,
> > > > + * - RTE_FLOW_ACTION_TYPE_RSS,
> > > > + * - RTE_FLOW_ACTION_TYPE_PF,
> > > > + * - RTE_FLOW_ACTION_TYPE_VF,
> > > > + * - RTE_FLOW_ACTION_TYPE_PHY_PORT,
> > > > + * - RTE_FLOW_ACTION_TYPE_PORT_ID
> > > > + */
> >
> > What does it mean?  I must use it only on rules that have one of the above actions?
> 
> See above.
> 
> >
> > > > +struct rte_flow_action_pfc_set_tc {
> > > > +       uint8_t tc; /**< Traffic class as per PFC (802.1Qbb) specification */
> > > > +};
> > > > +
> > > >  /**
> > > >   * Field IDs for MODIFY_FIELD action.
> > > >   */
> > > > --
> > > > 2.33.0
> > > >
> >
> > Best,
> > Ori

Re: [dpdk-dev] [RFC PATCH] ethdev: support priority based flow control

[Jerin Jacob]

On Wed, Nov 24, 2021 at 3:01 PM Ori Kam <orika@nvidia.com> wrote:
>
> Hi Jerin,
>
> > -----Original Message-----
> > From: Jerin Jacob <jerinjacobk@gmail.com>
> > Sent: Tuesday, November 23, 2021 12:58 PM
> > To: Ori Kam <orika@nvidia.com>
> >
> > On Sun, Nov 21, 2021 at 3:20 PM Ori Kam <orika@nvidia.com> wrote:
> > >
> > > Hi Jerin,
> >
> > Hi Ori,
> >
> > >
> > > Sorry for my late response,
> >
> > Thanks for the review.
> >
> > >
> > > > -----Original Message-----
> > > > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > > Sent: Wednesday, November 17, 2021 11:49 AM
> > > > To: Jerin Jacob <jerinj@marvell.com>
> > > > Subject: Re: [dpdk-dev] [RFC PATCH] ethdev: support priority based flow control
> > > >
> > > > On Tue, Oct 5, 2021 at 6:32 PM <jerinj@marvell.com> wrote:
> > > > >
> > > > > From: Jerin Jacob <jerinj@marvell.com>
> > > > >
> > > > > rte_eth_dev_priority_flow_ctrl_set() based API is not generic as it
> > > > > can not support other than VLAN priority mapping to PFC traffic class.
> > > > >
> > > > > Introducing RTE_FLOW_ACTION_TYPE_PFC_SET_TC rte_flow action to
> > > > > set the traffic class as per 802.1Qbb specification. This will enable,
> > > > > Traffic class(8bit) to be selected based on any packet field like DSCP.
> > > > >
> > > > > Also, making it as rte_flow action will enable fine control on
> > > > > traffic class selection to a specific queue or VF etc.
> > > > >
> > > > > Signed-off-by: Jerin Jacob <jerinj@marvell.com>
> > > >
> > > > Ping. If there are no comments on RFC, Planning to send v1 for 22.02.
> > >
> > > All the set type of functions are going to be deprecated.
> > > you should use RTE_FLOW_ACTION_TYPE_MODIFY_FIELD.
> > >
> > > What is the item that you are matching on when using rte_flow? Is it part of the tci in the vlan item?
> >
> > TC can be VLAN TCI field or DSCP field in IP header or any other field
> > in packet.
> > We need to set the traffic class as per 802.1Qbb specification, May I
> > know how the "modify"
> > attribute helps here. It should be a "set" operation. Right?
> >
>
> Yes, in the rte_flow_action_modify_field there is what operation you want to do,
> in this case the action should be set.

But RTE_FLOW_ACTION_TYPE_MODIFY_FIELD used for modify the packet
content[1]. RIght?
In this case, it is more of sideband data not anything on packet
content. If so, explicit action
makes sense. Right?


[1]
 * RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
 *
 * Modify a destination header field according to the specified
 * operation. Another field of the packet can be used as a source as well
 * as tag, mark, metadata, immediate value or a pointer to it.

>
> I assume that you are trying to set the VLAN tag priority field right?

Both VLAN tag and/or DSCP field.


>
> >
> >
> > >
> > >
> > > >
> > > > > ---
> > > > >
> > > > > Planning to submit the testpmd and cnxk ethdev driver changes after receiving
> > > > > the feedback on this.
> > > > >
> > > > >
> > > > >  doc/guides/prog_guide/rte_flow.rst | 24 ++++++++++++++++++++++++
> > > > >  lib/ethdev/rte_flow.c              |  1 +
> > > > >  lib/ethdev/rte_flow.h              | 27 +++++++++++++++++++++++++++
> > > > >  3 files changed, 52 insertions(+)
> > > > >
> > > > > diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
> > > > > index 2b42d5ec8c..e59f8a2902 100644
> > > > > --- a/doc/guides/prog_guide/rte_flow.rst
> > > > > +++ b/doc/guides/prog_guide/rte_flow.rst
> > > > > @@ -2999,6 +2999,30 @@ which is set in the packet meta-data (i.e. struct
> > ``rte_mbuf::sched::color``)
> > > > >     | ``meter_color`` | Packet color |
> > > > >     +-----------------+--------------+
> > > > >
> > > > > +Action: ``PFC_SET_TC``
> > > > > +^^^^^^^^^^^^^^^^^^^^^^
> > > > > +
> > > > > +Set traffic class as per PFC (802.1Qbb) specification.
> > > > > +
> > > > > +This action must be used with any of the following action.
> > > > > +
> > > > > +- ``RTE_FLOW_ACTION_TYPE_QUEUE``
> > > > > +- ``RTE_FLOW_ACTION_TYPE_RSS``
> > > > > +- ``RTE_FLOW_ACTION_TYPE_PF``
> > > > > +- ``RTE_FLOW_ACTION_TYPE_VF``
> > > > > +- ``RTE_FLOW_ACTION_TYPE_PHY_PORT``
> > > > > +- ``RTE_FLOW_ACTION_TYPE_PORT_ID``
> > > > > +
> > >
> > > Why? All the above actions are terminating actions so if I want ot match on the value
> > > it doesn't make sense to have it only on the last rule.
> >
> > In PFC, we are specifying, Given TC needs to steer to specific Queue,
> > RSS, PF etc.
> > Not sure how other actions are relevant for SET_TC action. Do you have any
> > specific action in mind where SET_TC valid in addition to above actions
> >
> First what happens in case of egress traffic? There is no dest action.

It will be invalid. I can change the documentation to specify egress
direction is not valid.
Thoughts?

> Second what happens if for example the priority is based on the outer tunnel
> which I want decap and at a latter stage I want to do connection tracking and only
> if everything is correct I want to send this packet to a queue?

Which is fine with the current scheme of things as per the documentation,
"This action must be used with any of the following action." it does
not preclude to
use of any other action. If it is not clear, we can reword like below,
---
This action must be used with any of the following action and not limited to
using any of other actions in conjunction with the following action.
---
Thoughts?

>
> >
> >
> > >
> > > > > +.. _table_rte_flow_action_pfc_set_tc:
> > > > > +
> > > > > +.. table:: PFC_SET_PRIORITY
> > > > > +
> > > > > +   +-----------------+-------------------------------------+
> > > > > +   | Field           | Value                               |
> > > > > +   +=================+=====================================+
> > > > > +   | ``tc``          | Traffic class as per PFC (802.1Qbb) |
> > > > > +   +-----------------+-------------------------------------+
> > > > > +
> > > > >  Negative types
> > > > >  ~~~~~~~~~~~~~~
> > > > >
> > > > > diff --git a/lib/ethdev/rte_flow.c b/lib/ethdev/rte_flow.c
> > > > > index 8cb7a069c8..75c661159e 100644
> > > > > --- a/lib/ethdev/rte_flow.c
> > > > > +++ b/lib/ethdev/rte_flow.c
> > > > > @@ -189,6 +189,7 @@ static const struct rte_flow_desc_data rte_flow_desc_action[] = {
> > > > >          */
> > > > >         MK_FLOW_ACTION(INDIRECT, 0),
> > > > >         MK_FLOW_ACTION(CONNTRACK, sizeof(struct rte_flow_action_conntrack)),
> > > > > +       MK_FLOW_ACTION(PFC_SET_TC, sizeof(struct rte_flow_action_pfc_set_tc)),
> > > > >  };
> > > > >
> > > > >  int
> > > > > diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
> > > > > index 7b1ed7f110..5298418e9e 100644
> > > > > --- a/lib/ethdev/rte_flow.h
> > > > > +++ b/lib/ethdev/rte_flow.h
> > > > > @@ -2409,6 +2409,13 @@ enum rte_flow_action_type {
> > > > >          * See struct rte_flow_action_meter_color.
> > > > >          */
> > > > >         RTE_FLOW_ACTION_TYPE_METER_COLOR,
> > > > > +
> > > > > +       /**
> > > > > +        * Set traffic class as per PFC (802.1Qbb) specification.
> > > > > +        *
> > > > > +        * See struct rte_flow_action_pfc_set_tc.
> > > > > +        */
> > > > > +       RTE_FLOW_ACTION_TYPE_PFC_SET_TC,
> > > > >  };
> > > > >
> > > > >  /**
> > > > > @@ -3168,6 +3175,26 @@ struct rte_flow_action_meter_color {
> > > > >         enum rte_color color; /**< Packet color. */
> > > > >  };
> > > > >
> > > > > +/**
> > > > > + * @warning
> > > > > + * @b EXPERIMENTAL: this structure may change without prior notice
> > > > > + *
> > > > > + * RTE_FLOW_ACTION_TYPE_PFC_SET_TC
> > > > > + *
> > > > > + * Set traffic class as per PFC (802.1Qbb) specification.
> > > > > + *
> > > > > + * This action must be used any of the following action.
> > > > > + * - RTE_FLOW_ACTION_TYPE_QUEUE,
> > > > > + * - RTE_FLOW_ACTION_TYPE_RSS,
> > > > > + * - RTE_FLOW_ACTION_TYPE_PF,
> > > > > + * - RTE_FLOW_ACTION_TYPE_VF,
> > > > > + * - RTE_FLOW_ACTION_TYPE_PHY_PORT,
> > > > > + * - RTE_FLOW_ACTION_TYPE_PORT_ID
> > > > > + */
> > >
> > > What does it mean?  I must use it only on rules that have one of the above actions?
> >
> > See above.
> >
> > >
> > > > > +struct rte_flow_action_pfc_set_tc {
> > > > > +       uint8_t tc; /**< Traffic class as per PFC (802.1Qbb) specification */
> > > > > +};
> > > > > +
> > > > >  /**
> > > > >   * Field IDs for MODIFY_FIELD action.
> > > > >   */
> > > > > --
> > > > > 2.33.0
> > > > >
> > >
> > > Best,
> > > Ori

RE: [dpdk-dev] [RFC PATCH] ethdev: support priority based flow control

[Ori Kam]

> -----Original Message-----
> From: Jerin Jacob <jerinjacobk@gmail.com>
> Sent: Wednesday, November 24, 2021 12:48 PM
> 
> On Wed, Nov 24, 2021 at 3:01 PM Ori Kam <orika@nvidia.com> wrote:
> >
> > Hi Jerin,
> >
> > > -----Original Message-----
> > > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > Sent: Tuesday, November 23, 2021 12:58 PM
> > > To: Ori Kam <orika@nvidia.com>
> > >
> > > On Sun, Nov 21, 2021 at 3:20 PM Ori Kam <orika@nvidia.com> wrote:
> > > >
> > > > Hi Jerin,
> > >
> > > Hi Ori,
> > >
> > > >
> > > > Sorry for my late response,
> > >
> > > Thanks for the review.
> > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > > > Sent: Wednesday, November 17, 2021 11:49 AM
> > > > > To: Jerin Jacob <jerinj@marvell.com>
> > > > > Subject: Re: [dpdk-dev] [RFC PATCH] ethdev: support priority based flow control
> > > > >
> > > > > On Tue, Oct 5, 2021 at 6:32 PM <jerinj@marvell.com> wrote:
> > > > > >
> > > > > > From: Jerin Jacob <jerinj@marvell.com>
> > > > > >
> > > > > > rte_eth_dev_priority_flow_ctrl_set() based API is not generic as it
> > > > > > can not support other than VLAN priority mapping to PFC traffic class.
> > > > > >
> > > > > > Introducing RTE_FLOW_ACTION_TYPE_PFC_SET_TC rte_flow action to
> > > > > > set the traffic class as per 802.1Qbb specification. This will enable,
> > > > > > Traffic class(8bit) to be selected based on any packet field like DSCP.
> > > > > >
> > > > > > Also, making it as rte_flow action will enable fine control on
> > > > > > traffic class selection to a specific queue or VF etc.
> > > > > >
> > > > > > Signed-off-by: Jerin Jacob <jerinj@marvell.com>
> > > > >
> > > > > Ping. If there are no comments on RFC, Planning to send v1 for 22.02.
> > > >
> > > > All the set type of functions are going to be deprecated.
> > > > you should use RTE_FLOW_ACTION_TYPE_MODIFY_FIELD.
> > > >
> > > > What is the item that you are matching on when using rte_flow? Is it part of the tci in the vlan
> item?
> > >
> > > TC can be VLAN TCI field or DSCP field in IP header or any other field
> > > in packet.
> > > We need to set the traffic class as per 802.1Qbb specification, May I
> > > know how the "modify"
> > > attribute helps here. It should be a "set" operation. Right?
> > >
> >
> > Yes, in the rte_flow_action_modify_field there is what operation you want to do,
> > in this case the action should be set.
> 
> But RTE_FLOW_ACTION_TYPE_MODIFY_FIELD used for modify the packet
> content[1]. RIght?
> In this case, it is more of sideband data not anything on packet
> content. If so, explicit action
> makes sense. Right?
> 

It looks like I'm missing something,
If you don't want to change the packet and this is just data,
why not use tag/mark/flag/metadata?

Who should get this data?
If the packet is hairpined and the packet is sent to wire this info should be part
of the packet right?

> 
> [1]
>  * RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
>  *
>  * Modify a destination header field according to the specified
>  * operation. Another field of the packet can be used as a source as well
>  * as tag, mark, metadata, immediate value or a pointer to it.
> 
> >
> > I assume that you are trying to set the VLAN tag priority field right?
> 
> Both VLAN tag and/or DSCP field.
> 

Going back to the above comment so you are changing something in the packet.

> 
> >
> > >
> > >
> > > >
> > > >
> > > > >
> > > > > > ---
> > > > > >
> > > > > > Planning to submit the testpmd and cnxk ethdev driver changes after receiving
> > > > > > the feedback on this.
> > > > > >
> > > > > >
> > > > > >  doc/guides/prog_guide/rte_flow.rst | 24 ++++++++++++++++++++++++
> > > > > >  lib/ethdev/rte_flow.c              |  1 +
> > > > > >  lib/ethdev/rte_flow.h              | 27 +++++++++++++++++++++++++++
> > > > > >  3 files changed, 52 insertions(+)
> > > > > >
> > > > > > diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
> > > > > > index 2b42d5ec8c..e59f8a2902 100644
> > > > > > --- a/doc/guides/prog_guide/rte_flow.rst
> > > > > > +++ b/doc/guides/prog_guide/rte_flow.rst
> > > > > > @@ -2999,6 +2999,30 @@ which is set in the packet meta-data (i.e. struct
> > > ``rte_mbuf::sched::color``)
> > > > > >     | ``meter_color`` | Packet color |
> > > > > >     +-----------------+--------------+
> > > > > >
> > > > > > +Action: ``PFC_SET_TC``
> > > > > > +^^^^^^^^^^^^^^^^^^^^^^
> > > > > > +
> > > > > > +Set traffic class as per PFC (802.1Qbb) specification.
> > > > > > +
> > > > > > +This action must be used with any of the following action.
> > > > > > +
> > > > > > +- ``RTE_FLOW_ACTION_TYPE_QUEUE``
> > > > > > +- ``RTE_FLOW_ACTION_TYPE_RSS``
> > > > > > +- ``RTE_FLOW_ACTION_TYPE_PF``
> > > > > > +- ``RTE_FLOW_ACTION_TYPE_VF``
> > > > > > +- ``RTE_FLOW_ACTION_TYPE_PHY_PORT``
> > > > > > +- ``RTE_FLOW_ACTION_TYPE_PORT_ID``
> > > > > > +
> > > >
> > > > Why? All the above actions are terminating actions so if I want ot match on the value
> > > > it doesn't make sense to have it only on the last rule.
> > >
> > > In PFC, we are specifying, Given TC needs to steer to specific Queue,
> > > RSS, PF etc.
> > > Not sure how other actions are relevant for SET_TC action. Do you have any
> > > specific action in mind where SET_TC valid in addition to above actions
> > >
> > First what happens in case of egress traffic? There is no dest action.
> 
> It will be invalid. I can change the documentation to specify egress
> direction is not valid.
> Thoughts?
> 

Why not? Isn't it possible that application will want to send some packet with this value?

> > Second what happens if for example the priority is based on the outer tunnel
> > which I want decap and at a latter stage I want to do connection tracking and only
> > if everything is correct I want to send this packet to a queue?
> 
> Which is fine with the current scheme of things as per the documentation,
> "This action must be used with any of the following action." it does
> not preclude to
> use of any other action. If it is not clear, we can reword like below,
> ---
> This action must be used with any of the following action and not limited to
> using any of other actions in conjunction with the following action.
> ---
> Thoughts?
> 

Like stated above I can see use case where you want to set this value at the start
of the pipe and then based on this value act.

For example:
1. decap the packet and based on the tunnel set this value and jump to connection tracking group.
2. run connection tracking and jump to next table
3. Based on the connection tracking and the TC value send to some queue.

Best,
Ori
> >
> > >
> > >
> > > >
> > > > > > +.. _table_rte_flow_action_pfc_set_tc:
> > > > > > +
> > > > > > +.. table:: PFC_SET_PRIORITY
> > > > > > +
> > > > > > +   +-----------------+-------------------------------------+
> > > > > > +   | Field           | Value                               |
> > > > > > +   +=================+=====================================+
> > > > > > +   | ``tc``          | Traffic class as per PFC (802.1Qbb) |
> > > > > > +   +-----------------+-------------------------------------+
> > > > > > +
> > > > > >  Negative types
> > > > > >  ~~~~~~~~~~~~~~
> > > > > >
> > > > > > diff --git a/lib/ethdev/rte_flow.c b/lib/ethdev/rte_flow.c
> > > > > > index 8cb7a069c8..75c661159e 100644
> > > > > > --- a/lib/ethdev/rte_flow.c
> > > > > > +++ b/lib/ethdev/rte_flow.c
> > > > > > @@ -189,6 +189,7 @@ static const struct rte_flow_desc_data rte_flow_desc_action[] = {
> > > > > >          */
> > > > > >         MK_FLOW_ACTION(INDIRECT, 0),
> > > > > >         MK_FLOW_ACTION(CONNTRACK, sizeof(struct rte_flow_action_conntrack)),
> > > > > > +       MK_FLOW_ACTION(PFC_SET_TC, sizeof(struct rte_flow_action_pfc_set_tc)),
> > > > > >  };
> > > > > >
> > > > > >  int
> > > > > > diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
> > > > > > index 7b1ed7f110..5298418e9e 100644
> > > > > > --- a/lib/ethdev/rte_flow.h
> > > > > > +++ b/lib/ethdev/rte_flow.h
> > > > > > @@ -2409,6 +2409,13 @@ enum rte_flow_action_type {
> > > > > >          * See struct rte_flow_action_meter_color.
> > > > > >          */
> > > > > >         RTE_FLOW_ACTION_TYPE_METER_COLOR,
> > > > > > +
> > > > > > +       /**
> > > > > > +        * Set traffic class as per PFC (802.1Qbb) specification.
> > > > > > +        *
> > > > > > +        * See struct rte_flow_action_pfc_set_tc.
> > > > > > +        */
> > > > > > +       RTE_FLOW_ACTION_TYPE_PFC_SET_TC,
> > > > > >  };
> > > > > >
> > > > > >  /**
> > > > > > @@ -3168,6 +3175,26 @@ struct rte_flow_action_meter_color {
> > > > > >         enum rte_color color; /**< Packet color. */
> > > > > >  };
> > > > > >
> > > > > > +/**
> > > > > > + * @warning
> > > > > > + * @b EXPERIMENTAL: this structure may change without prior notice
> > > > > > + *
> > > > > > + * RTE_FLOW_ACTION_TYPE_PFC_SET_TC
> > > > > > + *
> > > > > > + * Set traffic class as per PFC (802.1Qbb) specification.
> > > > > > + *
> > > > > > + * This action must be used any of the following action.
> > > > > > + * - RTE_FLOW_ACTION_TYPE_QUEUE,
> > > > > > + * - RTE_FLOW_ACTION_TYPE_RSS,
> > > > > > + * - RTE_FLOW_ACTION_TYPE_PF,
> > > > > > + * - RTE_FLOW_ACTION_TYPE_VF,
> > > > > > + * - RTE_FLOW_ACTION_TYPE_PHY_PORT,
> > > > > > + * - RTE_FLOW_ACTION_TYPE_PORT_ID
> > > > > > + */
> > > >
> > > > What does it mean?  I must use it only on rules that have one of the above actions?
> > >
> > > See above.
> > >
> > > >
> > > > > > +struct rte_flow_action_pfc_set_tc {
> > > > > > +       uint8_t tc; /**< Traffic class as per PFC (802.1Qbb) specification */
> > > > > > +};
> > > > > > +
> > > > > >  /**
> > > > > >   * Field IDs for MODIFY_FIELD action.
> > > > > >   */
> > > > > > --
> > > > > > 2.33.0
> > > > > >
> > > >
> > > > Best,
> > > > Ori

Re: [dpdk-dev] [RFC PATCH] ethdev: support priority based flow control

[Jerin Jacob]

On Wed, Nov 24, 2021 at 9:30 PM Ori Kam <orika@nvidia.com> wrote:
>
>
>
> > -----Original Message-----
> > From: Jerin Jacob <jerinjacobk@gmail.com>
> > Sent: Wednesday, November 24, 2021 12:48 PM
> >
> > On Wed, Nov 24, 2021 at 3:01 PM Ori Kam <orika@nvidia.com> wrote:
> > >
> > > Hi Jerin,
> > >
> > > > -----Original Message-----
> > > > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > > Sent: Tuesday, November 23, 2021 12:58 PM
> > > > To: Ori Kam <orika@nvidia.com>
> > > >
> > > > On Sun, Nov 21, 2021 at 3:20 PM Ori Kam <orika@nvidia.com> wrote:
> > > > >
> > > > > Hi Jerin,
> > > >
> > > > Hi Ori,
> > > >
> > > > >
> > > > > Sorry for my late response,
> > > >
> > > > Thanks for the review.
> > > >
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > > > > Sent: Wednesday, November 17, 2021 11:49 AM
> > > > > > To: Jerin Jacob <jerinj@marvell.com>
> > > > > > Subject: Re: [dpdk-dev] [RFC PATCH] ethdev: support priority based flow control
> > > > > >
> > > > > > On Tue, Oct 5, 2021 at 6:32 PM <jerinj@marvell.com> wrote:
> > > > > > >
> > > > > > > From: Jerin Jacob <jerinj@marvell.com>
> > > > > > >
> > > > > > > rte_eth_dev_priority_flow_ctrl_set() based API is not generic as it
> > > > > > > can not support other than VLAN priority mapping to PFC traffic class.
> > > > > > >
> > > > > > > Introducing RTE_FLOW_ACTION_TYPE_PFC_SET_TC rte_flow action to
> > > > > > > set the traffic class as per 802.1Qbb specification. This will enable,
> > > > > > > Traffic class(8bit) to be selected based on any packet field like DSCP.
> > > > > > >
> > > > > > > Also, making it as rte_flow action will enable fine control on
> > > > > > > traffic class selection to a specific queue or VF etc.
> > > > > > >
> > > > > > > Signed-off-by: Jerin Jacob <jerinj@marvell.com>
> > > > > >
> > > > > > Ping. If there are no comments on RFC, Planning to send v1 for 22.02.
> > > > >
> > > > > All the set type of functions are going to be deprecated.
> > > > > you should use RTE_FLOW_ACTION_TYPE_MODIFY_FIELD.
> > > > >
> > > > > What is the item that you are matching on when using rte_flow? Is it part of the tci in the vlan
> > item?
> > > >
> > > > TC can be VLAN TCI field or DSCP field in IP header or any other field
> > > > in packet.
> > > > We need to set the traffic class as per 802.1Qbb specification, May I
> > > > know how the "modify"
> > > > attribute helps here. It should be a "set" operation. Right?
> > > >
> > >
> > > Yes, in the rte_flow_action_modify_field there is what operation you want to do,
> > > in this case the action should be set.
> >
> > But RTE_FLOW_ACTION_TYPE_MODIFY_FIELD used for modify the packet
> > content[1]. RIght?
> > In this case, it is more of sideband data not anything on packet
> > content. If so, explicit action
> > makes sense. Right?
> >
>
> It looks like I'm missing something,
> If you don't want to change the packet and this is just data,
> why not use tag/mark/flag/metadata?
>
> Who should get this data?
> If the packet is hairpined and the packet is sent to wire this info should be part
> of the packet right?

No. Here is what I envisioned for working this,
User add riles like this.

Patten: VLAN TCI is value X or DSCP value Y
Action: RTE_FLOW_ACTION_TYPE_PFC_SET_TC with an value for TC(8bit
defined in 802.1Qbb)
Driver use this rule to enable TC (flow control) with that value for
the given VLAN TCI == X

tag/mark/flag/metadata used to embed something in mbuf. Here, This
action establishes, For a given
flow what TC value needs to be enabled(it does not need to be given in
mbuf or packet for application to use).
It just establishes the TC wiring for flow control enablement for a
given pattern.
Is it adding up?

>
> >
> > [1]
> >  * RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
> >  *
> >  * Modify a destination header field according to the specified
> >  * operation. Another field of the packet can be used as a source as well
> >  * as tag, mark, metadata, immediate value or a pointer to it.
> >
> > >
> > > I assume that you are trying to set the VLAN tag priority field right?
> >
> > Both VLAN tag and/or DSCP field.
> >
>
> Going back to the above comment so you are changing something in the packet.

No. See above.

>
> >
> > >
> > > >
> > > >
> > > > >
> > > > >
> > > > > >
> > > > > > > ---
> > > > > > >
> > > > > > > Planning to submit the testpmd and cnxk ethdev driver changes after receiving
> > > > > > > the feedback on this.
> > > > > > >
> > > > > > >
> > > > > > >  doc/guides/prog_guide/rte_flow.rst | 24 ++++++++++++++++++++++++
> > > > > > >  lib/ethdev/rte_flow.c              |  1 +
> > > > > > >  lib/ethdev/rte_flow.h              | 27 +++++++++++++++++++++++++++
> > > > > > >  3 files changed, 52 insertions(+)
> > > > > > >
> > > > > > > diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
> > > > > > > index 2b42d5ec8c..e59f8a2902 100644
> > > > > > > --- a/doc/guides/prog_guide/rte_flow.rst
> > > > > > > +++ b/doc/guides/prog_guide/rte_flow.rst
> > > > > > > @@ -2999,6 +2999,30 @@ which is set in the packet meta-data (i.e. struct
> > > > ``rte_mbuf::sched::color``)
> > > > > > >     | ``meter_color`` | Packet color |
> > > > > > >     +-----------------+--------------+
> > > > > > >
> > > > > > > +Action: ``PFC_SET_TC``
> > > > > > > +^^^^^^^^^^^^^^^^^^^^^^
> > > > > > > +
> > > > > > > +Set traffic class as per PFC (802.1Qbb) specification.
> > > > > > > +
> > > > > > > +This action must be used with any of the following action.
> > > > > > > +
> > > > > > > +- ``RTE_FLOW_ACTION_TYPE_QUEUE``
> > > > > > > +- ``RTE_FLOW_ACTION_TYPE_RSS``
> > > > > > > +- ``RTE_FLOW_ACTION_TYPE_PF``
> > > > > > > +- ``RTE_FLOW_ACTION_TYPE_VF``
> > > > > > > +- ``RTE_FLOW_ACTION_TYPE_PHY_PORT``
> > > > > > > +- ``RTE_FLOW_ACTION_TYPE_PORT_ID``
> > > > > > > +
> > > > >
> > > > > Why? All the above actions are terminating actions so if I want ot match on the value
> > > > > it doesn't make sense to have it only on the last rule.
> > > >
> > > > In PFC, we are specifying, Given TC needs to steer to specific Queue,
> > > > RSS, PF etc.
> > > > Not sure how other actions are relevant for SET_TC action. Do you have any
> > > > specific action in mind where SET_TC valid in addition to above actions
> > > >
> > > First what happens in case of egress traffic? There is no dest action.
> >
> > It will be invalid. I can change the documentation to specify egress
> > direction is not valid.
> > Thoughts?
> >
>
> Why not? Isn't it possible that application will want to send some packet with this value?

This is Rx Flow control(8bit TC value defined in 802.1Qbb), Not
relevant when using on Tx.

>
> > > Second what happens if for example the priority is based on the outer tunnel
> > > which I want decap and at a latter stage I want to do connection tracking and only
> > > if everything is correct I want to send this packet to a queue?
> >
> > Which is fine with the current scheme of things as per the documentation,
> > "This action must be used with any of the following action." it does
> > not preclude to
> > use of any other action. If it is not clear, we can reword like below,
> > ---
> > This action must be used with any of the following action and not limited to
> > using any of other actions in conjunction with the following action.
> > ---
> > Thoughts?
> >
>
> Like stated above I can see use case where you want to set this value at the start
> of the pipe and then based on this value act.
>
> For example:
> 1. decap the packet and based on the tunnel set this value and jump to connection tracking group.
> 2. run connection tracking and jump to next table
> 3. Based on the connection tracking and the TC value send to some queue.

Yes. It is possible to have decap + connection tracking +
RTE_FLOW_ACTION_TYPE_PFC_SET_TC +
[RTE_FLOW_ACTION_TYPE_QUEUE or RTE_FLOW_ACTION_TYPE_RSS or
RTE_FLOW_ACTION_TYPE_PF or RTE_FLOW_ACTION_TYPE_VF or
RTE_FLOW_ACTION_TYPE_PHY_PORT or RTE_FLOW_ACTION_TYPE_PORT_ID]
cascaded actions.

>
> Best,
> Ori
> > >
> > > >
> > > >
> > > > >
> > > > > > > +.. _table_rte_flow_action_pfc_set_tc:
> > > > > > > +
> > > > > > > +.. table:: PFC_SET_PRIORITY
> > > > > > > +
> > > > > > > +   +-----------------+-------------------------------------+
> > > > > > > +   | Field           | Value                               |
> > > > > > > +   +=================+=====================================+
> > > > > > > +   | ``tc``          | Traffic class as per PFC (802.1Qbb) |
> > > > > > > +   +-----------------+-------------------------------------+
> > > > > > > +
> > > > > > >  Negative types
> > > > > > >  ~~~~~~~~~~~~~~
> > > > > > >
> > > > > > > diff --git a/lib/ethdev/rte_flow.c b/lib/ethdev/rte_flow.c
> > > > > > > index 8cb7a069c8..75c661159e 100644
> > > > > > > --- a/lib/ethdev/rte_flow.c
> > > > > > > +++ b/lib/ethdev/rte_flow.c
> > > > > > > @@ -189,6 +189,7 @@ static const struct rte_flow_desc_data rte_flow_desc_action[] = {
> > > > > > >          */
> > > > > > >         MK_FLOW_ACTION(INDIRECT, 0),
> > > > > > >         MK_FLOW_ACTION(CONNTRACK, sizeof(struct rte_flow_action_conntrack)),
> > > > > > > +       MK_FLOW_ACTION(PFC_SET_TC, sizeof(struct rte_flow_action_pfc_set_tc)),
> > > > > > >  };
> > > > > > >
> > > > > > >  int
> > > > > > > diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
> > > > > > > index 7b1ed7f110..5298418e9e 100644
> > > > > > > --- a/lib/ethdev/rte_flow.h
> > > > > > > +++ b/lib/ethdev/rte_flow.h
> > > > > > > @@ -2409,6 +2409,13 @@ enum rte_flow_action_type {
> > > > > > >          * See struct rte_flow_action_meter_color.
> > > > > > >          */
> > > > > > >         RTE_FLOW_ACTION_TYPE_METER_COLOR,
> > > > > > > +
> > > > > > > +       /**
> > > > > > > +        * Set traffic class as per PFC (802.1Qbb) specification.
> > > > > > > +        *
> > > > > > > +        * See struct rte_flow_action_pfc_set_tc.
> > > > > > > +        */
> > > > > > > +       RTE_FLOW_ACTION_TYPE_PFC_SET_TC,
> > > > > > >  };
> > > > > > >
> > > > > > >  /**
> > > > > > > @@ -3168,6 +3175,26 @@ struct rte_flow_action_meter_color {
> > > > > > >         enum rte_color color; /**< Packet color. */
> > > > > > >  };
> > > > > > >
> > > > > > > +/**
> > > > > > > + * @warning
> > > > > > > + * @b EXPERIMENTAL: this structure may change without prior notice
> > > > > > > + *
> > > > > > > + * RTE_FLOW_ACTION_TYPE_PFC_SET_TC
> > > > > > > + *
> > > > > > > + * Set traffic class as per PFC (802.1Qbb) specification.
> > > > > > > + *
> > > > > > > + * This action must be used any of the following action.
> > > > > > > + * - RTE_FLOW_ACTION_TYPE_QUEUE,
> > > > > > > + * - RTE_FLOW_ACTION_TYPE_RSS,
> > > > > > > + * - RTE_FLOW_ACTION_TYPE_PF,
> > > > > > > + * - RTE_FLOW_ACTION_TYPE_VF,
> > > > > > > + * - RTE_FLOW_ACTION_TYPE_PHY_PORT,
> > > > > > > + * - RTE_FLOW_ACTION_TYPE_PORT_ID
> > > > > > > + */
> > > > >
> > > > > What does it mean?  I must use it only on rules that have one of the above actions?
> > > >
> > > > See above.
> > > >
> > > > >
> > > > > > > +struct rte_flow_action_pfc_set_tc {
> > > > > > > +       uint8_t tc; /**< Traffic class as per PFC (802.1Qbb) specification */
> > > > > > > +};
> > > > > > > +
> > > > > > >  /**
> > > > > > >   * Field IDs for MODIFY_FIELD action.
> > > > > > >   */
> > > > > > > --
> > > > > > > 2.33.0
> > > > > > >
> > > > >
> > > > > Best,
> > > > > Ori

RE: [dpdk-dev] [RFC PATCH] ethdev: support priority based flow control

[Ori Kam]

Hi Jerin,

I think that we are not on the same page and I'm missing some critical info to decide
on the best approch.

Can we please have a short meeting so you can explain to me about this feature?

I think it will be good if Thomas, Ferruh and Andrew could join.

Best,
Ori

> -----Original Message-----
> From: Jerin Jacob <jerinjacobk@gmail.com>
> Sent: Thursday, November 25, 2021 1:12 PM
> To: Ori Kam <orika@nvidia.com>
> Subject: Re: [dpdk-dev] [RFC PATCH] ethdev: support priority based flow control
> 
> On Wed, Nov 24, 2021 at 9:30 PM Ori Kam <orika@nvidia.com> wrote:
> >
> >
> >
> > > -----Original Message-----
> > > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > Sent: Wednesday, November 24, 2021 12:48 PM
> > >
> > > On Wed, Nov 24, 2021 at 3:01 PM Ori Kam <orika@nvidia.com> wrote:
> > > >
> > > > Hi Jerin,
> > > >
> > > > > -----Original Message-----
> > > > > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > > > Sent: Tuesday, November 23, 2021 12:58 PM
> > > > > To: Ori Kam <orika@nvidia.com>
> > > > >
> > > > > On Sun, Nov 21, 2021 at 3:20 PM Ori Kam <orika@nvidia.com> wrote:
> > > > > >
> > > > > > Hi Jerin,
> > > > >
> > > > > Hi Ori,
> > > > >
> > > > > >
> > > > > > Sorry for my late response,
> > > > >
> > > > > Thanks for the review.
> > > > >
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > > > > > Sent: Wednesday, November 17, 2021 11:49 AM
> > > > > > > To: Jerin Jacob <jerinj@marvell.com>
> > > > > > > Subject: Re: [dpdk-dev] [RFC PATCH] ethdev: support priority based flow control
> > > > > > >
> > > > > > > On Tue, Oct 5, 2021 at 6:32 PM <jerinj@marvell.com> wrote:
> > > > > > > >
> > > > > > > > From: Jerin Jacob <jerinj@marvell.com>
> > > > > > > >
> > > > > > > > rte_eth_dev_priority_flow_ctrl_set() based API is not generic as it
> > > > > > > > can not support other than VLAN priority mapping to PFC traffic class.
> > > > > > > >
> > > > > > > > Introducing RTE_FLOW_ACTION_TYPE_PFC_SET_TC rte_flow action to
> > > > > > > > set the traffic class as per 802.1Qbb specification. This will enable,
> > > > > > > > Traffic class(8bit) to be selected based on any packet field like DSCP.
> > > > > > > >
> > > > > > > > Also, making it as rte_flow action will enable fine control on
> > > > > > > > traffic class selection to a specific queue or VF etc.
> > > > > > > >
> > > > > > > > Signed-off-by: Jerin Jacob <jerinj@marvell.com>
> > > > > > >
> > > > > > > Ping. If there are no comments on RFC, Planning to send v1 for 22.02.
> > > > > >
> > > > > > All the set type of functions are going to be deprecated.
> > > > > > you should use RTE_FLOW_ACTION_TYPE_MODIFY_FIELD.
> > > > > >
> > > > > > What is the item that you are matching on when using rte_flow? Is it part of the tci in the vlan
> > > item?
> > > > >
> > > > > TC can be VLAN TCI field or DSCP field in IP header or any other field
> > > > > in packet.
> > > > > We need to set the traffic class as per 802.1Qbb specification, May I
> > > > > know how the "modify"
> > > > > attribute helps here. It should be a "set" operation. Right?
> > > > >
> > > >
> > > > Yes, in the rte_flow_action_modify_field there is what operation you want to do,
> > > > in this case the action should be set.
> > >
> > > But RTE_FLOW_ACTION_TYPE_MODIFY_FIELD used for modify the packet
> > > content[1]. RIght?
> > > In this case, it is more of sideband data not anything on packet
> > > content. If so, explicit action
> > > makes sense. Right?
> > >
> >
> > It looks like I'm missing something,
> > If you don't want to change the packet and this is just data,
> > why not use tag/mark/flag/metadata?
> >
> > Who should get this data?
> > If the packet is hairpined and the packet is sent to wire this info should be part
> > of the packet right?
> 
> No. Here is what I envisioned for working this,
> User add riles like this.
> 
> Patten: VLAN TCI is value X or DSCP value Y
> Action: RTE_FLOW_ACTION_TYPE_PFC_SET_TC with an value for TC(8bit
> defined in 802.1Qbb)
> Driver use this rule to enable TC (flow control) with that value for
> the given VLAN TCI == X
> 
> tag/mark/flag/metadata used to embed something in mbuf. Here, This
> action establishes, For a given
> flow what TC value needs to be enabled(it does not need to be given in
> mbuf or packet for application to use).
> It just establishes the TC wiring for flow control enablement for a
> given pattern.
> Is it adding up?
> 
> >
> > >
> > > [1]
> > >  * RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
> > >  *
> > >  * Modify a destination header field according to the specified
> > >  * operation. Another field of the packet can be used as a source as well
> > >  * as tag, mark, metadata, immediate value or a pointer to it.
> > >
> > > >
> > > > I assume that you are trying to set the VLAN tag priority field right?
> > >
> > > Both VLAN tag and/or DSCP field.
> > >
> >
> > Going back to the above comment so you are changing something in the packet.
> 
> No. See above.
> 
> >
> > >
> > > >
> > > > >
> > > > >
> > > > > >
> > > > > >
> > > > > > >
> > > > > > > > ---
> > > > > > > >
> > > > > > > > Planning to submit the testpmd and cnxk ethdev driver changes after receiving
> > > > > > > > the feedback on this.
> > > > > > > >
> > > > > > > >
> > > > > > > >  doc/guides/prog_guide/rte_flow.rst | 24 ++++++++++++++++++++++++
> > > > > > > >  lib/ethdev/rte_flow.c              |  1 +
> > > > > > > >  lib/ethdev/rte_flow.h              | 27 +++++++++++++++++++++++++++
> > > > > > > >  3 files changed, 52 insertions(+)
> > > > > > > >
> > > > > > > > diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
> > > > > > > > index 2b42d5ec8c..e59f8a2902 100644
> > > > > > > > --- a/doc/guides/prog_guide/rte_flow.rst
> > > > > > > > +++ b/doc/guides/prog_guide/rte_flow.rst
> > > > > > > > @@ -2999,6 +2999,30 @@ which is set in the packet meta-data (i.e. struct
> > > > > ``rte_mbuf::sched::color``)
> > > > > > > >     | ``meter_color`` | Packet color |
> > > > > > > >     +-----------------+--------------+
> > > > > > > >
> > > > > > > > +Action: ``PFC_SET_TC``
> > > > > > > > +^^^^^^^^^^^^^^^^^^^^^^
> > > > > > > > +
> > > > > > > > +Set traffic class as per PFC (802.1Qbb) specification.
> > > > > > > > +
> > > > > > > > +This action must be used with any of the following action.
> > > > > > > > +
> > > > > > > > +- ``RTE_FLOW_ACTION_TYPE_QUEUE``
> > > > > > > > +- ``RTE_FLOW_ACTION_TYPE_RSS``
> > > > > > > > +- ``RTE_FLOW_ACTION_TYPE_PF``
> > > > > > > > +- ``RTE_FLOW_ACTION_TYPE_VF``
> > > > > > > > +- ``RTE_FLOW_ACTION_TYPE_PHY_PORT``
> > > > > > > > +- ``RTE_FLOW_ACTION_TYPE_PORT_ID``
> > > > > > > > +
> > > > > >
> > > > > > Why? All the above actions are terminating actions so if I want ot match on the value
> > > > > > it doesn't make sense to have it only on the last rule.
> > > > >
> > > > > In PFC, we are specifying, Given TC needs to steer to specific Queue,
> > > > > RSS, PF etc.
> > > > > Not sure how other actions are relevant for SET_TC action. Do you have any
> > > > > specific action in mind where SET_TC valid in addition to above actions
> > > > >
> > > > First what happens in case of egress traffic? There is no dest action.
> > >
> > > It will be invalid. I can change the documentation to specify egress
> > > direction is not valid.
> > > Thoughts?
> > >
> >
> > Why not? Isn't it possible that application will want to send some packet with this value?
> 
> This is Rx Flow control(8bit TC value defined in 802.1Qbb), Not
> relevant when using on Tx.
> 
> >
> > > > Second what happens if for example the priority is based on the outer tunnel
> > > > which I want decap and at a latter stage I want to do connection tracking and only
> > > > if everything is correct I want to send this packet to a queue?
> > >
> > > Which is fine with the current scheme of things as per the documentation,
> > > "This action must be used with any of the following action." it does
> > > not preclude to
> > > use of any other action. If it is not clear, we can reword like below,
> > > ---
> > > This action must be used with any of the following action and not limited to
> > > using any of other actions in conjunction with the following action.
> > > ---
> > > Thoughts?
> > >
> >
> > Like stated above I can see use case where you want to set this value at the start
> > of the pipe and then based on this value act.
> >
> > For example:
> > 1. decap the packet and based on the tunnel set this value and jump to connection tracking group.
> > 2. run connection tracking and jump to next table
> > 3. Based on the connection tracking and the TC value send to some queue.
> 
> Yes. It is possible to have decap + connection tracking +
> RTE_FLOW_ACTION_TYPE_PFC_SET_TC +
> [RTE_FLOW_ACTION_TYPE_QUEUE or RTE_FLOW_ACTION_TYPE_RSS or
> RTE_FLOW_ACTION_TYPE_PF or RTE_FLOW_ACTION_TYPE_VF or
> RTE_FLOW_ACTION_TYPE_PHY_PORT or RTE_FLOW_ACTION_TYPE_PORT_ID]
> cascaded actions.
> 
> >
> > Best,
> > Ori
> > > >
> > > > >
> > > > >
> > > > > >
> > > > > > > > +.. _table_rte_flow_action_pfc_set_tc:
> > > > > > > > +
> > > > > > > > +.. table:: PFC_SET_PRIORITY
> > > > > > > > +
> > > > > > > > +   +-----------------+-------------------------------------+
> > > > > > > > +   | Field           | Value                               |
> > > > > > > > +   +=================+=====================================+
> > > > > > > > +   | ``tc``          | Traffic class as per PFC (802.1Qbb) |
> > > > > > > > +   +-----------------+-------------------------------------+
> > > > > > > > +
> > > > > > > >  Negative types
> > > > > > > >  ~~~~~~~~~~~~~~
> > > > > > > >
> > > > > > > > diff --git a/lib/ethdev/rte_flow.c b/lib/ethdev/rte_flow.c
> > > > > > > > index 8cb7a069c8..75c661159e 100644
> > > > > > > > --- a/lib/ethdev/rte_flow.c
> > > > > > > > +++ b/lib/ethdev/rte_flow.c
> > > > > > > > @@ -189,6 +189,7 @@ static const struct rte_flow_desc_data rte_flow_desc_action[] = {
> > > > > > > >          */
> > > > > > > >         MK_FLOW_ACTION(INDIRECT, 0),
> > > > > > > >         MK_FLOW_ACTION(CONNTRACK, sizeof(struct rte_flow_action_conntrack)),
> > > > > > > > +       MK_FLOW_ACTION(PFC_SET_TC, sizeof(struct rte_flow_action_pfc_set_tc)),
> > > > > > > >  };
> > > > > > > >
> > > > > > > >  int
> > > > > > > > diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
> > > > > > > > index 7b1ed7f110..5298418e9e 100644
> > > > > > > > --- a/lib/ethdev/rte_flow.h
> > > > > > > > +++ b/lib/ethdev/rte_flow.h
> > > > > > > > @@ -2409,6 +2409,13 @@ enum rte_flow_action_type {
> > > > > > > >          * See struct rte_flow_action_meter_color.
> > > > > > > >          */
> > > > > > > >         RTE_FLOW_ACTION_TYPE_METER_COLOR,
> > > > > > > > +
> > > > > > > > +       /**
> > > > > > > > +        * Set traffic class as per PFC (802.1Qbb) specification.
> > > > > > > > +        *
> > > > > > > > +        * See struct rte_flow_action_pfc_set_tc.
> > > > > > > > +        */
> > > > > > > > +       RTE_FLOW_ACTION_TYPE_PFC_SET_TC,
> > > > > > > >  };
> > > > > > > >
> > > > > > > >  /**
> > > > > > > > @@ -3168,6 +3175,26 @@ struct rte_flow_action_meter_color {
> > > > > > > >         enum rte_color color; /**< Packet color. */
> > > > > > > >  };
> > > > > > > >
> > > > > > > > +/**
> > > > > > > > + * @warning
> > > > > > > > + * @b EXPERIMENTAL: this structure may change without prior notice
> > > > > > > > + *
> > > > > > > > + * RTE_FLOW_ACTION_TYPE_PFC_SET_TC
> > > > > > > > + *
> > > > > > > > + * Set traffic class as per PFC (802.1Qbb) specification.
> > > > > > > > + *
> > > > > > > > + * This action must be used any of the following action.
> > > > > > > > + * - RTE_FLOW_ACTION_TYPE_QUEUE,
> > > > > > > > + * - RTE_FLOW_ACTION_TYPE_RSS,
> > > > > > > > + * - RTE_FLOW_ACTION_TYPE_PF,
> > > > > > > > + * - RTE_FLOW_ACTION_TYPE_VF,
> > > > > > > > + * - RTE_FLOW_ACTION_TYPE_PHY_PORT,
> > > > > > > > + * - RTE_FLOW_ACTION_TYPE_PORT_ID
> > > > > > > > + */
> > > > > >
> > > > > > What does it mean?  I must use it only on rules that have one of the above actions?
> > > > >
> > > > > See above.
> > > > >
> > > > > >
> > > > > > > > +struct rte_flow_action_pfc_set_tc {
> > > > > > > > +       uint8_t tc; /**< Traffic class as per PFC (802.1Qbb) specification */
> > > > > > > > +};
> > > > > > > > +
> > > > > > > >  /**
> > > > > > > >   * Field IDs for MODIFY_FIELD action.
> > > > > > > >   */
> > > > > > > > --
> > > > > > > > 2.33.0
> > > > > > > >
> > > > > >
> > > > > > Best,
> > > > > > Ori

Re: [dpdk-dev] [RFC PATCH] ethdev: support priority based flow control

[Jerin Jacob]

On Thu, Nov 25, 2021 at 7:32 PM Ori Kam <orika@nvidia.com> wrote:
>
> Hi Jerin,
>
> I think that we are not on the same page and I'm missing some critical info to decide
> on the best approch.
>
> Can we please have a short meeting so you can explain to me about this feature?
>
> I think it will be good if Thomas, Ferruh and Andrew could join.

Sure, Ori. Is 2 PM UTC on 29th Nov(Monday) on https://meet.jit.si/dpdk
is fine for you/Thomas/Ferruh/Andrew?
If not, Please suggest some time.


>
> Best,
> Ori
>
> > -----Original Message-----
> > From: Jerin Jacob <jerinjacobk@gmail.com>
> > Sent: Thursday, November 25, 2021 1:12 PM
> > To: Ori Kam <orika@nvidia.com>
> > Subject: Re: [dpdk-dev] [RFC PATCH] ethdev: support priority based flow control
> >
> > On Wed, Nov 24, 2021 at 9:30 PM Ori Kam <orika@nvidia.com> wrote:
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > > Sent: Wednesday, November 24, 2021 12:48 PM
> > > >
> > > > On Wed, Nov 24, 2021 at 3:01 PM Ori Kam <orika@nvidia.com> wrote:
> > > > >
> > > > > Hi Jerin,
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > > > > Sent: Tuesday, November 23, 2021 12:58 PM
> > > > > > To: Ori Kam <orika@nvidia.com>
> > > > > >
> > > > > > On Sun, Nov 21, 2021 at 3:20 PM Ori Kam <orika@nvidia.com> wrote:
> > > > > > >
> > > > > > > Hi Jerin,
> > > > > >
> > > > > > Hi Ori,
> > > > > >
> > > > > > >
> > > > > > > Sorry for my late response,
> > > > > >
> > > > > > Thanks for the review.
> > > > > >
> > > > > > >
> > > > > > > > -----Original Message-----
> > > > > > > > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > > > > > > Sent: Wednesday, November 17, 2021 11:49 AM
> > > > > > > > To: Jerin Jacob <jerinj@marvell.com>
> > > > > > > > Subject: Re: [dpdk-dev] [RFC PATCH] ethdev: support priority based flow control
> > > > > > > >
> > > > > > > > On Tue, Oct 5, 2021 at 6:32 PM <jerinj@marvell.com> wrote:
> > > > > > > > >
> > > > > > > > > From: Jerin Jacob <jerinj@marvell.com>
> > > > > > > > >
> > > > > > > > > rte_eth_dev_priority_flow_ctrl_set() based API is not generic as it
> > > > > > > > > can not support other than VLAN priority mapping to PFC traffic class.
> > > > > > > > >
> > > > > > > > > Introducing RTE_FLOW_ACTION_TYPE_PFC_SET_TC rte_flow action to
> > > > > > > > > set the traffic class as per 802.1Qbb specification. This will enable,
> > > > > > > > > Traffic class(8bit) to be selected based on any packet field like DSCP.
> > > > > > > > >
> > > > > > > > > Also, making it as rte_flow action will enable fine control on
> > > > > > > > > traffic class selection to a specific queue or VF etc.
> > > > > > > > >
> > > > > > > > > Signed-off-by: Jerin Jacob <jerinj@marvell.com>
> > > > > > > >
> > > > > > > > Ping. If there are no comments on RFC, Planning to send v1 for 22.02.
> > > > > > >
> > > > > > > All the set type of functions are going to be deprecated.
> > > > > > > you should use RTE_FLOW_ACTION_TYPE_MODIFY_FIELD.
> > > > > > >
> > > > > > > What is the item that you are matching on when using rte_flow? Is it part of the tci in the vlan
> > > > item?
> > > > > >
> > > > > > TC can be VLAN TCI field or DSCP field in IP header or any other field
> > > > > > in packet.
> > > > > > We need to set the traffic class as per 802.1Qbb specification, May I
> > > > > > know how the "modify"
> > > > > > attribute helps here. It should be a "set" operation. Right?
> > > > > >
> > > > >
> > > > > Yes, in the rte_flow_action_modify_field there is what operation you want to do,
> > > > > in this case the action should be set.
> > > >
> > > > But RTE_FLOW_ACTION_TYPE_MODIFY_FIELD used for modify the packet
> > > > content[1]. RIght?
> > > > In this case, it is more of sideband data not anything on packet
> > > > content. If so, explicit action
> > > > makes sense. Right?
> > > >
> > >
> > > It looks like I'm missing something,
> > > If you don't want to change the packet and this is just data,
> > > why not use tag/mark/flag/metadata?
> > >
> > > Who should get this data?
> > > If the packet is hairpined and the packet is sent to wire this info should be part
> > > of the packet right?
> >
> > No. Here is what I envisioned for working this,
> > User add riles like this.
> >
> > Patten: VLAN TCI is value X or DSCP value Y
> > Action: RTE_FLOW_ACTION_TYPE_PFC_SET_TC with an value for TC(8bit
> > defined in 802.1Qbb)
> > Driver use this rule to enable TC (flow control) with that value for
> > the given VLAN TCI == X
> >
> > tag/mark/flag/metadata used to embed something in mbuf. Here, This
> > action establishes, For a given
> > flow what TC value needs to be enabled(it does not need to be given in
> > mbuf or packet for application to use).
> > It just establishes the TC wiring for flow control enablement for a
> > given pattern.
> > Is it adding up?
> >
> > >
> > > >
> > > > [1]
> > > >  * RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
> > > >  *
> > > >  * Modify a destination header field according to the specified
> > > >  * operation. Another field of the packet can be used as a source as well
> > > >  * as tag, mark, metadata, immediate value or a pointer to it.
> > > >
> > > > >
> > > > > I assume that you are trying to set the VLAN tag priority field right?
> > > >
> > > > Both VLAN tag and/or DSCP field.
> > > >
> > >
> > > Going back to the above comment so you are changing something in the packet.
> >
> > No. See above.
> >
> > >
> > > >
> > > > >
> > > > > >
> > > > > >
> > > > > > >
> > > > > > >
> > > > > > > >
> > > > > > > > > ---
> > > > > > > > >
> > > > > > > > > Planning to submit the testpmd and cnxk ethdev driver changes after receiving
> > > > > > > > > the feedback on this.
> > > > > > > > >
> > > > > > > > >
> > > > > > > > >  doc/guides/prog_guide/rte_flow.rst | 24 ++++++++++++++++++++++++
> > > > > > > > >  lib/ethdev/rte_flow.c              |  1 +
> > > > > > > > >  lib/ethdev/rte_flow.h              | 27 +++++++++++++++++++++++++++
> > > > > > > > >  3 files changed, 52 insertions(+)
> > > > > > > > >
> > > > > > > > > diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
> > > > > > > > > index 2b42d5ec8c..e59f8a2902 100644
> > > > > > > > > --- a/doc/guides/prog_guide/rte_flow.rst
> > > > > > > > > +++ b/doc/guides/prog_guide/rte_flow.rst
> > > > > > > > > @@ -2999,6 +2999,30 @@ which is set in the packet meta-data (i.e. struct
> > > > > > ``rte_mbuf::sched::color``)
> > > > > > > > >     | ``meter_color`` | Packet color |
> > > > > > > > >     +-----------------+--------------+
> > > > > > > > >
> > > > > > > > > +Action: ``PFC_SET_TC``
> > > > > > > > > +^^^^^^^^^^^^^^^^^^^^^^
> > > > > > > > > +
> > > > > > > > > +Set traffic class as per PFC (802.1Qbb) specification.
> > > > > > > > > +
> > > > > > > > > +This action must be used with any of the following action.
> > > > > > > > > +
> > > > > > > > > +- ``RTE_FLOW_ACTION_TYPE_QUEUE``
> > > > > > > > > +- ``RTE_FLOW_ACTION_TYPE_RSS``
> > > > > > > > > +- ``RTE_FLOW_ACTION_TYPE_PF``
> > > > > > > > > +- ``RTE_FLOW_ACTION_TYPE_VF``
> > > > > > > > > +- ``RTE_FLOW_ACTION_TYPE_PHY_PORT``
> > > > > > > > > +- ``RTE_FLOW_ACTION_TYPE_PORT_ID``
> > > > > > > > > +
> > > > > > >
> > > > > > > Why? All the above actions are terminating actions so if I want ot match on the value
> > > > > > > it doesn't make sense to have it only on the last rule.
> > > > > >
> > > > > > In PFC, we are specifying, Given TC needs to steer to specific Queue,
> > > > > > RSS, PF etc.
> > > > > > Not sure how other actions are relevant for SET_TC action. Do you have any
> > > > > > specific action in mind where SET_TC valid in addition to above actions
> > > > > >
> > > > > First what happens in case of egress traffic? There is no dest action.
> > > >
> > > > It will be invalid. I can change the documentation to specify egress
> > > > direction is not valid.
> > > > Thoughts?
> > > >
> > >
> > > Why not? Isn't it possible that application will want to send some packet with this value?
> >
> > This is Rx Flow control(8bit TC value defined in 802.1Qbb), Not
> > relevant when using on Tx.
> >
> > >
> > > > > Second what happens if for example the priority is based on the outer tunnel
> > > > > which I want decap and at a latter stage I want to do connection tracking and only
> > > > > if everything is correct I want to send this packet to a queue?
> > > >
> > > > Which is fine with the current scheme of things as per the documentation,
> > > > "This action must be used with any of the following action." it does
> > > > not preclude to
> > > > use of any other action. If it is not clear, we can reword like below,
> > > > ---
> > > > This action must be used with any of the following action and not limited to
> > > > using any of other actions in conjunction with the following action.
> > > > ---
> > > > Thoughts?
> > > >
> > >
> > > Like stated above I can see use case where you want to set this value at the start
> > > of the pipe and then based on this value act.
> > >
> > > For example:
> > > 1. decap the packet and based on the tunnel set this value and jump to connection tracking group.
> > > 2. run connection tracking and jump to next table
> > > 3. Based on the connection tracking and the TC value send to some queue.
> >
> > Yes. It is possible to have decap + connection tracking +
> > RTE_FLOW_ACTION_TYPE_PFC_SET_TC +
> > [RTE_FLOW_ACTION_TYPE_QUEUE or RTE_FLOW_ACTION_TYPE_RSS or
> > RTE_FLOW_ACTION_TYPE_PF or RTE_FLOW_ACTION_TYPE_VF or
> > RTE_FLOW_ACTION_TYPE_PHY_PORT or RTE_FLOW_ACTION_TYPE_PORT_ID]
> > cascaded actions.
> >
> > >
> > > Best,
> > > Ori
> > > > >
> > > > > >
> > > > > >
> > > > > > >
> > > > > > > > > +.. _table_rte_flow_action_pfc_set_tc:
> > > > > > > > > +
> > > > > > > > > +.. table:: PFC_SET_PRIORITY
> > > > > > > > > +
> > > > > > > > > +   +-----------------+-------------------------------------+
> > > > > > > > > +   | Field           | Value                               |
> > > > > > > > > +   +=================+=====================================+
> > > > > > > > > +   | ``tc``          | Traffic class as per PFC (802.1Qbb) |
> > > > > > > > > +   +-----------------+-------------------------------------+
> > > > > > > > > +
> > > > > > > > >  Negative types
> > > > > > > > >  ~~~~~~~~~~~~~~
> > > > > > > > >
> > > > > > > > > diff --git a/lib/ethdev/rte_flow.c b/lib/ethdev/rte_flow.c
> > > > > > > > > index 8cb7a069c8..75c661159e 100644
> > > > > > > > > --- a/lib/ethdev/rte_flow.c
> > > > > > > > > +++ b/lib/ethdev/rte_flow.c
> > > > > > > > > @@ -189,6 +189,7 @@ static const struct rte_flow_desc_data rte_flow_desc_action[] = {
> > > > > > > > >          */
> > > > > > > > >         MK_FLOW_ACTION(INDIRECT, 0),
> > > > > > > > >         MK_FLOW_ACTION(CONNTRACK, sizeof(struct rte_flow_action_conntrack)),
> > > > > > > > > +       MK_FLOW_ACTION(PFC_SET_TC, sizeof(struct rte_flow_action_pfc_set_tc)),
> > > > > > > > >  };
> > > > > > > > >
> > > > > > > > >  int
> > > > > > > > > diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
> > > > > > > > > index 7b1ed7f110..5298418e9e 100644
> > > > > > > > > --- a/lib/ethdev/rte_flow.h
> > > > > > > > > +++ b/lib/ethdev/rte_flow.h
> > > > > > > > > @@ -2409,6 +2409,13 @@ enum rte_flow_action_type {
> > > > > > > > >          * See struct rte_flow_action_meter_color.
> > > > > > > > >          */
> > > > > > > > >         RTE_FLOW_ACTION_TYPE_METER_COLOR,
> > > > > > > > > +
> > > > > > > > > +       /**
> > > > > > > > > +        * Set traffic class as per PFC (802.1Qbb) specification.
> > > > > > > > > +        *
> > > > > > > > > +        * See struct rte_flow_action_pfc_set_tc.
> > > > > > > > > +        */
> > > > > > > > > +       RTE_FLOW_ACTION_TYPE_PFC_SET_TC,
> > > > > > > > >  };
> > > > > > > > >
> > > > > > > > >  /**
> > > > > > > > > @@ -3168,6 +3175,26 @@ struct rte_flow_action_meter_color {
> > > > > > > > >         enum rte_color color; /**< Packet color. */
> > > > > > > > >  };
> > > > > > > > >
> > > > > > > > > +/**
> > > > > > > > > + * @warning
> > > > > > > > > + * @b EXPERIMENTAL: this structure may change without prior notice
> > > > > > > > > + *
> > > > > > > > > + * RTE_FLOW_ACTION_TYPE_PFC_SET_TC
> > > > > > > > > + *
> > > > > > > > > + * Set traffic class as per PFC (802.1Qbb) specification.
> > > > > > > > > + *
> > > > > > > > > + * This action must be used any of the following action.
> > > > > > > > > + * - RTE_FLOW_ACTION_TYPE_QUEUE,
> > > > > > > > > + * - RTE_FLOW_ACTION_TYPE_RSS,
> > > > > > > > > + * - RTE_FLOW_ACTION_TYPE_PF,
> > > > > > > > > + * - RTE_FLOW_ACTION_TYPE_VF,
> > > > > > > > > + * - RTE_FLOW_ACTION_TYPE_PHY_PORT,
> > > > > > > > > + * - RTE_FLOW_ACTION_TYPE_PORT_ID
> > > > > > > > > + */
> > > > > > >
> > > > > > > What does it mean?  I must use it only on rules that have one of the above actions?
> > > > > >
> > > > > > See above.
> > > > > >
> > > > > > >
> > > > > > > > > +struct rte_flow_action_pfc_set_tc {
> > > > > > > > > +       uint8_t tc; /**< Traffic class as per PFC (802.1Qbb) specification */
> > > > > > > > > +};
> > > > > > > > > +
> > > > > > > > >  /**
> > > > > > > > >   * Field IDs for MODIFY_FIELD action.
> > > > > > > > >   */
> > > > > > > > > --
> > > > > > > > > 2.33.0
> > > > > > > > >
> > > > > > >
> > > > > > > Best,
> > > > > > > Ori

RE: [dpdk-dev] [RFC PATCH] ethdev: support priority based flow control

[Ori Kam]

Good for me

Please send meeting inviate.

Ori

> -----Original Message-----
> From: Jerin Jacob <jerinjacobk@gmail.com>
> Sent: Friday, November 26, 2021 8:46 AM
> To: Ori Kam <orika@nvidia.com>
> Subject: Re: [dpdk-dev] [RFC PATCH] ethdev: support priority based flow control
> 
> On Thu, Nov 25, 2021 at 7:32 PM Ori Kam <orika@nvidia.com> wrote:
> >
> > Hi Jerin,
> >
> > I think that we are not on the same page and I'm missing some critical info to decide
> > on the best approch.
> >
> > Can we please have a short meeting so you can explain to me about this feature?
> >
> > I think it will be good if Thomas, Ferruh and Andrew could join.
> 
> Sure, Ori. Is 2 PM UTC on 29th Nov(Monday) on
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmeet.jit.si%2Fdpdk&amp;dat
> a=04%7C01%7Corika%40nvidia.com%7C515af20d94c0419f1e2208d9b0a88983%7C43083d15727340c1b7
> db39efd9ccc17a%7C0%7C0%7C637735060187259293%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wL
> jAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=V3n8P%2F%2BM
> udw4IMntVry8PO71PgxBAJyS9QYfTJN5i7A%3D&amp;reserved=0
> is fine for you/Thomas/Ferruh/Andrew?
> If not, Please suggest some time.
> 
> 
> >
> > Best,
> > Ori
> >
> > > -----Original Message-----
> > > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > Sent: Thursday, November 25, 2021 1:12 PM
> > > To: Ori Kam <orika@nvidia.com>
> > > Subject: Re: [dpdk-dev] [RFC PATCH] ethdev: support priority based flow control
> > >
> > > On Wed, Nov 24, 2021 at 9:30 PM Ori Kam <orika@nvidia.com> wrote:
> > > >
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > > > Sent: Wednesday, November 24, 2021 12:48 PM
> > > > >
> > > > > On Wed, Nov 24, 2021 at 3:01 PM Ori Kam <orika@nvidia.com> wrote:
> > > > > >
> > > > > > Hi Jerin,
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > > > > > Sent: Tuesday, November 23, 2021 12:58 PM
> > > > > > > To: Ori Kam <orika@nvidia.com>
> > > > > > >
> > > > > > > On Sun, Nov 21, 2021 at 3:20 PM Ori Kam <orika@nvidia.com> wrote:
> > > > > > > >
> > > > > > > > Hi Jerin,
> > > > > > >
> > > > > > > Hi Ori,
> > > > > > >
> > > > > > > >
> > > > > > > > Sorry for my late response,
> > > > > > >
> > > > > > > Thanks for the review.
> > > > > > >
> > > > > > > >
> > > > > > > > > -----Original Message-----
> > > > > > > > > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > > > > > > > Sent: Wednesday, November 17, 2021 11:49 AM
> > > > > > > > > To: Jerin Jacob <jerinj@marvell.com>
> > > > > > > > > Subject: Re: [dpdk-dev] [RFC PATCH] ethdev: support priority based flow control
> > > > > > > > >
> > > > > > > > > On Tue, Oct 5, 2021 at 6:32 PM <jerinj@marvell.com> wrote:
> > > > > > > > > >
> > > > > > > > > > From: Jerin Jacob <jerinj@marvell.com>
> > > > > > > > > >
> > > > > > > > > > rte_eth_dev_priority_flow_ctrl_set() based API is not generic as it
> > > > > > > > > > can not support other than VLAN priority mapping to PFC traffic class.
> > > > > > > > > >
> > > > > > > > > > Introducing RTE_FLOW_ACTION_TYPE_PFC_SET_TC rte_flow action to
> > > > > > > > > > set the traffic class as per 802.1Qbb specification. This will enable,
> > > > > > > > > > Traffic class(8bit) to be selected based on any packet field like DSCP.
> > > > > > > > > >
> > > > > > > > > > Also, making it as rte_flow action will enable fine control on
> > > > > > > > > > traffic class selection to a specific queue or VF etc.
> > > > > > > > > >
> > > > > > > > > > Signed-off-by: Jerin Jacob <jerinj@marvell.com>
> > > > > > > > >
> > > > > > > > > Ping. If there are no comments on RFC, Planning to send v1 for 22.02.
> > > > > > > >
> > > > > > > > All the set type of functions are going to be deprecated.
> > > > > > > > you should use RTE_FLOW_ACTION_TYPE_MODIFY_FIELD.
> > > > > > > >
> > > > > > > > What is the item that you are matching on when using rte_flow? Is it part of the tci in the
> vlan
> > > > > item?
> > > > > > >
> > > > > > > TC can be VLAN TCI field or DSCP field in IP header or any other field
> > > > > > > in packet.
> > > > > > > We need to set the traffic class as per 802.1Qbb specification, May I
> > > > > > > know how the "modify"
> > > > > > > attribute helps here. It should be a "set" operation. Right?
> > > > > > >
> > > > > >
> > > > > > Yes, in the rte_flow_action_modify_field there is what operation you want to do,
> > > > > > in this case the action should be set.
> > > > >
> > > > > But RTE_FLOW_ACTION_TYPE_MODIFY_FIELD used for modify the packet
> > > > > content[1]. RIght?
> > > > > In this case, it is more of sideband data not anything on packet
> > > > > content. If so, explicit action
> > > > > makes sense. Right?
> > > > >
> > > >
> > > > It looks like I'm missing something,
> > > > If you don't want to change the packet and this is just data,
> > > > why not use tag/mark/flag/metadata?
> > > >
> > > > Who should get this data?
> > > > If the packet is hairpined and the packet is sent to wire this info should be part
> > > > of the packet right?
> > >
> > > No. Here is what I envisioned for working this,
> > > User add riles like this.
> > >
> > > Patten: VLAN TCI is value X or DSCP value Y
> > > Action: RTE_FLOW_ACTION_TYPE_PFC_SET_TC with an value for TC(8bit
> > > defined in 802.1Qbb)
> > > Driver use this rule to enable TC (flow control) with that value for
> > > the given VLAN TCI == X
> > >
> > > tag/mark/flag/metadata used to embed something in mbuf. Here, This
> > > action establishes, For a given
> > > flow what TC value needs to be enabled(it does not need to be given in
> > > mbuf or packet for application to use).
> > > It just establishes the TC wiring for flow control enablement for a
> > > given pattern.
> > > Is it adding up?
> > >
> > > >
> > > > >
> > > > > [1]
> > > > >  * RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
> > > > >  *
> > > > >  * Modify a destination header field according to the specified
> > > > >  * operation. Another field of the packet can be used as a source as well
> > > > >  * as tag, mark, metadata, immediate value or a pointer to it.
> > > > >
> > > > > >
> > > > > > I assume that you are trying to set the VLAN tag priority field right?
> > > > >
> > > > > Both VLAN tag and/or DSCP field.
> > > > >
> > > >
> > > > Going back to the above comment so you are changing something in the packet.
> > >
> > > No. See above.
> > >
> > > >
> > > > >
> > > > > >
> > > > > > >
> > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > > >
> > > > > > > > > > ---
> > > > > > > > > >
> > > > > > > > > > Planning to submit the testpmd and cnxk ethdev driver changes after receiving
> > > > > > > > > > the feedback on this.
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > >  doc/guides/prog_guide/rte_flow.rst | 24 ++++++++++++++++++++++++
> > > > > > > > > >  lib/ethdev/rte_flow.c              |  1 +
> > > > > > > > > >  lib/ethdev/rte_flow.h              | 27 +++++++++++++++++++++++++++
> > > > > > > > > >  3 files changed, 52 insertions(+)
> > > > > > > > > >
> > > > > > > > > > diff --git a/doc/guides/prog_guide/rte_flow.rst
> b/doc/guides/prog_guide/rte_flow.rst
> > > > > > > > > > index 2b42d5ec8c..e59f8a2902 100644
> > > > > > > > > > --- a/doc/guides/prog_guide/rte_flow.rst
> > > > > > > > > > +++ b/doc/guides/prog_guide/rte_flow.rst
> > > > > > > > > > @@ -2999,6 +2999,30 @@ which is set in the packet meta-data (i.e. struct
> > > > > > > ``rte_mbuf::sched::color``)
> > > > > > > > > >     | ``meter_color`` | Packet color |
> > > > > > > > > >     +-----------------+--------------+
> > > > > > > > > >
> > > > > > > > > > +Action: ``PFC_SET_TC``
> > > > > > > > > > +^^^^^^^^^^^^^^^^^^^^^^
> > > > > > > > > > +
> > > > > > > > > > +Set traffic class as per PFC (802.1Qbb) specification.
> > > > > > > > > > +
> > > > > > > > > > +This action must be used with any of the following action.
> > > > > > > > > > +
> > > > > > > > > > +- ``RTE_FLOW_ACTION_TYPE_QUEUE``
> > > > > > > > > > +- ``RTE_FLOW_ACTION_TYPE_RSS``
> > > > > > > > > > +- ``RTE_FLOW_ACTION_TYPE_PF``
> > > > > > > > > > +- ``RTE_FLOW_ACTION_TYPE_VF``
> > > > > > > > > > +- ``RTE_FLOW_ACTION_TYPE_PHY_PORT``
> > > > > > > > > > +- ``RTE_FLOW_ACTION_TYPE_PORT_ID``
> > > > > > > > > > +
> > > > > > > >
> > > > > > > > Why? All the above actions are terminating actions so if I want ot match on the value
> > > > > > > > it doesn't make sense to have it only on the last rule.
> > > > > > >
> > > > > > > In PFC, we are specifying, Given TC needs to steer to specific Queue,
> > > > > > > RSS, PF etc.
> > > > > > > Not sure how other actions are relevant for SET_TC action. Do you have any
> > > > > > > specific action in mind where SET_TC valid in addition to above actions
> > > > > > >
> > > > > > First what happens in case of egress traffic? There is no dest action.
> > > > >
> > > > > It will be invalid. I can change the documentation to specify egress
> > > > > direction is not valid.
> > > > > Thoughts?
> > > > >
> > > >
> > > > Why not? Isn't it possible that application will want to send some packet with this value?
> > >
> > > This is Rx Flow control(8bit TC value defined in 802.1Qbb), Not
> > > relevant when using on Tx.
> > >
> > > >
> > > > > > Second what happens if for example the priority is based on the outer tunnel
> > > > > > which I want decap and at a latter stage I want to do connection tracking and only
> > > > > > if everything is correct I want to send this packet to a queue?
> > > > >
> > > > > Which is fine with the current scheme of things as per the documentation,
> > > > > "This action must be used with any of the following action." it does
> > > > > not preclude to
> > > > > use of any other action. If it is not clear, we can reword like below,
> > > > > ---
> > > > > This action must be used with any of the following action and not limited to
> > > > > using any of other actions in conjunction with the following action.
> > > > > ---
> > > > > Thoughts?
> > > > >
> > > >
> > > > Like stated above I can see use case where you want to set this value at the start
> > > > of the pipe and then based on this value act.
> > > >
> > > > For example:
> > > > 1. decap the packet and based on the tunnel set this value and jump to connection tracking
> group.
> > > > 2. run connection tracking and jump to next table
> > > > 3. Based on the connection tracking and the TC value send to some queue.
> > >
> > > Yes. It is possible to have decap + connection tracking +
> > > RTE_FLOW_ACTION_TYPE_PFC_SET_TC +
> > > [RTE_FLOW_ACTION_TYPE_QUEUE or RTE_FLOW_ACTION_TYPE_RSS or
> > > RTE_FLOW_ACTION_TYPE_PF or RTE_FLOW_ACTION_TYPE_VF or
> > > RTE_FLOW_ACTION_TYPE_PHY_PORT or RTE_FLOW_ACTION_TYPE_PORT_ID]
> > > cascaded actions.
> > >
> > > >
> > > > Best,
> > > > Ori
> > > > > >
> > > > > > >
> > > > > > >
> > > > > > > >
> > > > > > > > > > +.. _table_rte_flow_action_pfc_set_tc:
> > > > > > > > > > +
> > > > > > > > > > +.. table:: PFC_SET_PRIORITY
> > > > > > > > > > +
> > > > > > > > > > +   +-----------------+-------------------------------------+
> > > > > > > > > > +   | Field           | Value                               |
> > > > > > > > > > +   +=================+=====================================+
> > > > > > > > > > +   | ``tc``          | Traffic class as per PFC (802.1Qbb) |
> > > > > > > > > > +   +-----------------+-------------------------------------+
> > > > > > > > > > +
> > > > > > > > > >  Negative types
> > > > > > > > > >  ~~~~~~~~~~~~~~
> > > > > > > > > >
> > > > > > > > > > diff --git a/lib/ethdev/rte_flow.c b/lib/ethdev/rte_flow.c
> > > > > > > > > > index 8cb7a069c8..75c661159e 100644
> > > > > > > > > > --- a/lib/ethdev/rte_flow.c
> > > > > > > > > > +++ b/lib/ethdev/rte_flow.c
> > > > > > > > > > @@ -189,6 +189,7 @@ static const struct rte_flow_desc_data rte_flow_desc_action[]
> = {
> > > > > > > > > >          */
> > > > > > > > > >         MK_FLOW_ACTION(INDIRECT, 0),
> > > > > > > > > >         MK_FLOW_ACTION(CONNTRACK, sizeof(struct rte_flow_action_conntrack)),
> > > > > > > > > > +       MK_FLOW_ACTION(PFC_SET_TC, sizeof(struct rte_flow_action_pfc_set_tc)),
> > > > > > > > > >  };
> > > > > > > > > >
> > > > > > > > > >  int
> > > > > > > > > > diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
> > > > > > > > > > index 7b1ed7f110..5298418e9e 100644
> > > > > > > > > > --- a/lib/ethdev/rte_flow.h
> > > > > > > > > > +++ b/lib/ethdev/rte_flow.h
> > > > > > > > > > @@ -2409,6 +2409,13 @@ enum rte_flow_action_type {
> > > > > > > > > >          * See struct rte_flow_action_meter_color.
> > > > > > > > > >          */
> > > > > > > > > >         RTE_FLOW_ACTION_TYPE_METER_COLOR,
> > > > > > > > > > +
> > > > > > > > > > +       /**
> > > > > > > > > > +        * Set traffic class as per PFC (802.1Qbb) specification.
> > > > > > > > > > +        *
> > > > > > > > > > +        * See struct rte_flow_action_pfc_set_tc.
> > > > > > > > > > +        */
> > > > > > > > > > +       RTE_FLOW_ACTION_TYPE_PFC_SET_TC,
> > > > > > > > > >  };
> > > > > > > > > >
> > > > > > > > > >  /**
> > > > > > > > > > @@ -3168,6 +3175,26 @@ struct rte_flow_action_meter_color {
> > > > > > > > > >         enum rte_color color; /**< Packet color. */
> > > > > > > > > >  };
> > > > > > > > > >
> > > > > > > > > > +/**
> > > > > > > > > > + * @warning
> > > > > > > > > > + * @b EXPERIMENTAL: this structure may change without prior notice
> > > > > > > > > > + *
> > > > > > > > > > + * RTE_FLOW_ACTION_TYPE_PFC_SET_TC
> > > > > > > > > > + *
> > > > > > > > > > + * Set traffic class as per PFC (802.1Qbb) specification.
> > > > > > > > > > + *
> > > > > > > > > > + * This action must be used any of the following action.
> > > > > > > > > > + * - RTE_FLOW_ACTION_TYPE_QUEUE,
> > > > > > > > > > + * - RTE_FLOW_ACTION_TYPE_RSS,
> > > > > > > > > > + * - RTE_FLOW_ACTION_TYPE_PF,
> > > > > > > > > > + * - RTE_FLOW_ACTION_TYPE_VF,
> > > > > > > > > > + * - RTE_FLOW_ACTION_TYPE_PHY_PORT,
> > > > > > > > > > + * - RTE_FLOW_ACTION_TYPE_PORT_ID
> > > > > > > > > > + */
> > > > > > > >
> > > > > > > > What does it mean?  I must use it only on rules that have one of the above actions?
> > > > > > >
> > > > > > > See above.
> > > > > > >
> > > > > > > >
> > > > > > > > > > +struct rte_flow_action_pfc_set_tc {
> > > > > > > > > > +       uint8_t tc; /**< Traffic class as per PFC (802.1Qbb) specification */
> > > > > > > > > > +};
> > > > > > > > > > +
> > > > > > > > > >  /**
> > > > > > > > > >   * Field IDs for MODIFY_FIELD action.
> > > > > > > > > >   */
> > > > > > > > > > --
> > > > > > > > > > 2.33.0
> > > > > > > > > >
> > > > > > > >
> > > > > > > > Best,
> > > > > > > > Ori

Re: [dpdk-dev] [RFC PATCH] ethdev: support priority based flow control

[Jerin Jacob]

On Sun, Nov 28, 2021 at 5:01 PM Ori Kam <orika@nvidia.com> wrote:
>
> Good for me
>
> Please send the meeting invite.


Meeting invite at 2 PM UTC on 29th Nov(Monday)

Hi there,

Jerin Jacob Kollanukkaran is inviting you to a scheduled Zoom meeting.

Topic: Jerin Jacob Kollanukkaran's Personal Meeting Room


Join Zoom Meeting:
https://marvell.zoom.us/j/9901077677?pwd=T2lTTGMwYlc1YTQzMnR4eGRWQXR6QT09
    Password: 339888


Or Telephone：
    Dial(for higher quality, dial a number based on your current location)：
        US: +1 301 715 8592  or +1 312 626 6799  or +1 346 248 7799
or +1 646 558 8656  or +1 669 900 6833  or +1 253 215 8782  or 888 788
0099 (Toll Free) or 833 548 0276 (Toll Free) or 833 548 0282 (Toll
Free) or 877 853 5247 (Toll Free)
    Meeting ID: 990 107 7677
    Password: 358309
    International numbers available: https://marvell.zoom.us/u/adpcCpMHYt

Or a Video Conference Room:
From Touchpad: Tap Join Zoom button. When prompted, enter 990 107 7677
Password: 358309

For China locations, from Touchpad: Dial* then 990 107 7677
    Password: 358309

>
> Ori
>
> > -----Original Message-----
> > From: Jerin Jacob <jerinjacobk@gmail.com>
> > Sent: Friday, November 26, 2021 8:46 AM
> > To: Ori Kam <orika@nvidia.com>
> > Subject: Re: [dpdk-dev] [RFC PATCH] ethdev: support priority based flow control
> >
> > On Thu, Nov 25, 2021 at 7:32 PM Ori Kam <orika@nvidia.com> wrote:
> > >
> > > Hi Jerin,
> > >
> > > I think that we are not on the same page and I'm missing some critical info to decide
> > > on the best approch.
> > >
> > > Can we please have a short meeting so you can explain to me about this feature?
> > >
> > > I think it will be good if Thomas, Ferruh and Andrew could join.
> >
> > Sure, Ori. Is 2 PM UTC on 29th Nov(Monday) on
> > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmeet.jit.si%2Fdpdk&amp;dat
> > a=04%7C01%7Corika%40nvidia.com%7C515af20d94c0419f1e2208d9b0a88983%7C43083d15727340c1b7
> > db39efd9ccc17a%7C0%7C0%7C637735060187259293%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wL
> > jAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=V3n8P%2F%2BM
> > udw4IMntVry8PO71PgxBAJyS9QYfTJN5i7A%3D&amp;reserved=0
> > is fine for you/Thomas/Ferruh/Andrew?
> > If not, Please suggest some time.
> >
> >
> > >
> > > Best,
> > > Ori
> > >
> > > > -----Original Message-----
> > > > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > > Sent: Thursday, November 25, 2021 1:12 PM
> > > > To: Ori Kam <orika@nvidia.com>
> > > > Subject: Re: [dpdk-dev] [RFC PATCH] ethdev: support priority based flow control
> > > >
> > > > On Wed, Nov 24, 2021 at 9:30 PM Ori Kam <orika@nvidia.com> wrote:
> > > > >
> > > > >
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > > > > Sent: Wednesday, November 24, 2021 12:48 PM
> > > > > >
> > > > > > On Wed, Nov 24, 2021 at 3:01 PM Ori Kam <orika@nvidia.com> wrote:
> > > > > > >
> > > > > > > Hi Jerin,
> > > > > > >
> > > > > > > > -----Original Message-----
> > > > > > > > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > > > > > > Sent: Tuesday, November 23, 2021 12:58 PM
> > > > > > > > To: Ori Kam <orika@nvidia.com>
> > > > > > > >
> > > > > > > > On Sun, Nov 21, 2021 at 3:20 PM Ori Kam <orika@nvidia.com> wrote:
> > > > > > > > >
> > > > > > > > > Hi Jerin,
> > > > > > > >
> > > > > > > > Hi Ori,
> > > > > > > >
> > > > > > > > >
> > > > > > > > > Sorry for my late response,
> > > > > > > >
> > > > > > > > Thanks for the review.
> > > > > > > >
> > > > > > > > >
> > > > > > > > > > -----Original Message-----
> > > > > > > > > > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > > > > > > > > Sent: Wednesday, November 17, 2021 11:49 AM
> > > > > > > > > > To: Jerin Jacob <jerinj@marvell.com>
> > > > > > > > > > Subject: Re: [dpdk-dev] [RFC PATCH] ethdev: support priority based flow control
> > > > > > > > > >
> > > > > > > > > > On Tue, Oct 5, 2021 at 6:32 PM <jerinj@marvell.com> wrote:
> > > > > > > > > > >
> > > > > > > > > > > From: Jerin Jacob <jerinj@marvell.com>
> > > > > > > > > > >
> > > > > > > > > > > rte_eth_dev_priority_flow_ctrl_set() based API is not generic as it
> > > > > > > > > > > can not support other than VLAN priority mapping to PFC traffic class.
> > > > > > > > > > >
> > > > > > > > > > > Introducing RTE_FLOW_ACTION_TYPE_PFC_SET_TC rte_flow action to
> > > > > > > > > > > set the traffic class as per 802.1Qbb specification. This will enable,
> > > > > > > > > > > Traffic class(8bit) to be selected based on any packet field like DSCP.
> > > > > > > > > > >
> > > > > > > > > > > Also, making it as rte_flow action will enable fine control on
> > > > > > > > > > > traffic class selection to a specific queue or VF etc.
> > > > > > > > > > >
> > > > > > > > > > > Signed-off-by: Jerin Jacob <jerinj@marvell.com>
> > > > > > > > > >
> > > > > > > > > > Ping. If there are no comments on RFC, Planning to send v1 for 22.02.
> > > > > > > > >
> > > > > > > > > All the set type of functions are going to be deprecated.
> > > > > > > > > you should use RTE_FLOW_ACTION_TYPE_MODIFY_FIELD.
> > > > > > > > >
> > > > > > > > > What is the item that you are matching on when using rte_flow? Is it part of the tci in the
> > vlan
> > > > > > item?
> > > > > > > >
> > > > > > > > TC can be VLAN TCI field or DSCP field in IP header or any other field
> > > > > > > > in packet.
> > > > > > > > We need to set the traffic class as per 802.1Qbb specification, May I
> > > > > > > > know how the "modify"
> > > > > > > > attribute helps here. It should be a "set" operation. Right?
> > > > > > > >
> > > > > > >
> > > > > > > Yes, in the rte_flow_action_modify_field there is what operation you want to do,
> > > > > > > in this case the action should be set.
> > > > > >
> > > > > > But RTE_FLOW_ACTION_TYPE_MODIFY_FIELD used for modify the packet
> > > > > > content[1]. RIght?
> > > > > > In this case, it is more of sideband data not anything on packet
> > > > > > content. If so, explicit action
> > > > > > makes sense. Right?
> > > > > >
> > > > >
> > > > > It looks like I'm missing something,
> > > > > If you don't want to change the packet and this is just data,
> > > > > why not use tag/mark/flag/metadata?
> > > > >
> > > > > Who should get this data?
> > > > > If the packet is hairpined and the packet is sent to wire this info should be part
> > > > > of the packet right?
> > > >
> > > > No. Here is what I envisioned for working this,
> > > > User add riles like this.
> > > >
> > > > Patten: VLAN TCI is value X or DSCP value Y
> > > > Action: RTE_FLOW_ACTION_TYPE_PFC_SET_TC with an value for TC(8bit
> > > > defined in 802.1Qbb)
> > > > Driver use this rule to enable TC (flow control) with that value for
> > > > the given VLAN TCI == X
> > > >
> > > > tag/mark/flag/metadata used to embed something in mbuf. Here, This
> > > > action establishes, For a given
> > > > flow what TC value needs to be enabled(it does not need to be given in
> > > > mbuf or packet for application to use).
> > > > It just establishes the TC wiring for flow control enablement for a
> > > > given pattern.
> > > > Is it adding up?
> > > >
> > > > >
> > > > > >
> > > > > > [1]
> > > > > >  * RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
> > > > > >  *
> > > > > >  * Modify a destination header field according to the specified
> > > > > >  * operation. Another field of the packet can be used as a source as well
> > > > > >  * as tag, mark, metadata, immediate value or a pointer to it.
> > > > > >
> > > > > > >
> > > > > > > I assume that you are trying to set the VLAN tag priority field right?
> > > > > >
> > > > > > Both VLAN tag and/or DSCP field.
> > > > > >
> > > > >
> > > > > Going back to the above comment so you are changing something in the packet.
> > > >
> > > > No. See above.
> > > >
> > > > >
> > > > > >
> > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > > ---
> > > > > > > > > > >
> > > > > > > > > > > Planning to submit the testpmd and cnxk ethdev driver changes after receiving
> > > > > > > > > > > the feedback on this.
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > >  doc/guides/prog_guide/rte_flow.rst | 24 ++++++++++++++++++++++++
> > > > > > > > > > >  lib/ethdev/rte_flow.c              |  1 +
> > > > > > > > > > >  lib/ethdev/rte_flow.h              | 27 +++++++++++++++++++++++++++
> > > > > > > > > > >  3 files changed, 52 insertions(+)
> > > > > > > > > > >
> > > > > > > > > > > diff --git a/doc/guides/prog_guide/rte_flow.rst
> > b/doc/guides/prog_guide/rte_flow.rst
> > > > > > > > > > > index 2b42d5ec8c..e59f8a2902 100644
> > > > > > > > > > > --- a/doc/guides/prog_guide/rte_flow.rst
> > > > > > > > > > > +++ b/doc/guides/prog_guide/rte_flow.rst
> > > > > > > > > > > @@ -2999,6 +2999,30 @@ which is set in the packet meta-data (i.e. struct
> > > > > > > > ``rte_mbuf::sched::color``)
> > > > > > > > > > >     | ``meter_color`` | Packet color |
> > > > > > > > > > >     +-----------------+--------------+
> > > > > > > > > > >
> > > > > > > > > > > +Action: ``PFC_SET_TC``
> > > > > > > > > > > +^^^^^^^^^^^^^^^^^^^^^^
> > > > > > > > > > > +
> > > > > > > > > > > +Set traffic class as per PFC (802.1Qbb) specification.
> > > > > > > > > > > +
> > > > > > > > > > > +This action must be used with any of the following action.
> > > > > > > > > > > +
> > > > > > > > > > > +- ``RTE_FLOW_ACTION_TYPE_QUEUE``
> > > > > > > > > > > +- ``RTE_FLOW_ACTION_TYPE_RSS``
> > > > > > > > > > > +- ``RTE_FLOW_ACTION_TYPE_PF``
> > > > > > > > > > > +- ``RTE_FLOW_ACTION_TYPE_VF``
> > > > > > > > > > > +- ``RTE_FLOW_ACTION_TYPE_PHY_PORT``
> > > > > > > > > > > +- ``RTE_FLOW_ACTION_TYPE_PORT_ID``
> > > > > > > > > > > +
> > > > > > > > >
> > > > > > > > > Why? All the above actions are terminating actions so if I want ot match on the value
> > > > > > > > > it doesn't make sense to have it only on the last rule.
> > > > > > > >
> > > > > > > > In PFC, we are specifying, Given TC needs to steer to specific Queue,
> > > > > > > > RSS, PF etc.
> > > > > > > > Not sure how other actions are relevant for SET_TC action. Do you have any
> > > > > > > > specific action in mind where SET_TC valid in addition to above actions
> > > > > > > >
> > > > > > > First what happens in case of egress traffic? There is no dest action.
> > > > > >
> > > > > > It will be invalid. I can change the documentation to specify egress
> > > > > > direction is not valid.
> > > > > > Thoughts?
> > > > > >
> > > > >
> > > > > Why not? Isn't it possible that application will want to send some packet with this value?
> > > >
> > > > This is Rx Flow control(8bit TC value defined in 802.1Qbb), Not
> > > > relevant when using on Tx.
> > > >
> > > > >
> > > > > > > Second what happens if for example the priority is based on the outer tunnel
> > > > > > > which I want decap and at a latter stage I want to do connection tracking and only
> > > > > > > if everything is correct I want to send this packet to a queue?
> > > > > >
> > > > > > Which is fine with the current scheme of things as per the documentation,
> > > > > > "This action must be used with any of the following action." it does
> > > > > > not preclude to
> > > > > > use of any other action. If it is not clear, we can reword like below,
> > > > > > ---
> > > > > > This action must be used with any of the following action and not limited to
> > > > > > using any of other actions in conjunction with the following action.
> > > > > > ---
> > > > > > Thoughts?
> > > > > >
> > > > >
> > > > > Like stated above I can see use case where you want to set this value at the start
> > > > > of the pipe and then based on this value act.
> > > > >
> > > > > For example:
> > > > > 1. decap the packet and based on the tunnel set this value and jump to connection tracking
> > group.
> > > > > 2. run connection tracking and jump to next table
> > > > > 3. Based on the connection tracking and the TC value send to some queue.
> > > >
> > > > Yes. It is possible to have decap + connection tracking +
> > > > RTE_FLOW_ACTION_TYPE_PFC_SET_TC +
> > > > [RTE_FLOW_ACTION_TYPE_QUEUE or RTE_FLOW_ACTION_TYPE_RSS or
> > > > RTE_FLOW_ACTION_TYPE_PF or RTE_FLOW_ACTION_TYPE_VF or
> > > > RTE_FLOW_ACTION_TYPE_PHY_PORT or RTE_FLOW_ACTION_TYPE_PORT_ID]
> > > > cascaded actions.
> > > >
> > > > >
> > > > > Best,
> > > > > Ori
> > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > > >
> > > > > > > > > > > +.. _table_rte_flow_action_pfc_set_tc:
> > > > > > > > > > > +
> > > > > > > > > > > +.. table:: PFC_SET_PRIORITY
> > > > > > > > > > > +
> > > > > > > > > > > +   +-----------------+-------------------------------------+
> > > > > > > > > > > +   | Field           | Value                               |
> > > > > > > > > > > +   +=================+=====================================+
> > > > > > > > > > > +   | ``tc``          | Traffic class as per PFC (802.1Qbb) |
> > > > > > > > > > > +   +-----------------+-------------------------------------+
> > > > > > > > > > > +
> > > > > > > > > > >  Negative types
> > > > > > > > > > >  ~~~~~~~~~~~~~~
> > > > > > > > > > >
> > > > > > > > > > > diff --git a/lib/ethdev/rte_flow.c b/lib/ethdev/rte_flow.c
> > > > > > > > > > > index 8cb7a069c8..75c661159e 100644
> > > > > > > > > > > --- a/lib/ethdev/rte_flow.c
> > > > > > > > > > > +++ b/lib/ethdev/rte_flow.c
> > > > > > > > > > > @@ -189,6 +189,7 @@ static const struct rte_flow_desc_data rte_flow_desc_action[]
> > = {
> > > > > > > > > > >          */
> > > > > > > > > > >         MK_FLOW_ACTION(INDIRECT, 0),
> > > > > > > > > > >         MK_FLOW_ACTION(CONNTRACK, sizeof(struct rte_flow_action_conntrack)),
> > > > > > > > > > > +       MK_FLOW_ACTION(PFC_SET_TC, sizeof(struct rte_flow_action_pfc_set_tc)),
> > > > > > > > > > >  };
> > > > > > > > > > >
> > > > > > > > > > >  int
> > > > > > > > > > > diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
> > > > > > > > > > > index 7b1ed7f110..5298418e9e 100644
> > > > > > > > > > > --- a/lib/ethdev/rte_flow.h
> > > > > > > > > > > +++ b/lib/ethdev/rte_flow.h
> > > > > > > > > > > @@ -2409,6 +2409,13 @@ enum rte_flow_action_type {
> > > > > > > > > > >          * See struct rte_flow_action_meter_color.
> > > > > > > > > > >          */
> > > > > > > > > > >         RTE_FLOW_ACTION_TYPE_METER_COLOR,
> > > > > > > > > > > +
> > > > > > > > > > > +       /**
> > > > > > > > > > > +        * Set traffic class as per PFC (802.1Qbb) specification.
> > > > > > > > > > > +        *
> > > > > > > > > > > +        * See struct rte_flow_action_pfc_set_tc.
> > > > > > > > > > > +        */
> > > > > > > > > > > +       RTE_FLOW_ACTION_TYPE_PFC_SET_TC,
> > > > > > > > > > >  };
> > > > > > > > > > >
> > > > > > > > > > >  /**
> > > > > > > > > > > @@ -3168,6 +3175,26 @@ struct rte_flow_action_meter_color {
> > > > > > > > > > >         enum rte_color color; /**< Packet color. */
> > > > > > > > > > >  };
> > > > > > > > > > >
> > > > > > > > > > > +/**
> > > > > > > > > > > + * @warning
> > > > > > > > > > > + * @b EXPERIMENTAL: this structure may change without prior notice
> > > > > > > > > > > + *
> > > > > > > > > > > + * RTE_FLOW_ACTION_TYPE_PFC_SET_TC
> > > > > > > > > > > + *
> > > > > > > > > > > + * Set traffic class as per PFC (802.1Qbb) specification.
> > > > > > > > > > > + *
> > > > > > > > > > > + * This action must be used any of the following action.
> > > > > > > > > > > + * - RTE_FLOW_ACTION_TYPE_QUEUE,
> > > > > > > > > > > + * - RTE_FLOW_ACTION_TYPE_RSS,
> > > > > > > > > > > + * - RTE_FLOW_ACTION_TYPE_PF,
> > > > > > > > > > > + * - RTE_FLOW_ACTION_TYPE_VF,
> > > > > > > > > > > + * - RTE_FLOW_ACTION_TYPE_PHY_PORT,
> > > > > > > > > > > + * - RTE_FLOW_ACTION_TYPE_PORT_ID
> > > > > > > > > > > + */
> > > > > > > > >
> > > > > > > > > What does it mean?  I must use it only on rules that have one of the above actions?
> > > > > > > >
> > > > > > > > See above.
> > > > > > > >
> > > > > > > > >
> > > > > > > > > > > +struct rte_flow_action_pfc_set_tc {
> > > > > > > > > > > +       uint8_t tc; /**< Traffic class as per PFC (802.1Qbb) specification */
> > > > > > > > > > > +};
> > > > > > > > > > > +
> > > > > > > > > > >  /**
> > > > > > > > > > >   * Field IDs for MODIFY_FIELD action.
> > > > > > > > > > >   */
> > > > > > > > > > > --
> > > > > > > > > > > 2.33.0
> > > > > > > > > > >
> > > > > > > > >
> > > > > > > > > Best,
> > > > > > > > > Ori

Re: [dpdk-dev] [RFC PATCH] ethdev: support priority based flow control

[Jerin Jacob]

On Sun, Nov 28, 2021 at 5:07 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
>
> On Sun, Nov 28, 2021 at 5:01 PM Ori Kam <orika@nvidia.com> wrote:
> >
> > Good for me
> >
> > Please send the meeting invite.


Thanks, Ori, Thomas, and Ferruh for attending the meeting.

General consensus to expose the feature as ethdev API instead of rte_flow i.e
rte_flow will be used for steering the traffic to Queue for given VLAN
TCI field or so and the
ethdev Rx queue will have the configuration for traffic class value.

I will reject this
http://patches.dpdk.org/project/dpdk/patch/20211005125923.2651449-1-jerinj@marvell.com/
patch and send a new one
based on the above theme.

>
>
> Meeting invite at 2 PM UTC on 29th Nov(Monday)
>
> Hi there,
>
> Jerin Jacob Kollanukkaran is inviting you to a scheduled Zoom meeting.
>
> Topic: Jerin Jacob Kollanukkaran's Personal Meeting Room
>
>
> Join Zoom Meeting:
> https://marvell.zoom.us/j/9901077677?pwd=T2lTTGMwYlc1YTQzMnR4eGRWQXR6QT09
>     Password: 339888
>
>
> Or Telephone：
>     Dial(for higher quality, dial a number based on your current location)：
>         US: +1 301 715 8592  or +1 312 626 6799  or +1 346 248 7799
> or +1 646 558 8656  or +1 669 900 6833  or +1 253 215 8782  or 888 788
> 0099 (Toll Free) or 833 548 0276 (Toll Free) or 833 548 0282 (Toll
> Free) or 877 853 5247 (Toll Free)
>     Meeting ID: 990 107 7677
>     Password: 358309
>     International numbers available: https://marvell.zoom.us/u/adpcCpMHYt
>
> Or a Video Conference Room:
> From Touchpad: Tap Join Zoom button. When prompted, enter 990 107 7677
> Password: 358309
>
> For China locations, from Touchpad: Dial* then 990 107 7677
>     Password: 358309
>
> >
> > Ori
> >
> > > -----Original Message-----
> > > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > Sent: Friday, November 26, 2021 8:46 AM
> > > To: Ori Kam <orika@nvidia.com>
> > > Subject: Re: [dpdk-dev] [RFC PATCH] ethdev: support priority based flow control
> > >
> > > On Thu, Nov 25, 2021 at 7:32 PM Ori Kam <orika@nvidia.com> wrote:
> > > >
> > > > Hi Jerin,
> > > >
> > > > I think that we are not on the same page and I'm missing some critical info to decide
> > > > on the best approch.
> > > >
> > > > Can we please have a short meeting so you can explain to me about this feature?
> > > >
> > > > I think it will be good if Thomas, Ferruh and Andrew could join.
> > >
> > > Sure, Ori. Is 2 PM UTC on 29th Nov(Monday) on
> > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmeet.jit.si%2Fdpdk&amp;dat
> > > a=04%7C01%7Corika%40nvidia.com%7C515af20d94c0419f1e2208d9b0a88983%7C43083d15727340c1b7
> > > db39efd9ccc17a%7C0%7C0%7C637735060187259293%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wL
> > > jAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=V3n8P%2F%2BM
> > > udw4IMntVry8PO71PgxBAJyS9QYfTJN5i7A%3D&amp;reserved=0
> > > is fine for you/Thomas/Ferruh/Andrew?
> > > If not, Please suggest some time.
> > >
> > >
> > > >
> > > > Best,
> > > > Ori
> > > >
> > > > > -----Original Message-----
> > > > > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > > > Sent: Thursday, November 25, 2021 1:12 PM
> > > > > To: Ori Kam <orika@nvidia.com>
> > > > > Subject: Re: [dpdk-dev] [RFC PATCH] ethdev: support priority based flow control
> > > > >
> > > > > On Wed, Nov 24, 2021 at 9:30 PM Ori Kam <orika@nvidia.com> wrote:
> > > > > >
> > > > > >
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > > > > > Sent: Wednesday, November 24, 2021 12:48 PM
> > > > > > >
> > > > > > > On Wed, Nov 24, 2021 at 3:01 PM Ori Kam <orika@nvidia.com> wrote:
> > > > > > > >
> > > > > > > > Hi Jerin,
> > > > > > > >
> > > > > > > > > -----Original Message-----
> > > > > > > > > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > > > > > > > Sent: Tuesday, November 23, 2021 12:58 PM
> > > > > > > > > To: Ori Kam <orika@nvidia.com>
> > > > > > > > >
> > > > > > > > > On Sun, Nov 21, 2021 at 3:20 PM Ori Kam <orika@nvidia.com> wrote:
> > > > > > > > > >
> > > > > > > > > > Hi Jerin,
> > > > > > > > >
> > > > > > > > > Hi Ori,
> > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > Sorry for my late response,
> > > > > > > > >
> > > > > > > > > Thanks for the review.
> > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > > -----Original Message-----
> > > > > > > > > > > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > > > > > > > > > Sent: Wednesday, November 17, 2021 11:49 AM
> > > > > > > > > > > To: Jerin Jacob <jerinj@marvell.com>
> > > > > > > > > > > Subject: Re: [dpdk-dev] [RFC PATCH] ethdev: support priority based flow control
> > > > > > > > > > >
> > > > > > > > > > > On Tue, Oct 5, 2021 at 6:32 PM <jerinj@marvell.com> wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > From: Jerin Jacob <jerinj@marvell.com>
> > > > > > > > > > > >
> > > > > > > > > > > > rte_eth_dev_priority_flow_ctrl_set() based API is not generic as it
> > > > > > > > > > > > can not support other than VLAN priority mapping to PFC traffic class.
> > > > > > > > > > > >
> > > > > > > > > > > > Introducing RTE_FLOW_ACTION_TYPE_PFC_SET_TC rte_flow action to
> > > > > > > > > > > > set the traffic class as per 802.1Qbb specification. This will enable,
> > > > > > > > > > > > Traffic class(8bit) to be selected based on any packet field like DSCP.
> > > > > > > > > > > >
> > > > > > > > > > > > Also, making it as rte_flow action will enable fine control on
> > > > > > > > > > > > traffic class selection to a specific queue or VF etc.
> > > > > > > > > > > >
> > > > > > > > > > > > Signed-off-by: Jerin Jacob <jerinj@marvell.com>
> > > > > > > > > > >
> > > > > > > > > > > Ping. If there are no comments on RFC, Planning to send v1 for 22.02.
> > > > > > > > > >
> > > > > > > > > > All the set type of functions are going to be deprecated.
> > > > > > > > > > you should use RTE_FLOW_ACTION_TYPE_MODIFY_FIELD.
> > > > > > > > > >
> > > > > > > > > > What is the item that you are matching on when using rte_flow? Is it part of the tci in the
> > > vlan
> > > > > > > item?
> > > > > > > > >
> > > > > > > > > TC can be VLAN TCI field or DSCP field in IP header or any other field
> > > > > > > > > in packet.
> > > > > > > > > We need to set the traffic class as per 802.1Qbb specification, May I
> > > > > > > > > know how the "modify"
> > > > > > > > > attribute helps here. It should be a "set" operation. Right?
> > > > > > > > >
> > > > > > > >
> > > > > > > > Yes, in the rte_flow_action_modify_field there is what operation you want to do,
> > > > > > > > in this case the action should be set.
> > > > > > >
> > > > > > > But RTE_FLOW_ACTION_TYPE_MODIFY_FIELD used for modify the packet
> > > > > > > content[1]. RIght?
> > > > > > > In this case, it is more of sideband data not anything on packet
> > > > > > > content. If so, explicit action
> > > > > > > makes sense. Right?
> > > > > > >
> > > > > >
> > > > > > It looks like I'm missing something,
> > > > > > If you don't want to change the packet and this is just data,
> > > > > > why not use tag/mark/flag/metadata?
> > > > > >
> > > > > > Who should get this data?
> > > > > > If the packet is hairpined and the packet is sent to wire this info should be part
> > > > > > of the packet right?
> > > > >
> > > > > No. Here is what I envisioned for working this,
> > > > > User add riles like this.
> > > > >
> > > > > Patten: VLAN TCI is value X or DSCP value Y
> > > > > Action: RTE_FLOW_ACTION_TYPE_PFC_SET_TC with an value for TC(8bit
> > > > > defined in 802.1Qbb)
> > > > > Driver use this rule to enable TC (flow control) with that value for
> > > > > the given VLAN TCI == X
> > > > >
> > > > > tag/mark/flag/metadata used to embed something in mbuf. Here, This
> > > > > action establishes, For a given
> > > > > flow what TC value needs to be enabled(it does not need to be given in
> > > > > mbuf or packet for application to use).
> > > > > It just establishes the TC wiring for flow control enablement for a
> > > > > given pattern.
> > > > > Is it adding up?
> > > > >
> > > > > >
> > > > > > >
> > > > > > > [1]
> > > > > > >  * RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
> > > > > > >  *
> > > > > > >  * Modify a destination header field according to the specified
> > > > > > >  * operation. Another field of the packet can be used as a source as well
> > > > > > >  * as tag, mark, metadata, immediate value or a pointer to it.
> > > > > > >
> > > > > > > >
> > > > > > > > I assume that you are trying to set the VLAN tag priority field right?
> > > > > > >
> > > > > > > Both VLAN tag and/or DSCP field.
> > > > > > >
> > > > > >
> > > > > > Going back to the above comment so you are changing something in the packet.
> > > > >
> > > > > No. See above.
> > > > >
> > > > > >
> > > > > > >
> > > > > > > >
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > > ---
> > > > > > > > > > > >
> > > > > > > > > > > > Planning to submit the testpmd and cnxk ethdev driver changes after receiving
> > > > > > > > > > > > the feedback on this.
> > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > > >  doc/guides/prog_guide/rte_flow.rst | 24 ++++++++++++++++++++++++
> > > > > > > > > > > >  lib/ethdev/rte_flow.c              |  1 +
> > > > > > > > > > > >  lib/ethdev/rte_flow.h              | 27 +++++++++++++++++++++++++++
> > > > > > > > > > > >  3 files changed, 52 insertions(+)
> > > > > > > > > > > >
> > > > > > > > > > > > diff --git a/doc/guides/prog_guide/rte_flow.rst
> > > b/doc/guides/prog_guide/rte_flow.rst
> > > > > > > > > > > > index 2b42d5ec8c..e59f8a2902 100644
> > > > > > > > > > > > --- a/doc/guides/prog_guide/rte_flow.rst
> > > > > > > > > > > > +++ b/doc/guides/prog_guide/rte_flow.rst
> > > > > > > > > > > > @@ -2999,6 +2999,30 @@ which is set in the packet meta-data (i.e. struct
> > > > > > > > > ``rte_mbuf::sched::color``)
> > > > > > > > > > > >     | ``meter_color`` | Packet color |
> > > > > > > > > > > >     +-----------------+--------------+
> > > > > > > > > > > >
> > > > > > > > > > > > +Action: ``PFC_SET_TC``
> > > > > > > > > > > > +^^^^^^^^^^^^^^^^^^^^^^
> > > > > > > > > > > > +
> > > > > > > > > > > > +Set traffic class as per PFC (802.1Qbb) specification.
> > > > > > > > > > > > +
> > > > > > > > > > > > +This action must be used with any of the following action.
> > > > > > > > > > > > +
> > > > > > > > > > > > +- ``RTE_FLOW_ACTION_TYPE_QUEUE``
> > > > > > > > > > > > +- ``RTE_FLOW_ACTION_TYPE_RSS``
> > > > > > > > > > > > +- ``RTE_FLOW_ACTION_TYPE_PF``
> > > > > > > > > > > > +- ``RTE_FLOW_ACTION_TYPE_VF``
> > > > > > > > > > > > +- ``RTE_FLOW_ACTION_TYPE_PHY_PORT``
> > > > > > > > > > > > +- ``RTE_FLOW_ACTION_TYPE_PORT_ID``
> > > > > > > > > > > > +
> > > > > > > > > >
> > > > > > > > > > Why? All the above actions are terminating actions so if I want ot match on the value
> > > > > > > > > > it doesn't make sense to have it only on the last rule.
> > > > > > > > >
> > > > > > > > > In PFC, we are specifying, Given TC needs to steer to specific Queue,
> > > > > > > > > RSS, PF etc.
> > > > > > > > > Not sure how other actions are relevant for SET_TC action. Do you have any
> > > > > > > > > specific action in mind where SET_TC valid in addition to above actions
> > > > > > > > >
> > > > > > > > First what happens in case of egress traffic? There is no dest action.
> > > > > > >
> > > > > > > It will be invalid. I can change the documentation to specify egress
> > > > > > > direction is not valid.
> > > > > > > Thoughts?
> > > > > > >
> > > > > >
> > > > > > Why not? Isn't it possible that application will want to send some packet with this value?
> > > > >
> > > > > This is Rx Flow control(8bit TC value defined in 802.1Qbb), Not
> > > > > relevant when using on Tx.
> > > > >
> > > > > >
> > > > > > > > Second what happens if for example the priority is based on the outer tunnel
> > > > > > > > which I want decap and at a latter stage I want to do connection tracking and only
> > > > > > > > if everything is correct I want to send this packet to a queue?
> > > > > > >
> > > > > > > Which is fine with the current scheme of things as per the documentation,
> > > > > > > "This action must be used with any of the following action." it does
> > > > > > > not preclude to
> > > > > > > use of any other action. If it is not clear, we can reword like below,
> > > > > > > ---
> > > > > > > This action must be used with any of the following action and not limited to
> > > > > > > using any of other actions in conjunction with the following action.
> > > > > > > ---
> > > > > > > Thoughts?
> > > > > > >
> > > > > >
> > > > > > Like stated above I can see use case where you want to set this value at the start
> > > > > > of the pipe and then based on this value act.
> > > > > >
> > > > > > For example:
> > > > > > 1. decap the packet and based on the tunnel set this value and jump to connection tracking
> > > group.
> > > > > > 2. run connection tracking and jump to next table
> > > > > > 3. Based on the connection tracking and the TC value send to some queue.
> > > > >
> > > > > Yes. It is possible to have decap + connection tracking +
> > > > > RTE_FLOW_ACTION_TYPE_PFC_SET_TC +
> > > > > [RTE_FLOW_ACTION_TYPE_QUEUE or RTE_FLOW_ACTION_TYPE_RSS or
> > > > > RTE_FLOW_ACTION_TYPE_PF or RTE_FLOW_ACTION_TYPE_VF or
> > > > > RTE_FLOW_ACTION_TYPE_PHY_PORT or RTE_FLOW_ACTION_TYPE_PORT_ID]
> > > > > cascaded actions.
> > > > >
> > > > > >
> > > > > > Best,
> > > > > > Ori
> > > > > > > >
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > > > +.. _table_rte_flow_action_pfc_set_tc:
> > > > > > > > > > > > +
> > > > > > > > > > > > +.. table:: PFC_SET_PRIORITY
> > > > > > > > > > > > +
> > > > > > > > > > > > +   +-----------------+-------------------------------------+
> > > > > > > > > > > > +   | Field           | Value                               |
> > > > > > > > > > > > +   +=================+=====================================+
> > > > > > > > > > > > +   | ``tc``          | Traffic class as per PFC (802.1Qbb) |
> > > > > > > > > > > > +   +-----------------+-------------------------------------+
> > > > > > > > > > > > +
> > > > > > > > > > > >  Negative types
> > > > > > > > > > > >  ~~~~~~~~~~~~~~
> > > > > > > > > > > >
> > > > > > > > > > > > diff --git a/lib/ethdev/rte_flow.c b/lib/ethdev/rte_flow.c
> > > > > > > > > > > > index 8cb7a069c8..75c661159e 100644
> > > > > > > > > > > > --- a/lib/ethdev/rte_flow.c
> > > > > > > > > > > > +++ b/lib/ethdev/rte_flow.c
> > > > > > > > > > > > @@ -189,6 +189,7 @@ static const struct rte_flow_desc_data rte_flow_desc_action[]
> > > = {
> > > > > > > > > > > >          */
> > > > > > > > > > > >         MK_FLOW_ACTION(INDIRECT, 0),
> > > > > > > > > > > >         MK_FLOW_ACTION(CONNTRACK, sizeof(struct rte_flow_action_conntrack)),
> > > > > > > > > > > > +       MK_FLOW_ACTION(PFC_SET_TC, sizeof(struct rte_flow_action_pfc_set_tc)),
> > > > > > > > > > > >  };
> > > > > > > > > > > >
> > > > > > > > > > > >  int
> > > > > > > > > > > > diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
> > > > > > > > > > > > index 7b1ed7f110..5298418e9e 100644
> > > > > > > > > > > > --- a/lib/ethdev/rte_flow.h
> > > > > > > > > > > > +++ b/lib/ethdev/rte_flow.h
> > > > > > > > > > > > @@ -2409,6 +2409,13 @@ enum rte_flow_action_type {
> > > > > > > > > > > >          * See struct rte_flow_action_meter_color.
> > > > > > > > > > > >          */
> > > > > > > > > > > >         RTE_FLOW_ACTION_TYPE_METER_COLOR,
> > > > > > > > > > > > +
> > > > > > > > > > > > +       /**
> > > > > > > > > > > > +        * Set traffic class as per PFC (802.1Qbb) specification.
> > > > > > > > > > > > +        *
> > > > > > > > > > > > +        * See struct rte_flow_action_pfc_set_tc.
> > > > > > > > > > > > +        */
> > > > > > > > > > > > +       RTE_FLOW_ACTION_TYPE_PFC_SET_TC,
> > > > > > > > > > > >  };
> > > > > > > > > > > >
> > > > > > > > > > > >  /**
> > > > > > > > > > > > @@ -3168,6 +3175,26 @@ struct rte_flow_action_meter_color {
> > > > > > > > > > > >         enum rte_color color; /**< Packet color. */
> > > > > > > > > > > >  };
> > > > > > > > > > > >
> > > > > > > > > > > > +/**
> > > > > > > > > > > > + * @warning
> > > > > > > > > > > > + * @b EXPERIMENTAL: this structure may change without prior notice
> > > > > > > > > > > > + *
> > > > > > > > > > > > + * RTE_FLOW_ACTION_TYPE_PFC_SET_TC
> > > > > > > > > > > > + *
> > > > > > > > > > > > + * Set traffic class as per PFC (802.1Qbb) specification.
> > > > > > > > > > > > + *
> > > > > > > > > > > > + * This action must be used any of the following action.
> > > > > > > > > > > > + * - RTE_FLOW_ACTION_TYPE_QUEUE,
> > > > > > > > > > > > + * - RTE_FLOW_ACTION_TYPE_RSS,
> > > > > > > > > > > > + * - RTE_FLOW_ACTION_TYPE_PF,
> > > > > > > > > > > > + * - RTE_FLOW_ACTION_TYPE_VF,
> > > > > > > > > > > > + * - RTE_FLOW_ACTION_TYPE_PHY_PORT,
> > > > > > > > > > > > + * - RTE_FLOW_ACTION_TYPE_PORT_ID
> > > > > > > > > > > > + */
> > > > > > > > > >
> > > > > > > > > > What does it mean?  I must use it only on rules that have one of the above actions?
> > > > > > > > >
> > > > > > > > > See above.
> > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > > > +struct rte_flow_action_pfc_set_tc {
> > > > > > > > > > > > +       uint8_t tc; /**< Traffic class as per PFC (802.1Qbb) specification */
> > > > > > > > > > > > +};
> > > > > > > > > > > > +
> > > > > > > > > > > >  /**
> > > > > > > > > > > >   * Field IDs for MODIFY_FIELD action.
> > > > > > > > > > > >   */
> > > > > > > > > > > > --
> > > > > > > > > > > > 2.33.0
> > > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > Best,
> > > > > > > > > > Ori

Re: [dpdk-dev] [RFC PATCH] ethdev: support priority based flow control

[Jerin Jacob]

On Mon, Nov 29, 2021 at 8:24 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
>
> On Sun, Nov 28, 2021 at 5:07 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> >
> > On Sun, Nov 28, 2021 at 5:01 PM Ori Kam <orika@nvidia.com> wrote:
> > >
> > > Good for me
> > >
> > > Please send the meeting invite.
>
>
> Thanks, Ori, Thomas, and Ferruh for attending the meeting.
>
> General consensus to expose the feature as ethdev API instead of rte_flow i.e
> rte_flow will be used for steering the traffic to Queue for given VLAN
> TCI field or so and the
> ethdev Rx queue will have the configuration for traffic class value.
>
> I will reject this
> http://patches.dpdk.org/project/dpdk/patch/20211005125923.2651449-1-jerinj@marvell.com/
> patch and send a new one
> based on the above theme.

Sent updated patch[1] based on the above scheme.
[1]
http://patches.dpdk.org/project/dpdk/patch/20211204172458.1904300-1-jerinj@marvell.com/

>
> >
> >
> > Meeting invite at 2 PM UTC on 29th Nov(Monday)
> >
> > Hi there,
> >
> > Jerin Jacob Kollanukkaran is inviting you to a scheduled Zoom meeting.
> >
> > Topic: Jerin Jacob Kollanukkaran's Personal Meeting Room
> >
> >
> > Join Zoom Meeting:
> > https://marvell.zoom.us/j/9901077677?pwd=T2lTTGMwYlc1YTQzMnR4eGRWQXR6QT09
> >     Password: 339888
> >
> >
> > Or Telephone：
> >     Dial(for higher quality, dial a number based on your current location)：
> >         US: +1 301 715 8592  or +1 312 626 6799  or +1 346 248 7799
> > or +1 646 558 8656  or +1 669 900 6833  or +1 253 215 8782  or 888 788
> > 0099 (Toll Free) or 833 548 0276 (Toll Free) or 833 548 0282 (Toll
> > Free) or 877 853 5247 (Toll Free)
> >     Meeting ID: 990 107 7677
> >     Password: 358309
> >     International numbers available: https://marvell.zoom.us/u/adpcCpMHYt
> >
> > Or a Video Conference Room:
> > From Touchpad: Tap Join Zoom button. When prompted, enter 990 107 7677
> > Password: 358309
> >
> > For China locations, from Touchpad: Dial* then 990 107 7677
> >     Password: 358309
> >
> > >
> > > Ori
> > >
> > > > -----Original Message-----
> > > > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > > Sent: Friday, November 26, 2021 8:46 AM
> > > > To: Ori Kam <orika@nvidia.com>
> > > > Subject: Re: [dpdk-dev] [RFC PATCH] ethdev: support priority based flow control
> > > >
> > > > On Thu, Nov 25, 2021 at 7:32 PM Ori Kam <orika@nvidia.com> wrote:
> > > > >
> > > > > Hi Jerin,
> > > > >
> > > > > I think that we are not on the same page and I'm missing some critical info to decide
> > > > > on the best approch.
> > > > >
> > > > > Can we please have a short meeting so you can explain to me about this feature?
> > > > >
> > > > > I think it will be good if Thomas, Ferruh and Andrew could join.
> > > >
> > > > Sure, Ori. Is 2 PM UTC on 29th Nov(Monday) on
> > > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmeet.jit.si%2Fdpdk&amp;dat
> > > > a=04%7C01%7Corika%40nvidia.com%7C515af20d94c0419f1e2208d9b0a88983%7C43083d15727340c1b7
> > > > db39efd9ccc17a%7C0%7C0%7C637735060187259293%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wL
> > > > jAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=V3n8P%2F%2BM
> > > > udw4IMntVry8PO71PgxBAJyS9QYfTJN5i7A%3D&amp;reserved=0
> > > > is fine for you/Thomas/Ferruh/Andrew?
> > > > If not, Please suggest some time.
> > > >
> > > >
> > > > >
> > > > > Best,
> > > > > Ori
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > > > > Sent: Thursday, November 25, 2021 1:12 PM
> > > > > > To: Ori Kam <orika@nvidia.com>
> > > > > > Subject: Re: [dpdk-dev] [RFC PATCH] ethdev: support priority based flow control
> > > > > >
> > > > > > On Wed, Nov 24, 2021 at 9:30 PM Ori Kam <orika@nvidia.com> wrote:
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > > -----Original Message-----
> > > > > > > > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > > > > > > Sent: Wednesday, November 24, 2021 12:48 PM
> > > > > > > >
> > > > > > > > On Wed, Nov 24, 2021 at 3:01 PM Ori Kam <orika@nvidia.com> wrote:
> > > > > > > > >
> > > > > > > > > Hi Jerin,
> > > > > > > > >
> > > > > > > > > > -----Original Message-----
> > > > > > > > > > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > > > > > > > > Sent: Tuesday, November 23, 2021 12:58 PM
> > > > > > > > > > To: Ori Kam <orika@nvidia.com>
> > > > > > > > > >
> > > > > > > > > > On Sun, Nov 21, 2021 at 3:20 PM Ori Kam <orika@nvidia.com> wrote:
> > > > > > > > > > >
> > > > > > > > > > > Hi Jerin,
> > > > > > > > > >
> > > > > > > > > > Hi Ori,
> > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > Sorry for my late response,
> > > > > > > > > >
> > > > > > > > > > Thanks for the review.
> > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > > -----Original Message-----
> > > > > > > > > > > > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > > > > > > > > > > Sent: Wednesday, November 17, 2021 11:49 AM
> > > > > > > > > > > > To: Jerin Jacob <jerinj@marvell.com>
> > > > > > > > > > > > Subject: Re: [dpdk-dev] [RFC PATCH] ethdev: support priority based flow control
> > > > > > > > > > > >
> > > > > > > > > > > > On Tue, Oct 5, 2021 at 6:32 PM <jerinj@marvell.com> wrote:
> > > > > > > > > > > > >
> > > > > > > > > > > > > From: Jerin Jacob <jerinj@marvell.com>
> > > > > > > > > > > > >
> > > > > > > > > > > > > rte_eth_dev_priority_flow_ctrl_set() based API is not generic as it
> > > > > > > > > > > > > can not support other than VLAN priority mapping to PFC traffic class.
> > > > > > > > > > > > >
> > > > > > > > > > > > > Introducing RTE_FLOW_ACTION_TYPE_PFC_SET_TC rte_flow action to
> > > > > > > > > > > > > set the traffic class as per 802.1Qbb specification. This will enable,
> > > > > > > > > > > > > Traffic class(8bit) to be selected based on any packet field like DSCP.
> > > > > > > > > > > > >
> > > > > > > > > > > > > Also, making it as rte_flow action will enable fine control on
> > > > > > > > > > > > > traffic class selection to a specific queue or VF etc.
> > > > > > > > > > > > >
> > > > > > > > > > > > > Signed-off-by: Jerin Jacob <jerinj@marvell.com>
> > > > > > > > > > > >
> > > > > > > > > > > > Ping. If there are no comments on RFC, Planning to send v1 for 22.02.
> > > > > > > > > > >
> > > > > > > > > > > All the set type of functions are going to be deprecated.
> > > > > > > > > > > you should use RTE_FLOW_ACTION_TYPE_MODIFY_FIELD.
> > > > > > > > > > >
> > > > > > > > > > > What is the item that you are matching on when using rte_flow? Is it part of the tci in the
> > > > vlan
> > > > > > > > item?
> > > > > > > > > >
> > > > > > > > > > TC can be VLAN TCI field or DSCP field in IP header or any other field
> > > > > > > > > > in packet.
> > > > > > > > > > We need to set the traffic class as per 802.1Qbb specification, May I
> > > > > > > > > > know how the "modify"
> > > > > > > > > > attribute helps here. It should be a "set" operation. Right?
> > > > > > > > > >
> > > > > > > > >
> > > > > > > > > Yes, in the rte_flow_action_modify_field there is what operation you want to do,
> > > > > > > > > in this case the action should be set.
> > > > > > > >
> > > > > > > > But RTE_FLOW_ACTION_TYPE_MODIFY_FIELD used for modify the packet
> > > > > > > > content[1]. RIght?
> > > > > > > > In this case, it is more of sideband data not anything on packet
> > > > > > > > content. If so, explicit action
> > > > > > > > makes sense. Right?
> > > > > > > >
> > > > > > >
> > > > > > > It looks like I'm missing something,
> > > > > > > If you don't want to change the packet and this is just data,
> > > > > > > why not use tag/mark/flag/metadata?
> > > > > > >
> > > > > > > Who should get this data?
> > > > > > > If the packet is hairpined and the packet is sent to wire this info should be part
> > > > > > > of the packet right?
> > > > > >
> > > > > > No. Here is what I envisioned for working this,
> > > > > > User add riles like this.
> > > > > >
> > > > > > Patten: VLAN TCI is value X or DSCP value Y
> > > > > > Action: RTE_FLOW_ACTION_TYPE_PFC_SET_TC with an value for TC(8bit
> > > > > > defined in 802.1Qbb)
> > > > > > Driver use this rule to enable TC (flow control) with that value for
> > > > > > the given VLAN TCI == X
> > > > > >
> > > > > > tag/mark/flag/metadata used to embed something in mbuf. Here, This
> > > > > > action establishes, For a given
> > > > > > flow what TC value needs to be enabled(it does not need to be given in
> > > > > > mbuf or packet for application to use).
> > > > > > It just establishes the TC wiring for flow control enablement for a
> > > > > > given pattern.
> > > > > > Is it adding up?
> > > > > >
> > > > > > >
> > > > > > > >
> > > > > > > > [1]
> > > > > > > >  * RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
> > > > > > > >  *
> > > > > > > >  * Modify a destination header field according to the specified
> > > > > > > >  * operation. Another field of the packet can be used as a source as well
> > > > > > > >  * as tag, mark, metadata, immediate value or a pointer to it.
> > > > > > > >
> > > > > > > > >
> > > > > > > > > I assume that you are trying to set the VLAN tag priority field right?
> > > > > > > >
> > > > > > > > Both VLAN tag and/or DSCP field.
> > > > > > > >
> > > > > > >
> > > > > > > Going back to the above comment so you are changing something in the packet.
> > > > > >
> > > > > > No. See above.
> > > > > >
> > > > > > >
> > > > > > > >
> > > > > > > > >
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > > > > ---
> > > > > > > > > > > > >
> > > > > > > > > > > > > Planning to submit the testpmd and cnxk ethdev driver changes after receiving
> > > > > > > > > > > > > the feedback on this.
> > > > > > > > > > > > >
> > > > > > > > > > > > >
> > > > > > > > > > > > >  doc/guides/prog_guide/rte_flow.rst | 24 ++++++++++++++++++++++++
> > > > > > > > > > > > >  lib/ethdev/rte_flow.c              |  1 +
> > > > > > > > > > > > >  lib/ethdev/rte_flow.h              | 27 +++++++++++++++++++++++++++
> > > > > > > > > > > > >  3 files changed, 52 insertions(+)
> > > > > > > > > > > > >
> > > > > > > > > > > > > diff --git a/doc/guides/prog_guide/rte_flow.rst
> > > > b/doc/guides/prog_guide/rte_flow.rst
> > > > > > > > > > > > > index 2b42d5ec8c..e59f8a2902 100644
> > > > > > > > > > > > > --- a/doc/guides/prog_guide/rte_flow.rst
> > > > > > > > > > > > > +++ b/doc/guides/prog_guide/rte_flow.rst
> > > > > > > > > > > > > @@ -2999,6 +2999,30 @@ which is set in the packet meta-data (i.e. struct
> > > > > > > > > > ``rte_mbuf::sched::color``)
> > > > > > > > > > > > >     | ``meter_color`` | Packet color |
> > > > > > > > > > > > >     +-----------------+--------------+
> > > > > > > > > > > > >
> > > > > > > > > > > > > +Action: ``PFC_SET_TC``
> > > > > > > > > > > > > +^^^^^^^^^^^^^^^^^^^^^^
> > > > > > > > > > > > > +
> > > > > > > > > > > > > +Set traffic class as per PFC (802.1Qbb) specification.
> > > > > > > > > > > > > +
> > > > > > > > > > > > > +This action must be used with any of the following action.
> > > > > > > > > > > > > +
> > > > > > > > > > > > > +- ``RTE_FLOW_ACTION_TYPE_QUEUE``
> > > > > > > > > > > > > +- ``RTE_FLOW_ACTION_TYPE_RSS``
> > > > > > > > > > > > > +- ``RTE_FLOW_ACTION_TYPE_PF``
> > > > > > > > > > > > > +- ``RTE_FLOW_ACTION_TYPE_VF``
> > > > > > > > > > > > > +- ``RTE_FLOW_ACTION_TYPE_PHY_PORT``
> > > > > > > > > > > > > +- ``RTE_FLOW_ACTION_TYPE_PORT_ID``
> > > > > > > > > > > > > +
> > > > > > > > > > >
> > > > > > > > > > > Why? All the above actions are terminating actions so if I want ot match on the value
> > > > > > > > > > > it doesn't make sense to have it only on the last rule.
> > > > > > > > > >
> > > > > > > > > > In PFC, we are specifying, Given TC needs to steer to specific Queue,
> > > > > > > > > > RSS, PF etc.
> > > > > > > > > > Not sure how other actions are relevant for SET_TC action. Do you have any
> > > > > > > > > > specific action in mind where SET_TC valid in addition to above actions
> > > > > > > > > >
> > > > > > > > > First what happens in case of egress traffic? There is no dest action.
> > > > > > > >
> > > > > > > > It will be invalid. I can change the documentation to specify egress
> > > > > > > > direction is not valid.
> > > > > > > > Thoughts?
> > > > > > > >
> > > > > > >
> > > > > > > Why not? Isn't it possible that application will want to send some packet with this value?
> > > > > >
> > > > > > This is Rx Flow control(8bit TC value defined in 802.1Qbb), Not
> > > > > > relevant when using on Tx.
> > > > > >
> > > > > > >
> > > > > > > > > Second what happens if for example the priority is based on the outer tunnel
> > > > > > > > > which I want decap and at a latter stage I want to do connection tracking and only
> > > > > > > > > if everything is correct I want to send this packet to a queue?
> > > > > > > >
> > > > > > > > Which is fine with the current scheme of things as per the documentation,
> > > > > > > > "This action must be used with any of the following action." it does
> > > > > > > > not preclude to
> > > > > > > > use of any other action. If it is not clear, we can reword like below,
> > > > > > > > ---
> > > > > > > > This action must be used with any of the following action and not limited to
> > > > > > > > using any of other actions in conjunction with the following action.
> > > > > > > > ---
> > > > > > > > Thoughts?
> > > > > > > >
> > > > > > >
> > > > > > > Like stated above I can see use case where you want to set this value at the start
> > > > > > > of the pipe and then based on this value act.
> > > > > > >
> > > > > > > For example:
> > > > > > > 1. decap the packet and based on the tunnel set this value and jump to connection tracking
> > > > group.
> > > > > > > 2. run connection tracking and jump to next table
> > > > > > > 3. Based on the connection tracking and the TC value send to some queue.
> > > > > >
> > > > > > Yes. It is possible to have decap + connection tracking +
> > > > > > RTE_FLOW_ACTION_TYPE_PFC_SET_TC +
> > > > > > [RTE_FLOW_ACTION_TYPE_QUEUE or RTE_FLOW_ACTION_TYPE_RSS or
> > > > > > RTE_FLOW_ACTION_TYPE_PF or RTE_FLOW_ACTION_TYPE_VF or
> > > > > > RTE_FLOW_ACTION_TYPE_PHY_PORT or RTE_FLOW_ACTION_TYPE_PORT_ID]
> > > > > > cascaded actions.
> > > > > >
> > > > > > >
> > > > > > > Best,
> > > > > > > Ori
> > > > > > > > >
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > > > +.. _table_rte_flow_action_pfc_set_tc:
> > > > > > > > > > > > > +
> > > > > > > > > > > > > +.. table:: PFC_SET_PRIORITY
> > > > > > > > > > > > > +
> > > > > > > > > > > > > +   +-----------------+-------------------------------------+
> > > > > > > > > > > > > +   | Field           | Value                               |
> > > > > > > > > > > > > +   +=================+=====================================+
> > > > > > > > > > > > > +   | ``tc``          | Traffic class as per PFC (802.1Qbb) |
> > > > > > > > > > > > > +   +-----------------+-------------------------------------+
> > > > > > > > > > > > > +
> > > > > > > > > > > > >  Negative types
> > > > > > > > > > > > >  ~~~~~~~~~~~~~~
> > > > > > > > > > > > >
> > > > > > > > > > > > > diff --git a/lib/ethdev/rte_flow.c b/lib/ethdev/rte_flow.c
> > > > > > > > > > > > > index 8cb7a069c8..75c661159e 100644
> > > > > > > > > > > > > --- a/lib/ethdev/rte_flow.c
> > > > > > > > > > > > > +++ b/lib/ethdev/rte_flow.c
> > > > > > > > > > > > > @@ -189,6 +189,7 @@ static const struct rte_flow_desc_data rte_flow_desc_action[]
> > > > = {
> > > > > > > > > > > > >          */
> > > > > > > > > > > > >         MK_FLOW_ACTION(INDIRECT, 0),
> > > > > > > > > > > > >         MK_FLOW_ACTION(CONNTRACK, sizeof(struct rte_flow_action_conntrack)),
> > > > > > > > > > > > > +       MK_FLOW_ACTION(PFC_SET_TC, sizeof(struct rte_flow_action_pfc_set_tc)),
> > > > > > > > > > > > >  };
> > > > > > > > > > > > >
> > > > > > > > > > > > >  int
> > > > > > > > > > > > > diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
> > > > > > > > > > > > > index 7b1ed7f110..5298418e9e 100644
> > > > > > > > > > > > > --- a/lib/ethdev/rte_flow.h
> > > > > > > > > > > > > +++ b/lib/ethdev/rte_flow.h
> > > > > > > > > > > > > @@ -2409,6 +2409,13 @@ enum rte_flow_action_type {
> > > > > > > > > > > > >          * See struct rte_flow_action_meter_color.
> > > > > > > > > > > > >          */
> > > > > > > > > > > > >         RTE_FLOW_ACTION_TYPE_METER_COLOR,
> > > > > > > > > > > > > +
> > > > > > > > > > > > > +       /**
> > > > > > > > > > > > > +        * Set traffic class as per PFC (802.1Qbb) specification.
> > > > > > > > > > > > > +        *
> > > > > > > > > > > > > +        * See struct rte_flow_action_pfc_set_tc.
> > > > > > > > > > > > > +        */
> > > > > > > > > > > > > +       RTE_FLOW_ACTION_TYPE_PFC_SET_TC,
> > > > > > > > > > > > >  };
> > > > > > > > > > > > >
> > > > > > > > > > > > >  /**
> > > > > > > > > > > > > @@ -3168,6 +3175,26 @@ struct rte_flow_action_meter_color {
> > > > > > > > > > > > >         enum rte_color color; /**< Packet color. */
> > > > > > > > > > > > >  };
> > > > > > > > > > > > >
> > > > > > > > > > > > > +/**
> > > > > > > > > > > > > + * @warning
> > > > > > > > > > > > > + * @b EXPERIMENTAL: this structure may change without prior notice
> > > > > > > > > > > > > + *
> > > > > > > > > > > > > + * RTE_FLOW_ACTION_TYPE_PFC_SET_TC
> > > > > > > > > > > > > + *
> > > > > > > > > > > > > + * Set traffic class as per PFC (802.1Qbb) specification.
> > > > > > > > > > > > > + *
> > > > > > > > > > > > > + * This action must be used any of the following action.
> > > > > > > > > > > > > + * - RTE_FLOW_ACTION_TYPE_QUEUE,
> > > > > > > > > > > > > + * - RTE_FLOW_ACTION_TYPE_RSS,
> > > > > > > > > > > > > + * - RTE_FLOW_ACTION_TYPE_PF,
> > > > > > > > > > > > > + * - RTE_FLOW_ACTION_TYPE_VF,
> > > > > > > > > > > > > + * - RTE_FLOW_ACTION_TYPE_PHY_PORT,
> > > > > > > > > > > > > + * - RTE_FLOW_ACTION_TYPE_PORT_ID
> > > > > > > > > > > > > + */
> > > > > > > > > > >
> > > > > > > > > > > What does it mean?  I must use it only on rules that have one of the above actions?
> > > > > > > > > >
> > > > > > > > > > See above.
> > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > > > +struct rte_flow_action_pfc_set_tc {
> > > > > > > > > > > > > +       uint8_t tc; /**< Traffic class as per PFC (802.1Qbb) specification */
> > > > > > > > > > > > > +};
> > > > > > > > > > > > > +
> > > > > > > > > > > > >  /**
> > > > > > > > > > > > >   * Field IDs for MODIFY_FIELD action.
> > > > > > > > > > > > >   */
> > > > > > > > > > > > > --
> > > > > > > > > > > > > 2.33.0
> > > > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > Best,
> > > > > > > > > > > Ori