[9front] [patch] ethervirtio - fix multicast without a cmd queue

[9front] [patch] ethervirtio - fix multicast without a cmd queue

[Arne Meyer]

[-- Attachment #1: Type: text/plain, Size: 431 bytes --]

Hello,

this fixes multicast on hypervisors without a command queue for the ethernet interface, like OpenBSD.
Right now the driver does not install a multicast function when no command queue is available. This breaks multicast because the network stack errors out in netif.c:netmulti. To fix this, move the check for the queue to the multicast and promiscuous functions and install those functions unconditionally.

Greetings,
Arne

[-- Attachment #2: ethervirtio.patch --]
[-- Type: application/octet-stream, Size: 2088 bytes --]

diff 9d09242bee8489b6cdee246489063488bb52b1fb uncommitted
--- a/sys/src/9/pc/ethervirtio.c
+++ b/sys/src/9/pc/ethervirtio.c
@@ -479,8 +479,12 @@
 promiscuous(void *arg, int on)
 {
 	Ether *edev = arg;
+	Ctlr *ctlr = edev->ctlr;
 	uchar b[1];
 
+	if((ctlr->feat & (Fctrlvq|Fctrlrx)) != (Fctrlvq|Fctrlrx))
+		return;
+
 	b[0] = on != 0;
 	vctlcmd(edev, CtrlRx, CmdPromisc, b, sizeof(b));
 }
@@ -489,8 +493,12 @@
 multicast(void *arg, uchar*, int)
 {
 	Ether *edev = arg;
+	Ctlr *ctlr = edev->ctlr;
 	uchar b[1];
 
+	if((ctlr->feat & (Fctrlvq|Fctrlrx)) != (Fctrlvq|Fctrlrx))
+		return;
+
 	b[0] = edev->nmaddr > 0;
 	vctlcmd(edev, CtrlRx, CmdAllmulti, b, sizeof(b));
 }
@@ -671,11 +679,8 @@
 	edev->attach = attach;
 	edev->shutdown = shutdown;
 	edev->ifstat = ifstat;
-
-	if((ctlr->feat & (Fctrlvq|Fctrlrx)) == (Fctrlvq|Fctrlrx)){
-		edev->multicast = multicast;
-		edev->promiscuous = promiscuous;
-	}
+	edev->multicast = multicast;
+	edev->promiscuous = promiscuous;
 
 	pcisetbme(ctlr->pcidev);
 	intrenable(edev->irq, interrupt, edev, edev->tbdf, edev->name);
--- a/sys/src/9/port/ethervirtio10.c
+++ b/sys/src/9/port/ethervirtio10.c
@@ -528,8 +528,12 @@
 promiscuous(void *arg, int on)
 {
 	Ether *edev = arg;
+	Ctlr *ctlr = edev->ctlr;
 	uchar b[1];
 
+	if((ctlr->feat[0] & (Fctrlvq|Fctrlrx)) != (Fctrlvq|Fctrlrx))
+		return;
+
 	b[0] = on != 0;
 	vctlcmd(edev, CtrlRx, CmdPromisc, b, sizeof(b));
 }
@@ -538,8 +542,12 @@
 multicast(void *arg, uchar*, int)
 {
 	Ether *edev = arg;
+	Ctlr *ctlr = edev->ctlr;
 	uchar b[1];
 
+    if((ctlr->feat[0] & (Fctrlvq|Fctrlrx)) != (Fctrlvq|Fctrlrx))
+		return;
+
 	b[0] = edev->nmaddr > 0;
 	vctlcmd(edev, CtrlRx, CmdAllmulti, b, sizeof(b));
 }
@@ -774,11 +782,9 @@
 	edev->attach = attach;
 	edev->shutdown = shutdown;
 	edev->ifstat = ifstat;
+	edev->multicast = multicast;
+	edev->promiscuous = promiscuous;
 
-	if((ctlr->feat[0] & (Fctrlvq|Fctrlrx)) == (Fctrlvq|Fctrlrx)){
-		edev->multicast = multicast;
-		edev->promiscuous = promiscuous;
-	}
 
 	pcisetbme(ctlr->pcidev);
 	intrenable(edev->irq, interrupt, edev, edev->tbdf, edev->name);

Re: [9front] [patch] ethervirtio - fix multicast without a cmd queue

[cinap_lenrek]

I dont follow. this doesnt change the fact that we'r not
really enabling mcast reception or not.

all it does is make it FAIL silently.

Are you saying that the cmd is not needed for devices that
do not have cmd queue and always are in promisc mode so to
speak?

If thats the case, add a comment in the code why we'r
basically doing nothing and why that is ok.

--
cinap

Re: [9front] [patch] ethervirtio - fix multicast without a cmd queue

[Arne Meyer]

Yes, that is how the virtio interface works but the driver needs to supply
some multicast function even if it does nothing.

if netmulti returns early when no multicast callback is supplied, the address is not tracked. 
ethermux in devether.c calls activemulti and drops all untracked multicast packages. 

With this patch i get ipv6 on my virtual machine.

Maybe we should change the way netmulti works? I don't know...

> cinap_lenrek@felloff.net hat am 17.12.2023 22:08 CET geschrieben:
> 
>  
> I dont follow. this doesnt change the fact that we'r not
> really enabling mcast reception or not.
> 
> all it does is make it FAIL silently.
> 
> Are you saying that the cmd is not needed for devices that
> do not have cmd queue and always are in promisc mode so to
> speak?
> 
> If thats the case, add a comment in the code why we'r
> basically doing nothing and why that is ok.
> 
> --
> cinap

Re: [9front] [patch] ethervirtio - fix multicast without a cmd queue

[cinap_lenrek]

> if netmulti returns early when no multicast callback is supplied, the address is not tracked. 
> ethermux in devether.c calls activemulti and drops all untracked multicast packages. 

ah! ok, that makes sense.

i was under the assumption that we wont even receive mcast packets
if that option was not available.

i'll apply shortly :)

--
cinap

Re: [9front] [patch] ethervirtio - fix multicast without a cmd queue

[cinap_lenrek]

pushed, thanks again!

--
cinap