patches for DPDK stable branches
 help / color / mirror / Atom feed
From: Didier Pallard <didier.pallard@6wind.com>
To: Slava Ovsiienko <viacheslavo@mellanox.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>,
	"stable@dpdk.org" <stable@dpdk.org>,
	Matan Azrad <matan@mellanox.com>
Subject: Re: [dpdk-stable] [dpdk-dev] [PATCH] net/mlx5: fix Rx descriptor status returned value
Date: Mon, 16 Mar 2020 18:24:00 +0100
Message-ID: <CAMinK25EVnv_-1qy6ecEMwKEW2SWRvL_2XN8Bhw=geGgUATveg@mail.gmail.com> (raw)
In-Reply-To: <AM4PR05MB3265F30B352172C25A306C38D2F90@AM4PR05MB3265.eurprd05.prod.outlook.com>

Well, you're right, another way to fix the problem could be to set up the
queue size assuming the provided number
is a number of packets in queue rather than a number of mbufs in queue.
But not sure it's better, it's also important for the application/user to
know the number of mbufs that could fit in a rx/tx queue,
whatever the number of packets that it covers, since it is very important
to size the memory pools correctly to avoid any
mbuf shortage during system life.
Thanks
Didier

On Mon, Mar 16, 2020 at 5:05 PM Slava Ovsiienko <viacheslavo@mellanox.com>
wrote:

> Hi, Didier
>
> First, thank you for the patch.
>
> If we have a look at the description of rte_eth_rx_queue_count(): "Get the
> number of used descriptors of a rx queue".
> It means the DPDK generic descriptors, not PMD specific ones. "DPDK
> descriptor" means the entity which can handle one packet.
> rte_eth_rx_queue_count() should return the potential number of packets
> could be fetched from the Rx queue on the next rx_burst() call.
> Application should know anything about PMD descriptors, it must be
> isolated. So, rx_queue_count() should return the number of expected
> packets not hardware entries. That's why the value being returned is
> compared with elts, not with HW desctriptors.
>
> As for -1 - I agree, should be fixed.
>
> It seems we have another bug - the Rx queue is created with number of
> hardware descriptors which does not correspond the requested packets
> in case of multi-segment packets  (requested desc is divided by
> (1<<sges_n), it seems it should not).
>
> With best regards, Slava
>
> > -----Original Message-----
> > From: dev <dev-bounces@dpdk.org> On Behalf Of Didier Pallard
> > Sent: Friday, March 13, 2020 11:57
> > To: dev@dpdk.org
> > Cc: stable@dpdk.org
> > Subject: [dpdk-dev] [PATCH] net/mlx5: fix Rx descriptor status returned
> value
> >
> > Two bugs in rx_queue_count function:
> > - One entry may contain several segments, so 'used' must be multiplied
> >   by number of segments per entry to properly reflect the queue usage.
> > - rx_queue_count returns the number of entries used in queue, so it
> ranges
> >   from 0 to max number of entries in queue, not this number minus
> >   one.
> >
> > Fixes: 8788fec1f269 ("net/mlx5: implement descriptor status API")
> > Cc: stable@dpdk.org
> > Signed-off-by: Didier Pallard <didier.pallard@6wind.com>
> > ---
> >  drivers/net/mlx5/mlx5_rxtx.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/mlx5/mlx5_rxtx.c b/drivers/net/mlx5/mlx5_rxtx.c
> index
> > 5ac63da8039d..17f80c25443e 100644
> > --- a/drivers/net/mlx5/mlx5_rxtx.c
> > +++ b/drivers/net/mlx5/mlx5_rxtx.c
> > @@ -500,7 +500,7 @@ rx_queue_count(struct mlx5_rxq_data *rxq)
> >               used += n;
> >               cqe = &(*rxq->cqes)[cq_ci & cqe_cnt];
> >       }
> > -     used = RTE_MIN(used, (1U << rxq->elts_n) - 1);
> > +     used = RTE_MIN(used * (1 << rxq->sges_n), 1U << rxq->elts_n);
> >       return used;
> >  }
> >
> > --
> > 2.24.1
>
>

  reply	other threads:[~2020-03-16 17:24 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-13  9:56 [dpdk-stable] " Didier Pallard
2020-03-16 16:05 ` [dpdk-stable] [dpdk-dev] " Slava Ovsiienko
2020-03-16 17:24   ` Didier Pallard [this message]
2020-03-17  8:33     ` Slava Ovsiienko
2020-03-17  9:19       ` Didier Pallard
2020-03-17  9:25         ` Slava Ovsiienko
2020-03-17  9:41           ` Didier Pallard
2020-10-29 11:29           ` Didier Pallard

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='CAMinK25EVnv_-1qy6ecEMwKEW2SWRvL_2XN8Bhw=geGgUATveg@mail.gmail.com' \
    --to=didier.pallard@6wind.com \
    --cc=dev@dpdk.org \
    --cc=matan@mellanox.com \
    --cc=stable@dpdk.org \
    --cc=viacheslavo@mellanox.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

patches for DPDK stable branches

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/stable/0 stable/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 stable stable/ https://inbox.dpdk.org/stable \
		stable@dpdk.org
	public-inbox-index stable

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


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