DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH v2] vhost: fix build
@ 2022-08-29  8:29 Min Zhou
  2022-09-01  0:06 ` zhoumin
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Min Zhou @ 2022-08-29  8:29 UTC (permalink / raw)
  To: david.marchand, maxime.coquelin, chenbo.xia, zhoumin; +Cc: dev, maobibo

On CentOS 8 or Debian 10.4 systems using gcc 12.1 to cross
compile DPDK, gcc shows a following warning which will cause
build to fail when build is run with -werror:

In function 'mbuf_to_desc',
    inlined from 'vhost_enqueue_async_packed' at ../lib/vhost/virtio_net.c:1826:6,
    inlined from 'virtio_dev_rx_async_packed' at ../lib/vhost/virtio_net.c:1840:6,
    inlined from 'virtio_dev_rx_async_submit_packed.constprop' at ../lib/vhost/virtio_net.c:1900:7:
../lib/vhost/virtio_net.c:1161:35: error: 'buf_vec[0].buf_len' may be used uninitialized [-Werror=maybe-uninitialized]
 1161 |         buf_len = buf_vec[vec_idx].buf_len;
      |                   ~~~~~~~~~~~~~~~~^~~~~~~~
../lib/vhost/virtio_net.c: In function 'virtio_dev_rx_async_submit_packed.constprop':
../lib/vhost/virtio_net.c:1838:27: note: 'buf_vec' declared here
 1838 |         struct buf_vector buf_vec[BUF_VECTOR_MAX];
      |                           ^~~~~~~
cc1: all warnings being treated as errors

Actually, there are eight places to see the same codes in the file
lib/vhost/virtio_net.c, and all these `buf_vec` arraies are
initialized by sub-function calls under various conditions.

Although It's hard to understand why gcc just emits warning at one
of the eight places, adding validity checks for array length is
reasonable and can also fix the warning.

Signed-off-by: David Marchand <david.marchand@redhat.com>
Signed-off-by: Min Zhou <zhoumin@loongson.cn>
---
 lib/vhost/virtio_net.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/lib/vhost/virtio_net.c b/lib/vhost/virtio_net.c
index 35fa4670fd..99233f1759 100644
--- a/lib/vhost/virtio_net.c
+++ b/lib/vhost/virtio_net.c
@@ -1153,7 +1153,7 @@ mbuf_to_desc(struct virtio_net *dev, struct vhost_virtqueue *vq,
 	struct virtio_net_hdr_mrg_rxbuf tmp_hdr, *hdr = NULL;
 	struct vhost_async *async = vq->async;
 
-	if (unlikely(m == NULL))
+	if (unlikely(m == NULL || nr_vec == 0))
 		return -1;
 
 	buf_addr = buf_vec[vec_idx].buf_addr;
@@ -2673,6 +2673,9 @@ desc_to_mbuf(struct virtio_net *dev, struct vhost_virtqueue *vq,
 	struct vhost_async *async = vq->async;
 	struct async_inflight_info *pkts_info;
 
+	if (unlikely(nr_vec == 0))
+		return -1;
+
 	buf_addr = buf_vec[vec_idx].buf_addr;
 	buf_iova = buf_vec[vec_idx].buf_iova;
 	buf_len = buf_vec[vec_idx].buf_len;
-- 
2.31.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] vhost: fix build
  2022-08-29  8:29 [PATCH v2] vhost: fix build Min Zhou
@ 2022-09-01  0:06 ` zhoumin
  2022-09-09  9:07 ` zhoumin
  2022-09-26  2:57 ` Xia, Chenbo
  2 siblings, 0 replies; 8+ messages in thread
From: zhoumin @ 2022-09-01  0:06 UTC (permalink / raw)
  To: david.marchand, maxime.coquelin, chenbo.xia; +Cc: dev, maobibo

Sincerely ping.


