qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PULL 00/12] Net patches
@ 2025-07-21  5:59 Jason Wang
  2025-07-21  5:59 ` [PULL 01/12] net/tap: drop too small packets Jason Wang
                   ` (12 more replies)
  0 siblings, 13 replies; 14+ messages in thread
From: Jason Wang @ 2025-07-21  5:59 UTC (permalink / raw)
  To: qemu-devel; +Cc: Jason Wang

The following changes since commit e82989544e38062beeeaad88c175afbeed0400f8:

  Merge tag 'for-upstream' of https://gitlab.com/bonzini/qemu into staging (2025-07-18 14:10:02 -0400)

are available in the Git repository at:

  https://github.com/jasowang/qemu.git tags/net-pull-request

for you to fetch changes up to ae9b09972bbf8ff49ae0edf3241fb413391b15ce:

  net/vhost-user: Remove unused "err" from chr_closed_bh() (CID 1612365) (2025-07-21 10:23:17 +0800)

----------------------------------------------------------------
-----BEGIN PGP SIGNATURE-----

iQEzBAABCAAdFiEEIV1G9IJGaJ7HfzVi7wSWWzmNYhEFAmh91p0ACgkQ7wSWWzmN
YhG+2wgAqw3G2TGRPT29ObyYDcd2Z54jdnNpX5gEND/UnqENprXfdD3PR58bnxe3
uJGPRkMXgkIDit61lshsb8DF8x9ZEIlm/Ax5FM0ksBczWDYHiyEuXoyt2Uai1kWY
fLBkVfjFqCu1AGniboCZiC4ZawZXIqkx/+DI3J/XRqa+bSCQ18I15dsLD/yxU/pp
Hwxp07/d+UayANdxs0mZ5Lr7a1ktTgytCt0O2jQNHlMzfOvdBbVbF/WGclMWfNgI
68HWPY7P8k8jRTRFx3H/0LyYQrPyseTpa3zHC+zW9jNskkPkhCwlAY4UDC8x8LII
OjsDc/0nre626rNCiJlifD3UJ7t86A==
=xj23
-----END PGP SIGNATURE-----

----------------------------------------------------------------
Laurent Vivier (6):
      net/passt: Remove unused "err" from passt_vhost_user_event() (CID 1612375)
      net/vhost-user: Remove unused "err" from net_vhost_user_event() (CID 1612372)
      net/passt: Remove dead code in passt_vhost_user_start error path (CID 1612371)
      net/passt: Check return value of g_remove() in net_passt_cleanup() (CID 1612369)
      net/passt: Initialize "error" variable in net_passt_send() (CID 1612368)
      net/vhost-user: Remove unused "err" from chr_closed_bh() (CID 1612365)

Peter Maydell (4):
      hw/net/npcm_gmac.c: Send the right data for second packet in a row
      hw/net/npcm_gmac.c: Unify length and prev_buf_size variables
      hw/net/npcm_gmac.c: Correct test for when to reallocate packet buffer
      hw/net/npcm_gmac.c: Drop 'buf' local variable

Steve Sistare (1):
      tap: fix net_init_tap() return code

Vladimir Sementsov-Ogievskiy (1):
      net/tap: drop too small packets

 hw/net/npcm_gmac.c | 26 ++++++++++++--------------
 net/passt.c        | 22 +++++++---------------
 net/tap.c          |  9 +++++++--
 net/vhost-user.c   |  9 ---------
 4 files changed, 26 insertions(+), 40 deletions(-)



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

* [PULL 01/12] net/tap: drop too small packets
  2025-07-21  5:59 [PULL 00/12] Net patches Jason Wang
