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

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

[Kamal Heib]

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@redhat.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(+)

diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h
index d546567e0286..f94671b6e7c6 100644
--- a/drivers/net/ethernet/intel/i40e/i40e.h
+++ b/drivers/net/ethernet/intel/i40e/i40e.h
@@ -465,6 +465,7 @@ static inline const u8 *i40e_channel_mac(struct i40e_channel *ch)
 struct i40e_pf {
 	struct pci_dev *pdev;
 	struct devlink_port devlink_port;
+	struct devlink_health_reporter *fw_health_report;
 	struct i40e_hw hw;
 	DECLARE_BITMAP(state, __I40E_STATE_SIZE__);
 	struct msix_entry *msix_entries;
diff --git a/drivers/net/ethernet/intel/i40e/i40e_devlink.c b/drivers/net/ethernet/intel/i40e/i40e_devlink.c
index cc4e9e2addb7..8fe64284e8d3 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_devlink.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_devlink.c
@@ -122,6 +122,25 @@ static int i40e_devlink_info_get(struct devlink *dl,
 	return err;
 }
 
+static int i40e_fw_reporter_diagnose(struct devlink_health_reporter *reporter,
+				     struct devlink_fmsg *fmsg,
+				     struct netlink_ext_ack *extack)
+{
+	struct i40e_pf *pf = devlink_health_reporter_priv(reporter);
+
+	if (test_bit(__I40E_RECOVERY_MODE, pf->state))
+		devlink_fmsg_string_pair_put(fmsg, "Mode", "recovery");
+	else
+		devlink_fmsg_string_pair_put(fmsg, "Mode", "normal");
+
+	return 0;
+}
+
+static const struct devlink_health_reporter_ops i40e_fw_reporter_ops = {
+	.name = "fw",
+	.diagnose = i40e_fw_reporter_diagnose,
+};
+
 static const struct devlink_ops i40e_devlink_ops = {
 	.info_get = i40e_devlink_info_get,
 };
@@ -233,3 +252,41 @@ void i40e_devlink_destroy_port(struct i40e_pf *pf)
 {
 	devlink_port_unregister(&pf->devlink_port);
 }
+
+/**
+ * i40e_devlink_create_health_reporter - Create the health reporter for this PF
+ * @pf: the PF to create reporter for
+ *
+ * Create health reporter for this PF.
+ *
+ * Return: zero on success or an error code on failure.
+ **/
+int i40e_devlink_create_health_reporter(struct i40e_pf *pf)
+{
+	struct devlink *devlink = priv_to_devlink(pf);
+	struct device *dev = &pf->pdev->dev;
+	int rc = 0;
+
+	devl_lock(devlink);
+	pf->fw_health_report =
+		devl_health_reporter_create(devlink, &i40e_fw_reporter_ops, 0, pf);
+	if (IS_ERR(pf->fw_health_report)) {
+		rc = PTR_ERR(pf->fw_health_report);
+		dev_err(dev, "Failed to create fw reporter, err = %d\n", rc);
+	}
+	devl_unlock(devlink);
+
+	return rc;
+}
+
+/**
+ * i40e_devlink_destroy_health_reporter - Destroy the health reporter
+ * @pf: the PF to cleanup
+ *
+ * Destroy the health reporter
+ **/
+void i40e_devlink_destroy_health_reporter(struct i40e_pf *pf)
+{
+	if (!IS_ERR_OR_NULL(pf->fw_health_report))
+		devlink_health_reporter_destroy(pf->fw_health_report);
+}
diff --git a/drivers/net/ethernet/intel/i40e/i40e_devlink.h b/drivers/net/ethernet/intel/i40e/i40e_devlink.h
index 469fb3d2ee25..018679094bb5 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_devlink.h
+++ b/drivers/net/ethernet/intel/i40e/i40e_devlink.h
@@ -14,5 +14,7 @@ void i40e_devlink_register(struct i40e_pf *pf);
 void i40e_devlink_unregister(struct i40e_pf *pf);
 int i40e_devlink_create_port(struct i40e_pf *pf);
 void i40e_devlink_destroy_port(struct i40e_pf *pf);
+int i40e_devlink_create_health_reporter(struct i40e_pf *pf);
+void i40e_devlink_destroy_health_reporter(struct i40e_pf *pf);
 
 #endif /* _I40E_DEVLINK_H_ */
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;
+	}
+
 	err = i40e_init_lan_hmc(hw, hw->func_caps.num_tx_qp,
 				hw->func_caps.num_rx_qp, 0, 0);
 	if (err) {
@@ -16175,6 +16185,8 @@ static int i40e_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	(void)i40e_shutdown_lan_hmc(hw);
 err_init_lan_hmc:
 	kfree(pf->qp_pile);
+	i40e_devlink_destroy_health_reporter(pf);
+err_health_reporter:
 err_sw_init:
 err_adminq_setup:
 err_pf_reset:
@@ -16209,6 +16221,8 @@ static void i40e_remove(struct pci_dev *pdev)
 
 	i40e_devlink_unregister(pf);
 
+	i40e_devlink_destroy_health_reporter(pf);
+
 	i40e_dbg_pf_exit(pf);
 
 	i40e_ptp_stop(pf);
-- 
2.45.2

Re: [Intel-wired-lan] [PATCH iwl-next v2] i40e: Add support for fw health report

[Paul Menzel]

Dear Kamal,


Thank you for your patch.

Am 18.07.24 um 20:13 schrieb Kamal Heib:
> Add support for reporting fw status via the devlink health report.

I do not know the interface. So, the driver already has some way to 
communicate with the firmware, and you only need to hook it up to devlink?

> 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

Any way to force it into recovery mode to also check it?

> Signed-off-by: Kamal Heib <kheib@redhat.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(+)
> 
> diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h
> index d546567e0286..f94671b6e7c6 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e.h
> +++ b/drivers/net/ethernet/intel/i40e/i40e.h
> @@ -465,6 +465,7 @@ static inline const u8 *i40e_channel_mac(struct i40e_channel *ch)
>   struct i40e_pf {
>   	struct pci_dev *pdev;
>   	struct devlink_port devlink_port;
> +	struct devlink_health_reporter *fw_health_report;
>   	struct i40e_hw hw;
>   	DECLARE_BITMAP(state, __I40E_STATE_SIZE__);
>   	struct msix_entry *msix_entries;
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_devlink.c b/drivers/net/ethernet/intel/i40e/i40e_devlink.c
> index cc4e9e2addb7..8fe64284e8d3 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_devlink.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_devlink.c
> @@ -122,6 +122,25 @@ static int i40e_devlink_info_get(struct devlink *dl,
>   	return err;
>   }
>   
> +static int i40e_fw_reporter_diagnose(struct devlink_health_reporter *reporter,
> +				     struct devlink_fmsg *fmsg,
> +				     struct netlink_ext_ack *extack)
> +{
> +	struct i40e_pf *pf = devlink_health_reporter_priv(reporter);
> +
> +	if (test_bit(__I40E_RECOVERY_MODE, pf->state))
> +		devlink_fmsg_string_pair_put(fmsg, "Mode", "recovery");
> +	else
> +		devlink_fmsg_string_pair_put(fmsg, "Mode", "normal");

I’d use ternary operator.

> +
> +	return 0;
> +}
> +
> +static const struct devlink_health_reporter_ops i40e_fw_reporter_ops = {
> +	.name = "fw",
> +	.diagnose = i40e_fw_reporter_diagnose,
> +};
> +
>   static const struct devlink_ops i40e_devlink_ops = {
>   	.info_get = i40e_devlink_info_get,
>   };
> @@ -233,3 +252,41 @@ void i40e_devlink_destroy_port(struct i40e_pf *pf)
>   {
>   	devlink_port_unregister(&pf->devlink_port);
>   }
> +
> +/**
> + * i40e_devlink_create_health_reporter - Create the health reporter for this PF
> + * @pf: the PF to create reporter for
> + *
> + * Create health reporter for this PF.
> + *
> + * Return: zero on success or an error code on failure.

No dot/period at the end.

> + **/
> +int i40e_devlink_create_health_reporter(struct i40e_pf *pf)
> +{
> +	struct devlink *devlink = priv_to_devlink(pf);
> +	struct device *dev = &pf->pdev->dev;
> +	int rc = 0;
> +
> +	devl_lock(devlink);
> +	pf->fw_health_report =
> +		devl_health_reporter_create(devlink, &i40e_fw_reporter_ops, 0, pf);
> +	if (IS_ERR(pf->fw_health_report)) {
> +		rc = PTR_ERR(pf->fw_health_report);
> +		dev_err(dev, "Failed to create fw reporter, err = %d\n", rc);
> +	}
> +	devl_unlock(devlink);
> +
> +	return rc;
> +}
> +
> +/**
> + * i40e_devlink_destroy_health_reporter - Destroy the health reporter
> + * @pf: the PF to cleanup

The verb is spelled with a space: clean up.

> + *
> + * Destroy the health reporter
> + **/
> +void i40e_devlink_destroy_health_reporter(struct i40e_pf *pf)
> +{
> +	if (!IS_ERR_OR_NULL(pf->fw_health_report))
> +		devlink_health_reporter_destroy(pf->fw_health_report);
> +}
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_devlink.h b/drivers/net/ethernet/intel/i40e/i40e_devlink.h
> index 469fb3d2ee25..018679094bb5 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_devlink.h
> +++ b/drivers/net/ethernet/intel/i40e/i40e_devlink.h
> @@ -14,5 +14,7 @@ void i40e_devlink_register(struct i40e_pf *pf);
>   void i40e_devlink_unregister(struct i40e_pf *pf);
>   int i40e_devlink_create_port(struct i40e_pf *pf);
>   void i40e_devlink_destroy_port(struct i40e_pf *pf);
> +int i40e_devlink_create_health_reporter(struct i40e_pf *pf);
> +void i40e_devlink_destroy_health_reporter(struct i40e_pf *pf);
>   
>   #endif /* _I40E_DEVLINK_H_ */
> 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;
> +	}
> +
>   	err = i40e_init_lan_hmc(hw, hw->func_caps.num_tx_qp,
>   				hw->func_caps.num_rx_qp, 0, 0);
>   	if (err) {
> @@ -16175,6 +16185,8 @@ static int i40e_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>   	(void)i40e_shutdown_lan_hmc(hw);
>   err_init_lan_hmc:
>   	kfree(pf->qp_pile);
> +	i40e_devlink_destroy_health_reporter(pf);
> +err_health_reporter:
>   err_sw_init:
>   err_adminq_setup:
>   err_pf_reset:
> @@ -16209,6 +16221,8 @@ static void i40e_remove(struct pci_dev *pdev)
>   
>   	i40e_devlink_unregister(pf);
>   
> +	i40e_devlink_destroy_health_reporter(pf);
> +
>   	i40e_dbg_pf_exit(pf);
>   
>   	i40e_ptp_stop(pf);


Kind regards,

Paul

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

[Jakub Kicinski]

On Thu, 18 Jul 2024 14:13:19 -0400 Kamal Heib wrote:
> 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

I'm not sure if health reporter is the right fit, depends on what 
the recovery mode is / does. We need documentation.

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

[Brett Creeley]

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@redhat.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

> +       }
> +
>          err = i40e_init_lan_hmc(hw, hw->func_caps.num_tx_qp,
>                                  hw->func_caps.num_rx_qp, 0, 0);
>          if (err) {
> @@ -16175,6 +16185,8 @@ static int i40e_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>          (void)i40e_shutdown_lan_hmc(hw);
>   err_init_lan_hmc:
>          kfree(pf->qp_pile);
> +       i40e_devlink_destroy_health_reporter(pf);
> +err_health_reporter:
>   err_sw_init:
>   err_adminq_setup:
>   err_pf_reset:
> @@ -16209,6 +16221,8 @@ static void i40e_remove(struct pci_dev *pdev)
> 
>          i40e_devlink_unregister(pf);
> 
> +       i40e_devlink_destroy_health_reporter(pf);
> +
>          i40e_dbg_pf_exit(pf);
> 
>          i40e_ptp_stop(pf);
> --
> 2.45.2
> 
> 

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

[Keller, Jacob E]

> -----Original Message-----
> From: Brett Creeley <bcreeley@amd.com>
> Sent: Tuesday, July 23, 2024 1:41 PM
> To: Kamal Heib <kheib@redhat.com>; intel-wired-lan@lists.osuosl.org
> Cc: netdev@vger.kernel.org; Nguyen, Anthony L <anthony.l.nguyen@intel.com>;
> Kitszel, Przemyslaw <przemyslaw.kitszel@intel.com>; ivecera
> <ivecera@redhat.com>; Jakub Kicinski <kuba@kernel.org>; David S . Miller
> <davem@davemloft.net>; Paolo Abeni <pabeni@redhat.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@redhat.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

Re: [Intel-wired-lan] [PATCH iwl-next v2] i40e: Add support for fw health report

[Przemek Kitszel]

On 7/18/24 20:13, Kamal Heib wrote:
> 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@redhat.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(+)
> 

[...]

> +int i40e_devlink_create_health_reporter(struct i40e_pf *pf)
> +{
> +	struct devlink *devlink = priv_to_devlink(pf);
> +	struct device *dev = &pf->pdev->dev;
> +	int rc = 0;
> +
> +	devl_lock(devlink);

if you are going to have just one health reporter, you could just use
devlink_health_reporter_create() and avoid lock+unlock calls here

> +	pf->fw_health_report =
> +		devl_health_reporter_create(devlink, &i40e_fw_reporter_ops, 0, pf);
> +	if (IS_ERR(pf->fw_health_report)) {
> +		rc = PTR_ERR(pf->fw_health_report);
> +		dev_err(dev, "Failed to create fw reporter, err = %d\n", rc);

you are not zeroing pf->fw_health_report here, so ...

> +	}
> +	devl_unlock(devlink);
> +
> +	return rc;
> +}

[snip]

> --- 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,

... you could possibly call devlink_health_report() with ERR_PTR as
the first argument

> +					      "recovery mode detected", pf);
>   
>   		return true;
>   	}