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: <20170622153800.67f9635f@cakuba.netronome.com>
Date:   Thu, 22 Jun 2017 15:38:00 -0700
From:   Jakub Kicinski <kubakici@...pl>
To:     Gal Pressman <galp.dev@...il.com>
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...