[prev in list] [next in list] [prev in thread] [next in thread] 

List:       dpdk-dev
Subject:    Re: [dpdk-dev] [PATCH 6/8] net/mlx5: update translate function for sample action
From:       Ori Kam <orika () mellanox ! com>
Date:       2020-06-30 19:54:32
Message-ID: AM6PR05MB51760AD3594935A615039445DB6F0 () AM6PR05MB5176 ! eurprd05 ! prod ! outlook ! com
[Download RAW message or body]

Hi Jiawei,
PSB,

Thanks,
Ori

> -----Original Message-----
> From: Jiawei Wang <jiaweiw@mellanox.com>
> Sent: Thursday, June 25, 2020 7:26 PM
> Subject: [PATCH 6/8] net/mlx5: update translate function for sample action
> 
> Translate the attribute of sample action that include sample ratio
> and sub actions list, then create the sample DR action.
> 
> Signed-off-by: Jiawei Wang <jiaweiw@mellanox.com>
> ---
>  drivers/net/mlx5/mlx5_flow.c    |  16 +-
>  drivers/net/mlx5/mlx5_flow.h    |  14 +-
>  drivers/net/mlx5/mlx5_flow_dv.c | 502
> +++++++++++++++++++++++++++++++++++++++-
>  3 files changed, 511 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
> index 7c65a9a..73ef290 100644
> --- a/drivers/net/mlx5/mlx5_flow.c
> +++ b/drivers/net/mlx5/mlx5_flow.c
> @@ -4569,10 +4569,14 @@ uint32_t mlx5_flow_adjust_priority(struct
> rte_eth_dev *dev, int32_t priority,
>  	int hairpin_flow;
>  	uint32_t hairpin_id = 0;
>  	struct rte_flow_attr attr_tx = { .priority = 0 };
> +	struct rte_flow_attr attr_factor = {0};
>  	int ret;
> 
> -	hairpin_flow = flow_check_hairpin_split(dev, attr, actions);
> -	ret = flow_drv_validate(dev, attr, items, p_actions_rx,
> +	memcpy((void *)&attr_factor, (const void *)attr, sizeof(*attr));
> +	if (external)
> +		attr_factor.group *= MLX5_FLOW_TABLE_FACTOR;
> +	hairpin_flow = flow_check_hairpin_split(dev, &attr_factor, actions);
> +	ret = flow_drv_validate(dev, &attr_factor, items, p_actions_rx,
>  				external, hairpin_flow, error);
>  	if (ret < 0)
>  		return 0;
> @@ -4591,7 +4595,7 @@ uint32_t mlx5_flow_adjust_priority(struct
> rte_eth_dev *dev, int32_t priority,
>  		rte_errno = ENOMEM;
>  		goto error_before_flow;
>  	}
> -	flow->drv_type = flow_get_drv_type(dev, attr);
> +	flow->drv_type = flow_get_drv_type(dev, &attr_factor);
>  	if (hairpin_id != 0)
>  		flow->hairpin_flow_id = hairpin_id;
>  	MLX5_ASSERT(flow->drv_type > MLX5_FLOW_TYPE_MIN &&
> @@ -4637,7 +4641,7 @@ uint32_t mlx5_flow_adjust_priority(struct
> rte_eth_dev *dev, int32_t priority,
>  		 * depending on configuration. In the simplest
>  		 * case it just creates unmodified original flow.
>  		 */
> -		ret = flow_create_split_outer(dev, flow, attr,
> +		ret = flow_create_split_outer(dev, flow, &attr_factor,
>  					      buf->entry[i].pattern,
>  					      p_actions_rx, external, idx,
>  					      error);
> @@ -4674,8 +4678,8 @@ uint32_t mlx5_flow_adjust_priority(struct
> rte_eth_dev *dev, int32_t priority,
>  	 * the egress Flows belong to the different device and
>  	 * copy table should be updated in peer NIC Rx domain.
>  	 */
> -	if (attr->ingress &&
> -	    (external || attr->group != MLX5_FLOW_MREG_CP_TABLE_GROUP))
> {
> +	if (attr_factor.ingress &&
> +	    (external || attr_factor.group !=
> MLX5_FLOW_MREG_CP_TABLE_GROUP)) {
>  		ret = flow_mreg_update_copy_table(dev, flow, actions, error);
>  		if (ret)
>  			goto error;
> diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
> index 941de5f..4163183 100644
> --- a/drivers/net/mlx5/mlx5_flow.h
> +++ b/drivers/net/mlx5/mlx5_flow.h
> @@ -369,6 +369,13 @@ enum mlx5_flow_fate_type {
>  	MLX5_FLOW_FATE_MAX,
>  };
> 
> +/*
> + * Max number of actions per DV flow.
> + * See CREATE_FLOW_MAX_FLOW_ACTIONS_SUPPORTED
> + * in rdma-core file providers/mlx5/verbs.c.
> + */
> +#define MLX5_DV_MAX_NUMBER_OF_ACTIONS 8
> +
>  /* Matcher PRM representation */
>  struct mlx5_flow_dv_match_params {
>  	size_t size;
> @@ -599,13 +606,6 @@ struct mlx5_flow_handle {
>  #define MLX5_FLOW_HANDLE_VERBS_SIZE (sizeof(struct mlx5_flow_handle))
>  #endif
> 
> -/*
> - * Max number of actions per DV flow.
> - * See CREATE_FLOW_MAX_FLOW_ACTIONS_SUPPORTED
> - * in rdma-core file providers/mlx5/verbs.c.
> - */
> -#define MLX5_DV_MAX_NUMBER_OF_ACTIONS 8
> -
>  /** Device flow structure only for DV flow creation. */
>  struct mlx5_flow_dv_workspace {
>  	uint32_t group; /**< The group index. */
> diff --git a/drivers/net/mlx5/mlx5_flow_dv.c
> b/drivers/net/mlx5/mlx5_flow_dv.c
> index 710c0f3..62a4a3b 100644
> --- a/drivers/net/mlx5/mlx5_flow_dv.c
> +++ b/drivers/net/mlx5/mlx5_flow_dv.c
> @@ -79,6 +79,10 @@
>  flow_dv_tbl_resource_release(struct rte_eth_dev *dev,
>  			     struct mlx5_flow_tbl_resource *tbl);
> 
> +static int
> +flow_dv_encap_decap_resource_release(struct rte_eth_dev *dev,
> +				     uint32_t encap_decap_idx);
> +
>  /**
>   * Initialize flow attributes structure according to flow items' types.
>   *
> @@ -7897,6 +7901,385 @@ struct field_modify_info modify_tcp[] = {
>  }
> 
>  /**
> + * Create an Rx Hash queue.
> + *
> + * @param dev
> + *   Pointer to Ethernet device.
> + * @param[in] dev_flow
> + *   Pointer to the mlx5_flow.
> + * @param[in] rss_desc
> + *   Pointer to the mlx5_flow_rss_desc.
> + * @param[in, out] hrxq_idx

I think this is only used as out.

> + *   Hash Rx queue index.
> + * @param[out] error
> + *   Pointer to error structure.
> + *
> + * @return
> + *   The Verbs/DevX object initialised, NULL otherwise and rte_errno is set.
> + */
> +static struct mlx5_hrxq *
> +flow_dv_handle_rx_queue(struct rte_eth_dev *dev,
> +			  struct mlx5_flow *dev_flow,
> +			  struct mlx5_flow_rss_desc *rss_desc,
> +			  uint32_t *hrxq_idx,
> +			  struct rte_flow_error *error)
> +{
> +	struct mlx5_priv *priv = dev->data->dev_private;
> +	struct mlx5_flow_handle *dh = dev_flow->handle;
> +	struct mlx5_hrxq *hrxq;
> +
> +	MLX5_ASSERT(rss_desc->queue_num);
> +	*hrxq_idx = mlx5_hrxq_get(dev, rss_desc->key,
> +				 MLX5_RSS_HASH_KEY_LEN,
> +				 dev_flow->hash_fields,
> +				 rss_desc->queue,
> +				 rss_desc->queue_num);
> +	if (!*hrxq_idx) {
> +		*hrxq_idx = mlx5_hrxq_new
> +				(dev, rss_desc->key,
> +				MLX5_RSS_HASH_KEY_LEN,
> +				dev_flow->hash_fields,
> +				rss_desc->queue,
> +				rss_desc->queue_num,
> +				!!(dh->layers &
> +				MLX5_FLOW_LAYER_TUNNEL));
> +	}
> +	hrxq = mlx5_ipool_get(priv->sh->ipool[MLX5_IPOOL_HRXQ],
> +			      *hrxq_idx);

Why do you need this line? You can compare the hrxq_idx to check for error.

> +	if (!hrxq) {
> +		rte_flow_error_set
> +			(error, rte_errno,
> +			 RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
> +			 "cannot get hash queue");
> +		goto error;
> +	}
> +	dh->rix_hrxq = *hrxq_idx;
> +	return hrxq;
> +error:
> +	/* hrxq is union, don't clear it if the flag is not set. */
> +	if (dh->rix_hrxq) {
> +		mlx5_hrxq_release(dev, dh->rix_hrxq);
> +		dh->rix_hrxq = 0;
> +	}
> +	return NULL;
> +}
> +


[snap...]
[prev in list] [next in list] [prev in thread] [next in thread]