netdev - Re: ethtool discrepancy between observed behaviour and comments

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: <20210615124050.50138c05@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Date:   Tue, 15 Jun 2021 12:40:50 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Kev Jackson <foamdino@...il.com>
Cc:     mkubecek@...e.cz, netdev@...r.kernel.org
Subject: Re: ethtool discrepancy between observed behaviour and comments

On Tue, 15 Jun 2021 07:30:39 +0100 Kev Jackson wrote:
> Hi,
> 
> I have been working with ethtool, ioctl and Mellanox multi-queue NICs and I think
> I have discovered an issue with either the code or the comments that describe
> the code or recent changes.
> 
> My focus here is simply the ethtool -L command to set the channels for a
> multi-queue nic.  Running this command with the following params on a host with
> a Mellanox NIC works fine:
> 
> ethtool -L eth0 combined 4

> From putting all of this together, I have come to the conclusion that:
> * ioctl / ETHTOOL_SCHANNELS is a legacy method of setting channels
> * nl_schannels is the new / preferred method of setting channels
> * ethtool has fallback code to run ioctl functions for commands which don't yet
> * have a netlink equivalent
> 
> Our user experience is that ethtool -L currently does support (and should continue to
> support) just setting combined_count rather than having to set combined_count +
> one of rx_count/tx_count, which would mean removing the check in the ioctl.c,
> ethtool_set_channels code to make the netlink.c and ioctl.c commands consistent.
> 
> Obviously the other approach is to add the check for setting one of rx_count /
> tx_count into the nl_schannels function.
> 
> We're happy to provide a patch for either approach, but would like to raise this
> as potentially a bug in the current code.

I'm not sure I grasped what the problem is. Could you perhaps share
what you're trying to do that works with netlink vs IOCTL? Best if
it's in form of:

$ ethtool -l $ifc
$ ./ethtool-ioctl -L $ifc ...
# presumably fails IIUC?
$ ./ethtool-nl -L $ifc ...
# and this one succeeds?

where ethtool-ioctl and ethtool-nl would be hacked up ethtool binaries
which only use one mechanism instead of trying to autoselect.