Re: mlx5 rte_eth_dev_info.reta_size value

["Robin Jarry" <rjarry@redhat.com>]

Robin Jarry, Dec 05, 2022 at 16:57:
> Hi Ori,
>
> While working on a patch for OvS[1], I have tried to reconfigure the
> redirection table using the code examples that are layout around in
> testpmd and other places.
>
> [1]: http://patchwork.ozlabs.org/project/openvswitch/patch/20221021145308.141933-1-rjarry@redhat.com/
>
> Here is a stripped down version of the code I use:
>
>  int update_reta(int port_id, int num_rxq)
>  {
>    struct rte_eth_rss_reta_entry64 *conf;
>    struct rte_eth_dev_info info;
>    size_t conf_size;
>    int err;
>
>    rte_eth_dev_info_get(port_id, &info);
>    conf_size = (info.reta_size / RTE_ETH_RETA_GROUP_SIZE) * sizeof(*conf);
>    conf = malloc(conf_size);
>    memset(conf, 0, conf_size);
>
>    for (uint16_t i = 0; i < info.reta_size; i++) {
>      uint16_t idx = i / RTE_ETH_RETA_GROUP_SIZE;
>      uint16_t shift = i % RTE_ETH_RETA_GROUP_SIZE;
>      reta_conf[idx].mask |= 1ULL << shift;
>      reta_conf[idx].reta[shift] = i % num_rxq;
>    }
>    err = rte_eth_dev_rss_reta_update(port_id, conf, info.reta_size);
>    free(conf);
>
>    return err;
>  }
>
> This works well for i40e and ice drivers but I get very confusing
> reta_size values with mlx5.
>
> mlx5_ethdev.c
>
> 333├>        info->reta_size = priv->reta_idx_n ?
> 334│                 priv->reta_idx_n : config->ind_table_max_size;
>
> (gdb) p priv->reta_idx_n
> $5 = 2
> (gdb) p config->ind_table_max_size
> $6 = 512
>
> Obviously, info.reta_size / RTE_ETH_RETA_GROUP_SIZE = 1 / 512 = 0
>
> From what I had understood info.reta_size should be a multiple of
> RTE_ETH_RETA_GROUP_SIZE. This is what I can observe with i40e and ice at
> least. Is it possible that the mlx5 driver has an issue there?
>
> I found this commit[2] from 2015 that may have introduced an issue but
> I am surprised that no one has ever encountered that before me. The
> suspicious code bit is:
>
> +    /* If the requested number of RX queues is not a power of two, use the
> +     * maximum indirection table size for better balancing.
> +     * The result is always rounded to the next power of two. */
> +    reta_idx_n = (1 << log2above((rxqs_n & (rxqs_n - 1)) ?
> +                    priv->ind_table_max_size :
> +                    rxqs_n));
>
> When rxqs_n == 2, reta_idx_n is initialized to 2 as well.
>
> [2]: https://git.dpdk.org/dpdk/commit/?id=634efbc2c8c05
>
> If you can provide any help, that would be much appreciated.
>
> Thanks!

To make sure, I have written a simple program that reuses log2above:

   #include <stdio.h>

   static unsigned int log2above(unsigned int v)
   {
       unsigned int l, r;
       for (l = 0, r = 0; (v >> 1); ++l, v >>= 1)
           r |= (v & 1);
       return l + r;
   }

   void main(void)
   {
       for (unsigned n = 1; n < 16; n++) {
           printf("n_rxq=%d -> reta_size=%d\n", n,
               1 << log2above((n & (n - 1)) ? 512 : n));
       }
   }

Running this yields:

   n_rxq=1 -> reta_size=1
   n_rxq=2 -> reta_size=2
   n_rxq=3 -> reta_size=512
   n_rxq=4 -> reta_size=4
   n_rxq=5 -> reta_size=512
   n_rxq=6 -> reta_size=512
   n_rxq=7 -> reta_size=512
   n_rxq=8 -> reta_size=8
   n_rxq=9 -> reta_size=512
   n_rxq=10 -> reta_size=512
   n_rxq=11 -> reta_size=512
   n_rxq=12 -> reta_size=512
   n_rxq=13 -> reta_size=512
   n_rxq=14 -> reta_size=512
   n_rxq=15 -> reta_size=512

There is obviously something wrong and I am not sure what was the
original intention. So I don't know what to do to fix this.

I added Nelio in the thread. Maybe he can help :)

Cheers.