netdev - Re: [patch net-next 3/7] mlxfw: Propagate error messages through extack

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190527133327.GA2236@nanopsycho.orion>
Date:   Mon, 27 May 2019 15:33:27 +0200
From:   Jiri Pirko <jiri@...nulli.us>
To:     Jakub Kicinski <jakub.kicinski@...ronome.com>
Cc:     David Ahern <dsahern@...il.com>, netdev@...r.kernel.org,
        davem@...emloft.net, mlxsw@...lanox.com, sthemmin@...rosoft.com,
        saeedm@...lanox.com, leon@...nel.org
Subject: Re: [patch net-next 3/7] mlxfw: Propagate error messages through
 extack

Sat, May 25, 2019 at 02:08:52AM CEST, jakub.kicinski@...ronome.com wrote:
>On Sat, 25 May 2019 00:26:35 +0200, Jiri Pirko wrote:
>> Fri, May 24, 2019 at 05:54:46PM CEST, jakub.kicinski@...ronome.com wrote:
>> >On Fri, 24 May 2019 10:11:10 +0200, Jiri Pirko wrote:  
>> >> Thu, May 23, 2019 at 05:19:46PM CEST, dsahern@...il.com wrote:  
>> >> >On 5/23/19 3:45 AM, Jiri Pirko wrote:    
>> >> >> @@ -57,11 +58,13 @@ static int mlxfw_fsm_state_wait(struct mlxfw_dev *mlxfw_dev, u32 fwhandle,
>> >> >>  	if (fsm_state_err != MLXFW_FSM_STATE_ERR_OK) {
>> >> >>  		pr_err("Firmware flash failed: %s\n",
>> >> >>  		       mlxfw_fsm_state_err_str[fsm_state_err]);
>> >> >> +		NL_SET_ERR_MSG_MOD(extack, "Firmware flash failed");
>> >> >>  		return -EINVAL;
>> >> >>  	}
>> >> >>  	if (curr_fsm_state != fsm_state) {
>> >> >>  		if (--times == 0) {
>> >> >>  			pr_err("Timeout reached on FSM state change");
>> >> >> +			NL_SET_ERR_MSG_MOD(extack, "Timeout reached on FSM state change");    
>> >> >
>> >> >FSM? Is the meaning obvious to users?    
>> >> 
>> >> It is specific to mlx drivers.  
>> >
>> >What does it stand for?  Isn't it just Finite State Machine?  
>> 
>> I believe so.
>
>In which case it doesn't really add much, no?  I second David's request
>to make the messages as easy to understand as possible.  

Well, FSM is something that is used in the code and known. I would
change it to "finite state machine" (which I'm still not sure it
really is) but I don't believe that would bring more info to the user.
Well, nothing. On contrary, a MLX engineer might get confused if customer
sends him the message, because he is used to "FSM" :)

Same with "MFA2" in the other message. I only know it is the format
of the binary, have no clue what it actually stands for (other than
it is version 2).


>
>PSID for better or worse I have previously capitulated on, so I guess
>the ship has indeed sailed there :)
>
>$ grep -A4 psid -- Documentation/networking/devlink-info-versions.rst 
>fw.psid
>=======
>
>Unique identifier of the firmware parameter set.