Re: [dpdk-dev] [PATCH 1/2] [RFC]: ethdev: add pre-defined meter policy API

[Jerin Jacob <jerinjacobk@gmail.com>]

On Thu, Mar 18, 2021 at 2:28 PM Li Zhang <lizh@nvidia.com> wrote:
>
> Currently, the flow meter policy does not support multiple actions
> per color; also the allowed action types per color are very limited.
> In addition, the policy cannot be pre-defined.
>
> Due to the growing in flow actions offload abilities there is a potential
> for the user to use variety of actions per color differently.
> This new meter policy API comes to allow this potential in the most ethdev
> common way using rte_flow action definition.
> A list of rte_flow actions will be provided by the user per color
> in order to create a meter policy.
> In addition, the API forces to pre-define the policy before
> the meters creation in order to allow sharing of single policy
> with multiple meters efficiently.
>
> meter_policy_id is added into struct rte_mtr_params.
> So that it can get the policy during the meters creation.
>
> Policy id 0 is default policy. Action per color as below:
> green - no action, yellow - no action, red - drop
>
> Allow coloring the packet using a new rte_flow_action_color
> as could be done by the old policy API,
>
> The next API function were added:
> - rte_mtr_meter_policy_add
> - rte_mtr_meter_policy_delete
> - rte_mtr_meter_policy_update
> - rte_mtr_meter_policy_validate
> The next struct was changed:
> - rte_mtr_params
> - rte_mtr_capabilities
> The next API was deleted:
> - rte_mtr_policer_actions_update
>
> Signed-off-by: Li Zhang <lizh@nvidia.com>
> ---
>  lib/librte_ethdev/rte_flow.h       |  18 ++++
>  lib/librte_ethdev/rte_mtr.c        |  55 ++++++++--
>  lib/librte_ethdev/rte_mtr.h        | 166 ++++++++++++++++++++---------
>  lib/librte_ethdev/rte_mtr_driver.h |  45 ++++++--
>  4 files changed, 210 insertions(+), 74 deletions(-)
>
> diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
> index 669e677e91..5f38aa7fa4 100644
> --- a/lib/librte_ethdev/rte_flow.h
> +++ b/lib/librte_ethdev/rte_flow.h
> @@ -31,6 +31,7 @@
>  #include <rte_ecpri.h>
>  #include <rte_mbuf.h>
>  #include <rte_mbuf_dyn.h>
> +#include <rte_meter.h>
>
>  #ifdef __cplusplus
>  extern "C" {
> @@ -2236,6 +2237,13 @@ enum rte_flow_action_type {
>          * See struct rte_flow_action_modify_field.
>          */
>         RTE_FLOW_ACTION_TYPE_MODIFY_FIELD,
> +
> +       /**
> +        * Color the packet to reflect the meter color result.
> +        *
> +        * See struct rte_flow_action_color.
> +        */
> +       RTE_FLOW_ACTION_TYPE_COlOR,

Based on my understanding of this API,
1) Application creates the policy
2) Attachs the policy ID to meter object in params
If so, Why we need this new action?

>  };
>
>  /**
> @@ -2828,6 +2836,16 @@ struct rte_flow_action_set_dscp {
>   */
>  struct rte_flow_shared_action;
>
> +/**
> + * Meter policy add
> + *
> + * Create a new meter policy. The new policy
> + * is used to create single or multiple MTR objects.
> + *
> + * @param[in] port_id
> + *   The port identifier of the Ethernet device.
> + * @param[in] policy_id
> + *   Policy identifier for the new meter policy.
> + * @param[in] actions
> + *   Associated actions per color.
> + *   list NULL is legal and means no special action.
> + *   (list terminated by the END action).
> + * @param[out] error
> + *   Error details. Filled in only on error, when not NULL.
> + * @return
> + *   0 on success, non-zero error code otherwise.
> + */
> +__rte_experimental
> +int
> +rte_mtr_meter_policy_add(uint16_t port_id,


_create() may be better here instead of _add() as you have used _delete()

> +       uint32_t policy_id,
> +       const struct rte_flow_action *actions[RTE_COLORS],


1) Does this mean that MLX HW can support any rte_flow actions like,
if packet color is green do RTE_FLOW_ACTION_TYPE_SECURITY etc.


2) Is there any real-world use case other than using normal action
like pass or drop as it is used
in conjunction with meter object?


3) Marvell HW has the following policy actions
a) PASS
b) DROP
c) RED (Random early discard)

Both (a) and (c) are not in enumated as rte_flow_actions.

Should we take rte_flow_action or create meter-specific policy actions?