[dpdk-dev] net: virtio: clear reserved vring properly at setup time

  After vring reservation, only the first bytes of the vring were
cleared.

This patch fixes this to clear the real size fo the vring.

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

Note: I found this bug while doing some code review, it is not a fix for
a problem I encountered.

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

On Mon, Jul 18, 2016 at 07:09:24PM +0200, Maxime Coquelin wrote:
> After vring reservation, only the first bytes of the vring were
> cleared.
> 
> This patch fixes this to clear the real size fo the vring.
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
> 
> Note: I found this bug while doing some code review,

Nice catch!

> it is not a fix for
> a problem I encountered.

Yes, there should be no problem: vring memory is completely zero-ed
at port start stage by virtio_dev_vring_start().

> ---
>  drivers/net/virtio/virtio_ethdev.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
> index 850e3ba..336b3fc 100644
> --- a/drivers/net/virtio/virtio_ethdev.c
> +++ b/drivers/net/virtio/virtio_ethdev.c
> @@ -387,7 +387,7 @@ int virtio_dev_queue_setup(struct rte_eth_dev *dev,
>  		}
>  	}
>  
> -	memset(mz->addr, 0, sizeof(mz->len));
> +	memset(mz->addr, 0, mz->len);

Actually, I think we could simply drop the memset here. It's redundant,
as stated.

And to Thomas, I don't find a good reason to have this in 16.07. Let's
delay the apply to v16.11.

	--yliu

>  	vq->vq_ring_mem = mz->phys_addr;
>  	vq->vq_ring_virt_mem = mz->addr;
> -- 
> 2.7.4
  

On 07/19/2016 03:39 AM, Yuanhan Liu wrote:
> On Mon, Jul 18, 2016 at 07:09:24PM +0200, Maxime Coquelin wrote:
>> After vring reservation, only the first bytes of the vring were
>> cleared.
>>
>> This patch fixes this to clear the real size fo the vring.
>>
>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>> ---
>>
>> Note: I found this bug while doing some code review,
>
> Nice catch!
>
>> it is not a fix for
>> a problem I encountered.
>
> Yes, there should be no problem: vring memory is completely zero-ed
> at port start stage by virtio_dev_vring_start().

Ok, thanks for the pointer.

>
>> ---
>>  drivers/net/virtio/virtio_ethdev.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
>> index 850e3ba..336b3fc 100644
>> --- a/drivers/net/virtio/virtio_ethdev.c
>> +++ b/drivers/net/virtio/virtio_ethdev.c
>> @@ -387,7 +387,7 @@ int virtio_dev_queue_setup(struct rte_eth_dev *dev,
>>  		}
>>  	}
>>
>> -	memset(mz->addr, 0, sizeof(mz->len));
>> +	memset(mz->addr, 0, mz->len);
>
> Actually, I think we could simply drop the memset here. It's redundant,
> as stated.
We can skip this patch then.

>
> And to Thomas, I don't find a good reason to have this in 16.07. Let's
> delay the apply to v16.11.
Yes, that's why I mentioned it didn't fixed any problem on my side.

So I propose we skip this patch, I'll resend one removing the memset for 
v16.11.

Thanks,
Maxime

>
> 	--yliu
>
>>  	vq->vq_ring_mem = mz->phys_addr;
>>  	vq->vq_ring_virt_mem = mz->addr;
>> --
>> 2.7.4
  

On Tue, Jul 19, 2016 at 07:31:35AM +0200, Maxime Coquelin wrote:
> >And to Thomas, I don't find a good reason to have this in 16.07. Let's
> >delay the apply to v16.11.
> Yes, that's why I mentioned it didn't fixed any problem on my side.
> 
> So I propose we skip this patch, I'll resend one removing the memset for
> v16.11.

Good to me.

	--yliu