netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [net-next PATCH 0/3] Add devlink and devlink health reporters to
@ 2020-11-02  5:06 George Cherian
  2020-11-02  5:06 ` [net-next PATCH 1/3] octeontx2-af: Add devlink suppoort to af driver George Cherian
                   ` (3 more replies)
  0 siblings, 4 replies; 15+ messages in thread
From: George Cherian @ 2020-11-02  5:06 UTC (permalink / raw)
  To: netdev, linux-kernel
  Cc: kuba, davem, sgoutham, lcherian, gakula, masahiroy,
	george.cherian

Add basic devlink and devlink health reporters.
Devlink health reporters are added for NPA and NIX blocks.
These reporters report the error count in respective blocks.

Address Jakub's comment to add devlink support for error reporting.
https://www.spinics.net/lists/netdev/msg670712.html


George Cherian (3):
  octeontx2-af: Add devlink suppoort to af driver
  octeontx2-af: Add devlink health reporters for NPA
  octeontx2-af: Add devlink health reporters for NIX

 .../net/ethernet/marvell/octeontx2/Kconfig    |   1 +
 .../ethernet/marvell/octeontx2/af/Makefile    |   3 +-
 .../net/ethernet/marvell/octeontx2/af/rvu.c   |   9 +-
 .../net/ethernet/marvell/octeontx2/af/rvu.h   |   5 +-
 .../marvell/octeontx2/af/rvu_devlink.c        | 875 ++++++++++++++++++
 .../marvell/octeontx2/af/rvu_devlink.h        |  67 ++
 .../marvell/octeontx2/af/rvu_struct.h         |  33 +
 7 files changed, 990 insertions(+), 3 deletions(-)
 create mode 100644 drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c
 create mode 100644 drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.h

-- 
2.25.1


