mbox series

[v2,00/25] Replace use of RTE_LOGTYPE_PMD

Message ID 20231213041920.729403-1-stephen@networkplumber.org (mailing list archive)
Headers
Series Replace use of RTE_LOGTYPE_PMD |

Message

Stephen Hemminger Dec. 13, 2023, 4:16 a.m. UTC 
  The generic RTE_LOGTYPE_PMD is a leftover and should be removed.
As a first step, fix many drivers to not use it, and add a
helper for the RTE_LOG_DP(). 

Most of this patchset is boiler plate but there were some
places where use of PMD type snuck in with changes to
original driver and get fixed here.

More work is needed before PMD logtype can be retired.

v2 - fix some typos from v1 from CI

Stephen Hemminger (25):
  log: fix doc comment for RTE_LOG_DP()
  log: add rte_log_dp()
  net/atlantic: replace RTE_LOG_DP with rte_log_dp
  net/avp: replace RTE_LOG_DP with rte_log_dp
  net/bnxt: replace RTE_LOG_DP with rte_log_dp
  net/dpaa: replace RTE_LOG_DP with rte_log_dp
  net/dpaa2: replace RTE_LOG_DP with rte_log_dp
  net/enetc, net/enetfec: replace RTE_LOG_DP with rte_log_dp
  net/igc: replace RTE_LOG_DP with rte_log_dp
  net/mana: replace RTE_LOG_DP with rte_log_dp
  net/mvpp2: do not use PMD logtype
  net/octeon_ep: replace RTE_LOG_DP with rte_log_dp
  net/pfe: replace RTE_LOG_DP with rte_log_dp
  net/qede: replace RTE_LOG_DP with rte_log_dp
  net/virtio: replace RTE_LOG_DP with rte_log_dp
  net/vmxnet3: do not use PMD logtype
  common/cnxk: replace RTE_LOG_DP with rte_log_dp
  common/cpt: replace RTE_LOG_DP with rte_log_dp
  common/sfc_efx: remove use of PMD logtype
  common/dpaax: do not use PMD logtype
  basband/la12xx: replace RTE_LOG_DP with rte_log_dp
  bus/cdx: replace RTE_LOG_DP with rte_log_dp
  bus/fslmc: replace RTE_LOG_DP with rte_log_dp
  dma/dpaa, dma/dpaa2: replace RTE_LOG_DP with rte_log_dp
  mempool/dpaa, mempool/dpaa2: do not use logtype PMD

 .../baseband/la12xx/bbdev_la12xx_pmd_logs.h   |  2 +-
 drivers/bus/cdx/cdx_logs.h                    |  2 +-
 drivers/bus/fslmc/fslmc_logs.h                |  2 +-
 drivers/common/cnxk/roc_platform.h            | 28 ++++++++++---------
 drivers/common/cpt/cpt_pmd_logs.h             |  2 +-
 drivers/common/dpaax/caamflib/compat.h        |  5 +++-
 drivers/common/dpaax/dpaax_logs.h             |  2 +-
 drivers/common/dpaax/version.map              |  1 +
 drivers/common/sfc_efx/sfc_efx.c              | 11 ++------
 drivers/common/sfc_efx/sfc_efx_log.h          |  2 +-
 drivers/dma/dpaa/dpaa_qdma_logs.h             |  5 ++--
 drivers/dma/dpaa2/dpaa2_qdma_logs.h           |  3 +-
 drivers/mempool/dpaa/dpaa_mempool.h           |  2 +-
 drivers/mempool/dpaa2/dpaa2_hw_mempool.c      |  4 +--
 drivers/mempool/dpaa2/dpaa2_hw_mempool_logs.h |  2 +-
 drivers/net/atlantic/atl_logs.h               | 15 +++++-----
 drivers/net/avp/avp_logs.h                    | 20 +++++++------
 drivers/net/bnxt/bnxt.h                       |  5 ++++
 drivers/net/bnxt/bnxt_rxtx_vec_neon.c         |  3 +-
 drivers/net/bnxt/bnxt_rxtx_vec_sse.c          |  3 +-
 drivers/net/bnxt/bnxt_txr.c                   |  4 +--
 drivers/net/dpaa/dpaa_ethdev.c                |  4 +--
 drivers/net/dpaa/dpaa_ethdev.h                |  2 +-
 drivers/net/dpaa2/dpaa2_ethdev.c              |  2 +-
 drivers/net/dpaa2/dpaa2_pmd_logs.h            |  2 +-
 drivers/net/dpaa2/dpaa2_sparser.c             |  4 +--
 drivers/net/enetc/enetc_logs.h                |  2 +-
 drivers/net/enetfec/enet_pmd_logs.h           |  2 +-
 drivers/net/igc/igc_logs.h                    | 10 ++++---
 drivers/net/mana/mana.h                       |  2 +-
 drivers/net/mvpp2/mrvl_ethdev.c               |  4 +--
 drivers/net/octeon_ep/otx_ep_common.h         |  7 +++++
 drivers/net/octeon_ep/otx_ep_rxtx.c           |  5 ++--
 drivers/net/octeontx/octeontx_logs.h          |  3 +-
 drivers/net/pfe/pfe_logs.h                    |  2 +-
 drivers/net/qede/qede_logs.h                  | 10 ++++---
 drivers/net/virtio/virtio_logs.h              | 16 ++++++-----
 drivers/net/vmxnet3/vmxnet3_ethdev.c          |  2 +-
 drivers/net/vmxnet3/vmxnet3_logs.h            | 12 +++++---
 lib/log/rte_log.h                             | 26 +++++++++++++++--
 40 files changed, 140 insertions(+), 100 deletions(-)

