[dpdk-dev] [PATCH] net/virtio-user: fix feature setting with vhost-net backend

[dpdk-dev] [PATCH] net/virtio-user: fix feature setting with vhost-net backend

[Jiayu Hu]

Currently, when the backend is vhost-net, which implies
virtio-user works in client mode, virtio-user is assigned
to the default feature VIRTIO_USER_SUPPORTED_FEATURES.
However, virtio-user should request features from the
backend in this case.

Fixes: bd8f50a45d0f ("net/virtio-user: support server mode")
Signed-off-by: Jiayu Hu <jiayu.hu@intel.com>
---
 drivers/net/virtio/virtio_user/virtio_user_dev.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c
index 38b8bc9..f745163 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
@@ -353,20 +353,22 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
 		return -1;
 	}
 
-	if (dev->vhostfd >= 0) {
+	if (!dev->is_server) {
 		if (dev->ops->send_request(dev, VHOST_USER_SET_OWNER,
-					   NULL) < 0) {
+					NULL) < 0) {
 			PMD_INIT_LOG(ERR, "set_owner fails: %s",
-				     strerror(errno));
+					strerror(errno));
 			return -1;
 		}
 
 		if (dev->ops->send_request(dev, VHOST_USER_GET_FEATURES,
-					   &dev->device_features) < 0) {
+					&dev->device_features) < 0) {
 			PMD_INIT_LOG(ERR, "get_features failed: %s",
-				     strerror(errno));
+					strerror(errno));
 			return -1;
 		}