@ 2025-07-21  5:59 ` Jason Wang
  2025-07-21  5:59 ` [PULL 02/12] tap: fix net_init_tap() return code Jason Wang
                   ` (11 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Jason Wang @ 2025-07-21  5:59 UTC (permalink / raw)
  To: qemu-devel; +Cc: Vladimir Sementsov-Ogievskiy, Lei Yang, Jason Wang

From: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>

Theoretically tap_read_packet() may return size less than
s->host_vnet_hdr_len, and next, we'll work with negative size
(in case of !s->using_vnet_hdr). Let's avoid it.

Don't proceed with size == s->host_vnet_hdr_len as well in case
of !s->using_vnet_hdr, it doesn't make sense.

Tested-by: Lei Yang <leiyang@redhat.com>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 net/tap.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/net/tap.c b/net/tap.c
index 23536c09b4..2a85936019 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -190,6 +190,11 @@ static void tap_send(void *opaque)
             break;
         }
 
+        if (s->host_vnet_hdr_len && size <= s->host_vnet_hdr_len) {
+            /* Invalid packet */
+            break;
+        }
+
         if (s->host_vnet_hdr_len && !s->using_vnet_hdr) {
             buf  += s->host_vnet_hdr_len;
             size -= s->host_vnet_hdr_len;
-- 
2.42.0



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

* [PULL 02/12] tap: fix net_init_tap() return code
  2025-07-21  5:59 [PULL 00/12] Net patches Jason Wang
  2025-07-21  5:59 ` [PULL 01/12] net/tap: drop too small packets Jason Wang
@ 2025-07-21  5:59 ` Jason Wang
  2025-07-21  5:59 ` [PULL 03/12] hw/net/npcm_gmac.c: Send the right data for second packet in a row Jason Wang
                   ` (10 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Jason Wang @ 2025-07-21  5:59 UTC (permalink / raw)
  To: qemu-devel; +Cc: Steve Sistare, Jason Wang

From: Steve Sistare <steven.sistare@oracle.com>

net_init_tap intends to return 0 for success and -1 on error.  However,
when net_init_tap() succeeds for a multi-queue device, it returns 1,
because of this code where ret becomes 1 when g_unix_set_fd_nonblocking
succeeds:

        ret = g_unix_set_fd_nonblocking(fd, true, NULL);
        if (!ret) {
            ... error ...
    free_fail:
        ...
        return ret;

Luckily, the only current call site checks for negative, rather than non-zero:

  net_client_init1()
      if (net_client_init_fun[](...) < 0)

Also, in the unlikely case that g_unix_set_fd_nonblocking fails and returns
false, ret=0 is returned, and net_client_init1 will use a broken interface.

Fix it to be future proof.

Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 net/tap.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/tap.c b/net/tap.c
index 2a85936019..f7df702f97 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -895,8 +895,8 @@ int net_init_tap(const Netdev *netdev, const char *name,
                 goto free_fail;
             }
 
-            ret = g_unix_set_fd_nonblocking(fd, true, NULL);
-            if (!ret) {
+            if (!g_unix_set_fd_nonblocking(fd, true, NULL)) {
+                ret = -1;
                 error_setg_errno(errp, errno, "%s: Can't use file descriptor %d",
                                  name, fd);
                 goto free_fail;
-- 
2.42.0



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

* [PULL 03/12] hw/net/npcm_gmac.c: Send the right data for second packet in a row
  2025-07-21  5:59 [PULL 00/12] Net patches Jason Wang
  2025-07-21  5:59 ` [PULL 01/12] net/tap: drop too small packets Jason Wang
  2025-07-21  5:59 ` [PULL 02/12] tap: fix net_init_tap() return code Jason Wang
@ 2025-07-21  5:59 ` Jason Wang
  2025-07-21  5:59 ` [PULL 04/12] hw/net/npcm_gmac.c: Unify length and prev_buf_size variables Jason Wang
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Jason Wang @ 2025-07-21  5:59 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, qemu-stable, Jason Wang

From: Peter Maydell <peter.maydell@linaro.org>

The transmit loop in gmac_try_send_next_packet() is constructed in a
way that means it will send incorrect data if it it sends more than
one packet.

The function assembles the outbound data in a dynamically allocated
block of memory which is pointed to by tx_send_buffer.  We track the
first point in this block of memory which is not yet used with the
prev_buf_size offset, initially zero.  We track the size of the
packet we're sending with the length variable, also initially zero.

As we read chunks of data out of guest memory, we write them to
tx_send_buffer[prev_buf_size], and then increment both prev_buf_size
and length.  (We might dynamically reallocate the buffer if needed.)

When we send a packet, we checksum and send length bytes, starting at
tx_send_buffer, and then we reset length to 0.  This gives the right
data for the first packet.  But we don't reset prev_buf_size.  This
means that if we process more descriptors with further data for the
next packet, that data will continue to accumulate at offset
prev_buf_size, i.e.  after the data for the first packet.  But when
we transmit that second packet, we send length bytes from
tx_send_buffer, so we will send a packet which has the length of the
second packet but the data of the first one.

The fix for this is to also clear prev_buf_size after the packet has
been sent -- we never need the data from packet one after we've sent
it, so we can write packet two's data starting at the beginning of
the buffer.

Cc: qemu-stable@nongnu.org
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/net/npcm_gmac.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/net/npcm_gmac.c b/hw/net/npcm_gmac.c
index a434112580..921327dd8c 100644
--- a/hw/net/npcm_gmac.c
+++ b/hw/net/npcm_gmac.c
@@ -615,6 +615,7 @@ static void gmac_try_send_next_packet(NPCMGMACState *gmac)
             trace_npcm_gmac_packet_sent(DEVICE(gmac)->canonical_path, length);
             buf = tx_send_buffer;
             length = 0;
+            prev_buf_size = 0;
         }
 
         /* step 6 */
-- 
2.42.0



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

* [PULL 04/12] hw/net/npcm_gmac.c: Unify length and prev_buf_size variables
  2025-07-21  5:59 [PULL 00/12] Net patches Jason Wang
                   ` (2 preceding siblings ...)
  2025-07-21  5:59 ` [PULL 03/12] hw/net/npcm_gmac.c: Send the right data for second packet in a row Jason Wang
@ 2025-07-21  5:59 ` Jason Wang
  2025-07-21  5:59 ` [PULL 05/12] hw/net/npcm_gmac.c: Correct test for when to reallocate packet buffer Jason Wang
                   ` (8 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Jason Wang @ 2025-07-21  5:59 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, Jason Wang

From: Peter Maydell <peter.maydell@linaro.org>

After the bug fix in the previous commit, the length and prev_buf_size
variables are identical, except that prev_buf_size is uint32_t and
length is uint16_t. We can therefore unify them. The only place where
the type makes a difference is that we will truncate the packet
at 64K when sending it; this commit preserves that behaviour
by using a local variable when doing the packet send.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/net/npcm_gmac.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/hw/net/npcm_gmac.c b/hw/net/npcm_gmac.c
index 921327dd8c..a0050a7725 100644
--- a/hw/net/npcm_gmac.c
+++ b/hw/net/npcm_gmac.c
@@ -516,7 +516,6 @@ static void gmac_try_send_next_packet(NPCMGMACState *gmac)
     uint32_t desc_addr;
     struct NPCMGMACTxDesc tx_desc;
     uint32_t tx_buf_addr, tx_buf_len;
-    uint16_t length = 0;
     uint8_t *buf = tx_send_buffer;
     uint32_t prev_buf_size = 0;
     int csum = 0;
@@ -583,7 +582,6 @@ static void gmac_try_send_next_packet(NPCMGMACState *gmac)
                         __func__, tx_buf_addr);
             return;
         }
