diff mbox series

[3/5] app/proc-info: fix never show RSS info

Message ID 20230315110033.30143-4-liudongdong3@huawei.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series support setting and querying RSS algorithms |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Dongdong Liu March 15, 2023, 11 a.m. UTC 
From: Jie Hai <haijie1@huawei.com>

Command show-port shows rss info only if rss_conf.rss_key
is not null but it will never be true. This patch allocates
memory for rss_conf.rss_key and makes it possible to show
rss info.

Fixes: 8a37f37fc243 ("app/procinfo: add --show-port")
Cc: stable@dpdk.org

Signed-off-by: Jie Hai <haijie1@huawei.com>
Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
---
 app/proc-info/main.c | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

Comments

Ferruh Yigit June 2, 2023, 8:19 p.m. UTC | #1 
On 3/15/2023 11:00 AM, Dongdong Liu wrote:

> From: Jie Hai <haijie1@huawei.com>
> 
> Command show-port shows rss info only if rss_conf.rss_key
> is not null but it will never be true. This patch allocates
> memory for rss_conf.rss_key and makes it possible to show
> rss info.
> 

Why 'rss_conf.rss_key == NULL' case is never true?

'rss_key' is pointer and 'rte_eth_dev_rss_hash_conf_get()' doesn't
allocate it, so can't it be NULL?


> Fixes: 8a37f37fc243 ("app/procinfo: add --show-port")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Jie Hai <haijie1@huawei.com>
> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
> ---
>  app/proc-info/main.c | 26 +++++++++++++++++---------
>  1 file changed, 17 insertions(+), 9 deletions(-)
> 
> diff --git a/app/proc-info/main.c b/app/proc-info/main.c
> index 53e852a07c..878ce37e8b 100644
> --- a/app/proc-info/main.c
> +++ b/app/proc-info/main.c
> @@ -823,6 +823,7 @@ show_port(void)
>                 struct rte_eth_fc_conf fc_conf;
>                 struct rte_ether_addr mac;
>                 struct rte_eth_dev_owner owner;
> +               uint8_t *rss_key;
> 
>                 /* Skip if port is not in mask */
>                 if ((enabled_port_mask & (1ul << i)) == 0)
> @@ -981,19 +982,26 @@ show_port(void)
>                         printf("\n");
>                 }
> 
> +               rss_key = rte_malloc(NULL,
> +                       dev_info.hash_key_size * sizeof(uint8_t), 0);
> +               if (rss_key == NULL)
> +                       return;
> +
> +               rss_conf.rss_key = rss_key;
> +               rss_conf.rss_key_len = dev_info.hash_key_size;
>                 ret = rte_eth_dev_rss_hash_conf_get(i, &rss_conf);
>                 if (ret == 0) {
> -                       if (rss_conf.rss_key) {
> -                               printf("  - RSS\n");
> -                               printf("\t  -- RSS len %u key (hex):",
> -                                               rss_conf.rss_key_len);
> -                               for (k = 0; k < rss_conf.rss_key_len; k++)
> -                                       printf(" %x", rss_conf.rss_key[k]);
> -                               printf("\t  -- hf 0x%"PRIx64"\n",
> -                                               rss_conf.rss_hf);
> -                       }
> +                       printf("  - RSS\n");
> +                       printf("\t  -- RSS len %u key (hex):",
> +                                       rss_conf.rss_key_len);
> +                       for (k = 0; k < rss_conf.rss_key_len; k++)
> +                               printf(" %x", rss_conf.rss_key[k]);
> +                       printf("\t  -- hf 0x%"PRIx64"\n",
> +                                       rss_conf.rss_hf);
>                 }
> 
> +               rte_free(rss_key);
> +
>  #ifdef RTE_LIB_SECURITY
>                 show_security_context(i, true);
>  #endif
> --
> 2.22.0
>
Stephen Hemminger June 2, 2023, 9:19 p.m. UTC | #2 
On Wed, 15 Mar 2023 19:00:31 +0800
Dongdong Liu <liudongdong3@huawei.com> wrote:

> +		rss_key = rte_malloc(NULL,
> +			dev_info.hash_key_size * sizeof(uint8_t), 0);
> +		if (rss_key == NULL)
> +			return;

Don't use rte_malloc() unless this is a structure that needs to be
shared between primary/secondary. In this case it doesn't need to be shared;
so just use calloc().
Dongdong Liu June 5, 2023, 1:04 p.m. UTC | #3 
Hi Ferruh

On 2023/6/3 4:19, Ferruh Yigit wrote:
> On 3/15/2023 11:00 AM, Dongdong Liu wrote:
>
>> From: Jie Hai <haijie1@huawei.com>
>>
>> Command show-port shows rss info only if rss_conf.rss_key
>> is not null but it will never be true. This patch allocates
>> memory for rss_conf.rss_key and makes it possible to show
>> rss info.
>>
>
> Why 'rss_conf.rss_key == NULL' case is never true?
>
> 'rss_key' is pointer and 'rte_eth_dev_rss_hash_conf_get()' doesn't
> allocate it, so can't it be NULL?

The code want to show rss info (rss_key, len and rss_hf),
but it does not allocate memory for rss_conf.rss_key,
so current code rss_conf.rss_key will be always NULL,
This patch fixes that.

Maybe the description of commit message is not correct,
will fix that.

