[PATCH net v3 0/2] net/mlx5: HWS, fix modify header pattern compare

[PATCH net v3 0/2] net/mlx5: HWS, fix modify header pattern compare

[Alexandre Cassen]

The fs_hws modify header pools and the HWS pattern cache decide whether
two modify headers can share a pattern. Two bugs let different patterns
compare equal:

- On big-endian the HWS core comparison checks the data dword of each
  action instead of its control dword.
- The fs_hws pool match ignores the destination of copy and add_field
  actions.

Patch 2 moves the pool match to the HWS core comparison that patch 1
fixes.

v3:
- Declare mlx5hws_pat_compare_pattern() in pat_arg.h instead of the HWS
  API header (Yevgeny).
- Name the test hardware in the commit messages.
- Add Simon's and Yevgeny's Reviewed-by to patch 1.
- Drop Simon's Reviewed-by from patch 2 since its code changed.

v2:
- Add patch 1 for the big-endian compare (Sashiko).
- Reword patch 2 to stay true on big-endian.

v2: https://lore.kernel.org/netdev/20261012101937.1335231-1-acassen@corp.free.fr/
v1: https://lore.kernel.org/netdev/20261010120147.135825-1-acassen@corp.free.fr/

Alexandre Cassen (2):
  net/mlx5: HWS, fix modify header pattern compare on big-endian
  net/mlx5: HWS, fix modify header pool match for copy actions

 .../mellanox/mlx5/core/steering/hws/fs_hws_pools.c   | 11 ++++-------
 .../mellanox/mlx5/core/steering/hws/pat_arg.c        | 12 ++++++------
 .../mellanox/mlx5/core/steering/hws/pat_arg.h        |  5 +++++
 3 files changed, 15 insertions(+), 13 deletions(-)

-- 
2.43.0

[PATCH net v3 1/2] net/mlx5: HWS, fix modify header pattern compare on big-endian

[Alexandre Cassen]

mlx5hws_pat_compare_pattern() casts each __be64 action to __be32 to
compare only its control dword. On big-endian this keeps the data dword
instead, which HWS clears from the pattern of set and add actions. Two
such actions writing the same value into different fields then share a
pattern and the second rule modifies the wrong field.

Read the first dword of each action from memory instead. Little-endian
behavior is unchanged.

Discovered by an AI code review agent [1] and confirmed by code
inspection. Not reproduced for lack of big-endian hardware running HWS.
Tested for regressions on ConnectX-7 with firmware 28.48.1000 on a
little-endian host.

Fixes: aefc15a0fa1c ("net/mlx5: HWS, added modify header pattern and args handling")
Link: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261010120147.135825-1-acassen%40corp.free.fr [1]
Reviewed-by: Simon Horman <horms@kernel.org>
Reviewed-by: Yevgeny Kliteynik <kliteyn@nvidia.com>
Signed-off-by: Alexandre Cassen <acassen@corp.free.fr>
---
 .../net/ethernet/mellanox/mlx5/core/steering/hws/pat_arg.c    | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/pat_arg.c b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/pat_arg.c
index 9af449c2b4d5..512299b59fd0 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/pat_arg.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/pat_arg.c
@@ -116,8 +116,8 @@ static bool mlx5hws_pat_compare_pattern(int cur_num_of_actions,
 				return false;
 		} else {
 			/* Compare just the control, not the values */
-			if ((__force __be32)actions[i] !=
-			    (__force __be32)cur_actions[i])
+			if (*(__be32 *)&actions[i] !=
+			    *(__be32 *)&cur_actions[i])
 				return false;
 		}
 	}
-- 
2.43.0

[PATCH net v3 2/2] net/mlx5: HWS, fix modify header pool match for copy actions

[Alexandre Cassen]

mlx5_fs_hws_mh_pool_match() compares a single dword of each action
since rules pass their values as arguments. Copy and add_field actions
keep their destination in the second dword and HWS reads them from the
pattern alone. Two modify headers whose copies differ only in
destination then share a pool and the second rule writes to the wrong
field.

Use the HWS core comparison that checks these actions in full.

Found while checking a Sashiko AI review report and confirmed against
the HWS pattern code. Not reproduced. No in-kernel user puts such
actions on the FDB but RDMA flow actions can. Tested for regressions on
ConnectX-7 with firmware 28.48.1000 on a little-endian host.

