[RFC] ethdev: add group set miss actions API

  Introduce new group set miss actions API:
rte_flow_group_set_miss_actions().

A group's miss actions are a set of actions to be performed
in case of a miss on a group, i.e. when a packet didn't hit any flow
rules in the group.

Currently, the expected behavior in this case is undefined.
In order to achieve such functionality, a user can add a flow rule
that matches on all traffic with the lowest priority in the group -
this is not explicit however, and can be overridden by another flow rule
with a lower priority.

This new API function allows a user to set a group's miss actions in an
explicit way.

Signed-off-by: Tomer Shmilovich <tshmilovich@nvidia.com>
---
 doc/guides/prog_guide/rte_flow.rst | 30 +++++++++++++++++++++++++
 lib/ethdev/rte_flow.c              | 22 +++++++++++++++++++
 lib/ethdev/rte_flow.h              | 35 ++++++++++++++++++++++++++++++
 lib/ethdev/rte_flow_driver.h       |  7 ++++++
 lib/ethdev/version.map             |  3 +++
 5 files changed, 97 insertions(+)
  

Hi Tomer,

This is a good proposal overall, but it's a bit questionable
with regard to the transfer domain and precisely group 0.

Say, the user starts a DPDK application and plugs a PF to it,
also plugs a representor for a VF. If I'm not mistaken, in
this case, default behaviour is hardly "undefined" for
group 0. Packets sent by a VF are expected to reach
the representor (and vice versa). Also, packets
arriving on the physical port are expected
to hit the PF port, ethdev 0, for instance.

Does this new API intend to re-define this? I mean,
if the application fails to set the default action
for group 0 (ENOTSUP), shall it assume that the
behaviour will be as described above? And if
it succeeds, then assume that such implicit
interconnections cease functioning?

So, this API is something like "isolated mode"
in the case of non-transfer API, but allowing
to choose a "default" action rather than DROP?

Also, it is not quite clear how the new API is
supposed to co-exist with the transfer proxy
concept. Has this been properly considered?

Thank you.

On Mon, 7 Aug 2023, Tomer Shmilovich wrote:

