netdev - RE: [PATCH net-next v5 0/6] 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:
 <CH0PR12MB85809DEB2B232B4628A795A6C9162@CH0PR12MB8580.namprd12.prod.outlook.com>
Date: Fri, 26 Apr 2024 13:16:12 +0000
From: Dan Jurgens <danielj@...dia.com>
To: Paolo Abeni <pabeni@...hat.com>, "netdev@...r.kernel.org"
	<netdev@...r.kernel.org>, "jasowang@...hat.com" <jasowang@...hat.com>,
	"mst@...hat.com" <mst@...hat.com>
CC: "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>, Jiri Pirko
	<jiri@...dia.com>
Subject: RE: [PATCH net-next v5 0/6] Remove RTNL lock protection of CVQ

> From: Paolo Abeni <pabeni@...hat.com>
> Sent: Friday, April 26, 2024 4:54 AM
> To: Dan Jurgens <danielj@...dia.com>; netdev@...r.kernel.org;
> jasowang@...hat.com; mst@...hat.com
> Cc: xuanzhuo@...ux.alibaba.com; virtualization@...ts.linux.dev;
> davem@...emloft.net; edumazet@...gle.com; kuba@...nel.org; Jiri Pirko
> <jiri@...dia.com>
> Subject: Re: [PATCH net-next v5 0/6] Remove RTNL lock protection of CVQ
> 
> On Tue, 2024-04-23 at 06:57 +0300, Daniel Jurgens wrote:
> > 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 dependency by introducing a mutex to
> > protect the control buffer and writing SGs to the control VQ.
> >
> > v5:
> > 	- Changed cvq_lock to a mutex.
> > 	- Changed dim_lock to mutex, because it's held taking
> > 	  the cvq_lock.
> > 	- Use spin/mutex_lock/unlock vs guard macros.
> > v4:
> > 	- Protect dim_enabled with same lock as well intr_coal.
> > 	- Rename intr_coal_lock to dim_lock.
> > 	- Remove some scoped_guard where the error path doesn't
> > 	  have to be in the lock.
> > v3:
> > 	- Changed type of _offloads to __virtio16 to fix static
> > 	  analysis warning.
> > 	- Moved a misplaced hunk to the correct patch.
> > v2:
> > 	- New patch to only process the provided queue in
> > 	  virtnet_dim_work
> > 	- New patch to lock per queue rx coalescing structure.
> 
> I had only some minor comments, possibly overall worth another iteration.
> 
> More importantly, this deserves an explicit ack from the virtio crew.
> @Jason, @Michael: could you please have a look?

Thanks for the review, Paolo. I'll give Jaso and Michael a chance to respond before sending again to address your comments.

> 
> Thanks!
> 
> Paolo