NetBSD Problem Report #47780

From www@NetBSD.org  Mon Apr 29 06:44:53 2013
Return-Path: <www@NetBSD.org>
Received: from mail.netbsd.org (mail.netbsd.org [149.20.53.66])
	by www.NetBSD.org (Postfix) with ESMTP id D907D63F175
	for <gnats-bugs@gnats.NetBSD.org>; Mon, 29 Apr 2013 06:44:52 +0000 (UTC)
Message-Id: <20130429064452.04F1863F175@www.NetBSD.org>
Date: Mon, 29 Apr 2013 06:44:51 +0000 (UTC)
From: aurelien@aurel32.net
Reply-To: aurelien@aurel32.net
To: gnats-bugs@NetBSD.org
Subject: if_vioif doesn't work with QEMU >= 1.4.0 due to features negotiation issues
X-Send-Pr-Version: www-1.0

>Number:         47780
>Category:       kern
>Synopsis:       if_vioif doesn't work with QEMU >= 1.4.0 due to features negotiation issues
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Mon Apr 29 06:45:00 +0000 2013
>Closed-Date:    Tue Jun 25 12:43:39 +0000 2013
>Last-Modified:  Tue Jun 25 12:43:39 +0000 2013
>Originator:     Aurelien Jarno
>Release:        6.0.1
>Organization:
>Environment:
NetBSD netbsd.aurel32.net 6.0.1 NetBSD 6.0.1 (GENERIC) #0: Mon Apr 29 02:07:25 CEST 2013  aurel32@netbsd.aurel32.net:/usr/src/sys/arch/amd64/compile/GENERIC amd64
>Description:
NetBSD includes paravirtualized network drivers to run under QEMU/KVM. While they work fine with QEMU < 1.4.0, they fail on QEMU >= 1.4.0 due to features negotiation issues on the NetBSD side. In vioif_attach() the negotiation is working correctly, however the features set is not saved. Later when the device is reset calling vioif_stop() and vioif_start(), the features are renogociated, using sc->sc_features which has not be initialized and thus equal to 0. This causes the host to drop support for some features, while the NetBSD guest later tries to use them. This in turn causes protocol issues (in this case NetBSD tries to write to the control queue, which has been disabled by the host), and causes QEMU to abort.

The attached patch fixes the issue.
>How-To-Repeat:
Run NetBSD under QEMU 1.4.0 with a virtio network interface:

  qemu-system-x86-64 -cdrom NetBSD-6.0.1-i386.iso -net nic,model=virtio -net user

