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 <akozy...@nvidia.com>
> Sent: Wednesday, April 7, 2021 4:14 AM
> To: dev@dpdk.org
> Cc: sta...@dpdk.org; Raslan Darawsheh <rasl...@nvidia.com>; Slava
> Ovsiienko <viachesl...@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: sta...@dpdk.org
>
> Signed-off-by: Alexander Kozyrev <akozy...@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
