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]

Date:   Fri, 24 May 2019 10:11:10 +0200
From:   Jiri Pirko <jiri@...nulli.us>
To:     David Ahern <dsahern@...il.com>
Cc:     netdev@...r.kernel.org, davem@...emloft.net, mlxsw@...lanox.com,
        jakub.kicinski@...ronome.com, sthemmin@...rosoft.com,
        saeedm@...lanox.com, leon@...nel.org
Subject: Re: [patch net-next 3/7] mlxfw: Propagate error messages through
 extack

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. But I think it is valuable to have
driver-specific terms in driver speficic extack messages.


>
>>  			return -ETIMEDOUT;
>>  		}
>>  		msleep(MLXFW_FSM_STATE_WAIT_CYCLE_MS);
>> @@ -76,7 +79,8 @@ static int mlxfw_fsm_state_wait(struct mlxfw_dev *mlxfw_dev, u32 fwhandle,
>>  
>>  static int mlxfw_flash_component(struct mlxfw_dev *mlxfw_dev,
>>  				 u32 fwhandle,
>> -				 struct mlxfw_mfa2_component *comp)
>> +				 struct mlxfw_mfa2_component *comp,
>> +				 struct netlink_ext_ack *extack)
>>  {
>>  	u16 comp_max_write_size;
>>  	u8 comp_align_bits;
>> @@ -96,6 +100,7 @@ static int mlxfw_flash_component(struct mlxfw_dev *mlxfw_dev,
>>  	if (comp->data_size > comp_max_size) {
>>  		pr_err("Component %d is of size %d which is bigger than limit %d\n",
>>  		       comp->index, comp->data_size, comp_max_size);
>> +		NL_SET_ERR_MSG_MOD(extack, "Component is which is bigger than limit");
>
>Need to drop 'is which'.

Will do.

>
>
>...
>
>> @@ -156,6 +163,7 @@ static int mlxfw_flash_components(struct mlxfw_dev *mlxfw_dev, u32 fwhandle,
>>  					      &component_count);
>>  	if (err) {
>>  		pr_err("Could not find device PSID in MFA2 file\n");
>> +		NL_SET_ERR_MSG_MOD(extack, "Could not find device PSID in MFA2 file");
>
>same here, is PSID understood by user?

PSID is actually exposed in "devlink dev info" for mlxsw.

>