Re: [Qemu-devel] wrong argument to qemu_flush_queued_packets() in network frontends ?

[Luigi Rizzo]

... and upon closer inspection, the problem described below (frontend

blocks the backend, then tries to drain the wrong queue causing a stall)

occurs because the hub in the middle breaks the flow of events.

In the configuration below ( -net nic -net tap,ifname=tap0,... ) we have

    e1000.0 <--> hub0port0 [hub] hub0port1 <--> tap.0

The hub0port1 reports as non-writable when all other ports

(just one in this case) are full, and the packet is queued

on hub0port1. However when the e1000 frontend tries to drain

the queue, it directly accesses the queue attached to hub0port0,

which is empty.

So it appears that the only fix is the following:

when a node is attached to a hub, instead of draining the

queue on the node one should drain all queues attached to the hub.

A new function qemu_flush_hub() would be handy, something like

    QLIST_FOREACH(port, &hub->ports, next) {

        if (port != source_port)

           qemu_flush_queued_packets(&port->nc);

    }

The other option (queueing on the output ports of the hub)

would require a bit more attention to make sure that

the callback is only executed once (and also, avoid exceeding

data replication). Not impossible, but it requires reference

counting the packet.

What do you think, which way do you prefer ?

cheers

luigi

On Sun, Jan 20, 2013 at 6:50 PM, Luigi Rizzo <address@hidden> wrote:

While running qemu 1.3.0 with the following network-related flags:

        -net nic -net tap,ifname=tap0,script=''

I encountered the same problem (should be common to several
frontends, e.g. e100, eepro100, virtio-net, xen_nic):

in net/tap.c :: tap_send(), if qemu_send_packet_async() returns 0
(e.g. because the NIC has no buffers available)
traffic stops, despite the fact that the frontend will try to pull
queued packets when the receive ring is updated.

Upon investigation, it turns out that the backend code does

    size = qemu_send_packet_async(&s->nc, buf, size, tap_send_completed);
    if (size == 0) {
        tap_read_poll(s, 0);

and the arguments are

        s->nc.name = "tap.0"
        s->nc.peer->name = "hub0port1"
        s->nc.send_queue = 0x7f40b2f61e20
        s->nc.peer->send_queue = 0x7f40b2f63690 <--- enqueued here

whereis the frontend is trying to pull from a different queue

    qemu_flush_queued_packets(&s->nic->nc);

with arguments

        s->nic->nc.name = "e1000.0"
        s->nic->nc.peer->name = "hub0port0" <--- try to flush this
        s->nic->nc.send_queue = 0x7f40b3008ae0
        s->nic->nc.peer->send_queue = 0x7f40b2f63660


Note, regular traffic flows correctly across the hub,
but qemu_flush_queued_packets() seems to try and pull
from the wrong place.

Any idea how to fix this (other than the inefficient solution
of leaving read_poll=1 in the frontend)

        cheers
        luigi

--
-----------------------------------------+-------------------------------
 Prof. Luigi RIZZO, address@hidden  . Dip. di Ing. dell'Informazione
 http://www.iet.unipi.it/~luigi/        . Universita` di Pisa
 TEL      +39-050-2211611               . via Diotisalvi 2
 Mobile   +39-338-6809875               . 56122 PISA (Italy)
-----------------------------------------+-------------------------------