netdev - Re: [PATCH net-next v7 5/6] net: ngbe: add sriov function support

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: <DF81ED4C-F36A-4D6C-8993-0389E2F39615@net-swift.com>
Date: Wed, 12 Feb 2025 19:06:52 +0800
From: "mengyuanlou@...-swift.com" <mengyuanlou@...-swift.com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: netdev@...r.kernel.org,
 jiawenwu@...stnetic.com,
 duanqiangwen@...-swift.com
Subject: Re: [PATCH net-next v7 5/6] net: ngbe: add sriov function support



> 2025年2月12日 06:06，Jakub Kicinski <kuba@...nel.org> 写道：
> 
> On Tue, 11 Feb 2025 19:14:54 +0800 mengyuanlou@...-swift.com wrote:
>> Due to hardware design, when 6 vfs are assigned. 
>> +------------------------------------------------------------+
>> |        | pf | pf  | vf5 | vf4 | vf3 | vf2 | vf1 | vf0 | pf |
>> |--------|----|-----|-----|-----|-----|-----|-----|-----|----|
>> | vector | 0  | 1   | 2   | 3   | 4   | 5   | 6   | 7   | 8  |
>> +------------------------------------------------------------+
>> 
>> When 7 vfs are assigned. 
>> +------------------------------------------------------------+
>> |        | pf | vf6 | vf5 | vf4 | vf3 | vf2 | vf1 | vf0 | pf |
>> |--------|----|-----|-----|-----|-----|-----|-----|-----|----|
>> | vector | 0  | 1   | 2   | 3   | 4   | 5   | 6   | 7   | 8  |
>> +------------------------------------------------------------+
>> 
>> When num_vfs < 7, pf can use 0 for misc and 1 for queue.
>> But when num_vfs == 7, vector 1 is assigned to vf6.
>> 1. Alloc 9 irq vectors, but only request_irq for 0 and 8. 
>> 2. Reuse interrupt vector 0.
> 
> Do you have proper synchronization in place to make sure IRQs
> don't get mis-routed when SR-IOV is enabled?

+ 	q_vector = wx->q_vector[0];
+
+ 	eicr = wx_misc_isb(wx, WX_ISB_MISC);
+
+ 	if (eicr & NGBE_PX_MISC_IC_VF_MBOX)
+ 	wx_msg_task(wx);
+
+ 	if (wx->num_vfs == 7) { 
+ 		napi_schedule_irqoff(&q_vector->napi);
+ 		ngbe_irq_enable(wx, true);
+ 	} else {
+ 		/* re-enable the original interrupt state, no lsc, no queues */
+ 		if (netif_running(wx->netdev))
+ 			ngbe_irq_enable(wx, false);
+ 	}

 if (wx->num_vfs == 7) { 
	if (!eicr) {
  		napi_schedule_irqoff(&q_vector->napi);
  		ngbe_irq_enable(wx, true);
	} else {
  		if (netif_running(wx->netdev))
  			ngbe_irq_enable(wx, false);
	}
 } else {
  	/* re-enable the original interrupt state, no lsc, no queues */
 	if (netif_running(wx->netdev))
 		ngbe_irq_enable(wx, false);
 }

Use eicr to determine whether it is an msic interrupt or a queue interrupt.

> The goal should be to make sure the right handler is register
> for the IRQ, or at least do the muxing earlier in a safe fashion.
> Not decide that it was a packet IRQ half way thru a function called
> ngbe_msix_other

Whether the first way(Alloc 9 irq vectors, but only request_irq for 0 and 8) can be better
than reuse vector 0 in the real?

>