[PATCH] sfc: Add error handling for devlink_info_serial_number_put()

[PATCH] sfc: Add error handling for devlink_info_serial_number_put()

[Wentao Liang]

In  efx_devlink_info_board_cfg(), the return value of
devlink_info_serial_number_put() needs to be checked.
This could result in silent failures if the function failed.

Add error checking for efx_devlink_info_board_cfg() and
propagate any errors immediately to ensure proper
error handling and prevents silent failures.

Fixes: 14743ddd2495 ("sfc: add devlink info support for ef100")
Cc: stable@vger.kernel.org # v6.3+
Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
---
 drivers/net/ethernet/sfc/efx_devlink.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/sfc/efx_devlink.c b/drivers/net/ethernet/sfc/efx_devlink.c
index 3cd750820fdd..17279bbd81d5 100644
--- a/drivers/net/ethernet/sfc/efx_devlink.c
+++ b/drivers/net/ethernet/sfc/efx_devlink.c
@@ -581,12 +581,14 @@ static int efx_devlink_info_board_cfg(struct efx_nic *efx,
 {
 	char sn[EFX_MAX_SERIALNUM_LEN];
 	u8 mac_address[ETH_ALEN];
-	int rc;
+	int rc, err;
 
 	rc = efx_mcdi_get_board_cfg(efx, (u8 *)mac_address, NULL, NULL);
 	if (!rc) {
 		snprintf(sn, EFX_MAX_SERIALNUM_LEN, "%pm", mac_address);
-		devlink_info_serial_number_put(req, sn);
+		err = devlink_info_serial_number_put(req, sn);
+		if (err)
+			return err;
 	}
 	return rc;
 }
-- 
2.42.0.windows.2

Re: [PATCH] sfc: Add error handling for devlink_info_serial_number_put()

[Jakub Kicinski]

On Tue,  1 Apr 2025 21:05:57 +0800 Wentao Liang wrote:
>  	if (!rc) {
>  		snprintf(sn, EFX_MAX_SERIALNUM_LEN, "%pm", mac_address);
> -		devlink_info_serial_number_put(req, sn);
> +		err = devlink_info_serial_number_put(req, sn);
> +		if (err)
> +			return err;

Why are you introducing another variable? You can do this:

	if (rc)
		return rc;
	snprintf...
	return devlink_info..
-- 
pw-bot: cr

Re: [PATCH] sfc: Add error handling for devlink_info_serial_number_put()

[Edward Cree]

On 01/04/2025 14:05, Wentao Liang wrote:
> In  efx_devlink_info_board_cfg(), the return value of
> devlink_info_serial_number_put() needs to be checked.
> This could result in silent failures if the function failed.
> 
> Add error checking for efx_devlink_info_board_cfg() and
> propagate any errors immediately to ensure proper
> error handling and prevents silent failures.

Looking at the rest of the file, all the calls to
 devlink_info_*_put() in this driver ignore the return value, not
 just this one.  I think this may have been an intentional decision
 to only report errors in getting the info from FW, which seems
 reasonable to me.
If not, then all the calls need fixing, not just this one.
CCing Alejandro, original author of this code, for his opinion.

-ed

> Fixes: 14743ddd2495 ("sfc: add devlink info support for ef100")
> Cc: stable@vger.kernel.org # v6.3+
> Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
> ---
>  drivers/net/ethernet/sfc/efx_devlink.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/sfc/efx_devlink.c b/drivers/net/ethernet/sfc/efx_devlink.c
> index 3cd750820fdd..17279bbd81d5 100644
> --- a/drivers/net/ethernet/sfc/efx_devlink.c
> +++ b/drivers/net/ethernet/sfc/efx_devlink.c
> @@ -581,12 +581,14 @@ static int efx_devlink_info_board_cfg(struct efx_nic *efx,
>  {
>  	char sn[EFX_MAX_SERIALNUM_LEN];
>  	u8 mac_address[ETH_ALEN];
> -	int rc;
> +	int rc, err;
>  
>  	rc = efx_mcdi_get_board_cfg(efx, (u8 *)mac_address, NULL, NULL);
>  	if (!rc) {
>  		snprintf(sn, EFX_MAX_SERIALNUM_LEN, "%pm", mac_address);
> -		devlink_info_serial_number_put(req, sn);
> +		err = devlink_info_serial_number_put(req, sn);
> +		if (err)
> +			return err;
>  	}
>  	return rc;
>  }
>