patches for DPDK stable branches
 help / color / mirror / Atom feed
* [PATCH v2] vhost: fix compilation issue in async path
@ 2022-10-05 15:11 Maxime Coquelin
  2022-10-05 15:36 ` Maxime Coquelin
  2022-10-05 20:35 ` [PATCH v3] vhost: fix build issues with GCC 12 Maxime Coquelin
  0 siblings, 2 replies; 9+ messages in thread
From: Maxime Coquelin @ 2022-10-05 15:11 UTC (permalink / raw)
  To: dev, cheng1.jiang, chenbo.xia, zhoumin, david.marchand, thomas
  Cc: Maxime Coquelin, stable

This patch fixes a compilation issue met with GCC 12 on
LoongArch64:

In function ‘mbuf_to_desc’,
    inlined from ‘vhost_enqueue_async_packed’ at ../../../dpdk/lib/vhost/virtio_net.c:1822:6,
    inlined from ‘virtio_dev_rx_async_packed’ at ../../../dpdk/lib/vhost/virtio_net.c:1836:6,
    inlined from ‘virtio_dev_rx_async_submit_packed’ at ../../../dpdk/lib/vhost/virtio_net.c:1895:7:
../../../dpdk/lib/vhost/virtio_net.c:1159:18: error: ‘buf_vec[0].buf_addr’ may be used uninitialized [-Werror=maybe-uninitialized]
 1159 |         buf_addr = buf_vec[vec_idx].buf_addr;
      |         ~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~
../../../dpdk/lib/vhost/virtio_net.c: In function ‘virtio_dev_rx_async_submit_packed’:
../../../dpdk/lib/vhost/virtio_net.c:1834:27: note: ‘buf_vec’ declared here
 1834 |         struct buf_vector buf_vec[BUF_VECTOR_MAX];
      |                           ^~~~~~~

It happens because the compiler assumes that 'size'
variable in vhost_enqueue_async_packed could wrap to 0 since
'size' is uint32_t and pkt->pkt_len too.

In practice, it would never happen since 'pkt->pkt_len' is
unlikely to be close to UINT32_MAX, but let's just change
'size' to uint64_t to make the compiler happy without
having to add runtime checks.

