|
| From: | Thibaut Collet |
| Subject: | Re: [Qemu-devel] [PATCH v2] net: Add support of VIRTIO_NET_F_GUEST_ANNOUNCE for vhost-net/vhost-user |
| Date: | Mon, 8 Jun 2015 15:20:23 +0200 |
On Mon, Jun 08, 2015 at 01:29:39PM +0200, Thibaut Collet wrote:
> Hi,
>
> > I don't think qemu_send_packet_raw can ever work for vhost user.
> > What happens if you merely add VIRTIO_NET_F_GUEST_ANNOUNCE
> > to the feature list, and drop the rest of the patch?
>
> If you merely add VIRTIO_NET_F_GUEST_ANNOUNCE you have the GARP with recent
> guest after a live migration.
> The rest of the patch (can be set in a separate commit) avoid a qemu crashes
> with live migration when vhost-user is present:
> - When a live migration migration is complete a RARP is automatically send to
> any net backend (self announce mechanism)
> - vhost user does not provide any receive function and RARP request is stored
> in the vhost-user queue
> - When a migration back is done all the net backend queues are purged. The
> stored RARP request for vhost-user is then sent to the register receive
> callback of vhost-user that is NULL.
>
> Support of live migration for vhost user needs the whole patch. (and maore if
> we want support legacy guest with no support of VIRTIO_NET_F_GUEST_ANNOUNCE)
How about implementing a receive function that discards all packets
then?
>
> > I don't get it. Why do you need any extra messages for old drivers? To
> > detect old drivers, simply have backend check whether
> > VIRTIO_NET_F_GUEST_ANNOUNCE is set.
>
> For old driver we can not use the mechanism implemented in virtio-net that
> manages VIRTIO_NET_F_GUEST_ANNOUNCE. In this case we must send the RARP on the
> network interface created by vhost-user.
> My first idea to do that was to add a message between QEMU and vhost client/
> backend (vapp or other) to let the vhost client/backend send the RARP.
> But maybe it will be easier to let QEMU send directly the RARP message on the
> network interface created by vhost user.
> This point can be done later if it is needed.
>
> Regards.
Can't vhost-user backend sends this automatically?
Why do we need to do anything in QEMU?
>
> On Mon, Jun 8, 2015 at 12:12 PM, Michael S. Tsirkin <address@hidden> wrote:
>
> On Fri, Jun 05, 2015 at 03:24:12PM +0200, Thibaut Collet wrote:
> > Add VIRTIO_NET_F_GUEST_ANNOUNCE capability to vhost-net when netdev
> backend is
> > vhost-user.
> >
> > For netdev backend using virtio-net NIC the self announce is managed
> directly
> > by the virtio-net NIC and not by the netdev backend itself.
> > To avoid duplication of announces (once from the guest and once from
> QEMU) a
> > bitfield is added in the NetClientState structure.
> > If this bit is set self announce does not send message to the guest to
> request
> > gratuitous ARP but let virtio-net NIC set the VIRTIO_NET_S_ANNOUNCE for
> > gratuitous ARP.
> >
> > Signed-off-by: Thibaut Collet <address@hidden>
> > ---
> > v2: do not discard anymore packets send to vhost-user: it is GARP request
> after
> > live migration.
> > As suggested by S. Hajnoczi qemu_announce_self skips virtio-net NIC
> that
> > already send GARP.
> >
> > hw/net/vhost_net.c | 2 ++
> > include/net/net.h | 1 +
> > net/vhost-user.c | 2 ++
> > savevm.c | 11 ++++++++---
> > 4 files changed, 13 insertions(+), 3 deletions(-)
> >
> > diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> > index 426b23e..a745f97 100644
> > --- a/hw/net/vhost_net.c
> > +++ b/hw/net/vhost_net.c
> > @@ -82,6 +82,8 @@ static const int user_feature_bits[] = {
> > VIRTIO_NET_F_CTRL_MAC_ADDR,
> > VIRTIO_NET_F_CTRL_GUEST_OFFLOADS,
> >
> > + VIRTIO_NET_F_GUEST_ANNOUNCE,
> > +
> > VIRTIO_NET_F_MQ,
> >
> > VHOST_INVALID_FEATURE_BIT
> > diff --git a/include/net/net.h b/include/net/net.h
> > index e66ca03..a78e9df 100644
> > --- a/include/net/net.h
> > +++ b/include/net/net.h
> > @@ -85,6 +85,7 @@ struct NetClientState {
> > char *name;
> > char info_str[256];
> > unsigned receive_disabled : 1;
> > + unsigned self_announce_disabled : 1;
> > NetClientDestructor *destructor;
> > unsigned int queue_index;
> > unsigned rxfilter_notify_enabled:1;
> > diff --git a/net/vhost-user.c b/net/vhost-user.c
> > index 8d26728..b345446 100644
> > --- a/net/vhost-user.c
> > +++ b/net/vhost-user.c
> > @@ -147,6 +147,8 @@ static int net_vhost_user_init(NetClientState *peer,
> const char *device,
> >
> > s = DO_UPCAST(VhostUserState, nc, nc);
> >
> > + /* Self announce is managed directly by virtio-net NIC */
> > + s->nc.self_announce_disabled = 1;
> > /* We don't provide a receive callback */
> > s->nc.receive_disabled = 1;
> > s->chr = chr;
> > diff --git a/savevm.c b/savevm.c
> > index 3b0e222..7a134b1 100644
> > --- a/savevm.c
> > +++ b/savevm.c
> > @@ -80,11 +80,16 @@ static void qemu_announce_self_iter(NICState *nic,
> void *opaque)
> > {
> > uint8_t buf[60];
> > int len;
> > + NetClientState *nc;
> >
> > - trace_qemu_announce_self_iter(qemu_ether_ntoa(&nic->conf->macaddr));
> > - len = announce_self_create(buf, nic->conf->macaddr.a);
> > + nc = qemu_get_queue(nic);
> >
> > - qemu_send_packet_raw(qemu_get_queue(nic), buf, len);
> > + if (!nc->peer->self_announce_disabled) {
> > + trace_qemu_announce_self_iter(qemu_ether_ntoa(&nic->conf->
> macaddr));
> > + len = announce_self_create(buf, nic->conf->macaddr.a);
> > +
> > + qemu_send_packet_raw(nc, buf, len);
> > + }
> > }
> >
>
> I don't think qemu_send_packet_raw can ever work for vhost user.
> What happens if you merely add VIRTIO_NET_F_GUEST_ANNOUNCE
> to the feature list, and drop the rest of the patch?
>
>
> > --
> > 1.7.10.4
>
>
| [Prev in Thread] | Current Thread | [Next in Thread] |