Re: [PATCH V5 1/7] app/testpmd: fix supported RSS offload display

[Ferruh Yigit <ferruh.yigit@xilinx.com>]

On 6/24/2022 8:23 AM, Huisong Li wrote:
> 
> The rte_eth_dev_info.flow_type_rss_offloads is populated in terms of
> RTE_ETH_RSS_* bits. If PMD sets RTE_ETH_RSS_L3_SRC_ONLY to
> dev_info->flow_type_rss_offloads. testpmd will display "user defined 63"
> when run 'show port info 0'. Because testpmd use flowtype_to_str()
> to display the supported RSS offload of PMD. In fact, the function is
> used to display flow type in FDIR commands for i40e or ixgbe. This patch
> uses the RTE_ETH_RSS_* bits to display supported RSS offload of PMD.
> 
> In addition, offloads that are not in rss_type_table[] should be displayed
> as "unknown offload xxx", instead of "user defined 63". So this patch fixes
> it.
> 

There is something as "user defined" RSS type, so please keep it as it is.
For more details please check:
Commit 8b94c81e3341 ("app/testpmd: port info prints dynamically mapped 
flow types")
Commit 5a4806d304e0 ("app/testpmd: support updating pctype mapping")

Simply this is to allow doing RSS on user defined protocols, supported 
by plugging like Intel DDP.

> Fixes: b12964f621dc ("ethdev: unification of RSS offload types")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> Signed-off-by: Ferruh Yigit <ferruh.yigit@xilinx.com>
> ---
>   app/test-pmd/config.c  | 40 ++++++++++++++++++++++++++--------------
>   app/test-pmd/testpmd.h |  2 ++
>   2 files changed, 28 insertions(+), 14 deletions(-)
> 
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> index 62833fe97c..36a828307c 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -66,8 +66,6 @@
> 
>   #define NS_PER_SEC 1E9
> 
> -static char *flowtype_to_str(uint16_t flow_type);
> -
>   static const struct {
>          enum tx_pkt_split split;
>          const char *name;
> @@ -675,6 +673,19 @@ print_dev_capabilities(uint64_t capabilities)
>          }
>   }
> 
> +const char *
> +rsstypes_to_str(uint64_t rss_type)
> +{
> +       uint16_t i;
> +
> +       for (i = 0; rss_type_table[i].str != NULL; i++) {
> +               if (rss_type_table[i].rss_type == rss_type)
> +                       return rss_type_table[i].str;
> +       }
> +
> +       return NULL;
> +}
> +
>   void
>   port_infos_display(portid_t port_id)
>   {
> @@ -779,19 +790,20 @@ port_infos_display(portid_t port_id)
>          if (!dev_info.flow_type_rss_offloads)
>                  printf("No RSS offload flow type is supported.\n");
>          else {
> +               uint64_t rss_offload_types = dev_info.flow_type_rss_offloads;
>                  uint16_t i;
> -               char *p;
> 
>                  printf("Supported RSS offload flow types:\n");
> -               for (i = RTE_ETH_FLOW_UNKNOWN + 1;
> -                    i < sizeof(dev_info.flow_type_rss_offloads) * CHAR_BIT; i++) {
> -                       if (!(dev_info.flow_type_rss_offloads & (1ULL << i)))
> -                               continue;
> -                       p = flowtype_to_str(i);
> -                       if (p)
> -                               printf("  %s\n", p);
> -                       else
> -                               printf("  user defined %d\n", i);
> +               for (i = 0; i < sizeof(rss_offload_types) * CHAR_BIT; i++) {
> +                       uint64_t rss_offload = RTE_BIT64(i);

This logic is wrong, as we talked before some RSS types can be multiple 
bit, with about logic you can't catch them.

The logic in the V2 of this set [1] is correct, which walks through 
'rss_type_table[]' array and check if any value in that array exists in 
'flow_type_rss_offloads'.

[1]
https://patchwork.dpdk.org/project/dpdk/patch/20220425092523.52338-2-lihuisong@huawei.com/

> +                       if ((rss_offload_types & rss_offload) != 0) {
> +                               const char *p = rsstypes_to_str(rss_offload);
> +                               if (p)
> +                                       printf("  %s\n", p);
> +                               else
> +                                       printf("  unknown_offload(BIT(%u))\n",
> +                                              i);
> +                       }
>                  }
>          }
> 
> @@ -5604,6 +5616,8 @@ set_record_burst_stats(uint8_t on_off)
>          record_burst_stats = on_off;
>   }
> 
> +#if defined(RTE_NET_I40E) || defined(RTE_NET_IXGBE)
> +
>   static char*
>   flowtype_to_str(uint16_t flow_type)
>   {
> @@ -5647,8 +5661,6 @@ flowtype_to_str(uint16_t flow_type)
>          return NULL;
>   }
> 
> -#if defined(RTE_NET_I40E) || defined(RTE_NET_IXGBE)
> -
>   static inline void
>   print_fdir_mask(struct rte_eth_fdir_masks *mask)
>   {
> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
> index eeefb5e70f..195488b602 100644
> --- a/app/test-pmd/testpmd.h
> +++ b/app/test-pmd/testpmd.h
> @@ -1199,6 +1199,8 @@ extern int flow_parse(const char *src, void *result, unsigned int size,
>                        struct rte_flow_item **pattern,
>                        struct rte_flow_action **actions);
> 
> +const char *rsstypes_to_str(uint64_t rss_type);
> +
>   /* For registering driver specific testpmd commands. */
>   struct testpmd_driver_commands {
>          TAILQ_ENTRY(testpmd_driver_commands) next;
> --
> 2.33.0
>