[dpdk-dev,01/12] lib/librte_vhost: enable VIRTIO_NET_F_CTRL_RX

VIRTIO_NET_F_CTRL_RX is dependant on VIRTIO_NET_F_CTRL_VQ.

Observed that virtio-net driver in guest would crash with only CTRL_RX enabled.

In virtnet_send_command:

	/* Caller should know better */
	BUG_ON(!virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_VQ) ||
		(out + in > VIRTNET_SEND_COMMAND_SG_MAX));

Signed-off-by: Huawei Xie <huawei.xie@intel.com>
---
 lib/librte_vhost/virtio-net.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
  

On 2015/01/30 15:36, Huawei Xie wrote:
> VIRTIO_NET_F_CTRL_RX is dependant on VIRTIO_NET_F_CTRL_VQ.
>
> Observed that virtio-net driver in guest would crash with only CTRL_RX enabled.
>
> In virtnet_send_command:
>
> 	/* Caller should know better */
> 	BUG_ON(!virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_VQ) ||
> 		(out + in > VIRTNET_SEND_COMMAND_SG_MAX));
>
> Signed-off-by: Huawei Xie <huawei.xie@intel.com>
> ---
>  lib/librte_vhost/virtio-net.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/lib/librte_vhost/virtio-net.c b/lib/librte_vhost/virtio-net.c
> index b041849..52b4957 100644
> --- a/lib/librte_vhost/virtio-net.c
> +++ b/lib/librte_vhost/virtio-net.c
> @@ -73,7 +73,8 @@ static struct virtio_net_config_ll *ll_root;
>  
>  /* Features supported by this lib. */
>  #define VHOST_SUPPORTED_FEATURES ((1ULL << VIRTIO_NET_F_MRG_RXBUF) | \
> -				  (1ULL << VIRTIO_NET_F_CTRL_RX))
> +				(1ULL << VIRTIO_NET_F_CTRL_VQ) | \
> +				(1ULL << VIRTIO_NET_F_CTRL_RX))
>  static uint64_t VHOST_FEATURES = VHOST_SUPPORTED_FEATURES;
>  
>  /* Line size for reading maps file. */

Hi Xie,

Could you please check below code?

---------------------
examples/vhost/main.c
---------------------
                case 'P':
                        promiscuous = 1;
                        vmdq_conf_default.rx_adv_conf.vmdq_rx_conf.rx_mode =
                                ETH_VMDQ_ACCEPT_BROADCAST |
                                ETH_VMDQ_ACCEPT_MULTICAST;
                        rte_vhost_feature_enable(1ULL <<
VIRTIO_NET_F_CTRL_RX);


VIRTIO_NET_F_CTRL_RX is always enabled by this patch.
So if 'P' isn't specified in vhost example, does it need to be disabled?

Thanks,
Tetsuya
  

> @@ -73,7 +73,8 @@ static struct virtio_net_config_ll *ll_root;

> >

> >  /* Features supported by this lib. */

> >  #define VHOST_SUPPORTED_FEATURES ((1ULL <<

> VIRTIO_NET_F_MRG_RXBUF) | \

> > -				  (1ULL << VIRTIO_NET_F_CTRL_RX))

> > +				(1ULL << VIRTIO_NET_F_CTRL_VQ) | \

> > +				(1ULL << VIRTIO_NET_F_CTRL_RX))

> >  static uint64_t VHOST_FEATURES = VHOST_SUPPORTED_FEATURES;

> >

> >  /* Line size for reading maps file. */

> 

> Hi Xie,

> 

> Could you please check below code?

> 

> ---------------------

> examples/vhost/main.c

> ---------------------

>                 case 'P':

>                         promiscuous = 1;

>                         vmdq_conf_default.rx_adv_conf.vmdq_rx_conf.rx_mode =

>                                 ETH_VMDQ_ACCEPT_BROADCAST |

>                                 ETH_VMDQ_ACCEPT_MULTICAST;

>                         rte_vhost_feature_enable(1ULL <<

> VIRTIO_NET_F_CTRL_RX);

> 

> 

> VIRTIO_NET_F_CTRL_RX is always enabled by this patch.

> So if 'P' isn't specified in vhost example, does it need to be disabled?


Sounds reasonable.
I find that the subject shoud be "enable CTRL_VQ".
So issue with CTRL_RX should be fixed in other patch rather than in  this patchset.

Besides, even CTRL_VQ is a little weird, as we don't do anything with 
multiple enabling in vhost library.

> 

> Thanks,

> Tetsuya