[v2,3/3] net/mlx5: add jump action support for NIC

Message ID 1553790741-69362-4-git-send-email-orika@mellanox.com (mailing list archive)
State Superseded, archived
Delegated to: Shahaf Shuler
Headers
Series net/mlx5: Add Direct Rule support |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Ori Kam March 28, 2019, 4:32 p.m. UTC
  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>
---
 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(-)
  

Comments

Slava Ovsiienko April 1, 2019, 2:38 p.m. UTC | #1
> -----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
  
Shahaf Shuler April 3, 2019, 10:16 a.m. UTC | #2
Thursday, March 28, 2019 6:33 PM, Ori Kam:
> 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>
> ---
>  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(-)
> 

[...]

>  /**
>   * 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

I suggest to have the HAVE_MLX5DV_DR inside the flow_dv_tbl_resource_get (return NULL if no DR support). 
Then we can have a cleaner code of this function.
  

Patch

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,