Re: [dpdk-dev] [PATCH v2 3/3] net/mlx5: add jump action support for NIC

[Slava Ovsiienko <viacheslavo@mellanox.com>]

> -----Original Message-----
> From: Ori Kam
> Sent: Thursday, March 28, 2019 18:33
> To: Matan Azrad <matan@mellanox.com>; Yongseok Koh
> <yskoh@mellanox.com>; Shahaf Shuler <shahafs@mellanox.com>
> Cc: dev@dpdk.org; Ori Kam <orika@mellanox.com>; Slava Ovsiienko
> <viacheslavo@mellanox.com>
> Subject: [PATCH v2 3/3] net/mlx5: add jump action support for NIC
> 
> When using Direct Rules we can add actions to jump between tables.
> This is extra useful since rule insertion rate is much higher on other tables
> compared to table zero.
> 
> if no group is selected the rule is added to group 0.
> 
> Signed-off-by: Ori Kam <orika@mellanox.com>
Acked-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>

> ---
>  drivers/net/mlx5/mlx5.h         |   6 +
>  drivers/net/mlx5/mlx5_flow.h    |  15 ++-
>  drivers/net/mlx5/mlx5_flow_dv.c | 278
> +++++++++++++++++++++++++++++++++++-----
>  drivers/net/mlx5/mlx5_glue.c    |  13 ++
>  drivers/net/mlx5/mlx5_glue.h    |   1 +
>  5 files changed, 282 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h index
> 73f6f0d..a3d5f8e 100644
> --- a/drivers/net/mlx5/mlx5.h
> +++ b/drivers/net/mlx5/mlx5.h
> @@ -279,6 +279,12 @@ struct mlx5_priv {
>  	LIST_HEAD(encap_decap, mlx5_flow_dv_encap_decap_resource)
> encaps_decaps;
>  	LIST_HEAD(modify_cmd, mlx5_flow_dv_modify_hdr_resource)
> modify_cmds;
>  	LIST_HEAD(tag, mlx5_flow_dv_tag_resource) tags;
> +	LIST_HEAD(jump, mlx5_flow_dv_jump_tbl_resource) jump_tbl;
> +	/* Pointer to next element. */
> +	rte_atomic32_t refcnt; /**< Reference counter. */
> +	struct ibv_flow_action *verbs_action;
> +	/**< Verbs modify header action object. */
> +	uint8_t ft_type; /**< Flow table type, Rx or Tx. */
>  	/* Tags resources cache. */
>  	uint32_t link_speed_capa; /* Link speed capabilities. */
>  	struct mlx5_xstats_ctrl xstats_ctrl; /* Extended stats control. */ diff --
> git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h index
> 8ba37a0..622e305 100644
> --- a/drivers/net/mlx5/mlx5_flow.h
> +++ b/drivers/net/mlx5/mlx5_flow.h
> @@ -115,7 +115,8 @@
>  #define MLX5_FLOW_ACTION_RAW_DECAP (1u << 27)
> 
>  #define MLX5_FLOW_FATE_ACTIONS \
> -	(MLX5_FLOW_ACTION_DROP | MLX5_FLOW_ACTION_QUEUE |
> MLX5_FLOW_ACTION_RSS)
> +	(MLX5_FLOW_ACTION_DROP | MLX5_FLOW_ACTION_QUEUE | \
> +	 MLX5_FLOW_ACTION_RSS | MLX5_FLOW_ACTION_JUMP)
> 
>  #define MLX5_FLOW_ENCAP_ACTIONS
> 	(MLX5_FLOW_ACTION_VXLAN_ENCAP | \
>  				 MLX5_FLOW_ACTION_NVGRE_ENCAP | \
> @@ -250,6 +251,16 @@ struct mlx5_flow_dv_modify_hdr_resource {
>  	/**< Modification actions. */
>  };
> 
> +/* Jump action resource structure. */
> +struct mlx5_flow_dv_jump_tbl_resource {
> +	LIST_ENTRY(mlx5_flow_dv_jump_tbl_resource) next;
> +	/* Pointer to next element. */
> +	rte_atomic32_t refcnt; /**< Reference counter. */
> +	void *action; /**< Pointer to the rdma core action. */
> +	uint8_t ft_type; /**< Flow table type, Rx or Tx. */
> +	struct mlx5_flow_tbl_resource *tbl; /**< The target table. */ };
> +
>  /*
>   * Max number of actions per DV flow.
>   * See CREATE_FLOW_MAX_FLOW_ACTIONS_SUPPORTED
> @@ -270,6 +281,8 @@ struct mlx5_flow_dv {
>  	struct mlx5_flow_dv_modify_hdr_resource *modify_hdr;
>  	/**< Pointer to modify header resource in cache. */
>  	struct ibv_flow *flow; /**< Installed flow. */
> +	struct mlx5_flow_dv_jump_tbl_resource *jump;
> +	/**< Pointer to the jump action resource. */
>  #ifdef HAVE_IBV_FLOW_DV_SUPPORT
>  	void *actions[MLX5_DV_MAX_NUMBER_OF_ACTIONS];
>  	/**< Action list. */
> diff --git a/drivers/net/mlx5/mlx5_flow_dv.c
> b/drivers/net/mlx5/mlx5_flow_dv.c index 6e4f6c4..6997dc6 100644
> --- a/drivers/net/mlx5/mlx5_flow_dv.c
> +++ b/drivers/net/mlx5/mlx5_flow_dv.c
> @@ -861,6 +861,68 @@ struct field_modify_info modify_tcp[] = {  }
> 
>  /**
> + * Find existing table jump resource or create and register a new one.
> + *
> + * @param dev[in, out]
> + *   Pointer to rte_eth_dev structure.
> + * @param[in, out] resource
> + *   Pointer to jump table resource.
> + * @parm[in, out] dev_flow
> + *   Pointer to the dev_flow.
> + * @param[out] error
> + *   pointer to error structure.
> + *
> + * @return
> + *   0 on success otherwise -errno and errno is set.
> + */
> +static int
> +flow_dv_jump_tbl_resource_register
> +			(struct rte_eth_dev *dev,
> +			 struct mlx5_flow_dv_jump_tbl_resource *resource,
> +			 struct mlx5_flow *dev_flow,
> +			 struct rte_flow_error *error)
> +{
> +	struct mlx5_priv *priv = dev->data->dev_private;
> +	struct mlx5_flow_dv_jump_tbl_resource *cache_resource;
> +
> +	/* Lookup a matching resource from cache. */
> +	LIST_FOREACH(cache_resource, &priv->jump_tbl, next) {
> +		if (resource->tbl == cache_resource->tbl) {
> +			DRV_LOG(DEBUG, "jump table resource resource %p:
> refcnt %d++",
> +				(void *)cache_resource,
> +				rte_atomic32_read(&cache_resource-
> >refcnt));
> +			rte_atomic32_inc(&cache_resource->refcnt);
> +			dev_flow->dv.jump = cache_resource;
> +			return 0;
> +		}
> +	}
> +	/* Register new jump table resource. */
> +	cache_resource = rte_calloc(__func__, 1, sizeof(*cache_resource), 0);
> +	if (!cache_resource)
> +		return rte_flow_error_set(error, ENOMEM,
> +
> RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
> +					  "cannot allocate resource
> memory");
> +	*cache_resource = *resource;
> +	cache_resource->action =
> +		mlx5_glue->dr_create_flow_action_dest_flow_tbl
> +		(resource->tbl->obj);
> +	if (!cache_resource->action) {
> +		rte_free(cache_resource);
> +		return rte_flow_error_set(error, ENOMEM,
> +
> RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
> +					  NULL, "cannot create action");
> +	}
> +	rte_atomic32_init(&cache_resource->refcnt);
> +	rte_atomic32_inc(&cache_resource->refcnt);
> +	LIST_INSERT_HEAD(&priv->jump_tbl, cache_resource, next);
> +	dev_flow->dv.jump = cache_resource;
> +	DRV_LOG(DEBUG, "new jump table  resource %p: refcnt %d++",
> +		(void *)cache_resource,
> +		rte_atomic32_read(&cache_resource->refcnt));
> +	return 0;
> +}
> +
> +/**
>   * Get the size of specific rte_flow_item_type
>   *
>   * @param[in] item_type
> @@ -1423,6 +1485,37 @@ struct field_modify_info modify_tcp[] = {  }
> 
>  /**
> + * Validate jump action.
> + *
> + * @param[in] action
> + *   Pointer to the modify action.
> + * @param[in] group
> + *   The group of the current flow.
> + * @param[out] error
> + *   Pointer to error structure.
> + *
> + * @return
> + *   0 on success, a negative errno value otherwise and rte_errno is set.
> + */
> +static int
> +flow_dv_validate_action_jump(const struct rte_flow_action *action,
> +			     uint32_t group,
> +			     struct rte_flow_error *error)
> +{
> +	if (action->type != RTE_FLOW_ACTION_TYPE_JUMP && !action->conf)
> +		return rte_flow_error_set(error, EINVAL,
> +
> RTE_FLOW_ERROR_TYPE_ACTION_CONF,
> +					  NULL, "action configuration not
> set");
> +	if (group >= ((const struct rte_flow_action_jump *)action->conf)-
> >group)
> +		return rte_flow_error_set(error, EINVAL,
> +					  RTE_FLOW_ERROR_TYPE_ACTION,
> NULL,
> +					  "target group must be higher then"
> +					  " the current flow group");
> +	return 0;
> +}
> +
> +
> +/**
>   * Find existing modify-header resource or create and register a new one.
>   *
>   * @param dev[in, out]
> @@ -1605,7 +1698,7 @@ struct field_modify_info modify_tcp[] = {
>  	struct mlx5_priv *priv = dev->data->dev_private;
>  	uint32_t priority_max = priv->config.flow_prio - 1;
> 
> -#ifdef HAVE_MLX5DV_DR
> +#ifndef HAVE_MLX5DV_DR
>  	if (attributes->group)
>  		return rte_flow_error_set(error, ENOTSUP,
> 
> RTE_FLOW_ERROR_TYPE_ATTR_GROUP,
> @@ -1977,6 +2070,14 @@ struct field_modify_info modify_tcp[] = {
> 
> 	MLX5_FLOW_ACTION_SET_TTL :
> 
> 	MLX5_FLOW_ACTION_DEC_TTL;
>  			break;
> +		case RTE_FLOW_ACTION_TYPE_JUMP:
> +			ret = flow_dv_validate_action_jump(actions,
> +							   attr->group, error);
> +			if (ret)
> +				return ret;
> +			++actions_n;
> +			action_flags |= MLX5_FLOW_ACTION_JUMP;
> +			break;
>  		default:
>  			return rte_flow_error_set(error, ENOTSUP,
> 
> RTE_FLOW_ERROR_TYPE_ACTION,
> @@ -2756,6 +2857,73 @@ struct field_modify_info modify_tcp[] = {
>  	return match_criteria_enable;
>  }
> 
> +
> +/**
> + * Get a flow table.
> + *
> + * @param dev[in, out]
> + *   Pointer to rte_eth_dev structure.
> + * @param[in] table_id
> + *   Table id to use.
> + * @param[in] egress
> + *   Direction of the table.
> + * @param[out] error
> + *   pointer to error structure.
> + *
> + * @return
> + *   Returns tables resource based on the index, NULL in case of failed.
> + */
> +static struct mlx5_flow_tbl_resource *
> +flow_dv_tbl_resource_get(struct rte_eth_dev *dev,
> +			 uint32_t table_id, uint8_t egress,
> +			 struct rte_flow_error *error)
> +{
> +	struct mlx5_priv *priv = dev->data->dev_private;
> +	struct mlx5_flow_tbl_resource *tbl;
> +
> +	if (egress) {
> +		tbl = &priv->tx_tbl[table_id];
> +		if (!tbl->obj)
> +			tbl->obj = mlx5_glue->dr_create_flow_tbl
> +				(priv->tx_ns, table_id);
> +	} else {
> +		tbl = &priv->rx_tbl[table_id];
> +		if (!tbl->obj)
> +			tbl->obj = mlx5_glue->dr_create_flow_tbl
> +				(priv->rx_ns, table_id);
> +	}
> +	if (!tbl->obj) {
> +		rte_flow_error_set(error, ENOMEM,
> +				   RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
> +				   NULL, "cannot create table");
> +		return NULL;
> +	}
> +	rte_atomic32_inc(&tbl->refcnt);
> +	return tbl;
> +}
> +
> +/**
> + * Release a flow table.
> + *
> + * @param[in] tbl
> + *   Table resource to be released.
> + *
> + * @return
> + *   Returns 0 if table was released, else return 1;
> + */
> +static int
> +flow_dv_tbl_resource_release(struct mlx5_flow_tbl_resource *tbl) {
> +	if (!tbl)
> +		return 0;
> +	if (rte_atomic32_dec_and_test(&tbl->refcnt)) {
> +		mlx5_glue->dr_destroy_flow_tbl(tbl->obj);
> +		tbl->obj = NULL;
> +		return 0;
> +	}
> +	return 1;
> +}
> +
>  /**
>   * Register the flow matcher.
>   *
> @@ -2784,6 +2952,9 @@ struct field_modify_info modify_tcp[] = {
>  		.match_mask = (void *)&matcher->mask,
>  	};
>  	struct mlx5_flow_tbl_resource *tbl = NULL;
> +#ifndef HAVE_MLX5DV_DR
> +	struct mlx5_flow_tbl_resource tbl_tmp; #endif
> 
>  	/* Lookup from cache. */
>  	LIST_FOREACH(cache_matcher, &priv->matchers, next) { @@ -
> 2805,33 +2976,24 @@ struct field_modify_info modify_tcp[] = {
>  			return 0;
>  		}
>  	}
> -#ifdef HAVE_MLX5DV_DR
> -	if (matcher->egress) {
> -		tbl = &priv->tx_tbl[matcher->group];
> -		if (!tbl->obj)
> -			tbl->obj = mlx5_glue->dr_create_flow_tbl
> -				(priv->tx_ns,
> -				 matcher->group * MLX5_GROUP_FACTOR);
> -	} else {
> -		tbl = &priv->rx_tbl[matcher->group];
> -		if (!tbl->obj)
> -			tbl->obj = mlx5_glue->dr_create_flow_tbl
> -				(priv->rx_ns,
> -				 matcher->group * MLX5_GROUP_FACTOR);
> -	}
> -	if (!tbl->obj)
> -		return rte_flow_error_set(error, ENOMEM,
> -
> RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
> -					  NULL, "cannot create table");
> -
> -	rte_atomic32_inc(&tbl->refcnt);
> -#endif
>  	/* Register new matcher. */
>  	cache_matcher = rte_calloc(__func__, 1, sizeof(*cache_matcher), 0);
>  	if (!cache_matcher)
>  		return rte_flow_error_set(error, ENOMEM,
> 
> RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
>  					  "cannot allocate matcher memory");
> +#ifdef HAVE_MLX5DV_DR
> +	tbl = flow_dv_tbl_resource_get(dev, matcher->group *
> MLX5_GROUP_FACTOR,
> +				       matcher->egress, error);
> +	if (!tbl) {
> +		rte_free(cache_matcher);
> +		return rte_flow_error_set(error, ENOMEM,
> +
> RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
> +					  NULL, "cannot create table");
> +	}
> +#else
> +	tbl = &tbl_tmp;
> +#endif
>  	*cache_matcher = *matcher;
>  	dv_attr.match_criteria_enable =
>  		flow_dv_matcher_enable(cache_matcher->mask.buf);
> @@ -2844,10 +3006,7 @@ struct field_modify_info modify_tcp[] = {
>  	if (!cache_matcher->matcher_object) {
>  		rte_free(cache_matcher);
>  #ifdef HAVE_MLX5DV_DR
> -		if (rte_atomic32_dec_and_test(&tbl->refcnt)) {
> -			mlx5_glue->dr_destroy_flow_tbl(tbl->obj);
> -			tbl->obj = NULL;
> -		}
> +		flow_dv_tbl_resource_release(tbl);
>  #endif
>  		return rte_flow_error_set(error, ENOMEM,
> 
> RTE_FLOW_ERROR_TYPE_UNSPECIFIED, @@ -3033,6 +3192,9 @@ struct
> field_modify_info modify_tcp[] = {
>  		const struct rte_flow_action *action = actions;
>  		const struct rte_flow_action_count *count = action->conf;
>  		const uint8_t *rss_key;
> +		const struct rte_flow_action_jump *jump_data;
> +		struct mlx5_flow_dv_jump_tbl_resource jump_tbl_resource;
> +		struct mlx5_flow_tbl_resource *tbl;
> 
>  		switch (actions->type) {
>  		case RTE_FLOW_ACTION_TYPE_VOID:
> @@ -3171,6 +3333,31 @@ struct field_modify_info modify_tcp[] = {
>  			/* If decap is followed by encap, handle it at encap.
> */
>  			action_flags |= MLX5_FLOW_ACTION_RAW_DECAP;
>  			break;
> +		case RTE_FLOW_ACTION_TYPE_JUMP:
> +			jump_data = action->conf;
> +			tbl = flow_dv_tbl_resource_get(dev, jump_data-
> >group *
> +						       MLX5_GROUP_FACTOR,
> +						       attr->egress, error);
> +			if (!tbl)
> +				return rte_flow_error_set
> +						(error, errno,
> +
> RTE_FLOW_ERROR_TYPE_ACTION,
> +						 NULL,
> +						 "cannot create jump
> action.");
> +			jump_tbl_resource.tbl = tbl;
> +			if (flow_dv_jump_tbl_resource_register
> +			    (dev, &jump_tbl_resource, dev_flow, error)) {
> +				flow_dv_tbl_resource_release(tbl);
> +				return rte_flow_error_set
> +						(error, errno,
> +
> RTE_FLOW_ERROR_TYPE_ACTION,
> +						 NULL,
> +						 "cannot create jump
> action.");
> +			}
> +			dev_flow->dv.actions[actions_n++] =
> +				dev_flow->dv.jump->action;
> +			action_flags |= MLX5_FLOW_ACTION_JUMP;
> +			break;
>  		case RTE_FLOW_ACTION_TYPE_SET_MAC_SRC:
>  		case RTE_FLOW_ACTION_TYPE_SET_MAC_DST:
>  			if (flow_dv_convert_action_modify_mac(&res,
> actions, @@ -3503,10 +3690,7 @@ struct field_modify_info modify_tcp[] = {
>  			tbl = &priv->tx_tbl[matcher->group];
>  		else
>  			tbl = &priv->rx_tbl[matcher->group];
> -		if (rte_atomic32_dec_and_test(&tbl->refcnt)) {
> -			mlx5_glue->dr_destroy_flow_tbl(tbl->obj);
> -			tbl->obj = NULL;
> -		}
> +		flow_dv_tbl_resource_release(tbl);
>  		rte_free(matcher);
>  		DRV_LOG(DEBUG, "port %u matcher %p: removed",
>  			dev->data->port_id, (void *)matcher); @@ -3547,6
> +3731,38 @@ struct field_modify_info modify_tcp[] = {  }
> 
>  /**
> + * Release an jump to table action resource.
> + *
> + * @param flow
> + *   Pointer to mlx5_flow.
> + *
> + * @return
> + *   1 while a reference on it exists, 0 when freed.
> + */
> +static int
> +flow_dv_jump_tbl_resource_release(struct mlx5_flow *flow) {
> +	struct mlx5_flow_dv_jump_tbl_resource *cache_resource =
> +						flow->dv.jump;
> +
> +	assert(cache_resource->action);
> +	DRV_LOG(DEBUG, "jump table resource %p: refcnt %d--",
> +		(void *)cache_resource,
> +		rte_atomic32_read(&cache_resource->refcnt));
> +	if (rte_atomic32_dec_and_test(&cache_resource->refcnt)) {
> +		claim_zero(mlx5_glue->destroy_flow_action
> +				(cache_resource->action));
> +		LIST_REMOVE(cache_resource, next);
> +		flow_dv_tbl_resource_release(cache_resource->tbl);
> +		rte_free(cache_resource);
> +		DRV_LOG(DEBUG, "jump table resource %p: removed",
> +			(void *)cache_resource);
> +		return 0;
> +	}
> +	return 1;
> +}
> +
> +/**
>   * Release a modify-header resource.
>   *
>   * @param flow
> @@ -3642,6 +3858,8 @@ struct field_modify_info modify_tcp[] = {
>  			flow_dv_encap_decap_resource_release(dev_flow);
>  		if (dev_flow->dv.modify_hdr)
>  			flow_dv_modify_hdr_resource_release(dev_flow);
> +		if (dev_flow->dv.jump)
> +			flow_dv_jump_tbl_resource_release(dev_flow);
>  		rte_free(dev_flow);
>  	}
>  }
> diff --git a/drivers/net/mlx5/mlx5_glue.c b/drivers/net/mlx5/mlx5_glue.c
> index f509f85..fca34ca 100644
> --- a/drivers/net/mlx5/mlx5_glue.c
> +++ b/drivers/net/mlx5/mlx5_glue.c
> @@ -370,6 +370,17 @@
>  }
> 
>  static void *
> +mlx5_glue_dr_create_flow_action_dest_flow_tbl(void *tbl) { #ifdef
> +HAVE_MLX5DV_DR
> +	return mlx5dv_dr_create_action_dest_flow_table(tbl);
> +#else
> +	(void)tbl;
> +	return NULL;
> +#endif
> +}
> +
> +static void *
>  mlx5_glue_dr_create_flow_tbl(void *ns, uint32_t level)  {  #ifdef
> HAVE_MLX5DV_DR @@ -833,6 +844,8 @@
>  	.get_async_event = mlx5_glue_get_async_event,
>  	.port_state_str = mlx5_glue_port_state_str,
>  	.cq_ex_to_cq = mlx5_glue_cq_ex_to_cq,
> +	.dr_create_flow_action_dest_flow_tbl =
> +		mlx5_glue_dr_create_flow_action_dest_flow_tbl,
>  	.dr_create_flow_tbl = mlx5_glue_dr_create_flow_tbl,
>  	.dr_destroy_flow_tbl = mlx5_glue_dr_destroy_flow_tbl,
>  	.dr_create_ns = mlx5_glue_dr_create_ns, diff --git
> a/drivers/net/mlx5/mlx5_glue.h b/drivers/net/mlx5/mlx5_glue.h index
> 7115575..16d5dd6 100644
> --- a/drivers/net/mlx5/mlx5_glue.h
> +++ b/drivers/net/mlx5/mlx5_glue.h
> @@ -145,6 +145,7 @@ struct mlx5_glue {
>  			       struct ibv_async_event *event);
>  	const char *(*port_state_str)(enum ibv_port_state port_state);
>  	struct ibv_cq *(*cq_ex_to_cq)(struct ibv_cq_ex *cq);
> +	void *(*dr_create_flow_action_dest_flow_tbl)(void *tbl);
>  	void *(*dr_create_flow_tbl)(void *ns, uint32_t level);
>  	int (*dr_destroy_flow_tbl)(void *tbl);
>  	void *(*dr_create_ns)(struct ibv_context *ctx,
> --
> 1.8.3.1