diff mbox series

[v4,2/3] app/testpmd: add maximum Rx buffer size display

Message ID 20231102121608.10170-3-lihuisong@huawei.com (mailing list archive)
State Changes Requested, archived
Delegated to: Ferruh Yigit
Headers
Series introduce maximum Rx buffer size |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

lihuisong (C) Nov. 2, 2023, 12:16 p.m. UTC 
  Add maximum Rx buffer size display.

Signed-off-by: Huisong Li <lihuisong@huawei.com>
Acked-by: Chengwen Feng <fengchengwen@huawei.com>
---
 app/test-pmd/config.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Ferruh Yigit Nov. 2, 2023, 4:42 p.m. UTC | #1 
On 11/2/2023 12:16 PM, Huisong Li wrote:
> Add maximum Rx buffer size display.
> 

I think there is a value to show what is the intended usage of this new
field in application level,
that is why what do you think to use testpmd?

Testpmd can be updated to check if mbuf data size is bigger than device
buffer size, and just log, similar to done in ethdev layer, just to
showcase how application should use it.

For many drivers, this won't have anything functional, because of
UINT32_MAX size, but it is still valuable as sample.


> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> Acked-by: Chengwen Feng <fengchengwen@huawei.com>
> ---
>  app/test-pmd/config.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> index b9fdb7e8f1..2ac6f15773 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -881,6 +881,8 @@ port_infos_display(portid_t port_id)
>  	}
>  
>  	printf("Minimum size of RX buffer: %u\n", dev_info.min_rx_bufsize);
> +	if (dev_info.max_rx_bufsize != UINT32_MAX)
> +		printf("Maximum size of RX buffer: %u\n", dev_info.max_rx_bufsize);
>  	printf("Maximum configurable length of RX packet: %u\n",
>  		dev_info.max_rx_pktlen);
>  	printf("Maximum configurable size of LRO aggregated packet: %u\n",
lihuisong (C) Nov. 3, 2023, 2:39 a.m. UTC | #2 
在 2023/11/3 0:42, Ferruh Yigit 写道:
> On 11/2/2023 12:16 PM, Huisong Li wrote:
>> Add maximum Rx buffer size display.
>>
> I think there is a value to show what is the intended usage of this new
> field in application level,
> that is why what do you think to use testpmd?
>
> Testpmd can be updated to check if mbuf data size is bigger than device
> buffer size, and just log, similar to done in ethdev layer, just to
> showcase how application should use it.
Just to check "--mbuf-size" in testpmd cmdline, right?
But we already did it in ethdev layer, is it necessary for testpmd?
Or we can add a usecase in test_ethdev_api.c.
what do you think?
>
> For many drivers, this won't have anything functional, because of
> UINT32_MAX size, but it is still valuable as sample.
>
>
>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>> Acked-by: Chengwen Feng <fengchengwen@huawei.com>
>> ---
>>   app/test-pmd/config.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
>> index b9fdb7e8f1..2ac6f15773 100644
>> --- a/app/test-pmd/config.c
>> +++ b/app/test-pmd/config.c
>> @@ -881,6 +881,8 @@ port_infos_display(portid_t port_id)
>>   	}
>>   
>>   	printf("Minimum size of RX buffer: %u\n", dev_info.min_rx_bufsize);
>> +	if (dev_info.max_rx_bufsize != UINT32_MAX)
>> +		printf("Maximum size of RX buffer: %u\n", dev_info.max_rx_bufsize);
>>   	printf("Maximum configurable length of RX packet: %u\n",
>>   		dev_info.max_rx_pktlen);
>>   	printf("Maximum configurable size of LRO aggregated packet: %u\n",
> .
Ferruh Yigit Nov. 3, 2023, 3:53 a.m. UTC | #3 
On 11/3/2023 2:39 AM, lihuisong (C) wrote:
> 
> 在 2023/11/3 0:42, Ferruh Yigit 写道:
>> On 11/2/2023 12:16 PM, Huisong Li wrote:
>>> Add maximum Rx buffer size display.
>>>
>> I think there is a value to show what is the intended usage of this new
>> field in application level,
>> that is why what do you think to use testpmd?
>>
>> Testpmd can be updated to check if mbuf data size is bigger than device
>> buffer size, and just log, similar to done in ethdev layer, just to
>> showcase how application should use it.
> Just to check "--mbuf-size" in testpmd cmdline, right?
> But we already did it in ethdev layer, is it necessary for testpmd?
>

What is the expected behavior from the application?

I assumed it should first get the max_rx_bufsize, and if the existing
pktmbuf_pool size bigger than max_rx_bufsize, it can free the mempool
and create a new one with reduced size.

I just thought it can be good to have a sample for this usage, instead
of creating a new mempool, since it will complicate things unnecessary,
testpmd can just log, but yes when it just logs, it will be duplication
of the ethdev.

And I think this is not suitable for the unit test, since this is not
API verification issue.


Thinking twice, lets scratch the request, to not duplicate or increase
complexity more, keep testpmd as it is.


> Or we can add a usecase in test_ethdev_api.c.
> what do you think?
>>
>> For many drivers, this won't have anything functional, because of
>> UINT32_MAX size, but it is still valuable as sample.
>>
>>
>>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>>> Acked-by: Chengwen Feng <fengchengwen@huawei.com>
>>> ---
>>>   app/test-pmd/config.c | 2 ++
>>>   1 file changed, 2 insertions(+)
>>>
>>> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
>>> index b9fdb7e8f1..2ac6f15773 100644
>>> --- a/app/test-pmd/config.c
>>> +++ b/app/test-pmd/config.c
>>> @@ -881,6 +881,8 @@ port_infos_display(portid_t port_id)
>>>       }
>>>         printf("Minimum size of RX buffer: %u\n",
>>> dev_info.min_rx_bufsize);
>>> +    if (dev_info.max_rx_bufsize != UINT32_MAX)
>>> +        printf("Maximum size of RX buffer: %u\n",
>>> dev_info.max_rx_bufsize);
>>>       printf("Maximum configurable length of RX packet: %u\n",
>>>           dev_info.max_rx_pktlen);
>>>       printf("Maximum configurable size of LRO aggregated packet: %u\n",
>> .
lihuisong (C) Nov. 3, 2023, 6:37 a.m. UTC | #4 
在 2023/11/3 11:53, Ferruh Yigit 写道:
> On 11/3/2023 2:39 AM, lihuisong (C) wrote:
>> 在 2023/11/3 0:42, Ferruh Yigit 写道:
>>> On 11/2/2023 12:16 PM, Huisong Li wrote:
>>>> Add maximum Rx buffer size display.
>>>>
>>> I think there is a value to show what is the intended usage of this new
>>> field in application level,
>>> that is why what do you think to use testpmd?
>>>
>>> Testpmd can be updated to check if mbuf data size is bigger than device
>>> buffer size, and just log, similar to done in ethdev layer, just to
>>> showcase how application should use it.
>> Just to check "--mbuf-size" in testpmd cmdline, right?
>> But we already did it in ethdev layer, is it necessary for testpmd?
>>
> What is the expected behavior from the application?
>
> I assumed it should first get the max_rx_bufsize, and if the existing
> pktmbuf_pool size bigger than max_rx_bufsize, it can free the mempool
> and create a new one with reduced size.
>
> I just thought it can be good to have a sample for this usage, instead
> of creating a new mempool, since it will complicate things unnecessary,
> testpmd can just log, but yes when it just logs, it will be duplication
> of the ethdev.
I understand you.
Yeah, it will be duplicate with ethdev.
>
> And I think this is not suitable for the unit test, since this is not
> API verification issue.
Agreed.
>
>
> Thinking twice, lets scratch the request, to not duplicate or increase
> complexity more, keep testpmd as it is.
ok, keep testpmd as it is.
will send next version.
>
>
>> Or we can add a usecase in test_ethdev_api.c.
>> what do you think?
>>> For many drivers, this won't have anything functional, because of
>>> UINT32_MAX size, but it is still valuable as sample.
>>>
>>>
>>>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>>>> Acked-by: Chengwen Feng <fengchengwen@huawei.com>
>>>> ---
>>>>    app/test-pmd/config.c | 2 ++
>>>>    1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
>>>> index b9fdb7e8f1..2ac6f15773 100644
>>>> --- a/app/test-pmd/config.c
>>>> +++ b/app/test-pmd/config.c
>>>> @@ -881,6 +881,8 @@ port_infos_display(portid_t port_id)
>>>>        }
>>>>          printf("Minimum size of RX buffer: %u\n",
>>>> dev_info.min_rx_bufsize);
>>>> +    if (dev_info.max_rx_bufsize != UINT32_MAX)
>>>> +        printf("Maximum size of RX buffer: %u\n",
>>>> dev_info.max_rx_bufsize);
>>>>        printf("Maximum configurable length of RX packet: %u\n",
>>>>            dev_info.max_rx_pktlen);
>>>>        printf("Maximum configurable size of LRO aggregated packet: %u\n",
>>> .
> .
diff mbox series

Patch

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index b9fdb7e8f1..2ac6f15773 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -881,6 +881,8 @@  port_infos_display(portid_t port_id)
 	}
 
 	printf("Minimum size of RX buffer: %u\n", dev_info.min_rx_bufsize);
+	if (dev_info.max_rx_bufsize != UINT32_MAX)
+		printf("Maximum size of RX buffer: %u\n", dev_info.max_rx_bufsize);
 	printf("Maximum configurable length of RX packet: %u\n",
 		dev_info.max_rx_pktlen);
 	printf("Maximum configurable size of LRO aggregated packet: %u\n",