DPDK patches and discussions
 help / color / mirror / Atom feed
From: Shahaf Shuler <shahafs@mellanox.com>
To: Dekel Peled <dekelp@mellanox.com>, Yongseok Koh <yskoh@mellanox.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>, Ori Kam <orika@mellanox.com>,
	Dekel Peled <dekelp@mellanox.com>
Subject: Re: [dpdk-dev] [PATCH] net/mlx5: modify-header support using Direct Verbs
Date: Tue, 25 Dec 2018 11:38:06 +0000	[thread overview]
Message-ID: <DB7PR05MB44260570F8B413552330EEE6C3B40@DB7PR05MB4426.eurprd05.prod.outlook.com> (raw)
In-Reply-To: <1545559083-60488-1-git-send-email-dekelp@mellanox.com>

Hi Dekel,

See some comments below,

Sunday, December 23, 2018 11:58 AM, Dekel Peled:
> Subject: [PATCH] net/mlx5: modify-header support using Direct Verbs

Maybe title can be: "support modify header using Direct Verbs"

> 
> This patch implements the set of actions to support offload of packet header
> modifications to MLX5 NIC.
> 
> Implamantation is based on RFC [1].
> 
> [1] http://mails.dpdk.org/archives/dev/2018-November/119971.html
> 
> Signed-off-by: Dekel Peled <dekelp@mellanox.com>
> ---
>  drivers/net/mlx5/mlx5.h         |   1 +
>  drivers/net/mlx5/mlx5_flow.h    |  56 ++-
>  drivers/net/mlx5/mlx5_flow_dv.c | 998
> +++++++++++++++++++++++++++++++++++++++-
>  drivers/net/mlx5/mlx5_glue.c    |  22 +
>  drivers/net/mlx5/mlx5_glue.h    |   5 +
>  drivers/net/mlx5/mlx5_prm.h     |  11 +-
>  6 files changed, 1084 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h index
> 75aeeb2..b2fe5cb 100644
> --- a/drivers/net/mlx5/mlx5.h
> +++ b/drivers/net/mlx5/mlx5.h
> @@ -227,6 +227,7 @@ struct priv {
>  	LIST_HEAD(ind_tables, mlx5_ind_table_ibv) ind_tbls;
>  	LIST_HEAD(matchers, mlx5_flow_dv_matcher) matchers;
>  	LIST_HEAD(encap_decap, mlx5_flow_dv_encap_decap_resource)
> encaps_decaps;
> +	LIST_HEAD(modify_cmd, mlx5_flow_dv_modify_hdr_resource)
> modify_cmds;
>  	uint32_t link_speed_capa; /* Link speed capabilities. */
>  	struct mlx5_xstats_ctrl xstats_ctrl; /* Extended stats control. */
>  	struct mlx5_stats_ctrl stats_ctrl; /* Stats control. */ diff --git
> a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h index
> 4a7c052..cb1e6fd 100644
> --- a/drivers/net/mlx5/mlx5_flow.h
> +++ b/drivers/net/mlx5/mlx5_flow.h
> @@ -69,6 +69,18 @@
>  	(MLX5_FLOW_LAYER_INNER_L2 | MLX5_FLOW_LAYER_INNER_L3 | \
>  	 MLX5_FLOW_LAYER_INNER_L4)
> 
> +/* Layer Masks. */
> +#define MLX5_FLOW_LAYER_L2 \
> +	(MLX5_FLOW_LAYER_OUTER_L2 | MLX5_FLOW_LAYER_INNER_L2)
> #define
> +MLX5_FLOW_LAYER_L3_IPV4 \
> +	(MLX5_FLOW_LAYER_OUTER_L3_IPV4 |
> MLX5_FLOW_LAYER_INNER_L3_IPV4)
> +#define MLX5_FLOW_LAYER_L3_IPV6 \
> +	(MLX5_FLOW_LAYER_OUTER_L3_IPV6 |
> MLX5_FLOW_LAYER_INNER_L3_IPV6)
> +#define MLX5_FLOW_LAYER_L3 \
> +	(MLX5_FLOW_LAYER_L3_IPV4 | MLX5_FLOW_LAYER_L3_IPV6) #define
> +MLX5_FLOW_LAYER_L4 \
> +	(MLX5_FLOW_LAYER_OUTER_L4 | MLX5_FLOW_LAYER_INNER_L4)
> +
>  /* Actions */
>  #define MLX5_FLOW_ACTION_DROP (1u << 0)  #define
> MLX5_FLOW_ACTION_QUEUE (1u << 1) @@ -110,6 +122,17 @@
>  				 MLX5_FLOW_ACTION_NVGRE_DECAP | \
>  				 MLX5_FLOW_ACTION_RAW_DECAP)
> 
> +#define MLX5_FLOW_MODIFY_HDR_ACTIONS
> (MLX5_FLOW_ACTION_SET_IPV4_SRC | \
> +				      MLX5_FLOW_ACTION_SET_IPV4_DST | \
> +				      MLX5_FLOW_ACTION_SET_IPV6_SRC | \
> +				      MLX5_FLOW_ACTION_SET_IPV6_DST | \
> +				      MLX5_FLOW_ACTION_SET_TP_SRC | \
> +				      MLX5_FLOW_ACTION_SET_TP_DST | \
> +				      MLX5_FLOW_ACTION_SET_TTL | \
> +				      MLX5_FLOW_ACTION_DEC_TTL | \
> +				      MLX5_FLOW_ACTION_SET_MAC_SRC | \
> +				      MLX5_FLOW_ACTION_SET_MAC_DST)
> +
>  #ifndef IPPROTO_MPLS
>  #define IPPROTO_MPLS 137
>  #endif
> @@ -153,9 +176,6 @@
>  /* IBV hash source bits  for IPV6. */
>  #define MLX5_IPV6_IBV_RX_HASH (IBV_RX_HASH_SRC_IPV6 |
> IBV_RX_HASH_DST_IPV6)
> 
> -/* Max number of actions per DV flow. */ -#define
> MLX5_DV_MAX_NUMBER_OF_ACTIONS 8
> -
>  enum mlx5_flow_drv_type {
>  	MLX5_FLOW_TYPE_MIN,
>  	MLX5_FLOW_TYPE_DV,
> @@ -172,9 +192,6 @@ struct mlx5_flow_dv_match_params {
>  	/**< Matcher value. This value is used as the mask or as a key. */  };
> 
> -#define MLX5_DV_MAX_NUMBER_OF_ACTIONS 8 -#define
> MLX5_ENCAP_MAX_LEN 132
> -
>  /* Matcher structure. */
>  struct mlx5_flow_dv_matcher {
>  	LIST_ENTRY(mlx5_flow_dv_matcher) next; @@ -187,6 +204,8 @@
> struct mlx5_flow_dv_matcher {
>  	struct mlx5_flow_dv_match_params mask; /**< Matcher mask. */  };
> 
> +#define MLX5_ENCAP_MAX_LEN 132
> +
>  /* Encap/decap resource structure. */
>  struct mlx5_flow_dv_encap_decap_resource {
>  	LIST_ENTRY(mlx5_flow_dv_encap_decap_resource) next; @@ -200,6
> +219,29 @@ struct mlx5_flow_dv_encap_decap_resource {
>  	uint8_t ft_type;
>  };
> 
> +/* Number of modification commands. */
> +#define MLX5_MODIFY_NUM 8
> +
> +/* Modify resource structure */
> +struct mlx5_flow_dv_modify_hdr_resource {
> +	LIST_ENTRY(mlx5_flow_dv_modify_hdr_resource) next;
> +	/* 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. */
> +	uint32_t actions_num; /**< Number of modification actions. */
> +	struct mlx5_modification_cmd actions[MLX5_MODIFY_NUM];
> +	/**< Modification actions. */
> +};
> +
> +/*
> + * 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
> +
>  /* DV flows structure. */
>  struct mlx5_flow_dv {
>  	uint64_t hash_fields; /**< Fields that participate in the hash. */ @@ -
> 210,6 +252,8 @@ struct mlx5_flow_dv {
>  	/**< Holds the value that the packet is compared to. */
>  	struct mlx5_flow_dv_encap_decap_resource *encap_decap;
>  	/**< Pointer to encap/decap resource in cache. */
> +	struct mlx5_flow_dv_modify_hdr_resource *modify_hdr;
> +	/**< Pointer to modify header resource in cache. */
>  	struct ibv_flow *flow; /**< Installed flow. */  #ifdef
> HAVE_IBV_FLOW_DV_SUPPORT
>  	struct mlx5dv_flow_action_attr
> actions[MLX5_DV_MAX_NUMBER_OF_ACTIONS];
> diff --git a/drivers/net/mlx5/mlx5_flow_dv.c
> b/drivers/net/mlx5/mlx5_flow_dv.c index 1f31874..ad4e501 100644
> --- a/drivers/net/mlx5/mlx5_flow_dv.c
> +++ b/drivers/net/mlx5/mlx5_flow_dv.c
> @@ -35,6 +35,504 @@
> 
>  #ifdef HAVE_IBV_FLOW_DV_SUPPORT
> 
> +union flow_dv_attr {
> +	struct {
> +		uint32_t valid:1;
> +		uint32_t ipv4:1;
> +		uint32_t ipv6:1;
> +		uint32_t tcp:1;
> +		uint32_t udp:1;
> +		uint32_t reserved:27;
> +	};
> +	uint32_t attr;
> +};
> +
> +static void
> +flow_dv_attr_init(const struct rte_flow_item *item, union flow_dv_attr
> +*attr) {

Can we avoid this duplicated parsing and use the parsing done on translate when traversing the items list (using item_flags)?
it will require to do the item translate before the actions translate.
Koh, Ori what do you think? 

If yes, then the swap between the item and action translate should be on a separate patch to ease the patch review. 

> +	for (; item->type != RTE_FLOW_ITEM_TYPE_END; item++) {
> +		switch (item->type) {
> +		case RTE_FLOW_ITEM_TYPE_IPV4:
> +			attr->ipv4 = 1;
> +			break;
> +		case RTE_FLOW_ITEM_TYPE_IPV6:
> +			attr->ipv6 = 1;
> +			break;
> +		case RTE_FLOW_ITEM_TYPE_UDP:
> +			attr->udp = 1;
> +			break;
> +		case RTE_FLOW_ITEM_TYPE_TCP:
> +			attr->tcp = 1;
> +			break;
> +		default:
> +			break;
> +		}
> +	}
> +	attr->valid = 1;
> +}
> +
> +struct field_modify_info {
> +	int bits; /* Offset of field in protocol header, in bits. */
> +	enum mlx5_modification_field outer_type;
> +	enum mlx5_modification_field inner_type; };
> +
> +struct field_modify_info modify_eth[] = {
> +	{4 * 8, MLX5_MODI_OUT_DMAC_47_16,
> MLX5_MODI_IN_DMAC_47_16},
> +	{2 * 8, MLX5_MODI_OUT_DMAC_15_0,
> MLX5_MODI_IN_DMAC_15_0},
> +	{4 * 8, MLX5_MODI_OUT_SMAC_47_16,
> MLX5_MODI_IN_SMAC_47_16},
> +	{2 * 8, MLX5_MODI_OUT_SMAC_15_0, MLX5_MODI_IN_SMAC_15_0},
> +	{2 * 8, MLX5_MODI_OUT_ETHERTYPE, MLX5_MODI_IN_ETHERTYPE},
> +	{0, 0, 0},
> +};
> +
> +struct field_modify_info modify_ipv4[] = {
> +	{1 * 8, 0, 0}, /* Ver,len. */
> +	{1 * 8, MLX5_MODI_OUT_IP_DSCP, MLX5_MODI_IN_IP_DSCP},
> +	{2 * 8, 0, 0}, /* Data length. */
> +	{4 * 8, 0, 0}, /* Fragment info. */
> +	{1 * 8, MLX5_MODI_OUT_IPV4_TTL, MLX5_MODI_IN_IPV4_TTL},
> +	{3 * 8, 0, 0}, /* Protocol and checksum. */
> +	{4 * 8, MLX5_MODI_OUT_SIPV4, MLX5_MODI_IN_SIPV4},
> +	{4 * 8, MLX5_MODI_OUT_DIPV4, MLX5_MODI_IN_DIPV4},
> +	{0, 0, 0},
> +};
> +
> +struct field_modify_info modify_ipv6[] = {
> +	{6 * 8, 0, 0}, /* Ver... */
> +	{2 * 8, MLX5_MODI_OUT_IPV6_HOPLIMIT,
> MLX5_MODI_IN_IPV6_HOPLIMIT},
> +	{4 * 8, MLX5_MODI_OUT_SIPV6_127_96,
> MLX5_MODI_IN_SIPV6_127_96},
> +	{4 * 8, MLX5_MODI_OUT_SIPV6_95_64,
> MLX5_MODI_IN_SIPV6_95_64},
> +	{4 * 8, MLX5_MODI_OUT_SIPV6_63_32,
> MLX5_MODI_IN_SIPV6_63_32},
> +	{4 * 8, MLX5_MODI_OUT_SIPV6_31_0, MLX5_MODI_IN_SIPV6_31_0},
> +	{4 * 8, MLX5_MODI_OUT_DIPV6_127_96,
> MLX5_MODI_IN_DIPV6_127_96},
> +	{4 * 8, MLX5_MODI_OUT_DIPV6_95_64,
> MLX5_MODI_IN_DIPV6_95_64},
> +	{4 * 8, MLX5_MODI_OUT_DIPV6_63_32,
> MLX5_MODI_IN_DIPV6_63_32},
> +	{4 * 8, MLX5_MODI_OUT_DIPV6_31_0, MLX5_MODI_IN_DIPV6_31_0},
> +	{0, 0, 0},
> +};
> +
> +struct field_modify_info modify_udp[] = {
> +	{2 * 8, MLX5_MODI_OUT_UDP_SPORT, MLX5_MODI_IN_UDP_SPORT},
> +	{2 * 8, MLX5_MODI_OUT_UDP_DPORT,
> MLX5_MODI_IN_UDP_DPORT},
> +	{4 * 8, 0, 0}, /* Length and checksum. */
> +	{0, 0, 0},
> +};
> +
> +struct field_modify_info modify_tcp[] = {
> +	{2 * 8, MLX5_MODI_OUT_TCP_SPORT, MLX5_MODI_IN_TCP_SPORT},
> +	{2 * 8, MLX5_MODI_OUT_TCP_DPORT, MLX5_MODI_IN_TCP_DPORT},
> +	{9 * 8, 0, 0}, /* Seq, ack and data offset. */
> +	{1 * 8, MLX5_MODI_OUT_TCP_FLAGS, MLX5_MODI_IN_TCP_FLAGS},
> +	{6 * 8, 0, 0}, /* Window, checksum and urgent pointer. */
> +	{0, 0, 0},
> +};
> +
> +/**
> + * Convert modify-header action to DV specification.
> + *
> + * @param[in] item
> + *   Pointer to item specification.
> + * @param[in] field
> + *   Pointer to field modification information.
> + * @param[in,out] resource
> + *   Pointer to the modify-header resource.
> + * @param[in] type
> + *   Type of modification.
> + * @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_convert_modify_action(struct rte_flow_item *item,
> +			      struct field_modify_info *field,
> +			      struct mlx5_flow_dv_modify_hdr_resource
> *resource,
> +			      uint32_t type,
> +			      struct rte_flow_error *error)
> +{
> +	uint32_t bits_offset;
> +	int bits;
> +	uint32_t i = resource->actions_num;
> +	struct mlx5_modification_cmd *actions = resource->actions;
> +	int found = 0;
> +	int j;
> +	int set;
> +	const uint8_t *spec = item->spec;
> +	const uint8_t *mask = item->mask;
> +
> +	for (bits_offset = 0; field->bits > 0; field++) {
> +		bits = field->bits;
> +		/* Scan and generate modify commands for each mask
> segment. */
> +		for (j = 0; j < bits; ++j) {
> +			set = mask[(bits_offset + j) / 8] & (1 << (j % 8));

I don't understand this logic. Let's say I would like to set the TCP source port. The mask item will have 0xff on the tcp.sport, the rest will be 0.
The first field will be the TCP source port which located in 2B offset of the TCP header. meaning bits  = 16, j = 0, base_offset = 0.
So it looks like the mask will be checked for offset 0, while in fact the source port located in offset of 16bits. 

I also don't understand why the whole logic is in bits granularity and not bytes.


> +			if (set && found && j != bits - 1)
> +				continue;
> +			if (set && !found) {
> +				if (!field->outer_type)
> +					DRV_LOG(DEBUG,
> +						"unsupported modification"
> +						" field");
> +				actions[i].type = type;
> +				actions[i].src_offset = j;
> +				actions[i].src_field = field->outer_type;
> +					found = 1;
> +					continue;
> +			}
> +			if ((set && (j == bits - 1)) || (found && !set)) {
> +				/* Reach end of mask or end of mask segment.
> */
> +				actions[i].bits = j - actions[i].src_offset;
> +				if (j == bits - 1)
> +					actions[i].bits++;
> +				rte_memcpy(&actions[i].data[4 - bits / 8],
> +					   &spec[bits_offset / 8], bits / 8);
> +				actions[i].data0 =
> +					rte_cpu_to_be_32(actions[i].data0);
> +				found = 0;
> +				++i;
> +				if (i > MLX5_MODIFY_NUM)
> +					return rte_flow_error_set(error,
> EINVAL,
> +
> RTE_FLOW_ERROR_TYPE_ACTION,
> +						 NULL,
> +						 "too many items to modify");
> +			}
> +			if (resource->actions_num != i)
> +				resource->actions_num = i;
> +		}
> +		bits_offset += field->bits;
> +	}
> +	if (!resource->actions_num)
> +		return rte_flow_error_set(error, EINVAL,
> +					  RTE_FLOW_ERROR_TYPE_ACTION,
> +					  NULL,
> +					  "invalid modification flow item");
> +	return 0;
> +}
> +
> +/**
> + * Convert modify-header set IPv4 address action to DV specification.
> + *
> + * @param[in,out] resource
> + *   Pointer to the modify-header resource.
> + * @param[in] action
> + *   Pointer to action specification.
> + * @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_convert_action_modify_ipv4
> +			(struct mlx5_flow_dv_modify_hdr_resource *resource,
> +			 const struct rte_flow_action *action,
> +			 struct rte_flow_error *error)
> +{
> +	const struct rte_flow_action_set_ipv4 *conf =
> +		(const struct rte_flow_action_set_ipv4 *)(action->conf);
> +	struct rte_flow_item item = { .type = RTE_FLOW_ITEM_TYPE_IPV4 };
> +	struct rte_flow_item_ipv4 ipv4;
> +	struct rte_flow_item_ipv4 ipv4_mask;
> +
> +	memset(&ipv4, 0, sizeof(ipv4));
> +	memset(&ipv4_mask, 0, sizeof(ipv4_mask));
> +	if (action->type == RTE_FLOW_ACTION_TYPE_SET_IPV4_SRC) {
> +		ipv4.hdr.src_addr = conf->ipv4_addr;
> +		ipv4_mask.hdr.src_addr =
> rte_flow_item_ipv4_mask.hdr.src_addr;
> +	} else {
> +		ipv4.hdr.dst_addr = conf->ipv4_addr;
> +		ipv4_mask.hdr.dst_addr =
> rte_flow_item_ipv4_mask.hdr.dst_addr;
> +	}
> +	item.spec = &ipv4;
> +	item.mask = &ipv4_mask;
> +	return flow_dv_convert_modify_action(&item, modify_ipv4, resource,
> +					     MLX5_MODIFICATION_TYPE_SET,
> error); }
> +
> +/**
> + * Convert modify-header set IPv6 address action to DV specification.
> + *
> + * @param[in,out] resource
> + *   Pointer to the modify-header resource.
> + * @param[in] action
> + *   Pointer to action specification.
> + * @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_convert_action_modify_ipv6
> +			(struct mlx5_flow_dv_modify_hdr_resource *resource,
> +			 const struct rte_flow_action *action,
> +			 struct rte_flow_error *error)
> +{
> +	const struct rte_flow_action_set_ipv6 *conf =
> +		(const struct rte_flow_action_set_ipv6 *)(action->conf);
> +	struct rte_flow_item item = { .type = RTE_FLOW_ITEM_TYPE_IPV6 };
> +	struct rte_flow_item_ipv6 ipv6;
> +	struct rte_flow_item_ipv6 ipv6_mask;
> +
> +	memset(&ipv6, 0, sizeof(ipv6));
> +	memset(&ipv6_mask, 0, sizeof(ipv6_mask));
> +	if (action->type == RTE_FLOW_ACTION_TYPE_SET_IPV6_SRC) {
> +		memcpy(&ipv6.hdr.src_addr, &conf->ipv6_addr,
> +		       sizeof(ipv6.hdr.src_addr));
> +		memcpy(&ipv6_mask.hdr.src_addr,
> +		       &rte_flow_item_ipv6_mask.hdr.src_addr,
> +		       sizeof(ipv6.hdr.src_addr));
> +	} else {
> +		memcpy(&ipv6.hdr.dst_addr, &conf->ipv6_addr,
> +		       sizeof(ipv6.hdr.dst_addr));
> +		memcpy(&ipv6_mask.hdr.dst_addr,
> +		       &rte_flow_item_ipv6_mask.hdr.dst_addr,
> +		       sizeof(ipv6.hdr.dst_addr));
> +	}
> +	item.spec = &ipv6;
> +	item.mask = &ipv6_mask;
> +	return flow_dv_convert_modify_action(&item, modify_ipv6, resource,
> +					     MLX5_MODIFICATION_TYPE_SET,
> error); }
> +
> +/**
> + * Convert modify-header set MAC address action to DV specification.
> + *
> + * @param[in,out] resource
> + *   Pointer to the modify-header resource.
> + * @param[in] action
> + *   Pointer to action specification.
> + * @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_convert_action_modify_mac
> +			(struct mlx5_flow_dv_modify_hdr_resource *resource,
> +			 const struct rte_flow_action *action,
> +			 struct rte_flow_error *error)
> +{
> +	const struct rte_flow_action_set_mac *conf =
> +		(const struct rte_flow_action_set_mac *)(action->conf);
> +	struct rte_flow_item item = { .type = RTE_FLOW_ITEM_TYPE_ETH };
> +	struct rte_flow_item_eth eth;
> +	struct rte_flow_item_eth eth_mask;
> +
> +	memset(&eth, 0, sizeof(eth));
> +	memset(&eth_mask, 0, sizeof(eth_mask));
> +	if (action->type == RTE_FLOW_ACTION_TYPE_SET_MAC_SRC) {
> +		memcpy(&eth.src.addr_bytes, &conf->mac_addr,
> +		       sizeof(eth.src.addr_bytes));
> +		memcpy(&eth_mask.src.addr_bytes,
> +		       &rte_flow_item_eth_mask.src.addr_bytes,
> +		       sizeof(eth_mask.src.addr_bytes));
> +	} else {
> +		memcpy(&eth.dst.addr_bytes, &conf->mac_addr,
> +		       sizeof(eth.dst.addr_bytes));
> +		memcpy(&eth_mask.dst.addr_bytes,
> +		       &rte_flow_item_eth_mask.dst.addr_bytes,
> +		       sizeof(eth_mask.dst.addr_bytes));
> +	}
> +	item.spec = &eth;
> +	item.mask = &eth_mask;
> +	return flow_dv_convert_modify_action(&item, modify_eth, resource,
> +					     MLX5_MODIFICATION_TYPE_SET,
> error); }
> +
> +/**
> + * Convert modify-header set TP action to DV specification.
> + *
> + * @param[in,out] resource
> + *   Pointer to the modify-header resource.
> + * @param[in] action
> + *   Pointer to action specification.
> + * @param[in] items
> + *   Pointer to rte_flow_item objects list.
> + * @param[in] attr
> + *   Pointer to flow attributes structure.
> + * @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_convert_action_modify_tp
> +			(struct mlx5_flow_dv_modify_hdr_resource *resource,
> +			 const struct rte_flow_action *action,
> +			 const struct rte_flow_item *items,
> +			 union flow_dv_attr *attr,
> +			 struct rte_flow_error *error)
> +{
> +	const struct rte_flow_action_set_tp *conf =
> +		(const struct rte_flow_action_set_tp *)(action->conf);
> +	struct rte_flow_item item;
> +	struct rte_flow_item_udp udp;
> +	struct rte_flow_item_udp udp_mask;
> +	struct rte_flow_item_tcp tcp;
> +	struct rte_flow_item_tcp tcp_mask;
> +	struct field_modify_info *field;
> +
> +	if (!attr->valid)
> +		flow_dv_attr_init(items, attr);
> +	if (attr->udp) {
> +		memset(&udp, 0, sizeof(udp));
> +		memset(&udp_mask, 0, sizeof(udp_mask));
> +		if (action->type == RTE_FLOW_ACTION_TYPE_SET_TP_SRC) {
> +			udp.hdr.src_port = conf->port;
> +			udp_mask.hdr.src_port =
> +
> 	rte_flow_item_udp_mask.hdr.src_port;
> +		} else {
> +			udp.hdr.dst_port = conf->port;
> +			udp_mask.hdr.dst_port =
> +
> 	rte_flow_item_udp_mask.hdr.dst_port;
> +		}
> +		item.type = RTE_FLOW_ITEM_TYPE_UDP;
> +		item.spec = &udp;
> +		item.mask = &udp_mask;
> +		field = modify_udp;
> +	}
> +	if (attr->tcp) {
> +		memset(&tcp, 0, sizeof(tcp));
> +		memset(&tcp_mask, 0, sizeof(tcp_mask));
> +		if (action->type == RTE_FLOW_ACTION_TYPE_SET_TP_SRC) {
> +			tcp.hdr.src_port = conf->port;
> +			tcp_mask.hdr.src_port =
> +					rte_flow_item_tcp_mask.hdr.src_port;
> +		} else {
> +			tcp.hdr.dst_port = conf->port;
> +			tcp_mask.hdr.dst_port =
> +					rte_flow_item_tcp_mask.hdr.dst_port;
> +		}
> +		item.type = RTE_FLOW_ITEM_TYPE_TCP;
> +		item.spec = &tcp;
> +		item.mask = &tcp_mask;
> +		field = modify_tcp;
> +	}
> +	return flow_dv_convert_modify_action(&item, field, resource,
> +					     MLX5_MODIFICATION_TYPE_SET,
> error); }
> +
> +/**
> + * Convert modify-header set TTL action to DV specification.
> + *
> + * @param[in,out] resource
> + *   Pointer to the modify-header resource.
> + * @param[in] action
> + *   Pointer to action specification.
> + * @param[in] items
> + *   Pointer to rte_flow_item objects list.
> + * @param[in] attr
> + *   Pointer to flow attributes structure.
> + * @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_convert_action_modify_ttl
> +			(struct mlx5_flow_dv_modify_hdr_resource *resource,
> +			 const struct rte_flow_action *action,
> +			 const struct rte_flow_item *items,
> +			 union flow_dv_attr *attr,
> +			 struct rte_flow_error *error)
> +{
> +	const struct rte_flow_action_set_ttl *conf =
> +		(const struct rte_flow_action_set_ttl *)(action->conf);
> +	struct rte_flow_item item;
> +	struct rte_flow_item_ipv4 ipv4;
> +	struct rte_flow_item_ipv4 ipv4_mask;
> +	struct rte_flow_item_ipv6 ipv6;
> +	struct rte_flow_item_ipv6 ipv6_mask;
> +	struct field_modify_info *field;
> +
> +	if (!attr->valid)
> +		flow_dv_attr_init(items, attr);
> +	if (attr->ipv4) {
> +		memset(&ipv4, 0, sizeof(ipv4));
> +		memset(&ipv4_mask, 0, sizeof(ipv4_mask));
> +		ipv4.hdr.time_to_live = conf->ttl_value;
> +		ipv4_mask.hdr.time_to_live = 0xFF;
> +		item.type = RTE_FLOW_ITEM_TYPE_IPV4;
> +		item.spec = &ipv4;
> +		item.mask = &ipv4_mask;
> +		field = modify_ipv4;
> +	}
> +	if (attr->ipv6) {
> +		memset(&ipv6, 0, sizeof(ipv6));
> +		memset(&ipv6_mask, 0, sizeof(ipv6_mask));
> +		ipv6.hdr.hop_limits = conf->ttl_value;
> +		ipv6_mask.hdr.hop_limits = 0xFF;
> +		item.type = RTE_FLOW_ITEM_TYPE_IPV6;
> +		item.spec = &ipv6;
> +		item.mask = &ipv6_mask;
> +		field = modify_ipv6;
> +	}
> +	return flow_dv_convert_modify_action(&item, field, resource,
> +					     MLX5_MODIFICATION_TYPE_SET,
> error); }
> +
> +/**
> + * Convert modify-header decrement TTL action to DV specification.
> + *
> + * @param[in,out] resource
> + *   Pointer to the modify-header resource.
> + * @param[in] action
> + *   Pointer to action specification.
> + * @param[in] items
> + *   Pointer to rte_flow_item objects list.
> + * @param[in] attr
> + *   Pointer to flow attributes structure.
> + * @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_convert_action_modify_dec_ttl
> +			(struct mlx5_flow_dv_modify_hdr_resource *resource,
> +			 const struct rte_flow_item *items,
> +			 union flow_dv_attr *attr,
> +			 struct rte_flow_error *error)
> +{
> +	struct rte_flow_item item;
> +	struct rte_flow_item_ipv4 ipv4;
> +	struct rte_flow_item_ipv4 ipv4_mask;
> +	struct rte_flow_item_ipv6 ipv6;
> +	struct rte_flow_item_ipv6 ipv6_mask;
> +	struct field_modify_info *field;
> +
> +	if (!attr->valid)
> +		flow_dv_attr_init(items, attr);
> +	if (attr->ipv4) {
> +		memset(&ipv4, 0, sizeof(ipv4));
> +		memset(&ipv4_mask, 0, sizeof(ipv4_mask));
> +		ipv4.hdr.time_to_live = 0xFF;
> +		ipv4_mask.hdr.time_to_live = 0xFF;
> +		item.type = RTE_FLOW_ITEM_TYPE_IPV4;
> +		item.spec = &ipv4;
> +		item.mask = &ipv4_mask;
> +		field = modify_ipv4;
> +	}
> +	if (attr->ipv6) {
> +		memset(&ipv6, 0, sizeof(ipv6));
> +		memset(&ipv6_mask, 0, sizeof(ipv6_mask));
> +		ipv6.hdr.hop_limits = 0xFF;
> +		ipv6_mask.hdr.hop_limits = 0xFF;
> +		item.type = RTE_FLOW_ITEM_TYPE_IPV6;
> +		item.spec = &ipv6;
> +		item.mask = &ipv6_mask;
> +		field = modify_ipv6;
> +	}
> +	return flow_dv_convert_modify_action(&item, field, resource,
> +					     MLX5_MODIFICATION_TYPE_ADD,
> error); }
> +
>  /**
>   * Validate META item.
>   *
> @@ -166,6 +664,11 @@
>  					  RTE_FLOW_ERROR_TYPE_ACTION,
> NULL,
>  					  "can only have a single encap or"
>  					  " decap action in a flow");
> +	if (action_flags & MLX5_FLOW_MODIFY_HDR_ACTIONS)
> +		return rte_flow_error_set(error, EINVAL,
> +					  RTE_FLOW_ERROR_TYPE_ACTION,
> NULL,
> +					  "can't have decap action after"
> +					  " modify action");
>  	if (attr->egress)
>  		return rte_flow_error_set(error, ENOTSUP,
> 
> RTE_FLOW_ERROR_TYPE_ATTR_EGRESS, @@ -254,6 +757,11 @@
>  					  RTE_FLOW_ERROR_TYPE_ACTION,
> NULL,
>  					  "can only have a single decap"
>  					  " action in a flow");
> +	if (action_flags & MLX5_FLOW_MODIFY_HDR_ACTIONS)
> +		return rte_flow_error_set(error, EINVAL,
> +					  RTE_FLOW_ERROR_TYPE_ACTION,
> NULL,
> +					  "can't have decap action after"
> +					  " modify action");
>  	/* decap action is valid on egress only if it is followed by encap */
>  	if (attr->egress) {
>  		for (; action->type != RTE_FLOW_ACTION_TYPE_END && @@ -
> 270,7 +778,6 @@
>  	return 0;
>  }
> 
> -
>  /**
>   * Find existing encap/decap resource or create and register a new one.
>   *
> @@ -704,6 +1211,302 @@
>  }
> 
>  /**
> + * Validate the modify-header actions.
> + *
> + * @param[in] action_flags
> + *   Holds the actions detected until now.
> + * @param[in] action
> + *   Pointer to the modify action.
> + * @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_modify_hdr(const uint64_t action_flags,
> +				   const struct rte_flow_action *action,
> +				   struct rte_flow_error *error)
> +{
> +	if (action->type != RTE_FLOW_ACTION_TYPE_DEC_TTL && !action-
> >conf)
> +		return rte_flow_error_set(error, EINVAL,
> +
> RTE_FLOW_ERROR_TYPE_ACTION_CONF,
> +					  NULL, "action configuration not set");
> +	if (action_flags & MLX5_FLOW_ENCAP_ACTIONS)
> +		return rte_flow_error_set(error, EINVAL,
> +					  RTE_FLOW_ERROR_TYPE_ACTION,
> NULL,
> +					  "can't have encap action before"
> +					  " modify action");
> +	return 0;
> +}
> +
> +/**
> + * Validate the modify-header MAC address actions.
> + *
> + * @param[in] action_flags
> + *   Holds the actions detected until now.
> + * @param[in] action
> + *   Pointer to the modify action.
> + * @param[in] item_flags
> + *   Holds the items detected.
> + * @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_modify_mac(const uint64_t action_flags,
> +				   const struct rte_flow_action *action,
> +				   const uint64_t item_flags,
> +				   struct rte_flow_error *error)
> +{
> +	int ret = 0;
> +
> +	ret = flow_dv_validate_action_modify_hdr(action_flags, action, error);
> +	if (!ret) {
> +		if (!(item_flags & MLX5_FLOW_LAYER_L2))
> +			return rte_flow_error_set(error, EINVAL,
> +
> RTE_FLOW_ERROR_TYPE_ACTION,
> +						  NULL,
> +						  "no L2 item in pattern");
> +		if (action_flags & MLX5_FLOW_DECAP_ACTIONS)
> +			return rte_flow_error_set(error, EINVAL,
> +
> RTE_FLOW_ERROR_TYPE_ACTION,
> +						  NULL, "after decap no L2 "
> +						  "item in pattern");
> +	}

I didn't get this part, why there is no L2?
Same question for the other verifications present here. 

> +	return ret;
> +}
> +
> +/**
> + * Validate the modify-header IPv4 address actions.
> + *
> + * @param[in] action_flags
> + *   Holds the actions detected until now.
> + * @param[in] action
> + *   Pointer to the modify action.
> + * @param[in] item_flags
> + *   Holds the items detected.
> + * @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_modify_ipv4(const uint64_t action_flags,
> +				    const struct rte_flow_action *action,
> +				    const uint64_t item_flags,
> +				    struct rte_flow_error *error)
> +{
> +	int ret = 0;
> +
> +	ret = flow_dv_validate_action_modify_hdr(action_flags, action, error);
> +	if (!ret) {
> +		if (!(item_flags & MLX5_FLOW_LAYER_L3_IPV4))
> +			return rte_flow_error_set(error, EINVAL,
> +
> RTE_FLOW_ERROR_TYPE_ACTION,
> +						  NULL,
> +						  "no ipv4 item in pattern");
> +		if (action_flags & MLX5_FLOW_DECAP_ACTIONS)
> +			return rte_flow_error_set(error, EINVAL,
> +
> RTE_FLOW_ERROR_TYPE_ACTION,
> +						  NULL, "after decap "
> +						  "no ipv4 item in pattern");
> +	}
> +	return ret;
> +}
> +
> +/**
> + * Validate the modify-header IPv6 address actions.
> + *
> + * @param[in] action_flags
> + *   Holds the actions detected until now.
> + * @param[in] action
> + *   Pointer to the modify action.
> + * @param[in] item_flags
> + *   Holds the items detected.
> + * @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_modify_ipv6(const uint64_t action_flags,
> +				    const struct rte_flow_action *action,
> +				    const uint64_t item_flags,
> +				    struct rte_flow_error *error)
> +{
> +	int ret = 0;
> +
> +	ret = flow_dv_validate_action_modify_hdr(action_flags, action, error);
> +	if (!ret) {
> +		if (!(item_flags & MLX5_FLOW_LAYER_L3_IPV6))
> +			return rte_flow_error_set(error, EINVAL,
> +
> RTE_FLOW_ERROR_TYPE_ACTION,
> +						  NULL,
> +						  "no ipv6 item in pattern");
> +		if (action_flags & MLX5_FLOW_DECAP_ACTIONS)
> +			return rte_flow_error_set(error, EINVAL,
> +
> RTE_FLOW_ERROR_TYPE_ACTION,
> +						  NULL, "after decap "
> +						  "no ipv6 item in pattern");
> +	}
> +	return ret;
> +}
> +
> +/**
> + * Validate the modify-header TP actions.
> + *
> + * @param[in] action_flags
> + *   Holds the actions detected until now.
> + * @param[in] action
> + *   Pointer to the modify action.
> + * @param[in] item_flags
> + *   Holds the items detected.
> + * @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_modify_tp(const uint64_t action_flags,
> +				  const struct rte_flow_action *action,
> +				  const uint64_t item_flags,
> +				  struct rte_flow_error *error)
> +{
> +	int ret = 0;
> +
> +	ret = flow_dv_validate_action_modify_hdr(action_flags, action, error);
> +	if (!ret) {
> +		if (!(item_flags & MLX5_FLOW_LAYER_L4))
> +			return rte_flow_error_set(error, EINVAL,
> +
> RTE_FLOW_ERROR_TYPE_ACTION,
> +						  NULL, "no transport layer "
> +						  "in pattern");
> +		if (action_flags & MLX5_FLOW_DECAP_ACTIONS)
> +			return rte_flow_error_set(error, EINVAL,
> +
> RTE_FLOW_ERROR_TYPE_ACTION,
> +						  NULL, "after decap no "
> +						  "transport layer in pattern");
> +	}
> +	return ret;
> +}
> +
> +/**
> + * Validate the modify-header TTL actions.
> + *
> + * @param[in] action_flags
> + *   Holds the actions detected until now.
> + * @param[in] action
> + *   Pointer to the modify action.
> + * @param[in] item_flags
> + *   Holds the items detected.
> + * @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_modify_ttl(const uint64_t action_flags,
> +				   const struct rte_flow_action *action,
> +				   const uint64_t item_flags,
> +				   struct rte_flow_error *error)
> +{
> +	int ret = 0;
> +
> +	ret = flow_dv_validate_action_modify_hdr(action_flags, action, error);
> +	if (!ret) {
> +		if (!(item_flags & MLX5_FLOW_LAYER_L3))
> +			return rte_flow_error_set(error, EINVAL,
> +
> RTE_FLOW_ERROR_TYPE_ACTION,
> +						  NULL,
> +						  "no IP protocol in pattern");
> +		if (action_flags & MLX5_FLOW_DECAP_ACTIONS)
> +			return rte_flow_error_set(error, EINVAL,
> +
> RTE_FLOW_ERROR_TYPE_ACTION,
> +						  NULL, "after decap "
> +						  "no IP protocol in pattern");
> +	}
> +	return ret;
> +}
> +
> +/**
> + * Find existing modify-header resource or create and register a new one.
> + *
> + * @param dev[in, out]
> + *   Pointer to rte_eth_dev structure.
> + * @param[in, out] resource
> + *   Pointer to modify-header 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_modify_hdr_resource_register
> +			(struct rte_eth_dev *dev,
> +			 struct mlx5_flow_dv_modify_hdr_resource *resource,
> +			 struct mlx5_flow *dev_flow,
> +			 struct rte_flow_error *error)
> +{
> +	struct priv *priv = dev->data->dev_private;
> +	struct mlx5_flow_dv_modify_hdr_resource *cache_resource;
> +
> +	/* Lookup a matching resource from cache. */
> +	LIST_FOREACH(cache_resource, &priv->modify_cmds, next) {
> +		if (resource->ft_type == cache_resource->ft_type &&
> +		    resource->actions_num == cache_resource->actions_num
> &&
> +		    !memcmp((const void *)resource->actions,
> +			    (const void *)cache_resource->actions,
> +			    (resource->actions_num *
> +					    sizeof(resource->actions[0])))) {
> +			DRV_LOG(DEBUG, "modify-header resource %p: refcnt
> %d++",
> +				(void *)cache_resource,
> +				rte_atomic32_read(&cache_resource-
> >refcnt));
> +			rte_atomic32_inc(&cache_resource->refcnt);
> +			dev_flow->dv.modify_hdr = cache_resource;
> +			return 0;
> +		}
> +	}
> +	/* Register new modify-header 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->verbs_action =
> +		mlx5_glue->dv_create_flow_action_modify_header
> +					(priv->ctx,
> +					 cache_resource->actions_num *
> +					 sizeof(cache_resource->actions[0]),
> +					 (uint64_t *)cache_resource->actions,
> +					 cache_resource->ft_type);
> +	if (!cache_resource->verbs_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->modify_cmds, cache_resource, next);
> +	dev_flow->dv.modify_hdr = cache_resource;
> +	DRV_LOG(DEBUG, "new modify-header resource %p: refcnt %d++",
> +		(void *)cache_resource,
> +		rte_atomic32_read(&cache_resource->refcnt));
> +	return 0;
> +}
> +
> +/**
>   * Verify the @p attributes will be correctly understood by the NIC and store
>   * them in the @p flow if everything is correct.
>   *
> @@ -1014,6 +1817,87 @@
>  			action_flags |= MLX5_FLOW_ACTION_RAW_DECAP;
>  			++actions_n;
>  			break;
> +		case RTE_FLOW_ACTION_TYPE_SET_MAC_SRC:
> +		case RTE_FLOW_ACTION_TYPE_SET_MAC_DST:
> +			ret =
> flow_dv_validate_action_modify_mac(action_flags,
> +								 actions,
> +								 item_flags,
> +								 error);
> +			if (ret < 0)
> +				return ret;
> +			/* Count all modify-header actions as one action. */
> +			if (!(action_flags &
> MLX5_FLOW_MODIFY_HDR_ACTIONS))
> +				++actions_n;
> +			action_flags |= actions->type ==
> +
> 	RTE_FLOW_ACTION_TYPE_SET_MAC_SRC ?
> +
> 	MLX5_FLOW_ACTION_SET_MAC_SRC :
> +
> 	MLX5_FLOW_ACTION_SET_MAC_DST;
> +			break;
> +
> +		case RTE_FLOW_ACTION_TYPE_SET_IPV4_SRC:
> +		case RTE_FLOW_ACTION_TYPE_SET_IPV4_DST:
> +			ret =
> flow_dv_validate_action_modify_ipv4(action_flags,
> +								  actions,
> +								  item_flags,
> +								  error);
> +			if (ret < 0)
> +				return ret;
> +			/* Count all modify-header actions as one action. */
> +			if (!(action_flags &
> MLX5_FLOW_MODIFY_HDR_ACTIONS))
> +				++actions_n;
> +			action_flags |= actions->type ==
> +
> 	RTE_FLOW_ACTION_TYPE_SET_IPV4_SRC ?
> +
> 	MLX5_FLOW_ACTION_SET_IPV4_SRC :
> +
> 	MLX5_FLOW_ACTION_SET_IPV4_DST;
> +			break;
> +		case RTE_FLOW_ACTION_TYPE_SET_IPV6_SRC:
> +		case RTE_FLOW_ACTION_TYPE_SET_IPV6_DST:
> +			ret =
> flow_dv_validate_action_modify_ipv6(action_flags,
> +								  actions,
> +								  item_flags,
> +								  error);
> +			if (ret < 0)
> +				return ret;
> +			/* Count all modify-header actions as one action. */
> +			if (!(action_flags &
> MLX5_FLOW_MODIFY_HDR_ACTIONS))
> +				++actions_n;
> +			action_flags |= actions->type ==
> +
> 	RTE_FLOW_ACTION_TYPE_SET_IPV6_SRC ?
> +
> 	MLX5_FLOW_ACTION_SET_IPV6_SRC :
> +
> 	MLX5_FLOW_ACTION_SET_IPV6_DST;
> +			break;
> +		case RTE_FLOW_ACTION_TYPE_SET_TP_SRC:
> +		case RTE_FLOW_ACTION_TYPE_SET_TP_DST:
> +			ret = flow_dv_validate_action_modify_tp(action_flags,
> +								actions,
> +								item_flags,
> +								error);
> +			if (ret < 0)
> +				return ret;
> +			/* Count all modify-header actions as one action. */
> +			if (!(action_flags &
> MLX5_FLOW_MODIFY_HDR_ACTIONS))
> +				++actions_n;
> +			action_flags |= actions->type ==
> +
> 	RTE_FLOW_ACTION_TYPE_SET_TP_SRC ?
> +
> 	MLX5_FLOW_ACTION_SET_TP_SRC :
> +
> 	MLX5_FLOW_ACTION_SET_TP_DST;
> +			break;
> +		case RTE_FLOW_ACTION_TYPE_DEC_TTL:
> +		case RTE_FLOW_ACTION_TYPE_SET_TTL:
> +			ret = flow_dv_validate_action_modify_ttl(action_flags,
> +								 actions,
> +								 item_flags,
> +								 error);
> +			if (ret < 0)
> +				return ret;
> +			/* Count all modify-header actions as one action. */
> +			if (!(action_flags &
> MLX5_FLOW_MODIFY_HDR_ACTIONS))
> +				++actions_n;
> +			action_flags |= actions->type ==
> +					RTE_FLOW_ACTION_TYPE_SET_TTL ?
> +
> 	MLX5_FLOW_ACTION_SET_TTL :
> +
> 	MLX5_FLOW_ACTION_DEC_TTL;
> +			break;
>  		default:
>  			return rte_flow_error_set(error, ENOTSUP,
> 
> RTE_FLOW_ERROR_TYPE_ACTION,
> @@ -1895,10 +2779,16 @@
>  		},
>  	};
>  	int actions_n = 0;
> +	bool actions_end = false;
> +	struct mlx5_flow_dv_modify_hdr_resource res = {
> +		.ft_type = attr->egress ? MLX5DV_FLOW_TABLE_TYPE_NIC_TX
> :
> +
> MLX5DV_FLOW_TABLE_TYPE_NIC_RX
> +	};
> +	union flow_dv_attr flow_attr = { .attr = 0 };
> 
>  	if (priority == MLX5_FLOW_PRIO_RSVD)
>  		priority = priv->config.flow_prio - 1;
> -	for (; actions->type != RTE_FLOW_ACTION_TYPE_END; actions++) {
> +	for (; !actions_end ; actions++) {
>  		const struct rte_flow_action_queue *queue;
>  		const struct rte_flow_action_rss *rss;
>  		const struct rte_flow_action *action = actions; @@ -2025,6
> +2915,77 @@
>  			/* If decap is followed by encap, handle it at encap. */
>  			action_flags |= MLX5_FLOW_ACTION_RAW_DECAP;
>  			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,
> +							      error))
> +				return -rte_errno;
> +			action_flags |= actions->type ==
> +
> 	RTE_FLOW_ACTION_TYPE_SET_MAC_SRC ?
> +					MLX5_FLOW_ACTION_SET_MAC_SRC
> :
> +					MLX5_FLOW_ACTION_SET_MAC_DST;
> +			break;
> +		case RTE_FLOW_ACTION_TYPE_SET_IPV4_SRC:
> +		case RTE_FLOW_ACTION_TYPE_SET_IPV4_DST:
> +			if (flow_dv_convert_action_modify_ipv4(&res, actions,
> +							       error))
> +				return -rte_errno;
> +			action_flags |= actions->type ==
> +
> 	RTE_FLOW_ACTION_TYPE_SET_IPV4_SRC ?
> +					MLX5_FLOW_ACTION_SET_IPV4_SRC :
> +					MLX5_FLOW_ACTION_SET_IPV4_DST;
> +			break;
> +		case RTE_FLOW_ACTION_TYPE_SET_IPV6_SRC:
> +		case RTE_FLOW_ACTION_TYPE_SET_IPV6_DST:
> +			if (flow_dv_convert_action_modify_ipv6(&res, actions,
> +							       error))
> +				return -rte_errno;
> +			action_flags |= actions->type ==
> +
> 	RTE_FLOW_ACTION_TYPE_SET_IPV6_SRC ?
> +					MLX5_FLOW_ACTION_SET_IPV6_SRC :
> +					MLX5_FLOW_ACTION_SET_IPV6_DST;
> +			break;
> +		case RTE_FLOW_ACTION_TYPE_SET_TP_SRC:
> +		case RTE_FLOW_ACTION_TYPE_SET_TP_DST:
> +			if (flow_dv_convert_action_modify_tp(&res, actions,
> +							     items, &flow_attr,
> +							     error))
> +				return -rte_errno;
> +			action_flags |= actions->type ==
> +
> 	RTE_FLOW_ACTION_TYPE_SET_TP_SRC ?
> +					MLX5_FLOW_ACTION_SET_TP_SRC :
> +					MLX5_FLOW_ACTION_SET_TP_DST;
> +			break;
> +		case RTE_FLOW_ACTION_TYPE_DEC_TTL:
> +			if (flow_dv_convert_action_modify_dec_ttl(&res,
> items,
> +								  &flow_attr,
> +								  error))
> +				return -rte_errno;
> +			action_flags |= MLX5_FLOW_ACTION_DEC_TTL;
> +			break;
> +		case RTE_FLOW_ACTION_TYPE_SET_TTL:
> +			if (flow_dv_convert_action_modify_ttl(&res, actions,
> +							     items, &flow_attr,
> +							     error))
> +				return -rte_errno;
> +			action_flags |= MLX5_FLOW_ACTION_SET_TTL;
> +			break;
> +		case RTE_FLOW_ACTION_TYPE_END:
> +			actions_end = true;
> +			if (action_flags &
> MLX5_FLOW_MODIFY_HDR_ACTIONS) {
> +				/* create modify action if needed. */
> +				if (flow_dv_modify_hdr_resource_register
> +								(dev, &res,
> +								 dev_flow,
> +								 error))
> +					return -rte_errno;
> +				dev_flow->dv.actions[actions_n].type =
> +
> 	MLX5DV_FLOW_ACTION_IBV_FLOW_ACTION;
> +				dev_flow->dv.actions[actions_n].action =
> +					dev_flow->dv.modify_hdr-
> >verbs_action;
> +				actions_n++;
> +			}
> +			break;
>  		default:
>  			break;
>  		}
> @@ -2309,6 +3270,37 @@
>  }
> 
>  /**
> + * Release a modify-header resource.
> + *
> + * @param flow
> + *   Pointer to mlx5_flow.
> + *
> + * @return
> + *   1 while a reference on it exists, 0 when freed.
> + */
> +static int
> +flow_dv_modify_hdr_resource_release(struct mlx5_flow *flow) {
> +	struct mlx5_flow_dv_modify_hdr_resource *cache_resource =
> +						flow->dv.modify_hdr;
> +
> +	assert(cache_resource->verbs_action);
> +	DRV_LOG(DEBUG, "modify-header 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->verbs_action));
> +		LIST_REMOVE(cache_resource, next);
> +		rte_free(cache_resource);
> +		DRV_LOG(DEBUG, "modify-header resource %p: removed",
> +			(void *)cache_resource);
> +		return 0;
> +	}
> +	return 1;
> +}
> +
> +/**
>   * Remove the flow from the NIC but keeps it in memory.
>   *
>   * @param[in] dev
> @@ -2365,6 +3357,8 @@
>  			flow_dv_matcher_release(dev, dev_flow);
>  		if (dev_flow->dv.encap_decap)
>  			flow_dv_encap_decap_resource_release(dev_flow);
> +		if (dev_flow->dv.modify_hdr)
> +			flow_dv_modify_hdr_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
> dd10ad6..a806d92 100644
> --- a/drivers/net/mlx5/mlx5_glue.c
> +++ b/drivers/net/mlx5/mlx5_glue.c
> @@ -479,6 +479,26 @@
>  #endif
>  }
> 
> +static struct ibv_flow_action *
> +mlx5_glue_dv_create_flow_action_modify_header
> +					(struct ibv_context *ctx,
> +					 size_t actions_sz,
> +					 uint64_t actions[],
> +					 enum mlx5dv_flow_table_type
> ft_type) { #ifdef
> +HAVE_IBV_FLOW_DV_SUPPORT
> +	return mlx5dv_create_flow_action_modify_header(ctx, actions_sz,
> +						       actions, ft_type);
> +#else
> +	(void)ctx;
> +	(void)actions_sz;
> +	(void)actions;
> +	(void)ft_type;
> +	return NULL;
> +#endif
> +}
> +
> +
>  alignas(RTE_CACHE_LINE_SIZE)
>  const struct mlx5_glue *mlx5_glue = &(const struct mlx5_glue){
>  	.version = MLX5_GLUE_VERSION,
> @@ -535,4 +555,6 @@
>  	.dv_create_flow = mlx5_glue_dv_create_flow,
>  	.dv_create_flow_action_packet_reformat =
>  			mlx5_glue_dv_create_flow_action_packet_reformat,
> +	.dv_create_flow_action_modify_header =
> +			mlx5_glue_dv_create_flow_action_modify_header,
>  };
> diff --git a/drivers/net/mlx5/mlx5_glue.h b/drivers/net/mlx5/mlx5_glue.h
> index 2d92ba8..9cfe836 100644
> --- a/drivers/net/mlx5/mlx5_glue.h
> +++ b/drivers/net/mlx5/mlx5_glue.h
> @@ -164,6 +164,11 @@ struct mlx5_glue {
>  		 void *data,
>  		 enum mlx5dv_flow_action_packet_reformat_type
> reformat_type,
>  		 enum mlx5dv_flow_table_type ft_type);
> +	struct ibv_flow_action *(*dv_create_flow_action_modify_header)
> +					(struct ibv_context *ctx,
> +					 size_t actions_sz,
> +					 uint64_t actions[],
> +					 enum mlx5dv_flow_table_type
> ft_type);

Need to bump up the LIB_GLUE_VERSION due to this ABI change. 

>  };
> 
>  const struct mlx5_glue *mlx5_glue;
> diff --git a/drivers/net/mlx5/mlx5_prm.h b/drivers/net/mlx5/mlx5_prm.h index
> 29742b1..545f84a 100644
> --- a/drivers/net/mlx5/mlx5_prm.h
> +++ b/drivers/net/mlx5/mlx5_prm.h
> @@ -280,8 +280,17 @@ struct mlx5_cqe {
>  /* CQE format value. */
>  #define MLX5_COMPRESSED 0x3
> 
> +/* Write a specific data value to a field. */ #define
> +MLX5_MODIFICATION_TYPE_SET 1
> +
> +/* Add a specific data value to a field. */ #define
> +MLX5_MODIFICATION_TYPE_ADD 2
> +
> +/* Copy a specific data value to another one in a packet. */ #define
> +MLX5_MODIFICATION_TYPE_COPY 3
> +
>  /* The field of packet to be modified. */ -enum mlx5_modificaiton_field {
> +enum mlx5_modification_field {
>  	MLX5_MODI_OUT_SMAC_47_16 = 1,
>  	MLX5_MODI_OUT_SMAC_15_0,
>  	MLX5_MODI_OUT_ETHERTYPE,
> --
> 1.8.3.1

  reply	other threads:[~2018-12-25 11:38 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-23  9:58 Dekel Peled
2018-12-25 11:38 ` Shahaf Shuler [this message]
2018-12-25 16:00   ` Dekel Peled
2018-12-26 10:01 ` [dpdk-dev] [PATCH v2] net/mlx5: support modify header " Dekel Peled
2018-12-27 11:09   ` [dpdk-dev] [PATCH v3] " Dekel Peled
2018-12-27 18:35     ` Shahaf Shuler

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DB7PR05MB44260570F8B413552330EEE6C3B40@DB7PR05MB4426.eurprd05.prod.outlook.com \
    --to=shahafs@mellanox.com \
    --cc=dekelp@mellanox.com \
    --cc=dev@dpdk.org \
    --cc=orika@mellanox.com \
    --cc=yskoh@mellanox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).