^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [net-next PATCH 2/3] octeontx2-af: Add devlink health reporters for NPA
@ 2020-11-03  3:58 George Cherian
  2020-11-03 13:50 ` Willem de Bruijn
  0 siblings, 1 reply; 15+ messages in thread
From: George Cherian @ 2020-11-03  3:58 UTC (permalink / raw)
  To: Willem de Bruijn
  Cc: Network Development, linux-kernel, Jakub Kicinski, David Miller,
	Sunil Kovvuri Goutham, Linu Cherian, Geethasowjanya Akula,
	masahiroy@kernel.org

Hi Willem,

Thanks for the review.

> -----Original Message-----
> From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
> Sent: Monday, November 2, 2020 7:12 PM
> To: George Cherian <gcherian@marvell.com>
> Cc: Network Development <netdev@vger.kernel.org>; linux-kernel <linux-
> kernel@vger.kernel.org>; Jakub Kicinski <kuba@kernel.org>; David Miller
> <davem@davemloft.net>; Sunil Kovvuri Goutham
> <sgoutham@marvell.com>; Linu Cherian <lcherian@marvell.com>;
> Geethasowjanya Akula <gakula@marvell.com>; masahiroy@kernel.org
> Subject: Re: [net-next PATCH 2/3] octeontx2-af: Add devlink health
> reporters for NPA
> 
> On Mon, Nov 2, 2020 at 12:07 AM George Cherian
> <george.cherian@marvell.com> wrote:
> >
> > Add health reporters for RVU NPA block.
> > Only reporter dump is supported
> >
> > Output:
> >  # devlink health
> >  pci/0002:01:00.0:
> >    reporter npa
> >      state healthy error 0 recover 0
> >  # devlink  health dump show pci/0002:01:00.0 reporter npa
> >  NPA_AF_GENERAL:
> >         Unmap PF Error: 0
> >         Free Disabled for NIX0 RX: 0
> >         Free Disabled for NIX0 TX: 0
> >         Free Disabled for NIX1 RX: 0
> >         Free Disabled for NIX1 TX: 0
> >         Free Disabled for SSO: 0
> >         Free Disabled for TIM: 0
> >         Free Disabled for DPI: 0
> >         Free Disabled for AURA: 0
> >         Alloc Disabled for Resvd: 0
> >   NPA_AF_ERR:
> >         Memory Fault on NPA_AQ_INST_S read: 0
> >         Memory Fault on NPA_AQ_RES_S write: 0
> >         AQ Doorbell Error: 0
> >         Poisoned data on NPA_AQ_INST_S read: 0
> >         Poisoned data on NPA_AQ_RES_S write: 0
> >         Poisoned data on HW context read: 0
> >   NPA_AF_RVU:
> >         Unmap Slot Error: 0
> >
> > Signed-off-by: Sunil Kovvuri Goutham <sgoutham@marvell.com>
> > Signed-off-by: Jerin Jacob <jerinj@marvell.com>
> > Signed-off-by: George Cherian <george.cherian@marvell.com>
> 
> 
> > +static bool rvu_npa_af_request_irq(struct rvu *rvu, int blkaddr, int offset,
> > +                                  const char *name, irq_handler_t fn)
> > +{
> > +       struct rvu_devlink *rvu_dl = rvu->rvu_dl;
> > +       int rc;
> > +
> > +       WARN_ON(rvu->irq_allocated[offset]);
> 
> Please use WARN_ON sparingly for important unrecoverable events. This
> seems like a basic precondition. If it can happen at all, can probably catch in a
> normal branch with a netdev_err. The stacktrace in the oops is not likely to
> point at the source of the non-zero value, anyway.
Okay, will fix it in v2.
> 
> > +       rvu->irq_allocated[offset] = false;
> 
> Why initialize this here? Are these fields not zeroed on alloc? Is this here only
> to safely call rvu_npa_unregister_interrupts on partial alloc? Then it might be
> simpler to just have jump labels in this function to free the successfully
> requested irqs.

It shouldn't be initialized like this; it is zeroed on alloc.
Will fix in v2.
> 
> > +       sprintf(&rvu->irq_name[offset * NAME_SIZE], name);
> > +       rc = request_irq(pci_irq_vector(rvu->pdev, offset), fn, 0,
> > +                        &rvu->irq_name[offset * NAME_SIZE], rvu_dl);
> > +       if (rc)
> > +               dev_warn(rvu->dev, "Failed to register %s irq\n", name);
> > +       else
> > +               rvu->irq_allocated[offset] = true;
> > +
> > +       return rvu->irq_allocated[offset]; }
> 
> > +static int rvu_npa_health_reporters_create(struct rvu_devlink
> > +*rvu_dl) {
> > +       struct devlink_health_reporter *rvu_npa_health_reporter;
> > +       struct rvu_npa_event_cnt *npa_event_count;
> > +       struct rvu *rvu = rvu_dl->rvu;
> > +
> > +       npa_event_count = kzalloc(sizeof(*npa_event_count), GFP_KERNEL);
> > +       if (!npa_event_count)
> > +               return -ENOMEM;
> > +
> > +       rvu_dl->npa_event_cnt = npa_event_count;
> > +       rvu_npa_health_reporter = devlink_health_reporter_create(rvu_dl-
> >dl,
> > +                                                                &rvu_npa_hw_fault_reporter_ops,
> > +                                                                0, rvu);
> > +       if (IS_ERR(rvu_npa_health_reporter)) {
> > +               dev_warn(rvu->dev, "Failed to create npa reporter, err =%ld\n",
> > +                        PTR_ERR(rvu_npa_health_reporter));
> > +               return PTR_ERR(rvu_npa_health_reporter);
> > +       }
> > +
> > +       rvu_dl->rvu_npa_health_reporter = rvu_npa_health_reporter;
> > +       return 0;
> > +}
> > +
> > +static void rvu_npa_health_reporters_destroy(struct rvu_devlink
> > +*rvu_dl) {
> > +       if (!rvu_dl->rvu_npa_health_reporter)
> > +               return;
> > +
> > +
> > +devlink_health_reporter_destroy(rvu_dl->rvu_npa_health_reporter);
> > +}
> > +
> > +static int rvu_health_reporters_create(struct rvu *rvu) {
> > +       struct rvu_devlink *rvu_dl;
> > +
> > +       if (!rvu->rvu_dl)
> > +               return -EINVAL;
> > +
> > +       rvu_dl = rvu->rvu_dl;
> > +       return rvu_npa_health_reporters_create(rvu_dl);
> 
> No need for local var rvu_dl. Here and below.
> 
> Without that, the entire helper is probably not needed.

This helper is needed as we add support for more HW blocks.

> 
> > +}
> > +
> > +static void rvu_health_reporters_destroy(struct rvu *rvu) {
> > +       struct rvu_devlink *rvu_dl;
> > +
> > +       if (!rvu->rvu_dl)
> > +               return;
> > +
> > +       rvu_dl = rvu->rvu_dl;
> > +       rvu_npa_health_reporters_destroy(rvu_dl);
> > +}
> > +
> >  static int rvu_devlink_info_get(struct devlink *devlink, struct
> devlink_info_req *req,
> >                                 struct netlink_ext_ack *extack)  { @@
> > -53,7 +483,8 @@ int rvu_register_dl(struct rvu *rvu)
> >         rvu_dl->dl = dl;
> >         rvu_dl->rvu = rvu;
> >         rvu->rvu_dl = rvu_dl;
> > -       return 0;
> > +
> > +       return rvu_health_reporters_create(rvu);
> 
> when would this be called with rvu->rvu_dl == NULL?

During initialization.

Regards,
-George

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [net-next PATCH 2/3] octeontx2-af: Add devlink health reporters for NPA
@ 2020-11-03 17:43 George Cherian
  2020-11-03 17:56 ` Willem de Bruijn
  0 siblings, 1 reply; 15+ messages in thread
