[dpdk-dev] [PATCH] vhost: Fix default value of kickfd and callfd

[dpdk-dev] [PATCH] vhost: Fix default value of kickfd and callfd

[Tetsuya Mukawa]

Currently, default value of kickfd and callfd is -1.
If the value is -1, current code guess kickfd and callfd hasn't been
initialized yet. And vhost library will guess the virtqueue isn't ready
for processing.
But callfd and kickfd will be set as -1 when "--enable-kvm"
isn't specified in QEMU command line. It means we cannot treat -1 as
uninitialized state. The patch changes default value to -2. And the
patch defines -2 as VIRTIO_UNINITIALIZED_EVENTFD.

Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
---
 lib/librte_vhost/rte_virtio_net.h             | 1 +
 lib/librte_vhost/vhost_user/virtio-net-user.c | 7 ++++---
 lib/librte_vhost/virtio-net.c                 | 4 ++--
 3 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/lib/librte_vhost/rte_virtio_net.h b/lib/librte_vhost/rte_virtio_net.h
index 7d1fde2..d745707 100644
--- a/lib/librte_vhost/rte_virtio_net.h
+++ b/lib/librte_vhost/rte_virtio_net.h
@@ -89,6 +89,7 @@ struct vhost_virtqueue {
 	uint16_t		vhost_hlen;		/**< Vhost header length (varies depending on RX merge buffers. */
 	volatile uint16_t	last_used_idx;		/**< Last index used on the available ring */
 	volatile uint16_t	last_used_idx_res;	/**< Used for multiple devices reserving buffers. */
+#define VIRTIO_UNINITIALIZED_EVENTFD	(-2)
 	int			callfd;			/**< Used to notify the guest (trigger interrupt). */
 	int			kickfd;			/**< Currently unused as polling mode is enabled. */
 	int			enabled;
diff --git a/lib/librte_vhost/vhost_user/virtio-net-user.c b/lib/librte_vhost/vhost_user/virtio-net-user.c
index 65b5652..591f9bf 100644
--- a/lib/librte_vhost/vhost_user/virtio-net-user.c
+++ b/lib/librte_vhost/vhost_user/virtio-net-user.c
@@ -225,8 +225,8 @@ static int
 vq_is_ready(struct vhost_virtqueue *vq)
 {
 	return vq && vq->desc   &&
-	       vq->kickfd != -1 &&
-	       vq->callfd != -1;
+	       vq->kickfd != VIRTIO_UNINITIALIZED_EVENTFD &&
+	       vq->callfd != VIRTIO_UNINITIALIZED_EVENTFD;
 }
 
 static int
@@ -318,7 +318,8 @@ user_get_vring_base(struct vhost_device_ctx ctx,
 	 */
 	if (dev->virtqueue[state->index]->kickfd >= 0) {
 		close(dev->virtqueue[state->index]->kickfd);
-		dev->virtqueue[state->index]->kickfd = -1;
+		dev->virtqueue[state->index]->kickfd =
+					VIRTIO_UNINITIALIZED_EVENTFD;
 	}
 
 	return 0;
diff --git a/lib/librte_vhost/virtio-net.c b/lib/librte_vhost/virtio-net.c
index fe1a77e..3498e9d 100644
--- a/lib/librte_vhost/virtio-net.c
+++ b/lib/librte_vhost/virtio-net.c
@@ -263,8 +263,8 @@ init_vring_queue(struct vhost_virtqueue *vq, int qp_idx)
 {
 	memset(vq, 0, sizeof(struct vhost_virtqueue));
 
-	vq->kickfd = -1;
-	vq->callfd = -1;
+	vq->kickfd = VIRTIO_UNINITIALIZED_EVENTFD;
+	vq->callfd = VIRTIO_UNINITIALIZED_EVENTFD;
 
 	/* Backends are set to -1 indicating an inactive device. */
 	vq->backend = -1;
-- 
2.1.4

Re: [dpdk-dev] [PATCH] vhost: Fix default value of kickfd and callfd

[Tan, Jianfeng]

On 3/10/2016 2:14 PM, Tetsuya Mukawa wrote:
> Currently, default value of kickfd and callfd is -1.
> If the value is -1, current code guess kickfd and callfd hasn't been
> initialized yet. And vhost library will guess the virtqueue isn't ready
> for processing.
> But callfd and kickfd will be set as -1 when "--enable-kvm"
> isn't specified in QEMU command line. It means we cannot treat -1 as
> uninitialized state. The patch changes default value to -2. And the
> patch defines -2 as VIRTIO_UNINITIALIZED_EVENTFD.
>
> Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
> ---
>   lib/librte_vhost/rte_virtio_net.h             | 1 +
>   lib/librte_vhost/vhost_user/virtio-net-user.c | 7 ++++---
>   lib/librte_vhost/virtio-net.c                 | 4 ++--
>   3 files changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/lib/librte_vhost/rte_virtio_net.h b/lib/librte_vhost/rte_virtio_net.h
> index 7d1fde2..d745707 100644
> --- a/lib/librte_vhost/rte_virtio_net.h
> +++ b/lib/librte_vhost/rte_virtio_net.h
> @@ -89,6 +89,7 @@ struct vhost_virtqueue {
>   	uint16_t		vhost_hlen;		/**< Vhost header length (varies depending on RX merge buffers. */
>   	volatile uint16_t	last_used_idx;		/**< Last index used on the available ring */
>   	volatile uint16_t	last_used_idx_res;	/**< Used for multiple devices reserving buffers. */
> +#define VIRTIO_UNINITIALIZED_EVENTFD	(-2)
>   	int			callfd;			/**< Used to notify the guest (trigger interrupt). */
>   	int			kickfd;			/**< Currently unused as polling mode is enabled. */
>   	int			enabled;
> diff --git a/lib/librte_vhost/vhost_user/virtio-net-user.c b/lib/librte_vhost/vhost_user/virtio-net-user.c
> index 65b5652..591f9bf 100644
> --- a/lib/librte_vhost/vhost_user/virtio-net-user.c
> +++ b/lib/librte_vhost/vhost_user/virtio-net-user.c
> @@ -225,8 +225,8 @@ static int
>   vq_is_ready(struct vhost_virtqueue *vq)
>   {
>   	return vq && vq->desc   &&
> -	       vq->kickfd != -1 &&
> -	       vq->callfd != -1;
> +	       vq->kickfd != VIRTIO_UNINITIALIZED_EVENTFD &&
> +	       vq->callfd != VIRTIO_UNINITIALIZED_EVENTFD;
>   }
>   
>   static int
> @@ -318,7 +318,8 @@ user_get_vring_base(struct vhost_device_ctx ctx,
>   	 */
>   	if (dev->virtqueue[state->index]->kickfd >= 0) {
>   		close(dev->virtqueue[state->index]->kickfd);

Then -1 will be a valid value, need to check if it's not -1 to close()? 
Nevertheless, close(-1) brings no big problem.

Thanks,
Jianfeng

> -		dev->virtqueue[state->index]->kickfd = -1;
> +		dev->virtqueue[state->index]->kickfd =
> +					VIRTIO_UNINITIALIZED_EVENTFD;
>   	}
>   
>   	return 0;
> diff --git a/lib/librte_vhost/virtio-net.c b/lib/librte_vhost/virtio-net.c
> index fe1a77e..3498e9d 100644
> --- a/lib/librte_vhost/virtio-net.c
> +++ b/lib/librte_vhost/virtio-net.c
> @@ -263,8 +263,8 @@ init_vring_queue(struct vhost_virtqueue *vq, int qp_idx)
>   {
>   	memset(vq, 0, sizeof(struct vhost_virtqueue));
>   
> -	vq->kickfd = -1;
> -	vq->callfd = -1;
> +	vq->kickfd = VIRTIO_UNINITIALIZED_EVENTFD;
> +	vq->callfd = VIRTIO_UNINITIALIZED_EVENTFD;
>   
>   	/* Backends are set to -1 indicating an inactive device. */
>   	vq->backend = -1;

Re: [dpdk-dev] [PATCH] vhost: Fix default value of kickfd and callfd

[Tetsuya Mukawa]

On 2016/03/10 15:25, Tan, Jianfeng wrote:
>
>
> On 3/10/2016 2:14 PM, Tetsuya Mukawa wrote:
>> Currently, default value of kickfd and callfd is -1.
>> If the value is -1, current code guess kickfd and callfd hasn't been
>> initialized yet. And vhost library will guess the virtqueue isn't ready
>> for processing.
>> But callfd and kickfd will be set as -1 when "--enable-kvm"
>> isn't specified in QEMU command line. It means we cannot treat -1 as
>> uninitialized state. The patch changes default value to -2. And the
>> patch defines -2 as VIRTIO_UNINITIALIZED_EVENTFD.
>>
>> Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
>> ---
>>   lib/librte_vhost/rte_virtio_net.h             | 1 +
>>   lib/librte_vhost/vhost_user/virtio-net-user.c | 7 ++++---
>>   lib/librte_vhost/virtio-net.c                 | 4 ++--
>>   3 files changed, 7 insertions(+), 5 deletions(-)
>>
>> diff --git a/lib/librte_vhost/rte_virtio_net.h
>> b/lib/librte_vhost/rte_virtio_net.h
>> index 7d1fde2..d745707 100644
>> --- a/lib/librte_vhost/rte_virtio_net.h
>> +++ b/lib/librte_vhost/rte_virtio_net.h
>> @@ -89,6 +89,7 @@ struct vhost_virtqueue {
>>       uint16_t        vhost_hlen;        /**< Vhost header length
>> (varies depending on RX merge buffers. */
>>       volatile uint16_t    last_used_idx;        /**< Last index used
>> on the available ring */
>>       volatile uint16_t    last_used_idx_res;    /**< Used for
>> multiple devices reserving buffers. */
>> +#define VIRTIO_UNINITIALIZED_EVENTFD    (-2)
>>       int            callfd;            /**< Used to notify the guest
>> (trigger interrupt). */
>>       int            kickfd;            /**< Currently unused as
>> polling mode is enabled. */
>>       int            enabled;
>> diff --git a/lib/librte_vhost/vhost_user/virtio-net-user.c
>> b/lib/librte_vhost/vhost_user/virtio-net-user.c
>> index 65b5652..591f9bf 100644
>> --- a/lib/librte_vhost/vhost_user/virtio-net-user.c
>> +++ b/lib/librte_vhost/vhost_user/virtio-net-user.c
>> @@ -225,8 +225,8 @@ static int
>>   vq_is_ready(struct vhost_virtqueue *vq)
>>   {
>>       return vq && vq->desc   &&
>> -           vq->kickfd != -1 &&
>> -           vq->callfd != -1;
>> +           vq->kickfd != VIRTIO_UNINITIALIZED_EVENTFD &&
>> +           vq->callfd != VIRTIO_UNINITIALIZED_EVENTFD;
>>   }
>>     static int
>> @@ -318,7 +318,8 @@ user_get_vring_base(struct vhost_device_ctx ctx,
>>        */
>>       if (dev->virtqueue[state->index]->kickfd >= 0) {
>>           close(dev->virtqueue[state->index]->kickfd);
>
> Then -1 will be a valid value, need to check if it's not -1 to
> close()? Nevertheless, close(-1) brings no big problem.
>

We did it in above 'if' condition checking, then close(-1) will not be
called.

Thanks,
Tetsuya

> Thanks,
> Jianfeng
>
>> -        dev->virtqueue[state->index]->kickfd = -1;
>> +        dev->virtqueue[state->index]->kickfd =
>> +                    VIRTIO_UNINITIALIZED_EVENTFD;
>>       }
>>         return 0;
>> diff --git a/lib/librte_vhost/virtio-net.c
>> b/lib/librte_vhost/virtio-net.c
>> index fe1a77e..3498e9d 100644
>> --- a/lib/librte_vhost/virtio-net.c
>> +++ b/lib/librte_vhost/virtio-net.c
>> @@ -263,8 +263,8 @@ init_vring_queue(struct vhost_virtqueue *vq, int
>> qp_idx)
>>   {
>>       memset(vq, 0, sizeof(struct vhost_virtqueue));
>>   -    vq->kickfd = -1;
>> -    vq->callfd = -1;
>> +    vq->kickfd = VIRTIO_UNINITIALIZED_EVENTFD;
>> +    vq->callfd = VIRTIO_UNINITIALIZED_EVENTFD;
>>         /* Backends are set to -1 indicating an inactive device. */
>>       vq->backend = -1;
>

Re: [dpdk-dev] [PATCH] vhost: Fix default value of kickfd and callfd

[Tan, Jianfeng]

On 3/10/2016 2:34 PM, Tetsuya Mukawa wrote:
> On 2016/03/10 15:25, Tan, Jianfeng wrote:
>>
>> On 3/10/2016 2:14 PM, Tetsuya Mukawa wrote:
>>> Currently, default value of kickfd and callfd is -1.
>>> If the value is -1, current code guess kickfd and callfd hasn't been
>>> initialized yet. And vhost library will guess the virtqueue isn't ready
>>> for processing.
>>> But callfd and kickfd will be set as -1 when "--enable-kvm"
>>> isn't specified in QEMU command line. It means we cannot treat -1 as
>>> uninitialized state. The patch changes default value to -2. And the
>>> patch defines -2 as VIRTIO_UNINITIALIZED_EVENTFD.
>>>
>>> Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
>>> ---
>>>    lib/librte_vhost/rte_virtio_net.h             | 1 +
>>>    lib/librte_vhost/vhost_user/virtio-net-user.c | 7 ++++---
>>>    lib/librte_vhost/virtio-net.c                 | 4 ++--
>>>    3 files changed, 7 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/lib/librte_vhost/rte_virtio_net.h
>>> b/lib/librte_vhost/rte_virtio_net.h
>>> index 7d1fde2..d745707 100644
>>> --- a/lib/librte_vhost/rte_virtio_net.h
>>> +++ b/lib/librte_vhost/rte_virtio_net.h
>>> @@ -89,6 +89,7 @@ struct vhost_virtqueue {
>>>        uint16_t        vhost_hlen;        /**< Vhost header length
>>> (varies depending on RX merge buffers. */
>>>        volatile uint16_t    last_used_idx;        /**< Last index used
>>> on the available ring */
>>>        volatile uint16_t    last_used_idx_res;    /**< Used for
>>> multiple devices reserving buffers. */
>>> +#define VIRTIO_UNINITIALIZED_EVENTFD    (-2)
>>>        int            callfd;            /**< Used to notify the guest
>>> (trigger interrupt). */
>>>        int            kickfd;            /**< Currently unused as
>>> polling mode is enabled. */
>>>        int            enabled;
>>> diff --git a/lib/librte_vhost/vhost_user/virtio-net-user.c
>>> b/lib/librte_vhost/vhost_user/virtio-net-user.c
>>> index 65b5652..591f9bf 100644
>>> --- a/lib/librte_vhost/vhost_user/virtio-net-user.c
>>> +++ b/lib/librte_vhost/vhost_user/virtio-net-user.c
>>> @@ -225,8 +225,8 @@ static int
>>>    vq_is_ready(struct vhost_virtqueue *vq)
>>>    {
>>>        return vq && vq->desc   &&
>>> -           vq->kickfd != -1 &&
>>> -           vq->callfd != -1;
>>> +           vq->kickfd != VIRTIO_UNINITIALIZED_EVENTFD &&
>>> +           vq->callfd != VIRTIO_UNINITIALIZED_EVENTFD;
>>>    }
>>>      static int
>>> @@ -318,7 +318,8 @@ user_get_vring_base(struct vhost_device_ctx ctx,
>>>         */
>>>        if (dev->virtqueue[state->index]->kickfd >= 0) {
>>>            close(dev->virtqueue[state->index]->kickfd);
>> Then -1 will be a valid value, need to check if it's not -1 to
>> close()? Nevertheless, close(-1) brings no big problem.
>>
> We did it in above 'if' condition checking, then close(-1) will not be
> called.

Sorry, missed that. So when kickfd == -1, need to be set to 
VIRTIO_UNINITIALIZED_EVENTFD?

Thanks,
Jianfeng

>
> Thanks,
> Tetsuya
>
>> Thanks,
>> Jianfeng
>>
>>> -        dev->virtqueue[state->index]->kickfd = -1;
>>> +        dev->virtqueue[state->index]->kickfd =
>>> +                    VIRTIO_UNINITIALIZED_EVENTFD;
>>>        }
>>>          return 0;
>>> diff --git a/lib/librte_vhost/virtio-net.c
>>> b/lib/librte_vhost/virtio-net.c
>>> index fe1a77e..3498e9d 100644
>>> --- a/lib/librte_vhost/virtio-net.c
>>> +++ b/lib/librte_vhost/virtio-net.c
>>> @@ -263,8 +263,8 @@ init_vring_queue(struct vhost_virtqueue *vq, int
>>> qp_idx)
>>>    {
>>>        memset(vq, 0, sizeof(struct vhost_virtqueue));
>>>    -    vq->kickfd = -1;
>>> -    vq->callfd = -1;
>>> +    vq->kickfd = VIRTIO_UNINITIALIZED_EVENTFD;
>>> +    vq->callfd = VIRTIO_UNINITIALIZED_EVENTFD;
>>>          /* Backends are set to -1 indicating an inactive device. */
>>>        vq->backend = -1;

Re: [dpdk-dev] [PATCH] vhost: Fix default value of kickfd and callfd

[Tetsuya Mukawa]

On 2016/03/10 15:39, Tan, Jianfeng wrote:
>
> On 3/10/2016 2:34 PM, Tetsuya Mukawa wrote:
>> On 2016/03/10 15:25, Tan, Jianfeng wrote:
>>>
>>> On 3/10/2016 2:14 PM, Tetsuya Mukawa wrote:
>>>> Currently, default value of kickfd and callfd is -1.
>>>> If the value is -1, current code guess kickfd and callfd hasn't been
>>>> initialized yet. And vhost library will guess the virtqueue isn't
>>>> ready
>>>> for processing.
>>>> But callfd and kickfd will be set as -1 when "--enable-kvm"
>>>> isn't specified in QEMU command line. It means we cannot treat -1 as
>>>> uninitialized state. The patch changes default value to -2. And the
>>>> patch defines -2 as VIRTIO_UNINITIALIZED_EVENTFD.
>>>>
>>>> Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
>>>> ---
>>>>    lib/librte_vhost/rte_virtio_net.h             | 1 +
>>>>    lib/librte_vhost/vhost_user/virtio-net-user.c | 7 ++++---
>>>>    lib/librte_vhost/virtio-net.c                 | 4 ++--
>>>>    3 files changed, 7 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/lib/librte_vhost/rte_virtio_net.h
>>>> b/lib/librte_vhost/rte_virtio_net.h
>>>> index 7d1fde2..d745707 100644
>>>> --- a/lib/librte_vhost/rte_virtio_net.h
>>>> +++ b/lib/librte_vhost/rte_virtio_net.h
>>>> @@ -89,6 +89,7 @@ struct vhost_virtqueue {
>>>>        uint16_t        vhost_hlen;        /**< Vhost header length
>>>> (varies depending on RX merge buffers. */
>>>>        volatile uint16_t    last_used_idx;        /**< Last index used
>>>> on the available ring */
>>>>        volatile uint16_t    last_used_idx_res;    /**< Used for
>>>> multiple devices reserving buffers. */
>>>> +#define VIRTIO_UNINITIALIZED_EVENTFD    (-2)
>>>>        int            callfd;            /**< Used to notify the guest
>>>> (trigger interrupt). */
>>>>        int            kickfd;            /**< Currently unused as
>>>> polling mode is enabled. */
>>>>        int            enabled;
>>>> diff --git a/lib/librte_vhost/vhost_user/virtio-net-user.c
>>>> b/lib/librte_vhost/vhost_user/virtio-net-user.c
>>>> index 65b5652..591f9bf 100644
>>>> --- a/lib/librte_vhost/vhost_user/virtio-net-user.c
>>>> +++ b/lib/librte_vhost/vhost_user/virtio-net-user.c
>>>> @@ -225,8 +225,8 @@ static int
>>>>    vq_is_ready(struct vhost_virtqueue *vq)
>>>>    {
>>>>        return vq && vq->desc   &&
>>>> -           vq->kickfd != -1 &&
>>>> -           vq->callfd != -1;
>>>> +           vq->kickfd != VIRTIO_UNINITIALIZED_EVENTFD &&
>>>> +           vq->callfd != VIRTIO_UNINITIALIZED_EVENTFD;
>>>>    }
>>>>      static int
>>>> @@ -318,7 +318,8 @@ user_get_vring_base(struct vhost_device_ctx ctx,
>>>>         */
>>>>        if (dev->virtqueue[state->index]->kickfd >= 0) {
>>>>            close(dev->virtqueue[state->index]->kickfd);
>>> Then -1 will be a valid value, need to check if it's not -1 to
>>> close()? Nevertheless, close(-1) brings no big problem.
>>>
>> We did it in above 'if' condition checking, then close(-1) will not be
>> called.
>
> Sorry, missed that. So when kickfd == -1, need to be set to
> VIRTIO_UNINITIALIZED_EVENTFD?
>

Aha, I've got it. Will change it.

Thanks,
Tetsuya

> Thanks,
> Jianfeng
>
>>
>> Thanks,
>> Tetsuya
>>
>>> Thanks,
>>> Jianfeng
>>>
>>>> -        dev->virtqueue[state->index]->kickfd = -1;
>>>> +        dev->virtqueue[state->index]->kickfd =
>>>> +                    VIRTIO_UNINITIALIZED_EVENTFD;
>>>>        }
>>>>          return 0;
>>>> diff --git a/lib/librte_vhost/virtio-net.c
>>>> b/lib/librte_vhost/virtio-net.c
>>>> index fe1a77e..3498e9d 100644
>>>> --- a/lib/librte_vhost/virtio-net.c
>>>> +++ b/lib/librte_vhost/virtio-net.c
>>>> @@ -263,8 +263,8 @@ init_vring_queue(struct vhost_virtqueue *vq, int
>>>> qp_idx)
>>>>    {
>>>>        memset(vq, 0, sizeof(struct vhost_virtqueue));
>>>>    -    vq->kickfd = -1;
>>>> -    vq->callfd = -1;
>>>> +    vq->kickfd = VIRTIO_UNINITIALIZED_EVENTFD;
>>>> +    vq->callfd = VIRTIO_UNINITIALIZED_EVENTFD;
>>>>          /* Backends are set to -1 indicating an inactive device. */
>>>>        vq->backend = -1;
>

[dpdk-dev] [PATCH v2] vhost: Fix default value of kickfd and callfd

[Tetsuya Mukawa]

Currently, default values of kickfd and callfd are -1.
If the values are -1, current code guesses kickfd and callfd haven't
been initialized yet. And vhost library will guess the virtqueue isn't
ready for processing.
But callfd and kickfd will be set as -1 when "--enable-kvm"
isn't specified in QEMU command line. It means we cannot treat -1 as
uninitialized state. The patch changes default values to -2. And the
patch defines -2 as VIRTIO_UNINITIALIZED_EVENTFD.

Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
---
 lib/librte_vhost/rte_virtio_net.h             |  1 +
 lib/librte_vhost/vhost_user/virtio-net-user.c | 10 +++++-----
 lib/librte_vhost/virtio-net.c                 |  4 ++--
 3 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/lib/librte_vhost/rte_virtio_net.h b/lib/librte_vhost/rte_virtio_net.h
index 7d1fde2..d745707 100644
--- a/lib/librte_vhost/rte_virtio_net.h
+++ b/lib/librte_vhost/rte_virtio_net.h
@@ -89,6 +89,7 @@ struct vhost_virtqueue {
 	uint16_t		vhost_hlen;		/**< Vhost header length (varies depending on RX merge buffers. */
 	volatile uint16_t	last_used_idx;		/**< Last index used on the available ring */
 	volatile uint16_t	last_used_idx_res;	/**< Used for multiple devices reserving buffers. */
+#define VIRTIO_UNINITIALIZED_EVENTFD	(-2)
 	int			callfd;			/**< Used to notify the guest (trigger interrupt). */
 	int			kickfd;			/**< Currently unused as polling mode is enabled. */
 	int			enabled;
diff --git a/lib/librte_vhost/vhost_user/virtio-net-user.c b/lib/librte_vhost/vhost_user/virtio-net-user.c
index 65b5652..4c156a6 100644
--- a/lib/librte_vhost/vhost_user/virtio-net-user.c
+++ b/lib/librte_vhost/vhost_user/virtio-net-user.c
@@ -225,8 +225,8 @@ static int
 vq_is_ready(struct vhost_virtqueue *vq)
 {
 	return vq && vq->desc   &&
-	       vq->kickfd != -1 &&
-	       vq->callfd != -1;
+	       vq->kickfd != VIRTIO_UNINITIALIZED_EVENTFD &&
+	       vq->callfd != VIRTIO_UNINITIALIZED_EVENTFD;
 }
 
 static int
@@ -316,10 +316,10 @@ user_get_vring_base(struct vhost_device_ctx ctx,
 	 * sent and only sent in vhost_vring_stop.
 	 * TODO: cleanup the vring, it isn't usable since here.
 	 */
-	if (dev->virtqueue[state->index]->kickfd >= 0) {
+	if (dev->virtqueue[state->index]->kickfd >= 0)
 		close(dev->virtqueue[state->index]->kickfd);
-		dev->virtqueue[state->index]->kickfd = -1;
-	}
+
+	dev->virtqueue[state->index]->kickfd = VIRTIO_UNINITIALIZED_EVENTFD;
 
 	return 0;
 }
diff --git a/lib/librte_vhost/virtio-net.c b/lib/librte_vhost/virtio-net.c
index fe1a77e..3498e9d 100644
--- a/lib/librte_vhost/virtio-net.c
+++ b/lib/librte_vhost/virtio-net.c
@@ -263,8 +263,8 @@ init_vring_queue(struct vhost_virtqueue *vq, int qp_idx)
 {
 	memset(vq, 0, sizeof(struct vhost_virtqueue));
 
-	vq->kickfd = -1;
-	vq->callfd = -1;
+	vq->kickfd = VIRTIO_UNINITIALIZED_EVENTFD;
+	vq->callfd = VIRTIO_UNINITIALIZED_EVENTFD;
 
 	/* Backends are set to -1 indicating an inactive device. */
 	vq->backend = -1;
-- 
2.1.4

Re: [dpdk-dev] [PATCH v2] vhost: Fix default value of kickfd and callfd

[Yuanhan Liu]

On Thu, Mar 10, 2016 at 04:06:05PM +0900, Tetsuya Mukawa wrote:
> Currently, default values of kickfd and callfd are -1.
> If the values are -1, current code guesses kickfd and callfd haven't
> been initialized yet. And vhost library will guess the virtqueue isn't
> ready for processing.
> But callfd and kickfd will be set as -1 when "--enable-kvm"
> isn't specified in QEMU command line. It means we cannot treat -1 as
> uninitialized state. The patch changes default values to -2. And the
> patch defines -2 as VIRTIO_UNINITIALIZED_EVENTFD.

This looks more like a workaround to me. Besides, this patch would make
following fail:

    eventfd_write(vq->callfd, (eventfd_t)1);

	--yliu

Re: [dpdk-dev] [PATCH v2] vhost: Fix default value of kickfd and callfd

[Tetsuya Mukawa]

On 2016/03/11 16:19, Yuanhan Liu wrote:
> On Thu, Mar 10, 2016 at 04:06:05PM +0900, Tetsuya Mukawa wrote:
>> Currently, default values of kickfd and callfd are -1.
>> If the values are -1, current code guesses kickfd and callfd haven't
>> been initialized yet. And vhost library will guess the virtqueue isn't
>> ready for processing.
>> But callfd and kickfd will be set as -1 when "--enable-kvm"
>> isn't specified in QEMU command line. It means we cannot treat -1 as
>> uninitialized state. The patch changes default values to -2. And the
>> patch defines -2 as VIRTIO_UNINITIALIZED_EVENTFD.
> This looks more like a workaround to me. 


Hi Yuanhan,

Sorry for late reply.
I have checked QEMU documentation, and found below.

----------
 * VHOST_USER_SET_VRING_CALL

      Id: 14
      Equivalent ioctl: VHOST_SET_VRING_CALL
      Master payload: u64

      Set the event file descriptor to signal when buffers are used. It
      is passed in the ancillary data.
      Bits (0-7) of the payload contain the vring index. Bit 8 is the
      invalid FD flag.
----------

VHOST_USER_SET_VRING_KICK has almost same description.
I will check this invalid flag, and if it works for our case, then will
use it.
How about it?

> Besides, this patch would make
> following fail:
>
>     eventfd_write(vq->callfd, (eventfd_t)1);

It's my fault.
I thought above case should be blocked by virtio spec itself. So just
leave it.
But eventfd is came from vhost spec, so VRING_AVAIL_F_NO_INTERRUPT might
be set even when kickfd and callfd are -1.
Thanks for checking it.

Tetsuya

> 	--yliu

Re: [dpdk-dev] [PATCH v2] vhost: Fix default value of kickfd and callfd

[Yuanhan Liu]

On Mon, Mar 14, 2016 at 10:54:14AM +0900, Tetsuya Mukawa wrote:
> On 2016/03/11 16:19, Yuanhan Liu wrote:
> > On Thu, Mar 10, 2016 at 04:06:05PM +0900, Tetsuya Mukawa wrote:
> >> Currently, default values of kickfd and callfd are -1.
> >> If the values are -1, current code guesses kickfd and callfd haven't
> >> been initialized yet. And vhost library will guess the virtqueue isn't
> >> ready for processing.
> >> But callfd and kickfd will be set as -1 when "--enable-kvm"
> >> isn't specified in QEMU command line. It means we cannot treat -1 as
> >> uninitialized state. The patch changes default values to -2. And the
> >> patch defines -2 as VIRTIO_UNINITIALIZED_EVENTFD.
> > This looks more like a workaround to me. 
> 
> 
> Hi Yuanhan,
> 
> Sorry for late reply.
> I have checked QEMU documentation, and found below.
> 
> ----------
>  * VHOST_USER_SET_VRING_CALL
> 
>       Id: 14
>       Equivalent ioctl: VHOST_SET_VRING_CALL
>       Master payload: u64
> 
>       Set the event file descriptor to signal when buffers are used. It
>       is passed in the ancillary data.
>       Bits (0-7) of the payload contain the vring index. Bit 8 is the
>       invalid FD flag.
> ----------
> 
> VHOST_USER_SET_VRING_KICK has almost same description.
> I will check this invalid flag, and if it works for our case, then will
> use it.
> How about it?

Yeah, that indeed sounds much better.

	--yliu

> 
> > Besides, this patch would make
> > following fail:
> >
> >     eventfd_write(vq->callfd, (eventfd_t)1);
> 
> It's my fault.
> I thought above case should be blocked by virtio spec itself. So just
> leave it.
> But eventfd is came from vhost spec, so VRING_AVAIL_F_NO_INTERRUPT might
> be set even when kickfd and callfd are -1.
> Thanks for checking it.
> 
> Tetsuya
> 
> > 	--yliu

Re: [dpdk-dev] [PATCH v2] vhost: Fix default value of kickfd and callfd

[Tetsuya Mukawa]

On 2016/03/14 11:08, Yuanhan Liu wrote:
> On Mon, Mar 14, 2016 at 10:54:14AM +0900, Tetsuya Mukawa wrote:
>> On 2016/03/11 16:19, Yuanhan Liu wrote:
>>> On Thu, Mar 10, 2016 at 04:06:05PM +0900, Tetsuya Mukawa wrote:
>>>> Currently, default values of kickfd and callfd are -1.
>>>> If the values are -1, current code guesses kickfd and callfd haven't
>>>> been initialized yet. And vhost library will guess the virtqueue isn't
>>>> ready for processing.
>>>> But callfd and kickfd will be set as -1 when "--enable-kvm"
>>>> isn't specified in QEMU command line. It means we cannot treat -1 as
>>>> uninitialized state. The patch changes default values to -2. And the
>>>> patch defines -2 as VIRTIO_UNINITIALIZED_EVENTFD.
>>> This looks more like a workaround to me. 
>>
>> Hi Yuanhan,
>>
>> Sorry for late reply.
>> I have checked QEMU documentation, and found below.
>>
>> ----------
>>  * VHOST_USER_SET_VRING_CALL
>>
>>       Id: 14
>>       Equivalent ioctl: VHOST_SET_VRING_CALL
>>       Master payload: u64
>>
>>       Set the event file descriptor to signal when buffers are used. It
>>       is passed in the ancillary data.
>>       Bits (0-7) of the payload contain the vring index. Bit 8 is the
>>       invalid FD flag.
>> ----------
>>
>> VHOST_USER_SET_VRING_KICK has almost same description.
>> I will check this invalid flag, and if it works for our case, then will
>> use it.
>> How about it?
> Yeah, that indeed sounds much better.

I've checked current dpdk code.
It seems we've already checked invalid flag like below.

        if (pmsg->payload.u64 & VHOST_USER_VRING_NOFD_MASK)
                file.fd = -1;
        else
                file.fd = pmsg->fds[0];

So how about adding below macros or enum?

#define VIRTIO_UNINITIALIZED_EVENTFD   (-2)
#define VIRTIO_INVALID_EVENTFD              (-1)

I am still not sure whether using enum is better or not.
But here is one of example patch.
What do you think?

diff --git a/lib/librte_vhost/rte_virtio_net.h
b/lib/librte_vhost/rte_virtio_net.h
index 7d1fde2..2a7566d 100644
--- a/lib/librte_vhost/rte_virtio_net.h
+++ b/lib/librte_vhost/rte_virtio_net.h
@@ -89,6 +89,8 @@ struct vhost_virtqueue {
        uint16_t                vhost_hlen;             /**< Vhost
header length (varies depending on RX merge buffers. */
        volatile uint16_t       last_used_idx;          /**< Last index
used on the available ring */
        volatile uint16_t       last_used_idx_res;      /**< Used for
multiple devices reserving buffers. */
+#define VIRTIO_UNINITIALIZED_EVENTFD   (-2)
+#define VIRTIO_INVALID_EVENTFD         (-1)
        int                     callfd;                 /**< Used to
notify the guest (trigger interrupt). */
        int                     kickfd;                 /**< Currently
unused as polling mode is enabled. */
        int                     enabled;
diff --git a/lib/librte_vhost/vhost_rxtx.c b/lib/librte_vhost/vhost_rxtx.c
index 9d23eb1..29d002d 100644
--- a/lib/librte_vhost/vhost_rxtx.c
+++ b/lib/librte_vhost/vhost_rxtx.c
@@ -333,7 +333,8 @@ virtio_dev_rx(struct virtio_net *dev, uint16_t queue_id,
        rte_mb();

        /* Kick the guest if necessary. */
-       if (!(vq->avail->flags & VRING_AVAIL_F_NO_INTERRUPT))
+       if (!(vq->avail->flags & VRING_AVAIL_F_NO_INTERRUPT)
+                       && (vq->callfd >= 0))
                eventfd_write(vq->callfd, (eventfd_t)1);
        return count;
 }
@@ -654,7 +655,8 @@ merge_rx_exit:
                rte_mb();

                /* Kick the guest if necessary. */
-               if (!(vq->avail->flags & VRING_AVAIL_F_NO_INTERRUPT))
+               if (!(vq->avail->flags & VRING_AVAIL_F_NO_INTERRUPT)
+                               && (vq->callfd >= 0))
                        eventfd_write(vq->callfd, (eventfd_t)1);
        }

@@ -1048,7 +1050,8 @@ rte_vhost_dequeue_burst(struct virtio_net *dev,
uint16_t queue_id,
                        sizeof(vq->used->idx));

        /* Kick guest if required. */
-       if (!(vq->avail->flags & VRING_AVAIL_F_NO_INTERRUPT))
+       if (!(vq->avail->flags & VRING_AVAIL_F_NO_INTERRUPT)
+                       && (vq->callfd >= 0))
                eventfd_write(vq->callfd, (eventfd_t)1);

 out:
diff --git a/lib/librte_vhost/vhost_user/virtio-net-user.c
b/lib/librte_vhost/vhost_user/virtio-net-user.c
index 65b5652..f5248bc 100644
--- a/lib/librte_vhost/vhost_user/virtio-net-user.c
+++ b/lib/librte_vhost/vhost_user/virtio-net-user.c
@@ -225,8 +225,8 @@ static int
 vq_is_ready(struct vhost_virtqueue *vq)
 {
        return vq && vq->desc   &&
-              vq->kickfd != -1 &&
-              vq->callfd != -1;
+              vq->kickfd != VIRTIO_UNINITIALIZED_EVENTFD &&
+              vq->callfd != VIRTIO_UNINITIALIZED_EVENTFD;
 }

 static int
@@ -258,7 +258,7 @@ user_set_vring_call(struct vhost_device_ctx ctx,
struct VhostUserMsg *pmsg)

        file.index = pmsg->payload.u64 & VHOST_USER_VRING_IDX_MASK;
        if (pmsg->payload.u64 & VHOST_USER_VRING_NOFD_MASK)
-               file.fd = -1;
+               file.fd = VIRTIO_INVALID_EVENTFD;
        else
                file.fd = pmsg->fds[0];
        RTE_LOG(INFO, VHOST_CONFIG,
@@ -279,7 +279,7 @@ user_set_vring_kick(struct vhost_device_ctx ctx,
struct VhostUserMsg *pmsg)

        file.index = pmsg->payload.u64 & VHOST_USER_VRING_IDX_MASK;
        if (pmsg->payload.u64 & VHOST_USER_VRING_NOFD_MASK)
-               file.fd = -1;
+               file.fd = VIRTIO_INVALID_EVENTFD;
        else
                file.fd = pmsg->fds[0];
        RTE_LOG(INFO, VHOST_CONFIG,
@@ -316,10 +316,10 @@ user_get_vring_base(struct vhost_device_ctx ctx,
         * sent and only sent in vhost_vring_stop.
         * TODO: cleanup the vring, it isn't usable since here.
         */
-       if (dev->virtqueue[state->index]->kickfd >= 0) {
+       if (dev->virtqueue[state->index]->kickfd >= 0)
                close(dev->virtqueue[state->index]->kickfd);
-               dev->virtqueue[state->index]->kickfd = -1;
-       }
+
+       dev->virtqueue[state->index]->kickfd = VIRTIO_UNINITIALIZED_EVENTFD;

        return 0;
 }
diff --git a/lib/librte_vhost/virtio-net.c b/lib/librte_vhost/virtio-net.c
index fe1a77e..3498e9d 100644
--- a/lib/librte_vhost/virtio-net.c
+++ b/lib/librte_vhost/virtio-net.c
@@ -263,8 +263,8 @@ init_vring_queue(struct vhost_virtqueue *vq, int qp_idx)
 {
        memset(vq, 0, sizeof(struct vhost_virtqueue));

-       vq->kickfd = -1;
-       vq->callfd = -1;
+       vq->kickfd = VIRTIO_UNINITIALIZED_EVENTFD;
+       vq->callfd = VIRTIO_UNINITIALIZED_EVENTFD;

        /* Backends are set to -1 indicating an inactive device. */
        vq->backend = -1;



> 	--yliu
>
>>> Besides, this patch would make
>>> following fail:
>>>
>>>     eventfd_write(vq->callfd, (eventfd_t)1);
>> It's my fault.
>> I thought above case should be blocked by virtio spec itself. So just
>> leave it.
>> But eventfd is came from vhost spec, so VRING_AVAIL_F_NO_INTERRUPT might
>> be set even when kickfd and callfd are -1.
>> Thanks for checking it.
>>
>> Tetsuya
>>
>>> 	--yliu

Re: [dpdk-dev] [PATCH v2] vhost: Fix default value of kickfd and callfd

[Yuanhan Liu]

On Mon, Mar 14, 2016 at 04:54:00PM +0900, Tetsuya Mukawa wrote:
> On 2016/03/14 11:08, Yuanhan Liu wrote:
> > On Mon, Mar 14, 2016 at 10:54:14AM +0900, Tetsuya Mukawa wrote:
> >> On 2016/03/11 16:19, Yuanhan Liu wrote:
> >>> On Thu, Mar 10, 2016 at 04:06:05PM +0900, Tetsuya Mukawa wrote:
> >>>> Currently, default values of kickfd and callfd are -1.
> >>>> If the values are -1, current code guesses kickfd and callfd haven't
> >>>> been initialized yet. And vhost library will guess the virtqueue isn't
> >>>> ready for processing.
> >>>> But callfd and kickfd will be set as -1 when "--enable-kvm"
> >>>> isn't specified in QEMU command line. It means we cannot treat -1 as
> >>>> uninitialized state. The patch changes default values to -2. And the
> >>>> patch defines -2 as VIRTIO_UNINITIALIZED_EVENTFD.
> >>> This looks more like a workaround to me. 
> >>
> >> Hi Yuanhan,
> >>
> >> Sorry for late reply.
> >> I have checked QEMU documentation, and found below.
> >>
> >> ----------
> >>  * VHOST_USER_SET_VRING_CALL
> >>
> >>       Id: 14
> >>       Equivalent ioctl: VHOST_SET_VRING_CALL
> >>       Master payload: u64
> >>
> >>       Set the event file descriptor to signal when buffers are used. It
> >>       is passed in the ancillary data.
> >>       Bits (0-7) of the payload contain the vring index. Bit 8 is the
> >>       invalid FD flag.
> >> ----------
> >>
> >> VHOST_USER_SET_VRING_KICK has almost same description.
> >> I will check this invalid flag, and if it works for our case, then will
> >> use it.
> >> How about it?
> > Yeah, that indeed sounds much better.
> 
> I've checked current dpdk code.
> It seems we've already checked invalid flag like below.
> 
>         if (pmsg->payload.u64 & VHOST_USER_VRING_NOFD_MASK)
>                 file.fd = -1;
>         else
>                 file.fd = pmsg->fds[0];
> 
> So how about adding below macros or enum?
> 
> #define VIRTIO_UNINITIALIZED_EVENTFD   (-2)
> #define VIRTIO_INVALID_EVENTFD              (-1)
> 
> I am still not sure whether using enum is better or not.

Both are Okay to me; I have no preference on that.

> But here is one of example patch.
> What do you think?

Looks okay to me

> 
> diff --git a/lib/librte_vhost/rte_virtio_net.h
> b/lib/librte_vhost/rte_virtio_net.h
> index 7d1fde2..2a7566d 100644
> --- a/lib/librte_vhost/rte_virtio_net.h
> +++ b/lib/librte_vhost/rte_virtio_net.h
> @@ -89,6 +89,8 @@ struct vhost_virtqueue {
>         uint16_t                vhost_hlen;             /**< Vhost
> header length (varies depending on RX merge buffers. */
>         volatile uint16_t       last_used_idx;          /**< Last index
> used on the available ring */
>         volatile uint16_t       last_used_idx_res;      /**< Used for
> multiple devices reserving buffers. */
> +#define VIRTIO_UNINITIALIZED_EVENTFD   (-2)
> +#define VIRTIO_INVALID_EVENTFD         (-1)

One nit: you may keep it in order.

	--yliu

Re: [dpdk-dev] [PATCH v2] vhost: Fix default value of kickfd and callfd

[Tetsuya Mukawa]

On 2016/03/14 17:21, Yuanhan Liu wrote:
> On Mon, Mar 14, 2016 at 04:54:00PM +0900, Tetsuya Mukawa wrote:
>> On 2016/03/14 11:08, Yuanhan Liu wrote:
>>> On Mon, Mar 14, 2016 at 10:54:14AM +0900, Tetsuya Mukawa wrote:
>>>> On 2016/03/11 16:19, Yuanhan Liu wrote:
>>>>> On Thu, Mar 10, 2016 at 04:06:05PM +0900, Tetsuya Mukawa wrote:
>>>>>> Currently, default values of kickfd and callfd are -1.
>>>>>> If the values are -1, current code guesses kickfd and callfd haven't
>>>>>> been initialized yet. And vhost library will guess the virtqueue isn't
>>>>>> ready for processing.
>>>>>> But callfd and kickfd will be set as -1 when "--enable-kvm"
>>>>>> isn't specified in QEMU command line. It means we cannot treat -1 as
>>>>>> uninitialized state. The patch changes default values to -2. And the
>>>>>> patch defines -2 as VIRTIO_UNINITIALIZED_EVENTFD.
>>>>> This looks more like a workaround to me. 
>>>> Hi Yuanhan,
>>>>
>>>> Sorry for late reply.
>>>> I have checked QEMU documentation, and found below.
>>>>
>>>> ----------
>>>>  * VHOST_USER_SET_VRING_CALL
>>>>
>>>>       Id: 14
>>>>       Equivalent ioctl: VHOST_SET_VRING_CALL
>>>>       Master payload: u64
>>>>
>>>>       Set the event file descriptor to signal when buffers are used. It
>>>>       is passed in the ancillary data.
>>>>       Bits (0-7) of the payload contain the vring index. Bit 8 is the
>>>>       invalid FD flag.
>>>> ----------
>>>>
>>>> VHOST_USER_SET_VRING_KICK has almost same description.
>>>> I will check this invalid flag, and if it works for our case, then will
>>>> use it.
>>>> How about it?
>>> Yeah, that indeed sounds much better.
>> I've checked current dpdk code.
>> It seems we've already checked invalid flag like below.
>>
>>         if (pmsg->payload.u64 & VHOST_USER_VRING_NOFD_MASK)
>>                 file.fd = -1;
>>         else
>>                 file.fd = pmsg->fds[0];
>>
>> So how about adding below macros or enum?
>>
>> #define VIRTIO_UNINITIALIZED_EVENTFD   (-2)
>> #define VIRTIO_INVALID_EVENTFD              (-1)
>>
>> I am still not sure whether using enum is better or not.
> Both are Okay to me; I have no preference on that.
>
>> But here is one of example patch.
>> What do you think?
> Looks okay to me
>
>> diff --git a/lib/librte_vhost/rte_virtio_net.h
>> b/lib/librte_vhost/rte_virtio_net.h
>> index 7d1fde2..2a7566d 100644
>> --- a/lib/librte_vhost/rte_virtio_net.h
>> +++ b/lib/librte_vhost/rte_virtio_net.h
>> @@ -89,6 +89,8 @@ struct vhost_virtqueue {
>>         uint16_t                vhost_hlen;             /**< Vhost
>> header length (varies depending on RX merge buffers. */
>>         volatile uint16_t       last_used_idx;          /**< Last index
>> used on the available ring */
>>         volatile uint16_t       last_used_idx_res;      /**< Used for
>> multiple devices reserving buffers. */
>> +#define VIRTIO_UNINITIALIZED_EVENTFD   (-2)
>> +#define VIRTIO_INVALID_EVENTFD         (-1)
> One nit: you may keep it in order.

Thanks for  your comments, will change it.

Tetsuya

> 	--yliu

[dpdk-dev] [PATCH v3] vhost: Fix default value of kickfd and callfd

[Tetsuya Mukawa]

Currently, default values of kickfd and callfd are -1.
If the values are -1, current code guesses kickfd and callfd haven't
been initialized yet. Then vhost library will guess the virtqueue isn't
ready for processing.
But callfd and kickfd will be set as -1 when "--enable-kvm"
isn't specified in QEMU command line. It means we cannot treat -1 as
uninitialized state.
The patch defines -1 and -2 as VIRTIO_INVALID_EVENTFD and
VIRTIO_UNINITIALIZED_EVENTFD, and uses VIRTIO_UNINITIALIZED_EVENTFD for
the default values of kickfd and callfd.

Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
---
 lib/librte_vhost/rte_virtio_net.h             |  2 ++
 lib/librte_vhost/vhost_rxtx.c                 |  9 ++++++---
 lib/librte_vhost/vhost_user/virtio-net-user.c | 14 +++++++-------
 lib/librte_vhost/virtio-net.c                 |  4 ++--
 4 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/lib/librte_vhost/rte_virtio_net.h b/lib/librte_vhost/rte_virtio_net.h
index 443245d..600b20b 100644
--- a/lib/librte_vhost/rte_virtio_net.h
+++ b/lib/librte_vhost/rte_virtio_net.h
@@ -89,6 +89,8 @@ struct vhost_virtqueue {
 	uint16_t		vhost_hlen;		/**< Vhost header length (varies depending on RX merge buffers. */
 	volatile uint16_t	last_used_idx;		/**< Last index used on the available ring */
 	volatile uint16_t	last_used_idx_res;	/**< Used for multiple devices reserving buffers. */
+#define VIRTIO_INVALID_EVENTFD		(-1)
+#define VIRTIO_UNINITIALIZED_EVENTFD	(-2)
 	int			callfd;			/**< Used to notify the guest (trigger interrupt). */
 	int			kickfd;			/**< Currently unused as polling mode is enabled. */
 	int			enabled;
diff --git a/lib/librte_vhost/vhost_rxtx.c b/lib/librte_vhost/vhost_rxtx.c
index 9d23eb1..29d002d 100644
--- a/lib/librte_vhost/vhost_rxtx.c
+++ b/lib/librte_vhost/vhost_rxtx.c
@@ -333,7 +333,8 @@ virtio_dev_rx(struct virtio_net *dev, uint16_t queue_id,
 	rte_mb();
 
 	/* Kick the guest if necessary. */
-	if (!(vq->avail->flags & VRING_AVAIL_F_NO_INTERRUPT))
+	if (!(vq->avail->flags & VRING_AVAIL_F_NO_INTERRUPT)
+			&& (vq->callfd >= 0))
 		eventfd_write(vq->callfd, (eventfd_t)1);
 	return count;
 }
@@ -654,7 +655,8 @@ merge_rx_exit:
 		rte_mb();
 
 		/* Kick the guest if necessary. */
-		if (!(vq->avail->flags & VRING_AVAIL_F_NO_INTERRUPT))
+		if (!(vq->avail->flags & VRING_AVAIL_F_NO_INTERRUPT)
+				&& (vq->callfd >= 0))
 			eventfd_write(vq->callfd, (eventfd_t)1);
 	}
 
@@ -1048,7 +1050,8 @@ rte_vhost_dequeue_burst(struct virtio_net *dev, uint16_t queue_id,
 			sizeof(vq->used->idx));
 
 	/* Kick guest if required. */
-	if (!(vq->avail->flags & VRING_AVAIL_F_NO_INTERRUPT))
+	if (!(vq->avail->flags & VRING_AVAIL_F_NO_INTERRUPT)
+			&& (vq->callfd >= 0))
 		eventfd_write(vq->callfd, (eventfd_t)1);
 
 out:
diff --git a/lib/librte_vhost/vhost_user/virtio-net-user.c b/lib/librte_vhost/vhost_user/virtio-net-user.c
index 65b5652..f5248bc 100644
--- a/lib/librte_vhost/vhost_user/virtio-net-user.c
+++ b/lib/librte_vhost/vhost_user/virtio-net-user.c
@@ -225,8 +225,8 @@ static int
 vq_is_ready(struct vhost_virtqueue *vq)
 {
 	return vq && vq->desc   &&
-	       vq->kickfd != -1 &&
-	       vq->callfd != -1;
+	       vq->kickfd != VIRTIO_UNINITIALIZED_EVENTFD &&
+	       vq->callfd != VIRTIO_UNINITIALIZED_EVENTFD;
 }
 
 static int
@@ -258,7 +258,7 @@ user_set_vring_call(struct vhost_device_ctx ctx, struct VhostUserMsg *pmsg)
 
 	file.index = pmsg->payload.u64 & VHOST_USER_VRING_IDX_MASK;
 	if (pmsg->payload.u64 & VHOST_USER_VRING_NOFD_MASK)
-		file.fd = -1;
+		file.fd = VIRTIO_INVALID_EVENTFD;
 	else
 		file.fd = pmsg->fds[0];
 	RTE_LOG(INFO, VHOST_CONFIG,
@@ -279,7 +279,7 @@ user_set_vring_kick(struct vhost_device_ctx ctx, struct VhostUserMsg *pmsg)
 
 	file.index = pmsg->payload.u64 & VHOST_USER_VRING_IDX_MASK;
 	if (pmsg->payload.u64 & VHOST_USER_VRING_NOFD_MASK)
-		file.fd = -1;
+		file.fd = VIRTIO_INVALID_EVENTFD;
 	else
 		file.fd = pmsg->fds[0];
 	RTE_LOG(INFO, VHOST_CONFIG,
@@ -316,10 +316,10 @@ user_get_vring_base(struct vhost_device_ctx ctx,
 	 * sent and only sent in vhost_vring_stop.
 	 * TODO: cleanup the vring, it isn't usable since here.
 	 */
-	if (dev->virtqueue[state->index]->kickfd >= 0) {
+	if (dev->virtqueue[state->index]->kickfd >= 0)
 		close(dev->virtqueue[state->index]->kickfd);
-		dev->virtqueue[state->index]->kickfd = -1;
-	}
+
+	dev->virtqueue[state->index]->kickfd = VIRTIO_UNINITIALIZED_EVENTFD;
 
 	return 0;
 }
diff --git a/lib/librte_vhost/virtio-net.c b/lib/librte_vhost/virtio-net.c
index 28c3912..90da9ba 100644
--- a/lib/librte_vhost/virtio-net.c
+++ b/lib/librte_vhost/virtio-net.c
@@ -167,8 +167,8 @@ init_vring_queue(struct vhost_virtqueue *vq, int qp_idx)
 {
 	memset(vq, 0, sizeof(struct vhost_virtqueue));
 
-	vq->kickfd = -1;
-	vq->callfd = -1;
+	vq->kickfd = VIRTIO_UNINITIALIZED_EVENTFD;
+	vq->callfd = VIRTIO_UNINITIALIZED_EVENTFD;
 
 	/* Backends are set to -1 indicating an inactive device. */
 	vq->backend = -1;
-- 
2.1.4

Re: [dpdk-dev] [PATCH v3] vhost: Fix default value of kickfd and callfd

[Yuanhan Liu]

On Mon, Mar 14, 2016 at 05:53:32PM +0900, Tetsuya Mukawa wrote:
> Currently, default values of kickfd and callfd are -1.
> If the values are -1, current code guesses kickfd and callfd haven't
> been initialized yet. Then vhost library will guess the virtqueue isn't
> ready for processing.
> But callfd and kickfd will be set as -1 when "--enable-kvm"
> isn't specified in QEMU command line. It means we cannot treat -1 as
> uninitialized state.
> The patch defines -1 and -2 as VIRTIO_INVALID_EVENTFD and
> VIRTIO_UNINITIALIZED_EVENTFD, and uses VIRTIO_UNINITIALIZED_EVENTFD for
> the default values of kickfd and callfd.

Don't be mean to put an empty line between paragraphs :)

> Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>

Acked-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>

Thanks.

	--yliu

Re: [dpdk-dev] [PATCH v3] vhost: Fix default value of kickfd and callfd

[Thomas Monjalon]

2016-03-14 17:12, Yuanhan Liu:
> On Mon, Mar 14, 2016 at 05:53:32PM +0900, Tetsuya Mukawa wrote:
> > Currently, default values of kickfd and callfd are -1.
> > If the values are -1, current code guesses kickfd and callfd haven't
> > been initialized yet. Then vhost library will guess the virtqueue isn't
> > ready for processing.
> > But callfd and kickfd will be set as -1 when "--enable-kvm"
> > isn't specified in QEMU command line. It means we cannot treat -1 as
> > uninitialized state.
> > The patch defines -1 and -2 as VIRTIO_INVALID_EVENTFD and
> > VIRTIO_UNINITIALIZED_EVENTFD, and uses VIRTIO_UNINITIALIZED_EVENTFD for
> > the default values of kickfd and callfd.
> 
> Don't be mean to put an empty line between paragraphs :)
> 
> > Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
> 
> Acked-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>

Applied, thanks