[PATCH net 0/2] mlxsw: spectrum_acl_bloom_filter: Fix compilation warning on s390x

[PATCH net 0/2] mlxsw: spectrum_acl_bloom_filter: Fix compilation warning on s390x

[WangYuli]

Solely when using clang to compile the mlxsw driver for the s390
architecture does this rather perplexing __write_overflow_field warning
manifest.

The seemingly underlying cause points to a possible anomaly within the
LLVM toolchain targeting this architecture.

Nevertheless, the kernel, being a software endeavor predicated on
robustness, should not normally countenance build failures stemming from
toolchain and compilation setups that are implicitly project-approved.

Implementing the workaround, as elucidated in PATCH 1/2, remediates the
issue.

WangYuli (2):
  mlxsw: spectrum_acl_bloom_filter: Expand
    chunk_key_offsets[chunk_index]
  mlxsw: spectrum_acl_bloom_filter: Type block_count to u32

 .../mlxsw/spectrum_acl_bloom_filter.c         | 42 ++++++++++++-------
 1 file changed, 27 insertions(+), 15 deletions(-)

-- 
2.47.2

[PATCH net 1/2] mlxsw: spectrum_acl_bloom_filter: Expand chunk_key_offsets[chunk_index]

[WangYuli]

This is a workaround to mitigate a compiler anomaly.

During LLVM toolchain compilation of this driver on s390x architecture, an
unreasonable __write_overflow_field warning occurs.

Contextually, chunk_index is restricted to 0, 1 or 2. By expanding these
possibilities, the compile warning is suppressed.

Fix follow error with clang-19 when -Werror:
  In file included from drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c:5:
  In file included from ./include/linux/gfp.h:7:
  In file included from ./include/linux/mmzone.h:8:
  In file included from ./include/linux/spinlock.h:63:
  In file included from ./include/linux/lockdep.h:14:
  In file included from ./include/linux/smp.h:13:
  In file included from ./include/linux/cpumask.h:12:
  In file included from ./include/linux/bitmap.h:13:
  In file included from ./include/linux/string.h:392:
  ./include/linux/fortify-string.h:571:4: error: call to '__write_overflow_field' declared with 'warning' attribute: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Werror,-Wattribute-warning]
    571 |                         __write_overflow_field(p_size_field, size);
        |                         ^
  1 error generated.

Co-developed-by: Zijian Chen <czj2441@163.com>
Signed-off-by: Zijian Chen <czj2441@163.com>
Co-developed-by: Wentao Guan <guanwentao@uniontech.com>
Signed-off-by: Wentao Guan <guanwentao@uniontech.com>
Signed-off-by: WangYuli <wangyuli@uniontech.com>
---
 .../mlxsw/spectrum_acl_bloom_filter.c         | 39 ++++++++++++-------
 1 file changed, 25 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c
index a54eedb69a3f..96105bab680b 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c
@@ -203,17 +203,6 @@ static const u8 mlxsw_sp4_acl_bf_crc6_tab[256] = {
 0x2f, 0x02, 0x18, 0x35, 0x2c, 0x01, 0x1b, 0x36,
 };
 
