From: Slava Ovsiienko <viacheslavo@nvidia.com>
To: Maxime Leroy <maxime.leroy@6wind.com>,
Matan Azrad <matan@nvidia.com>,
Shahaf Shuler <shahafs@nvidia.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>,
NBU-Contact-N?lio Laranjeiro <nelio.laranjeiro@6wind.com>
Subject: Re: [dpdk-dev] [PATCH 1/4] Revert "net/mlx5: fix Rx queue count calculation"
Date: Wed, 11 Nov 2020 19:51:49 +0000 [thread overview]
Message-ID: <MWHPR12MB150115F3D5E26D521C8C709CDFE80@MWHPR12MB1501.namprd12.prod.outlook.com> (raw)
In-Reply-To: <20201110140938.15046-2-maxime.leroy@6wind.com>
Hi, Maxime
Thanks a lot for the patch. There is the comment for the entire series.
[1]_____
>
> First issue, when there are more than 8 CQEs to uncompress, the computation
> done in this commit cannot work. Because the zip-ai variable describes the
> current index inside the CQE8 array and thus is limited from 0 to 7 included. So
> if we are decompressed the 9 packets, ai is 0. So in this case, n is equals to
> cqe_cnt - 0.
>
> Example with 11 packets we will have:
> C | a | e0 | e1 | e2 | e3 | e4 | e5 | C | a | e0
>
1. ai is not index in the array (just tree lsbs of ai). It is an index of the miniCQE being processed
in the compressed session and is in the range [0 . .zip.cqe_cnt-1]. In your example there will be
two compressed sessions. The bug was we corrected each compressed session for the ai of the
first one (in processing that we were).
[2]_____
> /* if we are processing a compressed cqe */
> if (zip->ai) {
>- used = zip->cqe_cnt - zip->ca;
> cq_ci = zip->cq_ci;
>+ cq_end = cq_ci + zip->cqe_cnt;
>+ cq_cur = zip->ca + zip->ai;
>+ used = cq_end - cq_cur;
> } else {
> used = 0;
> cq_ci = rxq->cq_ci;
Sorry, it seems to be incorrect.
zip->cq_ci is the index of the NEXT CQE, following the compressed session being processed.
zip->ai is index of miniCQE being processed. "used" should be calculated much simple:
used = zip->cqe_cnt - zip->ai
[3]_____
- if (dev->rx_pkt_burst == NULL ||
- dev->rx_pkt_burst == removed_rx_burst) {
+ if (dev->rx_pkt_burst != mlx5_rx_burst) {
In this way, we cut the support for other rx_burst routines, we should restore.
[4]______
I'am OK with Didier patch "net/mlx5: fix Rx descriptor status returned value"
I see you wrote the luxury commit messages, and I'm crying with bloody tears about what I'm going to ask you for -
could we squash the series in to single commit? Or at least two - Didier and yours?
With best regards, Slava
next prev parent reply other threads:[~2020-11-11 19:51 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-10 14:09 [dpdk-dev] [PATCH 0/4] net/mlx5: fixes for rx queue count calculation Maxime Leroy
2020-11-10 14:09 ` [dpdk-dev] [PATCH 1/4] Revert "net/mlx5: fix Rx queue count calculation" Maxime Leroy
2020-11-11 19:51 ` Slava Ovsiienko [this message]
2020-11-12 15:43 ` Maxime Leroy
2020-11-10 14:09 ` [dpdk-dev] [PATCH 2/4] net/mlx5: fixed used initialization in rx_queue_count Maxime Leroy
2020-11-10 14:09 ` [dpdk-dev] [PATCH 3/4] net/mlx5: fix Rx descriptor status returned value Maxime Leroy
2020-11-10 14:09 ` [dpdk-dev] [PATCH 4/4] mlx5: re-add support of rx_queue_count for mlx5_rx_burst_mprq Maxime Leroy
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=MWHPR12MB150115F3D5E26D521C8C709CDFE80@MWHPR12MB1501.namprd12.prod.outlook.com \
--to=viacheslavo@nvidia.com \
--cc=dev@dpdk.org \
--cc=matan@nvidia.com \
--cc=maxime.leroy@6wind.com \
--cc=nelio.laranjeiro@6wind.com \
--cc=shahafs@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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).