[v2] vhost: fix compilation issue in async path
Checks
Commit Message
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(-)
Comments
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];
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.
@@ -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];