netdev - Re: [RFC PATCH net-next 3/3] net/mlx5e: Expose link down reason to ethtool

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: <d812f75d-9ef1-99e0-527a-7a15be9a47a7@mellanox.com>
Date:   Fri, 23 Jun 2017 11:52:43 +0300
From:   Gal Pressman <galp@...lanox.com>
To:     Jakub Kicinski <kubakici@...pl>
Cc:     Gal Pressman <galp@...lanox.com>, netdev@...r.kernel.org,
        "David S. Miller" <davem@...emloft.net>,
        "John W. Linville" <linville@...driver.com>,
        Saeed Mahameed <saeedm@...lanox.com>,
        Vidya Sagar Ravipati <vidya@...ulusnetworks.com>,
        Jiri Pirko <jiri@...lanox.com>,
        David Decotigny <decot@...glers.com>, kernel-team@...com
Subject: Re: [RFC PATCH net-next 3/3] net/mlx5e: Expose link down reason to
 ethtool


> On Thu, 22 Jun 2017 11:33:39 +0300, Gal Pressman wrote:
>>> Is my reading correct that in case the reason is not in the
>>> pddr2ethtool_table opaque binary data will be passed from the firmware
>>> to user space?  Is there any particular reason to allow for this?  If
>>> it's just for the rare scenario where a new error code needs to be
>>> added perhaps it would be enough to dump the FW-provided message to the
>>> logs?  
>> No binary data is passed in this patch, the monitor_opcode is simply a vendor specific
>> 16 bit id that is used when the reason  is not generic enough to have it's own enum.
> Sorry if I'm wrong, I thought this would potentially copy
> ETH_GSTRING_LEN bytes to userspace:
>
> +	if (status_message)
> +		memcpy(status_message,
> +		       MLX5_ADDR_OF(pddr_reg, out, page_data.troubleshooting_info_page.status_message),
> +		       ETH_GSTRING_LEN);
>
> I'm also still not sure why a reason would not be generic enough for
> the enum, if it fits in the 16bit vendor enum... 
Some reasons might be very specific, and only relevant to my hardware for example.
In that case I'd like to avoid an enum entry that is not wide enough to be used by anyone else but me,
and still have a way to pass it to the user.

The userspace patches will allow for a vendor parser (similar to ethtool regdump parsers) that can
convert this 16bit id to a string.