| Message ID | 2695069b-5704-536d-33e3-66e8d014475c@redhat.com |
|---|---|
| State | New |
| Headers | show |
> From: Jason Wang [mailto:jasowang@redhat.com] > Sent: Tuesday, 28 March 2017 19:39 > > On 2017年03月29日 02:55, Andrew Baumann wrote: > >> From: Stefan Weil [mailto:sw@weilnetz.de] > >> Sent: Tuesday, 28 March 2017 11:28 > >> Am 25.03.2017 um 00:46 schrieb Andrew Baumann: > >>> The docs generally steer users away from using the legacy -net > >>> parameter, however on win32 attempting to enable a tap device using > >>> -netdev tap fails at an abort() in tap_enable(). Removing the abort()s > >>> seems to be enough to get everything working, so do that. > >>> > >>> Signed-off-by: Andrew Baumann <Andrew.Baumann@microsoft.com> [...] > >> Jason, what is the use of tap_enable, tap_disable? > > It should be only used when we want to enable and disable a specific > queue of a multiqueue supported tap. > > >> Is it fine > >> to simply do nothing on Windows here? > > Unless windows support multiqueue tap, we should keep the assert here. > > > I was also hoping for a review -- I'm no expert on this stuff either, but my > quick reading of those code paths is that they issue ioctls to enable/disable > packet reception on the underlying tap device. As win32 TAP is implemented, > that is already enabled from start of day. > > > > It's possible this patch still does not permit dynamic reconfiguration of tap > devices (e.g. from the monitor console). However, it does work with the - > netdev tap option on the command-line. > > > >> And is this something for QEMU 2.9 (I added question to subject line)? > > Ideally, yes. If not, -netdev tap will continue to blow up in the abort as it does > today... > > > > Andrew > > Yes, so the problem is we should prevent tap_enable() and tap_disable() > from being called if multiqueue is disabled. > > I believe the following patch can fix this issue, could you give a try > on this? > > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c > index c321680..7d091c9 100644 > --- a/hw/net/virtio-net.c > +++ b/hw/net/virtio-net.c > @@ -510,6 +510,10 @@ static int peer_attach(VirtIONet *n, int index) > return 0; > } > > + if (n->max_queues == 1) { > + return 0; > + } > + > return tap_enable(nc->peer); > } > Yep, this works. Thanks! Andrew
On 2017年03月29日 12:13, Andrew Baumann via Qemu-devel wrote: >> From: Jason Wang [mailto:jasowang@redhat.com] >> Sent: Tuesday, 28 March 2017 19:39 >> >> On 2017年03月29日 02:55, Andrew Baumann wrote: >>>> From: Stefan Weil [mailto:sw@weilnetz.de] >>>> Sent: Tuesday, 28 March 2017 11:28 >>>> Am 25.03.2017 um 00:46 schrieb Andrew Baumann: >>>>> The docs generally steer users away from using the legacy -net >>>>> parameter, however on win32 attempting to enable a tap device using >>>>> -netdev tap fails at an abort() in tap_enable(). Removing the abort()s >>>>> seems to be enough to get everything working, so do that. >>>>> >>>>> Signed-off-by: Andrew Baumann<Andrew.Baumann@microsoft.com> > [...] >>>> Jason, what is the use of tap_enable, tap_disable? >> It should be only used when we want to enable and disable a specific >> queue of a multiqueue supported tap. >> >>>> Is it fine >>>> to simply do nothing on Windows here? >> Unless windows support multiqueue tap, we should keep the assert here. >> >>> I was also hoping for a review -- I'm no expert on this stuff either, but my >> quick reading of those code paths is that they issue ioctls to enable/disable >> packet reception on the underlying tap device. As win32 TAP is implemented, >> that is already enabled from start of day. >>> It's possible this patch still does not permit dynamic reconfiguration of tap >> devices (e.g. from the monitor console). However, it does work with the - >> netdev tap option on the command-line. >>>> And is this something for QEMU 2.9 (I added question to subject line)? >>> Ideally, yes. If not, -netdev tap will continue to blow up in the abort as it does >> today... >>> Andrew >> Yes, so the problem is we should prevent tap_enable() and tap_disable() >> from being called if multiqueue is disabled. >> >> I believe the following patch can fix this issue, could you give a try >> on this? >> >> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c >> index c321680..7d091c9 100644 >> --- a/hw/net/virtio-net.c >> +++ b/hw/net/virtio-net.c >> @@ -510,6 +510,10 @@ static int peer_attach(VirtIONet *n, int index) >> return 0; >> } >> >> + if (n->max_queues == 1) { >> + return 0; >> + } >> + >> return tap_enable(nc->peer); >> } >> > Yep, this works. Thanks! > > Andrew Thanks, queued this for 2.9.
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index c321680..7d091c9 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -510,6 +510,10 @@ static int peer_attach(VirtIONet *n, int index) return 0; } + if (n->max_queues == 1) { + return 0; + } + return tap_enable(nc->peer); }