netdev - RE: [PATCH iwl-next v2] i40e: Add support for fw health report

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CO1PR11MB508983C35CB98A8DD33E7476D6A92@CO1PR11MB5089.namprd11.prod.outlook.com>
Date: Tue, 23 Jul 2024 21:17:14 +0000
From: "Keller, Jacob E" <jacob.e.keller@...el.com>
To: Brett Creeley <bcreeley@....com>, Kamal Heib <kheib@...hat.com>,
	"intel-wired-lan@...ts.osuosl.org" <intel-wired-lan@...ts.osuosl.org>
CC: "netdev@...r.kernel.org" <netdev@...r.kernel.org>, "Nguyen, Anthony L"
	<anthony.l.nguyen@...el.com>, "Kitszel, Przemyslaw"
	<przemyslaw.kitszel@...el.com>, ivecera <ivecera@...hat.com>, Jakub Kicinski
	<kuba@...nel.org>, "David S . Miller" <davem@...emloft.net>, Paolo Abeni
	<pabeni@...hat.com>
Subject: RE: [PATCH iwl-next v2] i40e: Add support for fw health report



> -----Original Message-----
> From: Brett Creeley <bcreeley@....com>
> Sent: Tuesday, July 23, 2024 1:41 PM
> To: Kamal Heib <kheib@...hat.com>; intel-wired-lan@...ts.osuosl.org
> Cc: netdev@...r.kernel.org; Nguyen, Anthony L <anthony.l.nguyen@...el.com>;
> Kitszel, Przemyslaw <przemyslaw.kitszel@...el.com>; ivecera
> <ivecera@...hat.com>; Jakub Kicinski <kuba@...nel.org>; David S . Miller
> <davem@...emloft.net>; Paolo Abeni <pabeni@...hat.com>
> Subject: Re: [PATCH iwl-next v2] i40e: Add support for fw health report
> 
> 
> 
> On 7/18/2024 11:13 AM, Kamal Heib wrote:
> > Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
> >
> >
> > Add support for reporting fw status via the devlink health report.
> >
> > Example:
> >   # devlink health show pci/0000:02:00.0 reporter fw
> >   pci/0000:02:00.0:
> >     reporter fw
> >       state healthy error 0 recover 0
> >   # devlink health diagnose pci/0000:02:00.0 reporter fw
> >   Mode: normal
> >
> > Signed-off-by: Kamal Heib <kheib@...hat.com>
> > ---
> > v2:
> > - Address comments from Jiri.
> > - Move the creation of the health report.
> > ---
> >   drivers/net/ethernet/intel/i40e/i40e.h        |  1 +
> >   .../net/ethernet/intel/i40e/i40e_devlink.c    | 57 +++++++++++++++++++
> >   .../net/ethernet/intel/i40e/i40e_devlink.h    |  2 +
> >   drivers/net/ethernet/intel/i40e/i40e_main.c   | 14 +++++
> >   4 files changed, 74 insertions(+)
> >
> 
> <snip>
> 
> > diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c
> b/drivers/net/ethernet/intel/i40e/i40e_main.c
> > index cbcfada7b357..b6b3ae299b28 100644
> > --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
> > +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
> > @@ -15370,6 +15370,9 @@ static bool i40e_check_recovery_mode(struct
> i40e_pf *pf)
> >                  dev_crit(&pf->pdev->dev, "Firmware recovery mode detected. Limiting
> functionality.\n");
> >                  dev_crit(&pf->pdev->dev, "Refer to the Intel(R) Ethernet Adapters and
> Devices User Guide for details on firmware recovery mode.\n");
> >                  set_bit(__I40E_RECOVERY_MODE, pf->state);
> > +               if (pf->fw_health_report)
> > +                       devlink_health_report(pf->fw_health_report,
> > +                                             "recovery mode detected", pf);
> >
> >                  return true;
> >          }
> > @@ -15810,6 +15813,13 @@ static int i40e_probe(struct pci_dev *pdev, const
> struct pci_device_id *ent)
> >          if (test_bit(__I40E_RECOVERY_MODE, pf->state))
> >                  return i40e_init_recovery_mode(pf, hw);
> >
> > +       err = i40e_devlink_create_health_reporter(pf);
> > +       if (err) {
> > +               dev_err(&pdev->dev,
> > +                       "Failed to create health reporter %d\n", err);
> > +               goto err_health_reporter;
> 
> Do you really want to fail probe if creating this simple health reporter
> fails?
> 
> Thanks,
> 
> Brett

I agree. I would make this a dev_warn and continue without failure.

Thanks,
Jake