| 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
| ||
|
Message-ID: <ZGXVT5p3SJdYQ48z@gmail.com>
Date: Thu, 18 May 2023 08:35:43 +0100
From: Martin Habets <habetsm.xilinx@...il.com>
To: alejandro.lucero-palau@....com
Cc: netdev@...r.kernel.org, linux-net-drivers@....com, davem@...emloft.net,
kuba@...nel.org, pabeni@...hat.com, edumazet@...gle.com,
martin.habets@....com, edward.cree@....com,
linux-doc@...r.kernel.org, corbet@....net, jiri@...dia.com
Subject: Re: [PATCH net] sfc: fix devlink info error handling
On Thu, May 18, 2023 at 06:48:22AM +0100, alejandro.lucero-palau@....com wrote:
> From: Alejandro Lucero <alejandro.lucero-palau@....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@....com>
Acked-by: Martin Habets <habetsm.xilinx@...il.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
>
Powered by blists - more mailing lists