Hi Ido Schimmel,
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.
OKAlso, 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")
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.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,
Attachment:
OpenPGP_0xC5DA1F3046F40BEE.asc
Description: OpenPGP public key
Attachment:
OpenPGP_signature.asc
Description: OpenPGP digital signature