Re: [dpdk-dev] [PATCH v5 4/7] net/mlx5: support push flow action on VLAN header

[Slava Ovsiienko <viacheslavo@mellanox.com>]

> -----Original Message-----
> From: Moti Haimovsky <motih@mellanox.com>
> Sent: Monday, September 9, 2019 18:57
> To: Slava Ovsiienko <viacheslavo@mellanox.com>; Raslan Darawsheh
> <rasland@mellanox.com>
> Cc: dev@dpdk.org
> Subject: [PATCH v5 4/7] net/mlx5: support push flow action on VLAN header
> 
> This commit adds support for RTE_FLOW_ACTION_TYPE_OF_PUSH_VLAN
> using direct verbs flow rules.
> If present in the flow, The VLAN default values are taken from the VLAN item
> configuration.
> In this commit only the VLAN TPID value can be set since VLAN modification
> actions are not supported yet.
> 
> Signed-off-by: Moti Haimovsky <motih@mellanox.com>
Acked-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>

> ---
> v5:
> - Fixed bug in retrieving VLAN VID and PCP from the existing VLAN flow item.
> ---
> 
>  doc/guides/nics/mlx5.rst               |   6 +-
>  doc/guides/rel_notes/release_19_11.rst |   1 +
>  drivers/net/mlx5/mlx5.h                |   2 +
>  drivers/net/mlx5/mlx5_flow.h           |  18 ++-
>  drivers/net/mlx5/mlx5_flow_dv.c        | 272
> +++++++++++++++++++++++++++++++++
>  5 files changed, 295 insertions(+), 4 deletions(-)
> 
> diff --git a/doc/guides/nics/mlx5.rst b/doc/guides/nics/mlx5.rst index
> 2ae2e8f..875a87f 100644
> --- a/doc/guides/nics/mlx5.rst
> +++ b/doc/guides/nics/mlx5.rst
> @@ -130,6 +130,8 @@ Limitations
>      are lacking a match on VLAN as one of their items are not supported.
>    - The command is not supported on egress traffic.
> 
> +- VLAN push offload is not supported on ingress traffic.
> +
>  - A multi segment packet must have not more segments than reported by
> dev_infos_get()
>    in tx_desc_lim.nb_seg_max field. This value depends on maximal supported
> Tx descriptor
>    size and ``txq_inline_min`` settings and may be from 2 (worst case forced
> by maximal @@ -1039,8 +1041,8 @@ Supported hardware offloads
>     |                       | | ConnectX-5    |     | N/A       |
>     +-----------------------+-----------------+-----------------+
>     | | VLAN                | | DPDK 19.11    | | DPDK 19.11    |
> -   | | (of_pop_vlan)       | | OFED 4.6-4    | | OFED 4.6-4    |
> -   |                       | | ConnectX-5    | | ConnectX-5    |
> +   | | (of_pop_vlan /      | | OFED 4.6-4    | | OFED 4.6-4    |
> +   | | of_push_vlan)       | | ConnectX-5    | | ConnectX-5    |
>     +-----------------------+-----------------+-----------------+
> 
>  Notes for testpmd
> diff --git a/doc/guides/rel_notes/release_19_11.rst
> b/doc/guides/rel_notes/release_19_11.rst
> index 5c2ac15..afe92ab 100644
> --- a/doc/guides/rel_notes/release_19_11.rst
> +++ b/doc/guides/rel_notes/release_19_11.rst
> @@ -228,4 +228,5 @@ Tested Platforms
>    Updated Mellanox mlx5 driver with new features and improvements,
> including:
> 
>    * Added support for VLAN pop flow offload command.
> +  * Added support for VLAN push flow offload command.
> 
> diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h index
> a18f588..dbdc3ce 100644
> --- a/drivers/net/mlx5/mlx5.h
> +++ b/drivers/net/mlx5/mlx5.h
> @@ -579,6 +579,8 @@ struct mlx5_ibv_shared {
>  	LIST_HEAD(jump, mlx5_flow_dv_jump_tbl_resource) jump_tbl;
>  	LIST_HEAD(port_id_action_list,
> mlx5_flow_dv_port_id_action_resource)
>  		port_id_action_list; /* List of port ID actions. */
> +	LIST_HEAD(push_vlan_action_list,
> mlx5_flow_dv_push_vlan_action_resource)
> +		push_vlan_action_list; /* List of push VLAN actions. */
>  	struct mlx5_flow_counter_mng cmng; /* Counters management
> structure. */
>  	/* Shared interrupt handler section. */
>  	pthread_mutex_t intr_mutex; /* Interrupt config mutex. */ diff --git
> a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h index
> 06b0470..15c9c04 100644
> --- a/drivers/net/mlx5/mlx5_flow.h
> +++ b/drivers/net/mlx5/mlx5_flow.h
> @@ -150,7 +150,8 @@
> 
>  #define MLX5_FLOW_ENCAP_ACTIONS
> 	(MLX5_FLOW_ACTION_VXLAN_ENCAP | \
>  				 MLX5_FLOW_ACTION_NVGRE_ENCAP | \
> -				 MLX5_FLOW_ACTION_RAW_ENCAP)
> +				 MLX5_FLOW_ACTION_RAW_ENCAP | \
> +				 MLX5_FLOW_ACTION_OF_PUSH_VLAN)
> 
>  #define MLX5_FLOW_DECAP_ACTIONS
> 	(MLX5_FLOW_ACTION_VXLAN_DECAP | \
>  				 MLX5_FLOW_ACTION_NVGRE_DECAP | \
> @@ -172,7 +173,8 @@
>  				      MLX5_FLOW_ACTION_INC_TCP_ACK | \
>  				      MLX5_FLOW_ACTION_DEC_TCP_ACK)
> 
> -#define MLX5_FLOW_VLAN_ACTIONS
> (MLX5_FLOW_ACTION_OF_POP_VLAN)
> +#define MLX5_FLOW_VLAN_ACTIONS
> (MLX5_FLOW_ACTION_OF_POP_VLAN | \
> +				MLX5_FLOW_ACTION_OF_PUSH_VLAN)
> 
>  #ifndef IPPROTO_MPLS
>  #define IPPROTO_MPLS 137
> @@ -309,6 +311,16 @@ struct mlx5_flow_dv_port_id_action_resource {
>  	uint32_t port_id; /**< Port ID value. */  };
> 
> +/* Push VLAN action resource structure */ struct
> +mlx5_flow_dv_push_vlan_action_resource {
> +	LIST_ENTRY(mlx5_flow_dv_push_vlan_action_resource) next;
> +	/* Pointer to next element. */
> +	rte_atomic32_t refcnt; /**< Reference counter. */
> +	void *action; /**< Direct verbs action object. */
> +	uint8_t ft_type; /**< Flow table type, Rx, Tx or FDB. */
> +	rte_be32_t vlan_tag; /**< VLAN tag value. */ };
> +
>  /*
>   * Max number of actions per DV flow.
>   * See CREATE_FLOW_MAX_FLOW_ACTIONS_SUPPORTED
> @@ -335,6 +347,8 @@ struct mlx5_flow_dv {
>  	/**< Pointer to port ID action resource. */
>  	struct mlx5_vf_vlan vf_vlan;
>  	/**< Structure for VF VLAN workaround. */
> +	struct mlx5_flow_dv_push_vlan_action_resource *push_vlan_res;
> +	/**< Pointer to push VLAN action resource in cache. */
>  #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 fd2e810..2780735 100644
> --- a/drivers/net/mlx5/mlx5_flow_dv.c
> +++ b/drivers/net/mlx5/mlx5_flow_dv.c
> @@ -50,6 +50,13 @@
>  #define MLX5DV_DR_ACTION_FLAGS_ROOT_LEVEL 1  #endif
> 
> +/* VLAN header definitions */
> +#define MLX5DV_FLOW_VLAN_PCP_SHIFT 13
> +#define MLX5DV_FLOW_VLAN_PCP_MASK (0x7 <<
> MLX5DV_FLOW_VLAN_PCP_SHIFT)
> +#define MLX5DV_FLOW_VLAN_VID_MASK 0x0fff #define
> +MLX5DV_FLOW_VLAN_PCP_MASK_BE
> RTE_BE16(MLX5DV_FLOW_VLAN_PCP_MASK)
> +#define MLX5DV_FLOW_VLAN_VID_MASK_BE
> +RTE_BE16(MLX5DV_FLOW_VLAN_VID_MASK)
> +
>  union flow_dv_attr {
>  	struct {
>  		uint32_t valid:1;
> @@ -817,6 +824,8 @@ struct field_modify_info modify_tcp[] = {
>  /**
>   * Validate the pop VLAN action.
>   *
> + * @param[in] dev
> + *   Pointer to the rte_eth_dev structure.
>   * @param[in] action_flags
>   *   Holds the actions detected until now.
>   * @param[in] action
> @@ -870,6 +879,101 @@ struct field_modify_info modify_tcp[] = {  }
> 
>  /**
> + * Get VLAN default info from vlan match info.
> + *
> + * @param[in] dev
> + *   Pointer to the rte_eth_dev structure.
> + * @param[in] item
> + *   the list of item specifications.
> + * @param[out] vlan
> + *   pointer VLAN info to fill to.
> + * @param[out] error
> + *   Pointer to error structure.
> + *
> + * @return
> + *   0 on success, a negative errno value otherwise and rte_errno is set.
> + */
> +static void
> +flow_dev_get_vlan_info_from_items(const struct rte_flow_item *items,
> +				  struct rte_vlan_hdr *vlan)
> +{
> +	const struct rte_flow_item_vlan nic_mask = {
> +		.tci = RTE_BE16(MLX5DV_FLOW_VLAN_PCP_MASK |
> +				MLX5DV_FLOW_VLAN_VID_MASK),
> +		.inner_type = RTE_BE16(0xffff),
> +	};
> +
> +	if (items == NULL)
> +		return;
> +	for (; items->type != RTE_FLOW_ITEM_TYPE_END &&
> +	       items->type != RTE_FLOW_ITEM_TYPE_VLAN; items++)
> +		;
> +	if (items->type == RTE_FLOW_ITEM_TYPE_VLAN) {
> +		const struct rte_flow_item_vlan *vlan_m = items->mask;
> +		const struct rte_flow_item_vlan *vlan_v = items->spec;
> +
> +		if (!vlan_m)
> +			vlan_m = &nic_mask;
> +		/* Only full match values are accepted */
> +		if ((vlan_m->tci & MLX5DV_FLOW_VLAN_PCP_MASK_BE) ==
> +		     MLX5DV_FLOW_VLAN_PCP_MASK_BE) {
> +			vlan->vlan_tci &= MLX5DV_FLOW_VLAN_PCP_MASK;
> +			vlan->vlan_tci |=
> +				rte_be_to_cpu_16(vlan_v->tci &
> +
> MLX5DV_FLOW_VLAN_PCP_MASK_BE);
> +		}
> +		if ((vlan_m->tci & MLX5DV_FLOW_VLAN_VID_MASK_BE) ==
> +		     MLX5DV_FLOW_VLAN_VID_MASK_BE) {
> +			vlan->vlan_tci &=
> ~MLX5DV_FLOW_VLAN_VID_MASK;
> +			vlan->vlan_tci |=
> +				rte_be_to_cpu_16(vlan_v->tci &
> +
> MLX5DV_FLOW_VLAN_VID_MASK_BE);
> +		}
> +		if (vlan_m->inner_type == nic_mask.inner_type)
> +			vlan->eth_proto = rte_be_to_cpu_16(vlan_v-
> >inner_type &
> +							   vlan_m-
> >inner_type);
> +	}
> +}
> +
> +/**
> + * Validate the push VLAN action.
> + *
> + * @param[in] action_flags
> + *   Holds the actions detected until now.
> + * @param[in] action
> + *   Pointer to the encap action.
> + * @param[in] attr
> + *   Pointer to flow attributes
> + * @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_push_vlan(uint64_t action_flags,
> +				  const struct rte_flow_action *action,
> +				  const struct rte_flow_attr *attr,
> +				  struct rte_flow_error *error)
> +{
> +	const struct rte_flow_action_of_push_vlan *push_vlan = action-
> >conf;
> +
> +	if (push_vlan->ethertype != RTE_BE16(RTE_ETHER_TYPE_VLAN) &&
> +	    push_vlan->ethertype != RTE_BE16(RTE_ETHER_TYPE_QINQ))
> +		return rte_flow_error_set(error, EINVAL,
> +					  RTE_FLOW_ERROR_TYPE_ACTION,
> action,
> +					  "invalid vlan ethertype");
> +	if (action_flags &
> +		(MLX5_FLOW_ACTION_OF_POP_VLAN |
> MLX5_FLOW_ACTION_OF_PUSH_VLAN))
> +		return rte_flow_error_set(error, ENOTSUP,
> +					  RTE_FLOW_ERROR_TYPE_ACTION,
> action,
> +					  "no support for multiple VLAN "
> +					  "actions");
> +	(void)attr;
> +	return 0;
> +}
> +
> +/**
>   * Validate count action.
>   *
>   * @param[in] dev
> @@ -1300,6 +1404,77 @@ struct field_modify_info modify_tcp[] = {  }
> 
>  /**
> + * Find existing push vlan resource or create and register a new one.
> + *
> + * @param dev[in, out]
> + *   Pointer to rte_eth_dev structure.
> + * @param[in, out] resource
> + *   Pointer to port ID action 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_push_vlan_action_resource_register
> +		       (struct rte_eth_dev *dev,
> +			struct mlx5_flow_dv_push_vlan_action_resource
> *resource,
> +			struct mlx5_flow *dev_flow,
> +			struct rte_flow_error *error)
> +{
> +	struct mlx5_priv *priv = dev->data->dev_private;
> +	struct mlx5_ibv_shared *sh = priv->sh;
> +	struct mlx5_flow_dv_push_vlan_action_resource *cache_resource;
> +	struct mlx5dv_dr_domain *domain;
> +
> +	/* Lookup a matching resource from cache. */
> +	LIST_FOREACH(cache_resource, &sh->push_vlan_action_list, next) {
> +		if (resource->vlan_tag == cache_resource->vlan_tag &&
> +		    resource->ft_type == cache_resource->ft_type) {
> +			DRV_LOG(DEBUG, "push-VLAN action resource
> resource %p: "
> +				"refcnt %d++",
> +				(void *)cache_resource,
> +				rte_atomic32_read(&cache_resource-
> >refcnt));
> +			rte_atomic32_inc(&cache_resource->refcnt);
> +			dev_flow->dv.push_vlan_res = cache_resource;
> +			return 0;
> +		}
> +	}
> +	/* Register new push_vlan action 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;
> +	if (resource->ft_type == MLX5DV_FLOW_TABLE_TYPE_FDB)
> +		domain = sh->fdb_domain;
> +	else if (resource->ft_type == MLX5DV_FLOW_TABLE_TYPE_NIC_RX)
> +		domain = sh->rx_domain;
> +	else
> +		domain = sh->tx_domain;
> +	cache_resource->action =
> +		mlx5_glue->dr_create_flow_action_push_vlan(domain,
> +							   resource-
> >vlan_tag);
> +	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(&sh->push_vlan_action_list, cache_resource,
> next);
> +	dev_flow->dv.push_vlan_res = cache_resource;
> +	DRV_LOG(DEBUG, "new push vlan action 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
> @@ -1719,6 +1894,44 @@ struct field_modify_info modify_tcp[] = {  }
> 
>  /**
> + * Create action push VLAN.
> + *
> + * @param[in] dev
> + *   Pointer to rte_eth_dev structure.
> + * @param[in] vlan_tag
> + *   the vlan tag to push to the Ethernet header.
> + * @param[in, out] dev_flow
> + *   Pointer to the mlx5_flow.
> + * @param[in] attr
> + *   Pointer to the flow attributes.
> + * @param[out] error
> + *   Pointer to the error structure.
> + *
> + * @return
> + *   0 on success, a negative errno value otherwise and rte_errno is set.
> + */
> +static int
> +flow_dv_create_action_push_vlan(struct rte_eth_dev *dev,
> +				const struct rte_flow_attr *attr,
> +				const struct rte_vlan_hdr *vlan,
> +				struct mlx5_flow *dev_flow,
> +				struct rte_flow_error *error)
> +{
> +	struct mlx5_flow_dv_push_vlan_action_resource res;
> +
> +	res.vlan_tag =
> +		rte_cpu_to_be_32(((uint32_t)vlan->eth_proto) << 16 |
> +				 vlan->vlan_tci);
> +	if (attr->transfer)
> +		res.ft_type = MLX5DV_FLOW_TABLE_TYPE_FDB;
> +	else
> +		res.ft_type = attr->egress ?
> MLX5DV_FLOW_TABLE_TYPE_NIC_TX :
> +
> MLX5DV_FLOW_TABLE_TYPE_NIC_RX;
> +	return flow_dv_push_vlan_action_resource_register
> +					    (dev, &res, dev_flow, error);
> +}
> +
> +/**
>   * Validate the modify-header actions.
>   *
>   * @param[in] action_flags
> @@ -3174,6 +3387,15 @@ struct field_modify_info modify_tcp[] = {
>  			action_flags |=
> MLX5_FLOW_ACTION_OF_POP_VLAN;
>  			++actions_n;
>  			break;
> +		case RTE_FLOW_ACTION_TYPE_OF_PUSH_VLAN:
> +			ret =
> flow_dv_validate_action_push_vlan(action_flags,
> +								actions, attr,
> +								error);
> +			if (ret < 0)
> +				return ret;
> +			action_flags |=
> MLX5_FLOW_ACTION_OF_PUSH_VLAN;
> +			++actions_n;
> +			break;
>  		case RTE_FLOW_ACTION_TYPE_VXLAN_ENCAP:
>  		case RTE_FLOW_ACTION_TYPE_NVGRE_ENCAP:
>  			ret =
> flow_dv_validate_action_l2_encap(action_flags,
> @@ -4765,6 +4987,8 @@ struct field_modify_info modify_tcp[] = {
>  	void *match_mask = matcher.mask.buf;
>  	void *match_value = dev_flow->dv.value.buf;
>  	uint8_t next_protocol = 0xff;
> +	struct rte_vlan_hdr vlan = { 0 };
> +	bool vlan_inherited = false;
> 
>  	flow->group = attr->group;
>  	if (attr->transfer)
> @@ -4879,6 +5103,21 @@ struct field_modify_info modify_tcp[] = {
>  						priv->sh->pop_vlan_action;
>  			action_flags |=
> MLX5_FLOW_ACTION_OF_POP_VLAN;
>  			break;
> +		case RTE_FLOW_ACTION_TYPE_OF_PUSH_VLAN:
> +			if (!vlan_inherited) {
> +				flow_dev_get_vlan_info_from_items(items,
> &vlan);
> +				vlan_inherited = true;
> +			}
> +			vlan.eth_proto = rte_be_to_cpu_16
> +			     ((((const struct rte_flow_action_of_push_vlan *)
> +						   actions->conf)-
> >ethertype));
> +			if (flow_dv_create_action_push_vlan
> +					    (dev, attr, &vlan, dev_flow, error))
> +				return -rte_errno;
> +			dev_flow->dv.actions[actions_n++] =
> +					   dev_flow->dv.push_vlan_res-
> >action;
> +			action_flags |=
> MLX5_FLOW_ACTION_OF_PUSH_VLAN;
> +			break;
>  		case RTE_FLOW_ACTION_TYPE_VXLAN_ENCAP:
>  		case RTE_FLOW_ACTION_TYPE_NVGRE_ENCAP:
>  			if (flow_dv_create_action_l2_encap(dev, actions,
> @@ -5526,6 +5765,37 @@ struct field_modify_info modify_tcp[] = {  }
> 
>  /**
> + * Release push vlan action resource.
> + *
> + * @param flow
> + *   Pointer to mlx5_flow.
> + *
> + * @return
> + *   1 while a reference on it exists, 0 when freed.
> + */
> +static int
> +flow_dv_push_vlan_action_resource_release(struct mlx5_flow *flow) {
> +	struct mlx5_flow_dv_push_vlan_action_resource *cache_resource =
> +		flow->dv.push_vlan_res;
> +
> +	assert(cache_resource->action);
> +	DRV_LOG(DEBUG, "push VLAN action 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);
> +		rte_free(cache_resource);
> +		DRV_LOG(DEBUG, "push vlan action resource %p: removed",
> +			(void *)cache_resource);
> +		return 0;
> +	}
> +	return 1;
> +}
> +
> +/**
>   * Remove the flow from the NIC but keeps it in memory.
>   *
>   * @param[in] dev
> @@ -5597,6 +5867,8 @@ struct field_modify_info modify_tcp[] = {
>  			flow_dv_jump_tbl_resource_release(dev_flow);
>  		if (dev_flow->dv.port_id_action)
> 
> 	flow_dv_port_id_action_resource_release(dev_flow);
> +		if (dev_flow->dv.push_vlan_res)
> +
> 	flow_dv_push_vlan_action_resource_release(dev_flow);
>  		rte_free(dev_flow);
>  	}
>  }
> --
> 1.8.3.1