Fixes: b36315ca69cb ("net/mlx5: fs, add HWS modify header API function")
Signed-off-by: Alexandre Cassen <acassen@corp.free.fr>
---
 .../mellanox/mlx5/core/steering/hws/fs_hws_pools.c    | 11 ++++-------
 .../mellanox/mlx5/core/steering/hws/pat_arg.c         |  8 ++++----
 .../mellanox/mlx5/core/steering/hws/pat_arg.h         |  5 +++++
 3 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/fs_hws_pools.c b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/fs_hws_pools.c
index 86828e329187..05aa143fa088 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/fs_hws_pools.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/fs_hws_pools.c
@@ -3,6 +3,7 @@
 
 #include <mlx5_core.h>
 #include "fs_hws_pools.h"
+#include "internal.h"
 
 #define MLX5_FS_HWS_DEFAULT_BULK_LEN 65536
 #define MLX5_FS_HWS_POOL_MAX_THRESHOLD BIT(18)
@@ -389,7 +390,7 @@ bool mlx5_fs_hws_mh_pool_match(struct mlx5_fs_pool *mh_pool,
 			       struct mlx5hws_action_mh_pattern *pattern)
 {
 	struct mlx5hws_action_mh_pattern *pool_pattern;
-	int num_actions, i;
+	int num_actions;
 
 	pool_pattern = mh_pool->pool_ctx;
 	if (WARN_ON_ONCE(!pool_pattern))
@@ -398,12 +399,8 @@ bool mlx5_fs_hws_mh_pool_match(struct mlx5_fs_pool *mh_pool,
 	if (pattern->sz != pool_pattern->sz)
 		return false;
 	num_actions = pattern->sz / MLX5_UN_SZ_BYTES(set_add_copy_action_in_auto);
-	for (i = 0; i < num_actions; i++) {
-		if ((__force __be32)pattern->data[i] !=
-		    (__force __be32)pool_pattern->data[i])
-			return false;
-	}
-	return true;
+	return mlx5hws_pat_compare_pattern(num_actions, pool_pattern->data,
+					   num_actions, pattern->data);
 }
 
 struct mlx5hws_action *mlx5_fc_get_hws_action(struct mlx5hws_context *ctx,
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/pat_arg.c b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/pat_arg.c
index 512299b59fd0..a49115f04025 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/pat_arg.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/pat_arg.c
@@ -96,10 +96,10 @@ void mlx5hws_pat_uninit_pattern_cache(struct mlx5hws_pattern_cache *cache)
 	kfree(cache);
 }
 
-static bool mlx5hws_pat_compare_pattern(int cur_num_of_actions,
-					__be64 cur_actions[],
-					int num_of_actions,
-					__be64 actions[])
+bool mlx5hws_pat_compare_pattern(int cur_num_of_actions,
+				 __be64 cur_actions[],
+				 int num_of_actions,
+				 __be64 actions[])
 {
 	int i;
 
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/pat_arg.h b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/pat_arg.h
index 7fbd8dc7aa18..2be7787104e1 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/pat_arg.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/pat_arg.h
@@ -49,6 +49,11 @@ int mlx5hws_pat_init_pattern_cache(struct mlx5hws_pattern_cache **cache);
 
 void mlx5hws_pat_uninit_pattern_cache(struct mlx5hws_pattern_cache *cache);
 
+bool mlx5hws_pat_compare_pattern(int cur_num_of_actions,
+				 __be64 cur_actions[],
+				 int num_of_actions,
+				 __be64 actions[]);
+
 bool mlx5hws_pat_verify_actions(struct mlx5hws_context *ctx, __be64 pattern[], size_t sz);
 
 int mlx5hws_arg_create(struct mlx5hws_context *ctx,
-- 
2.43.0

Re: [PATCH net v3 1/2] net/mlx5: HWS, fix modify header pattern compare on big-endian

[sashiko-bot]

> mlx5hws_pat_compare_pattern() casts each __be64 action to __be32 to
> compare only its control dword. On big-endian this keeps the data dword
> instead, which HWS clears from the pattern of set and add actions. Two
> such actions writing the same value into different fields then share a
> pattern and the second rule modifies the wrong field.
> 
> Read the first dword of each action from memory instead. Little-endian
> behavior is unchanged.
> 
> Discovered by an AI code review agent [1] and confirmed by code
> inspection. Not reproduced for lack of big-endian hardware running HWS.
> Tested for regressions on ConnectX-7 with firmware 28.48.1000 on a
> little-endian host.
> 
> Fixes: aefc15a0fa1c ("net/mlx5: HWS, added modify header pattern and args handling")
> Link: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261010120147.135825-1-acassen%40corp.free.fr [1]
> Reviewed-by: Simon Horman <horms@kernel.org>
> Reviewed-by: Yevgeny Kliteynik <kliteyn@nvidia.com>
> Signed-off-by: Alexandre Cassen <acassen@corp.free.fr>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261019133159.1742411-1-acassen@corp.free.fr?part=1

Re: [PATCH net v3 2/2] net/mlx5: HWS, fix modify header pool match for copy actions

[sashiko-bot]

> mlx5_fs_hws_mh_pool_match() compares a single dword of each action
> since rules pass their values as arguments. Copy and add_field actions
> keep their destination in the second dword and HWS reads them from the
> pattern alone. Two modify headers whose copies differ only in
> destination then share a pool and the second rule writes to the wrong
> field.
> 
> Use the HWS core comparison that checks these actions in full.
> 
> Found while checking a Sashiko AI review report and confirmed against
> the HWS pattern code. Not reproduced. No in-kernel user puts such
> actions on the FDB but RDMA flow actions can. Tested for regressions on
> ConnectX-7 with firmware 28.48.1000 on a little-endian host.
> 
> Fixes: b36315ca69cb ("net/mlx5: fs, add HWS modify header API function")
> Signed-off-by: Alexandre Cassen <acassen@corp.free.fr>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261019133159.1742411-1-acassen@corp.free.fr?part=2

Re: [PATCH net v3 1/2] net/mlx5: HWS, fix modify header pattern compare on big-endian

[Yevgeny Kliteynik]

On 19-Oct-26 16:31, Alexandre Cassen wrote:
> mlx5hws_pat_compare_pattern() casts each __be64 action to __be32 to
> compare only its control dword. On big-endian this keeps the data dword
> instead, which HWS clears from the pattern of set and add actions. Two
> such actions writing the same value into different fields then share a
> pattern and the second rule modifies the wrong field.
> 
> Read the first dword of each action from memory instead. Little-endian
> behavior is unchanged.
> 
> Discovered by an AI code review agent [1] and confirmed by code
> inspection. Not reproduced for lack of big-endian hardware running HWS.
> Tested for regressions on ConnectX-7 with firmware 28.48.1000 on a
> little-endian host.
> 
> Fixes: aefc15a0fa1c ("net/mlx5: HWS, added modify header pattern and args handling")
> Link: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261010120147.135825-1-acassen%40corp.free.fr [1]
> Reviewed-by: Simon Horman <horms@kernel.org>
> Reviewed-by: Yevgeny Kliteynik <kliteyn@nvidia.com>
> Signed-off-by: Alexandre Cassen <acassen@corp.free.fr>
> ---
>   .../net/ethernet/mellanox/mlx5/core/steering/hws/pat_arg.c    | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/pat_arg.c b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/pat_arg.c
> index 9af449c2b4d5..512299b59fd0 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/pat_arg.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/pat_arg.c
> @@ -116,8 +116,8 @@ static bool mlx5hws_pat_compare_pattern(int cur_num_of_actions,
>   				return false;
>   		} else {
>   			/* Compare just the control, not the values */
> -			if ((__force __be32)actions[i] !=
> -			    (__force __be32)cur_actions[i])
> +			if (*(__be32 *)&actions[i] !=
> +			    *(__be32 *)&cur_actions[i])
>   				return false;
>   		}
>   	}

Reviewed-by: Yevgeny Kliteynik <kliteyn@nvidia.com>

Re: [PATCH net v3 2/2] net/mlx5: HWS, fix modify header pool match for copy actions

[Yevgeny Kliteynik]

On 19-Oct-26 16:31, Alexandre Cassen wrote:
> mlx5_fs_hws_mh_pool_match() compares a single dword of each action
> since rules pass their values as arguments. Copy and add_field actions
> keep their destination in the second dword and HWS reads them from the
> pattern alone. Two modify headers whose copies differ only in
> destination then share a pool and the second rule writes to the wrong
> field.
> 
> Use the HWS core comparison that checks these actions in full.
> 
> Found while checking a Sashiko AI review report and confirmed against
> the HWS pattern code. Not reproduced. No in-kernel user puts such
> actions on the FDB but RDMA flow actions can. Tested for regressions on
> ConnectX-7 with firmware 28.48.1000 on a little-endian host.
> 
> Fixes: b36315ca69cb ("net/mlx5: fs, add HWS modify header API function")
> Signed-off-by: Alexandre Cassen <acassen@corp.free.fr>
> ---
>   .../mellanox/mlx5/core/steering/hws/fs_hws_pools.c    | 11 ++++-------
>   .../mellanox/mlx5/core/steering/hws/pat_arg.c         |  8 ++++----
>   .../mellanox/mlx5/core/steering/hws/pat_arg.h         |  5 +++++
>   3 files changed, 13 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/fs_hws_pools.c b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/fs_hws_pools.c
> index 86828e329187..05aa143fa088 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/fs_hws_pools.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/fs_hws_pools.c
> @@ -3,6 +3,7 @@
>   
>   #include <mlx5_core.h>
>   #include "fs_hws_pools.h"
> +#include "internal.h"
>   
>   #define MLX5_FS_HWS_DEFAULT_BULK_LEN 65536
>   #define MLX5_FS_HWS_POOL_MAX_THRESHOLD BIT(18)
> @@ -389,7 +390,7 @@ bool mlx5_fs_hws_mh_pool_match(struct mlx5_fs_pool *mh_pool,
>   			       struct mlx5hws_action_mh_pattern *pattern)
>   {
>   	struct mlx5hws_action_mh_pattern *pool_pattern;
> -	int num_actions, i;
> +	int num_actions;
>   
>   	pool_pattern = mh_pool->pool_ctx;
>   	if (WARN_ON_ONCE(!pool_pattern))
> @@ -398,12 +399,8 @@ bool mlx5_fs_hws_mh_pool_match(struct mlx5_fs_pool *mh_pool,
>   	if (pattern->sz != pool_pattern->sz)
>   		return false;
>   	num_actions = pattern->sz / MLX5_UN_SZ_BYTES(set_add_copy_action_in_auto);
> -	for (i = 0; i < num_actions; i++) {
> -		if ((__force __be32)pattern->data[i] !=
> -		    (__force __be32)pool_pattern->data[i])
> -			return false;
> -	}
> -	return true;
> +	return mlx5hws_pat_compare_pattern(num_actions, pool_pattern->data,
> +					   num_actions, pattern->data);
>   }
>   
>   struct mlx5hws_action *mlx5_fc_get_hws_action(struct mlx5hws_context *ctx,
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/pat_arg.c b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/pat_arg.c
> index 512299b59fd0..a49115f04025 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/pat_arg.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/pat_arg.c
> @@ -96,10 +96,10 @@ void mlx5hws_pat_uninit_pattern_cache(struct mlx5hws_pattern_cache *cache)
>   	kfree(cache);
>   }
>   
> -static bool mlx5hws_pat_compare_pattern(int cur_num_of_actions,
> -					__be64 cur_actions[],
> -					int num_of_actions,
> -					__be64 actions[])
> +bool mlx5hws_pat_compare_pattern(int cur_num_of_actions,
> +				 __be64 cur_actions[],
> +				 int num_of_actions,
> +				 __be64 actions[])
>   {
>   	int i;
>   
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/pat_arg.h b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/pat_arg.h
> index 7fbd8dc7aa18..2be7787104e1 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/pat_arg.h
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/pat_arg.h
> @@ -49,6 +49,11 @@ int mlx5hws_pat_init_pattern_cache(struct mlx5hws_pattern_cache **cache);
>   
>   void mlx5hws_pat_uninit_pattern_cache(struct mlx5hws_pattern_cache *cache);
>   
> +bool mlx5hws_pat_compare_pattern(int cur_num_of_actions,
> +				 __be64 cur_actions[],
> +				 int num_of_actions,
> +				 __be64 actions[]);
> +
>   bool mlx5hws_pat_verify_actions(struct mlx5hws_context *ctx, __be64 pattern[], size_t sz);
>   
>   int mlx5hws_arg_create(struct mlx5hws_context *ctx,

Reviewed-by: Yevgeny Kliteynik <kliteyn@nvidia.com>