qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PULL 00/12] Net patches
@ 2021-06-11  6:00 Jason Wang
  2021-06-11 12:02 ` Peter Maydell
  2021-06-14 22:43 ` no-reply
  0 siblings, 2 replies; 19+ messages in thread
From: Jason Wang @ 2021-06-11  6:00 UTC (permalink / raw)
  To: qemu-devel, peter.maydell; +Cc: Jason Wang

The following changes since commit 7fe7fae8b48e3f9c647fd685e5155ebc8e6fb84d:

  Merge remote-tracking branch 'remotes/dgilbert-gitlab/tags/pull-migration-20210609a' into staging (2021-06-09 16:40:21 +0100)

are available in the git repository at:

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

for you to fetch changes up to 5a2d9929ac1f01a1e8ef2a3f56f69e6069863dad:

  Fixed calculation error of pkt->header_size in fill_pkt_tcp_info() (2021-06-11 10:30:13 +0800)

----------------------------------------------------------------

----------------------------------------------------------------
Jason Wang (4):
      vhost-vdpa: skip ram device from the IOTLB mapping
      vhost-vdpa: map virtqueue notification area if possible
      vhost-vdpa: don't initialize backend_features
      vhost-vdpa: remove the unused vhost_vdpa_get_acked_features()

Paolo Bonzini (1):
      netdev: add more commands to preconfig mode

Rao, Lei (7):
      Remove some duplicate trace code.
      Fix the qemu crash when guest shutdown during checkpoint
      Optimize the function of filter_send
      Remove migrate_set_block_enabled in checkpoint
      Add a function named packet_new_nocopy for COLO.
      Add the function of colo_compare_cleanup
      Fixed calculation error of pkt->header_size in fill_pkt_tcp_info()

 hmp-commands.hx                |   2 +
 hw/virtio/vhost-vdpa.c         | 100 +++++++++++++++++++++++++++++++++++------
 include/hw/virtio/vhost-vdpa.h |   6 +++
 include/net/vhost-vdpa.h       |   1 -
 migration/colo.c               |   6 ---
 migration/migration.c          |   4 ++
 net/colo-compare.c             |  25 +++++------
 net/colo-compare.h             |   1 +
 net/colo.c                     |  25 +++++++----
 net/colo.h                     |   1 +
 net/filter-mirror.c            |   8 ++--
 net/filter-rewriter.c          |   3 +-
 net/net.c                      |   4 ++
 net/vhost-vdpa.c               |   9 ----
 qapi/net.json                  |   6 ++-
 softmmu/runstate.c             |   1 +
 16 files changed, 143 insertions(+), 59 deletions(-)





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

* Re: [PULL 00/12] Net patches
  2021-06-11  6:00 Jason Wang
@ 2021-06-11 12:02 ` Peter Maydell
  2021-06-14 22:43 ` no-reply
  1 sibling, 0 replies; 19+ messages in thread
From: Peter Maydell @ 2021-06-11 12:02 UTC (permalink / raw)
  To: Jason Wang; +Cc: QEMU Developers

On Fri, 11 Jun 2021 at 07:00, Jason Wang <jasowang@redhat.com> wrote:
>
> The following changes since commit 7fe7fae8b48e3f9c647fd685e5155ebc8e6fb84d:
>
>   Merge remote-tracking branch 'remotes/dgilbert-gitlab/tags/pull-migration-20210609a' into staging (2021-06-09 16:40:21 +0100)
>
> are available in the git repository at:
>
>   https://github.com/jasowang/qemu.git tags/net-pull-request
>
> for you to fetch changes up to 5a2d9929ac1f01a1e8ef2a3f56f69e6069863dad:
>
>   Fixed calculation error of pkt->header_size in fill_pkt_tcp_info() (2021-06-11 10:30:13 +0800)
>
> ----------------------------------------------------------------
>
> ----------------------------------------------------------------
> Jason Wang (4):
>       vhost-vdpa: skip ram device from the IOTLB mapping
>       vhost-vdpa: map virtqueue notification area if possible
>       vhost-vdpa: don't initialize backend_features
>       vhost-vdpa: remove the unused vhost_vdpa_get_acked_features()
>
> Paolo Bonzini (1):
>       netdev: add more commands to preconfig mode
>
> Rao, Lei (7):
>       Remove some duplicate trace code.
>       Fix the qemu crash when guest shutdown during checkpoint
>       Optimize the function of filter_send
>       Remove migrate_set_block_enabled in checkpoint
>       Add a function named packet_new_nocopy for COLO.
>       Add the function of colo_compare_cleanup
>       Fixed calculation error of pkt->header_size in fill_pkt_tcp_info()


Applied, thanks.

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

-- PMM


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

* Re: [PULL 00/12] Net patches
  2021-06-11  6:00 Jason Wang
  2021-06-11 12:02 ` Peter Maydell
@ 2021-06-14 22:43 ` no-reply
  1 sibling, 0 replies; 19+ messages in thread
From: no-reply @ 2021-06-14 22:43 UTC (permalink / raw)
  To: jasowang; +Cc: peter.maydell, jasowang, qemu-devel

