netdev - Re: [PATCH net-next 3/6] net/mlxfw: Improve FSM err message reporting and return codes

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <4ceac9cda5bf9de32254f1b9b395ebeaa7de5097.camel@mellanox.com>
Date:   Fri, 22 Nov 2019 22:17:57 +0000
From:   Saeed Mahameed <saeedm@...lanox.com>
To:     "davem@...emloft.net" <davem@...emloft.net>
CC:     Jiri Pirko <jiri@...lanox.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Ido Schimmel <idosch@...lanox.com>
Subject: Re: [PATCH net-next 3/6] net/mlxfw: Improve FSM err message reporting
 and return codes

On Fri, 2019-11-22 at 21:51 +0000, Saeed Mahameed wrote:
> Report unique and standard error codes corresponding to the specific
> FW flash error and report more detailed error messages to netlink.
> 
> Before:
> $ devlink dev flash pci/0000:05:00.0 file ...
> Error: mlxfw: Firmware flash failed.
> devlink answers: Invalid argument
> 
> After:
> $ devlink dev flash pci/0000:05:00.0 file ...
> Error: mlxfw: Firmware flash failed: pending reset.
> devlink answers: Operation already in progress
> 
> Signed-off-by: Saeed Mahameed <saeedm@...lanox.com>
> Reviewed-by: Ido Schimmel <idosch@...lanox.com>
> Acked-by: Jiri Pirko <jiri@...lanox.com>
> ---
>  .../net/ethernet/mellanox/mlxfw/mlxfw_fsm.c   | 55
> +++++++++++++++++--
>  1 file changed, 49 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlxfw/mlxfw_fsm.c
> b/drivers/net/ethernet/mellanox/mlxfw/mlxfw_fsm.c
> index afcdc579578c..ba1e5b276c54 100644
> --- a/drivers/net/ethernet/mellanox/mlxfw/mlxfw_fsm.c
> +++ b/drivers/net/ethernet/mellanox/mlxfw/mlxfw_fsm.c
> @@ -39,6 +39,52 @@ static const char * const
> mlxfw_fsm_state_err_str[] = {
>  		"unknown error"
>  };
>  
> +static int mlxfw_fsm_state_err(struct netlink_ext_ack *extack,
> +			       enum mlxfw_fsm_state_err fsm_state_err)
> +{
> +#define MLXFW_ERR_PRFX "Firmware flash failed: "
> +#define MLXFW_FSM_STATE_ERR_NL(extack, msg) \
> +	NL_SET_ERR_MSG_MOD((extack), MLXFW_ERR_PRFX msg)
> +
> +	fsm_state_err = min_t(enum mlxfw_fsm_state_err, fsm_state_err,
> +			      MLXFW_FSM_STATE_ERR_MAX);
> +	pr_err(MLXFW_ERR_PRFX "%s\n",
> mlxfw_fsm_state_err_str[fsm_state_err]);
> +	switch (fsm_state_err) {
> +	case MLXFW_FSM_STATE_ERR_ERROR:
> +		MLXFW_FSM_STATE_ERR_NL(extack, "general error");
> +		return -EREMOTEIO;
> +	case MLXFW_FSM_STATE_ERR_REJECTED_DIGEST_ERR:
> +		MLXFW_FSM_STATE_ERR_NL(extack, "component hash
> mismatch");
> +		return -EBADMSG;
> +	case MLXFW_FSM_STATE_ERR_REJECTED_NOT_APPLICABLE:
> +		MLXFW_FSM_STATE_ERR_NL(extack, "component not
> applicable");
> +		return -ENOENT;
> +	case MLXFW_FSM_STATE_ERR_REJECTED_UNKNOWN_KEY:
> +		MLXFW_FSM_STATE_ERR_NL(extack, "unknown key");
> +		return -ENOKEY;
> +	case MLXFW_FSM_STATE_ERR_REJECTED_AUTH_FAILED:
> +		MLXFW_FSM_STATE_ERR_NL(extack, "authentication
> failed");
> +		return -EACCES;
> +	case MLXFW_FSM_STATE_ERR_REJECTED_UNSIGNED:
> +		MLXFW_FSM_STATE_ERR_NL(extack, "component was not
> signed");
> +		return -EKEYREVOKED;
> +	case MLXFW_FSM_STATE_ERR_REJECTED_KEY_NOT_APPLICABLE:
> +		MLXFW_FSM_STATE_ERR_NL(extack, "key not applicable");
> +		return -EKEYREJECTED;
> +	case MLXFW_FSM_STATE_ERR_REJECTED_BAD_FORMAT:
> +		MLXFW_FSM_STATE_ERR_NL(extack, "bad format");
> +		return -ENOEXEC;
> +	case MLXFW_FSM_STATE_ERR_BLOCKED_PENDING_RESET:
> +		MLXFW_FSM_STATE_ERR_NL(extack, "pending reset");
> +		return -EALREADY;
> +	case MLXFW_FSM_STATE_ERR_OK: /* should never happen */
> +	case MLXFW_FSM_STATE_ERR_MAX:
> +		MLXFW_FSM_STATE_ERR_NL(extack, "unknown error");
> +		return -EINVAL;
> +	}

Actually with the introduction of the formattable extack buffer in
first patch this whole switch case can be reduced to 

+       NL_SET_ERR_MSG_MOD(extack,  MLXFW_ERR_PRFX "%s",
+                          mlxfw_fsm_state_err_str[fsm_state_err]);

I will send V2.