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:

quoted

On Mon, 4 Nov 2019 23:04:50 +0200, Ido Schimmel wrote:  

quoted

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.

quoted

quoted

Also, in case it was not clear, this is a read-only interface and only
from firmware to kernel. No hidden knobs or something fishy like that.  

I'm not saying it's fishy, I'm saying it's way harder to refactor code
if some of the user-visible outputs are not accessible (i.e. hidden in
a binary blob).  

Not sure I understand which code you're referring to? The error print
statement?

The main form of refactoring I'm talking about is pulling out common
driver code into the core so we can make the drivers less gargantuan.

quoted

quoted

quoted

Is there any precedent in the tree for printing FW errors into the logs
like this?    

The mlx5 driver prints a unique number for each firmware error. We tried
to do the same in switch firmware, but it lacked the infrastructure so
we decided on this solution instead. It achieves the same goal, but in a
different way.  

FWIW nfp FW also passes error numbers to the driver and based on that
driver makes decisions and prints errors of its own choosing. The big
difference being you can see all the relevant errors by looking at
driver code.  

This is done by mlxsw as well. See:
$ vi drivers/net/ethernet/mellanox/mlxsw/emad.h +50

But by far the most common error is "bad parameter" in which case we
would like to know exactly what is "bad" and why.

Right, when intelligence is placed in the FW, and the kernel doesn't
know which parameters may be bad then there is a need for FW log
reporting.. I am painfully familiar. FW engineers like to use terms
like "forward compatible" and "future proof".

Anyway, it's already tomorrow here, so I'll be back in a few hours
(IOW: expect some delay).