diff mbox series

net/mlx5: support modify field rte flow action

Message ID 20210127024852.819-1-akozyrev@nvidia.com (mailing list archive)
State Superseded, archived
Delegated to: Raslan Darawsheh
Headers show
Series net/mlx5: support modify field rte flow action | expand

Checks

Context Check Description
ci/iol-testing fail Testing issues
ci/intel-Testing success Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/Intel-compilation success Compilation OK
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/checkpatch success coding style OK

Commit Message

Alexander Kozyrev Jan. 27, 2021, 2:48 a.m. UTC
Add support for new MODIFY_FIELD action to the Mellanox PMD.
This is the generic API that allows to manipulate any packet
header field by copying data from another packet field or
mark, metadata, tag, or immediate value (or pointer to it).

Since the API is generic and covers a lot of action under its
umbrella it makes sense to implement all the mechanics gradually
in order to move to this API for any packet field manipulations
in the future. This is first step of RTE flows consolidation.

The modify field RTE flow supports three operations: set, add
and sub. This patch brings to live only the "set" operation.
Support is provided for any packet header field as well as
meta/tag/mark and immediate value can be used as a source.

There are few limitations for this first version of API support:
- encapsulation levels are not supported, just outermost header
can be manipulated for now.
- offsets can only be 4-bytes aligned: 32, 64 and 96 for IPv6.
- the special ITEM_START ID is not supported as we do not allow
to cross packet header field boundaries yet.

Signed-off-by: Alexander Kozyrev <akozyrev@nvidia.com>
---
 drivers/net/mlx5/mlx5_flow.h    |   4 +-
 drivers/net/mlx5/mlx5_flow_dv.c | 507 +++++++++++++++++++++++++++++++-
 2 files changed, 508 insertions(+), 3 deletions(-)

Comments

Viacheslav Ovsiienko Jan. 27, 2021, 11:36 a.m. UTC | #1
Hi, Alexander

Please, see my minor comments below.

Besides this: 
Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>

> -----Original Message-----
> From: Alexander Kozyrev <akozyrev@nvidia.com>
> Sent: Wednesday, January 27, 2021 4:49
> To: dev@dpdk.org
> Cc: Raslan Darawsheh <rasland@nvidia.com>; Slava Ovsiienko
> <viacheslavo@nvidia.com>; Matan Azrad <matan@nvidia.com>; Ori Kam
> <orika@nvidia.com>
> Subject: [PATCH] net/mlx5: support modify field rte flow action
> 
> Add support for new MODIFY_FIELD action to the Mellanox PMD.
> This is the generic API that allows to manipulate any packet header field by
> copying data from another packet field or mark, metadata, tag, or immediate
> value (or pointer to it).
> 
> Since the API is generic and covers a lot of action under its umbrella it makes
> sense to implement all the mechanics gradually in order to move to this API
> for any packet field manipulations in the future. This is first step of RTE flows
Typo: "the first"

> consolidation.
> 
> The modify field RTE flow supports three operations: set, add and sub. This
Missed API|action?   Should it be "The modify field RTE flow action supports" ?

> patch brings to live only the "set" operation.
> Support is provided for any packet header field as well as meta/tag/mark and
> immediate value can be used as a source.
> 
> There are few limitations for this first version of API support:
> - encapsulation levels are not supported, just outermost header can be
> manipulated for now.
> - offsets can only be 4-bytes aligned: 32, 64 and 96 for IPv6.
> - the special ITEM_START ID is not supported as we do not allow to cross
> packet header field boundaries yet.
> 
> Signed-off-by: Alexander Kozyrev <akozyrev@nvidia.com>
> ---
>  drivers/net/mlx5/mlx5_flow.h    |   4 +-
>  drivers/net/mlx5/mlx5_flow_dv.c | 507
> +++++++++++++++++++++++++++++++-
>  2 files changed, 508 insertions(+), 3 deletions(-)
> 
mlx5 documentation update?
release notes update?

> diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
> index 2178a04e3a..6f6828c6a1 100644
> --- a/drivers/net/mlx5/mlx5_flow.h
> +++ b/drivers/net/mlx5/mlx5_flow.h
> @@ -219,6 +219,7 @@ enum mlx5_feature_name {  #define
> MLX5_FLOW_ACTION_SAMPLE (1ull << 36)  #define
> MLX5_FLOW_ACTION_TUNNEL_SET (1ull << 37)  #define
> MLX5_FLOW_ACTION_TUNNEL_MATCH (1ull << 38)
> +#define MLX5_FLOW_ACTION_MODIFY_FIELD (1ull << 39)
> 
>  #define MLX5_FLOW_FATE_ACTIONS \
>  	(MLX5_FLOW_ACTION_DROP | MLX5_FLOW_ACTION_QUEUE | \
> @@ -249,7 +250,8 @@ enum mlx5_feature_name {
>  				      MLX5_FLOW_ACTION_MARK_EXT | \
>  				      MLX5_FLOW_ACTION_SET_META | \
>  				      MLX5_FLOW_ACTION_SET_IPV4_DSCP | \
> -				      MLX5_FLOW_ACTION_SET_IPV6_DSCP)
> +				      MLX5_FLOW_ACTION_SET_IPV6_DSCP | \
> +				      MLX5_FLOW_ACTION_MODIFY_FIELD)
> 
>  #define MLX5_FLOW_VLAN_ACTIONS (MLX5_FLOW_ACTION_OF_POP_VLAN
> | \
>  				MLX5_FLOW_ACTION_OF_PUSH_VLAN)
> diff --git a/drivers/net/mlx5/mlx5_flow_dv.c
> b/drivers/net/mlx5/mlx5_flow_dv.c index 1a0c0be680..d842dc9887 100644
> --- a/drivers/net/mlx5/mlx5_flow_dv.c
> +++ b/drivers/net/mlx5/mlx5_flow_dv.c
> @@ -209,6 +209,8 @@ rte_col_2_mlx5_col(enum rte_color rcol)
>  	return MLX5_FLOW_COLOR_UNDEFINED;
>  }
> 
> +#define MLX5DV_FLOW_MAX_MOD_FIELDS 5
Let's move to the mlx5_flow.h, closer to other #defines

