Extend the range of immediate value used in the MODIFY_FIELD action from 32 to 64 bits to conform to the rte_flow_action_modify_data spec. Apply appropriate big endian conversion to the immediate value according to a destination field bit width. Fixes: 641dbe4fb053 ("net/mlx5: support modify field flow action") Cc: stable@dpdk.org Signed-off-by: Alexander Kozyrev <akozyrev@nvidia.com> --- drivers/net/mlx5/mlx5_flow_dv.c | 158 +++++++++++++++++--------------- 1 file changed, 83 insertions(+), 75 deletions(-) diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c index 45e34395a8..6f8d16cec3 100644 --- a/drivers/net/mlx5/mlx5_flow_dv.c +++ b/drivers/net/mlx5/mlx5_flow_dv.c @@ -1325,16 +1325,77 @@ flow_dv_convert_action_modify_ipv6_dscp MLX5_MODIFICATION_TYPE_SET, error); } +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: + return 6; + 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_DSCP: + return 6; + 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: + return 32; + case RTE_FLOW_FIELD_MARK: + return 24; + case RTE_FLOW_FIELD_META: + return 32; + case RTE_FLOW_FIELD_POINTER: + case RTE_FLOW_FIELD_VALUE: + return 64; + default: + MLX5_ASSERT(false); + } + return 0; +} + 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, uint32_t width, + uint32_t *mask, uint32_t *value, + uint32_t width, uint32_t dst_width, struct rte_eth_dev *dev, const struct rte_flow_attr *attr, struct rte_flow_error *error) { uint32_t idx = 0; + uint64_t val = 0; switch (data->field) { case RTE_FLOW_FIELD_START: /* not supported yet */ @@ -1698,21 +1759,25 @@ mlx5_flow_field_id_to_modify_info } break; case RTE_FLOW_FIELD_POINTER: - for (idx = 0; idx < MLX5_ACT_MAX_MOD_FIELDS; idx++) { - if (mask[idx]) { - memcpy(&value[idx], - (void *)(uintptr_t)data->value, 32); - value[idx] = rte_cpu_to_be_32(value[idx]); - break; - } - } - break; case RTE_FLOW_FIELD_VALUE: + if (data->field == RTE_FLOW_FIELD_POINTER) + memcpy(&val, (void *)(uintptr_t)data->value, 64); + else + val = data->value; for (idx = 0; idx < MLX5_ACT_MAX_MOD_FIELDS; idx++) { if (mask[idx]) { - value[idx] = - rte_cpu_to_be_32((uint32_t)data->value); - break; + if (dst_width > 16) { + value[idx] = rte_cpu_to_be_32(val); + val >>= 32; + } else if (dst_width > 8) { + value[idx] = rte_cpu_to_be_16(val); + val >>= 16; + } else { + value[idx] = (uint8_t)val; + val >>= 8; + } + if (!val) + break; } } break; @@ -1757,25 +1822,26 @@ flow_dv_convert_action_modify_field uint32_t mask[MLX5_ACT_MAX_MOD_FIELDS] = {0, 0, 0, 0, 0}; uint32_t value[MLX5_ACT_MAX_MOD_FIELDS] = {0, 0, 0, 0, 0}; uint32_t type; + uint32_t dst_width = mlx5_flow_item_field_width(conf->dst.field); 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, conf->width, dev, attr, error); + value, conf->width, dst_width, dev, attr, error); /** Then copy immediate value from source as per mask. */ mlx5_flow_field_id_to_modify_info(&conf->src, dcopy, mask, - value, conf->width, dev, attr, error); + value, conf->width, dst_width, 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, conf->width, dev, attr, error); + value, conf->width, dst_width, dev, attr, error); /** Then construct the source field (field) with mask. */ mlx5_flow_field_id_to_modify_info(&conf->src, field, mask, - value, conf->width, dev, attr, error); + value, conf->width, dst_width, dev, attr, error); } item.mask = &mask; return flow_dv_convert_modify_action(&item, @@ -4469,64 +4535,6 @@ 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: - return 6; - 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_DSCP: - return 6; - 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: - return 32; - case RTE_FLOW_FIELD_MARK: - return 24; - 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] dev -- 2.24.1
> -----Original Message-----
> From: Alexander Kozyrev <akozyrev@nvidia.com>
> Sent: Wednesday, April 7, 2021 4:14
> To: dev@dpdk.org
> Cc: stable@dpdk.org; Raslan Darawsheh <rasland@nvidia.com>; Slava
> Ovsiienko <viacheslavo@nvidia.com>
> Subject: [PATCH] net/mlx5: support 64-bit value for modify field action
>
> Extend the range of immediate value used in the MODIFY_FIELD action from
> 32 to 64 bits to conform to the rte_flow_action_modify_data spec.
> Apply appropriate big endian conversion to the immediate value according to
> a destination field bit width.
>
> Fixes: 641dbe4fb053 ("net/mlx5: support modify field flow action")
> Cc: stable@dpdk.org
>
> Signed-off-by: Alexander Kozyrev <akozyrev@nvidia.com>
Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>
Hi Alex,
This patch is causing a compilation failure as following:
[1857/3005] Compiling C object drivers/libtmp_rte_net_mlx5.a.p/net_mlx5_mlx5_flow_dv.c.o
FAILED: drivers/libtmp_rte_net_mlx5.a.p/net_mlx5_mlx5_flow_dv.c.o
gcc -Idrivers/libtmp_rte_net_mlx5.a.p -Idrivers -I../../root/dpdk/drivers -Idrivers/net/mlx5 -I../../root/dpdk/drivers/net/mlx5 -Idrivers/net/mlx5/linux -I../../root/dpdk/drivers/net/mlx5/linux -Ilib/librte_ethdev -I../../root/dpdk/lib/librte_ethdev -I. -I../../root/dpdk -Iconfig -I../../root/dpdk/config -Ilib/librte_eal/include -I../../root/dpdk/lib/librte_eal/include -Ilib/librte_eal/linux/include -I../../root/dpdk/lib/librte_eal/linux/include -Ilib/librte_eal/x86/include -I../../root/dpdk/lib/librte_eal/x86/include -Ilib/librte_eal/common -I../../root/dpdk/lib/librte_eal/common -Ilib/librte_eal -I../../root/dpdk/lib/librte_eal -Ilib/librte_kvargs -I../../root/dpdk/lib/librte_kvargs -Ilib/librte_metrics -I../../root/dpdk/lib/librte_metrics -Ilib/librte_telemetry -I../../root/dpdk/lib/librte_telemetry -Ilib/librte_net -I../../root/dpdk/lib/librte_net -Ilib/librte_mbuf -I../../root/dpdk/lib/librte_mbuf -Ilib/librte_mempool -I../../root/dpdk/lib/librte_mempool -Ilib/librte_ring -I../../root/dpdk/lib/librte_ring -Ilib/librte_meter -I../../root/dpdk/lib/librte_meter -Idrivers/bus/pci -I../../root/dpdk/drivers/bus/pci -I../../root/dpdk/drivers/bus/pci/linux -Ilib/librte_pci -I../../root/dpdk/lib/librte_pci -Idrivers/bus/vdev -I../../root/dpdk/drivers/bus/vdev -Ilib/librte_hash -I../../root/dpdk/lib/librte_hash -Ilib/librte_rcu -I../../root/dpdk/lib/librte_rcu -Idrivers/common/mlx5 -I../../root/dpdk/drivers/common/mlx5 -Idrivers/common/mlx5/linux -I../../root/dpdk/drivers/common/mlx5/linux -I/usr/usr/include -fdiagnostics-color=always -pipe -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Werror -O2 -g -include rte_config.h -Wextra -Wcast-qual -Wdeprecated -Wformat -Wformat-nonliteral -Wformat-security -Wmissing-declarations -Wmissing-prototypes -Wnested-externs -Wold-style-definition -Wpointer-arith -Wsign-compare -Wstrict-prototypes -Wundef -Wwrite-strings -Wno-address-of-packed-member -Wno-packed-not-aligned -Wno-missing-field-initializers -Wno-zero-length-bounds -D_GNU_SOURCE -fPIC -march=native -DALLOW_EXPERIMENTAL_API -DALLOW_INTERNAL_API -Wno-format-truncation -std=c11 -Wno-strict-prototypes -D_BSD_SOURCE -D_DEFAULT_SOURCE -D_XOPEN_SOURCE=600 -pedantic -DPEDANTIC -MD -MQ drivers/libtmp_rte_net_mlx5.a.p/net_mlx5_mlx5_flow_dv.c.o -MF drivers/libtmp_rte_net_mlx5.a.p/net_mlx5_mlx5_flow_dv.c.o.d -o drivers/libtmp_rte_net_mlx5.a.p/net_mlx5_mlx5_flow_dv.c.o -c ../../root/dpdk/drivers/net/mlx5/mlx5_flow_dv.c
../../root/dpdk/drivers/net/mlx5/mlx5_flow_dv.c: In function 'mlx5_flow_field_id_to_modify_info':
../../root/dpdk/drivers/net/mlx5/mlx5_flow_dv.c:1777:4: error: 'memcpy' forming offset [8, 63] is out of the bounds [0, 8] of object 'val' with type 'uint64_t' {aka 'long unsigned int'} [-Werror=array-bounds]
1777 | memcpy(&val, (void *)(uintptr_t)data->value, 64);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../root/dpdk/drivers/net/mlx5/mlx5_flow_dv.c:1398:11: note: 'val' declared here
1398 | uint64_t val = 0;
| ^~~
cc1: all warnings being treated as errors
[1914/3005] Compiling C object drivers/libtmp_rte_net_mlx5.a.p/net_mlx5_mlx5_rxtx.c.o
ninja: build stopped: subcommand failed.
This is with gcc: CC(host): gcc (GCC) 10.2.1 20201125 (Red Hat 10.2.1-9)
Kindest regards,
Raslan Darawsheh
> -----Original Message-----
> From: Alexander Kozyrev <akozyrev@nvidia.com>
> Sent: Wednesday, April 7, 2021 4:14 AM
> To: dev@dpdk.org
> Cc: stable@dpdk.org; Raslan Darawsheh <rasland@nvidia.com>; Slava
> Ovsiienko <viacheslavo@nvidia.com>
> Subject: [PATCH] net/mlx5: support 64-bit value for modify field action
>
> Extend the range of immediate value used in the MODIFY_FIELD action
> from 32 to 64 bits to conform to the rte_flow_action_modify_data spec.
> Apply appropriate big endian conversion to the immediate value
> according to a destination field bit width.
>
> Fixes: 641dbe4fb053 ("net/mlx5: support modify field flow action")
> Cc: stable@dpdk.org
>
> Signed-off-by: Alexander Kozyrev <akozyrev@nvidia.com>
> ---
> drivers/net/mlx5/mlx5_flow_dv.c | 158 +++++++++++++++++---------------
> 1 file changed, 83 insertions(+), 75 deletions(-)
>
> diff --git a/drivers/net/mlx5/mlx5_flow_dv.c
> b/drivers/net/mlx5/mlx5_flow_dv.c
> index 45e34395a8..6f8d16cec3 100644
> --- a/drivers/net/mlx5/mlx5_flow_dv.c
> +++ b/drivers/net/mlx5/mlx5_flow_dv.c
> @@ -1325,16 +1325,77 @@ flow_dv_convert_action_modify_ipv6_dscp
> MLX5_MODIFICATION_TYPE_SET,
> error);
> }
>
> +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:
> + return 6;
> + 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_DSCP:
> + return 6;
> + 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:
> + return 32;
> + case RTE_FLOW_FIELD_MARK:
> + return 24;
> + case RTE_FLOW_FIELD_META:
> + return 32;
> + case RTE_FLOW_FIELD_POINTER:
> + case RTE_FLOW_FIELD_VALUE:
> + return 64;
> + default:
> + MLX5_ASSERT(false);
> + }
> + return 0;
> +}
> +
> 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, uint32_t width,
> + uint32_t *mask, uint32_t *value,
> + uint32_t width, uint32_t dst_width,
> struct rte_eth_dev *dev,
> const struct rte_flow_attr *attr,
> struct rte_flow_error *error)
> {
> uint32_t idx = 0;
> + uint64_t val = 0;
> switch (data->field) {
> case RTE_FLOW_FIELD_START:
> /* not supported yet */
> @@ -1698,21 +1759,25 @@ mlx5_flow_field_id_to_modify_info
> }
> break;
> case RTE_FLOW_FIELD_POINTER:
> - for (idx = 0; idx < MLX5_ACT_MAX_MOD_FIELDS; idx++) {
> - if (mask[idx]) {
> - memcpy(&value[idx],
> - (void *)(uintptr_t)data->value, 32);
> - value[idx] = rte_cpu_to_be_32(value[idx]);
> - break;
> - }
> - }
> - break;
> case RTE_FLOW_FIELD_VALUE:
> + if (data->field == RTE_FLOW_FIELD_POINTER)
> + memcpy(&val, (void *)(uintptr_t)data->value, 64);
> + else
> + val = data->value;
> for (idx = 0; idx < MLX5_ACT_MAX_MOD_FIELDS; idx++) {
> if (mask[idx]) {
> - value[idx] =
> - rte_cpu_to_be_32((uint32_t)data-
> >value);
> - break;
> + if (dst_width > 16) {
> + value[idx] = rte_cpu_to_be_32(val);
> + val >>= 32;
> + } else if (dst_width > 8) {
> + value[idx] = rte_cpu_to_be_16(val);
> + val >>= 16;
> + } else {
> + value[idx] = (uint8_t)val;
> + val >>= 8;
> + }
> + if (!val)
> + break;
> }
> }
> break;
> @@ -1757,25 +1822,26 @@ flow_dv_convert_action_modify_field
> uint32_t mask[MLX5_ACT_MAX_MOD_FIELDS] = {0, 0, 0, 0, 0};
> uint32_t value[MLX5_ACT_MAX_MOD_FIELDS] = {0, 0, 0, 0, 0};
> uint32_t type;
> + uint32_t dst_width = mlx5_flow_item_field_width(conf->dst.field);
>
> 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, conf->width, dev, attr, error);
> + value, conf->width, dst_width, dev, attr, error);
> /** Then copy immediate value from source as per mask. */
> mlx5_flow_field_id_to_modify_info(&conf->src, dcopy,
> mask,
> - value, conf->width, dev, attr, error);
> + value, conf->width, dst_width, 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, conf->width, dev, attr, error);
> + value, conf->width, dst_width, dev, attr, error);
> /** Then construct the source field (field) with mask. */
> mlx5_flow_field_id_to_modify_info(&conf->src, field, mask,
> - value, conf->width, dev, attr, error);
> + value, conf->width, dst_width, dev, attr, error);
> }
> item.mask = &mask;
> return flow_dv_convert_modify_action(&item,
> @@ -4469,64 +4535,6 @@ 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:
> - return 6;
> - 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_DSCP:
> - return 6;
> - 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:
> - return 32;
> - case RTE_FLOW_FIELD_MARK:
> - return 24;
> - 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] dev
> --
> 2.24.1
Extend the range of immediate value used in the MODIFY_FIELD action from 32 to 64 bits to conform to the rte_flow_action_modify_data spec. Apply appropriate big endian conversion to the immediate value according to a destination field bit width. Fixes: 641dbe4fb053 ("net/mlx5: support modify field flow action") Cc: stable@dpdk.org Signed-off-by: Alexander Kozyrev <akozyrev@nvidia.com> Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com> --- drivers/net/mlx5/mlx5_flow_dv.c | 159 +++++++++++++++++--------------- 1 file changed, 84 insertions(+), 75 deletions(-) diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c index 45e34395a8..d230060230 100644 --- a/drivers/net/mlx5/mlx5_flow_dv.c +++ b/drivers/net/mlx5/mlx5_flow_dv.c @@ -1325,16 +1325,77 @@ flow_dv_convert_action_modify_ipv6_dscp MLX5_MODIFICATION_TYPE_SET, error); } +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: + return 6; + 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_DSCP: + return 6; + 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: + return 32; + case RTE_FLOW_FIELD_MARK: + return 24; + case RTE_FLOW_FIELD_META: + return 32; + case RTE_FLOW_FIELD_POINTER: + case RTE_FLOW_FIELD_VALUE: + return 64; + default: + MLX5_ASSERT(false); + } + return 0; +} + 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, uint32_t width, + uint32_t *mask, uint32_t *value, + uint32_t width, uint32_t dst_width, struct rte_eth_dev *dev, const struct rte_flow_attr *attr, struct rte_flow_error *error) { uint32_t idx = 0; + uint64_t val = 0; switch (data->field) { case RTE_FLOW_FIELD_START: /* not supported yet */ @@ -1698,21 +1759,26 @@ mlx5_flow_field_id_to_modify_info } break; case RTE_FLOW_FIELD_POINTER: - for (idx = 0; idx < MLX5_ACT_MAX_MOD_FIELDS; idx++) { - if (mask[idx]) { - memcpy(&value[idx], - (void *)(uintptr_t)data->value, 32); - value[idx] = rte_cpu_to_be_32(value[idx]); - break; - } - } - break; case RTE_FLOW_FIELD_VALUE: + if (data->field == RTE_FLOW_FIELD_POINTER) + memcpy(&val, (void *)(uintptr_t)data->value, + sizeof(uint64_t)); + else + val = data->value; for (idx = 0; idx < MLX5_ACT_MAX_MOD_FIELDS; idx++) { if (mask[idx]) { - value[idx] = - rte_cpu_to_be_32((uint32_t)data->value); - break; + if (dst_width > 16) { + value[idx] = rte_cpu_to_be_32(val); + val >>= 32; + } else if (dst_width > 8) { + value[idx] = rte_cpu_to_be_16(val); + val >>= 16; + } else { + value[idx] = (uint8_t)val; + val >>= 8; + } + if (!val) + break; } } break; @@ -1757,25 +1823,26 @@ flow_dv_convert_action_modify_field uint32_t mask[MLX5_ACT_MAX_MOD_FIELDS] = {0, 0, 0, 0, 0}; uint32_t value[MLX5_ACT_MAX_MOD_FIELDS] = {0, 0, 0, 0, 0}; uint32_t type; + uint32_t dst_width = mlx5_flow_item_field_width(conf->dst.field); 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, conf->width, dev, attr, error); + value, conf->width, dst_width, dev, attr, error); /** Then copy immediate value from source as per mask. */ mlx5_flow_field_id_to_modify_info(&conf->src, dcopy, mask, - value, conf->width, dev, attr, error); + value, conf->width, dst_width, 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, conf->width, dev, attr, error); + value, conf->width, dst_width, dev, attr, error); /** Then construct the source field (field) with mask. */ mlx5_flow_field_id_to_modify_info(&conf->src, field, mask, - value, conf->width, dev, attr, error); + value, conf->width, dst_width, dev, attr, error); } item.mask = &mask; return flow_dv_convert_modify_action(&item, @@ -4469,64 +4536,6 @@ 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: - return 6; - 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_DSCP: - return 6; - 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: - return 32; - case RTE_FLOW_FIELD_MARK: - return 24; - 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] dev -- 2.24.1
Hi,
> -----Original Message-----
> From: Alexander Kozyrev <akozyrev@nvidia.com>
> Sent: Wednesday, April 7, 2021 6:18 PM
> To: dev@dpdk.org
> Cc: stable@dpdk.org; Raslan Darawsheh <rasland@nvidia.com>; Slava
> Ovsiienko <viacheslavo@nvidia.com>
> Subject: [PATCH v2] net/mlx5: support 64-bit value for modify field action
>
> Extend the range of immediate value used in the MODIFY_FIELD action
> from 32 to 64 bits to conform to the rte_flow_action_modify_data spec.
> Apply appropriate big endian conversion to the immediate value
> according to a destination field bit width.
>
> Fixes: 641dbe4fb053 ("net/mlx5: support modify field flow action")
> Cc: stable@dpdk.org
>
> Signed-off-by: Alexander Kozyrev <akozyrev@nvidia.com>
> Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>
Patch applied to next-net-mlx,
Kindest regards,
Raslan Darawsheh