[dpdk-dev] [PATCH v2 8/9] net/mlx5: support shared age action

Matan Azrad matan at nvidia.com
Mon Nov 2 07:16:58 CET 2020


Hi Self note

From: Matan Azrad:
> Add support for rte_flow shared action API for ASO age action.
> 
> First step here to support validate, create, query and destroy.
> 
> The support is only for age ASO mode.
> 
> Signed-off-by: Matan Azrad <matan at nvidia.com>
> Acked-by: Dekel Peled <dekelp at nvidia.com>
> ---
>  drivers/net/mlx5/mlx5.h         |   1 +
>  drivers/net/mlx5/mlx5_defs.h    |   2 +-
>  drivers/net/mlx5/mlx5_flow.c    |  61 ++++++++++++++--
>  drivers/net/mlx5/mlx5_flow.h    |  11 +++
>  drivers/net/mlx5/mlx5_flow_dv.c | 155 +++++++++++++++++++++++++++++----
> -------
>  5 files changed, 181 insertions(+), 49 deletions(-)
> 
> diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h index
> 2bc47d8..a156e5c 100644
> --- a/drivers/net/mlx5/mlx5.h
> +++ b/drivers/net/mlx5/mlx5.h
> @@ -524,6 +524,7 @@ struct mlx5_aso_sq {  struct mlx5_aso_age_action {
>  	LIST_ENTRY(mlx5_aso_age_action) next;
>  	void *dr_action;
> +	uint32_t refcnt;
>  	/* Following fields relevant only when action is active. */
>  	uint16_t offset; /* Offset of ASO Flow Hit flag in DevX object. */
>  	struct mlx5_age_param age_params;
> diff --git a/drivers/net/mlx5/mlx5_defs.h b/drivers/net/mlx5/mlx5_defs.h index
> 2657081..4980352 100644
> --- a/drivers/net/mlx5/mlx5_defs.h
> +++ b/drivers/net/mlx5/mlx5_defs.h
> @@ -197,7 +197,7 @@
>  #define MLX5_HAIRPIN_JUMBO_LOG_SIZE (14 + 2)
> 
>  /* Maximum number of shared actions supported by rte_flow */ -#define
> MLX5_MAX_SHARED_ACTIONS 1
> +#define MLX5_MAX_SHARED_ACTIONS 2
> 
>  /* Definition of static_assert found in /usr/include/assert.h */  #ifndef
> HAVE_STATIC_ASSERT diff --git a/drivers/net/mlx5/mlx5_flow.c
> b/drivers/net/mlx5/mlx5_flow.c index b08ee30..c0a2a04 100644
> --- a/drivers/net/mlx5/mlx5_flow.c
> +++ b/drivers/net/mlx5/mlx5_flow.c
> @@ -3266,6 +3266,29 @@ struct mlx5_flow_tunnel_info {
>  	return NULL;
>  }
> 
> +/**
> + * Get ASO age action by index.
> + *
> + * @param[in] dev
> + *   Pointer to the Ethernet device structure.
> + * @param[in] age_idx
> + *   Index to the ASO age action.
> + *
> + * @return
> + *   The specified ASO age action.
> + */
> +struct mlx5_aso_age_action*
> +flow_aso_age_get_by_idx(struct rte_eth_dev *dev, uint32_t age_idx) {
> +	uint16_t pool_idx = age_idx & UINT16_MAX;
> +	uint16_t offset = (age_idx >> 16) & UINT16_MAX;
> +	struct mlx5_priv *priv = dev->data->dev_private;
> +	struct mlx5_aso_age_mng *mng = priv->sh->aso_age_mng;
> +	struct mlx5_aso_age_pool *pool = mng->pools[pool_idx];
> +
> +	return &pool->actions[offset - 1];
> +}
> +
>  /* maps shared action to translated non shared in some actions array */  struct
> mlx5_translated_shared_action {
>  	struct rte_flow_shared_action *action; /**< Shared action */ @@ -
> 3353,6 +3376,15 @@ struct mlx5_translated_shared_action {
>  			translated[shared->index].conf =
>  				&shared_rss->origin;
>  			break;
> +		case MLX5_SHARED_ACTION_TYPE_AGE:
> +			if (priv->sh->flow_hit_aso_en) {
> +				translated[shared->index].type =

In order not get a warning in compilation for some gcc versions need to add this conversion to the enum below:
(enum rte_flow_action_type)


Raslan, let me know if you need more version for all the series for this, or you can do it in integration time....

> +
> MLX5_RTE_FLOW_ACTION_TYPE_AGE;
> +				translated[shared->index].conf =
> +							 (void *)(uintptr_t)idx;
> +				break;
> +			}
> +			/* Fall-through */
>  		default:
>  			mlx5_free(translated);
>  			return rte_flow_error_set
> @@ -7273,6 +7305,25 @@ struct mlx5_meter_domains_infos *
>  	return fops->action_update(dev, action, action_conf, error);  }
> 
> +/* Wrapper for driver action_destroy op callback */ static int
> +flow_drv_action_query(struct rte_eth_dev *dev,
> +		      const struct rte_flow_shared_action *action,
> +		      void *data,
> +		      const struct mlx5_flow_driver_ops *fops,
> +		      struct rte_flow_error *error)
> +{
> +	static const char err_msg[] = "shared action query unsupported";
> +
> +	if (!fops->action_query) {
> +		DRV_LOG(ERR, "port %u %s.", dev->data->port_id, err_msg);
> +		rte_flow_error_set(error, ENOTSUP,
> RTE_FLOW_ERROR_TYPE_ACTION,
> +				   NULL, err_msg);
> +		return -rte_errno;
> +	}
> +	return fops->action_query(dev, action, data, error); }
> +
>  /**
>   * Create shared action for reuse in multiple flow rules.
>   *
> @@ -7375,11 +7426,11 @@ struct mlx5_meter_domains_infos *
>  			 void *data,
>  			 struct rte_flow_error *error)
>  {
> -	(void)dev;
> -	(void)action;
> -	(void)data;
> -	return rte_flow_error_set(error, ENOTSUP,
> RTE_FLOW_ERROR_TYPE_ACTION,
> -				  NULL, "action type query not supported");
> +	struct rte_flow_attr attr = { .transfer = 0 };
> +	const struct mlx5_flow_driver_ops *fops =
> +			flow_get_drv_ops(flow_get_drv_type(dev, &attr));
> +
> +	return flow_drv_action_query(dev, action, data, fops, error);
>  }
> 
>  /**
> diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
> index b77df50..58185fb 100644
> --- a/drivers/net/mlx5/mlx5_flow.h
> +++ b/drivers/net/mlx5/mlx5_flow.h
> @@ -36,12 +36,14 @@ enum mlx5_rte_flow_action_type {
>  	MLX5_RTE_FLOW_ACTION_TYPE_COPY_MREG,
>  	MLX5_RTE_FLOW_ACTION_TYPE_DEFAULT_MISS,
>  	MLX5_RTE_FLOW_ACTION_TYPE_TUNNEL_SET,
> +	MLX5_RTE_FLOW_ACTION_TYPE_AGE,
>  };
> 
>  #define MLX5_SHARED_ACTION_TYPE_OFFSET 30
> 
>  enum {
>  	MLX5_SHARED_ACTION_TYPE_RSS,
> +	MLX5_SHARED_ACTION_TYPE_AGE,
>  };
> 
>  /* Matches on selected register. */
> @@ -1165,10 +1167,16 @@ typedef int (*mlx5_flow_action_update_t)
>  			 struct rte_flow_shared_action *action,
>  			 const void *action_conf,
>  			 struct rte_flow_error *error);
> +typedef int (*mlx5_flow_action_query_t)
> +			(struct rte_eth_dev *dev,
> +			 const struct rte_flow_shared_action *action,
> +			 void *data,
> +			 struct rte_flow_error *error);
>  typedef int (*mlx5_flow_sync_domain_t)
>  			(struct rte_eth_dev *dev,
>  			 uint32_t domains,
>  			 uint32_t flags);
> +
>  struct mlx5_flow_driver_ops {
>  	mlx5_flow_validate_t validate;
>  	mlx5_flow_prepare_t prepare;
> @@ -1189,6 +1197,7 @@ struct mlx5_flow_driver_ops {
>  	mlx5_flow_action_create_t action_create;
>  	mlx5_flow_action_destroy_t action_destroy;
>  	mlx5_flow_action_update_t action_update;
> +	mlx5_flow_action_query_t action_query;
>  	mlx5_flow_sync_domain_t sync_domain;
>  };
> 
> @@ -1457,4 +1466,6 @@ struct mlx5_cache_entry
> *flow_dv_dest_array_create_cb
>  				 struct mlx5_cache_entry *entry, void
> *cb_ctx);  void flow_dv_dest_array_remove_cb(struct mlx5_cache_list *list,
>  				  struct mlx5_cache_entry *entry);
> +struct mlx5_aso_age_action *flow_aso_age_get_by_idx(struct rte_eth_dev
> *dev,
> +						    uint32_t age_idx);
>  #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 be93ba9..d60626c 100644
> --- a/drivers/net/mlx5/mlx5_flow_dv.c
> +++ b/drivers/net/mlx5/mlx5_flow_dv.c
> @@ -5926,6 +5926,10 @@ struct mlx5_hlist_entry *
>  			/* Meter action will add one more TAG action. */
>  			rw_act_num += MLX5_ACT_NUM_SET_TAG;
>  			break;
> +		case MLX5_RTE_FLOW_ACTION_TYPE_AGE:
> +			action_flags |= MLX5_FLOW_ACTION_AGE;
> +			++actions_n;
> +			break;
>  		case RTE_FLOW_ACTION_TYPE_AGE:
>  			ret = flow_dv_validate_action_age(action_flags,
>  							  actions, dev,
> @@ -9241,29 +9245,6 @@ struct mlx5_cache_entry *  }
> 
>  /**
> - * Get ASO age action by index.
> - *
> - * @param[in] dev
> - *   Pointer to the Ethernet device structure.
> - * @param[in] age_idx
> - *   Index to the ASO age action.
> - *
> - * @return
> - *   The specified ASO age action.
> - */
> -static struct mlx5_aso_age_action*
> -flow_dv_aso_age_get_by_idx(struct rte_eth_dev *dev, uint32_t age_idx) -{
> -	uint16_t pool_idx = age_idx & UINT16_MAX;
> -	uint16_t offset = (age_idx >> 16) & UINT16_MAX;
> -	struct mlx5_priv *priv = dev->data->dev_private;
> -	struct mlx5_aso_age_mng *mng = priv->sh->aso_age_mng;
> -	struct mlx5_aso_age_pool *pool = mng->pools[pool_idx];
> -
> -	return &pool->actions[offset - 1];
> -}
> -
> -/**
>   * Remove an ASO age action from age actions list.
>   *
>   * @param[in] dev
> @@ -9295,18 +9276,35 @@ struct mlx5_cache_entry *
>  	}
>  }
> 
> -static void
> +/**
> + * Release an ASO age action.
> + *
> + * @param[in] dev
> + *   Pointer to the Ethernet device structure.
> + * @param[in] age_idx
> + *   Index of ASO age action to release.
> + * @param[in] flow
> + *   True if the release operation is during flow destroy operation.
> + *   False if the release operation is during action destroy operation.
> + *
> + * @return
> + *   0 when age action was removed, otherwise the number of references.
> + */
> +static int
>  flow_dv_aso_age_release(struct rte_eth_dev *dev, uint32_t age_idx)  {
>  	struct mlx5_priv *priv = dev->data->dev_private;
>  	struct mlx5_aso_age_mng *mng = priv->sh->aso_age_mng;
> -	struct mlx5_aso_age_action *age = flow_dv_aso_age_get_by_idx(dev,
> -								     age_idx);
> +	struct mlx5_aso_age_action *age = flow_aso_age_get_by_idx(dev,
> age_idx);
> +	uint32_t ret = __atomic_sub_fetch(&age->refcnt, 1,
> __ATOMIC_RELAXED);
> 
> -	flow_dv_aso_age_remove_from_age(dev, age);
> -	rte_spinlock_lock(&mng->free_sl);
> -	LIST_INSERT_HEAD(&mng->free, age, next);
> -	rte_spinlock_unlock(&mng->free_sl);
> +	if (!ret) {
> +		flow_dv_aso_age_remove_from_age(dev, age);
> +		rte_spinlock_lock(&mng->free_sl);
> +		LIST_INSERT_HEAD(&mng->free, age, next);
> +		rte_spinlock_unlock(&mng->free_sl);
> +	}
> +	return ret;
>  }
> 
>  /**
> @@ -9450,6 +9448,7 @@ struct mlx5_cache_entry *
>  			return 0; /* 0 is an error.*/
>  		}
>  	}
> +	__atomic_store_n(&age_free->refcnt, 1, __ATOMIC_RELAXED);
>  	return pool->index | ((age_free->offset + 1) << 16);  }
> 
> @@ -9469,12 +9468,12 @@ struct mlx5_cache_entry *
>  				 const struct rte_flow_action_age *age)  {
>  	uint32_t age_idx = 0;
> -	struct mlx5_aso_age_action *aso_age = NULL;
> +	struct mlx5_aso_age_action *aso_age;
> 
>  	age_idx = flow_dv_aso_age_alloc(dev);
>  	if (!age_idx)
>  		return 0;
> -	aso_age = flow_dv_aso_age_get_by_idx(dev, age_idx);
> +	aso_age = flow_aso_age_get_by_idx(dev, age_idx);
>  	aso_age->age_params.context = age->context;
>  	aso_age->age_params.timeout = age->timeout;
>  	aso_age->age_params.port_id = dev->data->port_id; @@ -9638,6
> +9637,7 @@ struct mlx5_cache_entry *
>  		const uint8_t *rss_key;
>  		const struct rte_flow_action_meter *mtr;
>  		struct mlx5_flow_tbl_resource *tbl;
> +		struct mlx5_aso_age_action *age_act;
>  		uint32_t port_id = 0;
>  		struct mlx5_flow_dv_port_id_action_resource
> port_id_resource;
>  		int action_type = actions->type;
> @@ -9774,6 +9774,14 @@ struct mlx5_cache_entry *
>  			action_flags |= MLX5_FLOW_ACTION_RSS;
>  			dev_flow->handle->fate_action =
> MLX5_FLOW_FATE_QUEUE;
>  			break;
> +		case MLX5_RTE_FLOW_ACTION_TYPE_AGE:
> +			flow->age = (uint32_t)(uintptr_t)(action->conf);
> +			age_act = flow_aso_age_get_by_idx(dev, flow->age);
> +			__atomic_fetch_add(&age_act->refcnt, 1,
> +					   __ATOMIC_RELAXED);
> +			dev_flow->dv.actions[actions_n++] = age_act-
> >dr_action;
> +			action_flags |= MLX5_FLOW_ACTION_AGE;
> +			break;
>  		case RTE_FLOW_ACTION_TYPE_AGE:
>  			if (priv->sh->flow_hit_aso_en) {
>  				flow->age =
> flow_dv_translate_create_aso_age @@ -9785,7 +9793,7 @@ struct
> mlx5_cache_entry *
>  						 NULL,
>  						 "can't create age action");
>  				dev_flow->dv.actions[actions_n++] =
> -					  (flow_dv_aso_age_get_by_idx
> +					  (flow_aso_age_get_by_idx
>  						(dev, flow->age))->dr_action;
>  				action_flags |= MLX5_FLOW_ACTION_AGE;
>  				break;
> @@ -11441,6 +11449,19 @@ struct mlx5_cache_entry *
>  		idx = (MLX5_SHARED_ACTION_TYPE_RSS <<
>  		       MLX5_SHARED_ACTION_TYPE_OFFSET) | ret;
>  		break;
> +	case RTE_FLOW_ACTION_TYPE_AGE:
> +		ret = flow_dv_translate_create_aso_age(dev, action->conf);
> +		idx = (MLX5_SHARED_ACTION_TYPE_AGE <<
> +		       MLX5_SHARED_ACTION_TYPE_OFFSET) | ret;
> +		if (ret) {
> +			struct mlx5_aso_age_action *aso_age =
> +					      flow_aso_age_get_by_idx(dev,
> ret);
> +
> +			if (!aso_age->age_params.context)
> +				aso_age->age_params.context =
> +							 (void *)(uintptr_t)idx;
> +		}
> +		break;
>  	default:
>  		rte_flow_error_set(err, ENOTSUP,
> RTE_FLOW_ERROR_TYPE_ACTION,
>  				   NULL, "action type not supported"); @@ -
> 11478,17 +11499,23 @@ struct mlx5_cache_entry *
> 
>  	switch (type) {
>  	case MLX5_SHARED_ACTION_TYPE_RSS:
> -		ret = __flow_dv_action_rss_release(dev, idx, error);
> -		break;
> +		return __flow_dv_action_rss_release(dev, idx, error);
> +	case MLX5_SHARED_ACTION_TYPE_AGE:
> +		ret = flow_dv_aso_age_release(dev, idx);
> +		if (ret)
> +			/*
> +			 * In this case, the last flow has a reference will
> +			 * actually release the age action.
> +			 */
> +			DRV_LOG(DEBUG, "Shared age action %" PRIu32 "
> was"
> +				" released with references %d.", idx, ret);
> +		return 0;
>  	default:
>  		return rte_flow_error_set(error, ENOTSUP,
>  					  RTE_FLOW_ERROR_TYPE_ACTION,
>  					  NULL,
>  					  "action type not supported");
>  	}
> -	if (ret)
> -		return ret;
> -	return 0;
>  }
> 
>  /**
> @@ -11609,9 +11636,41 @@ struct mlx5_cache_entry *
>  		return rte_flow_error_set(err, ENOTSUP,
>  					  RTE_FLOW_ERROR_TYPE_ACTION,
>  					  NULL,
> -					  "action type not supported");
> +					  "action type update not supported");
> +	}
> +}
> +
> +static int
> +flow_dv_action_query(struct rte_eth_dev *dev,
> +		     const struct rte_flow_shared_action *action, void *data,
> +		     struct rte_flow_error *error)
> +{
> +	struct mlx5_age_param *age_param;
> +	struct rte_flow_query_age *resp;
> +	uint32_t act_idx = (uint32_t)(uintptr_t)action;
> +	uint32_t type = act_idx >> MLX5_SHARED_ACTION_TYPE_OFFSET;
> +	uint32_t idx = act_idx & ((1u <<
> MLX5_SHARED_ACTION_TYPE_OFFSET) - 1);
> +
> +	switch (type) {
> +	case MLX5_SHARED_ACTION_TYPE_AGE:
> +		age_param = &flow_aso_age_get_by_idx(dev, idx)-
> >age_params;
> +		resp = data;
> +		resp->aged = __atomic_load_n(&age_param->state,
> +					      __ATOMIC_RELAXED) ==
> AGE_TMOUT ?
> +									  1 : 0;
> +		resp->sec_since_last_hit_valid = !resp->aged;
> +		if (resp->sec_since_last_hit_valid)
> +			resp->sec_since_last_hit = __atomic_load_n
> +			     (&age_param->sec_since_last_hit,
> __ATOMIC_RELAXED);
> +		return 0;
> +	default:
> +		return rte_flow_error_set(error, ENOTSUP,
> +					  RTE_FLOW_ERROR_TYPE_ACTION,
> +					  NULL,
> +					  "action type query not supported");
>  	}
>  }
> +
>  /**
>   * Query a dv flow  rule for its statistics via devx.
>   *
> @@ -11692,7 +11751,7 @@ struct mlx5_cache_entry *
> 
>  	if (flow->age) {
>  		struct mlx5_aso_age_action *act =
> -				     flow_dv_aso_age_get_by_idx(dev, flow-
> >age);
> +				     flow_aso_age_get_by_idx(dev, flow->age);
> 
>  		age_param = &act->age_params;
>  	} else if (flow->counter) {
> @@ -12402,14 +12461,23 @@ struct mlx5_cache_entry *
> flow_dv_action_validate(struct rte_eth_dev *dev,
>  			const struct rte_flow_shared_action_conf *conf,
>  			const struct rte_flow_action *action,
> -			struct rte_flow_error *error)
> +			struct rte_flow_error *err)
>  {
> +	struct mlx5_priv *priv = dev->data->dev_private;
> +
>  	RTE_SET_USED(conf);
>  	switch (action->type) {
>  	case RTE_FLOW_ACTION_TYPE_RSS:
> -		return mlx5_validate_action_rss(dev, action, error);
> +		return mlx5_validate_action_rss(dev, action, err);
> +	case RTE_FLOW_ACTION_TYPE_AGE:
> +		if (!priv->sh->aso_age_mng)
> +			return rte_flow_error_set(err, ENOTSUP,
> +
> 	RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
> +						NULL,
> +					     "shared age action not supported");
> +		return flow_dv_validate_action_age(0, action, dev, err);
>  	default:
> -		return rte_flow_error_set(error, ENOTSUP,
> +		return rte_flow_error_set(err, ENOTSUP,
>  					  RTE_FLOW_ERROR_TYPE_ACTION,
>  					  NULL,
>  					  "action type not supported");
> @@ -12461,6 +12529,7 @@ struct mlx5_cache_entry *
>  	.action_create = flow_dv_action_create,
>  	.action_destroy = flow_dv_action_destroy,
>  	.action_update = flow_dv_action_update,
> +	.action_query = flow_dv_action_query,
>  	.sync_domain = flow_dv_sync_domain,
>  };
> 
> --
> 1.8.3.1



More information about the dev mailing list