netdev - Re: [PATCH net-next 0/4] Remove RTNL lock protection of CVQ

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: <95369596-191b-4cd3-9262-26aff7558561@linux.alibaba.com>
Date: Tue, 26 Mar 2024 14:05:41 +0800
From: Heng Qi <hengqi@...ux.alibaba.com>
To: Dan Jurgens <danielj@...dia.com>,
 "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Cc: "mst@...hat.com" <mst@...hat.com>,
 "jasowang@...hat.com" <jasowang@...hat.com>,
 "xuanzhuo@...ux.alibaba.com" <xuanzhuo@...ux.alibaba.com>,
 "virtualization@...ts.linux.dev" <virtualization@...ts.linux.dev>,
 "davem@...emloft.net" <davem@...emloft.net>,
 "edumazet@...gle.com" <edumazet@...gle.com>,
 "kuba@...nel.org" <kuba@...nel.org>, "pabeni@...hat.com"
 <pabeni@...hat.com>, Jiri Pirko <jiri@...dia.com>
Subject: Re: [PATCH net-next 0/4] Remove RTNL lock protection of CVQ



在 2024/3/26 下午12:11, Dan Jurgens 写道:
>> From: Heng Qi <hengqi@...ux.alibaba.com>
>> Sent: Monday, March 25, 2024 9:54 PM
>> To: Dan Jurgens <danielj@...dia.com>; netdev@...r.kernel.org
>> Cc: mst@...hat.com; jasowang@...hat.com; xuanzhuo@...ux.alibaba.com;
>> virtualization@...ts.linux.dev; davem@...emloft.net;
>> edumazet@...gle.com; kuba@...nel.org; pabeni@...hat.com; Jiri Pirko
>> <jiri@...dia.com>
>> Subject: Re: [PATCH net-next 0/4] Remove RTNL lock protection of CVQ
>>
>>
>>
>> 在 2024/3/26 上午5:49, Daniel Jurgens 写道:
>>> Currently the buffer used for control VQ commands is protected by the
>>> RTNL lock. Previously this wasn't a major concern because the control
>>> VQ was only used during device setup and user interaction. With the
>>> recent addition of dynamic interrupt moderation the control VQ may be
>>> used frequently during normal operation.
>>>
>>> This series removes the RNTL lock dependancy by introducing a spin
>>> lock to protect the control buffer and writing SGs to the control VQ.
>> Hi Daniel.
>>
>> It's a nice piece of work, but now that we're talking about ctrlq adding
>> interrupts, spin lock has some conflicts with its goals. For example, we expect
>> the ethtool command to be blocked.
>> Therefore, a mutex lock may be more suitable.
>>
>> Any how, the final conclusion may require some waiting.
> Thanks, Heng
>
> I took this a step further and made the ctrlq interrupt driven, but an internal reviewer pointed me to this:
> https://lore.kernel.org/lkml/20230413064027.13267-1-jasowang@redhat.com/ (sorry if it gets safelinked)
>
> It seemed there was little appetite to go that route last year, because of set RX mode behavior change, and consumption of an additional IRQ.

Hi DanielJ.

Jason now supports this and wants to make changes to ctrlq.

Yes, our requirements for ctrlq have become higher and we need to make 
updates as expected.

So your patches look good:

         Reviewed-by: Heng Qi <hengqi@...ux.alibaba.com>

I will make further modifications on top of these.

Regards,
Heng

>
> Either way, I think the spin lock is still needed. In my interrupt driven implantation I was allocating a new control buffer instead of just the data fields. The spin lock was tighter around virtqueue_add_sgs, after the kick it would unlock and wait for a completion that would be triggered from the cvq callback.
>
>
>> Regards,
>> Heng
>>
>>> Daniel Jurgens (4):
>>>     virtio_net: Store RSS setting in virtnet_info
>>>     virtio_net: Remove command data from control_buf
>>>     virtio_net: Add a lock for the command VQ.
>>>     virtio_net: Remove rtnl lock protection of command buffers
>>>
>>>    drivers/net/virtio_net.c | 185 ++++++++++++++++++++++-----------------
>>>    1 file changed, 104 insertions(+), 81 deletions(-)
>>>