The warning message is not really necessary is just a reminder.
On Wed, Jun 10, 2015 at 10:50 PM, Michael S. Tsirkin <m...@redhat.com> wrote:
> On Wed, Jun 10, 2015 at 10:25:57PM +0200, Thibaut Collet wrote:
> > Yes backend can save everything to be able to send the rarp alone after
> a live
> > migration.
> > Main purpose of this patch is to answer to the point raise by Jason on
> the
> > previous version of my patch:
> > > Yes, your patch works well for recent drivers. But the problem is
> legacy
> > > guest/driver without VIRTIO_NET_F_GUEST_ANNOUNCE. In this case there
> > > will be no GARP sent after migration.
> >
> > If Jason is OK with this solution this patch can be forgotten.
> >
> > Maybe, to warn user of this issue if the backend does not send the rarp,
> it can
> > be useful to keep the warn message of the previous patch:
> > > + fprintf(stderr,
> > > + "Warning: Guest with no VIRTIO_NET_F_GUEST_ANNOUNCE
> support.
> > RARP must be sent by vhost-user backend\n");
> > > + fflush(stderr);
> > with a static bool to display this message only one time to limit the
> number of
> > message ?
>
> I don't see why it's necessary.
>
> > On Wed, Jun 10, 2015 at 6:00 PM, Michael S. Tsirkin <m...@redhat.com>
> wrote:
> >
> > On Wed, Jun 10, 2015 at 05:48:47PM +0200, Thibaut Collet wrote:
> > > I have involved QEMU because QEMU prepares the rarp. I agree that
> backend
> > has
> > > probably all the information to do that.
> > > But backend does not know if the guest supports
> > the VIRTIO_NET_F_GUEST_ANNOUNCE
> >
> > Why not? Backend has the acked feature mask.
> >
> >
> > > and will send a useless rarp.
> > > Maybe this duplication of requests is not very important and in
> this case
> > this
> > > patch is not mandatory.
> > >
> > > On Wed, Jun 10, 2015 at 5:34 PM, Michael S. Tsirkin <
> m...@redhat.com>
> > wrote:
> > >
> > > On Wed, Jun 10, 2015 at 03:43:03PM +0200, Thibaut Collet wrote:
> > > > In case of live migration with legacy guest (without
> > > VIRTIO_NET_F_GUEST_ANNOUNCE)
> > > > a message is added between QEMU and the vhost client/backend.
> > > > This message provides the RARP content, prepared by QEMU, to
> the
> > vhost
> > > > client/backend.
> > > > The vhost client/backend is responsible to send the RARP.
> > > >
> > > > Signed-off-by: Thibaut Collet <thibaut.col...@6wind.com>
> > > > ---
> > > > docs/specs/vhost-user.txt | 16 ++++++++++++++++
> > > > hw/net/vhost_net.c | 8 ++++++++
> > > > hw/virtio/vhost-user.c | 11 ++++++++++-
> > > > linux-headers/linux/vhost.h | 9 +++++++++
> > > > 4 files changed, 43 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/docs/specs/vhost-user.txt
> b/docs/specs/vhost-user.txt
> > > > index 2c8e934..ef5d475 100644
> > > > --- a/docs/specs/vhost-user.txt
> > > > +++ b/docs/specs/vhost-user.txt
> > > > @@ -97,6 +97,7 @@ typedef struct VhostUserMsg {
> > > > uint64_t u64;
> > > > struct vhost_vring_state state;
> > > > struct vhost_vring_addr addr;
> > > > + struct vhost_inject_rarp rarp;
> > > > VhostUserMemory memory;
> > > > };
> > > > } QEMU_PACKED VhostUserMsg;
> > > > @@ -132,6 +133,12 @@ Multi queue support
> > > > The protocol supports multiple queues by setting all index
> fields
> > in the
> > > sent
> > > > messages to a properly calculated value.
> > > >
> > > > +Live migration support
> > > > +----------------------
> > > > +The protocol supports live migration. GARP from the
> migrated guest
> > is
> > > done
> > > > +through the VIRTIO_NET_F_GUEST_ANNOUNCE mechanism for guest
> that
> > > supports it or
> > > > +through RARP.
> > > > +
> > > > Message types
> > > > -------------
> > > >
> > > > @@ -269,3 +276,12 @@ Message types
> > > > Bits (0-7) of the payload contain the vring index.
> Bit 8 is
> > the
> > > > invalid FD flag. This flag is set when there is no
> file
> > descriptor
> > > > in the ancillary data.
> > > > +
> > > > + * VHOST_USER_NET_INJECT_RARP
> > > > +
> > > > + Id: 15
> > > > + Master payload: rarp content
> > > > +
> > > > + Provide the RARP message to send to the guest after a
> live
> > > migration. This
> > > > + message is sent only for guest that does not support
> > > > + VIRTIO_NET_F_GUEST_ANNOUNCE.
> > >
> > > I don't see why this is needed.
> > > Can't backend simply send rarp itself?
> > > Why do we need to involve QEMU?
> > >
> > >
> > >
> > > > diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> > > > index 4a42325..f66d48d 100644
> > > > --- a/hw/net/vhost_net.c
> > > > +++ b/hw/net/vhost_net.c
> > > > @@ -369,10 +369,18 @@ void vhost_net_stop(VirtIODevice *dev,
> > > NetClientState *ncs,
> > > >
> > > > void vhost_net_inject_rarp(struct vhost_net *net, const
> uint8_t
> > *buf,
> > > size_t size)
> > > > {
> > > > + struct vhost_inject_rarp inject_rarp;
> > > > + memcpy(&inject_rarp.rarp, buf, size);
> > > > +
> > > > if ((net->dev.acked_features & (1 <<
> > VIRTIO_NET_F_GUEST_ANNOUNCE)) =
> > > = 0) {
> > > > + const VhostOps *vhost_ops = net->dev.vhost_ops;
> > > > + int r;
> > > > +
> > > > fprintf(stderr,
> > > > "Warning: Guest with no
> > VIRTIO_NET_F_GUEST_ANNOUNCE
> > > support. RARP must be sent by vhost-user backend\n");
> > > > fflush(stderr);
> > > > + r = vhost_ops->vhost_call(&net->dev,
> > VHOST_NET_INJECT_RARP, &
> > > inject_rarp);
> > > > + assert(r >= 0);
> > > > }
> > > > }
> > > >
> > > > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> > > > index d6f2163..2e752ab 100644
> > > > --- a/hw/virtio/vhost-user.c
> > > > +++ b/hw/virtio/vhost-user.c
> > > > @@ -41,6 +41,7 @@ typedef enum VhostUserRequest {
> > > > VHOST_USER_SET_VRING_KICK = 12,
> > > > VHOST_USER_SET_VRING_CALL = 13,
> > > > VHOST_USER_SET_VRING_ERR = 14,
> > > > + VHOST_USER_NET_INJECT_RARP = 15,
> > > > VHOST_USER_MAX
> > > > } VhostUserRequest;
> > > >
> > > > @@ -70,6 +71,7 @@ typedef struct VhostUserMsg {
> > > > uint64_t u64;
> > > > struct vhost_vring_state state;
> > > > struct vhost_vring_addr addr;
> > > > + struct vhost_inject_rarp rarp;
> > > > VhostUserMemory memory;
> > > > };
> > > > } QEMU_PACKED VhostUserMsg;
> > > > @@ -104,7 +106,8 @@ static unsigned long int
> > ioctl_to_vhost_user_request
> > > [VHOST_USER_MAX] = {
> > > > VHOST_GET_VRING_BASE, /* VHOST_USER_GET_VRING_BASE */
> > > > VHOST_SET_VRING_KICK, /* VHOST_USER_SET_VRING_KICK */
> > > > VHOST_SET_VRING_CALL, /* VHOST_USER_SET_VRING_CALL */
> > > > - VHOST_SET_VRING_ERR /* VHOST_USER_SET_VRING_ERR */
> > > > + VHOST_SET_VRING_ERR, /* VHOST_USER_SET_VRING_ERR */
> > > > + VHOST_NET_INJECT_RARP /* VHOST_USER_NET_INJECT_RARP */
> > > > };
> > > >
> > > > static VhostUserRequest
> vhost_user_request_translate(unsigned long
> > int
> > > request)
> > > > @@ -287,6 +290,12 @@ static int vhost_user_call(struct
> vhost_dev
> > *dev,
> > > unsigned long int request,
> > > > msg.u64 |= VHOST_USER_VRING_NOFD_MASK;
> > > > }
> > > > break;
> > > > +
> > > > + case VHOST_NET_INJECT_RARP:
> > > > + memcpy(&msg.rarp, arg, sizeof(struct
> vhost_inject_rarp));
> > > > + msg.size = sizeof(struct vhost_inject_rarp);
> > > > + break;
> > > > +
> > > > default:
> > > > error_report("vhost-user trying to send unhandled
> ioctl");
> > > > return -1;
> > > > diff --git a/linux-headers/linux/vhost.h
> b/linux-headers/linux/
> > vhost.h
> > > > index c656f61..1920134 100644
> > > > --- a/linux-headers/linux/vhost.h
> > > > +++ b/linux-headers/linux/vhost.h
> > > > @@ -63,6 +63,10 @@ struct vhost_memory {
> > > > struct vhost_memory_region regions[0];
> > > > };
> > > >
> > > > +struct vhost_inject_rarp {
> > > > + __u8 rarp[60];
> > > > +};
> > > > +
> > > > /* ioctls */
> > > >
> > > > #define VHOST_VIRTIO 0xAF
> > > > @@ -121,6 +125,11 @@ struct vhost_memory {
> > > > * device. This can be used to stop the ring (e.g. for
> > migration). */
> > > > #define VHOST_NET_SET_BACKEND _IOW(VHOST_VIRTIO, 0x30,
> struct
> > > vhost_vring_file)
> > > >
> > > > +/* Inject a RARP in case of live migration for guest that
> does not
> > > support
> > > > + * VIRTIO_NET_F_GUEST_ANNOUNCE */
> > > > +#define VHOST_NET_INJECT_RARP _IOW(VHOST_VIRTIO, 0x31,
> struct
> > > vhost_inject_rarp)
> > > > +
> > > > +
> > > > /* Feature bits */
> > > > /* Log all write descriptors. Can be changed while device is
> > active. */
> > > > #define VHOST_F_LOG_ALL 26
> > > > --
> > > > 1.7.10.4
> > >
> > >
> >
> >
>