Patchew URL: https://patchew.org/QEMU/20210611060024.46763-1-jasowang@redhat.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20210611060024.46763-1-jasowang@redhat.com
Subject: [PULL 00/12] Net patches

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 - [tag update]      patchew/20210614200940.2056770-1-philmd@redhat.com -> patchew/20210614200940.2056770-1-philmd@redhat.com
 * [new tag]         patchew/20210614202842.581640-1-mathieu.poirier@linaro.org -> patchew/20210614202842.581640-1-mathieu.poirier@linaro.org
Switched to a new branch 'test'

=== OUTPUT BEGIN ===
checkpatch.pl: no revisions returned for revlist 'base..'
=== OUTPUT END ===

Test command exited with code: 255


The full log is available at
http://patchew.org/logs/20210611060024.46763-1-jasowang@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

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

* [PULL 00/12] Net patches
@ 2023-03-28  5:19 Jason Wang
  2023-03-28 16:00 ` Peter Maydell
  0 siblings, 1 reply; 19+ messages in thread
From: Jason Wang @ 2023-03-28  5:19 UTC (permalink / raw)
  To: qemu-devel; +Cc: Jason Wang

The following changes since commit e3debd5e7d0ce031356024878a0a18b9d109354a:

  Merge tag 'pull-request-2023-03-24' of https://gitlab.com/thuth/qemu into staging (2023-03-24 16:08:46 +0000)

are available in the git repository at:

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

for you to fetch changes up to fba7c3b788dfcb99a3f9253f7d99cc0d217d6d3c:

  igb: respect VMVIR and VMOLR for VLAN (2023-03-28 13:10:55 +0800)

----------------------------------------------------------------

----------------------------------------------------------------
Akihiko Odaki (4):
      igb: Save more Tx states
      igb: Fix DMA requester specification for Tx packet
      hw/net/net_tx_pkt: Ignore ECN bit
      hw/net/net_tx_pkt: Align l3_hdr

Sriram Yagnaraman (8):
      MAINTAINERS: Add Sriram Yagnaraman as a igb reviewer
      igb: handle PF/VF reset properly
      igb: add ICR_RXDW
      igb: implement VFRE and VFTE registers
      igb: check oversized packets for VMDq
      igb: respect E1000_VMOLR_RSSE
      igb: implement VF Tx and Rx stats
      igb: respect VMVIR and VMOLR for VLAN

 MAINTAINERS          |   1 +
 hw/net/e1000e_core.c |   6 +-
 hw/net/e1000x_regs.h |   4 +
 hw/net/igb.c         |  26 ++++--
 hw/net/igb_core.c    | 256 ++++++++++++++++++++++++++++++++++++++-------------
 hw/net/igb_core.h    |   9 +-
 hw/net/igb_regs.h    |   6 ++
 hw/net/net_tx_pkt.c  |  30 +++---
 hw/net/net_tx_pkt.h  |   3 +-
 hw/net/trace-events  |   2 +
 hw/net/vmxnet3.c     |   4 +-
 11 files changed, 254 insertions(+), 93 deletions(-)




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

* Re: [PULL 00/12] Net patches
  2023-03-28  5:19 Jason Wang
@ 2023-03-28 16:00 ` Peter Maydell
  0 siblings, 0 replies; 19+ messages in thread
From: Peter Maydell @ 2023-03-28 16:00 UTC (permalink / raw)
  To: Jason Wang; +Cc: qemu-devel

On Tue, 28 Mar 2023 at 06:21, Jason Wang <jasowang@redhat.com> wrote:
>
> The following changes since commit e3debd5e7d0ce031356024878a0a18b9d109354a:
>
>   Merge tag 'pull-request-2023-03-24' of https://gitlab.com/thuth/qemu into staging (2023-03-24 16:08:46 +0000)
>
> are available in the git repository at:
>
>   https://github.com/jasowang/qemu.git tags/net-pull-request
>
> for you to fetch changes up to fba7c3b788dfcb99a3f9253f7d99cc0d217d6d3c:
>
>   igb: respect VMVIR and VMOLR for VLAN (2023-03-28 13:10:55 +0800)
>


Applied, thanks.

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

-- PMM


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

* [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; 19+ 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] 19+ 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; 19+ 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] 19+ 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; 19+ 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] 19+ 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; 19+ 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] 19+ 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; 19+ 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] 19+ 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; 19+ 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] 19+ 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; 19+ 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] 19+ 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; 19+ 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] 19+ 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; 19+ 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] 19+ 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; 19+ 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] 19+ 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; 19+ 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] 19+ 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; 19+ 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] 19+ 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; 19+ 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] 19+ 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; 19+ 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] 19+ messages in thread

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

Thread overview: 19+ 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
  -- strict thread matches above, loose matches on Subject: below --
2023-03-28  5:19 Jason Wang
2023-03-28 16:00 ` Peter Maydell
2021-06-11  6:00 Jason Wang
2021-06-11 12:02 ` Peter Maydell
2021-06-14 22:43 ` no-reply

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