> +
>  struct field_modify_info {
>  	uint32_t size; /* Size of field in protocol header, in bytes. */
>  	uint32_t offset; /* Offset of field in protocol header, in bytes. */ @@ -
> 431,6 +433,9 @@ flow_dv_convert_modify_action(struct rte_flow_item
> *item,
>  				(int)dcopy->offset < 0 ? off_b : dcopy->offset;
>  			/* Convert entire record to big-endian format. */
>  			actions[i].data1 = rte_cpu_to_be_32(actions[i].data1);
> +			++dcopy;
> +			if (!dcopy)
> +				break;
Sorry, I do not follow. dcopy is the structure pointer (we assume it points to the array).
How pointer can be NULL after update ++dcopy ?

>  		} else {
>  			MLX5_ASSERT(item->spec);
>  			data = flow_dv_fetch_field((const uint8_t *)item-
> >spec + @@ -1324,6 +1329,339 @@
> flow_dv_convert_action_modify_ipv6_dscp
>  					     MLX5_MODIFICATION_TYPE_SET,
> error);  }
> 
> +static void
> +mlx5_flow_field_id_to_modify_info
> +		(const struct rte_flow_action_modify_data *data,
> +		 struct field_modify_info *info,
> +		 uint32_t *mask, uint32_t *value,
> +		 struct rte_eth_dev *dev,
> +		 const struct rte_flow_attr *attr,
> +		 struct rte_flow_error *error)
> +{
> +	uint32_t idx = 0;
> +	switch (data->field) {
> +	case RTE_FLOW_FIELD_START:
> +		/* not supported yet */
> +		break;
Let's move to default? To catch validation gap? 

> +	case RTE_FLOW_FIELD_MAC_DST:
> +		if (mask) {
> +			info[idx] = (struct field_modify_info){4, 0,
> +
> 	MLX5_MODI_OUT_DMAC_47_16};
> +			mask[idx] = (data->offset == 32) ? 0x0 : 0xffffffff;
With zero mask we could just skip the element here 
(not critical - anyway, it will be dropped at conversion stage to HW actions)

> +			++idx;
> +			info[idx] = (struct field_modify_info){2, 4,
> +
> 	MLX5_MODI_OUT_DMAC_15_0};
> +			mask[idx] = 0xffff0000;
> +		} else {
> +			if (!data->offset)
> +				info[idx++] = (struct field_modify_info){4, 0,
> +
> 	MLX5_MODI_OUT_DMAC_47_16};
> +			info[idx] = (struct field_modify_info){2, 0,
> +
> 	MLX5_MODI_OUT_DMAC_15_0};
> +		}
> +		break;
> +	case RTE_FLOW_FIELD_MAC_SRC:
> +		if (mask) {
> +			info[idx] = (struct field_modify_info){4, 0,
> +
> 	MLX5_MODI_OUT_SMAC_47_16};
> +			mask[idx] = (data->offset == 32) ? 0x0 : 0xffffffff;
> +			++idx;
> +			info[idx] = (struct field_modify_info){2, 4,
> +
> 	MLX5_MODI_OUT_SMAC_15_0};
> +			mask[idx] = 0xffff0000;
> +		} else {
> +			if (!data->offset)
> +				info[idx++] = (struct field_modify_info){4, 0,
> +
> 	MLX5_MODI_OUT_SMAC_47_16};
> +			info[idx] = (struct field_modify_info){2, 0,
> +
> 	MLX5_MODI_OUT_SMAC_15_0};
> +		}
> +		break;
> +	case RTE_FLOW_FIELD_VLAN_TYPE:
> +		/* not supported yet */
> +		break;
> +	case RTE_FLOW_FIELD_VLAN_ID:
> +		info[idx] = (struct field_modify_info){2, 0,
> +					MLX5_MODI_OUT_FIRST_VID};
> +		if (mask)
> +			mask[idx] = 0xfffff000;
> +		break;
> +	case RTE_FLOW_FIELD_MAC_TYPE:
> +		info[idx] = (struct field_modify_info){2, 0,
> +					MLX5_MODI_OUT_ETHERTYPE};
> +		if (mask)
> +			mask[idx] = 0xffff0000;
> +		break;
> +	case RTE_FLOW_FIELD_IPV4_DSCP:
> +		info[idx] = (struct field_modify_info){1, 0,
> +					MLX5_MODI_OUT_IP_DSCP};
> +		if (mask)
> +			mask[idx] = 0xfc000000;
> +		break;
> +	case RTE_FLOW_FIELD_IPV4_TTL:
> +		info[idx] = (struct field_modify_info){1, 0,
> +					MLX5_MODI_OUT_IPV4_TTL};
> +		if (mask)
> +			mask[idx] = 0xff000000;
> +		break;
> +	case RTE_FLOW_FIELD_IPV4_SRC:
> +		info[idx] = (struct field_modify_info){4, 0,
> +					MLX5_MODI_OUT_SIPV4};
> +		if (mask)
> +			mask[idx] = 0xffffffff;
> +		break;
> +	case RTE_FLOW_FIELD_IPV4_DST:
> +		info[idx] = (struct field_modify_info){4, 0,
> +					MLX5_MODI_OUT_DIPV4};
> +		if (mask)
> +			mask[idx] = 0xffffffff;
> +		break;
> +	case RTE_FLOW_FIELD_IPV6_HOPLIMIT:
> +		info[idx] = (struct field_modify_info){1, 0,
> +					MLX5_MODI_OUT_IPV6_HOPLIMIT};
> +		if (mask)
> +			mask[idx] = 0xff000000;
> +		break;
> +	case RTE_FLOW_FIELD_IPV6_SRC:
> +		if (mask) {
> +			info[idx] = (struct field_modify_info){4, 0,
> +
> 	MLX5_MODI_OUT_SIPV6_127_96};
> +			mask[idx] = (data->offset >= 32) ? 0x0 : 0xffffffff;
> +			++idx;
> +			info[idx] = (struct field_modify_info){4, 4,
> +
> 	MLX5_MODI_OUT_SIPV6_95_64};
> +			mask[idx] = (data->offset >= 64) ? 0x0 : 0xffffffff;
> +			++idx;
> +			info[idx] = (struct field_modify_info){4, 8,
> +
> 	MLX5_MODI_OUT_SIPV6_63_32};
> +			mask[idx] = (data->offset >= 96) ? 0x0 : 0xffffffff;
> +			++idx;
> +			info[idx] = (struct field_modify_info){4, 12,
> +
> 	MLX5_MODI_OUT_SIPV6_31_0};
> +			mask[idx] = 0xffffffff;
> +		} else {
> +			if (data->offset < 32)
> +				info[idx++] = (struct field_modify_info){4, 0,
> +
> 	MLX5_MODI_OUT_SIPV6_127_96};
> +			if (data->offset < 64)
> +				info[idx++] = (struct field_modify_info){4, 0,
> +
> 	MLX5_MODI_OUT_SIPV6_95_64};
> +			if (data->offset < 96)
> +				info[idx++] = (struct field_modify_info){4, 0,
> +
> 	MLX5_MODI_OUT_SIPV6_63_32};
> +			if (data->offset < 128)
> +				info[idx++] = (struct field_modify_info){4, 0,
> +
> 	MLX5_MODI_OUT_SIPV6_31_0};
> +		}
> +		break;
> +	case RTE_FLOW_FIELD_IPV6_DST:
> +		if (mask) {
> +			info[idx] = (struct field_modify_info){4, 0,
> +
> 	MLX5_MODI_OUT_DIPV6_127_96};
> +			mask[idx] = (data->offset >= 32) ? 0x0 : 0xffffffff;
> +			++idx;
> +			info[idx] = (struct field_modify_info){4, 4,
> +
> 	MLX5_MODI_OUT_DIPV6_95_64};
> +			mask[idx] = (data->offset >= 64) ? 0x0 : 0xffffffff;
> +			++idx;
> +			info[idx] = (struct field_modify_info){4, 8,
> +
> 	MLX5_MODI_OUT_DIPV6_63_32};
> +			mask[idx] = (data->offset >= 96) ? 0x0 : 0xffffffff;
> +			++idx;
> +			info[idx] = (struct field_modify_info){4, 12,
> +
> 	MLX5_MODI_OUT_DIPV6_31_0};
> +			mask[idx] = 0xffffffff;
> +		} else {
> +			if (data->offset < 32)
> +				info[idx++] = (struct field_modify_info){4, 0,
> +
> 	MLX5_MODI_OUT_DIPV6_127_96};
> +			if (data->offset < 64)
> +				info[idx++] = (struct field_modify_info){4, 0,
> +
> 	MLX5_MODI_OUT_DIPV6_95_64};
> +			if (data->offset < 96)
> +				info[idx++] = (struct field_modify_info){4, 0,
> +
> 	MLX5_MODI_OUT_DIPV6_63_32};
> +			if (data->offset < 128)
> +				info[idx++] = (struct field_modify_info){4, 0,
> +
> 	MLX5_MODI_OUT_DIPV6_31_0};
> +		}
> +		break;
> +	case RTE_FLOW_FIELD_TCP_PORT_SRC:
> +		info[idx] = (struct field_modify_info){2, 0,
> +					MLX5_MODI_OUT_TCP_SPORT};
> +		if (mask)
> +			mask[idx] = 0xffff0000;
> +		break;
> +	case RTE_FLOW_FIELD_TCP_PORT_DST:
> +		info[idx] = (struct field_modify_info){2, 0,
> +					MLX5_MODI_OUT_TCP_DPORT};
> +		if (mask)
> +			mask[idx] = 0xffff0000;
> +		break;
> +	case RTE_FLOW_FIELD_TCP_SEQ_NUM:
> +		info[idx] = (struct field_modify_info){4, 0,
> +					MLX5_MODI_OUT_TCP_SEQ_NUM};
> +		if (mask)
> +			mask[idx] = 0xffffffff;
> +		break;
> +	case RTE_FLOW_FIELD_TCP_ACK_NUM:
> +		info[idx] = (struct field_modify_info){4, 0,
> +					MLX5_MODI_OUT_TCP_ACK_NUM};
> +		if (mask)
> +			mask[idx] = 0xffffffff;
> +		break;
> +	case RTE_FLOW_FIELD_TCP_FLAGS:
> +		info[idx] = (struct field_modify_info){1, 0,
> +					MLX5_MODI_IN_TCP_FLAGS};
> +		if (mask)
> +			mask[idx] = 0xfc000000;
> +		break;
> +	case RTE_FLOW_FIELD_UDP_PORT_SRC:
> +		info[idx] = (struct field_modify_info){2, 0,
> +					MLX5_MODI_OUT_UDP_SPORT};
> +		if (mask)
> +			mask[idx] = 0xffff0000;
> +		break;
> +	case RTE_FLOW_FIELD_UDP_PORT_DST:
> +		info[idx] = (struct field_modify_info){2, 0,
> +					MLX5_MODI_OUT_UDP_DPORT};
> +		if (mask)
> +			mask[idx] = 0xffff0000;
> +		break;
> +	case RTE_FLOW_FIELD_VXLAN_VNI:
> +		/* not supported yet */
> +		break;
> +	case RTE_FLOW_FIELD_GENEVE_VNI:
> +		/* not supported yet*/
> +		break;
> +	case RTE_FLOW_FIELD_GTP_TEID:
> +		info[idx] = (struct field_modify_info){4, 0, 0x6E};
> +		if (mask)
> +			mask[idx] = 0xffffffff;
> +		break;
> +	case RTE_FLOW_FIELD_TAG:
> +		{
> +			int ret;
> +			enum mlx5_modification_field reg_type;
> +			ret = mlx5_flow_get_reg_id(dev, MLX5_APP_TAG,
> +						   data->level, error);
> +			if (ret < 0)
> +				return;
> +			MLX5_ASSERT(ret != REG_NON);
> +			MLX5_ASSERT((unsigned int)ret <
> RTE_DIM(reg_to_field));
> +			reg_type = reg_to_field[ret];
> +			MLX5_ASSERT(reg_type > 0);
> +			info[idx] = (struct field_modify_info){4, 0, reg_type};
> +			if (mask)
> +				mask[idx] = 0xffffffff;
> +		}
> +		break;
> +	case RTE_FLOW_FIELD_MARK:
> +		{
> +			int reg = mlx5_flow_get_reg_id(dev,
> MLX5_FLOW_MARK,
> +						       0, error);
> +			if (reg < 0)
> +				return;
> +			MLX5_ASSERT(reg > 0);
> +			info[idx] = (struct field_modify_info){4, 0,
> +						reg_to_field[reg]};
> +			if (mask)
> +				mask[idx] = 0xffffffff;
> +		}
> +		break;
> +	case RTE_FLOW_FIELD_META:
> +		{
> +			int reg = flow_dv_get_metadata_reg(dev, attr, error);
> +			if (reg < 0)
> +				return;
> +			MLX5_ASSERT(reg != REG_NON);
> +			info[idx] = (struct field_modify_info){4, 0,
> +						reg_to_field[reg]};
> +			if (mask)
> +				mask[idx] = 0xffffffff;
> +		}
> +		break;
> +	case RTE_FLOW_FIELD_POINTER:
> +		for (idx = 0; idx < MLX5DV_FLOW_MAX_MOD_FIELDS; idx++) {
> +			if (mask[idx]) {
> +				value[idx] = RTE_BE32(*(uint64_t *)data-
> >value);
> +				break;
> +			}
> +		}
> +	case RTE_FLOW_FIELD_VALUE:
> +		for (idx = 0; idx < MLX5DV_FLOW_MAX_MOD_FIELDS; idx++) {
> +			if (mask[idx]) {
> +				value[idx] = RTE_BE32(data->value);
> +				break;
> +			}
> +		}
> +		break;
> +	default:
> +		MLX5_ASSERT(false);
> +		break;
> +	}
> +}
> +
> +/**
> + * Convert modify_field action to DV specification.
> + *
> + * @param[in] dev
> + *   Pointer to the rte_eth_dev structure.
> + * @param[in,out] resource
> + *   Pointer to the modify-header resource.
> + * @param[in] action
> + *   Pointer to action specification.
> + * @param[in] attr
> + *   Attributes of flow that includes this item.
> + * @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_field
> +			(struct rte_eth_dev *dev,
> +			 struct mlx5_flow_dv_modify_hdr_resource
> *resource,
> +			 const struct rte_flow_action *action,
> +			 const struct rte_flow_attr *attr,
> +			 struct rte_flow_error *error)
> +{
> +	const struct rte_flow_action_modify_field *conf =
> +		(const struct rte_flow_action_modify_field *)(action->conf);
> +	struct rte_flow_item item;
> +	struct field_modify_info field[MLX5DV_FLOW_MAX_MOD_FIELDS] = {
> +								{0, 0, 0} };
> +	struct field_modify_info dcopy[MLX5DV_FLOW_MAX_MOD_FIELDS]
> = {
> +								{0, 0, 0} };
> +	uint32_t mask[MLX5DV_FLOW_MAX_MOD_FIELDS] = {0, 0, 0, 0, 0};
> +	uint32_t value[MLX5DV_FLOW_MAX_MOD_FIELDS] = {0, 0, 0, 0, 0};
> +	uint32_t type;
> +
> +	if (conf->src.field == RTE_FLOW_FIELD_POINTER ||
> +		conf->src.field == RTE_FLOW_FIELD_VALUE) {
> +		type = MLX5_MODIFICATION_TYPE_SET;
> +		/** For SET fill the destination field (field) first. */
> +		mlx5_flow_field_id_to_modify_info(&conf->dst, field, mask,
> +						  value, dev, attr, error);
> +		/** Then copy immediate value from source as per mask. */
> +		mlx5_flow_field_id_to_modify_info(&conf->src, dcopy, mask,
> +						  value, dev, attr, error);
> +		item.spec = &value;
> +	} else {
> +		type = MLX5_MODIFICATION_TYPE_COPY;
> +		/** For COPY fill the destination field (dcopy) without mask.
> */
> +		mlx5_flow_field_id_to_modify_info(&conf->dst, dcopy, NULL,
> +						  value, dev, attr, error);
> +		/** Then cnstruct th source field (field) with mask. */
Typos: construct th

> +		mlx5_flow_field_id_to_modify_info(&conf->src, field, mask,
> +						  value, dev, attr, error);
> +	}
> +	item.mask = &mask;
> +	return flow_dv_convert_modify_action(&item,
> +			field, dcopy, resource, type, error); }
> +
>  /**
>   * Validate MARK item.
>   *
> @@ -3985,6 +4323,148 @@ flow_dv_validate_action_modify_ttl(const
> uint64_t action_flags,
>  	return ret;
>  }
> 
> +static int
> +mlx5_flow_item_field_width(enum rte_flow_field_id field) {
> +	switch (field) {
> +	case RTE_FLOW_FIELD_START:
> +		return 32;
> +	case RTE_FLOW_FIELD_MAC_DST:
> +	case RTE_FLOW_FIELD_MAC_SRC:
> +		return 48;
> +	case RTE_FLOW_FIELD_VLAN_TYPE:
> +		return 16;
> +	case RTE_FLOW_FIELD_VLAN_ID:
> +		return 12;
> +	case RTE_FLOW_FIELD_MAC_TYPE:
> +		return 16;
> +	case RTE_FLOW_FIELD_IPV4_DSCP:
> +	case RTE_FLOW_FIELD_IPV4_TTL:
> +		return 8;
> +	case RTE_FLOW_FIELD_IPV4_SRC:
> +	case RTE_FLOW_FIELD_IPV4_DST:
> +		return 32;
> +	case RTE_FLOW_FIELD_IPV6_HOPLIMIT:
> +		return 8;
> +	case RTE_FLOW_FIELD_IPV6_SRC:
> +	case RTE_FLOW_FIELD_IPV6_DST:
> +		return 128;
> +	case RTE_FLOW_FIELD_TCP_PORT_SRC:
> +	case RTE_FLOW_FIELD_TCP_PORT_DST:
> +		return 16;
> +	case RTE_FLOW_FIELD_TCP_SEQ_NUM:
> +	case RTE_FLOW_FIELD_TCP_ACK_NUM:
> +		return 32;
> +	case RTE_FLOW_FIELD_TCP_FLAGS:
> +		return 6;
> +	case RTE_FLOW_FIELD_UDP_PORT_SRC:
> +	case RTE_FLOW_FIELD_UDP_PORT_DST:
> +		return 16;
> +	case RTE_FLOW_FIELD_VXLAN_VNI:
> +	case RTE_FLOW_FIELD_GENEVE_VNI:
> +		return 24;
> +	case RTE_FLOW_FIELD_GTP_TEID:
> +	case RTE_FLOW_FIELD_TAG:
> +	case RTE_FLOW_FIELD_MARK:
> +	case RTE_FLOW_FIELD_META:
> +	case RTE_FLOW_FIELD_POINTER:
> +	case RTE_FLOW_FIELD_VALUE:
> +		return 32;
> +	default:
> +		MLX5_ASSERT(false);
> +	}
> +	return 0;
> +}
> +
> +/**
> + * Validate the generic modify field 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
> + *   Number of header fields to modify (0 or more) on success,
> + *   a negative errno value otherwise and rte_errno is set.
> + */
> +static int
> +flow_dv_validate_action_modify_field(const uint64_t action_flags,
> +				   const struct rte_flow_action *action,
> +				   struct rte_flow_error *error)
> +{
> +	int ret = 0;
> +	const struct rte_flow_action_modify_field *action_modify_field =
> +		action->conf;
> +	uint32_t dst_width =
> +		mlx5_flow_item_field_width(action_modify_field->dst.field);
> +	uint32_t src_width =
> +		mlx5_flow_item_field_width(action_modify_field->src.field);
> +
> +	ret = flow_dv_validate_action_modify_hdr(action_flags, action,
> error);
> +	if (ret)
> +		return ret;
> +
> +	if (action_modify_field->src.field != RTE_FLOW_FIELD_VALUE &&
> +	    action_modify_field->src.field != RTE_FLOW_FIELD_POINTER) {
> +		if (action_modify_field->dst.offset >= dst_width ||
> +		    (action_modify_field->dst.offset % 32) ||
> +		    action_modify_field->src.offset >= src_width ||
> +		    (action_modify_field->src.offset % 32))
> +			return rte_flow_error_set(error, EINVAL,
> +
> 	RTE_FLOW_ERROR_TYPE_ACTION,
> +						NULL,
> +						"offset is too big"
> +						" or not aligned to 4 bytes");
> +		if ((action_modify_field->dst.level &&
> +		     action_modify_field->dst.field != RTE_FLOW_FIELD_TAG)
> ||
> +		    (action_modify_field->src.level &&
> +		     action_modify_field->src.field != RTE_FLOW_FIELD_TAG))
> +			return rte_flow_error_set(error, EINVAL,
> +
> 	RTE_FLOW_ERROR_TYPE_ACTION,
> +						NULL,
> +						"inner header modifications"
> +						" are not supported");
> +	}
> +	if (action_modify_field->width == 0)
> +		return rte_flow_error_set(error, EINVAL,
> +
> 	RTE_FLOW_ERROR_TYPE_ACTION,
> +						NULL,
> +						"width is required for modify
> action");
> +	if (action_modify_field->dst.field ==
> +	    action_modify_field->src.field)
> +		return rte_flow_error_set(error, EINVAL,
> +					RTE_FLOW_ERROR_TYPE_ACTION,
> +					NULL,
> +					"source and destination fields"
> +					" cannot be the same");
> +	if (action_modify_field->dst.field == RTE_FLOW_FIELD_VALUE ||
> +	    action_modify_field->dst.field == RTE_FLOW_FIELD_POINTER)
> +		return rte_flow_error_set(error, EINVAL,
> +					RTE_FLOW_ERROR_TYPE_ACTION,
> +					NULL,
> +					"immediate value or a pointer to it"
> +					" cannot be used as a destination");
> +	if (action_modify_field->dst.field == RTE_FLOW_FIELD_START)
> +		return rte_flow_error_set(error, EINVAL,
> +				RTE_FLOW_ERROR_TYPE_ACTION,
> +				NULL,
> +				"modifications of an arbitrary"
> +				" place in a packet is not supported");
> +	if (action_modify_field->operation != RTE_FLOW_MODIFY_SET)
> +		return rte_flow_error_set(error, EINVAL,
> +				RTE_FLOW_ERROR_TYPE_ACTION,
> +				NULL,
> +				"add and sub operations"
> +				" are not supported");
> +	return (action_modify_field->width / 32) +
> +	       !!(action_modify_field->width % 32); }
> +
>  /**
>   * Validate jump action.
>   *
> @@ -5373,7 +5853,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const
> struct rte_flow_attr *attr,
>  	struct mlx5_dev_config *dev_conf = &priv->config;
>  	uint16_t queue_index = 0xFFFF;
>  	const struct rte_flow_item_vlan *vlan_m = NULL;
> -	int16_t rw_act_num = 0;
> +	uint32_t rw_act_num = 0;
>  	uint64_t is_root;
>  	const struct mlx5_flow_tunnel *tunnel;
>  	struct flow_grp_info grp_info = {
> @@ -6169,6 +6649,23 @@ flow_dv_validate(struct rte_eth_dev *dev, const
> struct rte_flow_attr *attr,
> 
>  			action_flags |= MLX5_FLOW_ACTION_TUNNEL_SET;
>  			break;
> +		case RTE_FLOW_ACTION_TYPE_MODIFY_FIELD:
> +			if (!attr->transfer && !attr->group)
> +				return rte_flow_error_set(error, ENOTSUP,
> +
> 	RTE_FLOW_ERROR_TYPE_ACTION,
> +						NULL, "modify field action "
> +						"is not supported for group
> 0");
> +			ret =
> flow_dv_validate_action_modify_field(action_flags,
> +								 actions,
> +								 error);
> +			if (ret < 0)
> +				return ret;
> +			/* Count all modify-header actions as one action. */
> +			if (!(action_flags &
> MLX5_FLOW_ACTION_MODIFY_FIELD))
> +				++actions_n;
> +			action_flags |=
> MLX5_FLOW_ACTION_MODIFY_FIELD;
> +			rw_act_num += ret;
> +			break;
>  		default:
>  			return rte_flow_error_set(error, ENOTSUP,
> 
> RTE_FLOW_ERROR_TYPE_ACTION,
> @@ -6326,7 +6823,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const
> struct rte_flow_attr *attr,
>  	    dev_conf->dv_xmeta_en != MLX5_XMETA_MODE_LEGACY &&
>  	    mlx5_flow_ext_mreg_supported(dev))
>  		rw_act_num += MLX5_ACT_NUM_SET_TAG;
> -	if ((uint32_t)rw_act_num >
> +	if (rw_act_num >
>  			flow_dv_modify_hdr_action_max(dev, is_root)) {
>  		return rte_flow_error_set(error, ENOTSUP,
>  					  RTE_FLOW_ERROR_TYPE_ACTION,
> @@ -10686,6 +11183,12 @@ flow_dv_translate(struct rte_eth_dev *dev,
>  				sample_act->action_flags |=
> 
> 	MLX5_FLOW_ACTION_ENCAP;
>  			break;
> +		case RTE_FLOW_ACTION_TYPE_MODIFY_FIELD:
> +			if (flow_dv_convert_action_modify_field
> +					(dev, mhdr_res, actions, attr, error))
> +				return -rte_errno;
> +			action_flags |=
> MLX5_FLOW_ACTION_MODIFY_FIELD;
> +			break;
>  		case RTE_FLOW_ACTION_TYPE_END:
>  			actions_end = true;
>  			if (mhdr_res->actions_num) {
> --
> 2.24.1
Alexander Kozyrev Jan. 27, 2021, 2:58 p.m. UTC | #2
< From: Slava Ovsiienko <viacheslavo@nvidia.com> on Wednesday, January 27, 2021 6:37
>
> Hi, Alexander
> 
> Please, see my minor comments below.
> 
> Besides this:
> Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>
> 
> > -----Original Message-----
> > From: Alexander Kozyrev <akozyrev@nvidia.com>
> > Sent: Wednesday, January 27, 2021 4:49
> > To: dev@dpdk.org
> > Cc: Raslan Darawsheh <rasland@nvidia.com>; Slava Ovsiienko
> > <viacheslavo@nvidia.com>; Matan Azrad <matan@nvidia.com>; Ori Kam
> > <orika@nvidia.com>
> > Subject: [PATCH] net/mlx5: support modify field rte flow action
> >
> > Add support for new MODIFY_FIELD action to the Mellanox PMD.
> > This is the generic API that allows to manipulate any packet header field by
> > copying data from another packet field or mark, metadata, tag, or
> immediate
> > value (or pointer to it).
> >
> > Since the API is generic and covers a lot of action under its umbrella it
> makes
> > sense to implement all the mechanics gradually in order to move to this API
> > for any packet field manipulations in the future. This is first step of RTE
> flows
> Typo: "the first"
> 
> > consolidation.
> >
> > The modify field RTE flow supports three operations: set, add and sub. This
> Missed API|action?   Should it be "The modify field RTE flow action supports"
> ?
Thanks, will fix typos.

> > patch brings to live only the "set" operation.
> > Support is provided for any packet header field as well as meta/tag/mark
> and
> > immediate value can be used as a source.
> >
> > There are few limitations for this first version of API support:
> > - encapsulation levels are not supported, just outermost header can be
> > manipulated for now.
> > - offsets can only be 4-bytes aligned: 32, 64 and 96 for IPv6.
> > - the special ITEM_START ID is not supported as we do not allow to cross
> > packet header field boundaries yet.
> >
> > Signed-off-by: Alexander Kozyrev <akozyrev@nvidia.com>
> > ---
> >  drivers/net/mlx5/mlx5_flow.h    |   4 +-
> >  drivers/net/mlx5/mlx5_flow_dv.c | 507
> > +++++++++++++++++++++++++++++++-
> >  2 files changed, 508 insertions(+), 3 deletions(-)
> >
> mlx5 documentation update?
> release notes update?
Let me add them.

> > diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
> > index 2178a04e3a..6f6828c6a1 100644
> > --- a/drivers/net/mlx5/mlx5_flow.h
> > +++ b/drivers/net/mlx5/mlx5_flow.h
> > @@ -219,6 +219,7 @@ enum mlx5_feature_name {  #define
> > MLX5_FLOW_ACTION_SAMPLE (1ull << 36)  #define
> > MLX5_FLOW_ACTION_TUNNEL_SET (1ull << 37)  #define
> > MLX5_FLOW_ACTION_TUNNEL_MATCH (1ull << 38)
> > +#define MLX5_FLOW_ACTION_MODIFY_FIELD (1ull << 39)
> >
> >  #define MLX5_FLOW_FATE_ACTIONS \
> >  	(MLX5_FLOW_ACTION_DROP | MLX5_FLOW_ACTION_QUEUE | \
> > @@ -249,7 +250,8 @@ enum mlx5_feature_name {
> >  				      MLX5_FLOW_ACTION_MARK_EXT | \
> >  				      MLX5_FLOW_ACTION_SET_META | \
> >  				      MLX5_FLOW_ACTION_SET_IPV4_DSCP | \
> > -				      MLX5_FLOW_ACTION_SET_IPV6_DSCP)
> > +				      MLX5_FLOW_ACTION_SET_IPV6_DSCP | \
> > +				      MLX5_FLOW_ACTION_MODIFY_FIELD)
> >
> >  #define MLX5_FLOW_VLAN_ACTIONS
> (MLX5_FLOW_ACTION_OF_POP_VLAN
> > | \
> >  				MLX5_FLOW_ACTION_OF_PUSH_VLAN)
> > diff --git a/drivers/net/mlx5/mlx5_flow_dv.c
> > b/drivers/net/mlx5/mlx5_flow_dv.c index 1a0c0be680..d842dc9887 100644
> > --- a/drivers/net/mlx5/mlx5_flow_dv.c
> > +++ b/drivers/net/mlx5/mlx5_flow_dv.c
> > @@ -209,6 +209,8 @@ rte_col_2_mlx5_col(enum rte_color rcol)
> >  	return MLX5_FLOW_COLOR_UNDEFINED;
> >  }
> >
> > +#define MLX5DV_FLOW_MAX_MOD_FIELDS 5
> Let's move to the mlx5_flow.h, closer to other #defines
Ok.

> > +
> >  struct field_modify_info {
> >  	uint32_t size; /* Size of field in protocol header, in bytes. */
> >  	uint32_t offset; /* Offset of field in protocol header, in bytes. */ @@
> -
> > 431,6 +433,9 @@ flow_dv_convert_modify_action(struct rte_flow_item
> > *item,
> >  				(int)dcopy->offset < 0 ? off_b : dcopy-
> >offset;
> >  			/* Convert entire record to big-endian format. */
> >  			actions[i].data1 =
> rte_cpu_to_be_32(actions[i].data1);
> > +			++dcopy;
> > +			if (!dcopy)
> > +				break;
> Sorry, I do not follow. dcopy is the structure pointer (we assume it points to
> the array).
> How pointer can be NULL after update ++dcopy ?
Just a precaution if we go past the array due to some bug.

> >  		} else {
> >  			MLX5_ASSERT(item->spec);
> >  			data = flow_dv_fetch_field((const uint8_t *)item-
> > >spec + @@ -1324,6 +1329,339 @@
> > flow_dv_convert_action_modify_ipv6_dscp
> >  					     MLX5_MODIFICATION_TYPE_SET,
> > error);  }
> >
> > +static void
> > +mlx5_flow_field_id_to_modify_info
> > +		(const struct rte_flow_action_modify_data *data,
> > +		 struct field_modify_info *info,
> > +		 uint32_t *mask, uint32_t *value,
> > +		 struct rte_eth_dev *dev,
> > +		 const struct rte_flow_attr *attr,
> > +		 struct rte_flow_error *error)
> > +{
> > +	uint32_t idx = 0;
> > +	switch (data->field) {
> > +	case RTE_FLOW_FIELD_START:
> > +		/* not supported yet */
> > +		break;
> Let's move to default? To catch validation gap?
Ok.

> > +	case RTE_FLOW_FIELD_MAC_DST:
> > +		if (mask) {
> > +			info[idx] = (struct field_modify_info){4, 0,
> > +
> > 	MLX5_MODI_OUT_DMAC_47_16};
> > +			mask[idx] = (data->offset == 32) ? 0x0 : 0xffffffff;
> With zero mask we could just skip the element here
> (not critical - anyway, it will be dropped at conversion stage to HW actions)
That is the ides - to skip element at the conversion stage.
diff mbox series

Patch

diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index 2178a04e3a..6f6828c6a1 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -219,6 +219,7 @@  enum mlx5_feature_name {
 #define MLX5_FLOW_ACTION_SAMPLE (1ull << 36)
 #define MLX5_FLOW_ACTION_TUNNEL_SET (1ull << 37)
 #define MLX5_FLOW_ACTION_TUNNEL_MATCH (1ull << 38)
+#define MLX5_FLOW_ACTION_MODIFY_FIELD (1ull << 39)
 
 #define MLX5_FLOW_FATE_ACTIONS \
 	(MLX5_FLOW_ACTION_DROP | MLX5_FLOW_ACTION_QUEUE | \
@@ -249,7 +250,8 @@  enum mlx5_feature_name {
 				      MLX5_FLOW_ACTION_MARK_EXT | \
 				      MLX5_FLOW_ACTION_SET_META | \
 				      MLX5_FLOW_ACTION_SET_IPV4_DSCP | \
-				      MLX5_FLOW_ACTION_SET_IPV6_DSCP)
+				      MLX5_FLOW_ACTION_SET_IPV6_DSCP | \
+				      MLX5_FLOW_ACTION_MODIFY_FIELD)
 
 #define MLX5_FLOW_VLAN_ACTIONS (MLX5_FLOW_ACTION_OF_POP_VLAN | \
 				MLX5_FLOW_ACTION_OF_PUSH_VLAN)
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 1a0c0be680..d842dc9887 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -209,6 +209,8 @@  rte_col_2_mlx5_col(enum rte_color rcol)
 	return MLX5_FLOW_COLOR_UNDEFINED;
 }
 
+#define MLX5DV_FLOW_MAX_MOD_FIELDS 5
+
 struct field_modify_info {
 	uint32_t size; /* Size of field in protocol header, in bytes. */
 	uint32_t offset; /* Offset of field in protocol header, in bytes. */
@@ -431,6 +433,9 @@  flow_dv_convert_modify_action(struct rte_flow_item *item,
 				(int)dcopy->offset < 0 ? off_b : dcopy->offset;
 			/* Convert entire record to big-endian format. */
 			actions[i].data1 = rte_cpu_to_be_32(actions[i].data1);
+			++dcopy;
+			if (!dcopy)
+				break;
 		} else {
 			MLX5_ASSERT(item->spec);
 			data = flow_dv_fetch_field((const uint8_t *)item->spec +
@@ -1324,6 +1329,339 @@  flow_dv_convert_action_modify_ipv6_dscp
 					     MLX5_MODIFICATION_TYPE_SET, error);
 }
 
+static void
+mlx5_flow_field_id_to_modify_info
+		(const struct rte_flow_action_modify_data *data,
+		 struct field_modify_info *info,
+		 uint32_t *mask, uint32_t *value,
+		 struct rte_eth_dev *dev,
+		 const struct rte_flow_attr *attr,
+		 struct rte_flow_error *error)
+{
+	uint32_t idx = 0;
+	switch (data->field) {
+	case RTE_FLOW_FIELD_START:
+		/* not supported yet */
+		break;
+	case RTE_FLOW_FIELD_MAC_DST:
+		if (mask) {
+			info[idx] = (struct field_modify_info){4, 0,
+						MLX5_MODI_OUT_DMAC_47_16};
+			mask[idx] = (data->offset == 32) ? 0x0 : 0xffffffff;
+			++idx;
+			info[idx] = (struct field_modify_info){2, 4,
+						MLX5_MODI_OUT_DMAC_15_0};
+			mask[idx] = 0xffff0000;
+		} else {
+			if (!data->offset)
+				info[idx++] = (struct field_modify_info){4, 0,
+						MLX5_MODI_OUT_DMAC_47_16};
+			info[idx] = (struct field_modify_info){2, 0,
+						MLX5_MODI_OUT_DMAC_15_0};
+		}
+		break;
+	case RTE_FLOW_FIELD_MAC_SRC:
+		if (mask) {
+			info[idx] = (struct field_modify_info){4, 0,
+						MLX5_MODI_OUT_SMAC_47_16};
+			mask[idx] = (data->offset == 32) ? 0x0 : 0xffffffff;
+			++idx;
+			info[idx] = (struct field_modify_info){2, 4,
+						MLX5_MODI_OUT_SMAC_15_0};
+			mask[idx] = 0xffff0000;
+		} else {
+			if (!data->offset)
+				info[idx++] = (struct field_modify_info){4, 0,
+						MLX5_MODI_OUT_SMAC_47_16};
+			info[idx] = (struct field_modify_info){2, 0,
+						MLX5_MODI_OUT_SMAC_15_0};
+		}
+		break;
+	case RTE_FLOW_FIELD_VLAN_TYPE:
+		/* not supported yet */
+		break;
+	case RTE_FLOW_FIELD_VLAN_ID:
+		info[idx] = (struct field_modify_info){2, 0,
+					MLX5_MODI_OUT_FIRST_VID};
+		if (mask)
+			mask[idx] = 0xfffff000;
+		break;
+	case RTE_FLOW_FIELD_MAC_TYPE:
+		info[idx] = (struct field_modify_info){2, 0,
+					MLX5_MODI_OUT_ETHERTYPE};
+		if (mask)
+			mask[idx] = 0xffff0000;
+		break;
+	case RTE_FLOW_FIELD_IPV4_DSCP:
+		info[idx] = (struct field_modify_info){1, 0,
+					MLX5_MODI_OUT_IP_DSCP};
+		if (mask)
+			mask[idx] = 0xfc000000;
+		break;
+	case RTE_FLOW_FIELD_IPV4_TTL:
+		info[idx] = (struct field_modify_info){1, 0,
+					MLX5_MODI_OUT_IPV4_TTL};
+		if (mask)
+			mask[idx] = 0xff000000;
+		break;
+	case RTE_FLOW_FIELD_IPV4_SRC:
+		info[idx] = (struct field_modify_info){4, 0,
+					MLX5_MODI_OUT_SIPV4};
+		if (mask)
+			mask[idx] = 0xffffffff;
+		break;
+	case RTE_FLOW_FIELD_IPV4_DST:
+		info[idx] = (struct field_modify_info){4, 0,
+					MLX5_MODI_OUT_DIPV4};
+		if (mask)
+			mask[idx] = 0xffffffff;
+		break;
+	case RTE_FLOW_FIELD_IPV6_HOPLIMIT:
+		info[idx] = (struct field_modify_info){1, 0,
+					MLX5_MODI_OUT_IPV6_HOPLIMIT};
+		if (mask)
+			mask[idx] = 0xff000000;
+		break;
+	case RTE_FLOW_FIELD_IPV6_SRC:
+		if (mask) {
+			info[idx] = (struct field_modify_info){4, 0,
+						MLX5_MODI_OUT_SIPV6_127_96};
+			mask[idx] = (data->offset >= 32) ? 0x0 : 0xffffffff;
+			++idx;
+			info[idx] = (struct field_modify_info){4, 4,
+						MLX5_MODI_OUT_SIPV6_95_64};
+			mask[idx] = (data->offset >= 64) ? 0x0 : 0xffffffff;
+			++idx;
+			info[idx] = (struct field_modify_info){4, 8,
+						MLX5_MODI_OUT_SIPV6_63_32};
+			mask[idx] = (data->offset >= 96) ? 0x0 : 0xffffffff;
+			++idx;
+			info[idx] = (struct field_modify_info){4, 12,
+						MLX5_MODI_OUT_SIPV6_31_0};
+			mask[idx] = 0xffffffff;
+		} else {
+			if (data->offset < 32)
+				info[idx++] = (struct field_modify_info){4, 0,
+						MLX5_MODI_OUT_SIPV6_127_96};
+			if (data->offset < 64)
+				info[idx++] = (struct field_modify_info){4, 0,
+						MLX5_MODI_OUT_SIPV6_95_64};
+			if (data->offset < 96)
+				info[idx++] = (struct field_modify_info){4, 0,
+						MLX5_MODI_OUT_SIPV6_63_32};
+			if (data->offset < 128)
+				info[idx++] = (struct field_modify_info){4, 0,
+						MLX5_MODI_OUT_SIPV6_31_0};
+		}
+		break;
+	case RTE_FLOW_FIELD_IPV6_DST:
+		if (mask) {
+			info[idx] = (struct field_modify_info){4, 0,
+						MLX5_MODI_OUT_DIPV6_127_96};
+			mask[idx] = (data->offset >= 32) ? 0x0 : 0xffffffff;
+			++idx;
+			info[idx] = (struct field_modify_info){4, 4,
+						MLX5_MODI_OUT_DIPV6_95_64};
+			mask[idx] = (data->offset >= 64) ? 0x0 : 0xffffffff;
+			++idx;
+			info[idx] = (struct field_modify_info){4, 8,
+						MLX5_MODI_OUT_DIPV6_63_32};
+			mask[idx] = (data->offset >= 96) ? 0x0 : 0xffffffff;
+			++idx;
+			info[idx] = (struct field_modify_info){4, 12,
+						MLX5_MODI_OUT_DIPV6_31_0};
+			mask[idx] = 0xffffffff;
+		} else {
+			if (data->offset < 32)
+				info[idx++] = (struct field_modify_info){4, 0,
+						MLX5_MODI_OUT_DIPV6_127_96};
+			if (data->offset < 64)
+				info[idx++] = (struct field_modify_info){4, 0,
+						MLX5_MODI_OUT_DIPV6_95_64};
+			if (data->offset < 96)
+				info[idx++] = (struct field_modify_info){4, 0,
+						MLX5_MODI_OUT_DIPV6_63_32};
+			if (data->offset < 128)
+				info[idx++] = (struct field_modify_info){4, 0,
+						MLX5_MODI_OUT_DIPV6_31_0};
+		}
+		break;
+	case RTE_FLOW_FIELD_TCP_PORT_SRC:
+		info[idx] = (struct field_modify_info){2, 0,
+					MLX5_MODI_OUT_TCP_SPORT};
+		if (mask)
+			mask[idx] = 0xffff0000;
+		break;
+	case RTE_FLOW_FIELD_TCP_PORT_DST:
+		info[idx] = (struct field_modify_info){2, 0,
+					MLX5_MODI_OUT_TCP_DPORT};
+		if (mask)
+			mask[idx] = 0xffff0000;
+		break;
+	case RTE_FLOW_FIELD_TCP_SEQ_NUM:
+		info[idx] = (struct field_modify_info){4, 0,
+					MLX5_MODI_OUT_TCP_SEQ_NUM};
+		if (mask)
+			mask[idx] = 0xffffffff;
+		break;
+	case RTE_FLOW_FIELD_TCP_ACK_NUM:
+		info[idx] = (struct field_modify_info){4, 0,
+					MLX5_MODI_OUT_TCP_ACK_NUM};
+		if (mask)
+			mask[idx] = 0xffffffff;
+		break;
+	case RTE_FLOW_FIELD_TCP_FLAGS:
+		info[idx] = (struct field_modify_info){1, 0,
+					MLX5_MODI_IN_TCP_FLAGS};
+		if (mask)
+			mask[idx] = 0xfc000000;
+		break;
+	case RTE_FLOW_FIELD_UDP_PORT_SRC:
+		info[idx] = (struct field_modify_info){2, 0,
+					MLX5_MODI_OUT_UDP_SPORT};
+		if (mask)
+			mask[idx] = 0xffff0000;
+		break;
+	case RTE_FLOW_FIELD_UDP_PORT_DST:
+		info[idx] = (struct field_modify_info){2, 0,
+					MLX5_MODI_OUT_UDP_DPORT};
+		if (mask)
+			mask[idx] = 0xffff0000;
+		break;
+	case RTE_FLOW_FIELD_VXLAN_VNI:
+		/* not supported yet */
+		break;
+	case RTE_FLOW_FIELD_GENEVE_VNI:
+		/* not supported yet*/
+		break;
+	case RTE_FLOW_FIELD_GTP_TEID:
+		info[idx] = (struct field_modify_info){4, 0, 0x6E};
+		if (mask)
+			mask[idx] = 0xffffffff;
+		break;
+	case RTE_FLOW_FIELD_TAG:
+		{
+			int ret;
+			enum mlx5_modification_field reg_type;
+			ret = mlx5_flow_get_reg_id(dev, MLX5_APP_TAG,
+						   data->level, error);
+			if (ret < 0)
+				return;
+			MLX5_ASSERT(ret != REG_NON);
+			MLX5_ASSERT((unsigned int)ret < RTE_DIM(reg_to_field));
+			reg_type = reg_to_field[ret];
+			MLX5_ASSERT(reg_type > 0);
+			info[idx] = (struct field_modify_info){4, 0, reg_type};
+			if (mask)
+				mask[idx] = 0xffffffff;
+		}
+		break;
+	case RTE_FLOW_FIELD_MARK:
+		{
+			int reg = mlx5_flow_get_reg_id(dev, MLX5_FLOW_MARK,
+						       0, error);
+			if (reg < 0)
+				return;
+			MLX5_ASSERT(reg > 0);
+			info[idx] = (struct field_modify_info){4, 0,
+						reg_to_field[reg]};
+			if (mask)
+				mask[idx] = 0xffffffff;
+		}
+		break;
+	case RTE_FLOW_FIELD_META:
+		{
+			int reg = flow_dv_get_metadata_reg(dev, attr, error);
+			if (reg < 0)
+				return;
+			MLX5_ASSERT(reg != REG_NON);
+			info[idx] = (struct field_modify_info){4, 0,
+						reg_to_field[reg]};
+			if (mask)
+				mask[idx] = 0xffffffff;
+		}
+		break;
+	case RTE_FLOW_FIELD_POINTER:
+		for (idx = 0; idx < MLX5DV_FLOW_MAX_MOD_FIELDS; idx++) {
+			if (mask[idx]) {
+				value[idx] = RTE_BE32(*(uint64_t *)data->value);
+				break;
+			}
+		}
+	case RTE_FLOW_FIELD_VALUE:
+		for (idx = 0; idx < MLX5DV_FLOW_MAX_MOD_FIELDS; idx++) {
+			if (mask[idx]) {
+				value[idx] = RTE_BE32(data->value);
+				break;
+			}
+		}
+		break;
+	default:
+		MLX5_ASSERT(false);
+		break;
+	}
+}
+
+/**
+ * Convert modify_field action to DV specification.
+ *
+ * @param[in] dev
+ *   Pointer to the rte_eth_dev structure.
+ * @param[in,out] resource
+ *   Pointer to the modify-header resource.
+ * @param[in] action
+ *   Pointer to action specification.
+ * @param[in] attr
+ *   Attributes of flow that includes this item.
+ * @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_field
+			(struct rte_eth_dev *dev,
+			 struct mlx5_flow_dv_modify_hdr_resource *resource,
+			 const struct rte_flow_action *action,
+			 const struct rte_flow_attr *attr,
+			 struct rte_flow_error *error)
+{
+	const struct rte_flow_action_modify_field *conf =
+		(const struct rte_flow_action_modify_field *)(action->conf);
+	struct rte_flow_item item;
+	struct field_modify_info field[MLX5DV_FLOW_MAX_MOD_FIELDS] = {
+								{0, 0, 0} };
+	struct field_modify_info dcopy[MLX5DV_FLOW_MAX_MOD_FIELDS] = {
+								{0, 0, 0} };
+	uint32_t mask[MLX5DV_FLOW_MAX_MOD_FIELDS] = {0, 0, 0, 0, 0};
+	uint32_t value[MLX5DV_FLOW_MAX_MOD_FIELDS] = {0, 0, 0, 0, 0};
+	uint32_t type;
+
+	if (conf->src.field == RTE_FLOW_FIELD_POINTER ||
+		conf->src.field == RTE_FLOW_FIELD_VALUE) {
+		type = MLX5_MODIFICATION_TYPE_SET;
+		/** For SET fill the destination field (field) first. */
+		mlx5_flow_field_id_to_modify_info(&conf->dst, field, mask,
+						  value, dev, attr, error);
+		/** Then copy immediate value from source as per mask. */
+		mlx5_flow_field_id_to_modify_info(&conf->src, dcopy, mask,
+						  value, dev, attr, error);
+		item.spec = &value;
+	} else {
+		type = MLX5_MODIFICATION_TYPE_COPY;
+		/** For COPY fill the destination field (dcopy) without mask. */
+		mlx5_flow_field_id_to_modify_info(&conf->dst, dcopy, NULL,
+						  value, dev, attr, error);
+		/** Then cnstruct th source field (field) with mask. */
+		mlx5_flow_field_id_to_modify_info(&conf->src, field, mask,
+						  value, dev, attr, error);
+	}
+	item.mask = &mask;
+	return flow_dv_convert_modify_action(&item,
+			field, dcopy, resource, type, error);
+}
+
 /**
  * Validate MARK item.
  *
@@ -3985,6 +4323,148 @@  flow_dv_validate_action_modify_ttl(const uint64_t action_flags,
 	return ret;
 }
 
+static int
+mlx5_flow_item_field_width(enum rte_flow_field_id field)
+{
+	switch (field) {
+	case RTE_FLOW_FIELD_START:
+		return 32;
+	case RTE_FLOW_FIELD_MAC_DST:
+	case RTE_FLOW_FIELD_MAC_SRC:
+		return 48;
+	case RTE_FLOW_FIELD_VLAN_TYPE:
+		return 16;
+	case RTE_FLOW_FIELD_VLAN_ID:
+		return 12;
+	case RTE_FLOW_FIELD_MAC_TYPE:
+		return 16;
+	case RTE_FLOW_FIELD_IPV4_DSCP:
+	case RTE_FLOW_FIELD_IPV4_TTL:
+		return 8;
+	case RTE_FLOW_FIELD_IPV4_SRC:
+	case RTE_FLOW_FIELD_IPV4_DST:
+		return 32;
+	case RTE_FLOW_FIELD_IPV6_HOPLIMIT:
+		return 8;
+	case RTE_FLOW_FIELD_IPV6_SRC:
+	case RTE_FLOW_FIELD_IPV6_DST:
+		return 128;
+	case RTE_FLOW_FIELD_TCP_PORT_SRC:
+	case RTE_FLOW_FIELD_TCP_PORT_DST:
+		return 16;
+	case RTE_FLOW_FIELD_TCP_SEQ_NUM:
+	case RTE_FLOW_FIELD_TCP_ACK_NUM:
+		return 32;
+	case RTE_FLOW_FIELD_TCP_FLAGS:
+		return 6;
+	case RTE_FLOW_FIELD_UDP_PORT_SRC:
+	case RTE_FLOW_FIELD_UDP_PORT_DST:
+		return 16;
+	case RTE_FLOW_FIELD_VXLAN_VNI:
+	case RTE_FLOW_FIELD_GENEVE_VNI:
+		return 24;
+	case RTE_FLOW_FIELD_GTP_TEID:
+	case RTE_FLOW_FIELD_TAG:
+	case RTE_FLOW_FIELD_MARK:
+	case RTE_FLOW_FIELD_META:
+	case RTE_FLOW_FIELD_POINTER:
+	case RTE_FLOW_FIELD_VALUE:
+		return 32;
+	default:
+		MLX5_ASSERT(false);
+	}
+	return 0;
+}
+
+/**
+ * Validate the generic modify field 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
+ *   Number of header fields to modify (0 or more) on success,
+ *   a negative errno value otherwise and rte_errno is set.
+ */
+static int
+flow_dv_validate_action_modify_field(const uint64_t action_flags,
+				   const struct rte_flow_action *action,
+				   struct rte_flow_error *error)
+{
+	int ret = 0;
+	const struct rte_flow_action_modify_field *action_modify_field =
+		action->conf;
+	uint32_t dst_width =
+		mlx5_flow_item_field_width(action_modify_field->dst.field);
+	uint32_t src_width =
+		mlx5_flow_item_field_width(action_modify_field->src.field);
+
+	ret = flow_dv_validate_action_modify_hdr(action_flags, action, error);
+	if (ret)
+		return ret;
+
+	if (action_modify_field->src.field != RTE_FLOW_FIELD_VALUE &&
+	    action_modify_field->src.field != RTE_FLOW_FIELD_POINTER) {
+		if (action_modify_field->dst.offset >= dst_width ||
+		    (action_modify_field->dst.offset % 32) ||
+		    action_modify_field->src.offset >= src_width ||
+		    (action_modify_field->src.offset % 32))
+			return rte_flow_error_set(error, EINVAL,
+						RTE_FLOW_ERROR_TYPE_ACTION,
+						NULL,
+						"offset is too big"
+						" or not aligned to 4 bytes");
+		if ((action_modify_field->dst.level &&
+		     action_modify_field->dst.field != RTE_FLOW_FIELD_TAG) ||
+		    (action_modify_field->src.level &&
+		     action_modify_field->src.field != RTE_FLOW_FIELD_TAG))
+			return rte_flow_error_set(error, EINVAL,
+						RTE_FLOW_ERROR_TYPE_ACTION,
+						NULL,
+						"inner header modifications"
+						" are not supported");
+	}
+	if (action_modify_field->width == 0)
+		return rte_flow_error_set(error, EINVAL,
+						RTE_FLOW_ERROR_TYPE_ACTION,
+						NULL,
+						"width is required for modify action");
+	if (action_modify_field->dst.field ==
+	    action_modify_field->src.field)
+		return rte_flow_error_set(error, EINVAL,
+					RTE_FLOW_ERROR_TYPE_ACTION,
+					NULL,
+					"source and destination fields"
+					" cannot be the same");
+	if (action_modify_field->dst.field == RTE_FLOW_FIELD_VALUE ||
+	    action_modify_field->dst.field == RTE_FLOW_FIELD_POINTER)
+		return rte_flow_error_set(error, EINVAL,
+					RTE_FLOW_ERROR_TYPE_ACTION,
+					NULL,
+					"immediate value or a pointer to it"
+					" cannot be used as a destination");
+	if (action_modify_field->dst.field == RTE_FLOW_FIELD_START)
+		return rte_flow_error_set(error, EINVAL,
+				RTE_FLOW_ERROR_TYPE_ACTION,
+				NULL,
+				"modifications of an arbitrary"
+				" place in a packet is not supported");
+	if (action_modify_field->operation != RTE_FLOW_MODIFY_SET)
+		return rte_flow_error_set(error, EINVAL,
+				RTE_FLOW_ERROR_TYPE_ACTION,
+				NULL,
+				"add and sub operations"
+				" are not supported");
+	return (action_modify_field->width / 32) +
+	       !!(action_modify_field->width % 32);
+}
+
 /**
  * Validate jump action.
  *
@@ -5373,7 +5853,7 @@  flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr,
 	struct mlx5_dev_config *dev_conf = &priv->config;
 	uint16_t queue_index = 0xFFFF;
 	const struct rte_flow_item_vlan *vlan_m = NULL;
-	int16_t rw_act_num = 0;
+	uint32_t rw_act_num = 0;
 	uint64_t is_root;
 	const struct mlx5_flow_tunnel *tunnel;
 	struct flow_grp_info grp_info = {
@@ -6169,6 +6649,23 @@  flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr,
 
 			action_flags |= MLX5_FLOW_ACTION_TUNNEL_SET;
 			break;
+		case RTE_FLOW_ACTION_TYPE_MODIFY_FIELD:
+			if (!attr->transfer && !attr->group)
+				return rte_flow_error_set(error, ENOTSUP,
+						RTE_FLOW_ERROR_TYPE_ACTION,
+						NULL, "modify field action "
+						"is not supported for group 0");
+			ret = flow_dv_validate_action_modify_field(action_flags,
+								 actions,
+								 error);
+			if (ret < 0)
+				return ret;
+			/* Count all modify-header actions as one action. */
+			if (!(action_flags & MLX5_FLOW_ACTION_MODIFY_FIELD))
+				++actions_n;
+			action_flags |= MLX5_FLOW_ACTION_MODIFY_FIELD;
+			rw_act_num += ret;
+			break;
 		default:
 			return rte_flow_error_set(error, ENOTSUP,
 						  RTE_FLOW_ERROR_TYPE_ACTION,
@@ -6326,7 +6823,7 @@  flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr,
 	    dev_conf->dv_xmeta_en != MLX5_XMETA_MODE_LEGACY &&
 	    mlx5_flow_ext_mreg_supported(dev))
 		rw_act_num += MLX5_ACT_NUM_SET_TAG;
-	if ((uint32_t)rw_act_num >
+	if (rw_act_num >
 			flow_dv_modify_hdr_action_max(dev, is_root)) {
 		return rte_flow_error_set(error, ENOTSUP,
 					  RTE_FLOW_ERROR_TYPE_ACTION,
@@ -10686,6 +11183,12 @@  flow_dv_translate(struct rte_eth_dev *dev,
 				sample_act->action_flags |=
 							MLX5_FLOW_ACTION_ENCAP;
 			break;
+		case RTE_FLOW_ACTION_TYPE_MODIFY_FIELD:
+			if (flow_dv_convert_action_modify_field
+					(dev, mhdr_res, actions, attr, error))
+				return -rte_errno;
+			action_flags |= MLX5_FLOW_ACTION_MODIFY_FIELD;
+			break;
 		case RTE_FLOW_ACTION_TYPE_END:
 			actions_end = true;
 			if (mhdr_res->actions_num) {