* [PATCH v1] net/mlx5: fix action flag data type
@ 2022-11-01 10:07 Shun Hao
2022-11-03 11:41 ` Raslan Darawsheh
2022-11-14 11:00 ` Thomas Monjalon
0 siblings, 2 replies; 3+ messages in thread
From: Shun Hao @ 2022-11-01 10:07 UTC (permalink / raw)
To: viacheslavo, matan, orika, Bing Zhao; +Cc: dev, rasland, stable
MLX5_FLOW_ACTION flags are used as uint64_t now, but some old flags
are not defined as 64 bits. So if they are type casted to uint64 after
bitwise operations, the high 32-bit data might be incorrect.
E.g. Currently MLX5_FLOW_ACTION_DROP is defined as 0x1u, when it is used
like:
(action_flags & ~MLX5_FLOW_ACTION_DROP)
action_flags is uint64_t so (~MLX5_FLOW_ACTION_DROP) will be casted to
uint64_t as well, but its high 32 bits will be all 0s. This will make the
result not as expected.
This patch fixes this by making all action flags definition as 64-bit
data type.
Fixes: 4b7bf3ff ("net/mlx5: support yellow in meter policy validation")
Cc: stable@dpdk.org
Signed-off-by: Shun Hao <shunh@nvidia.com>
Acked-by: Matan Azrad <matan@nvidia.com>
---
drivers/net/mlx5/mlx5_flow.h | 56 ++++++++++++++++++------------------
1 file changed, 28 insertions(+), 28 deletions(-)
diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index da9b65d8fe..91691806ad 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -258,34 +258,34 @@ enum mlx5_feature_name {
(MLX5_FLOW_LAYER_OUTER_L4 | MLX5_FLOW_LAYER_INNER_L4)
/* Actions */
-#define MLX5_FLOW_ACTION_DROP (1u << 0)
-#define MLX5_FLOW_ACTION_QUEUE (1u << 1)
-#define MLX5_FLOW_ACTION_RSS (1u << 2)
-#define MLX5_FLOW_ACTION_FLAG (1u << 3)
-#define MLX5_FLOW_ACTION_MARK (1u << 4)
-#define MLX5_FLOW_ACTION_COUNT (1u << 5)
-#define MLX5_FLOW_ACTION_PORT_ID (1u << 6)
-#define MLX5_FLOW_ACTION_OF_POP_VLAN (1u << 7)
-#define MLX5_FLOW_ACTION_OF_PUSH_VLAN (1u << 8)
-#define MLX5_FLOW_ACTION_OF_SET_VLAN_VID (1u << 9)
-#define MLX5_FLOW_ACTION_OF_SET_VLAN_PCP (1u << 10)
-#define MLX5_FLOW_ACTION_SET_IPV4_SRC (1u << 11)
-#define MLX5_FLOW_ACTION_SET_IPV4_DST (1u << 12)
-#define MLX5_FLOW_ACTION_SET_IPV6_SRC (1u << 13)
-#define MLX5_FLOW_ACTION_SET_IPV6_DST (1u << 14)
-#define MLX5_FLOW_ACTION_SET_TP_SRC (1u << 15)
-#define MLX5_FLOW_ACTION_SET_TP_DST (1u << 16)
-#define MLX5_FLOW_ACTION_JUMP (1u << 17)
-#define MLX5_FLOW_ACTION_SET_TTL (1u << 18)
-#define MLX5_FLOW_ACTION_DEC_TTL (1u << 19)
-#define MLX5_FLOW_ACTION_SET_MAC_SRC (1u << 20)
-#define MLX5_FLOW_ACTION_SET_MAC_DST (1u << 21)
-#define MLX5_FLOW_ACTION_ENCAP (1u << 22)
-#define MLX5_FLOW_ACTION_DECAP (1u << 23)
-#define MLX5_FLOW_ACTION_INC_TCP_SEQ (1u << 24)
-#define MLX5_FLOW_ACTION_DEC_TCP_SEQ (1u << 25)
-#define MLX5_FLOW_ACTION_INC_TCP_ACK (1u << 26)
-#define MLX5_FLOW_ACTION_DEC_TCP_ACK (1u << 27)
+#define MLX5_FLOW_ACTION_DROP (1ull << 0)
+#define MLX5_FLOW_ACTION_QUEUE (1ull << 1)
+#define MLX5_FLOW_ACTION_RSS (1ull << 2)
+#define MLX5_FLOW_ACTION_FLAG (1ull << 3)
+#define MLX5_FLOW_ACTION_MARK (1ull << 4)
+#define MLX5_FLOW_ACTION_COUNT (1ull << 5)
+#define MLX5_FLOW_ACTION_PORT_ID (1ull << 6)
+#define MLX5_FLOW_ACTION_OF_POP_VLAN (1ull << 7)
+#define MLX5_FLOW_ACTION_OF_PUSH_VLAN (1ull << 8)
+#define MLX5_FLOW_ACTION_OF_SET_VLAN_VID (1ull << 9)
+#define MLX5_FLOW_ACTION_OF_SET_VLAN_PCP (1ull << 10)
+#define MLX5_FLOW_ACTION_SET_IPV4_SRC (1ull << 11)
+#define MLX5_FLOW_ACTION_SET_IPV4_DST (1ull << 12)
+#define MLX5_FLOW_ACTION_SET_IPV6_SRC (1ull << 13)
+#define MLX5_FLOW_ACTION_SET_IPV6_DST (1ull << 14)
+#define MLX5_FLOW_ACTION_SET_TP_SRC (1ull << 15)
+#define MLX5_FLOW_ACTION_SET_TP_DST (1ull << 16)
+#define MLX5_FLOW_ACTION_JUMP (1ull << 17)
+#define MLX5_FLOW_ACTION_SET_TTL (1ull << 18)
+#define MLX5_FLOW_ACTION_DEC_TTL (1ull << 19)
+#define MLX5_FLOW_ACTION_SET_MAC_SRC (1ull << 20)
+#define MLX5_FLOW_ACTION_SET_MAC_DST (1ull << 21)
+#define MLX5_FLOW_ACTION_ENCAP (1ull << 22)
+#define MLX5_FLOW_ACTION_DECAP (1ull << 23)
+#define MLX5_FLOW_ACTION_INC_TCP_SEQ (1ull << 24)
+#define MLX5_FLOW_ACTION_DEC_TCP_SEQ (1ull << 25)
+#define MLX5_FLOW_ACTION_INC_TCP_ACK (1ull << 26)
+#define MLX5_FLOW_ACTION_DEC_TCP_ACK (1ull << 27)
#define MLX5_FLOW_ACTION_SET_TAG (1ull << 28)
#define MLX5_FLOW_ACTION_MARK_EXT (1ull << 29)
#define MLX5_FLOW_ACTION_SET_META (1ull << 30)
--
2.20.0
^ permalink raw reply [flat|nested] 3+ messages in thread
* RE: [PATCH v1] net/mlx5: fix action flag data type
2022-11-01 10:07 [PATCH v1] net/mlx5: fix action flag data type Shun Hao
@ 2022-11-03 11:41 ` Raslan Darawsheh
2022-11-14 11:00 ` Thomas Monjalon
1 sibling, 0 replies; 3+ messages in thread
From: Raslan Darawsheh @ 2022-11-03 11:41 UTC (permalink / raw)
To: Shun Hao, Slava Ovsiienko, Matan Azrad, Ori Kam, Bing Zhao; +Cc: dev, stable
Hi,
> -----Original Message-----
> From: Shun Hao <shunh@nvidia.com>
> Sent: Tuesday, November 1, 2022 12:07 PM
> To: Slava Ovsiienko <viacheslavo@nvidia.com>; Matan Azrad
> <matan@nvidia.com>; Ori Kam <orika@nvidia.com>; Bing Zhao
> <bingz@nvidia.com>
> Cc: dev@dpdk.org; Raslan Darawsheh <rasland@nvidia.com>;
> stable@dpdk.org
> Subject: [PATCH v1] net/mlx5: fix action flag data type
>
> MLX5_FLOW_ACTION flags are used as uint64_t now, but some old flags
> are not defined as 64 bits. So if they are type casted to uint64 after
> bitwise operations, the high 32-bit data might be incorrect.
>
> E.g. Currently MLX5_FLOW_ACTION_DROP is defined as 0x1u, when it is
> used
> like:
> (action_flags & ~MLX5_FLOW_ACTION_DROP)
> action_flags is uint64_t so (~MLX5_FLOW_ACTION_DROP) will be casted to
> uint64_t as well, but its high 32 bits will be all 0s. This will make the
> result not as expected.
>
> This patch fixes this by making all action flags definition as 64-bit
> data type.
>
> Fixes: 4b7bf3ff ("net/mlx5: support yellow in meter policy validation")
> Cc: stable@dpdk.org
Patch applied to next-net-mlx,
Kindest regards,
Raslan Darawsheh
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v1] net/mlx5: fix action flag data type
2022-11-01 10:07 [PATCH v1] net/mlx5: fix action flag data type Shun Hao
2022-11-03 11:41 ` Raslan Darawsheh
@ 2022-11-14 11:00 ` Thomas Monjalon
1 sibling, 0 replies; 3+ messages in thread
From: Thomas Monjalon @ 2022-11-14 11:00 UTC (permalink / raw)
To: Shun Hao; +Cc: viacheslavo, matan, orika, Bing Zhao, dev, rasland
01/11/2022 11:07, Shun Hao:
> This patch fixes this by making all action flags definition as 64-bit
> data type.
No, it is defining as unsigned long long.
[...]
> /* Actions */
> -#define MLX5_FLOW_ACTION_DROP (1u << 0)
> -#define MLX5_FLOW_ACTION_QUEUE (1u << 1)
> -#define MLX5_FLOW_ACTION_RSS (1u << 2)
> -#define MLX5_FLOW_ACTION_FLAG (1u << 3)
> -#define MLX5_FLOW_ACTION_MARK (1u << 4)
> -#define MLX5_FLOW_ACTION_COUNT (1u << 5)
> -#define MLX5_FLOW_ACTION_PORT_ID (1u << 6)
> -#define MLX5_FLOW_ACTION_OF_POP_VLAN (1u << 7)
> -#define MLX5_FLOW_ACTION_OF_PUSH_VLAN (1u << 8)
> -#define MLX5_FLOW_ACTION_OF_SET_VLAN_VID (1u << 9)
> -#define MLX5_FLOW_ACTION_OF_SET_VLAN_PCP (1u << 10)
> -#define MLX5_FLOW_ACTION_SET_IPV4_SRC (1u << 11)
> -#define MLX5_FLOW_ACTION_SET_IPV4_DST (1u << 12)
> -#define MLX5_FLOW_ACTION_SET_IPV6_SRC (1u << 13)
> -#define MLX5_FLOW_ACTION_SET_IPV6_DST (1u << 14)
> -#define MLX5_FLOW_ACTION_SET_TP_SRC (1u << 15)
> -#define MLX5_FLOW_ACTION_SET_TP_DST (1u << 16)
> -#define MLX5_FLOW_ACTION_JUMP (1u << 17)
> -#define MLX5_FLOW_ACTION_SET_TTL (1u << 18)
> -#define MLX5_FLOW_ACTION_DEC_TTL (1u << 19)
> -#define MLX5_FLOW_ACTION_SET_MAC_SRC (1u << 20)
> -#define MLX5_FLOW_ACTION_SET_MAC_DST (1u << 21)
> -#define MLX5_FLOW_ACTION_ENCAP (1u << 22)
> -#define MLX5_FLOW_ACTION_DECAP (1u << 23)
> -#define MLX5_FLOW_ACTION_INC_TCP_SEQ (1u << 24)
> -#define MLX5_FLOW_ACTION_DEC_TCP_SEQ (1u << 25)
> -#define MLX5_FLOW_ACTION_INC_TCP_ACK (1u << 26)
> -#define MLX5_FLOW_ACTION_DEC_TCP_ACK (1u << 27)
> +#define MLX5_FLOW_ACTION_DROP (1ull << 0)
> +#define MLX5_FLOW_ACTION_QUEUE (1ull << 1)
> +#define MLX5_FLOW_ACTION_RSS (1ull << 2)
> +#define MLX5_FLOW_ACTION_FLAG (1ull << 3)
> +#define MLX5_FLOW_ACTION_MARK (1ull << 4)
> +#define MLX5_FLOW_ACTION_COUNT (1ull << 5)
> +#define MLX5_FLOW_ACTION_PORT_ID (1ull << 6)
> +#define MLX5_FLOW_ACTION_OF_POP_VLAN (1ull << 7)
> +#define MLX5_FLOW_ACTION_OF_PUSH_VLAN (1ull << 8)
> +#define MLX5_FLOW_ACTION_OF_SET_VLAN_VID (1ull << 9)
> +#define MLX5_FLOW_ACTION_OF_SET_VLAN_PCP (1ull << 10)
> +#define MLX5_FLOW_ACTION_SET_IPV4_SRC (1ull << 11)
> +#define MLX5_FLOW_ACTION_SET_IPV4_DST (1ull << 12)
> +#define MLX5_FLOW_ACTION_SET_IPV6_SRC (1ull << 13)
> +#define MLX5_FLOW_ACTION_SET_IPV6_DST (1ull << 14)
> +#define MLX5_FLOW_ACTION_SET_TP_SRC (1ull << 15)
> +#define MLX5_FLOW_ACTION_SET_TP_DST (1ull << 16)
> +#define MLX5_FLOW_ACTION_JUMP (1ull << 17)
> +#define MLX5_FLOW_ACTION_SET_TTL (1ull << 18)
> +#define MLX5_FLOW_ACTION_DEC_TTL (1ull << 19)
> +#define MLX5_FLOW_ACTION_SET_MAC_SRC (1ull << 20)
> +#define MLX5_FLOW_ACTION_SET_MAC_DST (1ull << 21)
> +#define MLX5_FLOW_ACTION_ENCAP (1ull << 22)
> +#define MLX5_FLOW_ACTION_DECAP (1ull << 23)
> +#define MLX5_FLOW_ACTION_INC_TCP_SEQ (1ull << 24)
> +#define MLX5_FLOW_ACTION_DEC_TCP_SEQ (1ull << 25)
> +#define MLX5_FLOW_ACTION_INC_TCP_ACK (1ull << 26)
> +#define MLX5_FLOW_ACTION_DEC_TCP_ACK (1ull << 27)
> #define MLX5_FLOW_ACTION_SET_TAG (1ull << 28)
> #define MLX5_FLOW_ACTION_MARK_EXT (1ull << 29)
> #define MLX5_FLOW_ACTION_SET_META (1ull << 30)
The correct fix would be to use the macro RTE_BIT64.
Please can you convert the whole mlx5 to avoid using such unclear bit shifts?
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-11-14 11:01 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-01 10:07 [PATCH v1] net/mlx5: fix action flag data type Shun Hao
2022-11-03 11:41 ` Raslan Darawsheh
2022-11-14 11:00 ` Thomas Monjalon
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).