On 3/23/2021 7:58 AM, Li, Xiaoyun wrote:

quoted

-----Original Message-----
From: oulijun <redacted>
Sent: Tuesday, March 23, 2021 15:52
To: Tu, Lijuan <redacted>; Andrew Rybchenko
[off-list ref]; Li, Xiaoyun [off-list ref]; Yigit,
Ferruh [off-list ref]
Cc: dev@dpdk.org; linuxarm@openeuler.org
Subject: Re: [dpdk-dev] [PATCH 3/3] app/testpmd: use of Rx/Tx in testpmd



在 2021/3/23 15:46, Tu, Lijuan 写道:

quoted

quoted

-----Original Message-----
From: Andrew Rybchenko <redacted>
Sent: 2021年3月23日 15:25
To: Li, Xiaoyun <redacted>; Lijun Ou
[off-list ref]; Yigit, Ferruh [off-list ref]; Tu,
Lijuan [off-list ref]
Cc: dev@dpdk.org; linuxarm@openeuler.org
Subject: Re: [dpdk-dev] [PATCH 3/3] app/testpmd: use of Rx/Tx in
testpmd

On 3/23/21 6:17 AM, Li, Xiaoyun wrote:

quoted

Hi

quoted

-----Original Message-----
From: Lijun Ou <redacted>
Sent: Friday, March 5, 2021 18:22
To: Yigit, Ferruh <redacted>
Cc: Li, Xiaoyun <redacted>; dev@dpdk.org;
linuxarm@openeuler.org
Subject: [PATCH 3/3] app/testpmd: use of Rx/Tx in testpmd

From: Hongbo Zheng <redacted>

In testpmd, when we input "show config rxtx", we can see like this:

1: testpmd> show config rxtx
2:   io packet forwarding packets/burst=32
3:   nb forwarding cores=1 - nb forwarding ports=1
4:   port 0: RX queue number: 1 Tx queue number: 1
5:     Rx offloads=0x0 Tx offloads=0x10000
6:     RX queue: 0
7:       RX desc=1024 - RX free threshold=32
8:       RX threshold registers: pthresh=0 hthresh=0  wthresh=0
9:       RX Offloads=0x0
10:    TX queue: 0
11:      TX desc=1024 - TX free threshold=928
12:      TX threshold registers: pthresh=0 hthresh=0  wthresh=0
13:      TX offloads=0x10000 - TX RS bit threshold=32

We can see RX/Rx/TX/Tx is mixed used. Also in other places in
testpmd, RX/Rx/TX/Tx is mixed used.

This patch fix the mixed use of RX/Rx/TX/Tx in testpmd by change to
unified use Rx/Tx.

The commit log is too redundant. The following is enough to explain
what this

patch does:

quoted

RX/TX and Rx/Tx are mixed used in testpmd print and comments. This
patch

unifies them as Rx/Tx.

quoted

Except this, the patch looks good to me.

But one big concern, this patch will break all of the CI tests
because the DTS

scripts check if the results are the same as expected and this patch
change a lot of the print.

quoted

So I think the DTS maintainer needs to be aware of this.

I think test dpdk-testpmd output is a part of API. Of course, it is
not an API, but such cosmetic changes in output will be much more painful

than API changes.

quoted

quoted

Output parsers will simply stop to work. I think such changes should
go through the deprecation process.

Of course, it would be useful to change comments and may be even
error log messages right now, but not commands output.

quoted

+Lijuan DTS maintainer

It has big impact with CI system, most of test cases depended on the testpmd

output, if changed, our CI system will be broken, Is it really worth it ?

quoted

I think it's necessary to keep the style consistent and to constrain everyone to do
so in the future.However, if the DTS changes greatly, I think it is possible to
require the new print to be consistent and the historical part to remain
unchanged.Because the uniform change is also Xiaoyun's suggestion, we think
the opinion is reasonable, but we are not sure how much it will affect DTS.

I think we should drop this patch. But we'll keep consistent for future print when reviewing.
What do you think?

+1 to have consistency on the new code, and as the old code changed by time. 
Instead of big syntax cleanup.

As well as output parsing problem, this also can cause trouble on backporting 
patches for stable trees because of conflicts it will create.

quoted

quoted

quoted

quoted

And please separate this patch with the other twos. Don't mix them
in one

patchset. It's not for the same purpose.

quoted

quoted

Signed-off-by: Hongbo Zheng <redacted>
Signed-off-by: Lijun Ou <redacted>
---
V1->V2:
- fix all RX/TX
- rename patch title
---
   app/test-pmd/cmdline.c     | 104 ++++++++++++++++++------------------
   app/test-pmd/config.c      | 128 ++++++++++++++++++++++------------------

---

quoted

quoted

--

quoted

quoted

   app/test-pmd/csumonly.c    |  22 ++++----
   app/test-pmd/icmpecho.c    |   2 +-
   app/test-pmd/ieee1588fwd.c |  18 +++----
app/test-pmd/parameters.c
|  50 +++++++++---------
   app/test-pmd/testpmd.c     | 120 +++++++++++++++++++++------------------

---

quoted

quoted

quoted

quoted

   app/test-pmd/testpmd.h     |  28 +++++-----
   app/test-pmd/txonly.c      |   2 +-
   9 files changed, 237 insertions(+), 237 deletions(-)
--
2.7.4