Thanks,
Dongdong
>
>
>> Fixes: 8a37f37fc243 ("app/procinfo: add --show-port")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Jie Hai <haijie1@huawei.com>
>> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
>> ---
>>  app/proc-info/main.c | 26 +++++++++++++++++---------
>>  1 file changed, 17 insertions(+), 9 deletions(-)
>>
>> diff --git a/app/proc-info/main.c b/app/proc-info/main.c
>> index 53e852a07c..878ce37e8b 100644
>> --- a/app/proc-info/main.c
>> +++ b/app/proc-info/main.c
>> @@ -823,6 +823,7 @@ show_port(void)
>>                 struct rte_eth_fc_conf fc_conf;
>>                 struct rte_ether_addr mac;
>>                 struct rte_eth_dev_owner owner;
>> +               uint8_t *rss_key;
>>
>>                 /* Skip if port is not in mask */
>>                 if ((enabled_port_mask & (1ul << i)) == 0)
>> @@ -981,19 +982,26 @@ show_port(void)
>>                         printf("\n");
>>                 }
>>
>> +               rss_key = rte_malloc(NULL,
>> +                       dev_info.hash_key_size * sizeof(uint8_t), 0);
>> +               if (rss_key == NULL)
>> +                       return;
>> +
>> +               rss_conf.rss_key = rss_key;
>> +               rss_conf.rss_key_len = dev_info.hash_key_size;
>>                 ret = rte_eth_dev_rss_hash_conf_get(i, &rss_conf);
>>                 if (ret == 0) {
>> -                       if (rss_conf.rss_key) {
>> -                               printf("  - RSS\n");
>> -                               printf("\t  -- RSS len %u key (hex):",
>> -                                               rss_conf.rss_key_len);
>> -                               for (k = 0; k < rss_conf.rss_key_len; k++)
>> -                                       printf(" %x", rss_conf.rss_key[k]);
>> -                               printf("\t  -- hf 0x%"PRIx64"\n",
>> -                                               rss_conf.rss_hf);
>> -                       }
>> +                       printf("  - RSS\n");
>> +                       printf("\t  -- RSS len %u key (hex):",
>> +                                       rss_conf.rss_key_len);
>> +                       for (k = 0; k < rss_conf.rss_key_len; k++)
>> +                               printf(" %x", rss_conf.rss_key[k]);
>> +                       printf("\t  -- hf 0x%"PRIx64"\n",
>> +                                       rss_conf.rss_hf);
>>                 }
>>
>> +               rte_free(rss_key);
>> +
>>  #ifdef RTE_LIB_SECURITY
>>                 show_security_context(i, true);
>>  #endif
>> --
>> 2.22.0
>>
>
> .
>
Dongdong Liu June 5, 2023, 1:07 p.m. UTC | #4 
Hi Stephen
On 2023/6/3 5:19, Stephen Hemminger wrote:
> On Wed, 15 Mar 2023 19:00:31 +0800
> Dongdong Liu <liudongdong3@huawei.com> wrote:
>
>> +		rss_key = rte_malloc(NULL,
>> +			dev_info.hash_key_size * sizeof(uint8_t), 0);
>> +		if (rss_key == NULL)
>> +			return;
>
> Don't use rte_malloc() unless this is a structure that needs to be
> shared between primary/secondary. In this case it doesn't need to be shared;
> so just use calloc().
Thanks for pointing that,
Will do.

Thanks,
Dongdong
> .
>
diff mbox series

Patch

diff --git a/app/proc-info/main.c b/app/proc-info/main.c
index 53e852a07c..878ce37e8b 100644
--- a/app/proc-info/main.c
+++ b/app/proc-info/main.c
@@ -823,6 +823,7 @@  show_port(void)
 		struct rte_eth_fc_conf fc_conf;
 		struct rte_ether_addr mac;
 		struct rte_eth_dev_owner owner;
+		uint8_t *rss_key;
 
 		/* Skip if port is not in mask */
 		if ((enabled_port_mask & (1ul << i)) == 0)
@@ -981,19 +982,26 @@  show_port(void)
 			printf("\n");
 		}
 
+		rss_key = rte_malloc(NULL,
+			dev_info.hash_key_size * sizeof(uint8_t), 0);
+		if (rss_key == NULL)
+			return;
+
+		rss_conf.rss_key = rss_key;
+		rss_conf.rss_key_len = dev_info.hash_key_size;
 		ret = rte_eth_dev_rss_hash_conf_get(i, &rss_conf);
 		if (ret == 0) {
-			if (rss_conf.rss_key) {
-				printf("  - RSS\n");
-				printf("\t  -- RSS len %u key (hex):",
-						rss_conf.rss_key_len);
-				for (k = 0; k < rss_conf.rss_key_len; k++)
-					printf(" %x", rss_conf.rss_key[k]);
-				printf("\t  -- hf 0x%"PRIx64"\n",
-						rss_conf.rss_hf);
-			}
+			printf("  - RSS\n");
+			printf("\t  -- RSS len %u key (hex):",
+					rss_conf.rss_key_len);
+			for (k = 0; k < rss_conf.rss_key_len; k++)
+				printf(" %x", rss_conf.rss_key[k]);
+			printf("\t  -- hf 0x%"PRIx64"\n",
+					rss_conf.rss_hf);
 		}
 
+		rte_free(rss_key);
+
 #ifdef RTE_LIB_SECURITY
 		show_security_context(i, true);
 #endif