From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46325) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z2mxL-0005fL-2X for qemu-devel@nongnu.org; Wed, 10 Jun 2015 16:50:40 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z2mxH-0007sl-KP for qemu-devel@nongnu.org; Wed, 10 Jun 2015 16:50:39 -0400 Received: from mx1.redhat.com ([209.132.183.28]:38746) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z2mxG-0007rX-WE for qemu-devel@nongnu.org; Wed, 10 Jun 2015 16:50:35 -0400 Date: Wed, 10 Jun 2015 22:50:30 +0200 From: "Michael S. Tsirkin" Message-ID: <20150610224932-mutt-send-email-mst@redhat.com> References: <1433943783-20125-1-git-send-email-thibaut.collet@6wind.com> <1433943783-20125-3-git-send-email-thibaut.collet@6wind.com> <20150610173321-mutt-send-email-mst@redhat.com> <20150610180013-mutt-send-email-mst@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v3 2/2] vhost user: Add RARP injection for legacy guest List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Thibaut Collet Cc: Jason Wang , qemu-devel , Stefan Hajnoczi 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 leg= acy > > guest/driver without VIRTIO_NET_F_GUEST_ANNOUNCE. In this case there > > will be no GARP sent after migration. >=20 > If Jason is OK with this solution=A0this patch can be forgotten. >=20 > 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: > > +=A0 =A0 =A0 =A0 fprintf(stderr, > > +=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 "Warning: Guest with no VIRTIO_NET_F= _GUEST_ANNOUNCE support. > RARP must be sent by vhost-user backend\n"); > > +=A0 =A0 =A0 =A0 fflush(stderr); > with a static bool to display this message only one time to limit the n= umber of > message ? I don't see why it's necessary. > On Wed, Jun 10, 2015 at 6:00 PM, Michael S. Tsirkin wr= ote: >=20 > 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=A0VIRTIO_NET_F_GUEST_ANNOUNCE >=20 > Why not?=A0 Backend has the acked feature mask. >=20 >=20 > > and will send a useless rarp. > > Maybe this duplication of requests is not very important and in t= his case > this > > patch is not mandatory. > > > > On Wed, Jun 10, 2015 at 5:34 PM, Michael S. Tsirkin > wrote: > > > >=A0 =A0 =A0On Wed, Jun 10, 2015 at 03:43:03PM +0200, Thibaut Colle= t wrote: > >=A0 =A0 =A0> In case of live migration with legacy guest (without > >=A0 =A0 =A0VIRTIO_NET_F_GUEST_ANNOUNCE) > >=A0 =A0 =A0> a message is added between QEMU and the vhost client/= backend. > >=A0 =A0 =A0> This message provides the RARP content, prepared by Q= EMU, to the > vhost > >=A0 =A0 =A0> client/backend. > >=A0 =A0 =A0> The vhost client/backend is responsible to send the R= ARP. > >=A0 =A0 =A0> > >=A0 =A0 =A0> Signed-off-by: Thibaut Collet > >=A0 =A0 =A0> --- > >=A0 =A0 =A0>=A0 docs/specs/vhost-user.txt=A0 =A0|=A0 =A016 +++++++= +++++++++ > >=A0 =A0 =A0>=A0 hw/net/vhost_net.c=A0 =A0 =A0 =A0 =A0 |=A0 =A0 8 += +++++++ > >=A0 =A0 =A0>=A0 hw/virtio/vhost-user.c=A0 =A0 =A0 |=A0 =A011 +++++= +++++- > >=A0 =A0 =A0>=A0 linux-headers/linux/vhost.h |=A0 =A0 9 +++++++++ > >=A0 =A0 =A0>=A0 4 files changed, 43 insertions(+), 1 deletion(-) > >=A0 =A0 =A0> > >=A0 =A0 =A0> diff --git a/docs/specs/vhost-user.txt b/docs/specs/v= host-user.txt > >=A0 =A0 =A0> index 2c8e934..ef5d475 100644 > >=A0 =A0 =A0> --- a/docs/specs/vhost-user.txt > >=A0 =A0 =A0> +++ b/docs/specs/vhost-user.txt > >=A0 =A0 =A0> @@ -97,6 +97,7 @@ typedef struct VhostUserMsg { > >=A0 =A0 =A0>=A0 =A0 =A0 =A0 =A0 uint64_t u64; > >=A0 =A0 =A0>=A0 =A0 =A0 =A0 =A0 struct vhost_vring_state state; > >=A0 =A0 =A0>=A0 =A0 =A0 =A0 =A0 struct vhost_vring_addr addr; > >=A0 =A0 =A0> +=A0 =A0 =A0 =A0 struct vhost_inject_rarp rarp; > >=A0 =A0 =A0>=A0 =A0 =A0 =A0 =A0 VhostUserMemory memory; > >=A0 =A0 =A0>=A0 =A0 =A0 }; > >=A0 =A0 =A0>=A0 } QEMU_PACKED VhostUserMsg; > >=A0 =A0 =A0> @@ -132,6 +133,12 @@ Multi queue support > >=A0 =A0 =A0>=A0 The protocol supports multiple queues by setting a= ll index fields > in the > >=A0 =A0 =A0sent > >=A0 =A0 =A0>=A0 messages to a properly calculated value. > >=A0 =A0 =A0> > >=A0 =A0 =A0> +Live migration support > >=A0 =A0 =A0> +---------------------- > >=A0 =A0 =A0> +The protocol supports live migration. GARP from the = migrated guest > is > >=A0 =A0 =A0done > >=A0 =A0 =A0> +through the VIRTIO_NET_F_GUEST_ANNOUNCE mechanism fo= r guest that > >=A0 =A0 =A0supports it or > >=A0 =A0 =A0> +through RARP. > >=A0 =A0 =A0> + > >=A0 =A0 =A0>=A0 Message types > >=A0 =A0 =A0>=A0 ------------- > >=A0 =A0 =A0> > >=A0 =A0 =A0> @@ -269,3 +276,12 @@ Message types > >=A0 =A0 =A0>=A0 =A0 =A0 =A0 Bits (0-7) of the payload contain the = vring index. Bit 8 is > the > >=A0 =A0 =A0>=A0 =A0 =A0 =A0 invalid FD flag. This flag is set when= there is no file > descriptor > >=A0 =A0 =A0>=A0 =A0 =A0 =A0 in the ancillary data. > >=A0 =A0 =A0> + > >=A0 =A0 =A0> + * VHOST_USER_NET_INJECT_RARP > >=A0 =A0 =A0> + > >=A0 =A0 =A0> +=A0 =A0 =A0 Id: 15 > >=A0 =A0 =A0> +=A0 =A0 =A0 Master payload: rarp content > >=A0 =A0 =A0> + > >=A0 =A0 =A0> +=A0 =A0 =A0 Provide the RARP message to send to the = guest after a live > >=A0 =A0 =A0migration. This > >=A0 =A0 =A0> +=A0 =A0 =A0 message is sent only for guest that does= not support > >=A0 =A0 =A0> +=A0 =A0 =A0 VIRTIO_NET_F_GUEST_ANNOUNCE. > > > >=A0 =A0 =A0I don't see why this is needed. > >=A0 =A0 =A0Can't backend simply send rarp itself? > >=A0 =A0 =A0Why do we need to involve QEMU? > > > > > > > >=A0 =A0 =A0> diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c > >=A0 =A0 =A0> index 4a42325..f66d48d 100644 > >=A0 =A0 =A0> --- a/hw/net/vhost_net.c > >=A0 =A0 =A0> +++ b/hw/net/vhost_net.c > >=A0 =A0 =A0> @@ -369,10 +369,18 @@ void vhost_net_stop(VirtIODevic= e *dev, > >=A0 =A0 =A0NetClientState *ncs, > >=A0 =A0 =A0> > >=A0 =A0 =A0>=A0 void vhost_net_inject_rarp(struct vhost_net *net, = const uint8_t > *buf, > >=A0 =A0 =A0size_t size) > >=A0 =A0 =A0>=A0 { > >=A0 =A0 =A0> +=A0 =A0 struct vhost_inject_rarp inject_rarp; > >=A0 =A0 =A0> +=A0 =A0 memcpy(&inject_rarp.rarp, buf, size); > >=A0 =A0 =A0> + > >=A0 =A0 =A0>=A0 =A0 =A0 if ((net->dev.acked_features & (1 << > VIRTIO_NET_F_GUEST_ANNOUNCE)) =3D > >=A0 =A0 =A0=3D 0) { > >=A0 =A0 =A0> +=A0 =A0 =A0 =A0 const VhostOps *vhost_ops =3D net->d= ev.vhost_ops; > >=A0 =A0 =A0> +=A0 =A0 =A0 =A0 int r; > >=A0 =A0 =A0> + > >=A0 =A0 =A0>=A0 =A0 =A0 =A0 =A0 fprintf(stderr, > >=A0 =A0 =A0>=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 "Warning: Guest wi= th no > VIRTIO_NET_F_GUEST_ANNOUNCE > >=A0 =A0 =A0support. RARP must be sent by vhost-user backend\n"); > >=A0 =A0 =A0>=A0 =A0 =A0 =A0 =A0 fflush(stderr); > >=A0 =A0 =A0> +=A0 =A0 =A0 =A0 r =3D vhost_ops->vhost_call(&net->de= v, > VHOST_NET_INJECT_RARP, & > >=A0 =A0 =A0inject_rarp); > >=A0 =A0 =A0> +=A0 =A0 =A0 =A0 assert(r >=3D 0); > >=A0 =A0 =A0>=A0 =A0 =A0 } > >=A0 =A0 =A0>=A0 } > >=A0 =A0 =A0> > >=A0 =A0 =A0> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost= -user.c > >=A0 =A0 =A0> index d6f2163..2e752ab 100644 > >=A0 =A0 =A0> --- a/hw/virtio/vhost-user.c > >=A0 =A0 =A0> +++ b/hw/virtio/vhost-user.c > >=A0 =A0 =A0> @@ -41,6 +41,7 @@ typedef enum VhostUserRequest { > >=A0 =A0 =A0>=A0 =A0 =A0 VHOST_USER_SET_VRING_KICK =3D 12, > >=A0 =A0 =A0>=A0 =A0 =A0 VHOST_USER_SET_VRING_CALL =3D 13, > >=A0 =A0 =A0>=A0 =A0 =A0 VHOST_USER_SET_VRING_ERR =3D 14, > >=A0 =A0 =A0> +=A0 =A0 VHOST_USER_NET_INJECT_RARP =3D 15, > >=A0 =A0 =A0>=A0 =A0 =A0 VHOST_USER_MAX > >=A0 =A0 =A0>=A0 } VhostUserRequest; > >=A0 =A0 =A0> > >=A0 =A0 =A0> @@ -70,6 +71,7 @@ typedef struct VhostUserMsg { > >=A0 =A0 =A0>=A0 =A0 =A0 =A0 =A0 uint64_t u64; > >=A0 =A0 =A0>=A0 =A0 =A0 =A0 =A0 struct vhost_vring_state state; > >=A0 =A0 =A0>=A0 =A0 =A0 =A0 =A0 struct vhost_vring_addr addr; > >=A0 =A0 =A0> +=A0 =A0 =A0 =A0 struct vhost_inject_rarp rarp; > >=A0 =A0 =A0>=A0 =A0 =A0 =A0 =A0 VhostUserMemory memory; > >=A0 =A0 =A0>=A0 =A0 =A0 }; > >=A0 =A0 =A0>=A0 } QEMU_PACKED VhostUserMsg; > >=A0 =A0 =A0> @@ -104,7 +106,8 @@ static unsigned long int > ioctl_to_vhost_user_request > >=A0 =A0 =A0[VHOST_USER_MAX] =3D { > >=A0 =A0 =A0>=A0 =A0 =A0 VHOST_GET_VRING_BASE,=A0 =A0/* VHOST_USER_= GET_VRING_BASE */ > >=A0 =A0 =A0>=A0 =A0 =A0 VHOST_SET_VRING_KICK,=A0 =A0/* VHOST_USER_= SET_VRING_KICK */ > >=A0 =A0 =A0>=A0 =A0 =A0 VHOST_SET_VRING_CALL,=A0 =A0/* VHOST_USER_= SET_VRING_CALL */ > >=A0 =A0 =A0> -=A0 =A0 VHOST_SET_VRING_ERR=A0 =A0 =A0/* VHOST_USER_= SET_VRING_ERR */ > >=A0 =A0 =A0> +=A0 =A0 VHOST_SET_VRING_ERR,=A0 =A0 /* VHOST_USER_SE= T_VRING_ERR */ > >=A0 =A0 =A0> +=A0 =A0 VHOST_NET_INJECT_RARP=A0 =A0/* VHOST_USER_NE= T_INJECT_RARP */ > >=A0 =A0 =A0>=A0 }; > >=A0 =A0 =A0> > >=A0 =A0 =A0>=A0 static VhostUserRequest vhost_user_request_transla= te(unsigned long > int > >=A0 =A0 =A0request) > >=A0 =A0 =A0> @@ -287,6 +290,12 @@ static int vhost_user_call(struc= t vhost_dev > *dev, > >=A0 =A0 =A0unsigned long int request, > >=A0 =A0 =A0>=A0 =A0 =A0 =A0 =A0 =A0 =A0 msg.u64 |=3D VHOST_USER_VR= ING_NOFD_MASK; > >=A0 =A0 =A0>=A0 =A0 =A0 =A0 =A0 } > >=A0 =A0 =A0>=A0 =A0 =A0 =A0 =A0 break; > >=A0 =A0 =A0> + > >=A0 =A0 =A0> +=A0 =A0 case VHOST_NET_INJECT_RARP: > >=A0 =A0 =A0> +=A0 =A0 =A0 =A0 memcpy(&msg.rarp, arg, sizeof(struct= vhost_inject_rarp)); > >=A0 =A0 =A0> +=A0 =A0 =A0 =A0 msg.size =3D sizeof(struct vhost_inj= ect_rarp); > >=A0 =A0 =A0> +=A0 =A0 =A0 =A0 break; > >=A0 =A0 =A0> + > >=A0 =A0 =A0>=A0 =A0 =A0 default: > >=A0 =A0 =A0>=A0 =A0 =A0 =A0 =A0 error_report("vhost-user trying to= send unhandled ioctl"); > >=A0 =A0 =A0>=A0 =A0 =A0 =A0 =A0 return -1; > >=A0 =A0 =A0> diff --git a/linux-headers/linux/vhost.h b/linux-head= ers/linux/ > vhost.h > >=A0 =A0 =A0> index c656f61..1920134 100644 > >=A0 =A0 =A0> --- a/linux-headers/linux/vhost.h > >=A0 =A0 =A0> +++ b/linux-headers/linux/vhost.h > >=A0 =A0 =A0> @@ -63,6 +63,10 @@ struct vhost_memory { > >=A0 =A0 =A0>=A0 =A0 =A0 =A0struct vhost_memory_region regions[0]; > >=A0 =A0 =A0>=A0 }; > >=A0 =A0 =A0> > >=A0 =A0 =A0> +struct vhost_inject_rarp { > >=A0 =A0 =A0> +=A0 =A0 =A0__u8 rarp[60]; > >=A0 =A0 =A0> +}; > >=A0 =A0 =A0> + > >=A0 =A0 =A0>=A0 /* ioctls */ > >=A0 =A0 =A0> > >=A0 =A0 =A0>=A0 #define VHOST_VIRTIO 0xAF > >=A0 =A0 =A0> @@ -121,6 +125,11 @@ struct vhost_memory { > >=A0 =A0 =A0>=A0 =A0* device.=A0 This can be used to stop the ring = (e.g. for > migration). */ > >=A0 =A0 =A0>=A0 #define VHOST_NET_SET_BACKEND _IOW(VHOST_VIRTIO, 0= x30, struct > >=A0 =A0 =A0vhost_vring_file) > >=A0 =A0 =A0> > >=A0 =A0 =A0> +/* Inject a RARP in case of live migration for guest= that does not > >=A0 =A0 =A0support > >=A0 =A0 =A0> + * VIRTIO_NET_F_GUEST_ANNOUNCE */ > >=A0 =A0 =A0> +#define VHOST_NET_INJECT_RARP _IOW(VHOST_VIRTIO, 0x3= 1, struct > >=A0 =A0 =A0vhost_inject_rarp) > >=A0 =A0 =A0> + > >=A0 =A0 =A0> + > >=A0 =A0 =A0>=A0 /* Feature bits */ > >=A0 =A0 =A0>=A0 /* Log all write descriptors. Can be changed while= device is > active. */ > >=A0 =A0 =A0>=A0 #define VHOST_F_LOG_ALL 26 > >=A0 =A0 =A0> -- > >=A0 =A0 =A0> 1.7.10.4 > > > > >=20 >=20