DPDK patches and discussions
 help / color / mirror / Atom feed
From: David Marchand <david.marchand@redhat.com>
To: Maxime Coquelin <maxime.coquelin@redhat.com>
Cc: dev@dpdk.org, cheng1.jiang@intel.com, chenbo.xia@intel.com,
	 zhoumin@loongson.cn, thomas@monjalon.net, stable@dpdk.org
Subject: Re: [PATCH v2] vhost: fix compilation issue in async path
Date: Wed, 5 Oct 2022 18:34:18 +0200	[thread overview]
Message-ID: <CAJFAV8ywuRVGq-ZUque9K+k3Ur6k33ijXE2iR=U5TwU4BxXrVA@mail.gmail.com> (raw)
In-Reply-To: <d34c8aea-c70c-c6a6-d499-cf121c4b72f9@redhat.com>

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


  reply	other threads:[~2022-10-05 16:34 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-05 15:11 Maxime Coquelin
2022-10-05 15:36 ` Maxime Coquelin
2022-10-05 16:34   ` David Marchand [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAJFAV8ywuRVGq-ZUque9K+k3Ur6k33ijXE2iR=U5TwU4BxXrVA@mail.gmail.com' \
    --to=david.marchand@redhat.com \
    --cc=chenbo.xia@intel.com \
    --cc=cheng1.jiang@intel.com \
    --cc=dev@dpdk.org \
    --cc=maxime.coquelin@redhat.com \
    --cc=stable@dpdk.org \
    --cc=thomas@monjalon.net \
    --cc=zhoumin@loongson.cn \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).