netdev - Re: [PATCH net-next 3/6] ethtool: add FEC statistics

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: <20210415082144.260cf3ce@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Date:   Thu, 15 Apr 2021 08:21:44 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Saeed Mahameed <saeed@...nel.org>
Cc:     davem@...emloft.net, netdev@...r.kernel.org,
        michael.chan@...adcom.com, leon@...nel.org, ecree.xilinx@...il.com,
        habetsm.xilinx@...il.com, f.fainelli@...il.com, andrew@...n.ch,
        mkubecek@...e.cz, ariela@...dia.com, corbet@....net,
        linux-doc@...r.kernel.org
Subject: Re: [PATCH net-next 3/6] ethtool: add FEC statistics

On Wed, 14 Apr 2021 23:25:43 -0700 Saeed Mahameed wrote:
> On Tue, 2021-04-13 at 20:44 -0700, Jakub Kicinski wrote:
> > ethtool_link_ksettings *);
> > +       void    (*get_fec_stats)(struct net_device *dev,
> > +                                struct ethtool_fec_stats *fec_stats);  
> 
> why void ? some drivers need to access the FW and it could be an old
> FW/device where the fec stats are not supported.

When stats are not supported just returning is fine. Stats are
initialized to -1, core will not dump them into the netlink message 
if driver didn't assign anything.

> and sometimes e.g. in mlx5 case FW can fail for FW related businesses
> :)..

Can do. I was wondering if the entity reading the stats (from user
space) can do anything useful with the error, and didn't really come 
up with anything other than printing an error. Which the kernel can 
do as well. OTOH if there are multiple stats to read and one of them
fails its probably better to return partial results than fail 
the entire op. Therefore I went for no error - if something fails - 
the stats will be missing.

Does that make any sense? Or do you think errors are rare enough that
it's okay if they are fatal? (with the caveat that -EOPNOTSUPP should
be ignored).