-        length += tx_buf_len;
         prev_buf_size += tx_buf_len;
 
         /* If not chained we'll have a second buffer. */
@@ -606,15 +604,18 @@ static void gmac_try_send_next_packet(NPCMGMACState *gmac)
                               __func__, tx_buf_addr);
                 return;
             }
-            length += tx_buf_len;
             prev_buf_size += tx_buf_len;
         }
         if (tx_desc.tdes1 & TX_DESC_TDES1_LAST_SEG_MASK) {
+            /*
+             * This will truncate the packet at 64K.
+             * TODO: find out if this is the correct behaviour.
+             */
+            uint16_t length = prev_buf_size;
             net_checksum_calculate(tx_send_buffer, length, csum);
             qemu_send_packet(qemu_get_queue(gmac->nic), tx_send_buffer, length);
             trace_npcm_gmac_packet_sent(DEVICE(gmac)->canonical_path, length);
             buf = tx_send_buffer;
-            length = 0;
             prev_buf_size = 0;
         }
 
-- 
2.42.0



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

* [PULL 05/12] hw/net/npcm_gmac.c: Correct test for when to reallocate packet buffer
  2025-07-21  5:59 [PULL 00/12] Net patches Jason Wang
                   ` (3 preceding siblings ...)
  2025-07-21  5:59 ` [PULL 04/12] hw/net/npcm_gmac.c: Unify length and prev_buf_size variables Jason Wang
