[dpdk-dev] [PATCH v2 5/6] vhost: claim that we support GUEST_ANNOUNCE feature

2015-12-22 Thread Yuanhan Liu
On Tue, Dec 22, 2015 at 04:11:08PM +0800, Peter Xu wrote:
> On Thu, Dec 17, 2015 at 11:12:00AM +0800, Yuanhan Liu wrote:
> > It's actually a feature already enabled in Linux kernel. What we need to
> > do is simply to claim that we support such feature, and nothing else.
> > 
> > With that, the guest will send GARP messages after live migration.
> > 
> > Signed-off-by: Yuanhan Liu 
> > ---
> >  lib/librte_vhost/virtio-net.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/lib/librte_vhost/virtio-net.c b/lib/librte_vhost/virtio-net.c
> > index 03044f6..0ba5045 100644
> > --- a/lib/librte_vhost/virtio-net.c
> > +++ b/lib/librte_vhost/virtio-net.c
> > @@ -74,6 +74,7 @@ static struct virtio_net_config_ll *ll_root;
> >  #define VHOST_SUPPORTED_FEATURES ((1ULL << VIRTIO_NET_F_MRG_RXBUF) | \
> > (1ULL << VIRTIO_NET_F_CTRL_VQ) | \
> > (1ULL << VIRTIO_NET_F_CTRL_RX) | \
> > +   (1ULL << VIRTIO_NET_F_GUEST_ANNOUNCE) | \
> 
> Do we really need this? I can understand when guest declare with
> this VIRTIO_NET_F_GUEST_ANNOUNCE flag. With that, guest itself will
> handle the announcement after migration. However, how could I
> understand if it's declared by a vhost-user backend? What does it
> mean?
> 
> In vhost-user.txt (in QEMU repo docs/specs/), the only place that
> mentioned this is SEND_RARP:
> 
>  * VHOST_USER_SEND_RARP
> 
>   Id: 19
>   Equivalent ioctl: N/A
>   Master payload: u64
> 
>   Ask vhost user backend to broadcast a fake RARP to notify the migration
>   is terminated for guest that does not support GUEST_ANNOUNCE.
> ...
> 
> Here, it mention the GUEST_ANNOUNCE since when guest support this,
> we do not need to send SEND_RARP to vhost-user backend again. It
> never explain what does it mean when vhost-user declaring to have
> this flag...

The actually work is done in QEMU and guest kernel. QEMU sends a config
interrupt to guest kernel when such flag is enabled after live migration
(see QEMU code virtio_net_load()). Guest kernel generates the GARP
once it receives the interrupt (see the kernel code 
virtnet_config_changed_work()).

Therefore, we could simply claim that we support VIRTIO_NET_F_GUEST_ANNOUNCE
feature and do nothing here.

--yliu


[dpdk-dev] [PATCH v2 5/6] vhost: claim that we support GUEST_ANNOUNCE feature

2015-12-22 Thread Peter Xu
On Thu, Dec 17, 2015 at 11:12:00AM +0800, Yuanhan Liu wrote:
> It's actually a feature already enabled in Linux kernel. What we need to
> do is simply to claim that we support such feature, and nothing else.
> 
> With that, the guest will send GARP messages after live migration.
> 
> Signed-off-by: Yuanhan Liu 
> ---
>  lib/librte_vhost/virtio-net.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/lib/librte_vhost/virtio-net.c b/lib/librte_vhost/virtio-net.c
> index 03044f6..0ba5045 100644
> --- a/lib/librte_vhost/virtio-net.c
> +++ b/lib/librte_vhost/virtio-net.c
> @@ -74,6 +74,7 @@ static struct virtio_net_config_ll *ll_root;
>  #define VHOST_SUPPORTED_FEATURES ((1ULL << VIRTIO_NET_F_MRG_RXBUF) | \
>   (1ULL << VIRTIO_NET_F_CTRL_VQ) | \
>   (1ULL << VIRTIO_NET_F_CTRL_RX) | \
> + (1ULL << VIRTIO_NET_F_GUEST_ANNOUNCE) | \

