On 12/11/2020 3:07 PM, Viacheslav Ovsiienko wrote:
The --txpkts command line parameter was silently ignored due to
application was unable to check the Tx queue ring sizes for non
configured ports [1].
The "set txpkts <len0[,len1]*>" was also rejected if there
was some stopped or /unconfigured port.
May not be for the commit log but to understand the problem here,
what are the steps to reproduce the problem in "set txpkts" command?
This provides the following:
- number of segment check is performed against
configured Tx queues only
- the capability to send single packet is supposed to
be very basic and always supported, the setting segment
number to 1 is always allowed, no check performed
- at the moment of Tx queue setup the descriptor number is
checked against configured segment number
Not sure about this one, more comments below.
Fixes: 8dae835d88b7 ("app/testpmd: remove restriction on Tx segments set")
Cc: sta...@dpdk.org
Bugzilla ID: 584
Signed-off-by: Viacheslav Ovsiienko <viachesl...@nvidia.com>
---
app/test-pmd/cmdline.c | 5 +++++
app/test-pmd/config.c | 21 ++++++++++++++++-----
app/test-pmd/testpmd.c | 7 ++++++-
3 files changed, 27 insertions(+), 6 deletions(-)
diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 0d2d6aa..86388a2 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -2798,6 +2798,11 @@ struct cmd_setup_rxtx_queue {
if (!numa_support || socket_id == NUMA_NO_CONFIG)
socket_id = port->socket_id;
+ if (port->nb_tx_desc[res->qid] < tx_pkt_nb_segs) {
+ printf("Failed to setup TX queue: "
+ "not enough descriptors\n");
+ return;
+ }
The "port->nb_tx_desc[res->qid]" can be '0', and that is the default value in
the testpmd, in that case device suggested values are used. Above check fails
when '--txd' arguments is not provided.
Same problem with the same check below in 'start_port()', I think both can be
removed.
ret = rte_eth_tx_queue_setup(res->portid,
res->qid,
port->nb_tx_desc[res->qid],
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index b51de59..a6fccfa 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -3911,12 +3911,18 @@ struct igb_ring_desc_16_bytes {
for (queue_id = 0; queue_id < nb_txq; queue_id++) {
ret = get_tx_ring_size(port_id, queue_id, &ring_size);
- if (ret)
+ /* Do the check only for the active/configured ports. */
+ if (ret == -EINVAL)
+ continue;
+ if (ret) {
+ printf("failed to get ring size for TX "
+ "queue(%u) Port(%u) - txpkts ignored\n",
+ port_id, queue_id);
return true;
Is there a need to filter the '-EINVAL' errors only, the other error this
function can get is '-EINVAL' & '-ENODEV' (which is 'port_id' is not valid), to
simplify the logic, what do you think just ignore any kind of error and not fail
in that case?
-
+ }
if (ring_size < nb_segs) {
- printf("nb segments per TX packets=%u >= "
- "TX queue(%u) ring_size=%u - ignored\n",
+ printf("nb segments per TX packets=%u >= TX "
+ "queue(%u) ring_size=%u - txpkts
ignored\n",
nb_segs, queue_id, ring_size);
return true;
}
@@ -3932,7 +3938,12 @@ struct igb_ring_desc_16_bytes {
uint16_t tx_pkt_len;
unsigned int i;
- if (nb_segs_is_invalid(nb_segs))
+ /*
+ * For single sengment settings failed check is ignored.
+ * It is a very basic capability to send the single segment
+ * packets, suppose it is always supported.
+ */
+ if (nb_segs > 1 && nb_segs_is_invalid(nb_segs))
return;
if the user provided '--txd' in the command line, we can compare the segment
size with that without going into the device configured values, as similar to
what is done before:
At the very beginning of the 'get_tx_ring_size()':
if (nb_txd) {
*ring_size = nb_txd;
return 0;
}
So following combination of parameters will be supported
"--txpkts=X,Y --txd=Z" (segment size checked against nb_txd)
"--txpkts=N " (single segment, no check)
"--txpkts=X,Y" (segment size not checked)
And setting same in the command line always should be supported with segment
size checks against the
- nb_txd when '--txd=Z' provided
- dynamic device provided values when '--txd=Z' not provided
/*
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 33fc0fd..9ea0145 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -2575,6 +2575,11 @@ struct extmem_param {
port->need_reconfig_queues = 0;
/* setup tx queues */
for (qi = 0; qi < nb_txq; qi++) {
+ if (port->nb_tx_desc[qi] < tx_pkt_nb_segs) {
+ printf("Failed to setup TX queue: "
+ "not enough descriptors\n");
+ goto fail;
+ }
Please check above comment.
if ((numa_support) &&
(txring_numa[pi] != NUMA_NO_CONFIG))
diag = rte_eth_tx_queue_setup(pi, qi,
@@ -2589,7 +2594,7 @@ struct extmem_param {
if (diag == 0)
continue;
-
+fail:
/* Fail to setup tx queue, return */
if (rte_atomic16_cmpset(&(port->port_status),
RTE_PORT_HANDLING,
