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

Thanks, PSB.

> -----Original Message-----
> From: Shahaf Shuler
> Sent: Tuesday, December 25, 2018 1:38 PM
> To: Dekel Peled <dekelp@mellanox.com>; Yongseok Koh
> <yskoh@mellanox.com>
> Cc: dev@dpdk.org; Ori Kam <orika@mellanox.com>; Dekel Peled
> <dekelp@mellanox.com>
> Subject: RE: [PATCH] net/mlx5: modify-header support using Direct Verbs
> 
> 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"

OK, I'll change it.

> 
> >
> > 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.

I tried to avoid it but couldn't.
It used to be items before actions, changed in http://git.dpdk.org/dpdk/commit/?id=e31b6a4151bc8ef97b0fb1ac951b33d811d52969.
The duplicate parsing will occur at most once per flow rule, and only if specific items are present.

> 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.

I will change it to bytes and simplify the logic.

> 
> 
> > +			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.

My mistake, will remove this validation here and elsewhere.

> 
> > +	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.

I will change it.

> 
> >  };
> >
> >  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 16:00 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
2018-12-25 16:00   ` Dekel Peled [this message]
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=VI1PR05MB4224F41E95866B764E2DFD74B6B40@VI1PR05MB4224.eurprd05.prod.outlook.com \
    --to=dekelp@mellanox.com \
    --cc=dev@dpdk.org \
    --cc=orika@mellanox.com \
    --cc=shahafs@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).