@ 2025-07-21  5:59 ` Jason Wang
  2025-07-21  5:59 ` [PULL 06/12] hw/net/npcm_gmac.c: Drop 'buf' local variable Jason Wang
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Jason Wang @ 2025-07-21  5:59 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, Jason Wang

From: Peter Maydell <peter.maydell@linaro.org>

In gmac_try_send_next_packet() we have code that does "if this block
of data won't fit in the buffer, reallocate it".  However, the
condition it uses is
  if ((prev_buf_size + tx_buf_len) > sizeof(buf))
where buf is a uint8_t *.

This means that sizeof(buf) is always 8 bytes, and the condition will
almost always be true, so we will reallocate the buffer more often
than we need to.

Correct the condition to test against tx_buffer_size, which is
where we track how big the allocated buffer is.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/net/npcm_gmac.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/net/npcm_gmac.c b/hw/net/npcm_gmac.c
index a0050a7725..0c17ae9b2a 100644
--- a/hw/net/npcm_gmac.c
+++ b/hw/net/npcm_gmac.c
@@ -569,7 +569,7 @@ static void gmac_try_send_next_packet(NPCMGMACState *gmac)
         tx_buf_len = TX_DESC_TDES1_BFFR1_SZ_MASK(tx_desc.tdes1);
         buf = &tx_send_buffer[prev_buf_size];
 
