netdev - Re: [PATCH net-next 0/6] mlxsw: Add extended ACK for EMADs

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: <20191105074650.GA14631@splinter>
Date:   Tue, 5 Nov 2019 09:46:50 +0200
From:   Ido Schimmel <idosch@...sch.org>
To:     Jakub Kicinski <jakub.kicinski@...ronome.com>
Cc:     netdev@...r.kernel.org, davem@...emloft.net, jiri@...lanox.com,
        shalomt@...lanox.com, mlxsw@...lanox.com,
        Ido Schimmel <idosch@...lanox.com>
Subject: Re: [PATCH net-next 0/6] mlxsw: Add extended ACK for EMADs

On Mon, Nov 04, 2019 at 03:33:42PM -0800, Jakub Kicinski wrote:
> On Tue, 5 Nov 2019 01:20:36 +0200, Ido Schimmel wrote:
> > On Mon, Nov 04, 2019 at 02:44:19PM -0800, Jakub Kicinski wrote:
> > > On Mon, 4 Nov 2019 23:04:50 +0200, Ido Schimmel wrote:  
> > > > I don't understand the problem. If we get an error from firmware today,
> > > > we have no clue what the actual problem is. With this we can actually
> > > > understand what went wrong. How is it different from kernel passing a
> > > > string ("unstructured data") to user space in response to an erroneous
> > > > netlink request? Obviously it's much better than an "-EINVAL".  
> > > 
> > > The difference is obviously that I can look at the code in the kernel
> > > and understand it. FW code is a black box. Kernel should abstract its
> > > black boxiness away.  
> > 
> > But FW code is still code and it needs to be able to report errors in a
> > way that will aid us in debugging when problems occur. I want meaningful
> > errors from applications regardless if I can read their code or not.
> 
> And the usual way accessing FW logs is through ethtool dumps.

I assume you're referring to set_dump() / get_dump_flag() /
get_dump_data() callbacks?

In our case it's not really a dump. These are errors that are reported
inline to the driver for a specific erroneous operation. We currently
can't retrieve them from firmware later on. Using ethtool means that we
need to store these errors in the driver and then push them to user
space upon get operation. Seems like a stretch to me. Especially when
we're already reporting the error code today and this set merely
augments it with more data to make the error more specific.