And set and IP address on the virtio interface. This will causes QEMU to abort as NetBSD is trying to do forbidden things with the virtio network interface.
>Fix:
Index: if_vioif.c
===================================================================
RCS file: /cvsroot/src/sys/dev/pci/if_vioif.c,v
retrieving revision 1.2
diff -u -r1.2 if_vioif.c
--- if_vioif.c	19 Nov 2011 12:32:54 -0000	1.2
+++ if_vioif.c	29 Apr 2013 06:11:43 -0000
@@ -459,7 +459,6 @@
 {
 	struct vioif_softc *sc = device_private(self);
 	struct virtio_softc *vsc = device_private(parent);
-	uint32_t features;
 	struct ifnet *ifp = &sc->sc_ethercom.ec_if;

 	if (vsc->sc_child != NULL) {
@@ -478,13 +477,13 @@
 	vsc->sc_config_change = 0;
 	vsc->sc_intrhand = virtio_vq_intr;

-	features = virtio_negotiate_features(vsc,
-					     (VIRTIO_NET_F_MAC |
-					      VIRTIO_NET_F_STATUS |
-					      VIRTIO_NET_F_CTRL_VQ |
-					      VIRTIO_NET_F_CTRL_RX |
-					      VIRTIO_F_NOTIFY_ON_EMPTY));
-	if (features & VIRTIO_NET_F_MAC) {
+	sc->sc_features = virtio_negotiate_features(vsc,
+						    (VIRTIO_NET_F_MAC |
+						     VIRTIO_NET_F_STATUS |
+						     VIRTIO_NET_F_CTRL_VQ |
+						     VIRTIO_NET_F_CTRL_RX |
+						     VIRTIO_F_NOTIFY_ON_EMPTY));
+	if (sc->sc_features & VIRTIO_NET_F_MAC) {
 		sc->sc_mac[0] = virtio_read_device_config_1(vsc,
 						    VIRTIO_NET_CONFIG_MAC+0);
 		sc->sc_mac[1] = virtio_read_device_config_1(vsc,
@@ -544,8 +543,8 @@
 	sc->sc_vq[1].vq_done = vioif_tx_vq_done;
 	virtio_start_vq_intr(vsc, &sc->sc_vq[0]);
 	virtio_stop_vq_intr(vsc, &sc->sc_vq[1]); /* not urgent; do it later */
-	if ((features & VIRTIO_NET_F_CTRL_VQ)
-	    && (features & VIRTIO_NET_F_CTRL_RX)) {
+	if ((sc->sc_features & VIRTIO_NET_F_CTRL_VQ)
+	    && (sc->sc_features & VIRTIO_NET_F_CTRL_RX)) {
 		if (virtio_alloc_vq(vsc, &sc->sc_vq[2], 2,
 				    NBPG, 1, "control") == 0) {
 			sc->sc_vq[2].vq_done = vioif_ctrl_vq_done;

>Release-Note:

>Audit-Trail:

State-Changed-From-To: open->feedback
State-Changed-By: minoura@NetBSD.org
State-Changed-When: Mon, 13 May 2013 03:27:05 +0000
State-Changed-Why:
Another fix commited.


From: makoto@hauN.ORG (MINOURA Makoto)
To: gnats-bugs@NetBSD.org
Cc: kern-bug-people@netbsd.org, gnats-admin@netbsd.org, netbsd-bugs@netbsd.org
Subject: Re: kern/47780: if_vioif doesn't work with QEMU >= 1.4.0 due to features negotiation issues
Date: Mon, 13 May 2013 12:25:24 +0900

 if_vioif.c rev 1.4/1.2.14.1/1.2.8.1 should fix this problem.
 Thanks.

 -- 
 Minoura Makoto <makoto@hauN.org>

From: Aurelien Jarno <aurelien@aurel32.net>
To: gnats-bugs@NetBSD.org
Cc: kern-bug-people@netbsd.org, gnats-admin@netbsd.org,
	netbsd-bugs@netbsd.org
Subject: Re: kern/47780: if_vioif doesn't work with QEMU >= 1.4.0 due to
 features negotiation issues
Date: Wed, 22 May 2013 08:05:06 +0200

 On Mon, May 13, 2013 at 03:30:12AM +0000, MINOURA Makoto wrote:
 > The following reply was made to PR kern/47780; it has been noted by GNATS.
 > 
 > From: makoto@hauN.ORG (MINOURA Makoto)
 > To: gnats-bugs@NetBSD.org
 > Cc: kern-bug-people@netbsd.org, gnats-admin@netbsd.org, netbsd-bugs@netbsd.org
 > Subject: Re: kern/47780: if_vioif doesn't work with QEMU >= 1.4.0 due to features negotiation issues
 > Date: Mon, 13 May 2013 12:25:24 +0900
 > 
 >  if_vioif.c rev 1.4/1.2.14.1/1.2.8.1 should fix this problem.
 >  Thanks.
 >  

 Thanks a lot for the patch, I confirm it fixes the issue.

 -- 
 Aurelien Jarno                          GPG: 1024D/F1BCDB73
 aurelien@aurel32.net                 http://www.aurel32.net

From: "SAITOH Masanobu" <msaitoh@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/47780 CVS commit: [netbsd-5] src/sys/dev/pci
Date: Sun, 9 Jun 2013 11:51:40 +0000

 Module Name:	src
 Committed By:	msaitoh
 Date:		Sun Jun  9 11:51:40 UTC 2013

 Modified Files:
 	src/sys/dev/pci [netbsd-5]: if_vioif.c

 Log Message:
 Pull up following revision(s) (requested by minoura in ticket #1861):
 	sys/dev/pci/if_vioif.c: revision 1.4
 Fix a typo, and remove an unused member.
 This should fix the problem that recent Qemu dies during configuring a vioif.
 Fixes PR#47780.


 To generate a diff of this commit:
 cvs rdiff -u -r1.2.6.2 -r1.2.6.3 src/sys/dev/pci/if_vioif.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: feedback->closed
State-Changed-By: msaitoh@NetBSD.org
State-Changed-When: Tue, 25 Jun 2013 12:43:39 +0000
State-Changed-Why:
Fixed and pulled up to netbsd-[56] branches.


>Unformatted:

Home	PR Database Search