From: George Cherian @ 2020-11-03 17:43 UTC (permalink / raw)
  To: Willem de Bruijn
  Cc: Network Development, linux-kernel, Jakub Kicinski, David Miller,
	Sunil Kovvuri Goutham, Linu Cherian, Geethasowjanya Akula,
	masahiroy@kernel.org

Hi Willem,


> -----Original Message-----
> From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
> Sent: Tuesday, November 3, 2020 7:21 PM
> To: George Cherian <gcherian@marvell.com>
> Cc: Network Development <netdev@vger.kernel.org>; linux-kernel <linux-
> kernel@vger.kernel.org>; Jakub Kicinski <kuba@kernel.org>; David Miller
> <davem@davemloft.net>; Sunil Kovvuri Goutham
> <sgoutham@marvell.com>; Linu Cherian <lcherian@marvell.com>;
> Geethasowjanya Akula <gakula@marvell.com>; masahiroy@kernel.org
> Subject: [EXT] Re: [net-next PATCH 2/3] octeontx2-af: Add devlink health
> reporters for NPA
> 
> External Email
> 
> ----------------------------------------------------------------------
> > > >  static int rvu_devlink_info_get(struct devlink *devlink, struct
> > > devlink_info_req *req,
> > > >                                 struct netlink_ext_ack *extack)  { @@
> > > > -53,7 +483,8 @@ int rvu_register_dl(struct rvu *rvu)
> > > >         rvu_dl->dl = dl;
> > > >         rvu_dl->rvu = rvu;
> > > >         rvu->rvu_dl = rvu_dl;
> > > > -       return 0;
> > > > +
> > > > +       return rvu_health_reporters_create(rvu);
> > >
> > > when would this be called with rvu->rvu_dl == NULL?
> >
> > During initialization.
> 
> This is the only caller, and it is only reached if rvu_dl is non-zero.

Did you mean to ask, where is it de-initialized?
If so, it should be done in rvu_unregister_dl() after freeing rvu_dl.

Is that what you meant?

Regards,
-George

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [net-next PATCH 2/3] octeontx2-af: Add devlink health reporters for NPA
@ 2020-11-03 17:57 George Cherian
  0 siblings, 0 replies; 15+ messages in thread
From: George Cherian @ 2020-11-03 17:57 UTC (permalink / raw)
  To: Willem de Bruijn
  Cc: Network Development, linux-kernel, Jakub Kicinski, David Miller,
	Sunil Kovvuri Goutham, Linu Cherian, Geethasowjanya Akula,
	masahiroy@kernel.org

Hi Willem,


> -----Original Message-----
> From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
> Sent: Tuesday, November 3, 2020 7:21 PM
> To: George Cherian <gcherian@marvell.com>
> Cc: Network Development <netdev@vger.kernel.org>; linux-kernel <linux-
> kernel@vger.kernel.org>; Jakub Kicinski <kuba@kernel.org>; David Miller
> <davem@davemloft.net>; Sunil Kovvuri Goutham
> <sgoutham@marvell.com>; Linu Cherian <lcherian@marvell.com>;
> Geethasowjanya Akula <gakula@marvell.com>; masahiroy@kernel.org
> Subject: [EXT] Re: [net-next PATCH 2/3] octeontx2-af: Add devlink health
> reporters for NPA
> 
> > > >  static int rvu_devlink_info_get(struct devlink *devlink, struct
> > > devlink_info_req *req,
> > > >                                 struct netlink_ext_ack *extack)  { @@
> > > > -53,7 +483,8 @@ int rvu_register_dl(struct rvu *rvu)
> > > >         rvu_dl->dl = dl;
> > > >         rvu_dl->rvu = rvu;
> > > >         rvu->rvu_dl = rvu_dl;
> > > > -       return 0;
> > > > +
> > > > +       return rvu_health_reporters_create(rvu);
> > >
> > > when would this be called with rvu->rvu_dl == NULL?
> >
> > During initialization.
> 
> This is the only caller, and it is only reached if rvu_dl is non-zero.

Yes!!! I got it, will address it in v2.

Regards
-George

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [net-next PATCH 2/3] octeontx2-af: Add devlink health reporters for NPA
@ 2020-11-03 17:59 George Cherian
  0 siblings, 0 replies; 15+ messages in thread
From: George Cherian @ 2020-11-03 17:59 UTC (permalink / raw)
  To: Willem de Bruijn
  Cc: Network Development, linux-kernel, Jakub Kicinski, David Miller,
	Sunil Kovvuri Goutham, Linu Cherian, Geethasowjanya Akula,
	masahiroy@kernel.org

Hi Willem,

> -----Original Message-----
> From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
> Sent: Tuesday, November 3, 2020 11:26 PM
> To: George Cherian <gcherian@marvell.com>
> Cc: Network Development <netdev@vger.kernel.org>; linux-kernel <linux-
> kernel@vger.kernel.org>; Jakub Kicinski <kuba@kernel.org>; David Miller
> <davem@davemloft.net>; Sunil Kovvuri Goutham
> <sgoutham@marvell.com>; Linu Cherian <lcherian@marvell.com>;
> Geethasowjanya Akula <gakula@marvell.com>; masahiroy@kernel.org
> Subject: Re: [net-next PATCH 2/3] octeontx2-af: Add devlink health
> reporters for NPA
> 
> On Tue, Nov 3, 2020 at 12:43 PM George Cherian <gcherian@marvell.com>
> wrote:
> >
> > Hi Willem,
> >
> >
> > > -----Original Message-----
> > > From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
> > > Sent: Tuesday, November 3, 2020 7:21 PM
> > > To: George Cherian <gcherian@marvell.com>
> > > Cc: Network Development <netdev@vger.kernel.org>; linux-kernel
> > > <linux- kernel@vger.kernel.org>; Jakub Kicinski <kuba@kernel.org>;
> > > David Miller <davem@davemloft.net>; Sunil Kovvuri Goutham
> > > <sgoutham@marvell.com>; Linu Cherian <lcherian@marvell.com>;
> > > Geethasowjanya Akula <gakula@marvell.com>; masahiroy@kernel.org
> > > Subject: [EXT] Re: [net-next PATCH 2/3] octeontx2-af: Add devlink
> > > health reporters for NPA
> > >
> > > External Email
> > >
> > > --------------------------------------------------------------------
> > > --
> > > > > >  static int rvu_devlink_info_get(struct devlink *devlink,
> > > > > > struct
> > > > > devlink_info_req *req,
> > > > > >                                 struct netlink_ext_ack
> > > > > > *extack)  { @@
> > > > > > -53,7 +483,8 @@ int rvu_register_dl(struct rvu *rvu)
> > > > > >         rvu_dl->dl = dl;
> > > > > >         rvu_dl->rvu = rvu;
> > > > > >         rvu->rvu_dl = rvu_dl;
> > > > > > -       return 0;
> > > > > > +
> > > > > > +       return rvu_health_reporters_create(rvu);
> > > > >
> > > > > when would this be called with rvu->rvu_dl == NULL?
> > > >
> > > > During initialization.
> > >
> > > This is the only caller, and it is only reached if rvu_dl is non-zero.
> >
> > Did you mean to ask, where is it de-initialized?
> > If so, it should be done in rvu_unregister_dl() after freeing rvu_dl.
> 
> No, I meant that rvu_health_reporters_create does not need an !rvu-
> >rvu_dl precondition test, as the only callers calls with with a non-zero
> rvu_dl.

Yes understood!!
Will fix in v2.

Thanks,
-George

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2020-11-03 17:59 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-11-02  5:06 [net-next PATCH 0/3] Add devlink and devlink health reporters to George Cherian
2020-11-02  5:06 ` [net-next PATCH 1/3] octeontx2-af: Add devlink suppoort to af driver George Cherian
2020-11-02 13:31   ` Willem de Bruijn
2020-11-02  5:06 ` [net-next PATCH 2/3] octeontx2-af: Add devlink health reporters for NPA George Cherian
2020-11-02 13:42   ` Willem de Bruijn
2020-11-03  7:26   ` kernel test robot
2020-11-02  5:06 ` [net-next PATCH 3/3] octeontx2-af: Add devlink health reporters for NIX George Cherian
2020-11-03  7:30   ` kernel test robot
2020-11-02 18:00 ` [net-next PATCH 0/3] Add devlink and devlink health reporters to Jakub Kicinski
  -- strict thread matches above, loose matches on Subject: below --
2020-11-03  3:58 [net-next PATCH 2/3] octeontx2-af: Add devlink health reporters for NPA George Cherian
2020-11-03 13:50 ` Willem de Bruijn
2020-11-03 17:43 George Cherian
2020-11-03 17:56 ` Willem de Bruijn
2020-11-03 17:57 George Cherian
2020-11-03 17:59 George Cherian

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).