[v4,3/6] app/testpmd: remove restriction on txpkts set
Checks
Commit Message
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>
---
v3 -> v4:
add check 'rte_eth_rx_queue_info_get()' return value and
if it is '-ENOSTUP' calculate the 'ring_size'.
v3: initial version.
---
app/test-pmd/config.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 60 insertions(+), 4 deletions(-)
Comments
Hi, Wei
It was found this patch rejects the --txpkts command line settings.
set_tx_pkt_segments() is called before device started and
we have failure chain:
set_tx_pkt_segments()
nb_segs_is_invalid()
get_tx_ring_size ()
rte_eth_tx_queue_info_get()
It causes --txpkts testpmd command line option is ignored.
With best regards, Slava
> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Wei Hu (Xavier)
> Sent: Friday, September 25, 2020 15:47
> To: dev@dpdk.org
> Cc: xavier.huwei@huawei.com
> Subject: [dpdk-dev] [PATCH v4 3/6] app/testpmd: remove restriction on txpkts
> set
>
> 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>
> ---
> v3 -> v4:
> add check 'rte_eth_rx_queue_info_get()' return value and
> if it is '-ENOSTUP' calculate the 'ring_size'.
> v3: initial version.
> ---
> app/test-pmd/config.c | 64
> +++++++++++++++++++++++++++++++++++++++++++++++----
> 1 file changed, 60 insertions(+), 4 deletions(-)
>
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c index
> 6496d2f..8ebb927 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -1893,6 +1893,38 @@ tx_queue_id_is_invalid(queueid_t txq_id) }
>
> static int
> +get_tx_ring_size(portid_t port_id, queueid_t txq_id, uint16_t
> +*ring_size) {
> + struct rte_port *port = &ports[port_id];
> + struct rte_eth_txq_info tx_qinfo;
> + int ret;
> +
> + ret = rte_eth_tx_queue_info_get(port_id, txq_id, &tx_qinfo);
> + if (ret == 0) {
> + *ring_size = tx_qinfo.nb_desc;
> + return ret;
> + }
> +
> + if (ret != -ENOTSUP)
> + return ret;
> + /*
> + * If the rte_eth_tx_queue_info_get is not support for this PMD,
> + * ring_size stored in testpmd will be used for validity verification.
> + * 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.
> + */
> + if (port->nb_tx_desc[txq_id])
> + *ring_size = port->nb_tx_desc[txq_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;
> + return 0;
> +}
> +
> +static int
> rx_desc_id_is_invalid(uint16_t rxdesc_id) {
> if (rxdesc_id < nb_rxd)
> @@ -2986,17 +3018,41 @@ show_tx_pkt_segments(void)
> printf("Split packet: %s\n", split);
> }
>
> +static bool
> +nb_segs_is_invalid(unsigned int nb_segs) {
> + uint16_t ring_size;
> + uint16_t queue_id;
> + uint16_t port_id;
> + int ret;
> +
> + RTE_ETH_FOREACH_DEV(port_id) {
> + for (queue_id = 0; queue_id < nb_txq; queue_id++) {
> + ret = get_tx_ring_size(port_id, queue_id, &ring_size);
> +
> + if (ret)
> + return true;
> +
> + 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;
> + }
> + }
> + }
> +
> + return false;
> +}
> +
> void
> set_tx_pkt_segments(unsigned *seg_lengths, unsigned nb_segs) {
> uint16_t tx_pkt_len;
> unsigned i;
>
> - if (nb_segs >= (unsigned) nb_txd) {
> - printf("nb segments per TX packets=%u >= nb_txd=%u -
> ignored\n",
> - nb_segs, (unsigned int) nb_txd);
> + if (nb_segs_is_invalid(nb_segs))
> return;
> - }
>
> /*
> * Check that each segment length is greater or equal than
> --
> 2.9.5
Is it OK to keep this regression?
Ferruh, what do you suggest?
23/11/2020 12:50, Slava Ovsiienko:
> Hi, Wei
>
> It was found this patch rejects the --txpkts command line settings.
> set_tx_pkt_segments() is called before device started and
> we have failure chain:
>
> set_tx_pkt_segments()
> nb_segs_is_invalid()
> get_tx_ring_size ()
> rte_eth_tx_queue_info_get()
>
> It causes --txpkts testpmd command line option is ignored.
>
> With best regards, Slava
>
> > -----Original Message-----
> > From: dev <dev-bounces@dpdk.org> On Behalf Of Wei Hu (Xavier)
> > Sent: Friday, September 25, 2020 15:47
> > To: dev@dpdk.org
> > Cc: xavier.huwei@huawei.com
> > Subject: [dpdk-dev] [PATCH v4 3/6] app/testpmd: remove restriction on txpkts
> > set
> >
> > 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>
> > ---
> > v3 -> v4:
> > add check 'rte_eth_rx_queue_info_get()' return value and
> > if it is '-ENOSTUP' calculate the 'ring_size'.
> > v3: initial version.
> > ---
> > app/test-pmd/config.c | 64
> > +++++++++++++++++++++++++++++++++++++++++++++++----
> > 1 file changed, 60 insertions(+), 4 deletions(-)
> >
> > diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c index
> > 6496d2f..8ebb927 100644
> > --- a/app/test-pmd/config.c
> > +++ b/app/test-pmd/config.c
> > @@ -1893,6 +1893,38 @@ tx_queue_id_is_invalid(queueid_t txq_id) }
> >
> > static int
> > +get_tx_ring_size(portid_t port_id, queueid_t txq_id, uint16_t
> > +*ring_size) {
> > + struct rte_port *port = &ports[port_id];
> > + struct rte_eth_txq_info tx_qinfo;
> > + int ret;
> > +
> > + ret = rte_eth_tx_queue_info_get(port_id, txq_id, &tx_qinfo);
> > + if (ret == 0) {
> > + *ring_size = tx_qinfo.nb_desc;
> > + return ret;
> > + }
> > +
> > + if (ret != -ENOTSUP)
> > + return ret;
> > + /*
> > + * If the rte_eth_tx_queue_info_get is not support for this PMD,
> > + * ring_size stored in testpmd will be used for validity verification.
> > + * 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.
> > + */
> > + if (port->nb_tx_desc[txq_id])
> > + *ring_size = port->nb_tx_desc[txq_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;
> > + return 0;
> > +}
> > +
> > +static int
> > rx_desc_id_is_invalid(uint16_t rxdesc_id) {
> > if (rxdesc_id < nb_rxd)
> > @@ -2986,17 +3018,41 @@ show_tx_pkt_segments(void)
> > printf("Split packet: %s\n", split);
> > }
> >
> > +static bool
> > +nb_segs_is_invalid(unsigned int nb_segs) {
> > + uint16_t ring_size;
> > + uint16_t queue_id;
> > + uint16_t port_id;
> > + int ret;
> > +
> > + RTE_ETH_FOREACH_DEV(port_id) {
> > + for (queue_id = 0; queue_id < nb_txq; queue_id++) {
> > + ret = get_tx_ring_size(port_id, queue_id, &ring_size);
> > +
> > + if (ret)
> > + return true;
> > +
> > + 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;
> > + }
> > + }
> > + }
> > +
> > + return false;
> > +}
> > +
> > void
> > set_tx_pkt_segments(unsigned *seg_lengths, unsigned nb_segs) {
> > uint16_t tx_pkt_len;
> > unsigned i;
> >
> > - if (nb_segs >= (unsigned) nb_txd) {
> > - printf("nb segments per TX packets=%u >= nb_txd=%u -
> > ignored\n",
> > - nb_segs, (unsigned int) nb_txd);
> > + if (nb_segs_is_invalid(nb_segs))
> > return;
> > - }
> >
> > /*
> > * Check that each segment length is greater or equal than
> > --
> > 2.9.5
>
>
On 11/24/2020 10:27 AM, Thomas Monjalon wrote:
> Is it OK to keep this regression?
>
> Ferruh, what do you suggest?
>
I confirm the '--txpkts' parameter is broken now, I suggest submitting a defect
for it and continue with the regression.
We have alternative for the parameter, "set txpkts <len0[,len1]*>" command.
The parameter was only working when hardcoded '--txd=N' parameter is provided,
the command is more dynamic and works however queue size is configured.
We can fix the '--txpkts' in next release.
>
> 23/11/2020 12:50, Slava Ovsiienko:
>> Hi, Wei
>>
>> It was found this patch rejects the --txpkts command line settings.
>> set_tx_pkt_segments() is called before device started and
>> we have failure chain:
>>
>> set_tx_pkt_segments()
>> nb_segs_is_invalid()
>> get_tx_ring_size ()
>> rte_eth_tx_queue_info_get()
>>
>> It causes --txpkts testpmd command line option is ignored.
>>
>> With best regards, Slava
>>
>>> -----Original Message-----
>>> From: dev <dev-bounces@dpdk.org> On Behalf Of Wei Hu (Xavier)
>>> Sent: Friday, September 25, 2020 15:47
>>> To: dev@dpdk.org
>>> Cc: xavier.huwei@huawei.com
>>> Subject: [dpdk-dev] [PATCH v4 3/6] app/testpmd: remove restriction on txpkts
>>> set
>>>
>>> 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>
>>> ---
>>> v3 -> v4:
>>> add check 'rte_eth_rx_queue_info_get()' return value and
>>> if it is '-ENOSTUP' calculate the 'ring_size'.
>>> v3: initial version.
>>> ---
>>> app/test-pmd/config.c | 64
>>> +++++++++++++++++++++++++++++++++++++++++++++++----
>>> 1 file changed, 60 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c index
>>> 6496d2f..8ebb927 100644
>>> --- a/app/test-pmd/config.c
>>> +++ b/app/test-pmd/config.c
>>> @@ -1893,6 +1893,38 @@ tx_queue_id_is_invalid(queueid_t txq_id) }
>>>
>>> static int
>>> +get_tx_ring_size(portid_t port_id, queueid_t txq_id, uint16_t
>>> +*ring_size) {
>>> + struct rte_port *port = &ports[port_id];
>>> + struct rte_eth_txq_info tx_qinfo;
>>> + int ret;
>>> +
>>> + ret = rte_eth_tx_queue_info_get(port_id, txq_id, &tx_qinfo);
>>> + if (ret == 0) {
>>> + *ring_size = tx_qinfo.nb_desc;
>>> + return ret;
>>> + }
>>> +
>>> + if (ret != -ENOTSUP)
>>> + return ret;
>>> + /*
>>> + * If the rte_eth_tx_queue_info_get is not support for this PMD,
>>> + * ring_size stored in testpmd will be used for validity verification.
>>> + * 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.
>>> + */
>>> + if (port->nb_tx_desc[txq_id])
>>> + *ring_size = port->nb_tx_desc[txq_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;
>>> + return 0;
>>> +}
>>> +
>>> +static int
>>> rx_desc_id_is_invalid(uint16_t rxdesc_id) {
>>> if (rxdesc_id < nb_rxd)
>>> @@ -2986,17 +3018,41 @@ show_tx_pkt_segments(void)
>>> printf("Split packet: %s\n", split);
>>> }
>>>
>>> +static bool
>>> +nb_segs_is_invalid(unsigned int nb_segs) {
>>> + uint16_t ring_size;
>>> + uint16_t queue_id;
>>> + uint16_t port_id;
>>> + int ret;
>>> +
>>> + RTE_ETH_FOREACH_DEV(port_id) {
>>> + for (queue_id = 0; queue_id < nb_txq; queue_id++) {
>>> + ret = get_tx_ring_size(port_id, queue_id, &ring_size);
>>> +
>>> + if (ret)
>>> + return true;
>>> +
>>> + 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;
>>> + }
>>> + }
>>> + }
>>> +
>>> + return false;
>>> +}
>>> +
>>> void
>>> set_tx_pkt_segments(unsigned *seg_lengths, unsigned nb_segs) {
>>> uint16_t tx_pkt_len;
>>> unsigned i;
>>>
>>> - if (nb_segs >= (unsigned) nb_txd) {
>>> - printf("nb segments per TX packets=%u >= nb_txd=%u -
>>> ignored\n",
>>> - nb_segs, (unsigned int) nb_txd);
>>> + if (nb_segs_is_invalid(nb_segs))
>>> return;
>>> - }
>>>
>>> /*
>>> * Check that each segment length is greater or equal than
>>> --
>>> 2.9.5
>>
>>
>
>
>
>
>
On 24/11/2020 12:23, Ferruh Yigit wrote:
> On 11/24/2020 10:27 AM, Thomas Monjalon wrote:
>> Is it OK to keep this regression?
>>
>> Ferruh, what do you suggest?
>>
>
> I confirm the '--txpkts' parameter is broken now, I suggest submitting a defect
> for it and continue with the regression.
>
> We have alternative for the parameter, "set txpkts <len0[,len1]*>" command.
> The parameter was only working when hardcoded '--txd=N' parameter is provided,
> the command is more dynamic and works however queue size is configured.
>
> We can fix the '--txpkts' in next release.
>
For LTS, I will revert from 18.11 branch as I don't want to add a
regression on last 18.11 LTS release.
+cc Luca for 19.11
>>
>> 23/11/2020 12:50, Slava Ovsiienko:
>>> Hi, Wei
>>>
>>> It was found this patch rejects the --txpkts command line settings.
>>> set_tx_pkt_segments() is called before device started and
>>> we have failure chain:
>>>
>>> set_tx_pkt_segments()
>>> nb_segs_is_invalid()
>>> get_tx_ring_size ()
>>> rte_eth_tx_queue_info_get()
>>>
>>> It causes --txpkts testpmd command line option is ignored.
>>>
>>> With best regards, Slava
>>>
>>>> -----Original Message-----
>>>> From: dev <dev-bounces@dpdk.org> On Behalf Of Wei Hu (Xavier)
>>>> Sent: Friday, September 25, 2020 15:47
>>>> To: dev@dpdk.org
>>>> Cc: xavier.huwei@huawei.com
>>>> Subject: [dpdk-dev] [PATCH v4 3/6] app/testpmd: remove restriction on txpkts
>>>> set
>>>>
>>>> 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>
>>>> ---
>>>> v3 -> v4:
>>>> add check 'rte_eth_rx_queue_info_get()' return value and
>>>> if it is '-ENOSTUP' calculate the 'ring_size'.
>>>> v3: initial version.
>>>> ---
>>>> app/test-pmd/config.c | 64
>>>> +++++++++++++++++++++++++++++++++++++++++++++++----
>>>> 1 file changed, 60 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c index
>>>> 6496d2f..8ebb927 100644
>>>> --- a/app/test-pmd/config.c
>>>> +++ b/app/test-pmd/config.c
>>>> @@ -1893,6 +1893,38 @@ tx_queue_id_is_invalid(queueid_t txq_id) }
>>>>
>>>> static int
>>>> +get_tx_ring_size(portid_t port_id, queueid_t txq_id, uint16_t
>>>> +*ring_size) {
>>>> + struct rte_port *port = &ports[port_id];
>>>> + struct rte_eth_txq_info tx_qinfo;
>>>> + int ret;
>>>> +
>>>> + ret = rte_eth_tx_queue_info_get(port_id, txq_id, &tx_qinfo);
>>>> + if (ret == 0) {
>>>> + *ring_size = tx_qinfo.nb_desc;
>>>> + return ret;
>>>> + }
>>>> +
>>>> + if (ret != -ENOTSUP)
>>>> + return ret;
>>>> + /*
>>>> + * If the rte_eth_tx_queue_info_get is not support for this PMD,
>>>> + * ring_size stored in testpmd will be used for validity verification.
>>>> + * 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.
>>>> + */
>>>> + if (port->nb_tx_desc[txq_id])
>>>> + *ring_size = port->nb_tx_desc[txq_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;
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static int
>>>> rx_desc_id_is_invalid(uint16_t rxdesc_id) {
>>>> if (rxdesc_id < nb_rxd)
>>>> @@ -2986,17 +3018,41 @@ show_tx_pkt_segments(void)
>>>> printf("Split packet: %s\n", split);
>>>> }
>>>>
>>>> +static bool
>>>> +nb_segs_is_invalid(unsigned int nb_segs) {
>>>> + uint16_t ring_size;
>>>> + uint16_t queue_id;
>>>> + uint16_t port_id;
>>>> + int ret;
>>>> +
>>>> + RTE_ETH_FOREACH_DEV(port_id) {
>>>> + for (queue_id = 0; queue_id < nb_txq; queue_id++) {
>>>> + ret = get_tx_ring_size(port_id, queue_id, &ring_size);
>>>> +
>>>> + if (ret)
>>>> + return true;
>>>> +
>>>> + 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;
>>>> + }
>>>> + }
>>>> + }
>>>> +
>>>> + return false;
>>>> +}
>>>> +
>>>> void
>>>> set_tx_pkt_segments(unsigned *seg_lengths, unsigned nb_segs) {
>>>> uint16_t tx_pkt_len;
>>>> unsigned i;
>>>>
>>>> - if (nb_segs >= (unsigned) nb_txd) {
>>>> - printf("nb segments per TX packets=%u >= nb_txd=%u -
>>>> ignored\n",
>>>> - nb_segs, (unsigned int) nb_txd);
>>>> + if (nb_segs_is_invalid(nb_segs))
>>>> return;
>>>> - }
>>>>
>>>> /*
>>>> * Check that each segment length is greater or equal than
>>>> --
>>>> 2.9.5
>>>
>>>
>>
>>
>>
>>
>>
>
On 11/24/2020 12:23 PM, Ferruh Yigit wrote:
> On 11/24/2020 10:27 AM, Thomas Monjalon wrote:
>> Is it OK to keep this regression?
>>
>> Ferruh, what do you suggest?
>>
>
> I confirm the '--txpkts' parameter is broken now, I suggest submitting a defect
> for it and continue with the regression.
>
Hi Slava,
Can you please submit the Bugzilla defect?
Thanks,
ferruh
> We have alternative for the parameter, "set txpkts <len0[,len1]*>" command.
> The parameter was only working when hardcoded '--txd=N' parameter is provided,
> the command is more dynamic and works however queue size is configured.
>
> We can fix the '--txpkts' in next release.
>
>>
>> 23/11/2020 12:50, Slava Ovsiienko:
>>> Hi, Wei
>>>
>>> It was found this patch rejects the --txpkts command line settings.
>>> set_tx_pkt_segments() is called before device started and
>>> we have failure chain:
>>>
>>> set_tx_pkt_segments()
>>> nb_segs_is_invalid()
>>> get_tx_ring_size ()
>>> rte_eth_tx_queue_info_get()
>>>
>>> It causes --txpkts testpmd command line option is ignored.
>>>
>>> With best regards, Slava
>>>
>>>> -----Original Message-----
>>>> From: dev <dev-bounces@dpdk.org> On Behalf Of Wei Hu (Xavier)
>>>> Sent: Friday, September 25, 2020 15:47
>>>> To: dev@dpdk.org
>>>> Cc: xavier.huwei@huawei.com
>>>> Subject: [dpdk-dev] [PATCH v4 3/6] app/testpmd: remove restriction on txpkts
>>>> set
>>>>
>>>> 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>
>>>> ---
>>>> v3 -> v4:
>>>> add check 'rte_eth_rx_queue_info_get()' return value and
>>>> if it is '-ENOSTUP' calculate the 'ring_size'.
>>>> v3: initial version.
>>>> ---
>>>> app/test-pmd/config.c | 64
>>>> +++++++++++++++++++++++++++++++++++++++++++++++----
>>>> 1 file changed, 60 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c index
>>>> 6496d2f..8ebb927 100644
>>>> --- a/app/test-pmd/config.c
>>>> +++ b/app/test-pmd/config.c
>>>> @@ -1893,6 +1893,38 @@ tx_queue_id_is_invalid(queueid_t txq_id) }
>>>>
>>>> static int
>>>> +get_tx_ring_size(portid_t port_id, queueid_t txq_id, uint16_t
>>>> +*ring_size) {
>>>> + struct rte_port *port = &ports[port_id];
>>>> + struct rte_eth_txq_info tx_qinfo;
>>>> + int ret;
>>>> +
>>>> + ret = rte_eth_tx_queue_info_get(port_id, txq_id, &tx_qinfo);
>>>> + if (ret == 0) {
>>>> + *ring_size = tx_qinfo.nb_desc;
>>>> + return ret;
>>>> + }
>>>> +
>>>> + if (ret != -ENOTSUP)
>>>> + return ret;
>>>> + /*
>>>> + * If the rte_eth_tx_queue_info_get is not support for this PMD,
>>>> + * ring_size stored in testpmd will be used for validity verification.
>>>> + * 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.
>>>> + */
>>>> + if (port->nb_tx_desc[txq_id])
>>>> + *ring_size = port->nb_tx_desc[txq_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;
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static int
>>>> rx_desc_id_is_invalid(uint16_t rxdesc_id) {
>>>> if (rxdesc_id < nb_rxd)
>>>> @@ -2986,17 +3018,41 @@ show_tx_pkt_segments(void)
>>>> printf("Split packet: %s\n", split);
>>>> }
>>>>
>>>> +static bool
>>>> +nb_segs_is_invalid(unsigned int nb_segs) {
>>>> + uint16_t ring_size;
>>>> + uint16_t queue_id;
>>>> + uint16_t port_id;
>>>> + int ret;
>>>> +
>>>> + RTE_ETH_FOREACH_DEV(port_id) {
>>>> + for (queue_id = 0; queue_id < nb_txq; queue_id++) {
>>>> + ret = get_tx_ring_size(port_id, queue_id, &ring_size);
>>>> +
>>>> + if (ret)
>>>> + return true;
>>>> +
>>>> + 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;
>>>> + }
>>>> + }
>>>> + }
>>>> +
>>>> + return false;
>>>> +}
>>>> +
>>>> void
>>>> set_tx_pkt_segments(unsigned *seg_lengths, unsigned nb_segs) {
>>>> uint16_t tx_pkt_len;
>>>> unsigned i;
>>>>
>>>> - if (nb_segs >= (unsigned) nb_txd) {
>>>> - printf("nb segments per TX packets=%u >= nb_txd=%u -
>>>> ignored\n",
>>>> - nb_segs, (unsigned int) nb_txd);
>>>> + if (nb_segs_is_invalid(nb_segs))
>>>> return;
>>>> - }
>>>>
>>>> /*
>>>> * Check that each segment length is greater or equal than
>>>> --
>>>> 2.9.5
>>>
>>>
>>
>>
>>
>>
>>
>
The bug: https://bugs.dpdk.org/show_bug.cgi?id=584
Can we pass the nb_segs = 1 always?
One descriptor is minimal basic capability to send, it should be always supported.
With best regards, Slava
> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Wednesday, November 25, 2020 16:07
> To: NBU-Contact-Thomas Monjalon <thomas@monjalon.net>; Wei Hu (Xavier)
> <huwei013@chinasoftinc.com>
> Cc: dev@dpdk.org; xavier.huwei@huawei.com; Slava Ovsiienko
> <viacheslavo@nvidia.com>
> Subject: Re: [dpdk-dev] [PATCH v4 3/6] app/testpmd: remove restriction on
> txpkts set
>
> On 11/24/2020 12:23 PM, Ferruh Yigit wrote:
> > On 11/24/2020 10:27 AM, Thomas Monjalon wrote:
> >> Is it OK to keep this regression?
> >>
> >> Ferruh, what do you suggest?
> >>
> >
> > I confirm the '--txpkts' parameter is broken now, I suggest submitting
> > a defect for it and continue with the regression.
> >
>
> Hi Slava,
>
> Can you please submit the Bugzilla defect?
>
> Thanks,
> ferruh
>
>
> > We have alternative for the parameter, "set txpkts <len0[,len1]*>" command.
> > The parameter was only working when hardcoded '--txd=N' parameter is
> > provided, the command is more dynamic and works however queue size is
> configured.
> >
> > We can fix the '--txpkts' in next release.
> >
> >>
> >> 23/11/2020 12:50, Slava Ovsiienko:
> >>> Hi, Wei
> >>>
> >>> It was found this patch rejects the --txpkts command line settings.
> >>> set_tx_pkt_segments() is called before device started and we have
> >>> failure chain:
> >>>
> >>> set_tx_pkt_segments()
> >>> nb_segs_is_invalid()
> >>> get_tx_ring_size ()
> >>> rte_eth_tx_queue_info_get()
> >>>
> >>> It causes --txpkts testpmd command line option is ignored.
> >>>
> >>> With best regards, Slava
> >>>
> >>>> -----Original Message-----
> >>>> From: dev <dev-bounces@dpdk.org> On Behalf Of Wei Hu (Xavier)
> >>>> Sent: Friday, September 25, 2020 15:47
> >>>> To: dev@dpdk.org
> >>>> Cc: xavier.huwei@huawei.com
> >>>> Subject: [dpdk-dev] [PATCH v4 3/6] app/testpmd: remove restriction
> >>>> on txpkts set
> >>>>
> >>>> 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>
> >>>> ---
> >>>> v3 -> v4:
> >>>> add check 'rte_eth_rx_queue_info_get()' return value and
> >>>> if it is '-ENOSTUP' calculate the 'ring_size'.
> >>>> v3: initial version.
> >>>> ---
> >>>> app/test-pmd/config.c | 64
> >>>> +++++++++++++++++++++++++++++++++++++++++++++++----
> >>>> 1 file changed, 60 insertions(+), 4 deletions(-)
> >>>>
> >>>> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c index
> >>>> 6496d2f..8ebb927 100644
> >>>> --- a/app/test-pmd/config.c
> >>>> +++ b/app/test-pmd/config.c
> >>>> @@ -1893,6 +1893,38 @@ tx_queue_id_is_invalid(queueid_t txq_id) }
> >>>>
> >>>> static int
> >>>> +get_tx_ring_size(portid_t port_id, queueid_t txq_id, uint16_t
> >>>> +*ring_size) {
> >>>> + struct rte_port *port = &ports[port_id];
> >>>> + struct rte_eth_txq_info tx_qinfo;
> >>>> + int ret;
> >>>> +
> >>>> + ret = rte_eth_tx_queue_info_get(port_id, txq_id, &tx_qinfo);
> >>>> + if (ret == 0) {
> >>>> + *ring_size = tx_qinfo.nb_desc;
> >>>> + return ret;
> >>>> + }
> >>>> +
> >>>> + if (ret != -ENOTSUP)
> >>>> + return ret;
> >>>> + /*
> >>>> + * If the rte_eth_tx_queue_info_get is not support for this
> >>>> +PMD,
> >>>> + * ring_size stored in testpmd will be used for validity verification.
> >>>> + * 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.
> >>>> + */
> >>>> + if (port->nb_tx_desc[txq_id])
> >>>> + *ring_size = port->nb_tx_desc[txq_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;
> >>>> + return 0;
> >>>> +}
> >>>> +
> >>>> +static int
> >>>> rx_desc_id_is_invalid(uint16_t rxdesc_id) {
> >>>> if (rxdesc_id < nb_rxd)
> >>>> @@ -2986,17 +3018,41 @@ show_tx_pkt_segments(void)
> >>>> printf("Split packet: %s\n", split);
> >>>> }
> >>>>
> >>>> +static bool
> >>>> +nb_segs_is_invalid(unsigned int nb_segs) {
> >>>> + uint16_t ring_size;
> >>>> + uint16_t queue_id;
> >>>> + uint16_t port_id;
> >>>> + int ret;
> >>>> +
> >>>> + RTE_ETH_FOREACH_DEV(port_id) {
> >>>> + for (queue_id = 0; queue_id < nb_txq; queue_id++) {
> >>>> + ret = get_tx_ring_size(port_id, queue_id, &ring_size);
> >>>> +
> >>>> + if (ret)
> >>>> + return true;
> >>>> +
> >>>> + 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;
> >>>> + }
> >>>> + }
> >>>> + }
> >>>> +
> >>>> + return false;
> >>>> +}
> >>>> +
> >>>> void
> >>>> set_tx_pkt_segments(unsigned *seg_lengths, unsigned nb_segs) {
> >>>> uint16_t tx_pkt_len;
> >>>> unsigned i;
> >>>>
> >>>> - if (nb_segs >= (unsigned) nb_txd) {
> >>>> - printf("nb segments per TX packets=%u >= nb_txd=%u -
> >>>> ignored\n",
> >>>> - nb_segs, (unsigned int) nb_txd);
> >>>> + if (nb_segs_is_invalid(nb_segs))
> >>>> return;
> >>>> - }
> >>>>
> >>>> /*
> >>>> * Check that each segment length is greater or equal than
> >>>> --
> >>>> 2.9.5
> >>>
> >>>
> >>
> >>
> >>
> >>
> >>
> >
On 11/26/2020 7:24 AM, Slava Ovsiienko wrote:
> The bug: https://bugs.dpdk.org/show_bug.cgi?id=584
>
> Can we pass the nb_segs = 1 always?
> One descriptor is minimal basic capability to send, it should be always supported.
>
Hi Slava,
I didn't get your comment, can you please elaborate?
> With best regards, Slava
>
>> -----Original Message-----
>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>> Sent: Wednesday, November 25, 2020 16:07
>> To: NBU-Contact-Thomas Monjalon <thomas@monjalon.net>; Wei Hu (Xavier)
>> <huwei013@chinasoftinc.com>
>> Cc: dev@dpdk.org; xavier.huwei@huawei.com; Slava Ovsiienko
>> <viacheslavo@nvidia.com>
>> Subject: Re: [dpdk-dev] [PATCH v4 3/6] app/testpmd: remove restriction on
>> txpkts set
>>
>> On 11/24/2020 12:23 PM, Ferruh Yigit wrote:
>>> On 11/24/2020 10:27 AM, Thomas Monjalon wrote:
>>>> Is it OK to keep this regression?
>>>>
>>>> Ferruh, what do you suggest?
>>>>
>>>
>>> I confirm the '--txpkts' parameter is broken now, I suggest submitting
>>> a defect for it and continue with the regression.
>>>
>>
>> Hi Slava,
>>
>> Can you please submit the Bugzilla defect?
>>
>> Thanks,
>> ferruh
>>
>>
>>> We have alternative for the parameter, "set txpkts <len0[,len1]*>" command.
>>> The parameter was only working when hardcoded '--txd=N' parameter is
>>> provided, the command is more dynamic and works however queue size is
>> configured.
>>>
>>> We can fix the '--txpkts' in next release.
>>>
>>>>
>>>> 23/11/2020 12:50, Slava Ovsiienko:
>>>>> Hi, Wei
>>>>>
>>>>> It was found this patch rejects the --txpkts command line settings.
>>>>> set_tx_pkt_segments() is called before device started and we have
>>>>> failure chain:
>>>>>
>>>>> set_tx_pkt_segments()
>>>>> nb_segs_is_invalid()
>>>>> get_tx_ring_size ()
>>>>> rte_eth_tx_queue_info_get()
>>>>>
>>>>> It causes --txpkts testpmd command line option is ignored.
>>>>>
>>>>> With best regards, Slava
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: dev <dev-bounces@dpdk.org> On Behalf Of Wei Hu (Xavier)
>>>>>> Sent: Friday, September 25, 2020 15:47
>>>>>> To: dev@dpdk.org
>>>>>> Cc: xavier.huwei@huawei.com
>>>>>> Subject: [dpdk-dev] [PATCH v4 3/6] app/testpmd: remove restriction
>>>>>> on txpkts set
>>>>>>
>>>>>> 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>
>>>>>> ---
>>>>>> v3 -> v4:
>>>>>> add check 'rte_eth_rx_queue_info_get()' return value and
>>>>>> if it is '-ENOSTUP' calculate the 'ring_size'.
>>>>>> v3: initial version.
>>>>>> ---
>>>>>> app/test-pmd/config.c | 64
>>>>>> +++++++++++++++++++++++++++++++++++++++++++++++----
>>>>>> 1 file changed, 60 insertions(+), 4 deletions(-)
>>>>>>
>>>>>> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c index
>>>>>> 6496d2f..8ebb927 100644
>>>>>> --- a/app/test-pmd/config.c
>>>>>> +++ b/app/test-pmd/config.c
>>>>>> @@ -1893,6 +1893,38 @@ tx_queue_id_is_invalid(queueid_t txq_id) }
>>>>>>
>>>>>> static int
>>>>>> +get_tx_ring_size(portid_t port_id, queueid_t txq_id, uint16_t
>>>>>> +*ring_size) {
>>>>>> + struct rte_port *port = &ports[port_id];
>>>>>> + struct rte_eth_txq_info tx_qinfo;
>>>>>> + int ret;
>>>>>> +
>>>>>> + ret = rte_eth_tx_queue_info_get(port_id, txq_id, &tx_qinfo);
>>>>>> + if (ret == 0) {
>>>>>> + *ring_size = tx_qinfo.nb_desc;
>>>>>> + return ret;
>>>>>> + }
>>>>>> +
>>>>>> + if (ret != -ENOTSUP)
>>>>>> + return ret;
>>>>>> + /*
>>>>>> + * If the rte_eth_tx_queue_info_get is not support for this
>>>>>> +PMD,
>>>>>> + * ring_size stored in testpmd will be used for validity verification.
>>>>>> + * 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.
>>>>>> + */
>>>>>> + if (port->nb_tx_desc[txq_id])
>>>>>> + *ring_size = port->nb_tx_desc[txq_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;
>>>>>> + return 0;
>>>>>> +}
>>>>>> +
>>>>>> +static int
>>>>>> rx_desc_id_is_invalid(uint16_t rxdesc_id) {
>>>>>> if (rxdesc_id < nb_rxd)
>>>>>> @@ -2986,17 +3018,41 @@ show_tx_pkt_segments(void)
>>>>>> printf("Split packet: %s\n", split);
>>>>>> }
>>>>>>
>>>>>> +static bool
>>>>>> +nb_segs_is_invalid(unsigned int nb_segs) {
>>>>>> + uint16_t ring_size;
>>>>>> + uint16_t queue_id;
>>>>>> + uint16_t port_id;
>>>>>> + int ret;
>>>>>> +
>>>>>> + RTE_ETH_FOREACH_DEV(port_id) {
>>>>>> + for (queue_id = 0; queue_id < nb_txq; queue_id++) {
>>>>>> + ret = get_tx_ring_size(port_id, queue_id, &ring_size);
>>>>>> +
>>>>>> + if (ret)
>>>>>> + return true;
>>>>>> +
>>>>>> + 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;
>>>>>> + }
>>>>>> + }
>>>>>> + }
>>>>>> +
>>>>>> + return false;
>>>>>> +}
>>>>>> +
>>>>>> void
>>>>>> set_tx_pkt_segments(unsigned *seg_lengths, unsigned nb_segs) {
>>>>>> uint16_t tx_pkt_len;
>>>>>> unsigned i;
>>>>>>
>>>>>> - if (nb_segs >= (unsigned) nb_txd) {
>>>>>> - printf("nb segments per TX packets=%u >= nb_txd=%u -
>>>>>> ignored\n",
>>>>>> - nb_segs, (unsigned int) nb_txd);
>>>>>> + if (nb_segs_is_invalid(nb_segs))
>>>>>> return;
>>>>>> - }
>>>>>>
>>>>>> /*
>>>>>> * Check that each segment length is greater or equal than
>>>>>> --
>>>>>> 2.9.5
>>>>>
>>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>
>
> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Thursday, November 26, 2020 14:38
> To: Slava Ovsiienko <viacheslavo@nvidia.com>; NBU-Contact-Thomas
> Monjalon <thomas@monjalon.net>; Wei Hu (Xavier)
> <huwei013@chinasoftinc.com>
> Cc: dev@dpdk.org; xavier.huwei@huawei.com
> Subject: Re: [dpdk-dev] [PATCH v4 3/6] app/testpmd: remove restriction on
> txpkts set
>
> On 11/26/2020 7:24 AM, Slava Ovsiienko wrote:
> > The bug:
> > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs
> >
> .dpdk.org%2Fshow_bug.cgi%3Fid%3D584&data=04%7C01%7Cviacheslavo
> %40n
> >
> vidia.com%7Ce52ba5bbab184ac8592808d8920842c5%7C43083d15727340c1b7
> db39e
> >
> fd9ccc17a%7C0%7C0%7C637419911462011700%7CUnknown%7CTWFpbGZsb3
> d8eyJWIjo
> >
> iMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000
> &
> >
> ;sdata=QBB67WqEjUHgwqHNjqx2VLdaTRMzMeodh%2B%2FVFsHByQg%3D&am
> p;reserved
> > =0
> >
> > Can we pass the nb_segs = 1 always?
> > One descriptor is minimal basic capability to send, it should be always
> supported.
> >
>
> Hi Slava,
>
> I didn't get your comment, can you please elaborate?
>
The --txpkts is rejected on testpmd startup due to port is not configured yet
and we can't find out how many descriptors are actually configured
in the Tx queues.
Configuring Tx queues with zero descriptors seems to be meaningless,
it would disable a basic capability to send the packets. And we could assume
the single segment packet sending is always supported.
If --txpkts sets only the size for the single segment we can assume that the
packets with only one segment is going to be sent, and we could ignore
the Tx queue descriptor number check for the case.
With best regards, Slava
>
> > With best regards, Slava
> >
> >> -----Original Message-----
> >> From: Ferruh Yigit <ferruh.yigit@intel.com>
> >> Sent: Wednesday, November 25, 2020 16:07
> >> To: NBU-Contact-Thomas Monjalon <thomas@monjalon.net>; Wei Hu
> >> (Xavier) <huwei013@chinasoftinc.com>
> >> Cc: dev@dpdk.org; xavier.huwei@huawei.com; Slava Ovsiienko
> >> <viacheslavo@nvidia.com>
> >> Subject: Re: [dpdk-dev] [PATCH v4 3/6] app/testpmd: remove
> >> restriction on txpkts set
> >>
> >> On 11/24/2020 12:23 PM, Ferruh Yigit wrote:
> >>> On 11/24/2020 10:27 AM, Thomas Monjalon wrote:
> >>>> Is it OK to keep this regression?
> >>>>
> >>>> Ferruh, what do you suggest?
> >>>>
> >>>
> >>> I confirm the '--txpkts' parameter is broken now, I suggest
> >>> submitting a defect for it and continue with the regression.
> >>>
> >>
> >> Hi Slava,
> >>
> >> Can you please submit the Bugzilla defect?
> >>
> >> Thanks,
> >> ferruh
> >>
> >>
> >>> We have alternative for the parameter, "set txpkts <len0[,len1]*>"
> command.
> >>> The parameter was only working when hardcoded '--txd=N' parameter is
> >>> provided, the command is more dynamic and works however queue size
> >>> is
> >> configured.
> >>>
> >>> We can fix the '--txpkts' in next release.
> >>>
> >>>>
> >>>> 23/11/2020 12:50, Slava Ovsiienko:
> >>>>> Hi, Wei
> >>>>>
> >>>>> It was found this patch rejects the --txpkts command line settings.
> >>>>> set_tx_pkt_segments() is called before device started and we have
> >>>>> failure chain:
> >>>>>
> >>>>> set_tx_pkt_segments()
> >>>>> nb_segs_is_invalid()
> >>>>> get_tx_ring_size ()
> >>>>> rte_eth_tx_queue_info_get()
> >>>>>
> >>>>> It causes --txpkts testpmd command line option is ignored.
> >>>>>
> >>>>> With best regards, Slava
> >>>>>
> >>>>>> -----Original Message-----
> >>>>>> From: dev <dev-bounces@dpdk.org> On Behalf Of Wei Hu (Xavier)
> >>>>>> Sent: Friday, September 25, 2020 15:47
> >>>>>> To: dev@dpdk.org
> >>>>>> Cc: xavier.huwei@huawei.com
> >>>>>> Subject: [dpdk-dev] [PATCH v4 3/6] app/testpmd: remove
> >>>>>> restriction on txpkts set
> >>>>>>
> >>>>>> 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>
> >>>>>> ---
> >>>>>> v3 -> v4:
> >>>>>> add check 'rte_eth_rx_queue_info_get()' return value and
> >>>>>> if it is '-ENOSTUP' calculate the 'ring_size'.
> >>>>>> v3: initial version.
> >>>>>> ---
> >>>>>> app/test-pmd/config.c | 64
> >>>>>> +++++++++++++++++++++++++++++++++++++++++++++++----
> >>>>>> 1 file changed, 60 insertions(+), 4 deletions(-)
> >>>>>>
> >>>>>> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c index
> >>>>>> 6496d2f..8ebb927 100644
> >>>>>> --- a/app/test-pmd/config.c
> >>>>>> +++ b/app/test-pmd/config.c
> >>>>>> @@ -1893,6 +1893,38 @@ tx_queue_id_is_invalid(queueid_t txq_id)
> >>>>>> }
> >>>>>>
> >>>>>> static int
> >>>>>> +get_tx_ring_size(portid_t port_id, queueid_t txq_id, uint16_t
> >>>>>> +*ring_size) {
> >>>>>> + struct rte_port *port = &ports[port_id];
> >>>>>> + struct rte_eth_txq_info tx_qinfo;
> >>>>>> + int ret;
> >>>>>> +
> >>>>>> + ret = rte_eth_tx_queue_info_get(port_id, txq_id, &tx_qinfo);
> >>>>>> + if (ret == 0) {
> >>>>>> + *ring_size = tx_qinfo.nb_desc;
> >>>>>> + return ret;
> >>>>>> + }
> >>>>>> +
> >>>>>> + if (ret != -ENOTSUP)
> >>>>>> + return ret;
> >>>>>> + /*
> >>>>>> + * If the rte_eth_tx_queue_info_get is not support for this
> >>>>>> +PMD,
> >>>>>> + * ring_size stored in testpmd will be used for validity verification.
> >>>>>> + * 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.
> >>>>>> + */
> >>>>>> + if (port->nb_tx_desc[txq_id])
> >>>>>> + *ring_size = port->nb_tx_desc[txq_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;
> >>>>>> + return 0;
> >>>>>> +}
> >>>>>> +
> >>>>>> +static int
> >>>>>> rx_desc_id_is_invalid(uint16_t rxdesc_id) {
> >>>>>> if (rxdesc_id < nb_rxd)
> >>>>>> @@ -2986,17 +3018,41 @@ show_tx_pkt_segments(void)
> >>>>>> printf("Split packet: %s\n", split);
> >>>>>> }
> >>>>>>
> >>>>>> +static bool
> >>>>>> +nb_segs_is_invalid(unsigned int nb_segs) {
> >>>>>> + uint16_t ring_size;
> >>>>>> + uint16_t queue_id;
> >>>>>> + uint16_t port_id;
> >>>>>> + int ret;
> >>>>>> +
> >>>>>> + RTE_ETH_FOREACH_DEV(port_id) {
> >>>>>> + for (queue_id = 0; queue_id < nb_txq; queue_id++) {
> >>>>>> + ret = get_tx_ring_size(port_id, queue_id,
> >>>>>> +&ring_size);
> >>>>>> +
> >>>>>> + if (ret)
> >>>>>> + return true;
> >>>>>> +
> >>>>>> + 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;
> >>>>>> + }
> >>>>>> + }
> >>>>>> + }
> >>>>>> +
> >>>>>> + return false;
> >>>>>> +}
> >>>>>> +
> >>>>>> void
> >>>>>> set_tx_pkt_segments(unsigned *seg_lengths, unsigned nb_segs)
> >>>>>> {
> >>>>>> uint16_t tx_pkt_len;
> >>>>>> unsigned i;
> >>>>>>
> >>>>>> - if (nb_segs >= (unsigned) nb_txd) {
> >>>>>> - printf("nb segments per TX packets=%u >= nb_txd=%u -
> >>>>>> ignored\n",
> >>>>>> - nb_segs, (unsigned int) nb_txd);
> >>>>>> + if (nb_segs_is_invalid(nb_segs))
> >>>>>> return;
> >>>>>> - }
> >>>>>>
> >>>>>> /*
> >>>>>> * Check that each segment length is greater or equal than
> >>>>>> --
> >>>>>> 2.9.5
> >>>>>
> >>>>>
> >>>>
> >>>>
> >>>>
> >>>>
> >>>>
> >>>
> >
On 11/27/2020 1:05 PM, Slava Ovsiienko wrote:
>> -----Original Message-----
>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>> Sent: Thursday, November 26, 2020 14:38
>> To: Slava Ovsiienko <viacheslavo@nvidia.com>; NBU-Contact-Thomas
>> Monjalon <thomas@monjalon.net>; Wei Hu (Xavier)
>> <huwei013@chinasoftinc.com>
>> Cc: dev@dpdk.org; xavier.huwei@huawei.com
>> Subject: Re: [dpdk-dev] [PATCH v4 3/6] app/testpmd: remove restriction on
>> txpkts set
>>
>> On 11/26/2020 7:24 AM, Slava Ovsiienko wrote:
>>> The bug:
>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs
>>>
>> .dpdk.org%2Fshow_bug.cgi%3Fid%3D584&data=04%7C01%7Cviacheslavo
>> %40n
>>>
>> vidia.com%7Ce52ba5bbab184ac8592808d8920842c5%7C43083d15727340c1b7
>> db39e
>>>
>> fd9ccc17a%7C0%7C0%7C637419911462011700%7CUnknown%7CTWFpbGZsb3
>> d8eyJWIjo
>>>
>> iMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000
>> &
>>>
>> ;sdata=QBB67WqEjUHgwqHNjqx2VLdaTRMzMeodh%2B%2FVFsHByQg%3D&am
>> p;reserved
>>> =0
>>>
>>> Can we pass the nb_segs = 1 always?
>>> One descriptor is minimal basic capability to send, it should be always
>> supported.
>>>
>>
>> Hi Slava,
>>
>> I didn't get your comment, can you please elaborate?
>>
> The --txpkts is rejected on testpmd startup due to port is not configured yet
> and we can't find out how many descriptors are actually configured
> in the Tx queues.
>
> Configuring Tx queues with zero descriptors seems to be meaningless,
> it would disable a basic capability to send the packets. And we could assume
> the single segment packet sending is always supported.
>
> If --txpkts sets only the size for the single segment we can assume that the
> packets with only one segment is going to be sent, and we could ignore
> the Tx queue descriptor number check for the case.
>
Overall I was OK to remove the check completely, even multi segment used it is
very unlikely that number of segments will be more than descriptor size.
But at least OK to ignore the check for single segment, also we can force
'--txd' parameter provided to enable '--txpkts', like done before.
> With best regards, Slava
>
>
>>
>>> With best regards, Slava
>>>
>>>> -----Original Message-----
>>>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>>>> Sent: Wednesday, November 25, 2020 16:07
>>>> To: NBU-Contact-Thomas Monjalon <thomas@monjalon.net>; Wei Hu
>>>> (Xavier) <huwei013@chinasoftinc.com>
>>>> Cc: dev@dpdk.org; xavier.huwei@huawei.com; Slava Ovsiienko
>>>> <viacheslavo@nvidia.com>
>>>> Subject: Re: [dpdk-dev] [PATCH v4 3/6] app/testpmd: remove
>>>> restriction on txpkts set
>>>>
>>>> On 11/24/2020 12:23 PM, Ferruh Yigit wrote:
>>>>> On 11/24/2020 10:27 AM, Thomas Monjalon wrote:
>>>>>> Is it OK to keep this regression?
>>>>>>
>>>>>> Ferruh, what do you suggest?
>>>>>>
>>>>>
>>>>> I confirm the '--txpkts' parameter is broken now, I suggest
>>>>> submitting a defect for it and continue with the regression.
>>>>>
>>>>
>>>> Hi Slava,
>>>>
>>>> Can you please submit the Bugzilla defect?
>>>>
>>>> Thanks,
>>>> ferruh
>>>>
>>>>
>>>>> We have alternative for the parameter, "set txpkts <len0[,len1]*>"
>> command.
>>>>> The parameter was only working when hardcoded '--txd=N' parameter is
>>>>> provided, the command is more dynamic and works however queue size
>>>>> is
>>>> configured.
>>>>>
>>>>> We can fix the '--txpkts' in next release.
>>>>>
>>>>>>
>>>>>> 23/11/2020 12:50, Slava Ovsiienko:
>>>>>>> Hi, Wei
>>>>>>>
>>>>>>> It was found this patch rejects the --txpkts command line settings.
>>>>>>> set_tx_pkt_segments() is called before device started and we have
>>>>>>> failure chain:
>>>>>>>
>>>>>>> set_tx_pkt_segments()
>>>>>>> nb_segs_is_invalid()
>>>>>>> get_tx_ring_size ()
>>>>>>> rte_eth_tx_queue_info_get()
>>>>>>>
>>>>>>> It causes --txpkts testpmd command line option is ignored.
>>>>>>>
>>>>>>> With best regards, Slava
>>>>>>>
>>>>>>>> -----Original Message-----
>>>>>>>> From: dev <dev-bounces@dpdk.org> On Behalf Of Wei Hu (Xavier)
>>>>>>>> Sent: Friday, September 25, 2020 15:47
>>>>>>>> To: dev@dpdk.org
>>>>>>>> Cc: xavier.huwei@huawei.com
>>>>>>>> Subject: [dpdk-dev] [PATCH v4 3/6] app/testpmd: remove
>>>>>>>> restriction on txpkts set
>>>>>>>>
>>>>>>>> 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>
>>>>>>>> ---
>>>>>>>> v3 -> v4:
>>>>>>>> add check 'rte_eth_rx_queue_info_get()' return value and
>>>>>>>> if it is '-ENOSTUP' calculate the 'ring_size'.
>>>>>>>> v3: initial version.
>>>>>>>> ---
>>>>>>>> app/test-pmd/config.c | 64
>>>>>>>> +++++++++++++++++++++++++++++++++++++++++++++++----
>>>>>>>> 1 file changed, 60 insertions(+), 4 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c index
>>>>>>>> 6496d2f..8ebb927 100644
>>>>>>>> --- a/app/test-pmd/config.c
>>>>>>>> +++ b/app/test-pmd/config.c
>>>>>>>> @@ -1893,6 +1893,38 @@ tx_queue_id_is_invalid(queueid_t txq_id)
>>>>>>>> }
>>>>>>>>
>>>>>>>> static int
>>>>>>>> +get_tx_ring_size(portid_t port_id, queueid_t txq_id, uint16_t
>>>>>>>> +*ring_size) {
>>>>>>>> + struct rte_port *port = &ports[port_id];
>>>>>>>> + struct rte_eth_txq_info tx_qinfo;
>>>>>>>> + int ret;
>>>>>>>> +
>>>>>>>> + ret = rte_eth_tx_queue_info_get(port_id, txq_id, &tx_qinfo);
>>>>>>>> + if (ret == 0) {
>>>>>>>> + *ring_size = tx_qinfo.nb_desc;
>>>>>>>> + return ret;
>>>>>>>> + }
>>>>>>>> +
>>>>>>>> + if (ret != -ENOTSUP)
>>>>>>>> + return ret;
>>>>>>>> + /*
>>>>>>>> + * If the rte_eth_tx_queue_info_get is not support for this
>>>>>>>> +PMD,
>>>>>>>> + * ring_size stored in testpmd will be used for validity verification.
>>>>>>>> + * 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.
>>>>>>>> + */
>>>>>>>> + if (port->nb_tx_desc[txq_id])
>>>>>>>> + *ring_size = port->nb_tx_desc[txq_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;
>>>>>>>> + return 0;
>>>>>>>> +}
>>>>>>>> +
>>>>>>>> +static int
>>>>>>>> rx_desc_id_is_invalid(uint16_t rxdesc_id) {
>>>>>>>> if (rxdesc_id < nb_rxd)
>>>>>>>> @@ -2986,17 +3018,41 @@ show_tx_pkt_segments(void)
>>>>>>>> printf("Split packet: %s\n", split);
>>>>>>>> }
>>>>>>>>
>>>>>>>> +static bool
>>>>>>>> +nb_segs_is_invalid(unsigned int nb_segs) {
>>>>>>>> + uint16_t ring_size;
>>>>>>>> + uint16_t queue_id;
>>>>>>>> + uint16_t port_id;
>>>>>>>> + int ret;
>>>>>>>> +
>>>>>>>> + RTE_ETH_FOREACH_DEV(port_id) {
>>>>>>>> + for (queue_id = 0; queue_id < nb_txq; queue_id++) {
>>>>>>>> + ret = get_tx_ring_size(port_id, queue_id,
>>>>>>>> +&ring_size);
>>>>>>>> +
>>>>>>>> + if (ret)
>>>>>>>> + return true;
>>>>>>>> +
>>>>>>>> + 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;
>>>>>>>> + }
>>>>>>>> + }
>>>>>>>> + }
>>>>>>>> +
>>>>>>>> + return false;
>>>>>>>> +}
>>>>>>>> +
>>>>>>>> void
>>>>>>>> set_tx_pkt_segments(unsigned *seg_lengths, unsigned nb_segs)
>>>>>>>> {
>>>>>>>> uint16_t tx_pkt_len;
>>>>>>>> unsigned i;
>>>>>>>>
>>>>>>>> - if (nb_segs >= (unsigned) nb_txd) {
>>>>>>>> - printf("nb segments per TX packets=%u >= nb_txd=%u -
>>>>>>>> ignored\n",
>>>>>>>> - nb_segs, (unsigned int) nb_txd);
>>>>>>>> + if (nb_segs_is_invalid(nb_segs))
>>>>>>>> return;
>>>>>>>> - }
>>>>>>>>
>>>>>>>> /*
>>>>>>>> * Check that each segment length is greater or equal than
>>>>>>>> --
>>>>>>>> 2.9.5
>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>
>
Hi, Ferruh
> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Wednesday, December 2, 2020 14:07
> To: Slava Ovsiienko <viacheslavo@nvidia.com>; NBU-Contact-Thomas
> Monjalon <thomas@monjalon.net>; Wei Hu (Xavier)
> <huwei013@chinasoftinc.com>
> Cc: dev@dpdk.org; xavier.huwei@huawei.com
> Subject: Re: [dpdk-dev] [PATCH v4 3/6] app/testpmd: remove restriction on
> txpkts set
>
> On 11/27/2020 1:05 PM, Slava Ovsiienko wrote:
> >> -----Original Message-----
> >> From: Ferruh Yigit <ferruh.yigit@intel.com>
> >> Sent: Thursday, November 26, 2020 14:38
> >> To: Slava Ovsiienko <viacheslavo@nvidia.com>; NBU-Contact-Thomas
> >> Monjalon <thomas@monjalon.net>; Wei Hu (Xavier)
> >> <huwei013@chinasoftinc.com>
> >> Cc: dev@dpdk.org; xavier.huwei@huawei.com
> >> Subject: Re: [dpdk-dev] [PATCH v4 3/6] app/testpmd: remove
> >> restriction on txpkts set
> >>
> >> On 11/26/2020 7:24 AM, Slava Ovsiienko wrote:
> >>> The bug:
> >>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbu
> >>> gs
> >>>
> >>
> .dpdk.org%2Fshow_bug.cgi%3Fid%3D584&data=04%7C01%7Cviacheslavo
> >> %40n
> >>>
> >>
> vidia.com%7Ce52ba5bbab184ac8592808d8920842c5%7C43083d15727340c1b7
> >> db39e
> >>>
> >>
> fd9ccc17a%7C0%7C0%7C637419911462011700%7CUnknown%7CTWFpbGZsb3
> >> d8eyJWIjo
> >>>
> >>
> iMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000
> >> &
> >>>
> >>
> ;sdata=QBB67WqEjUHgwqHNjqx2VLdaTRMzMeodh%2B%2FVFsHByQg%3D&am
> >> p;reserved
> >>> =0
> >>>
> >>> Can we pass the nb_segs = 1 always?
> >>> One descriptor is minimal basic capability to send, it should be
> >>> always
> >> supported.
> >>>
> >>
> >> Hi Slava,
> >>
> >> I didn't get your comment, can you please elaborate?
> >>
> > The --txpkts is rejected on testpmd startup due to port is not
> > configured yet and we can't find out how many descriptors are actually
> > configured in the Tx queues.
> >
> > Configuring Tx queues with zero descriptors seems to be meaningless,
> > it would disable a basic capability to send the packets. And we could
> > assume the single segment packet sending is always supported.
> >
> > If --txpkts sets only the size for the single segment we can assume
> > that the packets with only one segment is going to be sent, and we
> > could ignore the Tx queue descriptor number check for the case.
> >
>
> Overall I was OK to remove the check completely, even multi segment used it
> is very unlikely that number of segments will be more than descriptor size.
>
> But at least OK to ignore the check for single segment, also we can force '--txd'
> parameter provided to enable '--txpkts', like done before.
OK, I'll provide the patch taking both approaches on testpmd startup:
- if --txd is specified the check will be done against it, failed check for non-configured port will be ignored
- if there is the only one segment specified in txpkts, failed check for non-configured port will be ignored
With best regards, Slava
> >
> >
> >>
> >>> With best regards, Slava
> >>>
> >>>> -----Original Message-----
> >>>> From: Ferruh Yigit <ferruh.yigit@intel.com>
> >>>> Sent: Wednesday, November 25, 2020 16:07
> >>>> To: NBU-Contact-Thomas Monjalon <thomas@monjalon.net>; Wei Hu
> >>>> (Xavier) <huwei013@chinasoftinc.com>
> >>>> Cc: dev@dpdk.org; xavier.huwei@huawei.com; Slava Ovsiienko
> >>>> <viacheslavo@nvidia.com>
> >>>> Subject: Re: [dpdk-dev] [PATCH v4 3/6] app/testpmd: remove
> >>>> restriction on txpkts set
> >>>>
> >>>> On 11/24/2020 12:23 PM, Ferruh Yigit wrote:
> >>>>> On 11/24/2020 10:27 AM, Thomas Monjalon wrote:
> >>>>>> Is it OK to keep this regression?
> >>>>>>
> >>>>>> Ferruh, what do you suggest?
> >>>>>>
> >>>>>
> >>>>> I confirm the '--txpkts' parameter is broken now, I suggest
> >>>>> submitting a defect for it and continue with the regression.
> >>>>>
> >>>>
> >>>> Hi Slava,
> >>>>
> >>>> Can you please submit the Bugzilla defect?
> >>>>
> >>>> Thanks,
> >>>> ferruh
> >>>>
> >>>>
> >>>>> We have alternative for the parameter, "set txpkts <len0[,len1]*>"
> >> command.
> >>>>> The parameter was only working when hardcoded '--txd=N' parameter is
> >>>>> provided, the command is more dynamic and works however queue size
> >>>>> is
> >>>> configured.
> >>>>>
> >>>>> We can fix the '--txpkts' in next release.
> >>>>>
> >>>>>>
> >>>>>> 23/11/2020 12:50, Slava Ovsiienko:
> >>>>>>> Hi, Wei
> >>>>>>>
> >>>>>>> It was found this patch rejects the --txpkts command line settings.
> >>>>>>> set_tx_pkt_segments() is called before device started and we have
> >>>>>>> failure chain:
> >>>>>>>
> >>>>>>> set_tx_pkt_segments()
> >>>>>>> nb_segs_is_invalid()
> >>>>>>> get_tx_ring_size ()
> >>>>>>> rte_eth_tx_queue_info_get()
> >>>>>>>
> >>>>>>> It causes --txpkts testpmd command line option is ignored.
> >>>>>>>
> >>>>>>> With best regards, Slava
> >>>>>>>
> >>>>>>>> -----Original Message-----
> >>>>>>>> From: dev <dev-bounces@dpdk.org> On Behalf Of Wei Hu (Xavier)
> >>>>>>>> Sent: Friday, September 25, 2020 15:47
> >>>>>>>> To: dev@dpdk.org
> >>>>>>>> Cc: xavier.huwei@huawei.com
> >>>>>>>> Subject: [dpdk-dev] [PATCH v4 3/6] app/testpmd: remove
> >>>>>>>> restriction on txpkts set
> >>>>>>>>
> >>>>>>>> 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>
> >>>>>>>> ---
> >>>>>>>> v3 -> v4:
> >>>>>>>> add check 'rte_eth_rx_queue_info_get()' return value and
> >>>>>>>> if it is '-ENOSTUP' calculate the 'ring_size'.
> >>>>>>>> v3: initial version.
> >>>>>>>> ---
> >>>>>>>> app/test-pmd/config.c | 64
> >>>>>>>> +++++++++++++++++++++++++++++++++++++++++++++++----
> >>>>>>>> 1 file changed, 60 insertions(+), 4 deletions(-)
> >>>>>>>>
> >>>>>>>> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c index
> >>>>>>>> 6496d2f..8ebb927 100644
> >>>>>>>> --- a/app/test-pmd/config.c
> >>>>>>>> +++ b/app/test-pmd/config.c
> >>>>>>>> @@ -1893,6 +1893,38 @@ tx_queue_id_is_invalid(queueid_t
> txq_id)
> >>>>>>>> }
> >>>>>>>>
> >>>>>>>> static int
> >>>>>>>> +get_tx_ring_size(portid_t port_id, queueid_t txq_id, uint16_t
> >>>>>>>> +*ring_size) {
> >>>>>>>> + struct rte_port *port = &ports[port_id];
> >>>>>>>> + struct rte_eth_txq_info tx_qinfo;
> >>>>>>>> + int ret;
> >>>>>>>> +
> >>>>>>>> + ret = rte_eth_tx_queue_info_get(port_id, txq_id, &tx_qinfo);
> >>>>>>>> + if (ret == 0) {
> >>>>>>>> + *ring_size = tx_qinfo.nb_desc;
> >>>>>>>> + return ret;
> >>>>>>>> + }
> >>>>>>>> +
> >>>>>>>> + if (ret != -ENOTSUP)
> >>>>>>>> + return ret;
> >>>>>>>> + /*
> >>>>>>>> + * If the rte_eth_tx_queue_info_get is not support for this
> >>>>>>>> +PMD,
> >>>>>>>> + * ring_size stored in testpmd will be used for validity
> verification.
> >>>>>>>> + * 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.
> >>>>>>>> + */
> >>>>>>>> + if (port->nb_tx_desc[txq_id])
> >>>>>>>> + *ring_size = port->nb_tx_desc[txq_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;
> >>>>>>>> + return 0;
> >>>>>>>> +}
> >>>>>>>> +
> >>>>>>>> +static int
> >>>>>>>> rx_desc_id_is_invalid(uint16_t rxdesc_id) {
> >>>>>>>> if (rxdesc_id < nb_rxd)
> >>>>>>>> @@ -2986,17 +3018,41 @@ show_tx_pkt_segments(void)
> >>>>>>>> printf("Split packet: %s\n", split);
> >>>>>>>> }
> >>>>>>>>
> >>>>>>>> +static bool
> >>>>>>>> +nb_segs_is_invalid(unsigned int nb_segs) {
> >>>>>>>> + uint16_t ring_size;
> >>>>>>>> + uint16_t queue_id;
> >>>>>>>> + uint16_t port_id;
> >>>>>>>> + int ret;
> >>>>>>>> +
> >>>>>>>> + RTE_ETH_FOREACH_DEV(port_id) {
> >>>>>>>> + for (queue_id = 0; queue_id < nb_txq; queue_id++) {
> >>>>>>>> + ret = get_tx_ring_size(port_id, queue_id,
> >>>>>>>> +&ring_size);
> >>>>>>>> +
> >>>>>>>> + if (ret)
> >>>>>>>> + return true;
> >>>>>>>> +
> >>>>>>>> + 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;
> >>>>>>>> + }
> >>>>>>>> + }
> >>>>>>>> + }
> >>>>>>>> +
> >>>>>>>> + return false;
> >>>>>>>> +}
> >>>>>>>> +
> >>>>>>>> void
> >>>>>>>> set_tx_pkt_segments(unsigned *seg_lengths, unsigned nb_segs)
> >>>>>>>> {
> >>>>>>>> uint16_t tx_pkt_len;
> >>>>>>>> unsigned i;
> >>>>>>>>
> >>>>>>>> - if (nb_segs >= (unsigned) nb_txd) {
> >>>>>>>> - printf("nb segments per TX packets=%u >= nb_txd=%u -
> >>>>>>>> ignored\n",
> >>>>>>>> - nb_segs, (unsigned int) nb_txd);
> >>>>>>>> + if (nb_segs_is_invalid(nb_segs))
> >>>>>>>> return;
> >>>>>>>> - }
> >>>>>>>>
> >>>>>>>> /*
> >>>>>>>> * Check that each segment length is greater or equal than
> >>>>>>>> --
> >>>>>>>> 2.9.5
> >>>>>>>
> >>>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>
> >>>
> >
On 12/3/2020 9:45 AM, Slava Ovsiienko wrote:
> Hi, Ferruh
>
>> -----Original Message-----
>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>> Sent: Wednesday, December 2, 2020 14:07
>> To: Slava Ovsiienko <viacheslavo@nvidia.com>; NBU-Contact-Thomas
>> Monjalon <thomas@monjalon.net>; Wei Hu (Xavier)
>> <huwei013@chinasoftinc.com>
>> Cc: dev@dpdk.org; xavier.huwei@huawei.com
>> Subject: Re: [dpdk-dev] [PATCH v4 3/6] app/testpmd: remove restriction on
>> txpkts set
>>
>> On 11/27/2020 1:05 PM, Slava Ovsiienko wrote:
>>>> -----Original Message-----
>>>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>>>> Sent: Thursday, November 26, 2020 14:38
>>>> To: Slava Ovsiienko <viacheslavo@nvidia.com>; NBU-Contact-Thomas
>>>> Monjalon <thomas@monjalon.net>; Wei Hu (Xavier)
>>>> <huwei013@chinasoftinc.com>
>>>> Cc: dev@dpdk.org; xavier.huwei@huawei.com
>>>> Subject: Re: [dpdk-dev] [PATCH v4 3/6] app/testpmd: remove
>>>> restriction on txpkts set
>>>>
>>>> On 11/26/2020 7:24 AM, Slava Ovsiienko wrote:
>>>>> The bug:
>>>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbu
>>>>> gs
>>>>>
>>>>
>> .dpdk.org%2Fshow_bug.cgi%3Fid%3D584&data=04%7C01%7Cviacheslavo
>>>> %40n
>>>>>
>>>>
>> vidia.com%7Ce52ba5bbab184ac8592808d8920842c5%7C43083d15727340c1b7
>>>> db39e
>>>>>
>>>>
>> fd9ccc17a%7C0%7C0%7C637419911462011700%7CUnknown%7CTWFpbGZsb3
>>>> d8eyJWIjo
>>>>>
>>>>
>> iMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000
>>>> &
>>>>>
>>>>
>> ;sdata=QBB67WqEjUHgwqHNjqx2VLdaTRMzMeodh%2B%2FVFsHByQg%3D&am
>>>> p;reserved
>>>>> =0
>>>>>
>>>>> Can we pass the nb_segs = 1 always?
>>>>> One descriptor is minimal basic capability to send, it should be
>>>>> always
>>>> supported.
>>>>>
>>>>
>>>> Hi Slava,
>>>>
>>>> I didn't get your comment, can you please elaborate?
>>>>
>>> The --txpkts is rejected on testpmd startup due to port is not
>>> configured yet and we can't find out how many descriptors are actually
>>> configured in the Tx queues.
>>>
>>> Configuring Tx queues with zero descriptors seems to be meaningless,
>>> it would disable a basic capability to send the packets. And we could
>>> assume the single segment packet sending is always supported.
>>>
>>> If --txpkts sets only the size for the single segment we can assume
>>> that the packets with only one segment is going to be sent, and we
>>> could ignore the Tx queue descriptor number check for the case.
>>>
>>
>> Overall I was OK to remove the check completely, even multi segment used it
>> is very unlikely that number of segments will be more than descriptor size.
>>
>> But at least OK to ignore the check for single segment, also we can force '--txd'
>> parameter provided to enable '--txpkts', like done before.
>
> OK, I'll provide the patch taking both approaches on testpmd startup:
> - if --txd is specified the check will be done against it, failed check for non-configured port will be ignored
> - if there is the only one segment specified in txpkts, failed check for non-configured port will be ignored
>
Sounds good, thank you.
> With best regards, Slava
>
>>>
>>>
>>>>
>>>>> With best regards, Slava
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>>>>>> Sent: Wednesday, November 25, 2020 16:07
>>>>>> To: NBU-Contact-Thomas Monjalon <thomas@monjalon.net>; Wei Hu
>>>>>> (Xavier) <huwei013@chinasoftinc.com>
>>>>>> Cc: dev@dpdk.org; xavier.huwei@huawei.com; Slava Ovsiienko
>>>>>> <viacheslavo@nvidia.com>
>>>>>> Subject: Re: [dpdk-dev] [PATCH v4 3/6] app/testpmd: remove
>>>>>> restriction on txpkts set
>>>>>>
>>>>>> On 11/24/2020 12:23 PM, Ferruh Yigit wrote:
>>>>>>> On 11/24/2020 10:27 AM, Thomas Monjalon wrote:
>>>>>>>> Is it OK to keep this regression?
>>>>>>>>
>>>>>>>> Ferruh, what do you suggest?
>>>>>>>>
>>>>>>>
>>>>>>> I confirm the '--txpkts' parameter is broken now, I suggest
>>>>>>> submitting a defect for it and continue with the regression.
>>>>>>>
>>>>>>
>>>>>> Hi Slava,
>>>>>>
>>>>>> Can you please submit the Bugzilla defect?
>>>>>>
>>>>>> Thanks,
>>>>>> ferruh
>>>>>>
>>>>>>
>>>>>>> We have alternative for the parameter, "set txpkts <len0[,len1]*>"
>>>> command.
>>>>>>> The parameter was only working when hardcoded '--txd=N' parameter is
>>>>>>> provided, the command is more dynamic and works however queue size
>>>>>>> is
>>>>>> configured.
>>>>>>>
>>>>>>> We can fix the '--txpkts' in next release.
>>>>>>>
>>>>>>>>
>>>>>>>> 23/11/2020 12:50, Slava Ovsiienko:
>>>>>>>>> Hi, Wei
>>>>>>>>>
>>>>>>>>> It was found this patch rejects the --txpkts command line settings.
>>>>>>>>> set_tx_pkt_segments() is called before device started and we have
>>>>>>>>> failure chain:
>>>>>>>>>
>>>>>>>>> set_tx_pkt_segments()
>>>>>>>>> nb_segs_is_invalid()
>>>>>>>>> get_tx_ring_size ()
>>>>>>>>> rte_eth_tx_queue_info_get()
>>>>>>>>>
>>>>>>>>> It causes --txpkts testpmd command line option is ignored.
>>>>>>>>>
>>>>>>>>> With best regards, Slava
>>>>>>>>>
>>>>>>>>>> -----Original Message-----
>>>>>>>>>> From: dev <dev-bounces@dpdk.org> On Behalf Of Wei Hu (Xavier)
>>>>>>>>>> Sent: Friday, September 25, 2020 15:47
>>>>>>>>>> To: dev@dpdk.org
>>>>>>>>>> Cc: xavier.huwei@huawei.com
>>>>>>>>>> Subject: [dpdk-dev] [PATCH v4 3/6] app/testpmd: remove
>>>>>>>>>> restriction on txpkts set
>>>>>>>>>>
>>>>>>>>>> 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>
>>>>>>>>>> ---
>>>>>>>>>> v3 -> v4:
>>>>>>>>>> add check 'rte_eth_rx_queue_info_get()' return value and
>>>>>>>>>> if it is '-ENOSTUP' calculate the 'ring_size'.
>>>>>>>>>> v3: initial version.
>>>>>>>>>> ---
>>>>>>>>>> app/test-pmd/config.c | 64
>>>>>>>>>> +++++++++++++++++++++++++++++++++++++++++++++++----
>>>>>>>>>> 1 file changed, 60 insertions(+), 4 deletions(-)
>>>>>>>>>>
>>>>>>>>>> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c index
>>>>>>>>>> 6496d2f..8ebb927 100644
>>>>>>>>>> --- a/app/test-pmd/config.c
>>>>>>>>>> +++ b/app/test-pmd/config.c
>>>>>>>>>> @@ -1893,6 +1893,38 @@ tx_queue_id_is_invalid(queueid_t
>> txq_id)
>>>>>>>>>> }
>>>>>>>>>>
>>>>>>>>>> static int
>>>>>>>>>> +get_tx_ring_size(portid_t port_id, queueid_t txq_id, uint16_t
>>>>>>>>>> +*ring_size) {
>>>>>>>>>> + struct rte_port *port = &ports[port_id];
>>>>>>>>>> + struct rte_eth_txq_info tx_qinfo;
>>>>>>>>>> + int ret;
>>>>>>>>>> +
>>>>>>>>>> + ret = rte_eth_tx_queue_info_get(port_id, txq_id, &tx_qinfo);
>>>>>>>>>> + if (ret == 0) {
>>>>>>>>>> + *ring_size = tx_qinfo.nb_desc;
>>>>>>>>>> + return ret;
>>>>>>>>>> + }
>>>>>>>>>> +
>>>>>>>>>> + if (ret != -ENOTSUP)
>>>>>>>>>> + return ret;
>>>>>>>>>> + /*
>>>>>>>>>> + * If the rte_eth_tx_queue_info_get is not support for this
>>>>>>>>>> +PMD,
>>>>>>>>>> + * ring_size stored in testpmd will be used for validity
>> verification.
>>>>>>>>>> + * 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.
>>>>>>>>>> + */
>>>>>>>>>> + if (port->nb_tx_desc[txq_id])
>>>>>>>>>> + *ring_size = port->nb_tx_desc[txq_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;
>>>>>>>>>> + return 0;
>>>>>>>>>> +}
>>>>>>>>>> +
>>>>>>>>>> +static int
>>>>>>>>>> rx_desc_id_is_invalid(uint16_t rxdesc_id) {
>>>>>>>>>> if (rxdesc_id < nb_rxd)
>>>>>>>>>> @@ -2986,17 +3018,41 @@ show_tx_pkt_segments(void)
>>>>>>>>>> printf("Split packet: %s\n", split);
>>>>>>>>>> }
>>>>>>>>>>
>>>>>>>>>> +static bool
>>>>>>>>>> +nb_segs_is_invalid(unsigned int nb_segs) {
>>>>>>>>>> + uint16_t ring_size;
>>>>>>>>>> + uint16_t queue_id;
>>>>>>>>>> + uint16_t port_id;
>>>>>>>>>> + int ret;
>>>>>>>>>> +
>>>>>>>>>> + RTE_ETH_FOREACH_DEV(port_id) {
>>>>>>>>>> + for (queue_id = 0; queue_id < nb_txq; queue_id++) {
>>>>>>>>>> + ret = get_tx_ring_size(port_id, queue_id,
>>>>>>>>>> +&ring_size);
>>>>>>>>>> +
>>>>>>>>>> + if (ret)
>>>>>>>>>> + return true;
>>>>>>>>>> +
>>>>>>>>>> + 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;
>>>>>>>>>> + }
>>>>>>>>>> + }
>>>>>>>>>> + }
>>>>>>>>>> +
>>>>>>>>>> + return false;
>>>>>>>>>> +}
>>>>>>>>>> +
>>>>>>>>>> void
>>>>>>>>>> set_tx_pkt_segments(unsigned *seg_lengths, unsigned nb_segs)
>>>>>>>>>> {
>>>>>>>>>> uint16_t tx_pkt_len;
>>>>>>>>>> unsigned i;
>>>>>>>>>>
>>>>>>>>>> - if (nb_segs >= (unsigned) nb_txd) {
>>>>>>>>>> - printf("nb segments per TX packets=%u >= nb_txd=%u -
>>>>>>>>>> ignored\n",
>>>>>>>>>> - nb_segs, (unsigned int) nb_txd);
>>>>>>>>>> + if (nb_segs_is_invalid(nb_segs))
>>>>>>>>>> return;
>>>>>>>>>> - }
>>>>>>>>>>
>>>>>>>>>> /*
>>>>>>>>>> * Check that each segment length is greater or equal than
>>>>>>>>>> --
>>>>>>>>>> 2.9.5
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>
>>>
>
@@ -1893,6 +1893,38 @@ tx_queue_id_is_invalid(queueid_t txq_id)
}
static int
+get_tx_ring_size(portid_t port_id, queueid_t txq_id, uint16_t *ring_size)
+{
+ struct rte_port *port = &ports[port_id];
+ struct rte_eth_txq_info tx_qinfo;
+ int ret;
+
+ ret = rte_eth_tx_queue_info_get(port_id, txq_id, &tx_qinfo);
+ if (ret == 0) {
+ *ring_size = tx_qinfo.nb_desc;
+ return ret;
+ }
+
+ if (ret != -ENOTSUP)
+ return ret;
+ /*
+ * If the rte_eth_tx_queue_info_get is not support for this PMD,
+ * ring_size stored in testpmd will be used for validity verification.
+ * 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.
+ */
+ if (port->nb_tx_desc[txq_id])
+ *ring_size = port->nb_tx_desc[txq_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;
+ return 0;
+}
+
+static int
rx_desc_id_is_invalid(uint16_t rxdesc_id)
{
if (rxdesc_id < nb_rxd)
@@ -2986,17 +3018,41 @@ show_tx_pkt_segments(void)
printf("Split packet: %s\n", split);
}
+static bool
+nb_segs_is_invalid(unsigned int nb_segs)
+{
+ uint16_t ring_size;
+ uint16_t queue_id;
+ uint16_t port_id;
+ int ret;
+
+ RTE_ETH_FOREACH_DEV(port_id) {
+ for (queue_id = 0; queue_id < nb_txq; queue_id++) {
+ ret = get_tx_ring_size(port_id, queue_id, &ring_size);
+
+ if (ret)
+ return true;
+
+ 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;
+ }
+ }
+ }
+
+ return false;
+}
+
void
set_tx_pkt_segments(unsigned *seg_lengths, unsigned nb_segs)
{
uint16_t tx_pkt_len;
unsigned i;
- if (nb_segs >= (unsigned) nb_txd) {
- printf("nb segments per TX packets=%u >= nb_txd=%u - ignored\n",
- nb_segs, (unsigned int) nb_txd);
+ if (nb_segs_is_invalid(nb_segs))
return;
- }
/*
* Check that each segment length is greater or equal than