netdev - Re: [PATCH net-next v7] ethtool: add netlink based get rss support

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: <20221207093248.x6dwbcdxkgaqb6zh@lion.mk-sys.cz>
Date:   Wed, 7 Dec 2022 10:32:48 +0100
From:   Michal Kubecek <mkubecek@...e.cz>
To:     Leon Romanovsky <leon@...nel.org>
Cc:     Jakub Kicinski <kuba@...nel.org>,
        Sudheer Mogilappagari <sudheer.mogilappagari@...el.com>,
        netdev@...r.kernel.org, andrew@...n.ch, corbet@....net,
        sridhar.samudrala@...el.com, anthony.l.nguyen@...el.com
Subject: Re: [PATCH net-next v7] ethtool: add netlink based get rss support

On Wed, Dec 07, 2022 at 10:42:38AM +0200, Leon Romanovsky wrote:
> On Tue, Dec 06, 2022 at 05:14:41PM +0100, Michal Kubecek wrote:
> > 
> >   - avoiding the inherently racy get/modify/set cycle
> 
> How? IMHO, it is achieved in netlink by holding relevant locks, it can
> be rtnl lock or specific to that netlink interface lock (devl). You cam
> and should have same locking protection for legacy flow as well.

What I had in mind is changing only one (or few) of the parameters which
are passed in a structure via ioctl interface, i.e. commands like

  ethtool -G eth0 rx 2048

To do that with ioctl interface, userspace needs to fetch the whole
ethtool_ringparam structure with ETHTOOL_GRINGPARAM first, modify its
rx_pending member and pass the structure back with ETHTOOL_SRINGPARAM.
Obviously you cannot hold a kernel lock over multiple ioctl() syscall.

In some cases, there is a special with "no change" meaning but that is
rather an exception. It would be possible to work around the problem
using some "version counter" that would kernel check against its own
(and reject the update if they do not match) but introducing that would
also be a backward incompatible change.

Michal