Do we really need this? I can understand when guest declare with
this VIRTIO_NET_F_GUEST_ANNOUNCE flag. With that, guest itself will
handle the announcement after migration. However, how could I
understand if it's declared by a vhost-user backend? What does it
mean?

In vhost-user.txt (in QEMU repo docs/specs/), the only place that
mentioned this is SEND_RARP:

 * VHOST_USER_SEND_RARP

  Id: 19
  Equivalent ioctl: N/A
  Master payload: u64

  Ask vhost user backend to broadcast a fake RARP to notify the migration
  is terminated for guest that does not support GUEST_ANNOUNCE.
  ...

Here, it mention the GUEST_ANNOUNCE since when guest support this,
we do not need to send SEND_RARP to vhost-user backend again. It
never explain what does it mean when vhost-user declaring to have
this flag...

Thanks.
Peter

>   (VHOST_SUPPORTS_MQ)| \
>   (1ULL << VIRTIO_F_VERSION_1)   | \
>   (1ULL << VHOST_F_LOG_ALL)  | \
> -- 
> 1.9.0
> 


[dpdk-dev] [PATCH v2 5/6] vhost: claim that we support GUEST_ANNOUNCE feature

2015-12-22 Thread Pavel Fedin
 Hello!

> > diff --git a/lib/librte_vhost/virtio-net.c b/lib/librte_vhost/virtio-net.c
> > index 03044f6..0ba5045 100644
> > --- a/lib/librte_vhost/virtio-net.c
> > +++ b/lib/librte_vhost/virtio-net.c
> > @@ -74,6 +74,7 @@ static struct virtio_net_config_ll *ll_root;
> >  #define VHOST_SUPPORTED_FEATURES ((1ULL << VIRTIO_NET_F_MRG_RXBUF) | \
> > (1ULL << VIRTIO_NET_F_CTRL_VQ) | \
> > (1ULL << VIRTIO_NET_F_CTRL_RX) | \
> > +   (1ULL << VIRTIO_NET_F_GUEST_ANNOUNCE) | \
> 
> Do we really need this? I can understand when guest declare with
> this VIRTIO_NET_F_GUEST_ANNOUNCE flag. With that, guest itself will
> handle the announcement after migration. However, how could I
> understand if it's declared by a vhost-user backend?

 I guess the documentation is unclear. This is due to way how qemu works. It 
queries vhost-user backend for the features, then offers them to the guest. The 
guest then responds with features FROM THE SUGGESTED SET, which it supports. 
So, if the backend does not claim to support this feature, qemu will not offer 
it to the guest, therefore the guest will not try to activate it.
 I think this is done because this feature is only useful for migration. If 
vhost-user backend does not support migration, it needs neither 
VHOST_USER_SEND_RARP nor guest-side announce.
 Actually, i was thinking about patching qemu once, but... The changeset seemed 
too complicated, and i imagined the situation described in the above sentence, 
so decided to abandon it.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia




[dpdk-dev] [PATCH v2 5/6] vhost: claim that we support GUEST_ANNOUNCE feature

2015-12-17 Thread Yuanhan Liu
It's actually a feature already enabled in Linux kernel. What we need to
do is simply to claim that we support such feature, and nothing else.

With that, the guest will send GARP messages after live migration.

Signed-off-by: Yuanhan Liu 
---
 lib/librte_vhost/virtio-net.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/librte_vhost/virtio-net.c b/lib/librte_vhost/virtio-net.c
index 03044f6..0ba5045 100644
--- a/lib/librte_vhost/virtio-net.c
+++ b/lib/librte_vhost/virtio-net.c
@@ -74,6 +74,7 @@ static struct virtio_net_config_ll *ll_root;
 #define VHOST_SUPPORTED_FEATURES ((1ULL << VIRTIO_NET_F_MRG_RXBUF) | \
(1ULL << VIRTIO_NET_F_CTRL_VQ) | \
(1ULL << VIRTIO_NET_F_CTRL_RX) | \
+   (1ULL << VIRTIO_NET_F_GUEST_ANNOUNCE) | \
(VHOST_SUPPORTS_MQ)| \
(1ULL << VIRTIO_F_VERSION_1)   | \
(1ULL << VHOST_F_LOG_ALL)  | \
-- 
1.9.0