[V5,1/7] app/testpmd: fix supported RSS offload display
Checks
Commit Message
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.
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(-)
Comments
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
>
在 2022/6/24 21:01, Ferruh Yigit 写道:
> 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.
All right.
>
>> 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/
>
Here is what I think. They have different purposes. The logic of current
patch
is to retain the original display behavior that is single bit offload and
"user defined xx". However, the logic in the V2 has changed the behavior.
I don't think this patch should change its original behavior. And it is
better
to print offload by single bit. In this way, the parsed offload
capability is
more accurate and convenient to use.
>
>> + 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
>>
>
> .
On 6/25/2022 3:12 AM, lihuisong (C) wrote:
>
> 在 2022/6/24 21:01, Ferruh Yigit 写道:
>> 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.
> All right.
>>
>>> 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/
>>
> Here is what I think. They have different purposes. The logic of current
> patch
> is to retain the original display behavior that is single bit offload and
> "user defined xx". However, the logic in the V2 has changed the behavior.
> I don't think this patch should change its original behavior. And it is
> better
> to print offload by single bit. In this way, the parsed offload
> capability is
> more accurate and convenient to use.
OK, lets keep original behavior.
Since 'rss_type_table[]' array has both single bit and combined bit
entries, same array can work for both purposes.
>>
>>> + 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
>>>
>>
>> .
@@ -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);
+ 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)
{
@@ -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;