From: Ferruh Yigit <ferruh.yigit@amd.com>
To: Chaoyong He <chaoyong.he@corigine.com>,
Stephen Hemminger <stephen@networkplumber.org>
Cc: "dev@dpdk.org" <dev@dpdk.org>,
oss-drivers <oss-drivers@corigine.com>,
Niklas Soderlund <niklas.soderlund@corigine.com>
Subject: Re: [PATCH 0/3] cleanup the PMD
Date: Wed, 8 Feb 2023 01:56:32 +0000 [thread overview]
Message-ID: <a06fbe67-dd37-a016-69c5-037e77d0a338@amd.com> (raw)
In-Reply-To: <SJ0PR13MB55451E50B93CCBC214B5E5929EDB9@SJ0PR13MB5545.namprd13.prod.outlook.com>
On 2/7/2023 2:17 AM, Chaoyong He wrote:
>> On Mon, 6 Feb 2023 15:05:46 +0800
>> Chaoyong He <chaoyong.he@corigine.com> wrote:
>>
>>> This patch series aims to better align the PMD with the preferred DPDK
>>> coding style.
>>> - Remove the usage of 'printf()'
>>> - Remove the unneeded header file includes
>>> - Explicitly compare pointer with NULL
>>> - Explicitly compare integer with 0
>>>
>>> James Hershaw (3):
>>> net/nfp: remove usage of print statements
>>> net/nfp: remove unnecessary include
>>> net/nfp: explicitly compare to null and 0
>>>
>>> drivers/net/nfp/nfp_common.c | 25 +++---
>>> drivers/net/nfp/nfp_cpp_bridge.c | 4 +-
>>> drivers/net/nfp/nfp_ethdev.c | 2 +-
>>> drivers/net/nfp/nfp_ethdev_vf.c | 2 +-
>>> drivers/net/nfp/nfp_rxtx.c | 14 ++--
>>> drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c | 88 +++++++++-------------
>>> drivers/net/nfp/nfpcore/nfp_cppcore.c | 31 ++++----
>>> drivers/net/nfp/nfpcore/nfp_hwinfo.c | 23 +++---
>>> drivers/net/nfp/nfpcore/nfp_mip.c | 16 ++--
>>> drivers/net/nfp/nfpcore/nfp_mutex.c | 14 ++--
>>> drivers/net/nfp/nfpcore/nfp_nffw.c | 10 +--
>>> drivers/net/nfp/nfpcore/nfp_nsp.c | 36 +++++----
>>> drivers/net/nfp/nfpcore/nfp_nsp_cmds.c | 10 +--
>>> drivers/net/nfp/nfpcore/nfp_nsp_eth.c | 38 +++++-----
>>> drivers/net/nfp/nfpcore/nfp_resource.c | 15 ++--
>>> drivers/net/nfp/nfpcore/nfp_rtsym.c | 48 +++++-------
>>> drivers/net/nfp/nfpcore/nfp_target.h | 2 +-
>>> 17 files changed, 176 insertions(+), 202 deletions(-)
>>>
>>
>> I am working on getting rid of RTE_LOGTYPE_PMD; all driver should be using
>> their own log type.
>>
>> Looks like this driver has a lot of places that need fixing.
>> $ git grep 'PMD, '
>> flower/nfp_flower.c: RTE_LOG_DP(ERR, PMD, "RX Bad queue\n");
>> flower/nfp_flower.c: RTE_LOG_DP(ERR, PMD, "rxb does not
>> exist!\n");
>> nfp_cpp_bridge.c: RTE_LOG(ERR, PMD, "%s: area alloc fail\n",
>> __func__);
>> nfp_cpp_bridge.c: RTE_LOG(ERR, PMD, "area acquire failed\n");
>> nfp_cpp_bridge.c: RTE_LOG(ERR, PMD, "nfp_cpp_area_write
>> error\n");
>> nfp_cpp_bridge.c: RTE_LOG(ERR, PMD, "%s: area alloc failed\n",
>> __func__);
>> nfp_cpp_bridge.c: RTE_LOG(ERR, PMD, "area acquire failed\n");
>> nfp_cpp_bridge.c: RTE_LOG(ERR, PMD, "nfp_cpp_area_read
>> error\n");
>> nfp_cpp_bridge.c: RTE_LOG(ERR, PMD, "%s: read error from
>> socket\n", __func__);
>> nfp_cpp_bridge.c: RTE_LOG(ERR, PMD, "%s: unknown cmd %d\n",
>> __func__, cmd);
>> nfp_cpp_bridge.c: RTE_LOG(ERR, PMD, "%s: read error from
>> socket\n", __func__);
>> nfp_cpp_bridge.c: RTE_LOG(ERR, PMD, "%s: error writing to socket\n",
>> __func__);
>> nfp_cpp_bridge.c: RTE_LOG(ERR, PMD, "%s: error writing to socket\n",
>> __func__);
>> nfp_cpp_bridge.c: RTE_LOG(ERR, PMD, "%s: socket creation error.
>> Service failed\n",
>> nfp_cpp_bridge.c: RTE_LOG(ERR, PMD, "%s: bind error (%d). Service
>> failed\n",
>> nfp_cpp_bridge.c: RTE_LOG(ERR, PMD, "%s: listen error(%d). Service
>> failed\n",
>> nfp_cpp_bridge.c: RTE_LOG(ERR, PMD, "%s: accept call error
>> (%d)\n",
>> nfp_cpp_bridge.c: RTE_LOG(ERR, PMD, "%s: service failed\n",
>> __func__);
>> nfp_logs.h: RTE_LOG(level, PMD, "%s() rx: " fmt "\n", __func__, ## args)
>> nfp_logs.h: RTE_LOG(level, PMD, "%s() tx: " fmt "\n", __func__, ## args)
>> nfp_rxtx.c: RTE_LOG_DP(ERR, PMD, "RX Bad queue\n");
>> nfp_rxtx.c: RTE_LOG_DP(ERR, PMD, "rxb does not exist!\n");
>> nfpcore/nfp_cpp_pcie_ops.c: RTE_LOG(ERR, PMD, "NFP lock for
>> secondary process failed\n");
>> nfpcore/nfp_cpp_pcie_ops.c: RTE_LOG(ERR, PMD, "NFP lock for
>> secondary process failed\n");
>
> Thanks for your review!
> This patch series just the first step to make things right, and we will send out another patch series
> to refactor the log system of nfp PMD, which will solve the problem you point out.
> Do you think it's okay?
Thanks Chaoyong,
I am merging this set as it is, remaining cleanups can be merged as they
received.
next prev parent reply other threads:[~2023-02-08 1:56 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-02-06 7:05 Chaoyong He
2023-02-06 7:05 ` [PATCH 1/3] net/nfp: remove usage of print statements Chaoyong He
2023-02-06 7:05 ` [PATCH 2/3] net/nfp: remove unnecessary include Chaoyong He
2023-02-06 7:05 ` [PATCH 3/3] net/nfp: explicitly compare to null and 0 Chaoyong He
2023-02-06 16:28 ` [PATCH 0/3] cleanup the PMD Stephen Hemminger
2023-02-07 2:17 ` Chaoyong He
2023-02-08 1:56 ` Ferruh Yigit [this message]
2023-02-08 1:55 ` 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=a06fbe67-dd37-a016-69c5-037e77d0a338@amd.com \
--to=ferruh.yigit@amd.com \
--cc=chaoyong.he@corigine.com \
--cc=dev@dpdk.org \
--cc=niklas.soderlund@corigine.com \
--cc=oss-drivers@corigine.com \
--cc=stephen@networkplumber.org \
/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).