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

Paul Menzel pmenzel at molgen.mpg.de
Thu Jul 18 19:57:43 UTC 2024


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


More information about the Intel-wired-lan mailing list