On 2022年08月29日 16:29, Min Zhou wrote:
> On CentOS 8 or Debian 10.4 systems using gcc 12.1 to cross
> compile DPDK, gcc shows a following warning which will cause
> build to fail when build is run with -werror:
>
> In function 'mbuf_to_desc',
>      inlined from 'vhost_enqueue_async_packed' at ../lib/vhost/virtio_net.c:1826:6,
>      inlined from 'virtio_dev_rx_async_packed' at ../lib/vhost/virtio_net.c:1840:6,
>      inlined from 'virtio_dev_rx_async_submit_packed.constprop' at ../lib/vhost/virtio_net.c:1900:7:
> ../lib/vhost/virtio_net.c:1161:35: error: 'buf_vec[0].buf_len' may be used uninitialized [-Werror=maybe-uninitialized]
>   1161 |         buf_len = buf_vec[vec_idx].buf_len;
>        |                   ~~~~~~~~~~~~~~~~^~~~~~~~
> ../lib/vhost/virtio_net.c: In function 'virtio_dev_rx_async_submit_packed.constprop':
> ../lib/vhost/virtio_net.c:1838:27: note: 'buf_vec' declared here
>   1838 |         struct buf_vector buf_vec[BUF_VECTOR_MAX];
>        |                           ^~~~~~~
> cc1: all warnings being treated as errors
>
> Actually, there are eight places to see the same codes in the file
> lib/vhost/virtio_net.c, and all these `buf_vec` arraies are
> initialized by sub-function calls under various conditions.
>
> Although It's hard to understand why gcc just emits warning at one
> of the eight places, adding validity checks for array length is
> reasonable and can also fix the warning.
>
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> Signed-off-by: Min Zhou <zhoumin@loongson.cn>
> ---
>   lib/vhost/virtio_net.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/lib/vhost/virtio_net.c b/lib/vhost/virtio_net.c
> index 35fa4670fd..99233f1759 100644
> --- a/lib/vhost/virtio_net.c
> +++ b/lib/vhost/virtio_net.c
> @@ -1153,7 +1153,7 @@ mbuf_to_desc(struct virtio_net *dev, struct vhost_virtqueue *vq,
>   	struct virtio_net_hdr_mrg_rxbuf tmp_hdr, *hdr = NULL;
>   	struct vhost_async *async = vq->async;
>   
> -	if (unlikely(m == NULL))
> +	if (unlikely(m == NULL || nr_vec == 0))
>   		return -1;
>   
>   	buf_addr = buf_vec[vec_idx].buf_addr;
> @@ -2673,6 +2673,9 @@ desc_to_mbuf(struct virtio_net *dev, struct vhost_virtqueue *vq,
>   	struct vhost_async *async = vq->async;
>   	struct async_inflight_info *pkts_info;
>   
> +	if (unlikely(nr_vec == 0))
> +		return -1;
> +
>   	buf_addr = buf_vec[vec_idx].buf_addr;
>   	buf_iova = buf_vec[vec_idx].buf_iova;
>   	buf_len = buf_vec[vec_idx].buf_len;

--
Thanks,
Min Zhou


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] vhost: fix build
  2022-08-29  8:29 [PATCH v2] vhost: fix build Min Zhou
  2022-09-01  0:06 ` zhoumin
@ 2022-09-09  9:07 ` zhoumin
  2022-09-26  2:57 ` Xia, Chenbo
  2 siblings, 0 replies; 8+ messages in thread
From: zhoumin @ 2022-09-09  9:07 UTC (permalink / raw)
  To: david.marchand, maxime.coquelin, chenbo.xia; +Cc: dev, maobibo

Kindly ping.

Any feedback is appreciated.


On 2022年08月29日 16:29, Min Zhou wrote:
> On CentOS 8 or Debian 10.4 systems using gcc 12.1 to cross
> compile DPDK, gcc shows a following warning which will cause
> build to fail when build is run with -werror:
>
> In function 'mbuf_to_desc',
>      inlined from 'vhost_enqueue_async_packed' at ../lib/vhost/virtio_net.c:1826:6,
>      inlined from 'virtio_dev_rx_async_packed' at ../lib/vhost/virtio_net.c:1840:6,
>      inlined from 'virtio_dev_rx_async_submit_packed.constprop' at ../lib/vhost/virtio_net.c:1900:7:
> ../lib/vhost/virtio_net.c:1161:35: error: 'buf_vec[0].buf_len' may be used uninitialized [-Werror=maybe-uninitialized]
>   1161 |         buf_len = buf_vec[vec_idx].buf_len;
>        |                   ~~~~~~~~~~~~~~~~^~~~~~~~
> ../lib/vhost/virtio_net.c: In function 'virtio_dev_rx_async_submit_packed.constprop':
> ../lib/vhost/virtio_net.c:1838:27: note: 'buf_vec' declared here
>   1838 |         struct buf_vector buf_vec[BUF_VECTOR_MAX];
>        |                           ^~~~~~~
> cc1: all warnings being treated as errors
>
> Actually, there are eight places to see the same codes in the file
> lib/vhost/virtio_net.c, and all these `buf_vec` arraies are
> initialized by sub-function calls under various conditions.
>
> Although It's hard to understand why gcc just emits warning at one
> of the eight places, adding validity checks for array length is
> reasonable and can also fix the warning.
>
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> Signed-off-by: Min Zhou <zhoumin@loongson.cn>
> ---
>   lib/vhost/virtio_net.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/lib/vhost/virtio_net.c b/lib/vhost/virtio_net.c
> index 35fa4670fd..99233f1759 100644
> --- a/lib/vhost/virtio_net.c
> +++ b/lib/vhost/virtio_net.c
> @@ -1153,7 +1153,7 @@ mbuf_to_desc(struct virtio_net *dev, struct vhost_virtqueue *vq,
>   	struct virtio_net_hdr_mrg_rxbuf tmp_hdr, *hdr = NULL;
>   	struct vhost_async *async = vq->async;
>   
> -	if (unlikely(m == NULL))
> +	if (unlikely(m == NULL || nr_vec == 0))
>   		return -1;
>   
>   	buf_addr = buf_vec[vec_idx].buf_addr;
> @@ -2673,6 +2673,9 @@ desc_to_mbuf(struct virtio_net *dev, struct vhost_virtqueue *vq,
>   	struct vhost_async *async = vq->async;
>   	struct async_inflight_info *pkts_info;
>   
> +	if (unlikely(nr_vec == 0))
> +		return -1;
> +
>   	buf_addr = buf_vec[vec_idx].buf_addr;
>   	buf_iova = buf_vec[vec_idx].buf_iova;
>   	buf_len = buf_vec[vec_idx].buf_len;


^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [PATCH v2] vhost: fix build
  2022-08-29  8:29 [PATCH v2] vhost: fix build Min Zhou
  2022-09-01  0:06 ` zhoumin
  2022-09-09  9:07 ` zhoumin
@ 2022-09-26  2:57 ` Xia, Chenbo
  2022-09-26  3:25   ` zhoumin
  2 siblings, 1 reply; 8+ messages in thread
From: Xia, Chenbo @ 2022-09-26  2:57 UTC (permalink / raw)
  To: Min Zhou, david.marchand, maxime.coquelin; +Cc: dev, maobibo

Hi Min,

> -----Original Message-----
> From: Min Zhou <zhoumin@loongson.cn>
> Sent: Monday, August 29, 2022 4:29 PM
> To: david.marchand@redhat.com; maxime.coquelin@redhat.com; Xia, Chenbo
> <chenbo.xia@intel.com>; zhoumin@loongson.cn
> Cc: dev@dpdk.org; maobibo@loongson.cn
> Subject: [PATCH v2] vhost: fix build
> 
> On CentOS 8 or Debian 10.4 systems using gcc 12.1 to cross
> compile DPDK, gcc shows a following warning which will cause
> build to fail when build is run with -werror:
> 
> In function 'mbuf_to_desc',
>     inlined from 'vhost_enqueue_async_packed'
> at ../lib/vhost/virtio_net.c:1826:6,
>     inlined from 'virtio_dev_rx_async_packed'
> at ../lib/vhost/virtio_net.c:1840:6,
>     inlined from 'virtio_dev_rx_async_submit_packed.constprop'
> at ../lib/vhost/virtio_net.c:1900:7:
> ../lib/vhost/virtio_net.c:1161:35: error: 'buf_vec[0].buf_len' may be used
> uninitialized [-Werror=maybe-uninitialized]
>  1161 |         buf_len = buf_vec[vec_idx].buf_len;
>       |                   ~~~~~~~~~~~~~~~~^~~~~~~~
> ../lib/vhost/virtio_net.c: In function
> 'virtio_dev_rx_async_submit_packed.constprop':
> ../lib/vhost/virtio_net.c:1838:27: note: 'buf_vec' declared here
>  1838 |         struct buf_vector buf_vec[BUF_VECTOR_MAX];
>       |                           ^~~~~~~
> cc1: all warnings being treated as errors
> 
> Actually, there are eight places to see the same codes in the file
> lib/vhost/virtio_net.c, and all these `buf_vec` arraies are
> initialized by sub-function calls under various conditions.
> 
> Although It's hard to understand why gcc just emits warning at one
> of the eight places, adding validity checks for array length is
> reasonable and can also fix the warning.
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> Signed-off-by: Min Zhou <zhoumin@loongson.cn>
> ---
>  lib/vhost/virtio_net.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)

Just want you to know that your patch is still pending because by accident
your fix is almost the same as a previous patch that fixes a real issue but
that patch is still in progress:

http://patchwork.dpdk.org/project/dpdk/patch/20220802004938.23670-2-cfontana@suse.de/

Thanks,
Chenbo

> 
> diff --git a/lib/vhost/virtio_net.c b/lib/vhost/virtio_net.c
> index 35fa4670fd..99233f1759 100644
> --- a/lib/vhost/virtio_net.c
> +++ b/lib/vhost/virtio_net.c
> @@ -1153,7 +1153,7 @@ mbuf_to_desc(struct virtio_net *dev, struct
> vhost_virtqueue *vq,
>  	struct virtio_net_hdr_mrg_rxbuf tmp_hdr, *hdr = NULL;
>  	struct vhost_async *async = vq->async;
> 
> -	if (unlikely(m == NULL))
> +	if (unlikely(m == NULL || nr_vec == 0))
>  		return -1;
> 
>  	buf_addr = buf_vec[vec_idx].buf_addr;
> @@ -2673,6 +2673,9 @@ desc_to_mbuf(struct virtio_net *dev, struct
> vhost_virtqueue *vq,
>  	struct vhost_async *async = vq->async;
>  	struct async_inflight_info *pkts_info;
> 
> +	if (unlikely(nr_vec == 0))
> +		return -1;
> +
>  	buf_addr = buf_vec[vec_idx].buf_addr;
>  	buf_iova = buf_vec[vec_idx].buf_iova;
>  	buf_len = buf_vec[vec_idx].buf_len;
> --
> 2.31.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] vhost: fix build
  2022-09-26  2:57 ` Xia, Chenbo
@ 2022-09-26  3:25   ` zhoumin
  2022-10-05 14:51     ` Maxime Coquelin
  0 siblings, 1 reply; 8+ messages in thread
From: zhoumin @ 2022-09-26  3:25 UTC (permalink / raw)
  To: Xia, Chenbo, david.marchand, maxime.coquelin; +Cc: dev, maobibo

Hi Chenbo,


On Mon, 26 Sep 2022, 10:57, Xia, Chenbo wrote:
> Hi Min,
>
>> -----Original Message-----
>> From: Min Zhou <zhoumin@loongson.cn>
>> Sent: Monday, August 29, 2022 4:29 PM
>> To: david.marchand@redhat.com; maxime.coquelin@redhat.com; Xia, Chenbo
>> <chenbo.xia@intel.com>; zhoumin@loongson.cn
>> Cc: dev@dpdk.org; maobibo@loongson.cn
>> Subject: [PATCH v2] vhost: fix build
>>
>> On CentOS 8 or Debian 10.4 systems using gcc 12.1 to cross
>> compile DPDK, gcc shows a following warning which will cause
>> build to fail when build is run with -werror:
>>
>> In function 'mbuf_to_desc',
>>      inlined from 'vhost_enqueue_async_packed'
>> at ../lib/vhost/virtio_net.c:1826:6,
>>      inlined from 'virtio_dev_rx_async_packed'
>> at ../lib/vhost/virtio_net.c:1840:6,
>>      inlined from 'virtio_dev_rx_async_submit_packed.constprop'
>> at ../lib/vhost/virtio_net.c:1900:7:
>> ../lib/vhost/virtio_net.c:1161:35: error: 'buf_vec[0].buf_len' may be used
>> uninitialized [-Werror=maybe-uninitialized]
>>   1161 |         buf_len = buf_vec[vec_idx].buf_len;
>>        |                   ~~~~~~~~~~~~~~~~^~~~~~~~
>> ../lib/vhost/virtio_net.c: In function
>> 'virtio_dev_rx_async_submit_packed.constprop':
>> ../lib/vhost/virtio_net.c:1838:27: note: 'buf_vec' declared here
>>   1838 |         struct buf_vector buf_vec[BUF_VECTOR_MAX];
>>        |                           ^~~~~~~
>> cc1: all warnings being treated as errors
>>
>> Actually, there are eight places to see the same codes in the file
>> lib/vhost/virtio_net.c, and all these `buf_vec` arraies are
>> initialized by sub-function calls under various conditions.
>>
>> Although It's hard to understand why gcc just emits warning at one
>> of the eight places, adding validity checks for array length is
>> reasonable and can also fix the warning.
>>
>> Signed-off-by: David Marchand <david.marchand@redhat.com>
>> Signed-off-by: Min Zhou <zhoumin@loongson.cn>
>> ---
>>   lib/vhost/virtio_net.c | 5 ++++-
>>   1 file changed, 4 insertions(+), 1 deletion(-)
> Just want you to know that your patch is still pending because by accident
> your fix is almost the same as a previous patch that fixes a real issue but
> that patch is still in progress:
>
> http://patchwork.dpdk.org/project/dpdk/patch/20220802004938.23670-2-cfontana@suse.de/
>
> Thanks,
> Chenbo

Thanks for your helpful reply.
I think I can drop this patch if the patch you mentioned above could be 
accepted.

Thanks,
Min Zhou
>> diff --git a/lib/vhost/virtio_net.c b/lib/vhost/virtio_net.c
>> index 35fa4670fd..99233f1759 100644
>> --- a/lib/vhost/virtio_net.c
>> +++ b/lib/vhost/virtio_net.c
>> @@ -1153,7 +1153,7 @@ mbuf_to_desc(struct virtio_net *dev, struct
>> vhost_virtqueue *vq,
>>   	struct virtio_net_hdr_mrg_rxbuf tmp_hdr, *hdr = NULL;
>>   	struct vhost_async *async = vq->async;
>>
>> -	if (unlikely(m == NULL))
>> +	if (unlikely(m == NULL || nr_vec == 0))
>>   		return -1;
>>
>>   	buf_addr = buf_vec[vec_idx].buf_addr;
>> @@ -2673,6 +2673,9 @@ desc_to_mbuf(struct virtio_net *dev, struct
>> vhost_virtqueue *vq,
>>   	struct vhost_async *async = vq->async;
>>   	struct async_inflight_info *pkts_info;
>>
>> +	if (unlikely(nr_vec == 0))
>> +		return -1;
>> +
>>   	buf_addr = buf_vec[vec_idx].buf_addr;
>>   	buf_iova = buf_vec[vec_idx].buf_iova;
>>   	buf_len = buf_vec[vec_idx].buf_len;
>> --
>> 2.31.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] vhost: fix build
  2022-09-26  3:25   ` zhoumin
@ 2022-10-05 14:51     ` Maxime Coquelin
  2022-10-06  3:56       ` zhoumin
  2022-10-06  3:57       ` zhoumin
  0 siblings, 2 replies; 8+ messages in thread
From: Maxime Coquelin @ 2022-10-05 14:51 UTC (permalink / raw)
  To: zhoumin, Xia, Chenbo, david.marchand; +Cc: dev, maobibo

Hi Min,

On 9/26/22 05:25, zhoumin wrote:
> Hi Chenbo,
> 
> 
> On Mon, 26 Sep 2022, 10:57, Xia, Chenbo wrote:
>> Hi Min,
>>
>>> -----Original Message-----
>>> From: Min Zhou <zhoumin@loongson.cn>
>>> Sent: Monday, August 29, 2022 4:29 PM
>>> To: david.marchand@redhat.com; maxime.coquelin@redhat.com; Xia, Chenbo
>>> <chenbo.xia@intel.com>; zhoumin@loongson.cn
>>> Cc: dev@dpdk.org; maobibo@loongson.cn
>>> Subject: [PATCH v2] vhost: fix build
>>>
>>> On CentOS 8 or Debian 10.4 systems using gcc 12.1 to cross
>>> compile DPDK, gcc shows a following warning which will cause
>>> build to fail when build is run with -werror:
>>>
>>> In function 'mbuf_to_desc',
>>>      inlined from 'vhost_enqueue_async_packed'
>>> at ../lib/vhost/virtio_net.c:1826:6,
>>>      inlined from 'virtio_dev_rx_async_packed'
>>> at ../lib/vhost/virtio_net.c:1840:6,
>>>      inlined from 'virtio_dev_rx_async_submit_packed.constprop'
>>> at ../lib/vhost/virtio_net.c:1900:7:
>>> ../lib/vhost/virtio_net.c:1161:35: error: 'buf_vec[0].buf_len' may be 
>>> used
>>> uninitialized [-Werror=maybe-uninitialized]
>>>   1161 |         buf_len = buf_vec[vec_idx].buf_len;
>>>        |                   ~~~~~~~~~~~~~~~~^~~~~~~~
>>> ../lib/vhost/virtio_net.c: In function
>>> 'virtio_dev_rx_async_submit_packed.constprop':
>>> ../lib/vhost/virtio_net.c:1838:27: note: 'buf_vec' declared here
>>>   1838 |         struct buf_vector buf_vec[BUF_VECTOR_MAX];
>>>        |                           ^~~~~~~
>>> cc1: all warnings being treated as errors
>>>
>>> Actually, there are eight places to see the same codes in the file
>>> lib/vhost/virtio_net.c, and all these `buf_vec` arraies are
>>> initialized by sub-function calls under various conditions.
>>>
>>> Although It's hard to understand why gcc just emits warning at one
>>> of the eight places, adding validity checks for array length is
>>> reasonable and can also fix the warning.
>>>
>>> Signed-off-by: David Marchand <david.marchand@redhat.com>
>>> Signed-off-by: Min Zhou <zhoumin@loongson.cn>
>>> ---
>>>   lib/vhost/virtio_net.c | 5 ++++-
>>>   1 file changed, 4 insertions(+), 1 deletion(-)
>> Just want you to know that your patch is still pending because by 
>> accident
>> your fix is almost the same as a previous patch that fixes a real 
>> issue but
>> that patch is still in progress:
>>
>> http://patchwork.dpdk.org/project/dpdk/patch/20220802004938.23670-2-cfontana@suse.de/
>>
>> Thanks,
>> Chenbo
> 
> Thanks for your helpful reply.
> I think I can drop this patch if the patch you mentioned above could be 
> accepted.

I just sent another patch that fixes your build issue without having to
add runtime checks:

http://patches.dpdk.org/project/dpdk/patch/20221005144859.70717-1-maxime.coquelin@redhat.com/

Thanks,
Maxime



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] vhost: fix build
  2022-10-05 14:51     ` Maxime Coquelin
@ 2022-10-06  3:56       ` zhoumin
  2022-10-06  3:57       ` zhoumin
  1 sibling, 0 replies; 8+ messages in thread
From: zhoumin @ 2022-10-06  3:56 UTC (permalink / raw)
  To: Maxime Coquelin, Xia, Chenbo, david.marchand; +Cc: dev, maobibo

Hi, Maxime,

Thanks a lot for your kind reply.

This patch is very useful and relieves the concerns in my heart.


On Wed, Oct 5, 2022 at 22:51, Maxime Coquelin wrote:

> Hi Min,
>
> On 9/26/22 05:25, zhoumin wrote:
>> Hi Chenbo,
>>
>>
>> On Mon, 26 Sep 2022, 10:57, Xia, Chenbo wrote:
>>> Hi Min,
>>>
>>>> -----Original Message-----
>>>> From: Min Zhou <zhoumin@loongson.cn>
>>>> Sent: Monday, August 29, 2022 4:29 PM
>>>> To: david.marchand@redhat.com; maxime.coquelin@redhat.com; Xia, Chenbo
>>>> <chenbo.xia@intel.com>; zhoumin@loongson.cn
>>>> Cc: dev@dpdk.org; maobibo@loongson.cn
>>>> Subject: [PATCH v2] vhost: fix build
>>>>
>>>> On CentOS 8 or Debian 10.4 systems using gcc 12.1 to cross
>>>> compile DPDK, gcc shows a following warning which will cause
>>>> build to fail when build is run with -werror:
>>>>
>>>> In function 'mbuf_to_desc',
>>>>      inlined from 'vhost_enqueue_async_packed'
>>>> at ../lib/vhost/virtio_net.c:1826:6,
>>>>      inlined from 'virtio_dev_rx_async_packed'
>>>> at ../lib/vhost/virtio_net.c:1840:6,
>>>>      inlined from 'virtio_dev_rx_async_submit_packed.constprop'
>>>> at ../lib/vhost/virtio_net.c:1900:7:
>>>> ../lib/vhost/virtio_net.c:1161:35: error: 'buf_vec[0].buf_len' may 
>>>> be used
>>>> uninitialized [-Werror=maybe-uninitialized]
>>>>   1161 |         buf_len = buf_vec[vec_idx].buf_len;
>>>>        |                   ~~~~~~~~~~~~~~~~^~~~~~~~
>>>> ../lib/vhost/virtio_net.c: In function
>>>> 'virtio_dev_rx_async_submit_packed.constprop':
>>>> ../lib/vhost/virtio_net.c:1838:27: note: 'buf_vec' declared here
>>>>   1838 |         struct buf_vector buf_vec[BUF_VECTOR_MAX];
>>>>        |                           ^~~~~~~
>>>> cc1: all warnings being treated as errors
>>>>
>>>> Actually, there are eight places to see the same codes in the file
>>>> lib/vhost/virtio_net.c, and all these `buf_vec` arraies are
>>>> initialized by sub-function calls under various conditions.
>>>>
>>>> Although It's hard to understand why gcc just emits warning at one
>>>> of the eight places, adding validity checks for array length is
>>>> reasonable and can also fix the warning.
>>>>
>>>> Signed-off-by: David Marchand <david.marchand@redhat.com>
>>>> Signed-off-by: Min Zhou <zhoumin@loongson.cn>
>>>> ---
>>>>   lib/vhost/virtio_net.c | 5 ++++-
>>>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>> Just want you to know that your patch is still pending because by 
>>> accident
>>> your fix is almost the same as a previous patch that fixes a real 
>>> issue but
>>> that patch is still in progress:
>>>
>>> http://patchwork.dpdk.org/project/dpdk/patch/20220802004938.23670-2-cfontana@suse.de/ 
>>>
>>>
>>> Thanks,
>>> Chenbo
>>
>> Thanks for your helpful reply.
>> I think I can drop this patch if the patch you mentioned above could 
>> be accepted.
>
> I just sent another patch that fixes your build issue without having to
> add runtime checks:
>
> http://patches.dpdk.org/project/dpdk/patch/20221005144859.70717-1-maxime.coquelin@redhat.com/ 
>
>

OK, thanks. I have read the whole thread, which is a good job. I'd like 
thank you all for your hard work.


> Thanks,
> Maxime
>


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] vhost: fix build
  2022-10-05 14:51     ` Maxime Coquelin
  2022-10-06  3:56       ` zhoumin
@ 2022-10-06  3:57       ` zhoumin
  1 sibling, 0 replies; 8+ messages in thread
From: zhoumin @ 2022-10-06  3:57 UTC (permalink / raw)
  To: Maxime Coquelin, Xia, Chenbo, david.marchand; +Cc: dev, maobibo

Hi, Maxime,

Thanks a lot for your kind reply.

This patch is very useful and relieves the concerns in my heart.


On Wed, Oct 5, 2022 at 22:51, Maxime Coquelin wrote:

> Hi Min,
>
> On 9/26/22 05:25, zhoumin wrote:
>> Hi Chenbo,
>>
>>
>> On Mon, 26 Sep 2022, 10:57, Xia, Chenbo wrote:
>>> Hi Min,
>>>
>>>> -----Original Message-----
>>>> From: Min Zhou <zhoumin@loongson.cn>
>>>> Sent: Monday, August 29, 2022 4:29 PM
>>>> To: david.marchand@redhat.com; maxime.coquelin@redhat.com; Xia, Chenbo
>>>> <chenbo.xia@intel.com>; zhoumin@loongson.cn
>>>> Cc: dev@dpdk.org; maobibo@loongson.cn
>>>> Subject: [PATCH v2] vhost: fix build
>>>>
>>>> On CentOS 8 or Debian 10.4 systems using gcc 12.1 to cross
>>>> compile DPDK, gcc shows a following warning which will cause
>>>> build to fail when build is run with -werror:
>>>>
>>>> In function 'mbuf_to_desc',
>>>>      inlined from 'vhost_enqueue_async_packed'
>>>> at ../lib/vhost/virtio_net.c:1826:6,
>>>>      inlined from 'virtio_dev_rx_async_packed'
>>>> at ../lib/vhost/virtio_net.c:1840:6,
>>>>      inlined from 'virtio_dev_rx_async_submit_packed.constprop'
>>>> at ../lib/vhost/virtio_net.c:1900:7:
>>>> ../lib/vhost/virtio_net.c:1161:35: error: 'buf_vec[0].buf_len' may 
>>>> be used
>>>> uninitialized [-Werror=maybe-uninitialized]
>>>>   1161 |         buf_len = buf_vec[vec_idx].buf_len;
>>>>        |                   ~~~~~~~~~~~~~~~~^~~~~~~~
>>>> ../lib/vhost/virtio_net.c: In function
>>>> 'virtio_dev_rx_async_submit_packed.constprop':
>>>> ../lib/vhost/virtio_net.c:1838:27: note: 'buf_vec' declared here
>>>>   1838 |         struct buf_vector buf_vec[BUF_VECTOR_MAX];
>>>>        |                           ^~~~~~~
>>>> cc1: all warnings being treated as errors
>>>>
>>>> Actually, there are eight places to see the same codes in the file
>>>> lib/vhost/virtio_net.c, and all these `buf_vec` arraies are
>>>> initialized by sub-function calls under various conditions.
>>>>
>>>> Although It's hard to understand why gcc just emits warning at one
>>>> of the eight places, adding validity checks for array length is
>>>> reasonable and can also fix the warning.
>>>>
>>>> Signed-off-by: David Marchand <david.marchand@redhat.com>
>>>> Signed-off-by: Min Zhou <zhoumin@loongson.cn>
>>>> ---
>>>>   lib/vhost/virtio_net.c | 5 ++++-
>>>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>> Just want you to know that your patch is still pending because by 
>>> accident
>>> your fix is almost the same as a previous patch that fixes a real 
>>> issue but
>>> that patch is still in progress:
>>>
>>> http://patchwork.dpdk.org/project/dpdk/patch/20220802004938.23670-2-cfontana@suse.de/ 
>>>
>>>
>>> Thanks,
>>> Chenbo
>>
>> Thanks for your helpful reply.
>> I think I can drop this patch if the patch you mentioned above could 
>> be accepted.
>
> I just sent another patch that fixes your build issue without having to
> add runtime checks:
>
> http://patches.dpdk.org/project/dpdk/patch/20221005144859.70717-1-maxime.coquelin@redhat.com/ 
>
>

OK, thanks. I have read the whole thread, which is a good job. I'd like 
thank you all for your hard work.


> Thanks,
> Maxime
>
Thanks,

Min Zhou


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2022-10-06  3:57 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-29  8:29 [PATCH v2] vhost: fix build Min Zhou
2022-09-01  0:06 ` zhoumin
2022-09-09  9:07 ` zhoumin
2022-09-26  2:57 ` Xia, Chenbo
2022-09-26  3:25   ` zhoumin
2022-10-05 14:51     ` Maxime Coquelin
2022-10-06  3:56       ` zhoumin
2022-10-06  3:57       ` zhoumin

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).