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

[WangYuli]

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

Attachment: OpenPGP_signature.asc
Description: OpenPGP digital signature