On Wed, Nov 23, 2016 at 09:03:46AM +0100, Stefan Fritsch wrote:
> On Wed, 23 Nov 2016, Mike Belopuhov wrote:
> > > I guess we could do that. But then we cannot free the mbufs on DOWN
> > > until the device has used them.
> > 
> > Diff to this effect is below.  Works on vmd and qemu (original
> > one didn't because I kept the virtio_reset).
> > 
> > > That sounds like an unnecessary waste of memory to me.
> > > 
> > 
> > This is not so much memory we lose and then if you up it again
> > you're going to have it all back.  We can revert to the present
> > behavior once vmd matures, in the meantime people won't have to
> > juggle diffs around in their trees :)
> 
> I am not convinced. Doing a reset allows to recover from all kinds of 
> problems with DOWN/UP. That was useful when we had bugs in the event_idx 
> implementation.
> 
> Also, I don't like to change code that is known to work with at least 4 
> independent device implementations to work around problems in one 
> incomplete implementation that we can easily change.
> 
> Maybe something like this is enough already (untested):

I tried your diff without Mike's if_vio diff and it doesn't panic anymore,
however it doesn't work.

vioX can send packets to host, host receives them and reply, but vioX
doesn't see any packets back. I don't even need to touch the interface
up/down status to see this happening. Also when the interface comes
up after being shutdown it sends a bunch of packets to host.

> 
> --- usr.sbin/vmd/virtio.c     2016-10-20 05:05:49.049943724 +0200
> +++ usr.sbin/vmd/virtio.c     2016-11-23 08:55:38.829501275 +0100
> @@ -796,6 +796,9 @@
>  
>       ret = 0;
>  
> +     if (dev->cfg.device_status != VIRTIO_CONFIG_DEVICE_STATUS_DRIVER_OK)
> +             return ret;
> +
>       vr_sz = vring_size(VIONET_QUEUE_SIZE);
>       q_gpa = dev->vq[0].qa;
>       q_gpa = q_gpa * VIRTIO_PAGE_SIZE;
> 

Reply via email to