DPDK patches and discussions
 help / color / mirror / Atom feed
From: Andrew Boyer <aboyer@pensando.io>
To: Viacheslav Ovsiienko <viacheslavo@nvidia.com>
Cc: dev@dpdk.org, thomas@monjalon.net, ferruh.yigit@intel.com,
	stable@dpdk.org
Subject: Re: [dpdk-dev] [PATCH] app/testpmd: fix segment number check
Date: Fri, 11 Dec 2020 11:00:20 -0500
Message-ID: <40F5C8DC-8461-43E4-BF37-16FB1D4CA990@pensando.io> (raw)
In-Reply-To: <1607699265-5238-1-git-send-email-viacheslavo@nvidia.com>



> On Dec 11, 2020, at 10:07 AM, Viacheslav Ovsiienko <viacheslavo@nvidia.com> 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].

... ignored because the application...

> The "set txpkts <len0[,len1]*>" was also rejected if there
> was some stopped or /unconfigured port.

... was a stopped or unconfigured ...

> 
> 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
> 
> Fixes: 8dae835d88b7 ("app/testpmd: remove restriction on Tx segments set")
> Cc: stable@dpdk.org
> Bugzilla ID: 584
> 
> Signed-off-by: Viacheslav Ovsiienko <viacheslavo@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: "

setup -> set up
I find it helpful when the numbers are logged in the error message.  Like “nb_desc 8 < nb_segs 16”.

> +			       "not enough descriptors\n");
> +			return;
> +		}

Why is there a relationship between the number of descriptors and the number of segments? For our device, there isn’t. We can send 16 Tx segments per descriptor and (I suppose) you could try to create an 8 descriptor ring.

Maybe this is to protect a simpler device that consumes one descriptor per segment? If so, the check would ideally be conditioned on a related device capability flag. I’m not sure if there is such a flag today.

> 		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;
> -
> +			}
> 			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.

sengment -> segment
... to send single segment...
suppose -> assume