+		if (dev->mac_specified)
+			dev->device_features |= (1ull << VIRTIO_NET_F_MAC);
 	} else {
 		/* We just pretend vhost-user can support all these features.
 		 * Note that this could be problematic that if some feature is
@@ -376,9 +378,6 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
 		dev->device_features = VIRTIO_USER_SUPPORTED_FEATURES;
 	}
 
-	if (dev->mac_specified)
-		dev->device_features |= (1ull << VIRTIO_NET_F_MAC);
-
 	if (cq) {
 		/* device does not really need to know anything about CQ,
 		 * so if necessary, we just claim to support CQ
-- 
2.7.4

Re: [dpdk-dev] [PATCH] net/virtio-user: fix feature setting with vhost-net backend

[Tiwei Bie]

On Mon, May 07, 2018 at 02:50:06PM +0800, Jiayu Hu wrote:
> Currently, when the backend is vhost-net, which implies
> virtio-user works in client mode, virtio-user is assigned
> to the default feature VIRTIO_USER_SUPPORTED_FEATURES.
> However, virtio-user should request features from the
> backend in this case.
> 
> Fixes: bd8f50a45d0f ("net/virtio-user: support server mode")
> Signed-off-by: Jiayu Hu <jiayu.hu@intel.com>
> ---
>  drivers/net/virtio/virtio_user/virtio_user_dev.c | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> index 38b8bc9..f745163 100644
> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> @@ -353,20 +353,22 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
>  		return -1;
>  	}
>  
> -	if (dev->vhostfd >= 0) {
> +	if (!dev->is_server) {

Server mode also needs to run below code when the
connection to the backend is established.

>  		if (dev->ops->send_request(dev, VHOST_USER_SET_OWNER,
> -					   NULL) < 0) {
> +					NULL) < 0) {

There is no need to change the indent.

>  			PMD_INIT_LOG(ERR, "set_owner fails: %s",
> -				     strerror(errno));
> +					strerror(errno));

There is no need to change the indent.

>  			return -1;
>  		}
>  
>  		if (dev->ops->send_request(dev, VHOST_USER_GET_FEATURES,
> -					   &dev->device_features) < 0) {
> +					&dev->device_features) < 0) {

There is no need to change the indent.

>  			PMD_INIT_LOG(ERR, "get_features failed: %s",
> -				     strerror(errno));
> +					strerror(errno));

There is no need to change the indent.

>  			return -1;
>  		}
> +		if (dev->mac_specified)
> +			dev->device_features |= (1ull << VIRTIO_NET_F_MAC);

Why move above code?

>  	} else {
>  		/* We just pretend vhost-user can support all these features.
>  		 * Note that this could be problematic that if some feature is
> @@ -376,9 +378,6 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
>  		dev->device_features = VIRTIO_USER_SUPPORTED_FEATURES;
>  	}
>  
> -	if (dev->mac_specified)
> -		dev->device_features |= (1ull << VIRTIO_NET_F_MAC);
> -
>  	if (cq) {
>  		/* device does not really need to know anything about CQ,
>  		 * so if necessary, we just claim to support CQ
> -- 
> 2.7.4
> 

Re: [dpdk-dev] [PATCH] net/virtio-user: fix feature setting with vhost-net backend

[Jiayu Hu]

Hi Tiwei,

On Tue, May 08, 2018 at 10:49:50AM +0800, Tiwei Bie wrote:
> On Mon, May 07, 2018 at 02:50:06PM +0800, Jiayu Hu wrote:
> > Currently, when the backend is vhost-net, which implies
> > virtio-user works in client mode, virtio-user is assigned
> > to the default feature VIRTIO_USER_SUPPORTED_FEATURES.
> > However, virtio-user should request features from the
> > backend in this case.
> > 
> > Fixes: bd8f50a45d0f ("net/virtio-user: support server mode")
> > Signed-off-by: Jiayu Hu <jiayu.hu@intel.com>
> > ---
> >  drivers/net/virtio/virtio_user/virtio_user_dev.c | 15 +++++++--------
> >  1 file changed, 7 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> > index 38b8bc9..f745163 100644
> > --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> > +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> > @@ -353,20 +353,22 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
> >  		return -1;
> >  	}
> >  
> > -	if (dev->vhostfd >= 0) {
> > +	if (!dev->is_server) {
> 
> Server mode also needs to run below code when the
> connection to the backend is established.

For server mode, the below code is called by virtio_user_start_device(),
instead of virtio_user_dev_init(). Only client mode virtio-user
needs to call it when init.

> 
> >  		if (dev->ops->send_request(dev, VHOST_USER_SET_OWNER,
> > -					   NULL) < 0) {
> > +					NULL) < 0) {
> 
> There is no need to change the indent.

Thanks, change in the next patch.

> 
> >  			PMD_INIT_LOG(ERR, "set_owner fails: %s",
> > -				     strerror(errno));
> > +					strerror(errno));
> 
> There is no need to change the indent.

ditto.

> 
> >  			return -1;
> >  		}
> >  
> >  		if (dev->ops->send_request(dev, VHOST_USER_GET_FEATURES,
> > -					   &dev->device_features) < 0) {
> > +					&dev->device_features) < 0) {
> 
> There is no need to change the indent.

ditto.

> 
> >  			PMD_INIT_LOG(ERR, "get_features failed: %s",
> > -				     strerror(errno));
> > +					strerror(errno));
> 
> There is no need to change the indent.

ditto.

> 
> >  			return -1;
> >  		}
> > +		if (dev->mac_specified)
> > +			dev->device_features |= (1ull << VIRTIO_NET_F_MAC);
> 
> Why move above code?

The server mode virtio-user is assigned the default features, which include
VIRTIO_NET_F_MAC. So checking if support VIRTIO_NET_F_MAC is only necessary
for client mode.

Thanks,
Jiayu

> 
> >  	} else {
> >  		/* We just pretend vhost-user can support all these features.
> >  		 * Note that this could be problematic that if some feature is
> > @@ -376,9 +378,6 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
> >  		dev->device_features = VIRTIO_USER_SUPPORTED_FEATURES;
> >  	}
> >  
> > -	if (dev->mac_specified)
> > -		dev->device_features |= (1ull << VIRTIO_NET_F_MAC);
> > -
> >  	if (cq) {
> >  		/* device does not really need to know anything about CQ,
> >  		 * so if necessary, we just claim to support CQ
> > -- 
> > 2.7.4
> > 

[dpdk-dev] [PATCH v2] net/virtio-user: fix feature setting with vhost-net backend

[Jiayu Hu]

When the backend is vhost-net, virtio-user must work in client mode and
needs to request features from the backend in virtio_user_dev_init().
But currently, virtio-user is assigned to default features in this case.

This patch is to fix this inappropriate feature setting.

Fixes: bd8f50a45d0f ("net/virtio-user: support server mode")
Signed-off-by: Jiayu Hu <jiayu.hu@intel.com>
---
changes in v2:
- remove unnecessary indent change.
- change commit log.

 drivers/net/virtio/virtio_user/virtio_user_dev.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c
index 38b8bc9..2d80188 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
@@ -353,7 +353,7 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
 		return -1;
 	}
 
-	if (dev->vhostfd >= 0) {
+	if (!dev->is_server) {
 		if (dev->ops->send_request(dev, VHOST_USER_SET_OWNER,
 					   NULL) < 0) {
 			PMD_INIT_LOG(ERR, "set_owner fails: %s",
@@ -367,6 +367,8 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
 				     strerror(errno));
 			return -1;
 		}
+		if (dev->mac_specified)
+			dev->device_features |= (1ull << VIRTIO_NET_F_MAC);
 	} else {
 		/* We just pretend vhost-user can support all these features.
 		 * Note that this could be problematic that if some feature is
@@ -376,9 +378,6 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
 		dev->device_features = VIRTIO_USER_SUPPORTED_FEATURES;
 	}
 
-	if (dev->mac_specified)
-		dev->device_features |= (1ull << VIRTIO_NET_F_MAC);
-
 	if (cq) {
 		/* device does not really need to know anything about CQ,
 		 * so if necessary, we just claim to support CQ
-- 
2.7.4

Re: [dpdk-dev] [PATCH v2] net/virtio-user: fix feature setting with vhost-net backend

[Yao, Lei A]

> -----Original Message-----
> From: Hu, Jiayu
> Sent: Tuesday, May 8, 2018 4:15 PM
> To: dev@dpdk.org
> Cc: Bie, Tiwei <tiwei.bie@intel.com>; Yang, Zhiyong
> <zhiyong.yang@intel.com>; maxime.coquelin@redhat.com; Yao, Lei A
> <lei.a.yao@intel.com>; Hu, Jiayu <jiayu.hu@intel.com>
> Subject: [PATCH v2] net/virtio-user: fix feature setting with vhost-net
> backend
> 
> When the backend is vhost-net, virtio-user must work in client mode and
> needs to request features from the backend in virtio_user_dev_init().
> But currently, virtio-user is assigned to default features in this case.
> 
> This patch is to fix this inappropriate feature setting.
> 
> Fixes: bd8f50a45d0f ("net/virtio-user: support server mode")
> Signed-off-by: Jiayu Hu <jiayu.hu@intel.com>
Tested-by: Lei Yao <lei.a.yao@intel.com>
Tested this patch based on 18.05-rc2. This patch fixed the vhost-net
kernel backend issue with DPDK. 
Basic test with virtio-user server mode is also pass. 
> ---
> changes in v2:
> - remove unnecessary indent change.
> - change commit log.
> 
>  drivers/net/virtio/virtio_user/virtio_user_dev.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> index 38b8bc9..2d80188 100644
> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> @@ -353,7 +353,7 @@ virtio_user_dev_init(struct virtio_user_dev *dev,
> char *path, int queues,
>  		return -1;
>  	}
> 
> -	if (dev->vhostfd >= 0) {
> +	if (!dev->is_server) {
>  		if (dev->ops->send_request(dev,
> VHOST_USER_SET_OWNER,
>  					   NULL) < 0) {
>  			PMD_INIT_LOG(ERR, "set_owner fails: %s",
> @@ -367,6 +367,8 @@ virtio_user_dev_init(struct virtio_user_dev *dev,
> char *path, int queues,
>  				     strerror(errno));
>  			return -1;
>  		}
> +		if (dev->mac_specified)
> +			dev->device_features |= (1ull <<
> VIRTIO_NET_F_MAC);
>  	} else {
>  		/* We just pretend vhost-user can support all these features.
>  		 * Note that this could be problematic that if some feature is
> @@ -376,9 +378,6 @@ virtio_user_dev_init(struct virtio_user_dev *dev,
> char *path, int queues,
>  		dev->device_features =
> VIRTIO_USER_SUPPORTED_FEATURES;
>  	}
> 
> -	if (dev->mac_specified)
> -		dev->device_features |= (1ull << VIRTIO_NET_F_MAC);
> -
>  	if (cq) {
>  		/* device does not really need to know anything about CQ,
>  		 * so if necessary, we just claim to support CQ
> --
> 2.7.4

Re: [dpdk-dev] [PATCH] net/virtio-user: fix feature setting with vhost-net backend

[Tiwei Bie]

On Tue, May 08, 2018 at 03:44:22PM +0800, Jiayu Hu wrote:
> Hi Tiwei,
> 
> On Tue, May 08, 2018 at 10:49:50AM +0800, Tiwei Bie wrote:
> > On Mon, May 07, 2018 at 02:50:06PM +0800, Jiayu Hu wrote:
> > > Currently, when the backend is vhost-net, which implies
> > > virtio-user works in client mode, virtio-user is assigned
> > > to the default feature VIRTIO_USER_SUPPORTED_FEATURES.
> > > However, virtio-user should request features from the
> > > backend in this case.
> > > 
> > > Fixes: bd8f50a45d0f ("net/virtio-user: support server mode")
> > > Signed-off-by: Jiayu Hu <jiayu.hu@intel.com>
> > > ---
> > >  drivers/net/virtio/virtio_user/virtio_user_dev.c | 15 +++++++--------
> > >  1 file changed, 7 insertions(+), 8 deletions(-)
> > > 
> > > diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> > > index 38b8bc9..f745163 100644
> > > --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> > > +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> > > @@ -353,20 +353,22 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
> > >  		return -1;
> > >  	}
> > >  
> > > -	if (dev->vhostfd >= 0) {
> > > +	if (!dev->is_server) {
> > 
> > Server mode also needs to run below code when the
> > connection to the backend is established.
> 
> For server mode, the below code is called by virtio_user_start_device(),
> instead of virtio_user_dev_init(). Only client mode virtio-user
> needs to call it when init.

Okay. So in server mode, virtio-user never get a chance
to do GET_FEATURES.

> 
> > 
> > >  		if (dev->ops->send_request(dev, VHOST_USER_SET_OWNER,
> > > -					   NULL) < 0) {
> > > +					NULL) < 0) {
> > 
> > There is no need to change the indent.
> 
> Thanks, change in the next patch.
> 
> > 
> > >  			PMD_INIT_LOG(ERR, "set_owner fails: %s",
> > > -				     strerror(errno));
> > > +					strerror(errno));
> > 
> > There is no need to change the indent.
> 
> ditto.
> 
> > 
> > >  			return -1;
> > >  		}
> > >  
> > >  		if (dev->ops->send_request(dev, VHOST_USER_GET_FEATURES,
> > > -					   &dev->device_features) < 0) {
> > > +					&dev->device_features) < 0) {
> > 
> > There is no need to change the indent.
> 
> ditto.
> 
> > 
> > >  			PMD_INIT_LOG(ERR, "get_features failed: %s",
> > > -				     strerror(errno));
> > > +					strerror(errno));
> > 
> > There is no need to change the indent.
> 
> ditto.
> 
> > 
> > >  			return -1;
> > >  		}
> > > +		if (dev->mac_specified)
> > > +			dev->device_features |= (1ull << VIRTIO_NET_F_MAC);
> > 
> > Why move above code?
> 
> The server mode virtio-user is assigned the default features, which include
> VIRTIO_NET_F_MAC. So checking if support VIRTIO_NET_F_MAC is only necessary
> for client mode.

I think we should set this bit only when mac is
specified. And in server mode, we need to clear
this bit when mac isn't specified. So instead
of moving it, it should be changed to something
like this:

	if (dev->mac_specified)
		dev->device_features |= (1ull << VIRTIO_NET_F_MAC);
	else
		dev->device_features &= ~(1ull << VIRTIO_NET_F_MAC);

Thanks

> 
> Thanks,
> Jiayu
> 
> > 
> > >  	} else {
> > >  		/* We just pretend vhost-user can support all these features.
> > >  		 * Note that this could be problematic that if some feature is
> > > @@ -376,9 +378,6 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
> > >  		dev->device_features = VIRTIO_USER_SUPPORTED_FEATURES;
> > >  	}
> > >  
> > > -	if (dev->mac_specified)
> > > -		dev->device_features |= (1ull << VIRTIO_NET_F_MAC);
> > > -
> > >  	if (cq) {
> > >  		/* device does not really need to know anything about CQ,
> > >  		 * so if necessary, we just claim to support CQ
> > > -- 
> > > 2.7.4
> > > 

Re: [dpdk-dev] [PATCH] net/virtio-user: fix feature setting with vhost-net backend

[Hu, Jiayu]

Hi Tiwei,

> -----Original Message-----
> From: Bie, Tiwei
> Sent: Thursday, May 10, 2018 1:05 PM
> To: Hu, Jiayu <jiayu.hu@intel.com>
> Cc: dev@dpdk.org; Yang, Zhiyong <zhiyong.yang@intel.com>;
> maxime.coquelin@redhat.com
> Subject: Re: [dpdk-dev] [PATCH] net/virtio-user: fix feature setting with
> vhost-net backend
> 
> On Tue, May 08, 2018 at 03:44:22PM +0800, Jiayu Hu wrote:
> > Hi Tiwei,
> >
> > On Tue, May 08, 2018 at 10:49:50AM +0800, Tiwei Bie wrote:
> > > On Mon, May 07, 2018 at 02:50:06PM +0800, Jiayu Hu wrote:
> > > > Currently, when the backend is vhost-net, which implies
> > > > virtio-user works in client mode, virtio-user is assigned
> > > > to the default feature VIRTIO_USER_SUPPORTED_FEATURES.
> > > > However, virtio-user should request features from the
> > > > backend in this case.
> > > >
> > > > Fixes: bd8f50a45d0f ("net/virtio-user: support server mode")
> > > > Signed-off-by: Jiayu Hu <jiayu.hu@intel.com>
> > > > ---
> > > >  drivers/net/virtio/virtio_user/virtio_user_dev.c | 15 +++++++--------
> > > >  1 file changed, 7 insertions(+), 8 deletions(-)
> > > >
> > > > diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> > > > index 38b8bc9..f745163 100644
> > > > --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> > > > +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> > > > @@ -353,20 +353,22 @@ virtio_user_dev_init(struct virtio_user_dev
> *dev, char *path, int queues,
> > > >  		return -1;
> > > >  	}
> > > >
> > > > -	if (dev->vhostfd >= 0) {
> > > > +	if (!dev->is_server) {
> > >
> > > Server mode also needs to run below code when the
> > > connection to the backend is established.
> >
> > For server mode, the below code is called by virtio_user_start_device(),
> > instead of virtio_user_dev_init(). Only client mode virtio-user
> > needs to call it when init.
> 
> Okay. So in server mode, virtio-user never get a chance
> to do GET_FEATURES.

In current design, virtio-user server mode always has no way to do GET_FEATURES.
I think it's the design problem of virtio-user server mode. This patch is just for solving
client mode virtio-user feature setting.

> 
> >
> > >
> > > >  		if (dev->ops->send_request(dev,
> VHOST_USER_SET_OWNER,
> > > > -					   NULL) < 0) {
> > > > +					NULL) < 0) {
> > >
> > > There is no need to change the indent.
> >
> > Thanks, change in the next patch.
> >
> > >
> > > >  			PMD_INIT_LOG(ERR, "set_owner fails: %s",
> > > > -				     strerror(errno));
> > > > +					strerror(errno));
> > >
> > > There is no need to change the indent.
> >
> > ditto.
> >
> > >
> > > >  			return -1;
> > > >  		}
> > > >
> > > >  		if (dev->ops->send_request(dev,
> VHOST_USER_GET_FEATURES,
> > > > -					   &dev->device_features) < 0) {
> > > > +					&dev->device_features) < 0) {
> > >
> > > There is no need to change the indent.
> >
> > ditto.
> >
> > >
> > > >  			PMD_INIT_LOG(ERR, "get_features failed: %s",
> > > > -				     strerror(errno));
> > > > +					strerror(errno));
> > >
> > > There is no need to change the indent.
> >
> > ditto.
> >
> > >
> > > >  			return -1;
> > > >  		}
> > > > +		if (dev->mac_specified)
> > > > +			dev->device_features |= (1ull <<
> VIRTIO_NET_F_MAC);
> > >
> > > Why move above code?
> >
> > The server mode virtio-user is assigned the default features, which include
> > VIRTIO_NET_F_MAC. So checking if support VIRTIO_NET_F_MAC is only
> necessary
> > for client mode.
> 
> I think we should set this bit only when mac is
> specified. And in server mode, we need to clear
> this bit when mac isn't specified. So instead
> of moving it, it should be changed to something
> like this:
> 
> 	if (dev->mac_specified)
> 		dev->device_features |= (1ull << VIRTIO_NET_F_MAC);
> 	else
> 		dev->device_features &= ~(1ull << VIRTIO_NET_F_MAC);
> 

Make sense. It shouldn't change in this patch. I will change it. Thanks.

Jiayu
> Thanks
> 
> >
> > Thanks,
> > Jiayu
> >
> > >
> > > >  	} else {
> > > >  		/* We just pretend vhost-user can support all these
> features.
> > > >  		 * Note that this could be problematic that if some feature
> is
> > > > @@ -376,9 +378,6 @@ virtio_user_dev_init(struct virtio_user_dev
> *dev, char *path, int queues,
> > > >  		dev->device_features =
> VIRTIO_USER_SUPPORTED_FEATURES;
> > > >  	}
> > > >
> > > > -	if (dev->mac_specified)
> > > > -		dev->device_features |= (1ull << VIRTIO_NET_F_MAC);
> > > > -
> > > >  	if (cq) {
> > > >  		/* device does not really need to know anything about CQ,
> > > >  		 * so if necessary, we just claim to support CQ
> > > > --
> > > > 2.7.4
> > > >

[dpdk-dev] [PATCH v3] net/virtio-user: fix feature setting with vhost-net backend

[Jiayu Hu]

When the backend is vhost-net, virtio-user must work in client mode and
needs to request features from the backend in virtio_user_dev_init().
But currently, virtio-user is assigned to default features in this case.

This patch is to fix this inappropriate feature setting.

Fixes: bd8f50a45d0f ("net/virtio-user: support server mode")
Signed-off-by: Jiayu Hu <jiayu.hu@intel.com>
Tested-by: Lei Yao <lei.a.yao@intel.com>
---
changes in v3:
- remove unnecessary code change.
changes in v2:
- remove unnecessary indent change.
- change commit log.

 drivers/net/virtio/virtio_user/virtio_user_dev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c
index 38b8bc9..f8ef7e2 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
@@ -353,7 +353,7 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
 		return -1;
 	}
 
-	if (dev->vhostfd >= 0) {
+	if (!dev->is_server) {
 		if (dev->ops->send_request(dev, VHOST_USER_SET_OWNER,
 					   NULL) < 0) {
 			PMD_INIT_LOG(ERR, "set_owner fails: %s",
-- 
2.7.4

Re: [dpdk-dev] [PATCH v3] net/virtio-user: fix feature setting with vhost-net backend

[Tiwei Bie]

On Fri, May 11, 2018 at 03:26:07PM +0800, Jiayu Hu wrote:
> When the backend is vhost-net, virtio-user must work in client mode and
> needs to request features from the backend in virtio_user_dev_init().
> But currently, virtio-user is assigned to default features in this case.
> 
> This patch is to fix this inappropriate feature setting.
> 
> Fixes: bd8f50a45d0f ("net/virtio-user: support server mode")
> Signed-off-by: Jiayu Hu <jiayu.hu@intel.com>
> Tested-by: Lei Yao <lei.a.yao@intel.com>
> ---
> changes in v3:
> - remove unnecessary code change.
> changes in v2:
> - remove unnecessary indent change.
> - change commit log.
> 
>  drivers/net/virtio/virtio_user/virtio_user_dev.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> index 38b8bc9..f8ef7e2 100644
> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> @@ -353,7 +353,7 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
>  		return -1;
>  	}
>  
> -	if (dev->vhostfd >= 0) {
> +	if (!dev->is_server) {
>  		if (dev->ops->send_request(dev, VHOST_USER_SET_OWNER,
>  					   NULL) < 0) {
>  			PMD_INIT_LOG(ERR, "set_owner fails: %s",
> -- 
> 2.7.4
> 

Reviewed-by: Tiwei Bie <tiwei.bie@intel.com>

Re: [dpdk-dev] [PATCH v3] net/virtio-user: fix feature setting with vhost-net backend

[Yang, Zhiyong]

> -----Original Message-----
> From: Bie, Tiwei
> Sent: Friday, May 11, 2018 3:33 PM
> To: Hu, Jiayu <jiayu.hu@intel.com>
> Cc: dev@dpdk.org; Yang, Zhiyong <zhiyong.yang@intel.com>
> Subject: Re: [PATCH v3] net/virtio-user: fix feature setting with vhost-net
> backend
> 
> On Fri, May 11, 2018 at 03:26:07PM +0800, Jiayu Hu wrote:
> > When the backend is vhost-net, virtio-user must work in client mode
> > and needs to request features from the backend in virtio_user_dev_init().
> > But currently, virtio-user is assigned to default features in this case.
> >
> > This patch is to fix this inappropriate feature setting.
> >
> > Fixes: bd8f50a45d0f ("net/virtio-user: support server mode")
> > Signed-off-by: Jiayu Hu <jiayu.hu@intel.com>
> > Tested-by: Lei Yao <lei.a.yao@intel.com>
> > ---
> > changes in v3:
> > - remove unnecessary code change.
> > changes in v2:
> > - remove unnecessary indent change.
> > - change commit log.
> >
> >  drivers/net/virtio/virtio_user/virtio_user_dev.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >

Reviewed-by: Zhiyong Yang <zhyong.yang@intel.com>

Re: [dpdk-dev] [PATCH v3] net/virtio-user: fix feature setting with vhost-net backend

[Maxime Coquelin]

On 05/11/2018 09:26 AM, Jiayu Hu wrote:
> When the backend is vhost-net, virtio-user must work in client mode and
> needs to request features from the backend in virtio_user_dev_init().
> But currently, virtio-user is assigned to default features in this case.
> 
> This patch is to fix this inappropriate feature setting.
> 
> Fixes: bd8f50a45d0f ("net/virtio-user: support server mode")
> Signed-off-by: Jiayu Hu <jiayu.hu@intel.com>
> Tested-by: Lei Yao <lei.a.yao@intel.com>
> ---
> changes in v3:
> - remove unnecessary code change.
> changes in v2:
> - remove unnecessary indent change.
> - change commit log.
> 
>   drivers/net/virtio/virtio_user/virtio_user_dev.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 


Applied to dpdk-next-virtio/master.

Thanks!
Maxime