> vhost_net_inject_rarp is vhost-user specific.
For this function test on VIRTIO_NET_F_GUEST_ANNOUNCE bit is important to
take a decision and call a new vhost ioctl (see next patch) if the guest
does not support this feature and allow the vhost client/backend send the
RARP packet.
Use of the VIRTIO_NET_F_GUEST_ANNOUNCE field and call to a vhost ioctl does
not seem relevant in the net/vhost-user.c file
Do you suggest to create a new hw/net/vhost_user_net.c file to manage this
new feature ?
On Wed, Jun 10, 2015 at 4:27 PM, Michael S. Tsirkin <m...@redhat.com> wrote:
> On Wed, Jun 10, 2015 at 04:22:10PM +0200, Thibaut Collet wrote:
> > I am not sure to understand your comments.
> >
> > Could you confirm that the useless warning is:
> > + fprintf(stderr,
> > + "Warning: Guest with no VIRTIO_NET_F_GUEST_ANNOUNCE
> support.
> > RARP must be sent by vhost-user backend\n");
>
> Yes.
>
> > > Can you move this out to vhost-user? It's not a generic function, is
> it?
> >
> > Which part of the code do you think it must be moved in a specific file ?
>
> vhost_net_inject_rarp is vhost-user specific.
>
> >
> > On Wed, Jun 10, 2015 at 3:52 PM, Michael S. Tsirkin <m...@redhat.com>
> wrote:
> >
> > On Wed, Jun 10, 2015 at 03:43:02PM +0200, Thibaut Collet wrote:
> > > Some vhost client/backend are able to support live migration.
> > > To provide this service the following features must be added:
> > > 1. GARP from guest after live migration:
> > > the VIRTIO_NET_F_GUEST_ANNOUNCE capability is added to
> vhost-net when
> > netdev
> > > backend is vhost-user to let virtio-net NIC manages GARP after
> > migration.
> > > 2. RARP on vhost-user:
> > > After a live migration RARP are automatically sent to any
> interfaces.
> > A
> > > receive callback is added to vhost-user to catch this packet.
> If guest
> > > supports VIRTIO_NET_F_GUEST_ANNOUNCE this packet is discarded
> to avoid
> > > message duplication (already done by virtio-net NIC). For
> legacy guest
> > > without VIRTIO_NET_F_GUEST_ANNOUNCE a warning message is
> displayed to
> > > alert the user. RARP must be sent to the vhost client/backend.
> > >
> > > Signed-off-by: Thibaut Collet <thibaut.col...@6wind.com>
> > > ---
> > > hw/net/vhost_net.c | 15 +++++++++++++++
> > > include/net/vhost_net.h | 1 +
> > > net/vhost-user.c | 21 +++++++++++++++++++--
> > > 3 files changed, 35 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> > > index 426b23e..4a42325 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
> > > @@ -365,6 +367,15 @@ void vhost_net_stop(VirtIODevice *dev,
> > NetClientState *ncs,
> > > assert(r >= 0);
> > > }
> > >
> > > +void vhost_net_inject_rarp(struct vhost_net *net, const uint8_t
> *buf,
> > size_t size)
> > > +{
> > > + if ((net->dev.acked_features & (1 <<
> VIRTIO_NET_F_GUEST_ANNOUNCE)) =
> > = 0) {
> > > + fprintf(stderr,
> > > + "Warning: Guest with no
> VIRTIO_NET_F_GUEST_ANNOUNCE
> > support. RARP must be sent by vhost-user backend\n");
> > > + fflush(stderr);
> >
> > And maybe it does, then you are just filling log up with useless
> > warnings. Pls add some ifdef so this isn't normally compiled in.
> >
> > > + }
> >
> > Can you move this out to vhost-user? It's not a generic function, is
> it?
> >
> > > +}
> > > +
> > > void vhost_net_cleanup(struct vhost_net *net)
> > > {
> > > vhost_dev_cleanup(&net->dev);
> > > @@ -427,6 +438,10 @@ void vhost_net_stop(VirtIODevice *dev,
> > > {
> > > }
> > >
> > > +void vhost_net_inject_rarp(struct vhost_net *net, const uint8_t
> *buf,
> > size_t size);
> > > +{
> > > +}
> > > +
> > > void vhost_net_cleanup(struct vhost_net *net)
> > > {
> > > }
> > > diff --git a/include/net/vhost_net.h b/include/net/vhost_net.h
> > > index b1c18a3..e82a0f9 100644
> > > --- a/include/net/vhost_net.h
> > > +++ b/include/net/vhost_net.h
> > > @@ -20,6 +20,7 @@ bool vhost_net_query(VHostNetState *net,
> VirtIODevice
> > *dev);
> > > int vhost_net_start(VirtIODevice *dev, NetClientState *ncs, int
> > total_queues);
> > > void vhost_net_stop(VirtIODevice *dev, NetClientState *ncs, int
> > total_queues);
> > >
> > > +void vhost_net_inject_rarp(VHostNetState *net, const uint8_t *buf,
> > size_t size);
> > > void vhost_net_cleanup(VHostNetState *net);
> > >
> > > unsigned vhost_net_get_features(VHostNetState *net, unsigned
> features);
> > > diff --git a/net/vhost-user.c b/net/vhost-user.c
> > > index 8d26728..69c2313 100644
> > > --- a/net/vhost-user.c
> > > +++ b/net/vhost-user.c
> > > @@ -66,6 +66,24 @@ static void vhost_user_stop(VhostUserState *s)
> > > s->vhost_net = 0;
> > > }
> > >
> > > +static ssize_t vhost_user_receive(NetClientState *nc, const
> uint8_t
> > *buf,
> > > + size_t size)
> > > +{
> > > + if (size == 60) {
> > > + /* Assume it is a RARP request sent automatically after a
> > > + * live migration */
> > > + /* The RARP must be sent if guest does not support
> > > + * VIRTIO_NET_F_GUEST_ANNOUNCE */
> > > + VhostUserState *s = DO_UPCAST(VhostUserState, nc, nc);
> > > +
> > > + vhost_net_inject_rarp(s->vhost_net, buf, size);
> > > + } else {
> > > + fprintf(stderr,"Vhost user receives unexpected
> packets\n");
> > > + fflush(stderr);
> > > + }
> > > + return size;
> > > +}
> > > +
> > > static void vhost_user_cleanup(NetClientState *nc)
> > > {
> > > VhostUserState *s = DO_UPCAST(VhostUserState, nc, nc);
> > > @@ -91,6 +109,7 @@ static bool vhost_user_has_ufo(NetClientState
> *nc)
> > > static NetClientInfo net_vhost_user_info = {
> > > .type = NET_CLIENT_OPTIONS_KIND_VHOST_USER,
> > > .size = sizeof(VhostUserState),
> > > + .receive = vhost_user_receive,
> > > .cleanup = vhost_user_cleanup,
> > > .has_vnet_hdr = vhost_user_has_vnet_hdr,
> > > .has_ufo = vhost_user_has_ufo,
> > > @@ -147,8 +166,6 @@ static int net_vhost_user_init(NetClientState
> *peer,
> > const char *device,
> > >
> > > s = DO_UPCAST(VhostUserState, nc, nc);
> > >
> > > - /* We don't provide a receive callback */
> > > - s->nc.receive_disabled = 1;
> > > s->chr = chr;
> > > s->nc.queue_index = i;
> > >
> > > --
> > > 1.7.10.4
> >
> >
>
