netdev - Re: [net-next, v6, 6/7] net-sysfs: Add interface for Rx queue(s) map per Tx queue

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: <20180802001157.GA29171@outlook.office365.com>
Date:   Wed, 1 Aug 2018 17:11:59 -0700
From:   Andrei Vagin <avagin@...tuozzo.com>
To:     "Nambiar, Amritha" <amritha.nambiar@...el.com>
Cc:     netdev@...r.kernel.org, davem@...emloft.net,
        alexander.h.duyck@...el.com, willemdebruijn.kernel@...il.com,
        sridhar.samudrala@...el.com, alexander.duyck@...il.com,
        edumazet@...gle.com, hannes@...essinduktion.org,
        tom@...bertland.com, tom@...ntonium.net, jasowang@...hat.com,
        gaowanlong@...fujitsu.com, "Michael S. Tsirkin" <mst@...hat.com>,
        virtualization@...ts.linux-foundation.org
Subject: Re: [net-next, v6, 6/7] net-sysfs: Add interface for Rx queue(s) map
 per Tx queue

On Tue, Jul 10, 2018 at 07:28:49PM -0700, Nambiar, Amritha wrote:
> With this patch series, I introduced static_key for XPS maps
> (xps_needed), so static_key_slow_inc() is used to switch branches. The
> definition of static_key_slow_inc() has cpus_read_lock in place. In the
> virtio_net driver, XPS queues are initialized after setting the
> queue:cpu affinity in virtnet_set_affinity() which is already protected
> within cpus_read_lock. Hence, the warning here trying to acquire
> cpus_read_lock when it is already held.
> 
> A quick fix for this would be to just extract netif_set_xps_queue() out
> of the lock by simply wrapping it with another put/get_online_cpus
> (unlock right before and hold lock right after).

virtnet_set_affinity() is called from virtnet_cpu_online(), which is
called under cpus_read_lock too.

It looks like now we can't call netif_set_xps_queue() from cpu hotplug
callbacks.

I can suggest a very straightforward fix for this problem. The patch is
attached.

> But this may not a
> clean solution. It'd help if I can get suggestions on what would be a
> clean option to fix this without extensively changing the code in
> virtio_net. Is it mandatory to protect the affinitization with
> read_lock? I don't see similar lock in other drivers while setting the
> affinity. I understand this warning should go away, but isn't it safe to
> have multiple readers.
> 
> > On Fri, Jun 29, 2018 at 09:27:07PM -0700, Amritha Nambiar wrote:

View attachment "patch" of type "text/plain" (4259 bytes)