-/* Each chunk contains 4 key blocks. Chunk 2 uses key blocks 11-8,
- * and we need to populate it with 4 key blocks copied from the entry encoded
- * key. The original keys layout is same for Spectrum-{2,3,4}.
- * Since the encoded key contains a 2 bytes padding, key block 11 starts at
- * offset 2. block 7 that is used in chunk 1 starts at offset 20 as 4 key blocks
- * take 18 bytes. See 'MLXSW_SP2_AFK_BLOCK_LAYOUT' for more details.
- * This array defines key offsets for easy access when copying key blocks from
- * entry key to Bloom filter chunk.
- */
-static const u8 chunk_key_offsets[MLXSW_BLOOM_KEY_CHUNKS] = {2, 20, 38};
-
 static u16 mlxsw_sp2_acl_bf_crc16_byte(u16 crc, u8 c)
 {
 	return (crc << 8) ^ mlxsw_sp2_acl_bf_crc16_tab[(crc >> 8) ^ c];
@@ -237,6 +226,7 @@ __mlxsw_sp_acl_bf_key_encode(struct mlxsw_sp_acl_atcam_region *aregion,
 	struct mlxsw_afk_key_info *key_info = aregion->region->key_info;
 	u8 chunk_index, chunk_count, block_count;
 	char *chunk = output;
+	char *enc_key_src_ptr;
 	__be16 erp_region_id;
 
 	block_count = mlxsw_afk_key_info_blocks_count_get(key_info);
@@ -248,9 +238,30 @@ __mlxsw_sp_acl_bf_key_encode(struct mlxsw_sp_acl_atcam_region *aregion,
 		memset(chunk, 0, pad_bytes);
 		memcpy(chunk + pad_bytes, &erp_region_id,
 		       sizeof(erp_region_id));
-		memcpy(chunk + key_offset,
-		       &aentry->ht_key.enc_key[chunk_key_offsets[chunk_index]],
-		       chunk_key_len);
+/* Each chunk contains 4 key blocks. Chunk 2 uses key blocks 11-8,
+ * and we need to populate it with 4 key blocks copied from the entry encoded
+ * key. The original keys layout is same for Spectrum-{2,3,4}.
+ * Since the encoded key contains a 2 bytes padding, key block 11 starts at
+ * offset 2. block 7 that is used in chunk 1 starts at offset 20 as 4 key blocks
+ * take 18 bytes. See 'MLXSW_SP2_AFK_BLOCK_LAYOUT' for more details.
+ * This array defines key offsets for easy access when copying key blocks from
+ * entry key to Bloom filter chunk.
+ *
+ * Define the acceptable values for chunk_index to prevent LLVM from failing
+ * compilation in certain circumstances.
+ */
+		switch (chunk_index) {
+		case 0:
+			enc_key_src_ptr = &aentry->ht_key.enc_key[2];
+			break;
+		case 1:
+			enc_key_src_ptr = &aentry->ht_key.enc_key[20];
+			break;
+		case 2:
+			enc_key_src_ptr = &aentry->ht_key.enc_key[38];
+			break;
+		}
+		memcpy(chunk + key_offset, enc_key_src_ptr, chunk_key_len);
 		chunk += chunk_len;
 	}
 	*len = chunk_count * chunk_len;
-- 
2.47.2

Re: [PATCH net 1/2] mlxsw: spectrum_acl_bloom_filter: Expand chunk_key_offsets[chunk_index]

[Ido Schimmel]

On Tue, Mar 11, 2025 at 10:17:00PM +0800, WangYuli wrote:
> This is a workaround to mitigate a compiler anomaly.
> 
> During LLVM toolchain compilation of this driver on s390x architecture, an
> unreasonable __write_overflow_field warning occurs.
> 
> Contextually, chunk_index is restricted to 0, 1 or 2. By expanding these
> possibilities, the compile warning is suppressed.

I'm not sure why the fix suppresses the warning when the warning is
about the destination buffer and the fix is about the source. Can you
check if the below helps? It removes the parameterization from
__mlxsw_sp_acl_bf_key_encode() and instead splits it to two variants.

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c
index a54eedb69a3f..3e1e4be72da2 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c
@@ -110,7 +110,6 @@ static const u16 mlxsw_sp2_acl_bf_crc16_tab[256] = {
  * +-----------+----------+-----------------------------------+
  */
 
-#define MLXSW_SP4_BLOOM_CHUNK_PAD_BYTES 0
 #define MLXSW_SP4_BLOOM_CHUNK_KEY_BYTES 18
 #define MLXSW_SP4_BLOOM_KEY_CHUNK_BYTES 20
 
@@ -229,10 +228,9 @@ static u16 mlxsw_sp2_acl_bf_crc(const u8 *buffer, size_t len)
 }
 
 static void
-__mlxsw_sp_acl_bf_key_encode(struct mlxsw_sp_acl_atcam_region *aregion,
-			     struct mlxsw_sp_acl_atcam_entry *aentry,
-			     char *output, u8 *len, u8 max_chunks, u8 pad_bytes,
-			     u8 key_offset, u8 chunk_key_len, u8 chunk_len)
+mlxsw_sp2_acl_bf_key_encode(struct mlxsw_sp_acl_atcam_region *aregion,
+			    struct mlxsw_sp_acl_atcam_entry *aentry,
+			    char *output, u8 *len)
 {
 	struct mlxsw_afk_key_info *key_info = aregion->region->key_info;
 	u8 chunk_index, chunk_count, block_count;
@@ -243,30 +241,17 @@ __mlxsw_sp_acl_bf_key_encode(struct mlxsw_sp_acl_atcam_region *aregion,
 	chunk_count = 1 + ((block_count - 1) >> 2);
 	erp_region_id = cpu_to_be16(aentry->ht_key.erp_id |
 				   (aregion->region->id << 4));
-	for (chunk_index = max_chunks - chunk_count; chunk_index < max_chunks;
-	     chunk_index++) {
-		memset(chunk, 0, pad_bytes);
-		memcpy(chunk + pad_bytes, &erp_region_id,
+	for (chunk_index = MLXSW_BLOOM_KEY_CHUNKS - chunk_count;
+	     chunk_index < MLXSW_BLOOM_KEY_CHUNKS; chunk_index++) {
+		memset(chunk, 0, MLXSW_SP2_BLOOM_CHUNK_PAD_BYTES);
+		memcpy(chunk + MLXSW_SP2_BLOOM_CHUNK_PAD_BYTES, &erp_region_id,
 		       sizeof(erp_region_id));
-		memcpy(chunk + key_offset,
+		memcpy(chunk + MLXSW_SP2_BLOOM_CHUNK_KEY_OFFSET,
 		       &aentry->ht_key.enc_key[chunk_key_offsets[chunk_index]],
-		       chunk_key_len);
-		chunk += chunk_len;
+		       MLXSW_SP2_BLOOM_CHUNK_KEY_BYTES);
+		chunk += MLXSW_SP2_BLOOM_KEY_CHUNK_BYTES;
 	}
-	*len = chunk_count * chunk_len;
-}
-
-static void
-mlxsw_sp2_acl_bf_key_encode(struct mlxsw_sp_acl_atcam_region *aregion,
-			    struct mlxsw_sp_acl_atcam_entry *aentry,
-			    char *output, u8 *len)
-{
-	__mlxsw_sp_acl_bf_key_encode(aregion, aentry, output, len,
-				     MLXSW_BLOOM_KEY_CHUNKS,
-				     MLXSW_SP2_BLOOM_CHUNK_PAD_BYTES,
-				     MLXSW_SP2_BLOOM_CHUNK_KEY_OFFSET,
-				     MLXSW_SP2_BLOOM_CHUNK_KEY_BYTES,
-				     MLXSW_SP2_BLOOM_KEY_CHUNK_BYTES);
+	*len = chunk_count * MLXSW_SP2_BLOOM_KEY_CHUNK_BYTES;
 }
 
 static unsigned int
@@ -375,15 +360,24 @@ mlxsw_sp4_acl_bf_key_encode(struct mlxsw_sp_acl_atcam_region *aregion,
 			    char *output, u8 *len)
 {
 	struct mlxsw_afk_key_info *key_info = aregion->region->key_info;
-	u8 block_count = mlxsw_afk_key_info_blocks_count_get(key_info);
-	u8 chunk_count = 1 + ((block_count - 1) >> 2);
-
-	__mlxsw_sp_acl_bf_key_encode(aregion, aentry, output, len,
-				     MLXSW_BLOOM_KEY_CHUNKS,
-				     MLXSW_SP4_BLOOM_CHUNK_PAD_BYTES,
-				     MLXSW_SP4_BLOOM_CHUNK_KEY_OFFSET,
-				     MLXSW_SP4_BLOOM_CHUNK_KEY_BYTES,
-				     MLXSW_SP4_BLOOM_KEY_CHUNK_BYTES);
+	u8 chunk_index, chunk_count, block_count;
+	char *chunk = output;
+	__be16 erp_region_id;
+
+	block_count = mlxsw_afk_key_info_blocks_count_get(key_info);
+	chunk_count = 1 + ((block_count - 1) >> 2);
+	erp_region_id = cpu_to_be16(aentry->ht_key.erp_id |
+				   (aregion->region->id << 4));
+	for (chunk_index = MLXSW_BLOOM_KEY_CHUNKS - chunk_count;
+	     chunk_index < MLXSW_BLOOM_KEY_CHUNKS; chunk_index++) {
+		memcpy(chunk, &erp_region_id, sizeof(erp_region_id));
+		memcpy(chunk + MLXSW_SP4_BLOOM_CHUNK_KEY_OFFSET,
+		       &aentry->ht_key.enc_key[chunk_key_offsets[chunk_index]],
+		       MLXSW_SP4_BLOOM_CHUNK_KEY_BYTES);
+		chunk += MLXSW_SP4_BLOOM_KEY_CHUNK_BYTES;
+	}
+	*len = chunk_count * MLXSW_SP4_BLOOM_KEY_CHUNK_BYTES;
+
 	mlxsw_sp4_bf_key_shift_chunks(chunk_count, output);
 }

> 
> Fix follow error with clang-19 when -Werror:
>   In file included from drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c:5:
>   In file included from ./include/linux/gfp.h:7:
>   In file included from ./include/linux/mmzone.h:8:
>   In file included from ./include/linux/spinlock.h:63:
>   In file included from ./include/linux/lockdep.h:14:
>   In file included from ./include/linux/smp.h:13:
>   In file included from ./include/linux/cpumask.h:12:
>   In file included from ./include/linux/bitmap.h:13:
>   In file included from ./include/linux/string.h:392:
>   ./include/linux/fortify-string.h:571:4: error: call to '__write_overflow_field' declared with 'warning' attribute: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Werror,-Wattribute-warning]
>     571 |                         __write_overflow_field(p_size_field, size);
>         |                         ^
>   1 error generated.
> 
> Co-developed-by: Zijian Chen <czj2441@163.com>
> Signed-off-by: Zijian Chen <czj2441@163.com>
> Co-developed-by: Wentao Guan <guanwentao@uniontech.com>
> Signed-off-by: Wentao Guan <guanwentao@uniontech.com>
> Signed-off-by: WangYuli <wangyuli@uniontech.com>
> ---
>  .../mlxsw/spectrum_acl_bloom_filter.c         | 39 ++++++++++++-------
>  1 file changed, 25 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c
> index a54eedb69a3f..96105bab680b 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c
> @@ -203,17 +203,6 @@ static const u8 mlxsw_sp4_acl_bf_crc6_tab[256] = {
>  0x2f, 0x02, 0x18, 0x35, 0x2c, 0x01, 0x1b, 0x36,
>  };
>  
> -/* Each chunk contains 4 key blocks. Chunk 2 uses key blocks 11-8,
> - * and we need to populate it with 4 key blocks copied from the entry encoded
> - * key. The original keys layout is same for Spectrum-{2,3,4}.
> - * Since the encoded key contains a 2 bytes padding, key block 11 starts at
> - * offset 2. block 7 that is used in chunk 1 starts at offset 20 as 4 key blocks
> - * take 18 bytes. See 'MLXSW_SP2_AFK_BLOCK_LAYOUT' for more details.
> - * This array defines key offsets for easy access when copying key blocks from
> - * entry key to Bloom filter chunk.
> - */
> -static const u8 chunk_key_offsets[MLXSW_BLOOM_KEY_CHUNKS] = {2, 20, 38};
> -
>  static u16 mlxsw_sp2_acl_bf_crc16_byte(u16 crc, u8 c)
>  {
>  	return (crc << 8) ^ mlxsw_sp2_acl_bf_crc16_tab[(crc >> 8) ^ c];
> @@ -237,6 +226,7 @@ __mlxsw_sp_acl_bf_key_encode(struct mlxsw_sp_acl_atcam_region *aregion,
>  	struct mlxsw_afk_key_info *key_info = aregion->region->key_info;
>  	u8 chunk_index, chunk_count, block_count;
>  	char *chunk = output;
> +	char *enc_key_src_ptr;
>  	__be16 erp_region_id;
>  
>  	block_count = mlxsw_afk_key_info_blocks_count_get(key_info);
> @@ -248,9 +238,30 @@ __mlxsw_sp_acl_bf_key_encode(struct mlxsw_sp_acl_atcam_region *aregion,
>  		memset(chunk, 0, pad_bytes);
>  		memcpy(chunk + pad_bytes, &erp_region_id,
>  		       sizeof(erp_region_id));
> -		memcpy(chunk + key_offset,
> -		       &aentry->ht_key.enc_key[chunk_key_offsets[chunk_index]],
> -		       chunk_key_len);
> +/* Each chunk contains 4 key blocks. Chunk 2 uses key blocks 11-8,
> + * and we need to populate it with 4 key blocks copied from the entry encoded
> + * key. The original keys layout is same for Spectrum-{2,3,4}.
> + * Since the encoded key contains a 2 bytes padding, key block 11 starts at
> + * offset 2. block 7 that is used in chunk 1 starts at offset 20 as 4 key blocks
> + * take 18 bytes. See 'MLXSW_SP2_AFK_BLOCK_LAYOUT' for more details.
> + * This array defines key offsets for easy access when copying key blocks from
> + * entry key to Bloom filter chunk.
> + *
> + * Define the acceptable values for chunk_index to prevent LLVM from failing
> + * compilation in certain circumstances.
> + */
> +		switch (chunk_index) {
> +		case 0:
> +			enc_key_src_ptr = &aentry->ht_key.enc_key[2];
> +			break;
> +		case 1:
> +			enc_key_src_ptr = &aentry->ht_key.enc_key[20];
> +			break;
> +		case 2:
> +			enc_key_src_ptr = &aentry->ht_key.enc_key[38];
> +			break;
> +		}
> +		memcpy(chunk + key_offset, enc_key_src_ptr, chunk_key_len);
>  		chunk += chunk_len;
>  	}
>  	*len = chunk_count * chunk_len;
> -- 
> 2.47.2
> 

Re: [PATCH net 1/2] mlxsw: spectrum_acl_bloom_filter: Expand chunk_key_offsets[chunk_index]

[WangYuli]

[-- Attachment #1.1.1: Type: text/plain, Size: 5310 bytes --]

Hi Ido Schimme,

On 2025/3/12 21:20, Ido Schimmel wrote:
> I'm not sure why the fix suppresses the warning when the warning is
> about the destination buffer and the fix is about the source. Can you
> check if the below helps? It removes the parameterization from
> __mlxsw_sp_acl_bf_key_encode() and instead splits it to two variants.
>
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c
> index a54eedb69a3f..3e1e4be72da2 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c
> @@ -110,7 +110,6 @@ static const u16 mlxsw_sp2_acl_bf_crc16_tab[256] = {
>    * +-----------+----------+-----------------------------------+
>    */
>   
> -#define MLXSW_SP4_BLOOM_CHUNK_PAD_BYTES 0
>   #define MLXSW_SP4_BLOOM_CHUNK_KEY_BYTES 18
>   #define MLXSW_SP4_BLOOM_KEY_CHUNK_BYTES 20
>   
> @@ -229,10 +228,9 @@ static u16 mlxsw_sp2_acl_bf_crc(const u8 *buffer, size_t len)
>   }
>   
>   static void
> -__mlxsw_sp_acl_bf_key_encode(struct mlxsw_sp_acl_atcam_region *aregion,
> -			     struct mlxsw_sp_acl_atcam_entry *aentry,
> -			     char *output, u8 *len, u8 max_chunks, u8 pad_bytes,
> -			     u8 key_offset, u8 chunk_key_len, u8 chunk_len)
> +mlxsw_sp2_acl_bf_key_encode(struct mlxsw_sp_acl_atcam_region *aregion,
> +			    struct mlxsw_sp_acl_atcam_entry *aentry,
> +			    char *output, u8 *len)
>   {
>   	struct mlxsw_afk_key_info *key_info = aregion->region->key_info;
>   	u8 chunk_index, chunk_count, block_count;
> @@ -243,30 +241,17 @@ __mlxsw_sp_acl_bf_key_encode(struct mlxsw_sp_acl_atcam_region *aregion,
>   	chunk_count = 1 + ((block_count - 1) >> 2);
>   	erp_region_id = cpu_to_be16(aentry->ht_key.erp_id |
>   				   (aregion->region->id << 4));
> -	for (chunk_index = max_chunks - chunk_count; chunk_index < max_chunks;
> -	     chunk_index++) {
> -		memset(chunk, 0, pad_bytes);
> -		memcpy(chunk + pad_bytes, &erp_region_id,
> +	for (chunk_index = MLXSW_BLOOM_KEY_CHUNKS - chunk_count;
> +	     chunk_index < MLXSW_BLOOM_KEY_CHUNKS; chunk_index++) {
> +		memset(chunk, 0, MLXSW_SP2_BLOOM_CHUNK_PAD_BYTES);
> +		memcpy(chunk + MLXSW_SP2_BLOOM_CHUNK_PAD_BYTES, &erp_region_id,
>   		       sizeof(erp_region_id));
> -		memcpy(chunk + key_offset,
> +		memcpy(chunk + MLXSW_SP2_BLOOM_CHUNK_KEY_OFFSET,
>   		       &aentry->ht_key.enc_key[chunk_key_offsets[chunk_index]],
> -		       chunk_key_len);
> -		chunk += chunk_len;
> +		       MLXSW_SP2_BLOOM_CHUNK_KEY_BYTES);
> +		chunk += MLXSW_SP2_BLOOM_KEY_CHUNK_BYTES;
>   	}
> -	*len = chunk_count * chunk_len;
> -}
> -
> -static void
> -mlxsw_sp2_acl_bf_key_encode(struct mlxsw_sp_acl_atcam_region *aregion,
> -			    struct mlxsw_sp_acl_atcam_entry *aentry,
> -			    char *output, u8 *len)
> -{
> -	__mlxsw_sp_acl_bf_key_encode(aregion, aentry, output, len,
> -				     MLXSW_BLOOM_KEY_CHUNKS,
> -				     MLXSW_SP2_BLOOM_CHUNK_PAD_BYTES,
> -				     MLXSW_SP2_BLOOM_CHUNK_KEY_OFFSET,
> -				     MLXSW_SP2_BLOOM_CHUNK_KEY_BYTES,
> -				     MLXSW_SP2_BLOOM_KEY_CHUNK_BYTES);
> +	*len = chunk_count * MLXSW_SP2_BLOOM_KEY_CHUNK_BYTES;
>   }
>   
>   static unsigned int
> @@ -375,15 +360,24 @@ mlxsw_sp4_acl_bf_key_encode(struct mlxsw_sp_acl_atcam_region *aregion,
>   			    char *output, u8 *len)
>   {
>   	struct mlxsw_afk_key_info *key_info = aregion->region->key_info;
> -	u8 block_count = mlxsw_afk_key_info_blocks_count_get(key_info);
> -	u8 chunk_count = 1 + ((block_count - 1) >> 2);
> -
> -	__mlxsw_sp_acl_bf_key_encode(aregion, aentry, output, len,
> -				     MLXSW_BLOOM_KEY_CHUNKS,
> -				     MLXSW_SP4_BLOOM_CHUNK_PAD_BYTES,
> -				     MLXSW_SP4_BLOOM_CHUNK_KEY_OFFSET,
> -				     MLXSW_SP4_BLOOM_CHUNK_KEY_BYTES,
> -				     MLXSW_SP4_BLOOM_KEY_CHUNK_BYTES);
> +	u8 chunk_index, chunk_count, block_count;
> +	char *chunk = output;
> +	__be16 erp_region_id;
> +
> +	block_count = mlxsw_afk_key_info_blocks_count_get(key_info);
> +	chunk_count = 1 + ((block_count - 1) >> 2);
> +	erp_region_id = cpu_to_be16(aentry->ht_key.erp_id |
> +				   (aregion->region->id << 4));
> +	for (chunk_index = MLXSW_BLOOM_KEY_CHUNKS - chunk_count;
> +	     chunk_index < MLXSW_BLOOM_KEY_CHUNKS; chunk_index++) {
> +		memcpy(chunk, &erp_region_id, sizeof(erp_region_id));
> +		memcpy(chunk + MLXSW_SP4_BLOOM_CHUNK_KEY_OFFSET,
> +		       &aentry->ht_key.enc_key[chunk_key_offsets[chunk_index]],
> +		       MLXSW_SP4_BLOOM_CHUNK_KEY_BYTES);
> +		chunk += MLXSW_SP4_BLOOM_KEY_CHUNK_BYTES;
> +	}
> +	*len = chunk_count * MLXSW_SP4_BLOOM_KEY_CHUNK_BYTES;
> +
>   	mlxsw_sp4_bf_key_shift_chunks(chunk_count, output);
>   }
>
Sadly, it would appear your modifications haven't taken effect.

The same error output is still present.

I can also detail how to reproduce the error. You can verify this via 
LLVM cross-compilation using the [1] config on any computer.

     [1]. 
https://github.com/deepin-community/kernel/blob/linux-6.6.y/arch/s390/configs/deepin_s390x_z13_defconfig

The command I've been employing is:

     make ARCH=s390 CROSS_COMPILE=s390x-linux-gnu- CC="ccache clang" 
-j$(nproc)


Thanks,

-- 
WangYuli

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 645 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

Re: [PATCH net 1/2] mlxsw: spectrum_acl_bloom_filter: Expand chunk_key_offsets[chunk_index]

[Paolo Abeni]

On 3/12/25 2:20 PM, Ido Schimmel wrote:
> On Tue, Mar 11, 2025 at 10:17:00PM +0800, WangYuli wrote:
>> This is a workaround to mitigate a compiler anomaly.
>>
>> During LLVM toolchain compilation of this driver on s390x architecture, an
>> unreasonable __write_overflow_field warning occurs.
>>
>> Contextually, chunk_index is restricted to 0, 1 or 2. By expanding these
>> possibilities, the compile warning is suppressed.
> 
> I'm not sure why the fix suppresses the warning when the warning is
> about the destination buffer and the fix is about the source. Can you
> check if the below helps? It removes the parameterization from
> __mlxsw_sp_acl_bf_key_encode() and instead splits it to two variants.
> 
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c
> index a54eedb69a3f..3e1e4be72da2 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c
> @@ -110,7 +110,6 @@ static const u16 mlxsw_sp2_acl_bf_crc16_tab[256] = {
>   * +-----------+----------+-----------------------------------+
>   */
>  
> -#define MLXSW_SP4_BLOOM_CHUNK_PAD_BYTES 0
>  #define MLXSW_SP4_BLOOM_CHUNK_KEY_BYTES 18
>  #define MLXSW_SP4_BLOOM_KEY_CHUNK_BYTES 20
>  
> @@ -229,10 +228,9 @@ static u16 mlxsw_sp2_acl_bf_crc(const u8 *buffer, size_t len)
>  }
>  
>  static void
> -__mlxsw_sp_acl_bf_key_encode(struct mlxsw_sp_acl_atcam_region *aregion,
> -			     struct mlxsw_sp_acl_atcam_entry *aentry,
> -			     char *output, u8 *len, u8 max_chunks, u8 pad_bytes,
> -			     u8 key_offset, u8 chunk_key_len, u8 chunk_len)
> +mlxsw_sp2_acl_bf_key_encode(struct mlxsw_sp_acl_atcam_region *aregion,
> +			    struct mlxsw_sp_acl_atcam_entry *aentry,
> +			    char *output, u8 *len)
>  {
>  	struct mlxsw_afk_key_info *key_info = aregion->region->key_info;
>  	u8 chunk_index, chunk_count, block_count;
> @@ -243,30 +241,17 @@ __mlxsw_sp_acl_bf_key_encode(struct mlxsw_sp_acl_atcam_region *aregion,
>  	chunk_count = 1 + ((block_count - 1) >> 2);
>  	erp_region_id = cpu_to_be16(aentry->ht_key.erp_id |
>  				   (aregion->region->id << 4));
> -	for (chunk_index = max_chunks - chunk_count; chunk_index < max_chunks;
> -	     chunk_index++) {
> -		memset(chunk, 0, pad_bytes);
> -		memcpy(chunk + pad_bytes, &erp_region_id,
> +	for (chunk_index = MLXSW_BLOOM_KEY_CHUNKS - chunk_count;
> +	     chunk_index < MLXSW_BLOOM_KEY_CHUNKS; chunk_index++) {

Possibly the compiler is inferring chunck count can be greater then
MLXSW_BLOOM_KEY_CHUNKS?

something alike:

	chunk_index = min_t(0, MLXSW_BLOOM_KEY_CHUNKS - chunk_count, u8);

Could possibly please it?

/P

Re: [PATCH net 1/2] mlxsw: spectrum_acl_bloom_filter: Expand chunk_key_offsets[chunk_index]

[Ido Schimmel]

On Thu, Mar 13, 2025 at 02:52:11PM +0100, Paolo Abeni wrote:
> On 3/12/25 2:20 PM, Ido Schimmel wrote:
> > On Tue, Mar 11, 2025 at 10:17:00PM +0800, WangYuli wrote:
> >> This is a workaround to mitigate a compiler anomaly.
> >>
> >> During LLVM toolchain compilation of this driver on s390x architecture, an
> >> unreasonable __write_overflow_field warning occurs.
> >>
> >> Contextually, chunk_index is restricted to 0, 1 or 2. By expanding these
> >> possibilities, the compile warning is suppressed.
> > 
> > I'm not sure why the fix suppresses the warning when the warning is
> > about the destination buffer and the fix is about the source. Can you
> > check if the below helps? It removes the parameterization from
> > __mlxsw_sp_acl_bf_key_encode() and instead splits it to two variants.
> > 
> > diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c
> > index a54eedb69a3f..3e1e4be72da2 100644
> > --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c
> > +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c
> > @@ -110,7 +110,6 @@ static const u16 mlxsw_sp2_acl_bf_crc16_tab[256] = {
> >   * +-----------+----------+-----------------------------------+
> >   */
> >  
> > -#define MLXSW_SP4_BLOOM_CHUNK_PAD_BYTES 0
> >  #define MLXSW_SP4_BLOOM_CHUNK_KEY_BYTES 18
> >  #define MLXSW_SP4_BLOOM_KEY_CHUNK_BYTES 20
> >  
> > @@ -229,10 +228,9 @@ static u16 mlxsw_sp2_acl_bf_crc(const u8 *buffer, size_t len)
> >  }
> >  
> >  static void
> > -__mlxsw_sp_acl_bf_key_encode(struct mlxsw_sp_acl_atcam_region *aregion,
> > -			     struct mlxsw_sp_acl_atcam_entry *aentry,
> > -			     char *output, u8 *len, u8 max_chunks, u8 pad_bytes,
> > -			     u8 key_offset, u8 chunk_key_len, u8 chunk_len)
> > +mlxsw_sp2_acl_bf_key_encode(struct mlxsw_sp_acl_atcam_region *aregion,
> > +			    struct mlxsw_sp_acl_atcam_entry *aentry,
> > +			    char *output, u8 *len)
> >  {
> >  	struct mlxsw_afk_key_info *key_info = aregion->region->key_info;
> >  	u8 chunk_index, chunk_count, block_count;
> > @@ -243,30 +241,17 @@ __mlxsw_sp_acl_bf_key_encode(struct mlxsw_sp_acl_atcam_region *aregion,
> >  	chunk_count = 1 + ((block_count - 1) >> 2);
> >  	erp_region_id = cpu_to_be16(aentry->ht_key.erp_id |
> >  				   (aregion->region->id << 4));
> > -	for (chunk_index = max_chunks - chunk_count; chunk_index < max_chunks;
> > -	     chunk_index++) {
> > -		memset(chunk, 0, pad_bytes);
> > -		memcpy(chunk + pad_bytes, &erp_region_id,
> > +	for (chunk_index = MLXSW_BLOOM_KEY_CHUNKS - chunk_count;
> > +	     chunk_index < MLXSW_BLOOM_KEY_CHUNKS; chunk_index++) {
> 
> Possibly the compiler is inferring chunck count can be greater then
> MLXSW_BLOOM_KEY_CHUNKS?

I think so.

> 
> something alike:
> 
> 	chunk_index = min_t(0, MLXSW_BLOOM_KEY_CHUNKS - chunk_count, u8);
> 
> Could possibly please it?

I would like to get a warning if 'chunk_count' is larger than
'MLXSW_BLOOM_KEY_CHUNKS' since it should never happen.

I was able to reproduce the build failure and the following patch seems
to solve it for me. It removes the 'max_chunks' argument since it's
always 'MLXSW_BLOOM_KEY_CHUNKS' (3) and verifies that the number of
chunks that was calculated is never greater than it.

WangYuli, can you please test it?
Also, if you want it in net.git (as opposed to net-next.git), then it
needs a Fixes tag:

Fixes: 7585cacdb978 ("mlxsw: spectrum_acl: Add Bloom filter handling")

And I don't think we need patch #2.

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c
index a54eedb69a3f..a1ab5b09a6c5 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c
@@ -231,7 +231,7 @@ static u16 mlxsw_sp2_acl_bf_crc(const u8 *buffer, size_t len)
 static void
 __mlxsw_sp_acl_bf_key_encode(struct mlxsw_sp_acl_atcam_region *aregion,
 			     struct mlxsw_sp_acl_atcam_entry *aentry,
-			     char *output, u8 *len, u8 max_chunks, u8 pad_bytes,
+			     char *output, u8 *len, u8 pad_bytes,
 			     u8 key_offset, u8 chunk_key_len, u8 chunk_len)
 {
 	struct mlxsw_afk_key_info *key_info = aregion->region->key_info;
@@ -241,10 +241,14 @@ __mlxsw_sp_acl_bf_key_encode(struct mlxsw_sp_acl_atcam_region *aregion,
 
 	block_count = mlxsw_afk_key_info_blocks_count_get(key_info);
 	chunk_count = 1 + ((block_count - 1) >> 2);
+	if (WARN_ON_ONCE(chunk_count > MLXSW_BLOOM_KEY_CHUNKS)) {
+		*len = 0;
+		return;
+	}
 	erp_region_id = cpu_to_be16(aentry->ht_key.erp_id |
 				   (aregion->region->id << 4));
-	for (chunk_index = max_chunks - chunk_count; chunk_index < max_chunks;
-	     chunk_index++) {
+	for (chunk_index = MLXSW_BLOOM_KEY_CHUNKS - chunk_count;
+	     chunk_index < MLXSW_BLOOM_KEY_CHUNKS; chunk_index++) {
 		memset(chunk, 0, pad_bytes);
 		memcpy(chunk + pad_bytes, &erp_region_id,
 		       sizeof(erp_region_id));
@@ -262,7 +266,6 @@ mlxsw_sp2_acl_bf_key_encode(struct mlxsw_sp_acl_atcam_region *aregion,
 			    char *output, u8 *len)
 {
 	__mlxsw_sp_acl_bf_key_encode(aregion, aentry, output, len,
-				     MLXSW_BLOOM_KEY_CHUNKS,
 				     MLXSW_SP2_BLOOM_CHUNK_PAD_BYTES,
 				     MLXSW_SP2_BLOOM_CHUNK_KEY_OFFSET,
 				     MLXSW_SP2_BLOOM_CHUNK_KEY_BYTES,
@@ -379,7 +382,6 @@ mlxsw_sp4_acl_bf_key_encode(struct mlxsw_sp_acl_atcam_region *aregion,
 	u8 chunk_count = 1 + ((block_count - 1) >> 2);
 
 	__mlxsw_sp_acl_bf_key_encode(aregion, aentry, output, len,
-				     MLXSW_BLOOM_KEY_CHUNKS,
 				     MLXSW_SP4_BLOOM_CHUNK_PAD_BYTES,
 				     MLXSW_SP4_BLOOM_CHUNK_KEY_OFFSET,
 				     MLXSW_SP4_BLOOM_CHUNK_KEY_BYTES,

Re: [PATCH net 1/2] mlxsw: spectrum_acl_bloom_filter: Expand chunk_key_offsets[chunk_index]

[WangYuli]

[-- Attachment #1.1.1.1: Type: text/plain, Size: 3435 bytes --]

Hi Ido Schimmel,

On 2025/3/13 23:25, Ido Schimmel wrote:
> I would like to get a warning if 'chunk_count' is larger than
> 'MLXSW_BLOOM_KEY_CHUNKS' since it should never happen.
>
> I was able to reproduce the build failure and the following patch seems
> to solve it for me. It removes the 'max_chunks' argument since it's
> always 'MLXSW_BLOOM_KEY_CHUNKS' (3) and verifies that the number of
> chunks that was calculated is never greater than it.
>
> WangYuli, can you please test it?

My tests still show the same compilation failing.

Indeed, I have already tried similar fixes during my investigation, but 
to no avail.

> Also, if you want it in net.git (as opposed to net-next.git), then it
> needs a Fixes tag:
>
> Fixes: 7585cacdb978 ("mlxsw: spectrum_acl: Add Bloom filter handling")
OK
>
> And I don't think we need patch #2.
While #2 is admittedly a secondary modification compared to #1, should 
you be agreeable, I believe it's still preferable to combine #2 with #1 
instead of outright discarding #2.
>
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c
> index a54eedb69a3f..a1ab5b09a6c5 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c
> @@ -231,7 +231,7 @@ static u16 mlxsw_sp2_acl_bf_crc(const u8 *buffer, size_t len)
>   static void
>   __mlxsw_sp_acl_bf_key_encode(struct mlxsw_sp_acl_atcam_region *aregion,
>   			     struct mlxsw_sp_acl_atcam_entry *aentry,
> -			     char *output, u8 *len, u8 max_chunks, u8 pad_bytes,
> +			     char *output, u8 *len, u8 pad_bytes,
>   			     u8 key_offset, u8 chunk_key_len, u8 chunk_len)
>   {
>   	struct mlxsw_afk_key_info *key_info = aregion->region->key_info;
> @@ -241,10 +241,14 @@ __mlxsw_sp_acl_bf_key_encode(struct mlxsw_sp_acl_atcam_region *aregion,
>   
>   	block_count = mlxsw_afk_key_info_blocks_count_get(key_info);
>   	chunk_count = 1 + ((block_count - 1) >> 2);
> +	if (WARN_ON_ONCE(chunk_count > MLXSW_BLOOM_KEY_CHUNKS)) {
> +		*len = 0;
> +		return;
> +	}
>   	erp_region_id = cpu_to_be16(aentry->ht_key.erp_id |
>   				   (aregion->region->id << 4));
> -	for (chunk_index = max_chunks - chunk_count; chunk_index < max_chunks;
> -	     chunk_index++) {
> +	for (chunk_index = MLXSW_BLOOM_KEY_CHUNKS - chunk_count;
> +	     chunk_index < MLXSW_BLOOM_KEY_CHUNKS; chunk_index++) {
>   		memset(chunk, 0, pad_bytes);
>   		memcpy(chunk + pad_bytes, &erp_region_id,
>   		       sizeof(erp_region_id));
> @@ -262,7 +266,6 @@ mlxsw_sp2_acl_bf_key_encode(struct mlxsw_sp_acl_atcam_region *aregion,
>   			    char *output, u8 *len)
>   {
>   	__mlxsw_sp_acl_bf_key_encode(aregion, aentry, output, len,
> -				     MLXSW_BLOOM_KEY_CHUNKS,
>   				     MLXSW_SP2_BLOOM_CHUNK_PAD_BYTES,
>   				     MLXSW_SP2_BLOOM_CHUNK_KEY_OFFSET,
>   				     MLXSW_SP2_BLOOM_CHUNK_KEY_BYTES,
> @@ -379,7 +382,6 @@ mlxsw_sp4_acl_bf_key_encode(struct mlxsw_sp_acl_atcam_region *aregion,
>   	u8 chunk_count = 1 + ((block_count - 1) >> 2);
>   
>   	__mlxsw_sp_acl_bf_key_encode(aregion, aentry, output, len,
> -				     MLXSW_BLOOM_KEY_CHUNKS,
>   				     MLXSW_SP4_BLOOM_CHUNK_PAD_BYTES,
>   				     MLXSW_SP4_BLOOM_CHUNK_KEY_OFFSET,
>   				     MLXSW_SP4_BLOOM_CHUNK_KEY_BYTES,
>
-- 
WangYuli

[-- Attachment #1.1.1.2: Type: text/html, Size: 4281 bytes --]

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 645 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

Re: [PATCH net 1/2] mlxsw: spectrum_acl_bloom_filter: Expand chunk_key_offsets[chunk_index]

[Ido Schimmel]

Please use plain text emails.

On Fri, Mar 14, 2025 at 12:10:42AM +0800, WangYuli wrote:
> My tests still show the same compilation failing.

It passed with clang 18 on Fedora 40, but now I tested with clang 19 on
Fedora 41 and it's indeed failing.

How about [1]? It's similar to yours and passes with both clang
versions.

Thanks

[1]
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c
index a54eedb69a3f..9c54dba5ad12 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c
@@ -212,7 +212,22 @@ static const u8 mlxsw_sp4_acl_bf_crc6_tab[256] = {
  * This array defines key offsets for easy access when copying key blocks from
  * entry key to Bloom filter chunk.
  */
-static const u8 chunk_key_offsets[MLXSW_BLOOM_KEY_CHUNKS] = {2, 20, 38};
+static char *
+mlxsw_sp_acl_bf_enc_key_get(struct mlxsw_sp_acl_atcam_entry *aentry,
+                           u8 chunk_index)
+{
+       switch (chunk_index) {
+       case 0:
+               return &aentry->ht_key.enc_key[2];
+       case 1:
+               return &aentry->ht_key.enc_key[20];
+       case 2:
+               return &aentry->ht_key.enc_key[38];
+       default:
+               WARN_ON_ONCE(1);
+               return &aentry->ht_key.enc_key[0];
+       }
+}
 
 static u16 mlxsw_sp2_acl_bf_crc16_byte(u16 crc, u8 c)
 {
@@ -245,12 +260,13 @@ __mlxsw_sp_acl_bf_key_encode(struct mlxsw_sp_acl_atcam_region *aregion,
                                   (aregion->region->id << 4));
        for (chunk_index = max_chunks - chunk_count; chunk_index < max_chunks;
             chunk_index++) {
+               char *enc_key;
+
                memset(chunk, 0, pad_bytes);
                memcpy(chunk + pad_bytes, &erp_region_id,
                       sizeof(erp_region_id));
-               memcpy(chunk + key_offset,
-                      &aentry->ht_key.enc_key[chunk_key_offsets[chunk_index]],
-                      chunk_key_len);
+               enc_key = mlxsw_sp_acl_bf_enc_key_get(aentry, chunk_index);
+               memcpy(chunk + key_offset, enc_key, chunk_key_len);
                chunk += chunk_len;
        }
        *len = chunk_count * chunk_len;

Re: [PATCH net 1/2] mlxsw: spectrum_acl_bloom_filter: Expand chunk_key_offsets[chunk_index]

[WangYuli]

[-- Attachment #1.1.1: Type: text/plain, Size: 3537 bytes --]

Hi Ido Schimmel,

On 2025/3/14 03:41, Ido Schimmel wrote:
> Please use plain text emails.
>
OK
> It passed with clang 18 on Fedora 40, but now I tested with clang 19 on
> Fedora 41 and it's indeed failing.
>
> How about [1]? It's similar to yours and passes with both clang
> versions.

Indeed, this iteration compiles cleanly with clang-19.

My apologies for the delayed response; I've discovered something rather 
more unexpected and odd.

To be precise, the original, unmodified code builds successfully on both 
the recently released clang-20 and the ongoing development of clang-21. [1]

This strongly points to a clang-specific compiler bug that only impacts 
clang-19 and earlier versions (and it appears even clang-18 and clang-19 
show different behavior according to your findings)...

I'm somewhat at a loss for words regarding this,

but if we intend for this driver to compile on s390x with clang-19 (as 
outlined in the cover letter of this patch set, we should strive to 
support any possible combinations that the Linux kernel project endorses),

maybe it's still better to apply this change......

[1]. My Linux distribution is Debian sid, and I'm getting a more recent 
clang compiler from the following link, which is newer than what's 
provided in the distribution's package repositories: https://apt.llvm.org/

> Thanks
>
> [1]
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c
> index a54eedb69a3f..9c54dba5ad12 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c
> @@ -212,7 +212,22 @@ static const u8 mlxsw_sp4_acl_bf_crc6_tab[256] = {
>    * This array defines key offsets for easy access when copying key blocks from
>    * entry key to Bloom filter chunk.
>    */
> -static const u8 chunk_key_offsets[MLXSW_BLOOM_KEY_CHUNKS] = {2, 20, 38};
> +static char *
> +mlxsw_sp_acl_bf_enc_key_get(struct mlxsw_sp_acl_atcam_entry *aentry,
> +                           u8 chunk_index)
> +{
> +       switch (chunk_index) {
> +       case 0:
> +               return &aentry->ht_key.enc_key[2];
> +       case 1:
> +               return &aentry->ht_key.enc_key[20];
> +       case 2:
> +               return &aentry->ht_key.enc_key[38];
> +       default:
> +               WARN_ON_ONCE(1);
> +               return &aentry->ht_key.enc_key[0];
> +       }
> +}
>   
>   static u16 mlxsw_sp2_acl_bf_crc16_byte(u16 crc, u8 c)
>   {
> @@ -245,12 +260,13 @@ __mlxsw_sp_acl_bf_key_encode(struct mlxsw_sp_acl_atcam_region *aregion,
>                                     (aregion->region->id << 4));
>          for (chunk_index = max_chunks - chunk_count; chunk_index < max_chunks;
>               chunk_index++) {
> +               char *enc_key;
> +
>                  memset(chunk, 0, pad_bytes);
>                  memcpy(chunk + pad_bytes, &erp_region_id,
>                         sizeof(erp_region_id));
> -               memcpy(chunk + key_offset,
> -                      &aentry->ht_key.enc_key[chunk_key_offsets[chunk_index]],
> -                      chunk_key_len);
> +               enc_key = mlxsw_sp_acl_bf_enc_key_get(aentry, chunk_index);
> +               memcpy(chunk + key_offset, enc_key, chunk_key_len);
>                  chunk += chunk_len;
>          }
>          *len = chunk_count * chunk_len;
>
Thanks
-- 
WangYuli

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 645 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

Re: [PATCH net 1/2] mlxsw: spectrum_acl_bloom_filter: Expand chunk_key_offsets[chunk_index]

[Paolo Abeni]

On 3/11/25 3:17 PM, WangYuli wrote:
>  .../mlxsw/spectrum_acl_bloom_filter.c         | 39 ++++++++++++-------
>  1 file changed, 25 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c
> index a54eedb69a3f..96105bab680b 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c
> @@ -203,17 +203,6 @@ static const u8 mlxsw_sp4_acl_bf_crc6_tab[256] = {
>  0x2f, 0x02, 0x18, 0x35, 0x2c, 0x01, 0x1b, 0x36,
>  };
>  
> -/* Each chunk contains 4 key blocks. Chunk 2 uses key blocks 11-8,
> - * and we need to populate it with 4 key blocks copied from the entry encoded
> - * key. The original keys layout is same for Spectrum-{2,3,4}.
> - * Since the encoded key contains a 2 bytes padding, key block 11 starts at
> - * offset 2. block 7 that is used in chunk 1 starts at offset 20 as 4 key blocks
> - * take 18 bytes. See 'MLXSW_SP2_AFK_BLOCK_LAYOUT' for more details.
> - * This array defines key offsets for easy access when copying key blocks from
> - * entry key to Bloom filter chunk.
> - */
> -static const u8 chunk_key_offsets[MLXSW_BLOOM_KEY_CHUNKS] = {2, 20, 38};
> -
>  static u16 mlxsw_sp2_acl_bf_crc16_byte(u16 crc, u8 c)
>  {
>  	return (crc << 8) ^ mlxsw_sp2_acl_bf_crc16_tab[(crc >> 8) ^ c];
> @@ -237,6 +226,7 @@ __mlxsw_sp_acl_bf_key_encode(struct mlxsw_sp_acl_atcam_region *aregion,
>  	struct mlxsw_afk_key_info *key_info = aregion->region->key_info;
>  	u8 chunk_index, chunk_count, block_count;
>  	char *chunk = output;
> +	char *enc_key_src_ptr;

Please respect the reverse christmas tree order (same in the next patch)

>  	__be16 erp_region_id;
>  
>  	block_count = mlxsw_afk_key_info_blocks_count_get(key_info);
> @@ -248,9 +238,30 @@ __mlxsw_sp_acl_bf_key_encode(struct mlxsw_sp_acl_atcam_region *aregion,
>  		memset(chunk, 0, pad_bytes);
>  		memcpy(chunk + pad_bytes, &erp_region_id,
>  		       sizeof(erp_region_id));
> -		memcpy(chunk + key_offset,
> -		       &aentry->ht_key.enc_key[chunk_key_offsets[chunk_index]],
> -		       chunk_key_len);

Please try instead explicitly cap-ing the loop to the expected
boundaries (0-3)

Thanks,

Paolo

[PATCH net 2/2] mlxsw: spectrum_acl_bloom_filter: Type block_count to u32

[WangYuli]

In light of the fact that block_count is populated by
mlxsw_afk_key_info_blocks_count_get(key_info), whose type is unsigned
int and which returns the blocks_count member (also of type unsigned
int) from the mlxsw_afk_key_info struct, it is illogical to define
block_count as u8.

Co-developed-by: Zijian Chen <czj2441@163.com>
Signed-off-by: Zijian Chen <czj2441@163.com>
Signed-off-by: WangYuli <wangyuli@uniontech.com>
---
 .../net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c    | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c
index 96105bab680b..174bfda53985 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c
@@ -224,7 +224,8 @@ __mlxsw_sp_acl_bf_key_encode(struct mlxsw_sp_acl_atcam_region *aregion,
 			     u8 key_offset, u8 chunk_key_len, u8 chunk_len)
 {
 	struct mlxsw_afk_key_info *key_info = aregion->region->key_info;
-	u8 chunk_index, chunk_count, block_count;
+	u8 chunk_index, chunk_count;
+	u32 block_count;
 	char *chunk = output;
 	char *enc_key_src_ptr;
 	__be16 erp_region_id;
-- 
2.47.2