-        if ((prev_buf_size + tx_buf_len) > sizeof(buf)) {
+        if ((prev_buf_size + tx_buf_len) > tx_buffer_size) {
             tx_buffer_size = prev_buf_size + tx_buf_len;
             tx_send_buffer = g_realloc(tx_send_buffer, tx_buffer_size);
             buf = &tx_send_buffer[prev_buf_size];
@@ -591,7 +591,7 @@ static void gmac_try_send_next_packet(NPCMGMACState *gmac)
             tx_buf_len = TX_DESC_TDES1_BFFR2_SZ_MASK(tx_desc.tdes1);
             buf = &tx_send_buffer[prev_buf_size];
 
-            if ((prev_buf_size + tx_buf_len) > sizeof(buf)) {
+            if ((prev_buf_size + tx_buf_len) > tx_buffer_size) {
                 tx_buffer_size = prev_buf_size + tx_buf_len;
                 tx_send_buffer = g_realloc(tx_send_buffer, tx_buffer_size);
                 buf = &tx_send_buffer[prev_buf_size];
-- 
2.42.0



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

* [PULL 06/12] hw/net/npcm_gmac.c: Drop 'buf' local variable
  2025-07-21  5:59 [PULL 00/12] Net patches Jason Wang
                   ` (4 preceding siblings ...)
  2025-07-21  5:59 ` [PULL 05/12] hw/net/npcm_gmac.c: Correct test for when to reallocate packet buffer Jason Wang
@ 2025-07-21  5:59 ` Jason Wang
  2025-07-21  5:59 ` [PULL 07/12] net/passt: Remove unused "err" from passt_vhost_user_event() (CID 1612375) Jason Wang
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Jason Wang @ 2025-07-21  5:59 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, Jason Wang

From: Peter Maydell <peter.maydell@linaro.org>

We use the local variable 'buf' only when we call dma_memory_read(),
and it is always set to &tx_send_buffer[prev_buf_size] immediately
before both of those calls.  So remove the variable and pass
tx_send_buffer + prev_buf_size to dma_memory_read().

This fixes in passing a place where we set buf = tx_send_buffer
but never used that value because we always updated buf to
something else later before using it.

Coverity: CID 1534027
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/net/npcm_gmac.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/hw/net/npcm_gmac.c b/hw/net/npcm_gmac.c
index 0c17ae9b2a..5e32cd3edf 100644
--- a/hw/net/npcm_gmac.c
+++ b/hw/net/npcm_gmac.c
@@ -516,7 +516,6 @@ static void gmac_try_send_next_packet(NPCMGMACState *gmac)
     uint32_t desc_addr;
     struct NPCMGMACTxDesc tx_desc;
     uint32_t tx_buf_addr, tx_buf_len;
-    uint8_t *buf = tx_send_buffer;
     uint32_t prev_buf_size = 0;
     int csum = 0;
 
@@ -567,16 +566,15 @@ static void gmac_try_send_next_packet(NPCMGMACState *gmac)
         tx_buf_addr = tx_desc.tdes2;
         gmac->regs[R_NPCM_DMA_CUR_TX_BUF_ADDR] = tx_buf_addr;
         tx_buf_len = TX_DESC_TDES1_BFFR1_SZ_MASK(tx_desc.tdes1);
-        buf = &tx_send_buffer[prev_buf_size];
 
         if ((prev_buf_size + tx_buf_len) > tx_buffer_size) {
             tx_buffer_size = prev_buf_size + tx_buf_len;
             tx_send_buffer = g_realloc(tx_send_buffer, tx_buffer_size);
-            buf = &tx_send_buffer[prev_buf_size];
         }
 
         /* step 5 */
-        if (dma_memory_read(&address_space_memory, tx_buf_addr, buf,
+        if (dma_memory_read(&address_space_memory, tx_buf_addr,
+                            tx_send_buffer + prev_buf_size,
                             tx_buf_len, MEMTXATTRS_UNSPECIFIED)) {
             qemu_log_mask(LOG_GUEST_ERROR, "%s: Failed to read packet @ 0x%x\n",
                         __func__, tx_buf_addr);
@@ -589,15 +587,14 @@ static void gmac_try_send_next_packet(NPCMGMACState *gmac)
             tx_buf_addr = tx_desc.tdes3;
             gmac->regs[R_NPCM_DMA_CUR_TX_BUF_ADDR] = tx_buf_addr;
             tx_buf_len = TX_DESC_TDES1_BFFR2_SZ_MASK(tx_desc.tdes1);
-            buf = &tx_send_buffer[prev_buf_size];
 
             if ((prev_buf_size + tx_buf_len) > tx_buffer_size) {
                 tx_buffer_size = prev_buf_size + tx_buf_len;
                 tx_send_buffer = g_realloc(tx_send_buffer, tx_buffer_size);
-                buf = &tx_send_buffer[prev_buf_size];
             }
 
-            if (dma_memory_read(&address_space_memory, tx_buf_addr, buf,
+            if (dma_memory_read(&address_space_memory, tx_buf_addr,
+                                tx_send_buffer + prev_buf_size,
                                 tx_buf_len, MEMTXATTRS_UNSPECIFIED)) {
                 qemu_log_mask(LOG_GUEST_ERROR,
                               "%s: Failed to read packet @ 0x%x\n",
@@ -615,7 +612,6 @@ static void gmac_try_send_next_packet(NPCMGMACState *gmac)
             net_checksum_calculate(tx_send_buffer, length, csum);
             qemu_send_packet(qemu_get_queue(gmac->nic), tx_send_buffer, length);
             trace_npcm_gmac_packet_sent(DEVICE(gmac)->canonical_path, length);
-            buf = tx_send_buffer;
             prev_buf_size = 0;
         }
 
-- 
2.42.0



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

* [PULL 07/12] net/passt: Remove unused "err" from passt_vhost_user_event() (CID 1612375)
  2025-07-21  5:59 [PULL 00/12] Net patches Jason Wang
                   ` (5 preceding siblings ...)
  2025-07-21  5:59 ` [PULL 06/12] hw/net/npcm_gmac.c: Drop 'buf' local variable Jason Wang
@ 2025-07-21  5:59 ` Jason Wang
  2025-07-21  5:59 ` [PULL 08/12] net/vhost-user: Remove unused "err" from net_vhost_user_event() (CID 1612372) Jason Wang
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Jason Wang @ 2025-07-21  5:59 UTC (permalink / raw)
  To: qemu-devel; +Cc: Laurent Vivier, Peter Maydell, Jason Wang

From: Laurent Vivier <lvivier@redhat.com>

The "err" variable was declared but never used within the
passt_vhost_user_event() function. This resulted in a dead code
warning (CID 1612375) from Coverity.

Remove the unused variable and the associated error block
to resolve the issue.

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 net/passt.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/net/passt.c b/net/passt.c
index 6f616ba3c2..9cd5b3e6f2 100644
--- a/net/passt.c
+++ b/net/passt.c
@@ -397,7 +397,6 @@ err:
 static void passt_vhost_user_event(void *opaque, QEMUChrEvent event)
 {
     NetPasstState *s = opaque;
-    Error *err = NULL;
 
     switch (event) {
     case CHR_EVENT_OPENED:
@@ -428,10 +427,6 @@ static void passt_vhost_user_event(void *opaque, QEMUChrEvent event)
         /* Ignore */
         break;
     }
-
-    if (err) {
-        error_report_err(err);
-    }
 }
 
 static int net_passt_vhost_user_init(NetPasstState *s, Error **errp)
-- 
2.42.0



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

* [PULL 08/12] net/vhost-user: Remove unused "err" from net_vhost_user_event() (CID 1612372)
  2025-07-21  5:59 [PULL 00/12] Net patches Jason Wang
                   ` (6 preceding siblings ...)
  2025-07-21  5:59 ` [PULL 07/12] net/passt: Remove unused "err" from passt_vhost_user_event() (CID 1612375) Jason Wang
@ 2025-07-21  5:59 ` Jason Wang
  2025-07-21  5:59 ` [PULL 09/12] net/passt: Remove dead code in passt_vhost_user_start error path (CID 1612371) Jason Wang
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Jason Wang @ 2025-07-21  5:59 UTC (permalink / raw)
  To: qemu-devel; +Cc: Laurent Vivier, Peter Maydell, Jason Wang

From: Laurent Vivier <lvivier@redhat.com>

The "err" variable was declared but never used within the
net_vhost_user_event() function. This resulted in a dead code
warning (CID 1612372) from Coverity.

Remove the unused variable and the associated error block
to resolve the issue.

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 net/vhost-user.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/net/vhost-user.c b/net/vhost-user.c
index 1c3b8b36f3..cec83e925f 100644
--- a/net/vhost-user.c
+++ b/net/vhost-user.c
@@ -329,7 +329,6 @@ static void net_vhost_user_event(void *opaque, QEMUChrEvent event)
     NetClientState *ncs[MAX_QUEUE_NUM];
     NetVhostUserState *s;
     Chardev *chr;
-    Error *err = NULL;
     int queues;
 
     queues = qemu_find_net_clients_except(name, ncs,
@@ -375,10 +374,6 @@ static void net_vhost_user_event(void *opaque, QEMUChrEvent event)
         /* Ignore */
         break;
     }
-
-    if (err) {
-        error_report_err(err);
-    }
 }
 
 static int net_vhost_user_init(NetClientState *peer, const char *device,
-- 
2.42.0



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

* [PULL 09/12] net/passt: Remove dead code in passt_vhost_user_start error path (CID 1612371)
  2025-07-21  5:59 [PULL 00/12] Net patches Jason Wang
                   ` (7 preceding siblings ...)
  2025-07-21  5:59 ` [PULL 08/12] net/vhost-user: Remove unused "err" from net_vhost_user_event() (CID 1612372) Jason Wang
@ 2025-07-21  5:59 ` Jason Wang
  2025-07-21  5:59 ` [PULL 10/12] net/passt: Check return value of g_remove() in net_passt_cleanup() (CID 1612369) Jason Wang
                   ` (3 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Jason Wang @ 2025-07-21  5:59 UTC (permalink / raw)
  To: qemu-devel; +Cc: Laurent Vivier, Peter Maydell, Jason Wang

From: Laurent Vivier <lvivier@redhat.com>

In passt_vhost_user_start(), if vhost_net_init() fails, the "net"
variable is NULL and execution jumps to the "err:" label.

The cleanup code within this label is conditioned on "if (net)",
which can never be true in this error case. This makes the cleanup
block dead code, as reported by Coverity (CID 1612371).

Refactor the error handling to occur inline, removing the goto and
the unreachable cleanup block.

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 net/passt.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/net/passt.c b/net/passt.c
index 9cd5b3e6f2..ef59d0682b 100644
--- a/net/passt.c
+++ b/net/passt.c
@@ -375,7 +375,8 @@ static int passt_vhost_user_start(NetPasstState *s, VhostUserState *be)
     net = vhost_net_init(&options);
     if (!net) {
         error_report("failed to init passt vhost_net");
-        goto err;
+        passt_vhost_user_stop(s);
+        return -1;
     }
 
     if (s->vhost_net) {
@@ -385,13 +386,6 @@ static int passt_vhost_user_start(NetPasstState *s, VhostUserState *be)
     s->vhost_net = net;
 
     return 0;
-err:
-    if (net) {
-        vhost_net_cleanup(net);
-        g_free(net);
-    }
-    passt_vhost_user_stop(s);
-    return -1;
 }
 
 static void passt_vhost_user_event(void *opaque, QEMUChrEvent event)
-- 
2.42.0



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

* [PULL 10/12] net/passt: Check return value of g_remove() in net_passt_cleanup() (CID 1612369)
  2025-07-21  5:59 [PULL 00/12] Net patches Jason Wang
                   ` (8 preceding siblings ...)
  2025-07-21  5:59 ` [PULL 09/12] net/passt: Remove dead code in passt_vhost_user_start error path (CID 1612371) Jason Wang
@ 2025-07-21  5:59 ` Jason Wang
  2025-07-21  5:59 ` [PULL 11/12] net/passt: Initialize "error" variable in net_passt_send() (CID 1612368) Jason Wang
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Jason Wang @ 2025-07-21  5:59 UTC (permalink / raw)
  To: qemu-devel; +Cc: Laurent Vivier, Peter Maydell, Jason Wang

From: Laurent Vivier <lvivier@redhat.com>

If g_remove() fails, use warn_report() to log an error.

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 net/passt.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/net/passt.c b/net/passt.c
index ef59d0682b..43c336e596 100644
--- a/net/passt.c
+++ b/net/passt.c
@@ -103,7 +103,10 @@ static void net_passt_cleanup(NetClientState *nc)
 #endif
 
     kill(s->pid, SIGTERM);
-    g_remove(s->pidfile);
+    if (g_remove(s->pidfile) != 0) {
+        warn_report("Failed to remove passt pidfile %s: %s",
+                    s->pidfile, strerror(errno));
+    }
     g_free(s->pidfile);
     g_ptr_array_free(s->args, TRUE);
 }
-- 
2.42.0



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

* [PULL 11/12] net/passt: Initialize "error" variable in net_passt_send() (CID 1612368)
  2025-07-21  5:59 [PULL 00/12] Net patches Jason Wang
                   ` (9 preceding siblings ...)
  2025-07-21  5:59 ` [PULL 10/12] net/passt: Check return value of g_remove() in net_passt_cleanup() (CID 1612369) Jason Wang
@ 2025-07-21  5:59 ` Jason Wang
  2025-07-21  5:59 ` [PULL 12/12] net/vhost-user: Remove unused "err" from chr_closed_bh() (CID 1612365) Jason Wang
  2025-07-21 13:59 ` [PULL 00/12] Net patches Stefan Hajnoczi
  12 siblings, 0 replies; 14+ messages in thread
From: Jason Wang @ 2025-07-21  5:59 UTC (permalink / raw)
  To: qemu-devel; +Cc: Laurent Vivier, Peter Maydell, Jason Wang

From: Laurent Vivier <lvivier@redhat.com>

This was flagged by Coverity as a memory illegal access.

Initialize the pointer to NULL at declaration.

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 net/passt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/passt.c b/net/passt.c
index 43c336e596..32ecffb763 100644
--- a/net/passt.c
+++ b/net/passt.c
@@ -124,7 +124,7 @@ static gboolean net_passt_send(QIOChannel *ioc, GIOCondition condition,
 {
     if (net_stream_data_send(ioc, condition, data) == G_SOURCE_REMOVE) {
         NetPasstState *s = DO_UPCAST(NetPasstState, data, data);
-        Error *error;
+        Error *error = NULL;
 
         /* we need to restart passt */
         kill(s->pid, SIGTERM);
-- 
2.42.0



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

* [PULL 12/12] net/vhost-user: Remove unused "err" from chr_closed_bh() (CID 1612365)
  2025-07-21  5:59 [PULL 00/12] Net patches Jason Wang
                   ` (10 preceding siblings ...)
  2025-07-21  5:59 ` [PULL 11/12] net/passt: Initialize "error" variable in net_passt_send() (CID 1612368) Jason Wang
@ 2025-07-21  5:59 ` Jason Wang
  2025-07-21 13:59 ` [PULL 00/12] Net patches Stefan Hajnoczi
  12 siblings, 0 replies; 14+ messages in thread
From: Jason Wang @ 2025-07-21  5:59 UTC (permalink / raw)
  To: qemu-devel; +Cc: Laurent Vivier, Peter Maydell, Jason Wang

From: Laurent Vivier <lvivier@redhat.com>

The "err" variable was declared but never used within the
chr_closed_bh() function. This resulted in a dead code
warning (CID 1612365) from Coverity.

Remove the unused variable and the associated error block
to resolve the issue.

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 net/vhost-user.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/net/vhost-user.c b/net/vhost-user.c
index cec83e925f..8b96157145 100644
--- a/net/vhost-user.c
+++ b/net/vhost-user.c
@@ -298,7 +298,6 @@ static void chr_closed_bh(void *opaque)
     const char *name = opaque;
     NetClientState *ncs[MAX_QUEUE_NUM];
     NetVhostUserState *s;
-    Error *err = NULL;
     int queues, i;
 
     queues = qemu_find_net_clients_except(name, ncs,
@@ -317,9 +316,6 @@ static void chr_closed_bh(void *opaque)
     qemu_chr_fe_set_handlers(&s->chr, NULL, NULL, net_vhost_user_event,
                              NULL, opaque, NULL, true);
 
-    if (err) {
-        error_report_err(err);
-    }
     qapi_event_send_netdev_vhost_user_disconnected(name);
 }
 
-- 
2.42.0



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

* Re: [PULL 00/12] Net patches
  2025-07-21  5:59 [PULL 00/12] Net patches Jason Wang
                   ` (11 preceding siblings ...)
  2025-07-21  5:59 ` [PULL 12/12] net/vhost-user: Remove unused "err" from chr_closed_bh() (CID 1612365) Jason Wang
@ 2025-07-21 13:59 ` Stefan Hajnoczi
  12 siblings, 0 replies; 14+ messages in thread
From: Stefan Hajnoczi @ 2025-07-21 13:59 UTC (permalink / raw)
  To: Jason Wang; +Cc: qemu-devel, Jason Wang

[-- Attachment #1: Type: text/plain, Size: 116 bytes --]

Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/10.1 for any user-visible changes.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

end of thread, other threads:[~2025-07-21 14:00 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-21  5:59 [PULL 00/12] Net patches Jason Wang
2025-07-21  5:59 ` [PULL 01/12] net/tap: drop too small packets Jason Wang
2025-07-21  5:59 ` [PULL 02/12] tap: fix net_init_tap() return code Jason Wang
2025-07-21  5:59 ` [PULL 03/12] hw/net/npcm_gmac.c: Send the right data for second packet in a row Jason Wang
2025-07-21  5:59 ` [PULL 04/12] hw/net/npcm_gmac.c: Unify length and prev_buf_size variables Jason Wang
2025-07-21  5:59 ` [PULL 05/12] hw/net/npcm_gmac.c: Correct test for when to reallocate packet buffer Jason Wang
2025-07-21  5:59 ` [PULL 06/12] hw/net/npcm_gmac.c: Drop 'buf' local variable Jason Wang
2025-07-21  5:59 ` [PULL 07/12] net/passt: Remove unused "err" from passt_vhost_user_event() (CID 1612375) Jason Wang
2025-07-21  5:59 ` [PULL 08/12] net/vhost-user: Remove unused "err" from net_vhost_user_event() (CID 1612372) Jason Wang
2025-07-21  5:59 ` [PULL 09/12] net/passt: Remove dead code in passt_vhost_user_start error path (CID 1612371) Jason Wang
2025-07-21  5:59 ` [PULL 10/12] net/passt: Check return value of g_remove() in net_passt_cleanup() (CID 1612369) Jason Wang
2025-07-21  5:59 ` [PULL 11/12] net/passt: Initialize "error" variable in net_passt_send() (CID 1612368) Jason Wang
2025-07-21  5:59 ` [PULL 12/12] net/vhost-user: Remove unused "err" from chr_closed_bh() (CID 1612365) Jason Wang
2025-07-21 13:59 ` [PULL 00/12] Net patches Stefan Hajnoczi

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