Re: [dpdk-dev] [PATCH 3/3] app/testpmd: use of Rx/Tx in testpmd
From: Tu, Lijuan <hidden>
Date: 2021-03-23 07:46:41
-----Original Message----- From: Andrew Rybchenko <redacted> Sent: 2021年3月23日 15:25 To: Li, Xiaoyun <redacted>; Lijun Ou <redacted>; 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
Hiquoted
-----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 thispatch does:quoted
RX/TX and Rx/Tx are mixed used in testpmd print and comments. This patchunifies 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 DTSscripts 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. 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
And please separate this patch with the other twos. Don't mix them in onepatchset. 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
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 +++++++++++++++++++++--------------------- app/test-pmd/testpmd.h | 28 +++++----- app/test-pmd/txonly.c | 2 +- 9 files changed, 237 insertions(+), 237 deletions(-) -- 2.7.4