* [dpdk-dev] [PATCH] net: virtio: clear reserved vring properly at setup time
@ 2016-07-18 17:09 Maxime Coquelin
2016-07-19 1:39 ` Yuanhan Liu
0 siblings, 1 reply; 4+ messages in thread
From: Maxime Coquelin @ 2016-07-18 17:09 UTC (permalink / raw)
To: huawei.xie, yuanhan.liu; +Cc: dev, Maxime Coquelin
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(-)
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);
vq->vq_ring_mem = mz->phys_addr;
vq->vq_ring_virt_mem = mz->addr;
--
2.7.4
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [dpdk-dev] [PATCH] net: virtio: clear reserved vring properly at setup time
2016-07-18 17:09 [dpdk-dev] [PATCH] net: virtio: clear reserved vring properly at setup time Maxime Coquelin
@ 2016-07-19 1:39 ` Yuanhan Liu
2016-07-19 5:31 ` Maxime Coquelin
0 siblings, 1 reply; 4+ messages in thread
From: Yuanhan Liu @ 2016-07-19 1:39 UTC (permalink / raw)
To: Maxime Coquelin; +Cc: huawei.xie, dev, Thomas Monjalon
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [dpdk-dev] [PATCH] net: virtio: clear reserved vring properly at setup time
2016-07-19 1:39 ` Yuanhan Liu
@ 2016-07-19 5:31 ` Maxime Coquelin
2016-07-19 5:39 ` Yuanhan Liu
0 siblings, 1 reply; 4+ messages in thread
From: Maxime Coquelin @ 2016-07-19 5:31 UTC (permalink / raw)
To: Yuanhan Liu; +Cc: huawei.xie, dev, Thomas Monjalon
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [dpdk-dev] [PATCH] net: virtio: clear reserved vring properly at setup time
2016-07-19 5:31 ` Maxime Coquelin
@ 2016-07-19 5:39 ` Yuanhan Liu
0 siblings, 0 replies; 4+ messages in thread
From: Yuanhan Liu @ 2016-07-19 5:39 UTC (permalink / raw)
To: Maxime Coquelin; +Cc: huawei.xie, dev, Thomas Monjalon
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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-07-19 5:35 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-18 17:09 [dpdk-dev] [PATCH] net: virtio: clear reserved vring properly at setup time Maxime Coquelin
2016-07-19 1:39 ` Yuanhan Liu
2016-07-19 5:31 ` Maxime Coquelin
2016-07-19 5:39 ` Yuanhan Liu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).