Re: [Qemu-devel] [PATCH] net: Forbid dealing with packets when VM is not running

[Stefan Hajnoczi]

On Wed, Aug 20, 2014 at 11:17:56AM +0800, Jason Wang wrote:
> On 08/19/2014 08:29 PM, Stefan Hajnoczi wrote:
> > On Mon, Aug 18, 2014 at 04:32:42PM +0800, zhanghailiang wrote:
> >> On 2014/8/18 14:55, Jason Wang wrote:
> >>> On 08/18/2014 12:46 PM, zhanghailiang wrote:
> >>>> diff --git a/net/net.c b/net/net.c
> >>>> index 6d930ea..21f0d48 100644
> >>>> --- a/net/net.c
> >>>> +++ b/net/net.c
> >>>> @@ -242,6 +242,29 @@ NetClientState *qemu_new_net_client(NetClientInfo 
> >>>> *info,
> >>>>      return nc;
> >>>>  }
> >>>>
> >>>> +static void nic_vmstate_change_handler(void *opaque,
> >>>> +                                       int running,
> >>>> +                                       RunState state)
> >>>> +{
> >>>> +    NICState *nic = opaque;
> >>>> +    NetClientState *nc;
> >>>> +    int i, queues;
> >>>> +
> >>>> +    if (!running) {
> >>>> +        return;
> >>>> +    }
> >>>> +
> >>>> +    queues = MAX(1, nic->conf->peers.queues);
> >>>> +    for (i = 0; i<  queues; i++) {
> >>>> +        nc =&nic->ncs[i];
> >>>> +        if (nc->receive_disabled
> >>>> +            || (nc->info->can_receive&&  !nc->info->can_receive(nc))) {
> >>>> +            continue;
> >>>> +        }
> >>>> +        qemu_flush_queued_packets(nc);
> >>> How about simply purge the receive queue during stop? If ok, there's no
> >>> need to introduce extra vmstate change handler.
> >>>
> >> I don't know whether it is OK to purge the receive packages, it was
> >> suggested by Stefan Hajnoczi, and i am waiting for his opinion .:)
> >>
> >> I think we still need the extra vmstate change handler, Without the
> >> change handler, we don't know if the VM will go to stop and the time
> >> when to call qemu_purge_queued_packets.
> > qemu_flush_queued_packets() sets nc->received_disabled = 0.  This may be
> > needed to get packets flowing again if ->receive() previously returned 0.
> >
> > Purging the queue does not clear nc->received_disabled so it is not
> > enough.
> 
> Confused.
> 
> virtio_net_receive() only returns 0 when it does not have enough rx
> buffers. In this case, it just wait for the guest to refill and kick
> again. Its rx kick handler will call qemu_flush_queued_packets() to
> clear nc->received_disabled. So does usbnet and others.
> 
> If nic_received_disabled is 1, it means the no available rx buffer. We
> need wait guest to do the processing and refilling. Then why need clear
> it after vm was started?

I took a look at other emulated NICs, they don't use return 0 in
->receive().

I think you are right, we don't need to worry about flushing.

Stefan

pgppJpew3q3M4.pgp
Description: PGP signature