Fixes: 873e8dad6f49 ("vhost: support packed ring in async datapath")
Cc: stable@dpdk.org

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 lib/vhost/virtio_net.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/vhost/virtio_net.c b/lib/vhost/virtio_net.c
index 8f4d0f0502..b86fb26040 100644
--- a/lib/vhost/virtio_net.c
+++ b/lib/vhost/virtio_net.c
@@ -1780,7 +1780,7 @@ vhost_enqueue_async_packed(struct virtio_net *dev,
 	uint16_t buf_id = 0;
 	uint32_t len = 0;
 	uint16_t desc_count = 0;
-	uint32_t size = pkt->pkt_len + sizeof(struct virtio_net_hdr_mrg_rxbuf);
+	uint64_t size = pkt->pkt_len + sizeof(struct virtio_net_hdr_mrg_rxbuf);
 	uint32_t buffer_len[vq->size];
 	uint16_t buffer_buf_id[vq->size];
 	uint16_t buffer_desc_count[vq->size];
-- 
2.37.3


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

* Re: [PATCH v2] vhost: fix compilation issue in async path
  2022-10-05 15:11 [PATCH v2] vhost: fix compilation issue in async path Maxime Coquelin
@ 2022-10-05 15:36 ` Maxime Coquelin
  2022-10-05 16:34   ` David Marchand
  2022-10-05 20:35 ` [PATCH v3] vhost: fix build issues with GCC 12 Maxime Coquelin
  1 sibling, 1 reply; 9+ messages in thread
From: Maxime Coquelin @ 2022-10-05 15:36 UTC (permalink / raw)
  To: dev, cheng1.jiang, chenbo.xia, zhoumin, david.marchand, thomas; +Cc: stable



On 10/5/22 17:11, Maxime Coquelin wrote:
> This patch fixes a compilation issue met with GCC 12 on
> LoongArch64:
> 
> In function ‘mbuf_to_desc’,
>      inlined from ‘vhost_enqueue_async_packed’ at ../../../dpdk/lib/vhost/virtio_net.c:1822:6,
>      inlined from ‘virtio_dev_rx_async_packed’ at ../../../dpdk/lib/vhost/virtio_net.c:1836:6,
>      inlined from ‘virtio_dev_rx_async_submit_packed’ at ../../../dpdk/lib/vhost/virtio_net.c:1895:7:
> ../../../dpdk/lib/vhost/virtio_net.c:1159:18: error: ‘buf_vec[0].buf_addr’ may be used uninitialized [-Werror=maybe-uninitialized]
>   1159 |         buf_addr = buf_vec[vec_idx].buf_addr;
>        |         ~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~
> ../../../dpdk/lib/vhost/virtio_net.c: In function ‘virtio_dev_rx_async_submit_packed’:
> ../../../dpdk/lib/vhost/virtio_net.c:1834:27: note: ‘buf_vec’ declared here
>   1834 |         struct buf_vector buf_vec[BUF_VECTOR_MAX];
>        |                           ^~~~~~~
> 
> It happens because the compiler assumes that 'size'
> variable in vhost_enqueue_async_packed could wrap to 0 since
> 'size' is uint32_t and pkt->pkt_len too.
> 
> In practice, it would never happen since 'pkt->pkt_len' is
> unlikely to be close to UINT32_MAX, but let's just change
> 'size' to uint64_t to make the compiler happy without
> having to add runtime checks.
> 
> Fixes: 873e8dad6f49 ("vhost: support packed ring in async datapath")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Forgot to report the R-by from David on v1:
Reviewed-by: David Marchand <david.marchand@redhat.com>

> ---
>   lib/vhost/virtio_net.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/vhost/virtio_net.c b/lib/vhost/virtio_net.c
> index 8f4d0f0502..b86fb26040 100644
> --- a/lib/vhost/virtio_net.c
> +++ b/lib/vhost/virtio_net.c
> @@ -1780,7 +1780,7 @@ vhost_enqueue_async_packed(struct virtio_net *dev,
>   	uint16_t buf_id = 0;
>   	uint32_t len = 0;
>   	uint16_t desc_count = 0;
> -	uint32_t size = pkt->pkt_len + sizeof(struct virtio_net_hdr_mrg_rxbuf);
> +	uint64_t size = pkt->pkt_len + sizeof(struct virtio_net_hdr_mrg_rxbuf);
>   	uint32_t buffer_len[vq->size];
>   	uint16_t buffer_buf_id[vq->size];
>   	uint16_t buffer_desc_count[vq->size];


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

* Re: [PATCH v2] vhost: fix compilation issue in async path
  2022-10-05 15:36 ` Maxime Coquelin
@ 2022-10-05 16:34   ` David Marchand
  0 siblings, 0 replies; 9+ messages in thread
From: David Marchand @ 2022-10-05 16:34 UTC (permalink / raw)
  To: Maxime Coquelin; +Cc: dev, cheng1.jiang, chenbo.xia, zhoumin, thomas, stable

On Wed, Oct 5, 2022 at 5:36 PM Maxime Coquelin
<maxime.coquelin@redhat.com> wrote:
> On 10/5/22 17:11, Maxime Coquelin wrote:
> > This patch fixes a compilation issue met with GCC 12 on
> > LoongArch64:
> >
> > In function ‘mbuf_to_desc’,
> >      inlined from ‘vhost_enqueue_async_packed’ at ../../../dpdk/lib/vhost/virtio_net.c:1822:6,
> >      inlined from ‘virtio_dev_rx_async_packed’ at ../../../dpdk/lib/vhost/virtio_net.c:1836:6,
> >      inlined from ‘virtio_dev_rx_async_submit_packed’ at ../../../dpdk/lib/vhost/virtio_net.c:1895:7:
> > ../../../dpdk/lib/vhost/virtio_net.c:1159:18: error: ‘buf_vec[0].buf_addr’ may be used uninitialized [-Werror=maybe-uninitialized]
> >   1159 |         buf_addr = buf_vec[vec_idx].buf_addr;
> >        |         ~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~
> > ../../../dpdk/lib/vhost/virtio_net.c: In function ‘virtio_dev_rx_async_submit_packed’:
> > ../../../dpdk/lib/vhost/virtio_net.c:1834:27: note: ‘buf_vec’ declared here
> >   1834 |         struct buf_vector buf_vec[BUF_VECTOR_MAX];
> >        |                           ^~~~~~~
> >
> > It happens because the compiler assumes that 'size'
> > variable in vhost_enqueue_async_packed could wrap to 0 since
> > 'size' is uint32_t and pkt->pkt_len too.
> >
> > In practice, it would never happen since 'pkt->pkt_len' is
> > unlikely to be close to UINT32_MAX, but let's just change
> > 'size' to uint64_t to make the compiler happy without
> > having to add runtime checks.
> >
> > Fixes: 873e8dad6f49 ("vhost: support packed ring in async datapath")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>
> Forgot to report the R-by from David on v1:
> Reviewed-by: David Marchand <david.marchand@redhat.com>

After retesting with other arches, I have an error for cross compiling
ARM, on another part of the code.

C compiler for the host machine: ccache aarch64-linux-gnu-gcc (gcc
12.0.1 "aarch64-linux-gnu-gcc (GCC) 12.0.1 20220205 (experimental)
[master revision f49b8d25b1ff96e9cd09326666cc510b3a3accde]")

Same symptom:

[251/2797] Compiling C object lib/librte_vhost.a.p/vhost_virtio_net.c.o
FAILED: lib/librte_vhost.a.p/vhost_virtio_net.c.o
ccache aarch64-linux-gnu-gcc -Ilib/librte_vhost.a.p -Ilib
-I../../../git/pub/dpdk.org/main/lib -Ilib/vhost
-I../../../git/pub/dpdk.org/main/lib/vhost -I.
-I../../../git/pub/dpdk.org/main -Iconfig
-I../../../git/pub/dpdk.org/main/config -Ilib/eal/include
-I../../../git/pub/dpdk.org/main/lib/eal/include
-Ilib/eal/linux/include
-I../../../git/pub/dpdk.org/main/lib/eal/linux/include
-Ilib/eal/arm/include
-I../../../git/pub/dpdk.org/main/lib/eal/arm/include -Ilib/eal/common
-I../../../git/pub/dpdk.org/main/lib/eal/common -Ilib/eal
-I../../../git/pub/dpdk.org/main/lib/eal -Ilib/kvargs
-I../../../git/pub/dpdk.org/main/lib/kvargs -Ilib/metrics
-I../../../git/pub/dpdk.org/main/lib/metrics -Ilib/telemetry
-I../../../git/pub/dpdk.org/main/lib/telemetry -Ilib/ethdev
-I../../../git/pub/dpdk.org/main/lib/ethdev -Ilib/net
-I../../../git/pub/dpdk.org/main/lib/net -Ilib/mbuf
-I../../../git/pub/dpdk.org/main/lib/mbuf -Ilib/mempool
-I../../../git/pub/dpdk.org/main/lib/mempool -Ilib/ring
-I../../../git/pub/dpdk.org/main/lib/ring -Ilib/meter
-I../../../git/pub/dpdk.org/main/lib/meter -Ilib/cryptodev
-I../../../git/pub/dpdk.org/main/lib/cryptodev -Ilib/rcu
-I../../../git/pub/dpdk.org/main/lib/rcu -Ilib/hash
-I../../../git/pub/dpdk.org/main/lib/hash -Ilib/pci
-I../../../git/pub/dpdk.org/main/lib/pci -Ilib/dmadev
-I../../../git/pub/dpdk.org/main/lib/dmadev -fdiagnostics-color=always
-D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wextra -Werror -O2 -g
-include rte_config.h -Wcast-qual -Wdeprecated -Wformat
-Wformat-nonliteral -Wformat-security -Wmissing-declarations
-Wmissing-prototypes -Wnested-externs -Wold-style-definition
-Wpointer-arith -Wsign-compare -Wstrict-prototypes -Wundef
-Wwrite-strings -Wno-address-of-packed-member -Wno-packed-not-aligned
-Wno-missing-field-initializers -Wno-zero-length-bounds -D_GNU_SOURCE
-fPIC -march=armv8-a+crc -moutline-atomics -DALLOW_EXPERIMENTAL_API
-DALLOW_INTERNAL_API -Wno-format-truncation -DVHOST_GCC_UNROLL_PRAGMA
-fno-strict-aliasing -DRTE_LOG_DEFAULT_LOGTYPE=lib.vhost -MD -MQ
lib/librte_vhost.a.p/vhost_virtio_net.c.o -MF
lib/librte_vhost.a.p/vhost_virtio_net.c.o.d -o
lib/librte_vhost.a.p/vhost_virtio_net.c.o -c
../../../git/pub/dpdk.org/main/lib/vhost/virtio_net.c
In function ‘mbuf_to_desc’,
    inlined from ‘vhost_enqueue_single_packed’ at
../../../git/pub/dpdk.org/main/lib/vhost/virtio_net.c:1321:6,
    inlined from ‘virtio_dev_rx_single_packed’ at
../../../git/pub/dpdk.org/main/lib/vhost/virtio_net.c:1506:6,
    inlined from ‘virtio_dev_rx_packed’ at
../../../git/pub/dpdk.org/main/lib/vhost/virtio_net.c:1540:7:
../../../git/pub/dpdk.org/main/lib/vhost/virtio_net.c:1159:18: error:
‘buf_vec[0].buf_addr’ may be used uninitialized
[-Werror=maybe-uninitialized]
 1159 |         buf_addr = buf_vec[vec_idx].buf_addr;
      |         ~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~
../../../git/pub/dpdk.org/main/lib/vhost/virtio_net.c: In function
‘virtio_dev_rx_packed’:
../../../git/pub/dpdk.org/main/lib/vhost/virtio_net.c:1503:27: note:
‘buf_vec’ declared here
 1503 |         struct buf_vector buf_vec[BUF_VECTOR_MAX];
      |                           ^~~~~~~
In function ‘mbuf_to_desc’,
    inlined from ‘vhost_enqueue_single_packed’ at
../../../git/pub/dpdk.org/main/lib/vhost/virtio_net.c:1321:6,
    inlined from ‘virtio_dev_rx_single_packed’ at
../../../git/pub/dpdk.org/main/lib/vhost/virtio_net.c:1506:6,
    inlined from ‘virtio_dev_rx_packed’ at
../../../git/pub/dpdk.org/main/lib/vhost/virtio_net.c:1540:7:
../../../git/pub/dpdk.org/main/lib/vhost/virtio_net.c:1160:18: error:
‘buf_vec[0].buf_iova’ may be used uninitialized
[-Werror=maybe-uninitialized]
 1160 |         buf_iova = buf_vec[vec_idx].buf_iova;
      |         ~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~
../../../git/pub/dpdk.org/main/lib/vhost/virtio_net.c: In function
‘virtio_dev_rx_packed’:
../../../git/pub/dpdk.org/main/lib/vhost/virtio_net.c:1503:27: note:
‘buf_vec’ declared here
 1503 |         struct buf_vector buf_vec[BUF_VECTOR_MAX];
      |                           ^~~~~~~
In function ‘mbuf_to_desc’,
    inlined from ‘vhost_enqueue_single_packed’ at
../../../git/pub/dpdk.org/main/lib/vhost/virtio_net.c:1321:6,
    inlined from ‘virtio_dev_rx_single_packed’ at
../../../git/pub/dpdk.org/main/lib/vhost/virtio_net.c:1506:6,
    inlined from ‘virtio_dev_rx_packed’ at
../../../git/pub/dpdk.org/main/lib/vhost/virtio_net.c:1540:7:
../../../git/pub/dpdk.org/main/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;
      |                   ~~~~~~~~~~~~~~~~^~~~~~~~
../../../git/pub/dpdk.org/main/lib/vhost/virtio_net.c: In function
‘virtio_dev_rx_packed’:
../../../git/pub/dpdk.org/main/lib/vhost/virtio_net.c:1503:27: note:
‘buf_vec’ declared here
 1503 |         struct buf_vector buf_vec[BUF_VECTOR_MAX];
      |                           ^~~~~~~
cc1: all warnings being treated as errors


And same fix:

@@ -1277,7 +1277,7 @@ vhost_enqueue_single_packed(struct virtio_net *dev,
        uint16_t buf_id = 0;
        uint32_t len = 0;
        uint16_t desc_count;
-       uint32_t size = pkt->pkt_len + sizeof(struct virtio_net_hdr_mrg_rxbuf);
+       uint64_t size = pkt->pkt_len + sizeof(struct virtio_net_hdr_mrg_rxbuf);
        uint16_t num_buffers = 0;


        uint32_t buffer_len[vq->size];
        uint16_t buffer_buf_id[vq->size];


I suspect we may need some code inspection.
I see two other locations with the same pattern.


-- 
David Marchand


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

* [PATCH v3] vhost: fix build issues with GCC 12
  2022-10-05 15:11 [PATCH v2] vhost: fix compilation issue in async path Maxime Coquelin
  2022-10-05 15:36 ` Maxime Coquelin
@ 2022-10-05 20:35 ` Maxime Coquelin
  2022-10-06  0:01   ` Stephen Hemminger
  2022-10-06 10:21   ` Thomas Monjalon
  1 sibling, 2 replies; 9+ messages in thread
From: Maxime Coquelin @ 2022-10-05 20:35 UTC (permalink / raw)
  To: dev, cheng1.jiang, chenbo.xia, zhoumin, david.marchand, thomas
  Cc: Maxime Coquelin, stable

This patch fixes a compilation issue met with GCC 12 on
LoongArch64:

In function ‘mbuf_to_desc’,
    inlined from ‘vhost_enqueue_async_packed’ at ../../../dpdk/lib/vhost/virtio_net.c:1822:6,
    inlined from ‘virtio_dev_rx_async_packed’ at ../../../dpdk/lib/vhost/virtio_net.c:1836:6,
    inlined from ‘virtio_dev_rx_async_submit_packed’ at ../../../dpdk/lib/vhost/virtio_net.c:1895:7:
../../../dpdk/lib/vhost/virtio_net.c:1159:18: error: ‘buf_vec[0].buf_addr’ may be used uninitialized [-Werror=maybe-uninitialized]
 1159 |         buf_addr = buf_vec[vec_idx].buf_addr;
      |         ~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~
../../../dpdk/lib/vhost/virtio_net.c: In function ‘virtio_dev_rx_async_submit_packed’:
../../../dpdk/lib/vhost/virtio_net.c:1834:27: note: ‘buf_vec’ declared here
 1834 |         struct buf_vector buf_vec[BUF_VECTOR_MAX];
      |                           ^~~~~~~

It happens because the compiler assumes that 'size'
variable in vhost_enqueue_async_packed could wrap to 0 since
'size' is uint32_t and pkt->pkt_len too.

In practice, it would never happen since 'pkt->pkt_len' is
unlikely to be close to UINT32_MAX, but let's just change
'size' to uint64_t to make the compiler happy without
having to add runtime checks.

This patch also fixes similar patterns in three other
places, including one that also produces similar build
issue on ARM64 in vhost_enqueue_single_packed().

Fixes: 873e8dad6f49 ("vhost: support packed ring in async datapath")
Cc: stable@dpdk.org

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
- Fix similar issue in 3 other places (David)


 lib/vhost/virtio_net.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/lib/vhost/virtio_net.c b/lib/vhost/virtio_net.c
index 8f4d0f0502..6b4a062df3 100644
--- a/lib/vhost/virtio_net.c
+++ b/lib/vhost/virtio_net.c
@@ -787,7 +787,7 @@ fill_vec_buf_split(struct virtio_net *dev, struct vhost_virtqueue *vq,
  */
 static inline int
 reserve_avail_buf_split(struct virtio_net *dev, struct vhost_virtqueue *vq,
-				uint32_t size, struct buf_vector *buf_vec,
+				uint64_t size, struct buf_vector *buf_vec,
 				uint16_t *num_buffers, uint16_t avail_head,
 				uint16_t *nr_vec)
 {
@@ -1277,7 +1277,7 @@ vhost_enqueue_single_packed(struct virtio_net *dev,
 	uint16_t buf_id = 0;
 	uint32_t len = 0;
 	uint16_t desc_count;
-	uint32_t size = pkt->pkt_len + sizeof(struct virtio_net_hdr_mrg_rxbuf);
+	uint64_t size = pkt->pkt_len + sizeof(struct virtio_net_hdr_mrg_rxbuf);
 	uint16_t num_buffers = 0;
 	uint32_t buffer_len[vq->size];
 	uint16_t buffer_buf_id[vq->size];
@@ -1345,7 +1345,7 @@ virtio_dev_rx_split(struct virtio_net *dev, struct vhost_virtqueue *vq,
 	rte_prefetch0(&vq->avail->ring[vq->last_avail_idx & (vq->size - 1)]);
 
 	for (pkt_idx = 0; pkt_idx < count; pkt_idx++) {
-		uint32_t pkt_len = pkts[pkt_idx]->pkt_len + dev->vhost_hlen;
+		uint64_t pkt_len = pkts[pkt_idx]->pkt_len + dev->vhost_hlen;
 		uint16_t nr_vec = 0;
 
 		if (unlikely(reserve_avail_buf_split(dev, vq,
@@ -1689,7 +1689,7 @@ virtio_dev_rx_async_submit_split(struct virtio_net *dev, struct vhost_virtqueue
 	async_iter_reset(async);
 
 	for (pkt_idx = 0; pkt_idx < count; pkt_idx++) {
-		uint32_t pkt_len = pkts[pkt_idx]->pkt_len + dev->vhost_hlen;
+		uint64_t pkt_len = pkts[pkt_idx]->pkt_len + dev->vhost_hlen;
 		uint16_t nr_vec = 0;
 
 		if (unlikely(reserve_avail_buf_split(dev, vq, pkt_len, buf_vec,
@@ -1780,7 +1780,7 @@ vhost_enqueue_async_packed(struct virtio_net *dev,
 	uint16_t buf_id = 0;
 	uint32_t len = 0;
 	uint16_t desc_count = 0;
-	uint32_t size = pkt->pkt_len + sizeof(struct virtio_net_hdr_mrg_rxbuf);
+	uint64_t size = pkt->pkt_len + sizeof(struct virtio_net_hdr_mrg_rxbuf);
 	uint32_t buffer_len[vq->size];
 	uint16_t buffer_buf_id[vq->size];
 	uint16_t buffer_desc_count[vq->size];
-- 
2.37.3


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

* Re: [PATCH v3] vhost: fix build issues with GCC 12
  2022-10-05 20:35 ` [PATCH v3] vhost: fix build issues with GCC 12 Maxime Coquelin
@ 2022-10-06  0:01   ` Stephen Hemminger
  2022-10-06  7:51     ` Maxime Coquelin
  2022-10-06 10:21   ` Thomas Monjalon
  1 sibling, 1 reply; 9+ messages in thread
From: Stephen Hemminger @ 2022-10-06  0:01 UTC (permalink / raw)
  To: Maxime Coquelin
  Cc: dev, cheng1.jiang, chenbo.xia, zhoumin, david.marchand, thomas, stable

On Wed,  5 Oct 2022 22:35:24 +0200
Maxime Coquelin <maxime.coquelin@redhat.com> wrote:

> In practice, it would never happen since 'pkt->pkt_len' is
> unlikely to be close to UINT32_MAX, but let's just change
> 'size' to uint64_t to make the compiler happy without
> having to add runtime checks.


Would the standard typedef size_t work since that is what sizeof() returns.

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

* Re: [PATCH v3] vhost: fix build issues with GCC 12
  2022-10-06  0:01   ` Stephen Hemminger
@ 2022-10-06  7:51     ` Maxime Coquelin
  2022-10-06  8:22       ` [EXT] " Amit Prakash Shukla
  0 siblings, 1 reply; 9+ messages in thread
From: Maxime Coquelin @ 2022-10-06  7:51 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: dev, cheng1.jiang, chenbo.xia, zhoumin, david.marchand, thomas, stable

Hi Stephen,

On 10/6/22 02:01, Stephen Hemminger wrote:
> On Wed,  5 Oct 2022 22:35:24 +0200
> Maxime Coquelin <maxime.coquelin@redhat.com> wrote:
> 
>> In practice, it would never happen since 'pkt->pkt_len' is
>> unlikely to be close to UINT32_MAX, but let's just change
>> 'size' to uint64_t to make the compiler happy without
>> having to add runtime checks.
> 
> 
> Would the standard typedef size_t work since that is what sizeof() returns.
> 

I'm not sure it would not create issues on 32 bits architectures build
given size_t is 32bits in this case if I'm not mistaken, as it adds
something to a u32, so it could wrap to 0.


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

* RE: [EXT] Re: [PATCH v3] vhost: fix build issues with GCC 12
  2022-10-06  7:51     ` Maxime Coquelin
@ 2022-10-06  8:22       ` Amit Prakash Shukla
  2022-10-06  9:05         ` Maxime Coquelin
  0 siblings, 1 reply; 9+ messages in thread
From: Amit Prakash Shukla @ 2022-10-06  8:22 UTC (permalink / raw)
  To: Maxime Coquelin, Stephen Hemminger
  Cc: dev, cheng1.jiang, chenbo.xia, zhoumin, david.marchand, thomas, stable

Hi Maxime,

Compiled this patch with aarch64-linux-gnu-gcc (GCC) 12.2.0 and it compiles fine.

Thanks,
Amit Shukla

> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Thursday, October 6, 2022 1:21 PM
> To: Stephen Hemminger <stephen@networkplumber.org>
> Cc: dev@dpdk.org; cheng1.jiang@intel.com; chenbo.xia@intel.com;
> zhoumin@loongson.cn; david.marchand@redhat.com;
> thomas@monjalon.net; stable@dpdk.org
> Subject: [EXT] Re: [PATCH v3] vhost: fix build issues with GCC 12
> 
> External Email
> 
> ----------------------------------------------------------------------
> Hi Stephen,
> 
> On 10/6/22 02:01, Stephen Hemminger wrote:
> > On Wed,  5 Oct 2022 22:35:24 +0200
> > Maxime Coquelin <maxime.coquelin@redhat.com> wrote:
> >
> >> In practice, it would never happen since 'pkt->pkt_len' is unlikely
> >> to be close to UINT32_MAX, but let's just change 'size' to uint64_t
> >> to make the compiler happy without having to add runtime checks.
> >
> >
> > Would the standard typedef size_t work since that is what sizeof() returns.
> >
> 
> I'm not sure it would not create issues on 32 bits architectures build given
> size_t is 32bits in this case if I'm not mistaken, as it adds something to a u32,
> so it could wrap to 0.


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

* Re: [EXT] Re: [PATCH v3] vhost: fix build issues with GCC 12
  2022-10-06  8:22       ` [EXT] " Amit Prakash Shukla
@ 2022-10-06  9:05         ` Maxime Coquelin
  0 siblings, 0 replies; 9+ messages in thread
From: Maxime Coquelin @ 2022-10-06  9:05 UTC (permalink / raw)
  To: Amit Prakash Shukla, Stephen Hemminger
  Cc: dev, cheng1.jiang, chenbo.xia, zhoumin, david.marchand, thomas, stable



On 10/6/22 10:22, Amit Prakash Shukla wrote:
> Hi Maxime,
> 
> Compiled this patch with aarch64-linux-gnu-gcc (GCC) 12.2.0 and it compiles fine.

Thanks Amit!

> Thanks,
> Amit Shukla
> 
>> -----Original Message-----
>> From: Maxime Coquelin <maxime.coquelin@redhat.com>
>> Sent: Thursday, October 6, 2022 1:21 PM
>> To: Stephen Hemminger <stephen@networkplumber.org>
>> Cc: dev@dpdk.org; cheng1.jiang@intel.com; chenbo.xia@intel.com;
>> zhoumin@loongson.cn; david.marchand@redhat.com;
>> thomas@monjalon.net; stable@dpdk.org
>> Subject: [EXT] Re: [PATCH v3] vhost: fix build issues with GCC 12
>>
>> External Email
>>
>> ----------------------------------------------------------------------
>> Hi Stephen,
>>
>> On 10/6/22 02:01, Stephen Hemminger wrote:
>>> On Wed,  5 Oct 2022 22:35:24 +0200
>>> Maxime Coquelin <maxime.coquelin@redhat.com> wrote:
>>>
>>>> In practice, it would never happen since 'pkt->pkt_len' is unlikely
>>>> to be close to UINT32_MAX, but let's just change 'size' to uint64_t
>>>> to make the compiler happy without having to add runtime checks.
>>>
>>>
>>> Would the standard typedef size_t work since that is what sizeof() returns.
>>>
>>
>> I'm not sure it would not create issues on 32 bits architectures build given
>> size_t is 32bits in this case if I'm not mistaken, as it adds something to a u32,
>> so it could wrap to 0.
> 


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

* Re: [PATCH v3] vhost: fix build issues with GCC 12
  2022-10-05 20:35 ` [PATCH v3] vhost: fix build issues with GCC 12 Maxime Coquelin
  2022-10-06  0:01   ` Stephen Hemminger
@ 2022-10-06 10:21   ` Thomas Monjalon
  1 sibling, 0 replies; 9+ messages in thread
From: Thomas Monjalon @ 2022-10-06 10:21 UTC (permalink / raw)
  To: Maxime Coquelin
  Cc: dev, cheng1.jiang, chenbo.xia, zhoumin, david.marchand, stable

05/10/2022 22:35, Maxime Coquelin:
> This patch fixes a compilation issue met with GCC 12 on
> LoongArch64:
> 
> In function ‘mbuf_to_desc’,
>     inlined from ‘vhost_enqueue_async_packed’ at ../../../dpdk/lib/vhost/virtio_net.c:1822:6,
>     inlined from ‘virtio_dev_rx_async_packed’ at ../../../dpdk/lib/vhost/virtio_net.c:1836:6,
>     inlined from ‘virtio_dev_rx_async_submit_packed’ at ../../../dpdk/lib/vhost/virtio_net.c:1895:7:
> ../../../dpdk/lib/vhost/virtio_net.c:1159:18: error: ‘buf_vec[0].buf_addr’ may be used uninitialized [-Werror=maybe-uninitialized]
>  1159 |         buf_addr = buf_vec[vec_idx].buf_addr;
>       |         ~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~
> ../../../dpdk/lib/vhost/virtio_net.c: In function ‘virtio_dev_rx_async_submit_packed’:
> ../../../dpdk/lib/vhost/virtio_net.c:1834:27: note: ‘buf_vec’ declared here
>  1834 |         struct buf_vector buf_vec[BUF_VECTOR_MAX];
>       |                           ^~~~~~~
> 
> It happens because the compiler assumes that 'size'
> variable in vhost_enqueue_async_packed could wrap to 0 since
> 'size' is uint32_t and pkt->pkt_len too.
> 
> In practice, it would never happen since 'pkt->pkt_len' is
> unlikely to be close to UINT32_MAX, but let's just change
> 'size' to uint64_t to make the compiler happy without
> having to add runtime checks.
> 
> This patch also fixes similar patterns in three other
> places, including one that also produces similar build
> issue on ARM64 in vhost_enqueue_single_packed().
> 
> Fixes: 873e8dad6f49 ("vhost: support packed ring in async datapath")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Reviewed-by: David Marchand <david.marchand@redhat.com>
Tested-by: Amit Prakash Shukla <amitprakashs@marvell.com>

Applied, thanks.




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

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

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-05 15:11 [PATCH v2] vhost: fix compilation issue in async path Maxime Coquelin
2022-10-05 15:36 ` Maxime Coquelin
2022-10-05 16:34   ` David Marchand
2022-10-05 20:35 ` [PATCH v3] vhost: fix build issues with GCC 12 Maxime Coquelin
2022-10-06  0:01   ` Stephen Hemminger
2022-10-06  7:51     ` Maxime Coquelin
2022-10-06  8:22       ` [EXT] " Amit Prakash Shukla
2022-10-06  9:05         ` Maxime Coquelin
2022-10-06 10:21   ` Thomas Monjalon

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