netdev - Re: [net-next 09/15] net/mlx5: Create FW devlink health reporter

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:   Mon, 6 May 2019 10:45:44 +0000
From:   Moshe Shemesh <moshe@...lanox.com>
To:     Jiri Pirko <jiri@...nulli.us>, Saeed Mahameed <saeedm@...lanox.com>
CC:     "David S. Miller" <davem@...emloft.net>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Jiri Pirko <jiri@...lanox.com>,
        Eran Ben Elisha <eranbe@...lanox.com>
Subject: Re: [net-next 09/15] net/mlx5: Create FW devlink health reporter



On 5/5/2019 6:42 PM, Jiri Pirko wrote:
> Sun, May 05, 2019 at 02:33:23AM CEST, saeedm@...lanox.com wrote:
>> From: Moshe Shemesh <moshe@...lanox.com>
>>
>> Create mlx5_devlink_health_reporter for FW reporter. The FW reporter
>> implements devlink_health_reporter diagnose callback.
>>
>> The fw reporter diagnose command can be triggered any time by the user
>> to check current fw status.
>> In healthy status, it will return clear syndrome. Otherwise it will dump
>> the health info buffer.
>>
>> Command example and output on healthy status:
>> $ devlink health diagnose pci/0000:82:00.0 reporter fw
>> Syndrome: 0
>>
>> Command example and output on non healthy status:
>> $ devlink health diagnose pci/0000:82:00.0 reporter fw
>> diagnose data:
>> assert_var[0] 0xfc3fc043
>> assert_var[1] 0x0001b41c
>> assert_var[2] 0x00000000
>> assert_var[3] 0x00000000
>> assert_var[4] 0x00000000
>> assert_exit_ptr 0x008033b4
>> assert_callra 0x0080365c
>> fw_ver 16.24.1000
>> hw_id 0x0000020d
>> irisc_index 0
>> synd 0x8: unrecoverable hardware error
>> ext_synd 0x003d
>> raw fw_ver 0x101803e8
>>
>> Signed-off-by: Moshe Shemesh <moshe@...lanox.com>
>> Signed-off-by: Eran Ben Elisha <eranbe@...lanox.com>
>> Signed-off-by: Saeed Mahameed <saeedm@...lanox.com>
> 
> 	
> [...]	
> 	
> 	
>> +static int
>> +mlx5_fw_reporter_diagnose(struct devlink_health_reporter *reporter,
>> +			  struct devlink_fmsg *fmsg)
>> +{
>> +	struct mlx5_core_dev *dev = devlink_health_reporter_priv(reporter);
>> +	struct mlx5_core_health *health = &dev->priv.health;
>> +	u8 synd;
>> +	int err;
>> +
>> +	mutex_lock(&health->info_buf_lock);
>> +	mlx5_get_health_info(dev, &synd);
>> +
>> +	if (!synd) {
>> +		mutex_unlock(&health->info_buf_lock);
>> +		return devlink_fmsg_u8_pair_put(fmsg, "Syndrome", synd);
>> +	}
>> +
>> +	err = devlink_fmsg_string_pair_put(fmsg, "diagnose data",
>> +					   health->info_buf);
> 
> No! This is wrong! You are sneaking in text blob. Please put the info in
> structured form using proper fmsg helpers.
> 
This is the fw output format, it is already in use, I don't want to 
change it, just have it here in the diagnose output too.