netdev - Re: [PATCH net] net-sysfs: update the queue counts in the unregistration path

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] [day] [month] [year] [list]

Date:   Wed, 01 Dec 2021 18:03:33 +0100
From:   Antoine Tenart <atenart@...nel.org>
To:     Jakub Kicinski <kuba@...nel.org>
Cc:     davem@...emloft.net, alexander.duyck@...il.com,
        netdev@...r.kernel.org
Subject: Re: [PATCH net] net-sysfs: update the queue counts in the unregistration path

Quoting Jakub Kicinski (2021-12-01 16:05:13)
> On Wed, 01 Dec 2021 10:32:01 +0100 Antoine Tenart wrote:
> > > Would you mind pointing where in the code that happens? I can't seem 
> > > to find anything looking at real_num_.x_queues outside dev.c and
> > > net-sysfs.c :S  
> > 
> > The above trace was triggered using veths and this patch would solve
> > this as veths do use real_num_x_queues to fill 'struct ethtool_channels'
> > in its get_channels ops[1] which is then used to avoid making channel
> > counts updates if it is 0[2].
> 
> But when we are at line 175 in [2] we already updated the values from
> the user space request at lines 144-151. This check validates the new
> config so a transition from 0 -> n should not be prevented here AFAICT.

You're right. It worked in my testbed because I was not providing the
number of Rx channels with Ethtool, so channels.rx_counts wasn't updated
and still had the value veth provided. It "fixed" the issue for a
completely unrelated reason and obviously can't be a fix for the issue
described here. Thanks for having a second look at this, I completely
missed it...

> Any way of fixing this is fine. If you ask me personally I'd probably
> go with the ethtool fix to net and the zeroing and warn to net-next.
> Unless I'm misreading and this fix does work, in which case your plan
> is good, too.

I think you're right. Let's target net with the ethtool patch[1]. This
patch (still needed to keep track of the queue counts) and the warning
one can target net-next.

[1] And probably another one for the old ethtool ioctl interface too.

Thanks!
Antoine