> +	 */
> +	if (nb_segs > 1 && nb_segs_is_invalid(nb_segs))
> 		return;
> 
> 	/*
> 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");

Same comments as above

> +					goto fail;
> +				}
> 				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,
> -- 
> 1.8.3.1
> 


  reply	other threads:[~2020-12-11 16:00 UTC|newest]

Thread overview: 64+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-18 12:02 [dpdk-dev] [PATCH 0/4] minor fixes for testpmd Wei Hu (Xavier)
2020-08-18 12:02 ` [dpdk-dev] [PATCH 1/4] app/testpmd: fix missing verification of port id Wei Hu (Xavier)
2020-08-18 12:02 ` [dpdk-dev] [PATCH 2/4] app/testpmd: fix VLAN offload configuration when config fail Wei Hu (Xavier)
2020-08-18 12:02 ` [dpdk-dev] [PATCH 3/4] app/testpmd: fix packet header in txonly mode Wei Hu (Xavier)
2020-08-18 12:02 ` [dpdk-dev] [PATCH 4/4] app/testpmd: fix displaying Rx Tx queues information Wei Hu (Xavier)
2020-08-20  1:42 ` [dpdk-dev] [PATCH v2 0/4] minor fixes for testpmd Wei Hu (Xavier)
2020-08-20  1:42   ` [dpdk-dev] [PATCH v2 1/4] app/testpmd: fix missing verification of port id Wei Hu (Xavier)
2020-09-14 16:13     ` Ferruh Yigit
2020-08-20  1:42   ` [dpdk-dev] [PATCH v2 2/4] app/testpmd: fix VLAN offload configuration when config fail Wei Hu (Xavier)
2020-09-14 16:13     ` Ferruh Yigit
2020-08-20  1:42   ` [dpdk-dev] [PATCH v2 3/4] app/testpmd: fix packet header in txonly mode Wei Hu (Xavier)
2020-09-14 16:23     ` Ferruh Yigit
2020-09-17  7:10       ` Chengchang Tang
2020-09-17 11:16         ` Ferruh Yigit
2020-09-17 11:48           ` Chengchang Tang
2020-08-20  1:42   ` [dpdk-dev] [PATCH v2 4/4] app/testpmd: fix displaying Rx Tx queues information Wei Hu (Xavier)
2020-09-14 16:31     ` Ferruh Yigit
2020-09-16  9:23       ` Wei Hu (Xavier)
2020-09-16 15:58         ` Ferruh Yigit
2020-09-03  3:19   ` [dpdk-dev] [PATCH v2 0/4] minor fixes for testpmd Wei Hu (Xavier)
2020-09-19 10:47 ` [dpdk-dev] [PATCH v3 0/6] " Wei Hu (Xavier)
2020-09-19 10:47   ` [dpdk-dev] [PATCH v3 1/6] app/testpmd: fix missing verification of port id Wei Hu (Xavier)
2020-09-22 14:49     ` Ferruh Yigit
2020-09-19 10:47   ` [dpdk-dev] [PATCH v3 2/6] app/testpmd: fix VLAN offload configuration when config fail Wei Hu (Xavier)
2020-09-19 10:47   ` [dpdk-dev] [PATCH v3 3/6] app/testpmd: remove restriction on txpkts set Wei Hu (Xavier)
2020-09-22 14:51     ` Ferruh Yigit
2020-09-23  3:14       ` Wei Hu (Xavier)
2020-09-23 11:57         ` Wei Hu (Xavier)
2020-09-23 16:59           ` Ferruh Yigit
2020-09-24  6:08             ` Chengchang Tang
2020-09-24 12:19               ` Ferruh Yigit
2020-09-19 10:47   ` [dpdk-dev] [PATCH v3 4/6] app/testpmd: fix packet header in txonly mode Wei Hu (Xavier)
2020-09-19 10:47   ` [dpdk-dev] [PATCH v3 5/6] app/testpmd: fix valid desc id check Wei Hu (Xavier)
2020-09-22 14:53     ` Ferruh Yigit
2020-09-19 10:47   ` [dpdk-dev] [PATCH v3 6/6] app/testpmd: fix displaying Rx Tx queues information Wei Hu (Xavier)
2020-09-25 12:47 ` [dpdk-dev] [PATCH v4 0/6] minor fixes for testpmd Wei Hu (Xavier)
2020-09-25 12:47   ` [dpdk-dev] [PATCH v4 1/6] app/testpmd: fix missing verification of port id Wei Hu (Xavier)
2020-09-25 12:47   ` [dpdk-dev] [PATCH v4 2/6] app/testpmd: fix VLAN offload configuration when config fail Wei Hu (Xavier)
2020-09-25 12:47   ` [dpdk-dev] [PATCH v4 3/6] app/testpmd: remove restriction on txpkts set Wei Hu (Xavier)
2020-11-23 11:50     ` Slava Ovsiienko
2020-11-24 10:27       ` Thomas Monjalon
2020-11-24 12:23         ` Ferruh Yigit
2020-11-24 13:01           ` Kevin Traynor
2020-11-25 14:06           ` Ferruh Yigit
2020-11-26  7:24             ` Slava Ovsiienko
2020-11-26 12:38               ` Ferruh Yigit
2020-11-27 13:05                 ` Slava Ovsiienko
2020-12-02 12:07                   ` Ferruh Yigit
2020-12-03  9:45                     ` Slava Ovsiienko
2020-12-03 10:18                       ` Ferruh Yigit
2020-12-11 15:07                         ` [dpdk-dev] [PATCH] app/testpmd: fix segment number check Viacheslav Ovsiienko
2020-12-11 16:00                           ` Andrew Boyer [this message]
2020-12-11 16:14                             ` Slava Ovsiienko
2020-12-16 12:12                               ` Ferruh Yigit
2020-12-16 12:33                                 ` Slava Ovsiienko
2020-12-16 12:36                           ` Ferruh Yigit
2021-04-23 16:09                           ` [dpdk-dev] [PATCH v2] " Ferruh Yigit
2021-04-26 11:23                             ` Li, Xiaoyun
2021-04-27 11:42                               ` Ferruh Yigit
2020-09-25 12:47   ` [dpdk-dev] [PATCH v4 4/6] app/testpmd: fix packet header in txonly mode Wei Hu (Xavier)
2020-09-29 15:40     ` Ferruh Yigit
2020-09-25 12:47   ` [dpdk-dev] [PATCH v4 5/6] app/testpmd: fix valid desc id check Wei Hu (Xavier)
2020-09-25 12:47   ` [dpdk-dev] [PATCH v4 6/6] app/testpmd: fix displaying Rx Tx queues information Wei Hu (Xavier)
2020-09-29 15:40   ` [dpdk-dev] [PATCH v4 0/6] minor fixes for testpmd Ferruh Yigit

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=40F5C8DC-8461-43E4-BF37-16FB1D4CA990@pensando.io \
    --to=aboyer@pensando.io \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=stable@dpdk.org \
    --cc=thomas@monjalon.net \
    --cc=viacheslavo@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git