linux-kernel - RE: [PATCH net] net: mana: Fix doorbell out of order violation and avoid unnecessary doorbell rings

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]

Message-ID:
 <DM4PR21MB3536C872C817868E2F6DC9E1CEB82@DM4PR21MB3536.namprd21.prod.outlook.com>
Date: Wed, 7 Aug 2024 15:26:55 +0000
From: Long Li <longli@...rosoft.com>
To: Dexuan Cui <decui@...rosoft.com>, KY Srinivasan <kys@...rosoft.com>,
	Haiyang Zhang <haiyangz@...rosoft.com>, Wei Liu <wei.liu@...nel.org>, "David
 S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Jakub
 Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Shradha Gupta
	<shradhagupta@...ux.microsoft.com>, Simon Horman <horms@...nel.org>,
	Konstantin Taranov <kotaranov@...rosoft.com>, Souradeep Chakrabarti
	<schakrabarti@...ux.microsoft.com>, Erick Archer <erick.archer@...look.com>,
	"linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-rdma@...r.kernel.org" <linux-rdma@...r.kernel.org>
CC: "stable@...r.kernel.org" <stable@...r.kernel.org>
Subject: RE: [PATCH net] net: mana: Fix doorbell out of order violation and
 avoid unnecessary doorbell rings

> Subject: RE: [PATCH net] net: mana: Fix doorbell out of order violation and avoid
> unnecessary doorbell rings
> 
> > From: longli@...uxonhyperv.com <longli@...uxonhyperv.com>
> > Sent: Monday, August 5, 2024 4:38 PM
> > [...]
> > After napi_complete_done() is called, another NAPI may be running on
> > another CPU and ring the doorbell before the current CPU does. When
> 
> Can you please share more details about "another NAPI"? Is it about busy_poll?
> 
> > combined with unnecessary rings when there is no need to ARM the CQ,
> > this triggers error paths in the hardware.
> >
> > Fix this by always ring the doorbell in sequence and avoid unnecessary
> > rings.
> 
> I'm not sure what "error paths in the hardware" means. It's better to describe the
> user-visible consequence.
> 
> Maybe this is clearer:
> 
> When there is no need to arm the CQ from NAPI's perspective, the driver must
> not combine "too many" arming operations due to a MANA hardware
> requirement:
> the driver must ring the doorbell at least once within every 8 wraparounds of the
> CQ, otherwise "XXX" would happen. //Dexuan: I don't know what the "XXX" is
> 
> Add a per-CQ counter cq->work_done_since_doorbell, and make sure the CQ is
> armed within 4 wraparounds of the CQ. //Dexuan: why not 8 or 7?

I'm sending v2 to address the details in the comments.

> 
> 
> > +	if (w < cq->budget) {
> > +		mana_gd_ring_cq(gdma_queue, SET_ARM_BIT);
> > +		cq->work_done_since_doorbell = 0;
> > +		napi_complete_done(&cq->napi, w);
> > +	} else if (cq->work_done_since_doorbell >
> > +		   cq->gdma_cq->queue_size / COMP_ENTRY_SIZE * 4) {
> > +		/* MANA hardware requires at least one doorbell ring every 8
> s/ring every 8/arming within every 8/ ?
> 
> > +		 * wraparounds of CQ even there is no need to ARM. This
> > driver
> 
> s/ARM/arming/ ?
> s/even/even if/ ?

Will fix this in v2.

Thanks,

Long