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

From: WangYuli
Date: Thu Mar 13 2025 - 12:16:56 EST


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: OpenPGP_0xC5DA1F3046F40BEE.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature.asc
Description: OpenPGP digital signature