Re: [dpdk-stable] [PATCH 19.11] net/mlx5: workaround drop action with old kernel

[Christian Ehrhardt <christian.ehrhardt@canonical.com>]

On Mon, Aug 16, 2021 at 3:52 AM Suanming Mou <suanmingm@nvidia.com> wrote:
>
> [ upstream commit 45633c460c223a67dd1a7cc084c3eceb5e17687c ]

Thanks, applied

> Currently, there are two types of drop action implementation
> in the PMD. One is the DR (Direct Rules) dummy placeholder drop
> action and another is the dedicated dummy queue drop action.
> When creates flow on the root table with DR drop action, the
> action will be converted to MLX5_IB_ATTR_CREATE_FLOW_FLAGS_DROP
> Verbs attribute in rdma-core.
>
> In some inbox systems, MLX5_IB_ATTR_CREATE_FLOW_FLAGS_DROP Verbs
> attribute may not be supported in the kernel driver. Create flow
> with drop action on the root table will be failed as it is not
> supported. In this case, the dummy queue drop action should be
> used instead of DR dummy placeholder drop action.
>
> This commit adds the DR drop action support detect on the root
> table. If MLX5_IB_ATTR_CREATE_FLOW_FLAGS_DROP Verbs is not
> supported in the system, a dummy queue will be used as drop
> action.
>
> Fixes: da845ae9d7c1 ("net/mlx5: fix drop action for Direct Rules/Verbs")
> Cc: stable@dpdk.org
>
> Signed-off-by: Suanming Mou <suanmingm@nvidia.com>
> Acked-by: Matan Azrad <matan@nvidia.com>
> ---
>  drivers/net/mlx5/mlx5.c         | 28 +++++++++++++
>  drivers/net/mlx5/mlx5.h         |  1 +
>  drivers/net/mlx5/mlx5_flow.h    |  1 +
>  drivers/net/mlx5/mlx5_flow_dv.c | 72 +++++++++++++++++++++++++++++++--
>  4 files changed, 99 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
> index 3208b2eda7..ee019d6db1 100644
> --- a/drivers/net/mlx5/mlx5.c
> +++ b/drivers/net/mlx5/mlx5.c
> @@ -2129,6 +2129,33 @@ mlx5_dev_check_sibling_config(struct mlx5_priv *priv,
>         }
>         return 0;
>  }
> +
> +/**
> + * DR flow drop action support detect.
> + *
> + * @param dev
> + *   Pointer to rte_eth_dev structure.
> + *
> + */
> +static void
> +mlx5_flow_drop_action_config(struct rte_eth_dev *dev __rte_unused)
> +{
> +#ifdef HAVE_MLX5DV_DR
> +       struct mlx5_priv *priv = dev->data->dev_private;
> +
> +       if (!priv->config.dv_flow_en || !priv->sh->dr_drop_action)
> +               return;
> +       /**
> +        * DR supports drop action placeholder when it is supported;
> +        * otherwise, use the queue drop action.
> +        */
> +       if (mlx5_flow_discover_dr_action_support(dev))
> +               priv->root_verbs_drop_action = 1;
> +       else
> +               priv->root_verbs_drop_action = 0;
> +#endif
> +}
> +
>  /**
>   * Spawn an Ethernet device from Verbs information.
>   *
> @@ -2844,6 +2871,7 @@ mlx5_dev_spawn(struct rte_device *dpdk_dev,
>                         goto error;
>                 }
>         }
> +       mlx5_flow_drop_action_config(eth_dev);
>         return eth_dev;
>  error:
>         if (priv) {
> diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
> index 70509959fe..b2dc9e291b 100644
> --- a/drivers/net/mlx5/mlx5.h
> +++ b/drivers/net/mlx5/mlx5.h
> @@ -736,6 +736,7 @@ struct mlx5_priv {
>         unsigned int counter_fallback:1; /* Use counter fallback management. */
>         unsigned int mtr_en:1; /* Whether support meter. */
>         unsigned int mtr_reg_share:1; /* Whether support meter REG_C share. */
> +       unsigned int root_verbs_drop_action; /* Root uses verbs drop action. */
>         uint16_t domain_id; /* Switch domain identifier. */
>         uint16_t vport_id; /* Associated VF vport index (if any). */
>         uint32_t vport_meta_tag; /* Used for vport index match ove VF LAG. */
> diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
> index 4300e62fad..caf6afd4d8 100644
> --- a/drivers/net/mlx5/mlx5_flow.h
> +++ b/drivers/net/mlx5/mlx5_flow.h
> @@ -893,4 +893,5 @@ int mlx5_flow_destroy_policer_rules(struct rte_eth_dev *dev,
>                                     const struct rte_flow_attr *attr);
>  int mlx5_flow_meter_flush(struct rte_eth_dev *dev,
>                           struct rte_mtr_error *error);
> +int mlx5_flow_discover_dr_action_support(struct rte_eth_dev *dev);
>  #endif /* RTE_PMD_MLX5_FLOW_H_ */
> diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
> index a021ac9d20..22649cd79b 100644
> --- a/drivers/net/mlx5/mlx5_flow_dv.c
> +++ b/drivers/net/mlx5/mlx5_flow_dv.c
> @@ -7920,12 +7920,14 @@ __flow_dv_apply(struct rte_eth_dev *dev, struct rte_flow *flow,
>                         if (dev_flow->transfer) {
>                                 assert(priv->sh->dr_drop_action);
>                                 dv->actions[n++] = priv->sh->dr_drop_action;
> -                       } else {
>  #ifdef HAVE_MLX5DV_DR
> +                       } else if (dev_flow->group ||
> +                                  !priv->root_verbs_drop_action) {
>                                 /* DR supports drop action placeholder. */
>                                 assert(priv->sh->dr_drop_action);
>                                 dv->actions[n++] = priv->sh->dr_drop_action;
> -#else
> +#endif
> +                       } else {
>                                 /* For DV we use the explicit drop queue. */
>                                 dv->hrxq = mlx5_hrxq_drop_new(dev);
>                                 if (!dv->hrxq) {
> @@ -7937,7 +7939,6 @@ __flow_dv_apply(struct rte_eth_dev *dev, struct rte_flow *flow,
>                                         goto error;
>                                 }
>                                 dv->actions[n++] = dv->hrxq->action;
> -#endif
>                         }
>                 } else if (dev_flow->actions &
>                            (MLX5_FLOW_ACTION_QUEUE | MLX5_FLOW_ACTION_RSS)) {
> @@ -8373,6 +8374,71 @@ flow_dv_query(struct rte_eth_dev *dev,
>         return ret;
>  }
>
> +/**
> + * Check whether the DR drop action is supported on the root table or not.
> + *
> + * Create a simple flow with DR drop action on root table to validate
> + * if DR drop action on root table is supported or not.
> + *
> + * @param[in] dev
> + *   Pointer to rte_eth_dev structure.
> + *
> + * @return
> + *   0 on success, a negative errno value otherwise and rte_errno is set.
> + */
> +int
> +mlx5_flow_discover_dr_action_support(struct rte_eth_dev *dev)
> +{
> +       struct mlx5_priv *priv = dev->data->dev_private;
> +       struct mlx5_ibv_shared *sh = priv->sh;
> +       struct mlx5_flow_dv_match_params mask = {
> +               .size = sizeof(mask.buf),
> +       };
> +       struct mlx5_flow_dv_match_params value = {
> +               .size = sizeof(value.buf),
> +       };
> +       struct mlx5dv_flow_matcher_attr dv_attr = {
> +               .type = IBV_FLOW_ATTR_NORMAL,
> +               .priority = 0,
> +               .match_criteria_enable = 0,
> +               .match_mask = (void *)&mask,
> +       };
> +       struct mlx5_flow_tbl_resource *tbl = NULL;
> +       void *matcher = NULL;
> +       void *flow = NULL;
> +       int ret = -1;
> +
> +       tbl = flow_dv_tbl_resource_get(dev, 0, 0, 0, NULL);
> +       if (!tbl)
> +               goto err;
> +       matcher = mlx5_glue->dv_create_flow_matcher(sh->ctx, &dv_attr,
> +                                                   tbl->obj);
> +       if (!matcher)
> +               goto err;
> +       flow = mlx5_glue->dv_create_flow(matcher, (void *)&value, 1,
> +                                        &sh->dr_drop_action);
> +err:
> +       /*
> +        * If DR drop action is not supported on root table, flow create will
> +        * be failed with EOPNOTSUPP or EPROTONOSUPPORT.
> +        */
> +       if (!flow) {
> +               if (matcher &&
> +                   (errno == EPROTONOSUPPORT || errno == EOPNOTSUPP))
> +                       DRV_LOG(INFO, "DR drop action is not supported in root table.");
> +               else
> +                       DRV_LOG(ERR, "Unexpected error in DR drop action support detection");
> +               ret = -1;
> +       } else {
> +               claim_zero(mlx5_glue->dv_destroy_flow(flow));
> +       }
> +       if (matcher)
> +               claim_zero(mlx5_glue->dv_destroy_flow_matcher(matcher));
> +       if (tbl)
> +               flow_dv_tbl_resource_release(dev, tbl);
> +       return ret;
> +}
> +
>  /**
>   * Destroy the meter table set.
>   * Lock free, (mutex should be acquired by caller).
> --
> 2.25.1
>


-- 
Christian Ehrhardt
Staff Engineer, Ubuntu Server
Canonical Ltd