Re: [PATCH net-next 3/6] ethtool: add FEC statistics
From: Saeed Mahameed <saeed@kernel.org>
Date: 2021-04-15 22:33:24
Also in:
linux-doc
On Thu, 2021-04-15 at 08:21 -0700, Jakub Kicinski wrote:
On Wed, 14 Apr 2021 23:25:43 -0700 Saeed Mahameed wrote:quoted
On Tue, 2021-04-13 at 20:44 -0700, Jakub Kicinski wrote:quoted
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.quoted
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).
Agreed, Thanks for the explanation but you still need to handle the error internally in the driver, otherwise the command returns garbage or 0 if you didn't check return status.