[dpdk-dev] [PATCH v11 2/6] app/testpmd: add multiple pools per core creation

[Ferruh Yigit]

On 10/16/2020 4:05 PM, Ferruh Yigit wrote:
> On 10/16/2020 2:39 PM, Viacheslav Ovsiienko wrote:
>> The command line parameter --mbuf-size is updated, it can handle
>> the multiple values like the following:
>>
>> --mbuf-size=2176,512,768,4096
>>
>> specifying the creation the extra memory pools with the requested
>> mbuf data buffer sizes. If some buffer split feature is engaged
>> the extra memory pools can be used to configure the Rx queues
>> with rte_the_dev_rx_queue_setup_ex().
>>
>> The extra pools are created with requested sizes, and pool names
>> are assigned with appended index: mbuf_pool_socket_%socket_%index.
>> Index zero is used to specify the first mandatory pool to maintain
>> compatibility with existing code.
>>
>> Signed-off-by: Viacheslav Ovsiienko <viacheslavo at nvidia.com>
> 
> <...>
> 
>>   /* Mbuf Pools */
>>   static inline void
>> -mbuf_poolname_build(unsigned int sock_id, char* mp_name, int name_size)
>> +mbuf_poolname_build(unsigned int sock_id, char *mp_name,
>> +            int name_size, unsigned int idx)
>>   {
>> -    snprintf(mp_name, name_size, "mbuf_pool_socket_%u", sock_id);
>> +    if (!idx)
>> +        snprintf(mp_name, name_size, "mbuf_pool_socket_%u", sock_id);
>> +    else
>> +        snprintf(mp_name, name_size, "mbuf_pool_socket_%u_%u",
>> +             sock_id, idx);
> 
> 'mp_name' can theoretically overflow and gives a compiler warning, although not 
> sure if this truncation is a problem in practice.
> 
> ../app/test-pmd/testpmd.c: In function ‘rx_queue_setup’:
> ../app/test-pmd/testpmd.h:666:53: error: ‘%u’ directive output may be truncated 
> writing between 1 and 10 bytes into a region of size between 0 and 7 
> [-Werror=format-truncation=]
>   666 |   snprintf(mp_name, name_size, "mbuf_pool_socket_%u_%u",
>       |                                                     ^~
> ../app/test-pmd/testpmd.h:666:32: note: directive argument in the range [1, 
> 4294967295]
>    666 |   snprintf(mp_name, name_size, "mbuf_pool_socket_%u_%u",
>        |                                ^~~~~~~~~~~~~~~~~~~~~~~~
> ../app/test-pmd/testpmd.h:666:3: note: ‘snprintf’ output between 21 and 39 bytes 
> into a destination of size 26
>    666 |   snprintf(mp_name, name_size, "mbuf_pool_socket_%u_%u",
>        |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>    667 |     sock_id, idx);
>        |     ~~~~~~~~~~~~~
> 
> Any suggestion for fix? Can we shorten the string above, is it used somewhere 
> else? Or casting variables to a smaller size may work too..

What do you think to following:

  diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
  index 4cd0f967f0..4ac1c1f86e 100644
  --- a/app/test-pmd/testpmd.h
  +++ b/app/test-pmd/testpmd.h
  @@ -661,10 +661,11 @@ mbuf_poolname_build(unsigned int sock_id, char *mp_name,
                      int name_size, unsigned int idx)
   {
          if (!idx)
  -               snprintf(mp_name, name_size, "mbuf_pool_socket_%u", sock_id);
  +               snprintf(mp_name, name_size, "mbuf_pool_s%u",
  +                       (uint16_t)sock_id);
          else
  -               snprintf(mp_name, name_size, "mbuf_pool_socket_%u_%u",
  -                        sock_id, idx);
  +               snprintf(mp_name, name_size, "mbuf_pool_s%u_%u",
  +                       (uint16_t)sock_id, (uint16_t)idx);
   }

   static inline struct rte_mempool *