On 10/16/2020 4:55 PM, Slava Ovsiienko wrote:
-----Original Message----- From: Ferruh Yigit <ferruh.yi...@intel.com> Sent: Friday, October 16, 2020 18:52 To: Slava Ovsiienko <viachesl...@nvidia.com>; dev@dpdk.org Cc: NBU-Contact-Thomas Monjalon <tho...@monjalon.net>; step...@networkplumber.org; olivier.m...@6wind.com; jerinjac...@gmail.com; maxime.coque...@redhat.com; david.march...@redhat.com; arybche...@solarflare.com Subject: Re: [dpdk-dev] [PATCH v11 2/6] app/testpmd: add multiple pools per core creationOn 10/16/2020 4:48 PM, Slava Ovsiienko wrote:-----Original Message----- From: Ferruh Yigit <ferruh.yi...@intel.com> Sent: Friday, October 16, 2020 18:39 To: Slava Ovsiienko <viachesl...@nvidia.com>; dev@dpdk.org Cc: NBU-Contact-Thomas Monjalon <tho...@monjalon.net>; step...@networkplumber.org; olivier.m...@6wind.com; jerinjac...@gmail.com; maxime.coque...@redhat.com; david.march...@redhat.com; arybche...@solarflare.com Subject: Re: [dpdk-dev] [PATCH v11 2/6] app/testpmd: add multiple pools per core creation 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 <viachesl...@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 *Testpmd build mbuf pool names with mbuf_poolname_build() routine only (invoked from mbuf_pool_find/mbuf_pool_create). I suppose it is safe to replace "mbuf_pool_socket_" prefix with shorter one. Say something like this: #define TESTPMD_MBUF_POOL_PFX "mbuf"Just 'mbuf' may not give enough context in the logs, what about "mbuf_pool_%u_%u"?It is place in the pool structure, so it is an object name and tightly coupled with the pool. OK, let's try "mb_pool" ?
sounds good
