| Message ID | 20230518054822.20242-1-alejandro.lucero-palau@amd.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | cfcb942863f6fce9266e1957a021e6c7295dee42 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] sfc: fix devlink info error handling | expand |
On Thu, May 18, 2023 at 06:48:22AM +0100, alejandro.lucero-palau@amd.com wrote: > From: Alejandro Lucero <alejandro.lucero-palau@amd.com> > > Avoid early devlink info return if errors arise with MCDI commands > executed for getting the required info from the device. The rationale > is some commands can fail but later ones could still give useful data. > Moreover, some nvram partitions could not be present which needs to be > handled as a non error. > > The specific errors are reported through system messages and if any > error appears, it will be reported generically through extack. > > Fixes 14743ddd2495 (sfc: add devlink info support for ef100) > Signed-off-by: Alejandro Lucero <alejandro.lucero-palau@amd.com> Acked-by: Martin Habets <habetsm.xilinx@gmail.com> > --- > drivers/net/ethernet/sfc/efx_devlink.c | 95 ++++++++++++-------------- > 1 file changed, 45 insertions(+), 50 deletions(-) > > diff --git a/drivers/net/ethernet/sfc/efx_devlink.c b/drivers/net/ethernet/sfc/efx_devlink.c > index 381b805659d3..ef9971cbb695 100644 > --- a/drivers/net/ethernet/sfc/efx_devlink.c > +++ b/drivers/net/ethernet/sfc/efx_devlink.c > @@ -171,9 +171,14 @@ static int efx_devlink_info_nvram_partition(struct efx_nic *efx, > > rc = efx_mcdi_nvram_metadata(efx, partition_type, NULL, version, NULL, > 0); > + > + /* If the partition does not exist, that is not an error. */ > + if (rc == -ENOENT) > + return 0; > + > if (rc) { > - netif_err(efx, drv, efx->net_dev, "mcdi nvram %s: failed\n", > - version_name); > + netif_err(efx, drv, efx->net_dev, "mcdi nvram %s: failed (rc=%d)\n", > + version_name, rc); > return rc; > } > > @@ -187,36 +192,33 @@ static int efx_devlink_info_nvram_partition(struct efx_nic *efx, > static int efx_devlink_info_stored_versions(struct efx_nic *efx, > struct devlink_info_req *req) > { > - int rc; > - > - rc = efx_devlink_info_nvram_partition(efx, req, > - NVRAM_PARTITION_TYPE_BUNDLE, > - DEVLINK_INFO_VERSION_GENERIC_FW_BUNDLE_ID); > - if (rc) > - return rc; > - > - rc = efx_devlink_info_nvram_partition(efx, req, > - NVRAM_PARTITION_TYPE_MC_FIRMWARE, > - DEVLINK_INFO_VERSION_GENERIC_FW_MGMT); > - if (rc) > - return rc; > - > - rc = efx_devlink_info_nvram_partition(efx, req, > - NVRAM_PARTITION_TYPE_SUC_FIRMWARE, > - EFX_DEVLINK_INFO_VERSION_FW_MGMT_SUC); > - if (rc) > - return rc; > - > - rc = efx_devlink_info_nvram_partition(efx, req, > - NVRAM_PARTITION_TYPE_EXPANSION_ROM, > - EFX_DEVLINK_INFO_VERSION_FW_EXPROM); > - if (rc) > - return rc; > + int err; > > - rc = efx_devlink_info_nvram_partition(efx, req, > - NVRAM_PARTITION_TYPE_EXPANSION_UEFI, > - EFX_DEVLINK_INFO_VERSION_FW_UEFI); > - return rc; > + /* We do not care here about the specific error but just if an error > + * happened. The specific error will be reported inside the call > + * through system messages, and if any error happened in any call > + * below, we report it through extack. > + */ > + err = efx_devlink_info_nvram_partition(efx, req, > + NVRAM_PARTITION_TYPE_BUNDLE, > + DEVLINK_INFO_VERSION_GENERIC_FW_BUNDLE_ID); > + > + err |= efx_devlink_info_nvram_partition(efx, req, > + NVRAM_PARTITION_TYPE_MC_FIRMWARE, > + DEVLINK_INFO_VERSION_GENERIC_FW_MGMT); > + > + err |= efx_devlink_info_nvram_partition(efx, req, > + NVRAM_PARTITION_TYPE_SUC_FIRMWARE, > + EFX_DEVLINK_INFO_VERSION_FW_MGMT_SUC); > + > + err |= efx_devlink_info_nvram_partition(efx, req, > + NVRAM_PARTITION_TYPE_EXPANSION_ROM, > + EFX_DEVLINK_INFO_VERSION_FW_EXPROM); > + > + err |= efx_devlink_info_nvram_partition(efx, req, > + NVRAM_PARTITION_TYPE_EXPANSION_UEFI, > + EFX_DEVLINK_INFO_VERSION_FW_UEFI); > + return err; > } > > #define EFX_VER_FLAG(_f) \ > @@ -587,27 +589,20 @@ static int efx_devlink_info_get(struct devlink *devlink, > { > struct efx_devlink *devlink_private = devlink_priv(devlink); > struct efx_nic *efx = devlink_private->efx; > - int rc; > + int err; > > - /* Several different MCDI commands are used. We report first error > - * through extack returning at that point. Specific error > - * information via system messages. > + /* Several different MCDI commands are used. We report if errors > + * happened through extack. Specific error information via system > + * messages inside the calls. > */ > - rc = efx_devlink_info_board_cfg(efx, req); > - if (rc) { > - NL_SET_ERR_MSG_MOD(extack, "Getting board info failed"); > - return rc; > - } > - rc = efx_devlink_info_stored_versions(efx, req); > - if (rc) { > - NL_SET_ERR_MSG_MOD(extack, "Getting stored versions failed"); > - return rc; > - } > - rc = efx_devlink_info_running_versions(efx, req); > - if (rc) { > - NL_SET_ERR_MSG_MOD(extack, "Getting running versions failed"); > - return rc; > - } > + err = efx_devlink_info_board_cfg(efx, req); > + > + err |= efx_devlink_info_stored_versions(efx, req); > + > + err |= efx_devlink_info_running_versions(efx, req); > + > + if (err) > + NL_SET_ERR_MSG_MOD(extack, "Errors when getting device info. Check system messages"); > > return 0; > } > -- > 2.17.1 >
Hello: This patch was applied to netdev/net.git (main) by David S. Miller <davem@davemloft.net>: On Thu, 18 May 2023 06:48:22 +0100 you wrote: > From: Alejandro Lucero <alejandro.lucero-palau@amd.com> > > Avoid early devlink info return if errors arise with MCDI commands > executed for getting the required info from the device. The rationale > is some commands can fail but later ones could still give useful data. > Moreover, some nvram partitions could not be present which needs to be > handled as a non error. > > [...] Here is the summary with links: - [net] sfc: fix devlink info error handling https://git.kernel.org/netdev/net/c/cfcb942863f6 You are awesome, thank you!
diff --git a/drivers/net/ethernet/sfc/efx_devlink.c b/drivers/net/ethernet/sfc/efx_devlink.c index 381b805659d3..ef9971cbb695 100644 --- a/drivers/net/ethernet/sfc/efx_devlink.c +++ b/drivers/net/ethernet/sfc/efx_devlink.c @@ -171,9 +171,14 @@ static int efx_devlink_info_nvram_partition(struct efx_nic *efx, rc = efx_mcdi_nvram_metadata(efx, partition_type, NULL, version, NULL, 0); + + /* If the partition does not exist, that is not an error. */ + if (rc == -ENOENT) + return 0; + if (rc) { - netif_err(efx, drv, efx->net_dev, "mcdi nvram %s: failed\n", - version_name); + netif_err(efx, drv, efx->net_dev, "mcdi nvram %s: failed (rc=%d)\n", + version_name, rc); return rc; } @@ -187,36 +192,33 @@ static int efx_devlink_info_nvram_partition(struct efx_nic *efx, static int efx_devlink_info_stored_versions(struct efx_nic *efx, struct devlink_info_req *req) { - int rc; - - rc = efx_devlink_info_nvram_partition(efx, req, - NVRAM_PARTITION_TYPE_BUNDLE, - DEVLINK_INFO_VERSION_GENERIC_FW_BUNDLE_ID); - if (rc) - return rc; - - rc = efx_devlink_info_nvram_partition(efx, req, - NVRAM_PARTITION_TYPE_MC_FIRMWARE, - DEVLINK_INFO_VERSION_GENERIC_FW_MGMT); - if (rc) - return rc; - - rc = efx_devlink_info_nvram_partition(efx, req, - NVRAM_PARTITION_TYPE_SUC_FIRMWARE, - EFX_DEVLINK_INFO_VERSION_FW_MGMT_SUC); - if (rc) - return rc; - - rc = efx_devlink_info_nvram_partition(efx, req, - NVRAM_PARTITION_TYPE_EXPANSION_ROM, - EFX_DEVLINK_INFO_VERSION_FW_EXPROM); - if (rc) - return rc; + int err; - rc = efx_devlink_info_nvram_partition(efx, req, - NVRAM_PARTITION_TYPE_EXPANSION_UEFI, - EFX_DEVLINK_INFO_VERSION_FW_UEFI); - return rc; + /* We do not care here about the specific error but just if an error + * happened. The specific error will be reported inside the call + * through system messages, and if any error happened in any call + * below, we report it through extack. + */ + err = efx_devlink_info_nvram_partition(efx, req, + NVRAM_PARTITION_TYPE_BUNDLE, + DEVLINK_INFO_VERSION_GENERIC_FW_BUNDLE_ID); + + err |= efx_devlink_info_nvram_partition(efx, req, + NVRAM_PARTITION_TYPE_MC_FIRMWARE, + DEVLINK_INFO_VERSION_GENERIC_FW_MGMT); + + err |= efx_devlink_info_nvram_partition(efx, req, + NVRAM_PARTITION_TYPE_SUC_FIRMWARE, + EFX_DEVLINK_INFO_VERSION_FW_MGMT_SUC); + + err |= efx_devlink_info_nvram_partition(efx, req, + NVRAM_PARTITION_TYPE_EXPANSION_ROM, + EFX_DEVLINK_INFO_VERSION_FW_EXPROM); + + err |= efx_devlink_info_nvram_partition(efx, req, + NVRAM_PARTITION_TYPE_EXPANSION_UEFI, + EFX_DEVLINK_INFO_VERSION_FW_UEFI); + return err; } #define EFX_VER_FLAG(_f) \ @@ -587,27 +589,20 @@ static int efx_devlink_info_get(struct devlink *devlink, { struct efx_devlink *devlink_private = devlink_priv(devlink); struct efx_nic *efx = devlink_private->efx; - int rc; + int err; - /* Several different MCDI commands are used. We report first error - * through extack returning at that point. Specific error - * information via system messages. + /* Several different MCDI commands are used. We report if errors + * happened through extack. Specific error information via system + * messages inside the calls. */ - rc = efx_devlink_info_board_cfg(efx, req); - if (rc) { - NL_SET_ERR_MSG_MOD(extack, "Getting board info failed"); - return rc; - } - rc = efx_devlink_info_stored_versions(efx, req); - if (rc) { - NL_SET_ERR_MSG_MOD(extack, "Getting stored versions failed"); - return rc; - } - rc = efx_devlink_info_running_versions(efx, req); - if (rc) { - NL_SET_ERR_MSG_MOD(extack, "Getting running versions failed"); - return rc; - } + err = efx_devlink_info_board_cfg(efx, req); + + err |= efx_devlink_info_stored_versions(efx, req); + + err |= efx_devlink_info_running_versions(efx, req); + + if (err) + NL_SET_ERR_MSG_MOD(extack, "Errors when getting device info. Check system messages"); return 0; }