Re: [RFC] ethdev: add group set miss actions API

[Ivan Malov <ivan.malov@arknetworks.am>]

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
>
>