> Introduce new group set miss actions API:
> rte_flow_group_set_miss_actions().
>
> A group's miss actions are a set of actions to be performed
> in case of a miss on a group, i.e. when a packet didn't hit any flow
> rules in the group.
>
> Currently, the expected behavior in this case is undefined.
> In order to achieve such functionality, a user can add a flow rule
> that matches on all traffic with the lowest priority in the group -
> this is not explicit however, and can be overridden by another flow rule
> with a lower priority.
>
> This new API function allows a user to set a group's miss actions in an
> explicit way.
>
> Signed-off-by: Tomer Shmilovich <tshmilovich@nvidia.com>
> ---
> doc/guides/prog_guide/rte_flow.rst | 30 +++++++++++++++++++++++++
> lib/ethdev/rte_flow.c              | 22 +++++++++++++++++++
> lib/ethdev/rte_flow.h              | 35 ++++++++++++++++++++++++++++++
> lib/ethdev/rte_flow_driver.h       |  7 ++++++
> lib/ethdev/version.map             |  3 +++
> 5 files changed, 97 insertions(+)
>
> diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
> index 5bc998a433..590d2a770e 100644
> --- a/doc/guides/prog_guide/rte_flow.rst
> +++ b/doc/guides/prog_guide/rte_flow.rst
> @@ -3758,6 +3758,36 @@ Information about the number of available resources can be retrieved via
>                      struct rte_flow_queue_info *queue_info,
>                      struct rte_flow_error *error);
>
> +Group Miss Actions
> +~~~~~~~~~~~~~~~~~~
> +
> +In an application, many flow rules share common group attributes, meaning they can be grouped and
> +classified together. A user can explicitly specify a set of actions performed on a packet when it
> +did not match any flows rules in a group using the following API:
> +
> +.. code-block:: c
> +
> +      int
> +      rte_flow_group_set_miss_actions(uint16_t port_id,
> +                                      uint32_t group_id,
> +                                      const struct rte_flow_group_attr *attr,
> +                                      const struct rte_flow_action actions[],
> +                                      struct rte_flow_error *error);
> +
> +For example, to configure a RTE_FLOW_TYPE_JUMP action as a miss action for ingress group 1:
> +
> +.. code-block:: c
> +
> +      struct rte_flow_group_attr attr = {.ingress = 1};
> +      struct rte_flow_action act[] = {
> +      /* Setting miss actions to jump to group 3 */
> +          [0] = {.type = RTE_FLOW_ACTION_TYPE_JUMP,
> +                 .conf = &(struct rte_flow_action_jump){.group = 3}},
> +          [1] = {.type = RTE_FLOW_ACTION_TYPE_END},
> +      };
> +      struct rte_flow_error err;
> +      rte_flow_group_set_miss_actions(port, 1, &attr, act, &err);
> +
> Flow templates
> ~~~~~~~~~~~~~~
>
> diff --git a/lib/ethdev/rte_flow.c b/lib/ethdev/rte_flow.c
> index 271d854f78..a98d87265f 100644
> --- a/lib/ethdev/rte_flow.c
> +++ b/lib/ethdev/rte_flow.c
> @@ -1973,6 +1973,28 @@ rte_flow_template_table_destroy(uint16_t port_id,
> 				  NULL, rte_strerror(ENOTSUP));
> }
>
> +int
> +rte_flow_group_set_miss_actions(uint16_t port_id,
> +				uint32_t group_id,
> +				const struct rte_flow_group_attr *attr,
> +				const struct rte_flow_action actions[],
> +				struct rte_flow_error *error)
> +{
> +	struct rte_eth_dev *dev = &rte_eth_devices[port_id];
> +	const struct rte_flow_ops *ops = rte_flow_ops_get(port_id, error);
> +
> +	if (unlikely(!ops))
> +		return -rte_errno;
> +	if (likely(!!ops->group_set_miss_actions)) {
> +		return flow_err(port_id,
> +				ops->group_set_miss_actions(dev, group_id, attr, actions, error),
> +				error);
> +	}
> +	return rte_flow_error_set(error, ENOTSUP,
> +				  RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
> +				  NULL, rte_strerror(ENOTSUP));
> +}
> +
> struct rte_flow *
> rte_flow_async_create(uint16_t port_id,
> 		      uint32_t queue_id,
> diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
> index 86ed98c562..2d4fd49eb7 100644
> --- a/lib/ethdev/rte_flow.h
> +++ b/lib/ethdev/rte_flow.h
> @@ -129,6 +129,12 @@ struct rte_flow_attr {
> 	uint32_t reserved:29; /**< Reserved, must be zero. */
> };
>
> +struct rte_flow_group_attr {
> +	uint32_t ingress:1;
> +	uint32_t egress:1;
> +	uint32_t transfer:1;
> +};
> +
> /**
>  * Matching pattern item types.
>  *
> @@ -5839,6 +5845,35 @@ rte_flow_template_table_destroy(uint16_t port_id,
> 		struct rte_flow_template_table *template_table,
> 		struct rte_flow_error *error);
>
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice.
> + *
> + * Set group miss actions.
> + *
> + * @param port_id
> + *   Port identifier of Ethernet device.
> + * @param group_id
> + *   Identifier of a group to set miss actions for.
> + * @param attr
> + *   Group attributes.
> + * @param actions
> + *   List of group miss actions.
> + * @param[out] error
> + *   Perform verbose error reporting if not NULL.
> + *   PMDs initialize this structure in case of error only.
> + *
> + * @return
> + *   0 on success, a negative errno value otherwise and rte_errno is set.
> + */
> +__rte_experimental
> +int
> +rte_flow_group_set_miss_actions(uint16_t port_id,
> +				uint32_t group_id,
> +				const struct rte_flow_group_attr *attr,
> +				const struct rte_flow_action actions[],
> +				struct rte_flow_error *error);
> +
> /**
>  * @warning
>  * @b EXPERIMENTAL: this API may change without prior notice.
> diff --git a/lib/ethdev/rte_flow_driver.h b/lib/ethdev/rte_flow_driver.h
> index f9fb01b8a2..3ced086c47 100644
> --- a/lib/ethdev/rte_flow_driver.h
> +++ b/lib/ethdev/rte_flow_driver.h
> @@ -227,6 +227,13 @@ struct rte_flow_ops {
> 		(struct rte_eth_dev *dev,
> 		 struct rte_flow_template_table *template_table,
> 		 struct rte_flow_error *err);
> +	/** See rte_flow_group_set_miss_actions() */
> +	int (*group_set_miss_actions)
> +		(struct rte_eth_dev *dev,
> +		 uint32_t group_id,
> +		 const struct rte_flow_group_attr *attr,
> +		 const struct rte_flow_action actions[],
> +		 struct rte_flow_error *err);
> 	/** See rte_flow_async_create() */
> 	struct rte_flow *(*async_create)
> 		(struct rte_eth_dev *dev,
> diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
> index fc492ee839..bdd41ecb5e 100644
> --- a/lib/ethdev/version.map
> +++ b/lib/ethdev/version.map
> @@ -312,6 +312,9 @@ EXPERIMENTAL {
> 	rte_flow_async_action_list_handle_query_update;
> 	rte_flow_async_actions_update;
> 	rte_flow_restore_info_dynflag;
> +
> +	# added in 23.11
> +	rte_flow_group_set_miss_actions;
> };
>
> INTERNAL {
> -- 
> 2.34.1
>
>
  

Hi Ivan, please see inline comments.

Thanks, Tomer

> -----Original Message-----
> From: Ivan Malov <ivan.malov@arknetworks.am>
> Sent: Tuesday, 8 August 2023 2:03
> To: Tomer Shmilovich <tshmilovich@nvidia.com>
> Cc: Ori Kam <orika@nvidia.com>; NBU-Contact-Thomas Monjalon
> (EXTERNAL) <thomas@monjalon.net>; Ferruh Yigit <ferruh.yigit@amd.com>;
> Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>; dev@dpdk.org
> Subject: Re: [RFC] ethdev: add group set miss actions API
> 
> External email: Use caution opening links or attachments
> 
> 
> Hi Tomer,
> 
> This is a good proposal overall, but it's a bit questionable with regard to the
> transfer domain and precisely group 0.
> 
> Say, the user starts a DPDK application and plugs a PF to it, also plugs a
> representor for a VF. If I'm not mistaken, in this case, default behaviour is
> hardly "undefined" for group 0. Packets sent by a VF are expected to reach the
> representor (and vice versa). Also, packets arriving on the physical port are
> expected to hit the PF port, ethdev 0, for instance.
> 

True.

> Does this new API intend to re-define this? I mean, if the application fails to set
> the default action for group 0 (ENOTSUP), shall it assume that the behaviour
> will be as described above? And if it succeeds, then assume that such implicit
> interconnections cease functioning?
> 
> So, this API is something like "isolated mode"
> in the case of non-transfer API, but allowing to choose a "default" action
> rather than DROP?

You have a point. These are the default "miss actions" for all use cases as I understand them:
Transfer - miss group 0 - goes to the other side of the connection: rep --> VF, VF --> rep.
Transfer - miss group > 0 - goes to E-switch manager (proxy port).
Ingress - miss group 0 - goes to application when expected (i.e. promiscuous mode); otherwise drop/go to kernel in case of bifurcated driver.
Ingress - miss group > 0 - drop.
Egress - miss any group - goes to wire.

I suggest documenting these default "miss actions", and have the new function update the miss actions for a given group.
If an application sets the group's miss actions as none (i.e. actions[0].type == RTE_FLOW_ACTION_TYPE_END), the miss actions should be restored to the aforementioned default miss actions.

Also, a different PMD may define other default miss actions and they should be documented in the NIC doc.

> 
> Also, it is not quite clear how the new API is supposed to co-exist with the
> transfer proxy concept. Has this been properly considered?

All transfer groups are created on the proxy port, so I don't see any conflict when taking into consideration the above definition.

> 
> Thank you.
> 
> On Mon, 7 Aug 2023, Tomer Shmilovich wrote:
> 
> > Introduce new group set miss actions API:
> > rte_flow_group_set_miss_actions().
> >
> > A group's miss actions are a set of actions to be performed in case of
> > a miss on a group, i.e. when a packet didn't hit any flow rules in the
> > group.
> >
> > Currently, the expected behavior in this case is undefined.
> > In order to achieve such functionality, a user can add a flow rule
> > that matches on all traffic with the lowest priority in the group -
> > this is not explicit however, and can be overridden by another flow
> > rule with a lower priority.
> >
> > This new API function allows a user to set a group's miss actions in
> > an explicit way.
> >
> > Signed-off-by: Tomer Shmilovich <tshmilovich@nvidia.com>
> > ---
> > doc/guides/prog_guide/rte_flow.rst | 30 +++++++++++++++++++++++++
> > lib/ethdev/rte_flow.c              | 22 +++++++++++++++++++
> > lib/ethdev/rte_flow.h              | 35 ++++++++++++++++++++++++++++++
> > lib/ethdev/rte_flow_driver.h       |  7 ++++++
> > lib/ethdev/version.map             |  3 +++
> > 5 files changed, 97 insertions(+)
> >
> > diff --git a/doc/guides/prog_guide/rte_flow.rst
> > b/doc/guides/prog_guide/rte_flow.rst
> > index 5bc998a433..590d2a770e 100644
> > --- a/doc/guides/prog_guide/rte_flow.rst
> > +++ b/doc/guides/prog_guide/rte_flow.rst
> > @@ -3758,6 +3758,36 @@ Information about the number of available
> resources can be retrieved via
> >                      struct rte_flow_queue_info *queue_info,
> >                      struct rte_flow_error *error);
> >
> > +Group Miss Actions
> > +~~~~~~~~~~~~~~~~~~
> > +
> > +In an application, many flow rules share common group attributes,
> > +meaning they can be grouped and classified together. A user can
> > +explicitly specify a set of actions performed on a packet when it did not
> match any flows rules in a group using the following API:
> > +
> > +.. code-block:: c
> > +
> > +      int
> > +      rte_flow_group_set_miss_actions(uint16_t port_id,
> > +                                      uint32_t group_id,
> > +                                      const struct rte_flow_group_attr *attr,
> > +                                      const struct rte_flow_action actions[],
> > +                                      struct rte_flow_error *error);
> > +
> > +For example, to configure a RTE_FLOW_TYPE_JUMP action as a miss action
> for ingress group 1:
> > +
> > +.. code-block:: c
> > +
> > +      struct rte_flow_group_attr attr = {.ingress = 1};
> > +      struct rte_flow_action act[] = {
> > +      /* Setting miss actions to jump to group 3 */
> > +          [0] = {.type = RTE_FLOW_ACTION_TYPE_JUMP,
> > +                 .conf = &(struct rte_flow_action_jump){.group = 3}},
> > +          [1] = {.type = RTE_FLOW_ACTION_TYPE_END},
> > +      };
> > +      struct rte_flow_error err;
> > +      rte_flow_group_set_miss_actions(port, 1, &attr, act, &err);
> > +
> > Flow templates
> > ~~~~~~~~~~~~~~
> >
> > diff --git a/lib/ethdev/rte_flow.c b/lib/ethdev/rte_flow.c index
> > 271d854f78..a98d87265f 100644
> > --- a/lib/ethdev/rte_flow.c
> > +++ b/lib/ethdev/rte_flow.c
> > @@ -1973,6 +1973,28 @@ rte_flow_template_table_destroy(uint16_t
> port_id,
> >                                 NULL, rte_strerror(ENOTSUP)); }
> >
> > +int
> > +rte_flow_group_set_miss_actions(uint16_t port_id,
> > +                             uint32_t group_id,
> > +                             const struct rte_flow_group_attr *attr,
> > +                             const struct rte_flow_action actions[],
> > +                             struct rte_flow_error *error) {
> > +     struct rte_eth_dev *dev = &rte_eth_devices[port_id];
> > +     const struct rte_flow_ops *ops = rte_flow_ops_get(port_id,
> > +error);
> > +
> > +     if (unlikely(!ops))
> > +             return -rte_errno;
> > +     if (likely(!!ops->group_set_miss_actions)) {
> > +             return flow_err(port_id,
> > +                             ops->group_set_miss_actions(dev, group_id, attr, actions,
> error),
> > +                             error);
> > +     }
> > +     return rte_flow_error_set(error, ENOTSUP,
> > +                               RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
> > +                               NULL, rte_strerror(ENOTSUP)); }
> > +
> > struct rte_flow *
> > rte_flow_async_create(uint16_t port_id,
> >                     uint32_t queue_id, diff --git
> > a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h index
> > 86ed98c562..2d4fd49eb7 100644
> > --- a/lib/ethdev/rte_flow.h
> > +++ b/lib/ethdev/rte_flow.h
> > @@ -129,6 +129,12 @@ struct rte_flow_attr {
> >       uint32_t reserved:29; /**< Reserved, must be zero. */ };
> >
> > +struct rte_flow_group_attr {
> > +     uint32_t ingress:1;
> > +     uint32_t egress:1;
> > +     uint32_t transfer:1;
> > +};
> > +
> > /**
> >  * Matching pattern item types.
> >  *
> > @@ -5839,6 +5845,35 @@ rte_flow_template_table_destroy(uint16_t
> port_id,
> >               struct rte_flow_template_table *template_table,
> >               struct rte_flow_error *error);
> >
> > +/**
> > + * @warning
> > + * @b EXPERIMENTAL: this API may change without prior notice.
> > + *
> > + * Set group miss actions.
> > + *
> > + * @param port_id
> > + *   Port identifier of Ethernet device.
> > + * @param group_id
> > + *   Identifier of a group to set miss actions for.
> > + * @param attr
> > + *   Group attributes.
> > + * @param actions
> > + *   List of group miss actions.
> > + * @param[out] error
> > + *   Perform verbose error reporting if not NULL.
> > + *   PMDs initialize this structure in case of error only.
> > + *
> > + * @return
> > + *   0 on success, a negative errno value otherwise and rte_errno is set.
> > + */
> > +__rte_experimental
> > +int
> > +rte_flow_group_set_miss_actions(uint16_t port_id,
> > +                             uint32_t group_id,
> > +                             const struct rte_flow_group_attr *attr,
> > +                             const struct rte_flow_action actions[],
> > +                             struct rte_flow_error *error);
> > +
> > /**
> >  * @warning
> >  * @b EXPERIMENTAL: this API may change without prior notice.
> > diff --git a/lib/ethdev/rte_flow_driver.h
> > b/lib/ethdev/rte_flow_driver.h index f9fb01b8a2..3ced086c47 100644
> > --- a/lib/ethdev/rte_flow_driver.h
> > +++ b/lib/ethdev/rte_flow_driver.h
> > @@ -227,6 +227,13 @@ struct rte_flow_ops {
> >               (struct rte_eth_dev *dev,
> >                struct rte_flow_template_table *template_table,
> >                struct rte_flow_error *err);
> > +     /** See rte_flow_group_set_miss_actions() */
> > +     int (*group_set_miss_actions)
> > +             (struct rte_eth_dev *dev,
> > +              uint32_t group_id,
> > +              const struct rte_flow_group_attr *attr,
> > +              const struct rte_flow_action actions[],
> > +              struct rte_flow_error *err);
> >       /** See rte_flow_async_create() */
> >       struct rte_flow *(*async_create)
> >               (struct rte_eth_dev *dev, diff --git
> > a/lib/ethdev/version.map b/lib/ethdev/version.map index
> > fc492ee839..bdd41ecb5e 100644
> > --- a/lib/ethdev/version.map
> > +++ b/lib/ethdev/version.map
> > @@ -312,6 +312,9 @@ EXPERIMENTAL {
> >       rte_flow_async_action_list_handle_query_update;
> >       rte_flow_async_actions_update;
> >       rte_flow_restore_info_dynflag;
> > +
> > +     # added in 23.11
> > +     rte_flow_group_set_miss_actions;
> > };
> >
> > INTERNAL {
> > --
> > 2.34.1
> >
> >
  

Hi Tomer,

OK, let's see what others will say.
Minor comment below.

On Tue, 8 Aug 2023, Tomer Shmilovich wrote:

> Hi Ivan, please see inline comments.
>
> Thanks, Tomer
>
>> -----Original Message-----
>> From: Ivan Malov <ivan.malov@arknetworks.am>
>> Sent: Tuesday, 8 August 2023 2:03
>> To: Tomer Shmilovich <tshmilovich@nvidia.com>
>> Cc: Ori Kam <orika@nvidia.com>; NBU-Contact-Thomas Monjalon
>> (EXTERNAL) <thomas@monjalon.net>; Ferruh Yigit <ferruh.yigit@amd.com>;
>> Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>; dev@dpdk.org
>> Subject: Re: [RFC] ethdev: add group set miss actions API
>>
>> External email: Use caution opening links or attachments
>>
>>
>> Hi Tomer,
>>
>> This is a good proposal overall, but it's a bit questionable with regard to the
>> transfer domain and precisely group 0.
>>
>> Say, the user starts a DPDK application and plugs a PF to it, also plugs a
>> representor for a VF. If I'm not mistaken, in this case, default behaviour is
>> hardly "undefined" for group 0. Packets sent by a VF are expected to reach the
>> representor (and vice versa). Also, packets arriving on the physical port are
>> expected to hit the PF port, ethdev 0, for instance.
>>
>
> True.
>
>> Does this new API intend to re-define this? I mean, if the application fails to set
>> the default action for group 0 (ENOTSUP), shall it assume that the behaviour
>> will be as described above? And if it succeeds, then assume that such implicit
>> interconnections cease functioning?
>>
>> So, this API is something like "isolated mode"
>> in the case of non-transfer API, but allowing to choose a "default" action
>> rather than DROP?
>
> You have a point. These are the default "miss actions" for all use cases as I understand them:
> Transfer - miss group 0 - goes to the other side of the connection: rep --> VF, VF --> rep.
> Transfer - miss group > 0 - goes to E-switch manager (proxy port).
> Ingress - miss group 0 - goes to application when expected (i.e. promiscuous mode); otherwise drop/go to kernel in case of bifurcated driver.
> Ingress - miss group > 0 - drop.
> Egress - miss any group - goes to wire.
>
> I suggest documenting these default "miss actions", and have the new function update the miss actions for a given group.
> If an application sets the group's miss actions as none (i.e. actions[0].type == RTE_FLOW_ACTION_TYPE_END), the miss actions should be restored to the aforementioned default miss actions.

There is also an action "PASSTHRU". It can potentially be used with this
meaning as "use implicit defaults", as "PASSTHRU" + "END".
Or just "END", as you suggest. Perhaps others will
take a look at this, too.

Thank you.

>
> Also, a different PMD may define other default miss actions and they should be documented in the NIC doc.
>
>>
>> Also, it is not quite clear how the new API is supposed to co-exist with the
>> transfer proxy concept. Has this been properly considered?
>
> All transfer groups are created on the proxy port, so I don't see any conflict when taking into consideration the above definition.
>
>>
>> Thank you.
>>
>> On Mon, 7 Aug 2023, Tomer Shmilovich wrote:
>>
>>> Introduce new group set miss actions API:
>>> rte_flow_group_set_miss_actions().
>>>
>>> A group's miss actions are a set of actions to be performed in case of
>>> a miss on a group, i.e. when a packet didn't hit any flow rules in the
>>> group.
>>>
>>> Currently, the expected behavior in this case is undefined.
>>> In order to achieve such functionality, a user can add a flow rule
>>> that matches on all traffic with the lowest priority in the group -
>>> this is not explicit however, and can be overridden by another flow
>>> rule with a lower priority.
>>>
>>> This new API function allows a user to set a group's miss actions in
>>> an explicit way.
>>>
>>> Signed-off-by: Tomer Shmilovich <tshmilovich@nvidia.com>
>>> ---
>>> doc/guides/prog_guide/rte_flow.rst | 30 +++++++++++++++++++++++++
>>> lib/ethdev/rte_flow.c              | 22 +++++++++++++++++++
>>> lib/ethdev/rte_flow.h              | 35 ++++++++++++++++++++++++++++++
>>> lib/ethdev/rte_flow_driver.h       |  7 ++++++
>>> lib/ethdev/version.map             |  3 +++
>>> 5 files changed, 97 insertions(+)
>>>
>>> diff --git a/doc/guides/prog_guide/rte_flow.rst
>>> b/doc/guides/prog_guide/rte_flow.rst
>>> index 5bc998a433..590d2a770e 100644
>>> --- a/doc/guides/prog_guide/rte_flow.rst
>>> +++ b/doc/guides/prog_guide/rte_flow.rst
>>> @@ -3758,6 +3758,36 @@ Information about the number of available
>> resources can be retrieved via
>>>                      struct rte_flow_queue_info *queue_info,
>>>                      struct rte_flow_error *error);
>>>
>>> +Group Miss Actions
>>> +~~~~~~~~~~~~~~~~~~
>>> +
>>> +In an application, many flow rules share common group attributes,
>>> +meaning they can be grouped and classified together. A user can
>>> +explicitly specify a set of actions performed on a packet when it did not
>> match any flows rules in a group using the following API:
>>> +
>>> +.. code-block:: c
>>> +
>>> +      int
>>> +      rte_flow_group_set_miss_actions(uint16_t port_id,
>>> +                                      uint32_t group_id,
>>> +                                      const struct rte_flow_group_attr *attr,
>>> +                                      const struct rte_flow_action actions[],
>>> +                                      struct rte_flow_error *error);
>>> +
>>> +For example, to configure a RTE_FLOW_TYPE_JUMP action as a miss action
>> for ingress group 1:
>>> +
>>> +.. code-block:: c
>>> +
>>> +      struct rte_flow_group_attr attr = {.ingress = 1};
>>> +      struct rte_flow_action act[] = {
>>> +      /* Setting miss actions to jump to group 3 */
>>> +          [0] = {.type = RTE_FLOW_ACTION_TYPE_JUMP,
>>> +                 .conf = &(struct rte_flow_action_jump){.group = 3}},
>>> +          [1] = {.type = RTE_FLOW_ACTION_TYPE_END},
>>> +      };
>>> +      struct rte_flow_error err;
>>> +      rte_flow_group_set_miss_actions(port, 1, &attr, act, &err);
>>> +
>>> Flow templates
>>> ~~~~~~~~~~~~~~
>>>
>>> diff --git a/lib/ethdev/rte_flow.c b/lib/ethdev/rte_flow.c index
>>> 271d854f78..a98d87265f 100644
>>> --- a/lib/ethdev/rte_flow.c
>>> +++ b/lib/ethdev/rte_flow.c
>>> @@ -1973,6 +1973,28 @@ rte_flow_template_table_destroy(uint16_t
>> port_id,
>>>                                 NULL, rte_strerror(ENOTSUP)); }
>>>
>>> +int
>>> +rte_flow_group_set_miss_actions(uint16_t port_id,
>>> +                             uint32_t group_id,
>>> +                             const struct rte_flow_group_attr *attr,
>>> +                             const struct rte_flow_action actions[],
>>> +                             struct rte_flow_error *error) {
>>> +     struct rte_eth_dev *dev = &rte_eth_devices[port_id];
>>> +     const struct rte_flow_ops *ops = rte_flow_ops_get(port_id,
>>> +error);
>>> +
>>> +     if (unlikely(!ops))
>>> +             return -rte_errno;
>>> +     if (likely(!!ops->group_set_miss_actions)) {
>>> +             return flow_err(port_id,
>>> +                             ops->group_set_miss_actions(dev, group_id, attr, actions,
>> error),
>>> +                             error);
>>> +     }
>>> +     return rte_flow_error_set(error, ENOTSUP,
>>> +                               RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
>>> +                               NULL, rte_strerror(ENOTSUP)); }
>>> +
>>> struct rte_flow *
>>> rte_flow_async_create(uint16_t port_id,
>>>                     uint32_t queue_id, diff --git
>>> a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h index
>>> 86ed98c562..2d4fd49eb7 100644
>>> --- a/lib/ethdev/rte_flow.h
>>> +++ b/lib/ethdev/rte_flow.h
>>> @@ -129,6 +129,12 @@ struct rte_flow_attr {
>>>       uint32_t reserved:29; /**< Reserved, must be zero. */ };
>>>
>>> +struct rte_flow_group_attr {
>>> +     uint32_t ingress:1;
>>> +     uint32_t egress:1;
>>> +     uint32_t transfer:1;
>>> +};
>>> +
>>> /**
>>>  * Matching pattern item types.
>>>  *
>>> @@ -5839,6 +5845,35 @@ rte_flow_template_table_destroy(uint16_t
>> port_id,
>>>               struct rte_flow_template_table *template_table,
>>>               struct rte_flow_error *error);
>>>
>>> +/**
>>> + * @warning
>>> + * @b EXPERIMENTAL: this API may change without prior notice.
>>> + *
>>> + * Set group miss actions.
>>> + *
>>> + * @param port_id
>>> + *   Port identifier of Ethernet device.
>>> + * @param group_id
>>> + *   Identifier of a group to set miss actions for.
>>> + * @param attr
>>> + *   Group attributes.
>>> + * @param actions
>>> + *   List of group miss actions.
>>> + * @param[out] error
>>> + *   Perform verbose error reporting if not NULL.
>>> + *   PMDs initialize this structure in case of error only.
>>> + *
>>> + * @return
>>> + *   0 on success, a negative errno value otherwise and rte_errno is set.
>>> + */
>>> +__rte_experimental
>>> +int
>>> +rte_flow_group_set_miss_actions(uint16_t port_id,
>>> +                             uint32_t group_id,
>>> +                             const struct rte_flow_group_attr *attr,
>>> +                             const struct rte_flow_action actions[],
>>> +                             struct rte_flow_error *error);
>>> +
>>> /**
>>>  * @warning
>>>  * @b EXPERIMENTAL: this API may change without prior notice.
>>> diff --git a/lib/ethdev/rte_flow_driver.h
>>> b/lib/ethdev/rte_flow_driver.h index f9fb01b8a2..3ced086c47 100644
>>> --- a/lib/ethdev/rte_flow_driver.h
>>> +++ b/lib/ethdev/rte_flow_driver.h
>>> @@ -227,6 +227,13 @@ struct rte_flow_ops {
>>>               (struct rte_eth_dev *dev,
>>>                struct rte_flow_template_table *template_table,
>>>                struct rte_flow_error *err);
>>> +     /** See rte_flow_group_set_miss_actions() */
>>> +     int (*group_set_miss_actions)
>>> +             (struct rte_eth_dev *dev,
>>> +              uint32_t group_id,
>>> +              const struct rte_flow_group_attr *attr,
>>> +              const struct rte_flow_action actions[],
>>> +              struct rte_flow_error *err);
>>>       /** See rte_flow_async_create() */
>>>       struct rte_flow *(*async_create)
>>>               (struct rte_eth_dev *dev, diff --git
>>> a/lib/ethdev/version.map b/lib/ethdev/version.map index
>>> fc492ee839..bdd41ecb5e 100644
>>> --- a/lib/ethdev/version.map
>>> +++ b/lib/ethdev/version.map
>>> @@ -312,6 +312,9 @@ EXPERIMENTAL {
>>>       rte_flow_async_action_list_handle_query_update;
>>>       rte_flow_async_actions_update;
>>>       rte_flow_restore_info_dynflag;
>>> +
>>> +     # added in 23.11
>>> +     rte_flow_group_set_miss_actions;
>>> };
>>>
>>> INTERNAL {
>>> --
>>> 2.34.1
>>>
>>>
>
  

Hi Ivan and Tomer,

The RFC looks good to me, some comments inline.

> -----Original Message-----
> From: Ivan Malov <ivan.malov@arknetworks.am>
> Sent: Tuesday, August 8, 2023 7:15 PM
> 
> Hi Tomer,
> 
> OK, let's see what others will say.
> Minor comment below.
> 
> On Tue, 8 Aug 2023, Tomer Shmilovich wrote:
> 
> > Hi Ivan, please see inline comments.
> >
> > Thanks, Tomer
> >
> >> -----Original Message-----
> >> From: Ivan Malov <ivan.malov@arknetworks.am>
> >> Sent: Tuesday, 8 August 2023 2:03
> >> To: Tomer Shmilovich <tshmilovich@nvidia.com>
> >> Cc: Ori Kam <orika@nvidia.com>; NBU-Contact-Thomas Monjalon
> >> (EXTERNAL) <thomas@monjalon.net>; Ferruh Yigit <ferruh.yigit@amd.com>;
> >> Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>; dev@dpdk.org
> >> Subject: Re: [RFC] ethdev: add group set miss actions API
> >>
> >> External email: Use caution opening links or attachments
> >>
> >>
> >> Hi Tomer,
> >>
> >> This is a good proposal overall, but it's a bit questionable with regard to the
> >> transfer domain and precisely group 0.
> >>
> >> Say, the user starts a DPDK application and plugs a PF to it, also plugs a
> >> representor for a VF. If I'm not mistaken, in this case, default behaviour is
> >> hardly "undefined" for group 0. Packets sent by a VF are expected to reach
> the
> >> representor (and vice versa). Also, packets arriving on the physical port are
> >> expected to hit the PF port, ethdev 0, for instance.
> >>
> >
> > True.
> >
> >> Does this new API intend to re-define this? I mean, if the application fails to
> set
> >> the default action for group 0 (ENOTSUP), shall it assume that the behaviour
> >> will be as described above? And if it succeeds, then assume that such implicit
> >> interconnections cease functioning?
> >>
> >> So, this API is something like "isolated mode"
> >> in the case of non-transfer API, but allowing to choose a "default" action
> >> rather than DROP?
> >
> > You have a point. These are the default "miss actions" for all use cases as I
> understand them:
> > Transfer - miss group 0 - goes to the other side of the connection: rep --> VF,
> VF --> rep.
> > Transfer - miss group > 0 - goes to E-switch manager (proxy port).
> > Ingress - miss group 0 - goes to application when expected (i.e. promiscuous
> mode); otherwise drop/go to kernel in case of bifurcated driver.
> > Ingress - miss group > 0 - drop.
> > Egress - miss any group - goes to wire.
> >
> > I suggest documenting these default "miss actions", and have the new
> function update the miss actions for a given group.
> > If an application sets the group's miss actions as none (i.e. actions[0].type ==
> RTE_FLOW_ACTION_TYPE_END), the miss actions should be restored to the
> aforementioned default miss actions.
> 
> There is also an action "PASSTHRU". It can potentially be used with this
> meaning as "use implicit defaults", as "PASSTHRU" + "END".
> Or just "END", as you suggest. Perhaps others will
> take a look at this, too.
> 
> Thank you.
> 
According to my understanding "PASSTHRU" action
means that a packet after hitting a rule will continue in the same table trying to match lower priority/different rules.
"PASSTHRU" doesn't define what happens to flows that matched the first rule but didn't match
any other rule. 
Based on the above if we combine the two we can set all rules for example with the "PASSTHRU"
set the requested default action. This will result in all rules having the same terminating action.

Might be nice since using this you can change with one command all the terminating action for all flows.

Best,
Ori

[Snip]