Re: [dpdk-dev] [PATCH] net/vhost: fix segfault when creating vdev dynamically

Re: [dpdk-dev] [PATCH] net/vhost: fix segfault when creating vdev dynamically

[Tan, Jianfeng]

On 3/28/2018 12:05 AM, Junjie Chen wrote:
> when creating vdev dynamically, vhost pmd driver start directly without
> checking TX/RX queues ready or not, and thus cause segmentation fault when
> vhost library accessing queues. This patch add flag to check whether queues
> setup or not, and add driver start call into dev_start to allow user start
> it after setting up queue.

The issue is clear now. But this patch just puts the situation before 
below fix: "it doesn't create the actual datagram socket until you call 
.dev_start()."

To fix this issue, we might delay the queue setup until we really have 
queues?

> Fixes: aed0b12930b33("net/vhost: fix socket file deleted on stop")

Missed the space between commit number and title. And we keep 12 
characters for the commit id.

Thanks,
Jianfeng

> Signed-off-by: Junjie Chen <junjie.j.chen@intel.com>
> ---
>   drivers/net/vhost/rte_eth_vhost.c | 21 +++++++++++++++++++--
>   1 file changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/vhost/rte_eth_vhost.c b/drivers/net/vhost/rte_eth_vhost.c
> index 3aae01c..719a150 100644
> --- a/drivers/net/vhost/rte_eth_vhost.c
> +++ b/drivers/net/vhost/rte_eth_vhost.c
> @@ -118,6 +118,7 @@ struct pmd_internal {
>   	char *iface_name;
>   	uint16_t max_queues;
>   	rte_atomic32_t started;
> +	rte_atomic32_t once;
>   };
>   
>   struct internal_list {
> @@ -772,12 +773,24 @@ rte_eth_vhost_get_vid_from_port_id(uint16_t port_id)
>   static int
>   eth_dev_start(struct rte_eth_dev *dev)
>   {
> +	int ret = 0;
>   	struct pmd_internal *internal = dev->data->dev_private;
>   
> +	if (unlikely(rte_atomic32_read(&internal->once) == 0)) {
> +		ret = rte_vhost_driver_start(internal->iface_name);
> +		if (ret < 0) {
> +			RTE_LOG(ERR, PMD, "Failed to start driver for %s\n",
> +				internal->iface_name);
> +			return ret;
> +		}
> +
> +		rte_atomic32_set(&internal->once, 1);
> +	}
> +
>   	rte_atomic32_set(&internal->started, 1);
>   	update_queuing_status(dev);
>   
> -	return 0;
> +	return ret;
>   }
>   
>   static void
> @@ -1101,7 +1114,11 @@ eth_dev_vhost_create(struct rte_vdev_device *dev, char *iface_name,
>   		goto error;
>   	}
>   
> -	if (rte_vhost_driver_start(iface_name) < 0) {
> +	if (!data->rx_queues || !data->tx_queues) {
> +		RTE_LOG(INFO, PMD,
> +			"TX/RX queue is not ready, driver will not start\n");
> +		rte_atomic32_set(&internal->once, 0);
> +	} else if (rte_vhost_driver_start(iface_name) < 0) {
>   		RTE_LOG(ERR, PMD, "Failed to start driver for %s\n",
>   			iface_name);
>   		goto error;

Re: [dpdk-dev] [PATCH] net/vhost: fix segfault when creating vdev dynamically

[Chen, Junjie J]

Hi Jianfeng

> On 3/28/2018 12:05 AM, Junjie Chen wrote:
> > when creating vdev dynamically, vhost pmd driver start directly
> > without checking TX/RX queues ready or not, and thus cause
> > segmentation fault when vhost library accessing queues. This patch add
> > flag to check whether queues setup or not, and add driver start call
> > into dev_start to allow user start it after setting up queue.
> 
> The issue is clear now. But this patch just puts the situation before below fix:
> "it doesn't create the actual datagram socket until you call .dev_start()."

No, if the queue exist, the datagram socket still get created in vhost_create API, since the vhost_driver_register still exist in vhost_create.

> 
> To fix this issue, we might delay the queue setup until we really have queues?
> 
> > Fixes: aed0b12930b33("net/vhost: fix socket file deleted on stop")
> 
> Missed the space between commit number and title. And we keep 12
> characters for the commit id.

Thanks for reminder, will update in v2.
> 
> Thanks,
> Jianfeng
> 
> > Signed-off-by: Junjie Chen <junjie.j.chen@intel.com>
> > ---
> >   drivers/net/vhost/rte_eth_vhost.c | 21 +++++++++++++++++++--
> >   1 file changed, 19 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/vhost/rte_eth_vhost.c
> > b/drivers/net/vhost/rte_eth_vhost.c
> > index 3aae01c..719a150 100644
> > --- a/drivers/net/vhost/rte_eth_vhost.c
> > +++ b/drivers/net/vhost/rte_eth_vhost.c
> > @@ -118,6 +118,7 @@ struct pmd_internal {
> >   	char *iface_name;
> >   	uint16_t max_queues;
> >   	rte_atomic32_t started;
> > +	rte_atomic32_t once;
> >   };
> >
> >   struct internal_list {
> > @@ -772,12 +773,24 @@ rte_eth_vhost_get_vid_from_port_id(uint16_t
> port_id)
> >   static int
> >   eth_dev_start(struct rte_eth_dev *dev)
> >   {
> > +	int ret = 0;
> >   	struct pmd_internal *internal = dev->data->dev_private;
> >
> > +	if (unlikely(rte_atomic32_read(&internal->once) == 0)) {
> > +		ret = rte_vhost_driver_start(internal->iface_name);
> > +		if (ret < 0) {
> > +			RTE_LOG(ERR, PMD, "Failed to start driver for %s\n",
> > +				internal->iface_name);
> > +			return ret;
> > +		}
> > +
> > +		rte_atomic32_set(&internal->once, 1);
> > +	}
> > +
> >   	rte_atomic32_set(&internal->started, 1);
> >   	update_queuing_status(dev);
> >
> > -	return 0;
> > +	return ret;
> >   }
> >
> >   static void
> > @@ -1101,7 +1114,11 @@ eth_dev_vhost_create(struct rte_vdev_device
> *dev, char *iface_name,
> >   		goto error;
> >   	}
> >
> > -	if (rte_vhost_driver_start(iface_name) < 0) {
> > +	if (!data->rx_queues || !data->tx_queues) {
> > +		RTE_LOG(INFO, PMD,
> > +			"TX/RX queue is not ready, driver will not start\n");
> > +		rte_atomic32_set(&internal->once, 0);
> > +	} else if (rte_vhost_driver_start(iface_name) < 0) {
> >   		RTE_LOG(ERR, PMD, "Failed to start driver for %s\n",
> >   			iface_name);
> >   		goto error;

Re: [dpdk-dev] [PATCH] net/vhost: fix segfault when creating vdev dynamically

[Tan, Jianfeng]

On 3/27/2018 5:02 PM, Chen, Junjie J wrote:
> Hi Jianfeng
>
>> On 3/28/2018 12:05 AM, Junjie Chen wrote:
>>> when creating vdev dynamically, vhost pmd driver start directly
>>> without checking TX/RX queues ready or not, and thus cause
>>> segmentation fault when vhost library accessing queues. This patch add
>>> flag to check whether queues setup or not, and add driver start call
>>> into dev_start to allow user start it after setting up queue.
>> The issue is clear now. But this patch just puts the situation before below fix:
>> "it doesn't create the actual datagram socket until you call .dev_start()."
> No, if the queue exist, the datagram socket still get created in vhost_create API, since the vhost_driver_register still exist in vhost_create.

The queue can never be created, as it's still not probed.

Re: [dpdk-dev] [PATCH] net/vhost: fix segfault when creating vdev dynamically

[Chen, Junjie J]

> >
> >> On 3/28/2018 12:05 AM, Junjie Chen wrote:
> >>> when creating vdev dynamically, vhost pmd driver start directly
> >>> without checking TX/RX queues ready or not, and thus cause
> >>> segmentation fault when vhost library accessing queues. This patch
> >>> add flag to check whether queues setup or not, and add driver start
> >>> call into dev_start to allow user start it after setting up queue.
> >> The issue is clear now. But this patch just puts the situation before below
> fix:
> >> "it doesn't create the actual datagram socket until you call .dev_start()."
> > No, if the queue exist, the datagram socket still get created in vhost_create
> API, since the vhost_driver_register still exist in vhost_create.
> 
> The queue can never be created, as it's still not probed.

I think we need to separate this into two cases:
	Statically create vdev, the datagram recreate logical is still there since queues are exist already, this patch doesn't change anything.
	Dynamic create vdev, as you pointed out, queue can never be created, while this should be not valid since In normal process of creating vdev dynamically, we always need to config queues. Correct me if I'm wrong.

In summary, I think the previously commit fixes the static code path and this patch fixes the dynamic code path (we need to at least setup queue once).

Re: [dpdk-dev] [PATCH] net/vhost: fix segfault when creating vdev dynamically

[Tan, Jianfeng]

On 3/27/2018 5:24 PM, Chen, Junjie J wrote:
>>>> On 3/28/2018 12:05 AM, Junjie Chen wrote:
>>>>> when creating vdev dynamically, vhost pmd driver start directly
>>>>> without checking TX/RX queues ready or not, and thus cause
>>>>> segmentation fault when vhost library accessing queues. This patch
>>>>> add flag to check whether queues setup or not, and add driver start
>>>>> call into dev_start to allow user start it after setting up queue.
>>>> The issue is clear now. But this patch just puts the situation before below
>> fix:
>>>> "it doesn't create the actual datagram socket until you call .dev_start()."
>>> No, if the queue exist, the datagram socket still get created in vhost_create
>> API, since the vhost_driver_register still exist in vhost_create.
>>
>> The queue can never be created, as it's still not probed.
> I think we need to separate this into two cases:
> 	Statically create vdev, the datagram recreate logical is still there since queues are exist already, this patch doesn't change anything.
> 	Dynamic create vdev, as you pointed out, queue can never be created, while this should be not valid since In normal process of creating vdev dynamically, we always need to config queues. Correct me if I'm wrong.

My point is, either vdev is created statically or dynamically, when 
probe(), queues are not setup yet definitely, then *the unix socket will 
not be created* until we set up the queues and do dev_start(). If the 
unix socket is not created, then VM cannot connect to it.

> In summary, I think the previously commit fixes the static code path and this patch fixes the dynamic code path (we need to at least setup queue once).

Re: [dpdk-dev] [PATCH] net/vhost: fix segfault when creating vdev dynamically

[Chen, Junjie J]

> 
> On 3/27/2018 5:24 PM, Chen, Junjie J wrote:
> >>>> On 3/28/2018 12:05 AM, Junjie Chen wrote:
> >>>>> when creating vdev dynamically, vhost pmd driver start directly
> >>>>> without checking TX/RX queues ready or not, and thus cause
> >>>>> segmentation fault when vhost library accessing queues. This patch
> >>>>> add flag to check whether queues setup or not, and add driver
> >>>>> start call into dev_start to allow user start it after setting up queue.
> >>>> The issue is clear now. But this patch just puts the situation
> >>>> before below
> >> fix:
> >>>> "it doesn't create the actual datagram socket until you call .dev_start()."
> >>> No, if the queue exist, the datagram socket still get created in
> >>> vhost_create
> >> API, since the vhost_driver_register still exist in vhost_create.
> >>
> >> The queue can never be created, as it's still not probed.
> > I think we need to separate this into two cases:
> > 	Statically create vdev, the datagram recreate logical is still there since
> queues are exist already, this patch doesn't change anything.
> > 	Dynamic create vdev, as you pointed out, queue can never be created,
> while this should be not valid since In normal process of creating vdev
> dynamically, we always need to config queues. Correct me if I'm wrong.
> 
> My point is, either vdev is created statically or dynamically, when probe(),
> queues are not setup yet definitely, then *the unix socket will not be created*
> until we set up the queues and do dev_start(). If the unix socket is not created,
> then VM cannot connect to it.

Yes, I agree this. 
In this patch, it just check whether queue is setup or not and give user a chance to setup queue with dev_start, it doesn't revert the logical from previously commit.

So the logical is change to stop creating unix socket before queue setup, what do you think about this? 
> 
> > In summary, I think the previously commit fixes the static code path and this
> patch fixes the dynamic code path (we need to at least setup queue once).

Re: [dpdk-dev] [PATCH] net/vhost: fix segfault when creating vdev dynamically

[Maxime Coquelin]

On 03/27/2018 11:42 AM, Tan, Jianfeng wrote:
> 
> 
> On 3/27/2018 5:24 PM, Chen, Junjie J wrote:
>>>>> On 3/28/2018 12:05 AM, Junjie Chen wrote:
>>>>>> when creating vdev dynamically, vhost pmd driver start directly
>>>>>> without checking TX/RX queues ready or not, and thus cause
>>>>>> segmentation fault when vhost library accessing queues. This patch
>>>>>> add flag to check whether queues setup or not, and add driver start
>>>>>> call into dev_start to allow user start it after setting up queue.
>>>>> The issue is clear now. But this patch just puts the situation 
>>>>> before below
>>> fix:
>>>>> "it doesn't create the actual datagram socket until you call 
>>>>> .dev_start()."
>>>> No, if the queue exist, the datagram socket still get created in 
>>>> vhost_create
>>> API, since the vhost_driver_register still exist in vhost_create.
>>>
>>> The queue can never be created, as it's still not probed.
>> I think we need to separate this into two cases:
>>     Statically create vdev, the datagram recreate logical is still 
>> there since queues are exist already, this patch doesn't change anything.
>>     Dynamic create vdev, as you pointed out, queue can never be 
>> created, while this should be not valid since In normal process of 
>> creating vdev dynamically, we always need to config queues. Correct me 
>> if I'm wrong.
> 
> My point is, either vdev is created statically or dynamically, when 
> probe(), queues are not setup yet definitely, then *the unix socket will 
> not be created* until we set up the queues and do dev_start(). If the 
> unix socket is not created, then VM cannot connect to it.

FYI, I think I reproduced such an issue with the vdev statically created
in the past, while doing some experiments. I didn't went further into 
the analysis at that time, but it looks like the issue Junjie is trying
to address with this patch for dynamically created vdev.

Cheers,
Maxime
>> In summary, I think the previously commit fixes the static code path 
>> and this patch fixes the dynamic code path (we need to at least setup 
>> queue once).
> 

Re: [dpdk-dev] [PATCH] net/vhost: fix segfault when creating vdev dynamically

[Tan, Jianfeng]

On 3/27/2018 6:18 PM, Chen, Junjie J wrote:
>> On 3/27/2018 5:24 PM, Chen, Junjie J wrote:
>>>>>> On 3/28/2018 12:05 AM, Junjie Chen wrote:
>>>>>>> when creating vdev dynamically, vhost pmd driver start directly
>>>>>>> without checking TX/RX queues ready or not, and thus cause
>>>>>>> segmentation fault when vhost library accessing queues. This patch
>>>>>>> add flag to check whether queues setup or not, and add driver
>>>>>>> start call into dev_start to allow user start it after setting up queue.
>>>>>> The issue is clear now. But this patch just puts the situation
>>>>>> before below
>>>> fix:
>>>>>> "it doesn't create the actual datagram socket until you call .dev_start()."
>>>>> No, if the queue exist, the datagram socket still get created in
>>>>> vhost_create
>>>> API, since the vhost_driver_register still exist in vhost_create.
>>>>
>>>> The queue can never be created, as it's still not probed.
>>> I think we need to separate this into two cases:
>>> 	Statically create vdev, the datagram recreate logical is still there since
>> queues are exist already, this patch doesn't change anything.
>>> 	Dynamic create vdev, as you pointed out, queue can never be created,
>> while this should be not valid since In normal process of creating vdev
>> dynamically, we always need to config queues. Correct me if I'm wrong.
>>
>> My point is, either vdev is created statically or dynamically, when probe(),
>> queues are not setup yet definitely, then *the unix socket will not be created*
>> until we set up the queues and do dev_start(). If the unix socket is not created,
>> then VM cannot connect to it.
> Yes, I agree this.
> In this patch, it just check whether queue is setup or not and give user a chance to setup queue with dev_start, it doesn't revert the logical from previously commit.
>
> So the logical is change to stop creating unix socket before queue setup, what do you think about this?

As you said, we partially revert this back, of delaying the unix socket 
creation to queue setup, which can be observed by users.

So what I'm suggesting is: we still keep unix socket creation at probe. 
But in the new_device(), check if queues are setup or not: if yes, we 
just do the queue setting (vid, internal, port); if not, we will delay 
the queue setting until dev_start().

Thanks,
Jianfeng

Re: [dpdk-dev] [PATCH] net/vhost: fix segfault when creating vdev dynamically

[Tan, Jianfeng]

On 3/27/2018 7:28 PM, Maxime Coquelin wrote:
>
>
> On 03/27/2018 11:42 AM, Tan, Jianfeng wrote:
>>
>>
>> On 3/27/2018 5:24 PM, Chen, Junjie J wrote:
>>>>>> On 3/28/2018 12:05 AM, Junjie Chen wrote:
>>>>>>> when creating vdev dynamically, vhost pmd driver start directly
>>>>>>> without checking TX/RX queues ready or not, and thus cause
>>>>>>> segmentation fault when vhost library accessing queues. This patch
>>>>>>> add flag to check whether queues setup or not, and add driver start
>>>>>>> call into dev_start to allow user start it after setting up queue.
>>>>>> The issue is clear now. But this patch just puts the situation 
>>>>>> before below
>>>> fix:
>>>>>> "it doesn't create the actual datagram socket until you call 
>>>>>> .dev_start()."
>>>>> No, if the queue exist, the datagram socket still get created in 
>>>>> vhost_create
>>>> API, since the vhost_driver_register still exist in vhost_create.
>>>>
>>>> The queue can never be created, as it's still not probed.
>>> I think we need to separate this into two cases:
>>>     Statically create vdev, the datagram recreate logical is still 
>>> there since queues are exist already, this patch doesn't change 
>>> anything.
>>>     Dynamic create vdev, as you pointed out, queue can never be 
>>> created, while this should be not valid since In normal process of 
>>> creating vdev dynamically, we always need to config queues. Correct 
>>> me if I'm wrong.
>>
>> My point is, either vdev is created statically or dynamically, when 
>> probe(), queues are not setup yet definitely, then *the unix socket 
>> will not be created* until we set up the queues and do dev_start(). 
>> If the unix socket is not created, then VM cannot connect to it.
>
> FYI, I think I reproduced such an issue with the vdev statically created
> in the past, while doing some experiments. I didn't went further into 
> the analysis at that time, but it looks like the issue Junjie is trying
> to address with this patch for dynamically created vdev.

Yes, I have noticed that this issue mostly happens at dynamic case. Just 
try to suggest a proper way to fix. Please check if my suggestion in 
another email makes sense.

Thanks,
Jianfeng

>
> Cheers,
> Maxime

[dpdk-dev] [PATCH] net/vhost: fix segfault when creating vdev dynamically

[Junjie Chen]

when creating vdev dynamically, vhost pmd driver start directly without
checking TX/RX queues ready or not, and thus cause segmentation fault when
vhost library accessing queues. This patch add flag to check whether queues
setup or not, and add driver start call into dev_start to allow user start
it after setting up queue.

Fixes: aed0b12930b33("net/vhost: fix socket file deleted on stop")
Signed-off-by: Junjie Chen <junjie.j.chen@intel.com>
---
 drivers/net/vhost/rte_eth_vhost.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/net/vhost/rte_eth_vhost.c b/drivers/net/vhost/rte_eth_vhost.c
index 3aae01c..719a150 100644
--- a/drivers/net/vhost/rte_eth_vhost.c
+++ b/drivers/net/vhost/rte_eth_vhost.c
@@ -118,6 +118,7 @@ struct pmd_internal {
 	char *iface_name;
 	uint16_t max_queues;
 	rte_atomic32_t started;
+	rte_atomic32_t once;
 };
 
 struct internal_list {
@@ -772,12 +773,24 @@ rte_eth_vhost_get_vid_from_port_id(uint16_t port_id)
 static int
 eth_dev_start(struct rte_eth_dev *dev)
 {
+	int ret = 0;
 	struct pmd_internal *internal = dev->data->dev_private;
 
+	if (unlikely(rte_atomic32_read(&internal->once) == 0)) {
+		ret = rte_vhost_driver_start(internal->iface_name);
+		if (ret < 0) {
+			RTE_LOG(ERR, PMD, "Failed to start driver for %s\n",
+				internal->iface_name);
+			return ret;
+		}
+
+		rte_atomic32_set(&internal->once, 1);
+	}
+
 	rte_atomic32_set(&internal->started, 1);
 	update_queuing_status(dev);
 
-	return 0;
+	return ret;
 }
 
 static void
@@ -1101,7 +1114,11 @@ eth_dev_vhost_create(struct rte_vdev_device *dev, char *iface_name,
 		goto error;
 	}
 
-	if (rte_vhost_driver_start(iface_name) < 0) {
+	if (!data->rx_queues || !data->tx_queues) {
+		RTE_LOG(INFO, PMD,
+			"TX/RX queue is not ready, driver will not start\n");
+		rte_atomic32_set(&internal->once, 0);
+	} else if (rte_vhost_driver_start(iface_name) < 0) {
 		RTE_LOG(ERR, PMD, "Failed to start driver for %s\n",
 			iface_name);
 		goto error;
-- 
2.7.4

Re: [dpdk-dev] [PATCH] net/vhost: fix segfault when creating vdev dynamically

[Maxime Coquelin]

On 03/27/2018 04:01 PM, Tan, Jianfeng wrote:
> 
> 
> On 3/27/2018 7:28 PM, Maxime Coquelin wrote:
>>
>>
>> On 03/27/2018 11:42 AM, Tan, Jianfeng wrote:
>>>
>>>
>>> On 3/27/2018 5:24 PM, Chen, Junjie J wrote:
>>>>>>> On 3/28/2018 12:05 AM, Junjie Chen wrote:
>>>>>>>> when creating vdev dynamically, vhost pmd driver start directly
>>>>>>>> without checking TX/RX queues ready or not, and thus cause
>>>>>>>> segmentation fault when vhost library accessing queues. This patch
>>>>>>>> add flag to check whether queues setup or not, and add driver start
>>>>>>>> call into dev_start to allow user start it after setting up queue.
>>>>>>> The issue is clear now. But this patch just puts the situation 
>>>>>>> before below
>>>>> fix:
>>>>>>> "it doesn't create the actual datagram socket until you call 
>>>>>>> .dev_start()."
>>>>>> No, if the queue exist, the datagram socket still get created in 
>>>>>> vhost_create
>>>>> API, since the vhost_driver_register still exist in vhost_create.
>>>>>
>>>>> The queue can never be created, as it's still not probed.
>>>> I think we need to separate this into two cases:
>>>>     Statically create vdev, the datagram recreate logical is still 
>>>> there since queues are exist already, this patch doesn't change 
>>>> anything.
>>>>     Dynamic create vdev, as you pointed out, queue can never be 
>>>> created, while this should be not valid since In normal process of 
>>>> creating vdev dynamically, we always need to config queues. Correct 
>>>> me if I'm wrong.
>>>
>>> My point is, either vdev is created statically or dynamically, when 
>>> probe(), queues are not setup yet definitely, then *the unix socket 
>>> will not be created* until we set up the queues and do dev_start(). 
>>> If the unix socket is not created, then VM cannot connect to it.
>>
>> FYI, I think I reproduced such an issue with the vdev statically created
>> in the past, while doing some experiments. I didn't went further into 
>> the analysis at that time, but it looks like the issue Junjie is trying
>> to address with this patch for dynamically created vdev.
> 
> Yes, I have noticed that this issue mostly happens at dynamic case. Just 
> try to suggest a proper way to fix. Please check if my suggestion in 
> another email makes sense.

Yes, it makes sense, that's the right thing to do I think.

Thanks,
Maxime
> 
> Thanks,
> Jianfeng
> 
>>
>> Cheers,
>> Maxime
> 

Re: [dpdk-dev] [PATCH v2] net/vhost: fix segfault when creating vdev dynamically

[Maxime Coquelin]

Hi Junjie,

Thanks for the patch, it looks like the right thing to do.
Please find my comments below:

On 03/29/2018 05:35 PM, Junjie Chen wrote:
> when creating vdev dynamically, vhost pmd driver start directly without
s/when/When/

s/start/starts/
> checking TX/RX queues ready or not, and thus cause segmentation fault when
s/ready/are ready/
s/cause/causes/
> vhost library accessing queues. This patch add flag to check whether queues
s/accessing/accesses/
s/add flag/adds a flag/
> setup or not, and add driver start call into dev_start to allow user start
s/setup/are setup/
s/add/adds/
s/start/ to start/
> it after setting up queue.
s/queue/queues/
> 
> Signed-off-by: Junjie Chen <junjie.j.chen@intel.com>
> ---
> Changes in v2:
> - check queue status in new_device, create queue in dev_start if not setup yet
>   drivers/net/vhost/rte_eth_vhost.c | 73 ++++++++++++++++++++++++++++-----------
>   1 file changed, 53 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/net/vhost/rte_eth_vhost.c b/drivers/net/vhost/rte_eth_vhost.c
> index 3aae01c39..41410fa5a 100644
> --- a/drivers/net/vhost/rte_eth_vhost.c
> +++ b/drivers/net/vhost/rte_eth_vhost.c
> @@ -117,7 +117,9 @@ struct pmd_internal {
>   	char *dev_name;
>   	char *iface_name;
>   	uint16_t max_queues;
> +	uint16_t vid;
>   	rte_atomic32_t started;
> +	rte_atomic32_t once;

The name is a bit vague.
Also, I wonder if we need a new variable.
Couldn't we rely on dev_attached? In new_device(), only set
dev_attached=1 if queues are allocated. And in eth_dev_start(),
attach queues if !dev_attached and then set dev_attached to 1.

>   };
>   
>   struct internal_list {
> @@ -580,21 +582,27 @@ new_device(int vid)
>   		eth_dev->data->numa_node = newnode;
>   #endif
>   
> -	for (i = 0; i < eth_dev->data->nb_rx_queues; i++) {
> -		vq = eth_dev->data->rx_queues[i];
> -		if (vq == NULL)
> -			continue;
> -		vq->vid = vid;
> -		vq->internal = internal;
> -		vq->port = eth_dev->data->port_id;
> -	}
> -	for (i = 0; i < eth_dev->data->nb_tx_queues; i++) {
> -		vq = eth_dev->data->tx_queues[i];
> -		if (vq == NULL)
> -			continue;
> -		vq->vid = vid;
> -		vq->internal = internal;
> -		vq->port = eth_dev->data->port_id;
> +	if (eth_dev->data->rx_queues && eth_dev->data->tx_queues) {
> +		for (i = 0; i < eth_dev->data->nb_rx_queues; i++) {
> +			vq = eth_dev->data->rx_queues[i];
> +			if (!vq)
> +				continue;
> +			vq->vid = vid;
> +			vq->internal = internal;
> +			vq->port = eth_dev->data->port_id;
> +		}
> +		for (i = 0; i < eth_dev->data->nb_tx_queues; i++) {
> +			vq = eth_dev->data->tx_queues[i];
> +			if (!vq)
> +				continue;
> +			vq->vid = vid;
> +			vq->internal = internal;
> +			vq->port = eth_dev->data->port_id;
> +		}
> +	} else {
> +		RTE_LOG(INFO, PMD, "RX/TX queues have not setup yet\n");
> +		internal->vid = vid;
> +		rte_atomic32_set(&internal->once, 0);
>   	}
>   
>   	for (i = 0; i < rte_vhost_get_vring_num(vid); i++)
> @@ -605,7 +613,8 @@ new_device(int vid)
>   	eth_dev->data->dev_link.link_status = ETH_LINK_UP;
>   
>   	rte_atomic32_set(&internal->dev_attached, 1);
> -	update_queuing_status(eth_dev);
> +	if (likely(rte_atomic32_read(&internal->once) == 1))
> +		update_queuing_status(eth_dev);
>   
>   	RTE_LOG(INFO, PMD, "Vhost device %d created\n", vid);
>   
> @@ -770,12 +779,34 @@ rte_eth_vhost_get_vid_from_port_id(uint16_t port_id)
>   }
>   
>   static int
> -eth_dev_start(struct rte_eth_dev *dev)
> +eth_dev_start(struct rte_eth_dev *eth_dev)
>   {
> -	struct pmd_internal *internal = dev->data->dev_private;
> +	struct pmd_internal *internal = eth_dev->data->dev_private;
> +	struct vhost_queue *vq;
> +	int i;
> +
> +	if (unlikely(rte_atomic32_read(&internal->once) == 0)) {
> +		for (i = 0; i < eth_dev->data->nb_rx_queues; i++) {
> +			vq = eth_dev->data->rx_queues[i];
> +			if (!vq)
> +				continue;
> +			vq->vid = internal->vid;
> +			vq->internal = internal;
> +			vq->port = eth_dev->data->port_id;
> +		}
> +		for (i = 0; i < eth_dev->data->nb_tx_queues; i++) {
> +			vq = eth_dev->data->tx_queues[i];
> +			if (!vq)
> +				continue;
> +			vq->vid = internal->vid;
> +			vq->internal = internal;
> +			vq->port = eth_dev->data->port_id;
> +		}

I think you could avoid the code duplication by creating a new function 
to setup queues:

void queue_setup(struct rte_eth_dev *eth_dev,
		 struct pmd_internal *internal)
{
	int i;

	for (i = 0; i < eth_dev->data->nb_rx_queues; i++) {
		vq = eth_dev->data->rx_queues[i];
		if (!vq)
			continue;
		vq->vid = internal->vid;
		vq->internal = internal;
		vq->port = eth_dev->data->port_id;
	}
	for (i = 0; i < eth_dev->data->nb_tx_queues; i++) {
		vq = eth_dev->data->tx_queues[i];
		if (!vq)
			continue;
		vq->vid = internal->vid;
		vq->internal = internal;
		vq->port = eth_dev->data->port_id;
	}
}

> +			rte_atomic32_set(&internal->once, 1);

Indentation looks wrong here.

> +	}
>   
>   	rte_atomic32_set(&internal->started, 1);
> -	update_queuing_status(dev);
> +	update_queuing_status(eth_dev);
>   
>   	return 0;
>   }
> @@ -786,7 +817,9 @@ eth_dev_stop(struct rte_eth_dev *dev)
>   	struct pmd_internal *internal = dev->data->dev_private;
>   
>   	rte_atomic32_set(&internal->started, 0);
> -	update_queuing_status(dev);
> +
> +	if (likely(rte_atomic32_read(&internal->once) == 1))
> +		update_queuing_status(dev);
>   }
>   
>   static void
> 

[dpdk-dev] [PATCH v2] net/vhost: fix segfault when creating vdev dynamically

[Junjie Chen]

when creating vdev dynamically, vhost pmd driver start directly without
checking TX/RX queues ready or not, and thus cause segmentation fault when
vhost library accessing queues. This patch add flag to check whether queues
setup or not, and add driver start call into dev_start to allow user start
it after setting up queue.

Signed-off-by: Junjie Chen <junjie.j.chen@intel.com>
---
Changes in v2:
- check queue status in new_device, create queue in dev_start if not setup yet
 drivers/net/vhost/rte_eth_vhost.c | 73 ++++++++++++++++++++++++++++-----------
 1 file changed, 53 insertions(+), 20 deletions(-)

diff --git a/drivers/net/vhost/rte_eth_vhost.c b/drivers/net/vhost/rte_eth_vhost.c
index 3aae01c39..41410fa5a 100644
--- a/drivers/net/vhost/rte_eth_vhost.c
+++ b/drivers/net/vhost/rte_eth_vhost.c
@@ -117,7 +117,9 @@ struct pmd_internal {
 	char *dev_name;
 	char *iface_name;
 	uint16_t max_queues;
+	uint16_t vid;
 	rte_atomic32_t started;
+	rte_atomic32_t once;
 };
 
 struct internal_list {
@@ -580,21 +582,27 @@ new_device(int vid)
 		eth_dev->data->numa_node = newnode;
 #endif
 
-	for (i = 0; i < eth_dev->data->nb_rx_queues; i++) {
-		vq = eth_dev->data->rx_queues[i];
-		if (vq == NULL)
-			continue;
-		vq->vid = vid;
-		vq->internal = internal;
-		vq->port = eth_dev->data->port_id;
-	}
-	for (i = 0; i < eth_dev->data->nb_tx_queues; i++) {
-		vq = eth_dev->data->tx_queues[i];
-		if (vq == NULL)
-			continue;
-		vq->vid = vid;
-		vq->internal = internal;
-		vq->port = eth_dev->data->port_id;
+	if (eth_dev->data->rx_queues && eth_dev->data->tx_queues) {
+		for (i = 0; i < eth_dev->data->nb_rx_queues; i++) {
+			vq = eth_dev->data->rx_queues[i];
+			if (!vq)
+				continue;
+			vq->vid = vid;
+			vq->internal = internal;
+			vq->port = eth_dev->data->port_id;
+		}
+		for (i = 0; i < eth_dev->data->nb_tx_queues; i++) {
+			vq = eth_dev->data->tx_queues[i];
+			if (!vq)
+				continue;
+			vq->vid = vid;
+			vq->internal = internal;
+			vq->port = eth_dev->data->port_id;
+		}
+	} else {
+		RTE_LOG(INFO, PMD, "RX/TX queues have not setup yet\n");
+		internal->vid = vid;
+		rte_atomic32_set(&internal->once, 0);
 	}
 
 	for (i = 0; i < rte_vhost_get_vring_num(vid); i++)
@@ -605,7 +613,8 @@ new_device(int vid)
 	eth_dev->data->dev_link.link_status = ETH_LINK_UP;
 
 	rte_atomic32_set(&internal->dev_attached, 1);
-	update_queuing_status(eth_dev);
+	if (likely(rte_atomic32_read(&internal->once) == 1))
+		update_queuing_status(eth_dev);
 
 	RTE_LOG(INFO, PMD, "Vhost device %d created\n", vid);
 
@@ -770,12 +779,34 @@ rte_eth_vhost_get_vid_from_port_id(uint16_t port_id)
 }
 
 static int
-eth_dev_start(struct rte_eth_dev *dev)
+eth_dev_start(struct rte_eth_dev *eth_dev)
 {
-	struct pmd_internal *internal = dev->data->dev_private;
+	struct pmd_internal *internal = eth_dev->data->dev_private;
+	struct vhost_queue *vq;
+	int i;
+
+	if (unlikely(rte_atomic32_read(&internal->once) == 0)) {
+		for (i = 0; i < eth_dev->data->nb_rx_queues; i++) {
+			vq = eth_dev->data->rx_queues[i];
+			if (!vq)
+				continue;
+			vq->vid = internal->vid;
+			vq->internal = internal;
+			vq->port = eth_dev->data->port_id;
+		}
+		for (i = 0; i < eth_dev->data->nb_tx_queues; i++) {
+			vq = eth_dev->data->tx_queues[i];
+			if (!vq)
+				continue;
+			vq->vid = internal->vid;
+			vq->internal = internal;
+			vq->port = eth_dev->data->port_id;
+		}
+			rte_atomic32_set(&internal->once, 1);
+	}
 
 	rte_atomic32_set(&internal->started, 1);
-	update_queuing_status(dev);
+	update_queuing_status(eth_dev);
 
 	return 0;
 }
@@ -786,7 +817,9 @@ eth_dev_stop(struct rte_eth_dev *dev)
 	struct pmd_internal *internal = dev->data->dev_private;
 
 	rte_atomic32_set(&internal->started, 0);
-	update_queuing_status(dev);
+
+	if (likely(rte_atomic32_read(&internal->once) == 1))
+		update_queuing_status(dev);
 }
 
 static void
-- 
2.16.0

[dpdk-dev] [PATCH v3] net/vhost: fix segfault when creating vdev dynamically

[Junjie Chen]

When creating vdev dynamically, vhost pmd driver starts directly without
checking TX/RX queues are ready or not, and thus causes segmentation fault
when vhost library accesses queues. This patch adds a flag to check whether
queues are setup or not, and adds queues setup into dev_start function to
allow user to start them after setting up.

Fixes: aed0b12930b3 ("net/vhost: fix socket file deleted on stop")
Signed-off-by: Chen, Junjie <junjie.j.chen@intel.com>
---
Changes in v3:
- Update commit log, refine duplicated code
Changes in v2:
- Check queues status in new_device, create queue in dev_start if not setup yet

 drivers/net/vhost/rte_eth_vhost.c | 69 ++++++++++++++++++++++++++-------------
 1 file changed, 46 insertions(+), 23 deletions(-)

diff --git a/drivers/net/vhost/rte_eth_vhost.c b/drivers/net/vhost/rte_eth_vhost.c
index 3aae01c39..11b607650 100644
--- a/drivers/net/vhost/rte_eth_vhost.c
+++ b/drivers/net/vhost/rte_eth_vhost.c
@@ -117,6 +117,7 @@ struct pmd_internal {
 	char *dev_name;
 	char *iface_name;
 	uint16_t max_queues;
+	uint16_t vid;
 	rte_atomic32_t started;
 };
 
@@ -527,8 +528,10 @@ update_queuing_status(struct rte_eth_dev *dev)
 	unsigned int i;
 	int allow_queuing = 1;
 
-	if (rte_atomic32_read(&internal->started) == 0 ||
-	    rte_atomic32_read(&internal->dev_attached) == 0)
+	if (rte_atomic32_read(&internal->dev_attached) == 0)
+		return;
+
+	if (rte_atomic32_read(&internal->started) == 0)
 		allow_queuing = 0;
 
 	/* Wait until rx/tx_pkt_burst stops accessing vhost device */
@@ -551,13 +554,36 @@ update_queuing_status(struct rte_eth_dev *dev)
 	}
 }
 
+static void
+queue_setup(struct rte_eth_dev *eth_dev, struct pmd_internal *internal)
+{
+	struct vhost_queue *vq;
+	int i;
+
+	for (i = 0; i < eth_dev->data->nb_rx_queues; i++) {
+		vq = eth_dev->data->rx_queues[i];
+		if (!vq)
+			continue;
+		vq->vid = internal->vid;
+		vq->internal = internal;
+		vq->port = eth_dev->data->port_id;
+	}
+	for (i = 0; i < eth_dev->data->nb_tx_queues; i++) {
+		vq = eth_dev->data->tx_queues[i];
+		if (!vq)
+			continue;
+		vq->vid = internal->vid;
+		vq->internal = internal;
+		vq->port = eth_dev->data->port_id;
+	}
+}
+
 static int
 new_device(int vid)
 {
 	struct rte_eth_dev *eth_dev;
 	struct internal_list *list;
 	struct pmd_internal *internal;
-	struct vhost_queue *vq;
 	unsigned i;
 	char ifname[PATH_MAX];
 #ifdef RTE_LIBRTE_VHOST_NUMA
@@ -580,21 +606,13 @@ new_device(int vid)
 		eth_dev->data->numa_node = newnode;
 #endif
 
-	for (i = 0; i < eth_dev->data->nb_rx_queues; i++) {
-		vq = eth_dev->data->rx_queues[i];
-		if (vq == NULL)
-			continue;
-		vq->vid = vid;
-		vq->internal = internal;
-		vq->port = eth_dev->data->port_id;
-	}
-	for (i = 0; i < eth_dev->data->nb_tx_queues; i++) {
-		vq = eth_dev->data->tx_queues[i];
-		if (vq == NULL)
-			continue;
-		vq->vid = vid;
-		vq->internal = internal;
-		vq->port = eth_dev->data->port_id;
+	internal->vid = vid;
+	if (eth_dev->data->rx_queues && eth_dev->data->tx_queues) {
+		queue_setup(eth_dev, internal);
+		rte_atomic32_set(&internal->dev_attached, 1);
+	} else {
+		RTE_LOG(INFO, PMD, "RX/TX queues have not setup yet\n");
+		rte_atomic32_set(&internal->dev_attached, 0);
 	}
 
 	for (i = 0; i < rte_vhost_get_vring_num(vid); i++)
@@ -604,7 +622,6 @@ new_device(int vid)
 
 	eth_dev->data->dev_link.link_status = ETH_LINK_UP;
 
-	rte_atomic32_set(&internal->dev_attached, 1);
 	update_queuing_status(eth_dev);
 
 	RTE_LOG(INFO, PMD, "Vhost device %d created\n", vid);
@@ -634,8 +651,9 @@ destroy_device(int vid)
 	eth_dev = list->eth_dev;
 	internal = eth_dev->data->dev_private;
 
-	rte_atomic32_set(&internal->dev_attached, 0);
+	rte_atomic32_set(&internal->started, 0);
 	update_queuing_status(eth_dev);
+	rte_atomic32_set(&internal->dev_attached, 0);
 
 	eth_dev->data->dev_link.link_status = ETH_LINK_DOWN;
 
@@ -770,12 +788,17 @@ rte_eth_vhost_get_vid_from_port_id(uint16_t port_id)
 }
 
 static int
-eth_dev_start(struct rte_eth_dev *dev)
+eth_dev_start(struct rte_eth_dev *eth_dev)
 {
-	struct pmd_internal *internal = dev->data->dev_private;
+	struct pmd_internal *internal = eth_dev->data->dev_private;
+
+	if (unlikely(rte_atomic32_read(&internal->dev_attached) == 0)) {
+		queue_setup(eth_dev, internal);
+		rte_atomic32_set(&internal->dev_attached, 1);
+	}
 
 	rte_atomic32_set(&internal->started, 1);
-	update_queuing_status(dev);
+	update_queuing_status(eth_dev);
 
 	return 0;
 }
-- 
2.16.0

Re: [dpdk-dev] [PATCH v3] net/vhost: fix segfault when creating vdev dynamically

[Yang, Zhiyong]

Hi Maxime, Junjie,

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Junjie Chen
> Sent: Friday, March 30, 2018 2:59 PM
> To: Tan, Jianfeng <jianfeng.tan@intel.com>; maxime.coquelin@redhat.com;
> mtetsuyah@gmail.com
> Cc: dev@dpdk.org; Chen, Junjie J <junjie.j.chen@intel.com>;
> Chen@dpdk.org
> Subject: [dpdk-dev] [PATCH v3] net/vhost: fix segfault when creating vdev
> dynamically
> 
> When creating vdev dynamically, vhost pmd driver starts directly without
> checking TX/RX queues are ready or not, and thus causes segmentation fault
> when vhost library accesses queues. This patch adds a flag to check whether
> queues are setup or not, and adds queues setup into dev_start function to
> allow user to start them after setting up.
> 
> Fixes: aed0b12930b3 ("net/vhost: fix socket file deleted on stop")
> Signed-off-by: Chen, Junjie <junjie.j.chen@intel.com>
> ---

Thanks for Junjie's patch!

I also came across the similar issue when developing virtio-user server mode.
>From user's perspective, the patch can fix the issue in my user case instead of 
the patch http://www.dpdk.org/dev/patchwork/patch/36340/

Tested-by: Zhiyong Yang <zhiyong.yang@intel.com>

Thanks
Zhiyong

Re: [dpdk-dev] [PATCH v3] net/vhost: fix segfault when creating vdev dynamically

[Maxime Coquelin]

Hi Junjie,

On 03/30/2018 08:58 AM, Junjie Chen wrote:
> When creating vdev dynamically, vhost pmd driver starts directly without
> checking TX/RX queues are ready or not, and thus causes segmentation fault
> when vhost library accesses queues. This patch adds a flag to check whether
> queues are setup or not, and adds queues setup into dev_start function to
> allow user to start them after setting up.
> 
> Fixes: aed0b12930b3 ("net/vhost: fix socket file deleted on stop")
> Signed-off-by: Chen, Junjie <junjie.j.chen@intel.com>
> ---
> Changes in v3:
> - Update commit log, refine duplicated code
> Changes in v2:
> - Check queues status in new_device, create queue in dev_start if not setup yet
> 
>   drivers/net/vhost/rte_eth_vhost.c | 69 ++++++++++++++++++++++++++-------------
>   1 file changed, 46 insertions(+), 23 deletions(-)

Nice patch!
Thanks for having handled the changes that quickly.

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Maxime

Re: [dpdk-dev] [PATCH v3] net/vhost: fix segfault when creating vdev dynamically

[Maxime Coquelin]

On 03/30/2018 09:32 AM, Yang, Zhiyong wrote:
> Hi Maxime, Junjie,
> 
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Junjie Chen
>> Sent: Friday, March 30, 2018 2:59 PM
>> To: Tan, Jianfeng <jianfeng.tan@intel.com>; maxime.coquelin@redhat.com;
>> mtetsuyah@gmail.com
>> Cc: dev@dpdk.org; Chen, Junjie J <junjie.j.chen@intel.com>;
>> Chen@dpdk.org
>> Subject: [dpdk-dev] [PATCH v3] net/vhost: fix segfault when creating vdev
>> dynamically
>>
>> When creating vdev dynamically, vhost pmd driver starts directly without
>> checking TX/RX queues are ready or not, and thus causes segmentation fault
>> when vhost library accesses queues. This patch adds a flag to check whether
>> queues are setup or not, and adds queues setup into dev_start function to
>> allow user to start them after setting up.
>>
>> Fixes: aed0b12930b3 ("net/vhost: fix socket file deleted on stop")
>> Signed-off-by: Chen, Junjie <junjie.j.chen@intel.com>
>> ---
> 
> Thanks for Junjie's patch!
> 
> I also came across the similar issue when developing virtio-user server mode.
>  From user's perspective, the patch can fix the issue in my user case instead of
> the patch http://www.dpdk.org/dev/patchwork/patch/36340/
> 
> Tested-by: Zhiyong Yang <zhiyong.yang@intel.com>

Great it solves your issue, and thanks for having tested it.

Maxime

> Thanks
> Zhiyong
> 

Re: [dpdk-dev] [PATCH v3] net/vhost: fix segfault when creating vdev dynamically

[Maxime Coquelin]

On 03/30/2018 08:58 AM, Junjie Chen wrote:
> When creating vdev dynamically, vhost pmd driver starts directly without
> checking TX/RX queues are ready or not, and thus causes segmentation fault
> when vhost library accesses queues. This patch adds a flag to check whether
> queues are setup or not, and adds queues setup into dev_start function to
> allow user to start them after setting up.
> 
> Fixes: aed0b12930b3 ("net/vhost: fix socket file deleted on stop")
> Signed-off-by: Chen, Junjie <junjie.j.chen@intel.com>
> ---
> Changes in v3:
> - Update commit log, refine duplicated code
> Changes in v2:
> - Check queues status in new_device, create queue in dev_start if not setup yet
> 
>   drivers/net/vhost/rte_eth_vhost.c | 69 ++++++++++++++++++++++++++-------------
>   1 file changed, 46 insertions(+), 23 deletions(-)

Applied to dpdk-next-virtio/master.

Thanks!
Maxime

Re: [dpdk-dev] [PATCH v3] net/vhost: fix segfault when creating vdev dynamically

[Jens Freimann]

Hi,

On Fri, Mar 30, 2018 at 02:58:31PM +0800, Junjie Chen wrote:
>When creating vdev dynamically, vhost pmd driver starts directly without
>checking TX/RX queues are ready or not, and thus causes segmentation fault
>when vhost library accesses queues. This patch adds a flag to check whether
>queues are setup or not, and adds queues setup into dev_start function to
>allow user to start them after setting up.

for me, with this patch vhost enqueue/dequeue code is never called because 

  if (unlikely(rte_atomic32_read(&r->allow_queuing) == 0))
 
this check in eth_vhost_rx() is always true. 

When I revert this patch it works as usual. 

My testpmd cmdline is: 

gdb --args $RTE_SDK/install/bin/testpmd -l 0,2,3,4,5 --socket-mem=1024 -n 4 \
    --vdev 'net_vhost0,iface=/tmp/vhost-user1' \
    --vdev 'net_vhost1,iface=/tmp/vhost-user2' -- \
    --portmask=f  --rxq=1 --txq=1 \
    --nb-cores=4 --forward-mode=io  -i

After starting testpmd I issue commands "set portlist 0,2,1,3", start
my guest and start another testpmd issue in the guest. 

Another problem I see: Before this patch I could start testpmd, issue
the portlist command and type "start". If I do this now I get an infinite
loop of "VHOST CONFIG device not found" messages.


regards,
Jens 

Re: [dpdk-dev] [PATCH v3] net/vhost: fix segfault when creating vdev dynamically

[Chen, Junjie J]

Thanks for report, I just summited this patch to fix:
https://dpdk.org/dev/patchwork/patch/37765/ 

> 
> Hi,
> 
> On Fri, Mar 30, 2018 at 02:58:31PM +0800, Junjie Chen wrote:
> >When creating vdev dynamically, vhost pmd driver starts directly
> >without checking TX/RX queues are ready or not, and thus causes
> >segmentation fault when vhost library accesses queues. This patch adds
> >a flag to check whether queues are setup or not, and adds queues setup
> >into dev_start function to allow user to start them after setting up.
> 
> for me, with this patch vhost enqueue/dequeue code is never called because
> 
>   if (unlikely(rte_atomic32_read(&r->allow_queuing) == 0))
> 
> this check in eth_vhost_rx() is always true.
> 
> When I revert this patch it works as usual.
> 
> My testpmd cmdline is:
> 
> gdb --args $RTE_SDK/install/bin/testpmd -l 0,2,3,4,5 --socket-mem=1024 -n 4 \
>     --vdev 'net_vhost0,iface=/tmp/vhost-user1' \
>     --vdev 'net_vhost1,iface=/tmp/vhost-user2' -- \
>     --portmask=f  --rxq=1 --txq=1 \
>     --nb-cores=4 --forward-mode=io  -i
> 
> After starting testpmd I issue commands "set portlist 0,2,1,3", start my guest
> and start another testpmd issue in the guest.
> 
> Another problem I see: Before this patch I could start testpmd, issue the
> portlist command and type "start". If I do this now I get an infinite loop of
> "VHOST CONFIG device not found" messages.
> 
> 
> regards,
> Jens