Re: [dpdk-dev] [PATCH v3 3/6] app/testpmd: remove restriction on txpkts set

["Wei Hu (Xavier)" <huwei013@chinasoftinc.com>]

Hi, Ferruh Yigit

On 2020/9/23 11:14, Wei Hu (Xavier) wrote:
> Hi, Ferruh Yigit
> 
> On 2020/9/22 22:51, Ferruh Yigit wrote:
>> On 9/19/2020 11:47 AM, Wei Hu (Xavier) wrote:
>>> From: Chengchang Tang <tangchengchang@huawei.com>
>>>
>>> Currently, if nb_txd is not set, the txpkts is not allowed to be set
>>> because the nb_txd is used to avoid the numer of segments exceed the Tx
>>> ring size and the default value of nb_txd is 0. And there is a bug that
>>> nb_txd is the global configuration for Tx ring size and the ring size
>>> could be changed by some command per queue. So these valid check is
>>> unreliable and introduced unnecessary constraints.
>>>
>>> This patch adds a valid check function to use the real Tx ring size to
>>> check the validity of txpkts.
>>>
>>> Fixes: af75078fece3 ("first public release")
>>> Cc: stable@dpdk.org
>>>
>>> Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
>>> Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
>>> ---
>>>   app/test-pmd/config.c | 42 ++++++++++++++++++++++++++++++++++++++----
>>>   1 file changed, 38 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
>>> index 4e33208..882de2d 100644
>>> --- a/app/test-pmd/config.c
>>> +++ b/app/test-pmd/config.c
>>> @@ -2984,17 +2984,51 @@ show_tx_pkt_segments(void)
>>>       printf("Split packet: %s\n", split);
>>>   }
>>>   +static bool
>>> +nb_segs_is_invalid(unsigned int nb_segs)
>>> +{
>>> +    uint16_t port_id;
>>> +
>>> +    RTE_ETH_FOREACH_DEV(port_id) {
>>> +        struct rte_port *port = &ports[port_id];
>>> +        uint16_t ring_size;
>>> +        uint16_t queue_id;
>>> +
>>> +        /*
>>> +         * When configure the txq by rte_eth_tx_queue_setup with
>>> +         * nb_tx_desc being 0, it will use a default value provided by
>>> +         * PMDs to setup this txq. If the default value is 0, it will
>>> +         * use the RTE_ETH_DEV_FALLBACK_TX_RINGSIZE to setup this txq.
>>> +         */
>>> +        for (queue_id = 0; queue_id < nb_txq; queue_id++) {
>>> +            if (port->nb_tx_desc[queue_id])
>>> +                ring_size = port->nb_tx_desc[queue_id];
>>> +            else if (port->dev_info.default_txportconf.ring_size)
>>> +                ring_size =
>>> + port->dev_info.default_txportconf.ring_size;
>>> +            else
>>> +                ring_size = RTE_ETH_DEV_FALLBACK_TX_RINGSIZE;
>>> +
>>> +            if (ring_size < nb_segs) {
>>> +                printf("nb segments per TX packets=%u >= TX "
>>> +                       "queue(%u) ring_size=%u - ignored\n",
>>> +                    nb_segs, queue_id, ring_size);
>>> +                return true;
>>> +            }
>>> +        }
>>
>> What do you think calling 'rte_eth_rx_queue_info_get()' & 
>> 'rte_eth_tx_queue_info_get()' to get the 'nb_desc'?
> 
> Currently not all PMD driver implement the .rxq_info_get and
> 
> .txq_info_get hook function. If calling rte_eth_rx_queue_info_get
> 
> return -ENOSTUP, we still need to obtain the ring_size in this way.
> 
If calling rte_eth_rx_queue_info_get function get ring_size, because not 
all PMDS implement the relevant the related hook function, we need to
check the return value and if the return value is -ENOSTUP, we must 
obtain the ring_size in this way.
Do you prefer this method, right?

Hopes for your suggestion.
Thanks, Xavier
> 
> Thanks, Xavier