Comments

David Marchand Dec. 13, 2023, 8 a.m. UTC | #1 
On Wed, Dec 13, 2023 at 5:19 AM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> The generic RTE_LOGTYPE_PMD is a leftover and should be removed.
> As a first step, fix many drivers to not use it, and add a
> helper for the RTE_LOG_DP().

I don't see why we need a new rte_log_dp helper.
The #define RTE_LOGTYPE_XXX trick should work fine with RTE_LOG_DP().
What else is missing?


>
> Most of this patchset is boiler plate but there were some
> places where use of PMD type snuck in with changes to
> original driver and get fixed here.

I see odd changes, like a change in PMD_DRV_LOG meaning in the first patch:
https://patchwork.dpdk.org/project/dpdk/patch/20231213041920.729403-4-stephen@networkplumber.org/

A lot of drivers use this macro to log "driver"/configuration level
errors (see below).
Switching this macro to the DP version looks wrong to me.

$ git grep PMD_DRV_LOG drivers/net/atlantic/
drivers/net/atlantic/atl_ethdev.c:              PMD_DRV_LOG(ERR,
"rte_eal_alarm_set fail");
drivers/net/atlantic/atl_ethdev.c:              PMD_DRV_LOG(INFO,
"Port %d: Link Up - speed %u Mbps - %s",
drivers/net/atlantic/atl_ethdev.c:              PMD_DRV_LOG(INFO, "
Port %d: Link Down",
drivers/net/atlantic/atl_ethdev.c:      PMD_DRV_LOG(DEBUG, "PCI
Address: " PCI_PRI_FMT,
...


--
David Marchand
Stephen Hemminger Dec. 13, 2023, 3:09 p.m. UTC | #2 
On Wed, 13 Dec 2023 09:00:48 +0100
David Marchand <david.marchand@redhat.com> wrote:

> On Wed, Dec 13, 2023 at 5:19 AM Stephen Hemminger
> <stephen@networkplumber.org> wrote:
> >
> > The generic RTE_LOGTYPE_PMD is a leftover and should be removed.
> > As a first step, fix many drivers to not use it, and add a
> > helper for the RTE_LOG_DP().  
> 
> I don't see why we need a new rte_log_dp helper.
> The #define RTE_LOGTYPE_XXX trick should work fine with RTE_LOG_DP().
> What else is missing?

The problem is RTE_LOG_DP() usage today takes suffixes. So the param
log type can only be one of the pre-defined fixed values.

Changing the macro in one patch would be a huge patch.

It seemed best to have RTE_LOG_DP() stay like RTE_LOG()
and introduce rte_log_dp() to match rte_log().
David Marchand Dec. 18, 2023, 2:03 p.m. UTC | #3 
On Wed, Dec 13, 2023 at 4:09 PM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> On Wed, 13 Dec 2023 09:00:48 +0100
> David Marchand <david.marchand@redhat.com> wrote:
>
> > On Wed, Dec 13, 2023 at 5:19 AM Stephen Hemminger
> > <stephen@networkplumber.org> wrote:
> > >
> > > The generic RTE_LOGTYPE_PMD is a leftover and should be removed.
> > > As a first step, fix many drivers to not use it, and add a
> > > helper for the RTE_LOG_DP().
> >
> > I don't see why we need a new rte_log_dp helper.
> > The #define RTE_LOGTYPE_XXX trick should work fine with RTE_LOG_DP().
> > What else is missing?
>
> The problem is RTE_LOG_DP() usage today takes suffixes. So the param
> log type can only be one of the pre-defined fixed values.
>
> Changing the macro in one patch would be a huge patch.

I am not asking for change in the RTE_LOG_DP() API.
I sent a patch doing for what I had in mind:
https://patchwork.dpdk.org/project/dpdk/patch/20231218135932.1497163-1-david.marchand@redhat.com/


>
> It seemed best to have RTE_LOG_DP() stay like RTE_LOG()
> and introduce rte_log_dp() to match rte_log().
>

rte_log() seems a low level API, I don't like the idea of adding
another one with rte_log_dp().
And it will conflict with the cleanup I started on multiline / line
with \n in logs.