From: Raslan Darawsheh <rasland@nvidia.com>
To: Alexander Kozyrev <akozyrev@nvidia.com>, "dev@dpdk.org" <dev@dpdk.org>
Cc: "stable@dpdk.org" <stable@dpdk.org>,
Slava Ovsiienko <viacheslavo@nvidia.com>
Subject: Re: [dpdk-stable] [PATCH] net/mlx5: support 64-bit value for modify field action
Date: Wed, 7 Apr 2021 15:09:00 +0000 [thread overview]
Message-ID: <DM6PR12MB274873270DF39AA4A9383051CF759@DM6PR12MB2748.namprd12.prod.outlook.com> (raw)
In-Reply-To: <20210407011417.22675-1-akozyrev@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
next prev parent reply other threads:[~2021-04-07 15:09 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-07 1:14 Alexander Kozyrev
2021-04-07 7:32 ` Slava Ovsiienko
2021-04-07 15:09 ` Raslan Darawsheh [this message]
2021-04-07 15:18 ` [dpdk-stable] [PATCH v2] " Alexander Kozyrev
2021-04-11 7:23 ` Raslan Darawsheh
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=DM6PR12MB274873270DF39AA4A9383051CF759@DM6PR12MB2748.namprd12.prod.outlook.com \
--to=rasland@nvidia.com \
--cc=akozyrev@nvidia.com \
--cc=dev@dpdk.org \
--cc=stable@dpdk.org \
--cc=viacheslavo@nvidia.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).