The following changes since commit d9a4282c4b690e45d25c2b933f318bb41eeb271d:

Merge tag 'pull-tcg-20250308' of https://gitlab.com/rth7680/qemu into staging (2025-03-09 11:45:00 +0800)

are available in the Git repository at:

for you to fetch changes up to ac2ff9b840ce82cc7d5fd9ce4fd3019a434d7dc9:

tap-linux: Open ipvtap and macvtap (2025-03-10 17:07:16 +0800)

Akihiko Odaki (3):

util/iov: Do not assert offset is in iov

Revert "hw/net/net_tx_pkt: Fix overrun in update_sctp_checksum()"

tap-linux: Open ipvtap and macvtap

Eugenio Pérez (2):

net: parameterize the removing client from nc list

net: move backend cleanup to NIC cleanup

hw/net/net_tx_pkt.c | 4 ----

include/qemu/iov.h | 5 +++--

net/net.c | 44 ++++++++++++++++++++++++++++++++++----------

net/tap-linux.c | 17 ++++++++++++++---

net/vhost-vdpa.c | 8 --------

util/iov.c | 5 -----

6 files changed, 51 insertions(+), 32 deletions(-)

From: Eugenio Pérez <eperezma@redhat.com>

This change is used in later commits so we can avoid the removal of the

netclient if it is delayed.

No functional change intended.

Reviewed-by: Si-Wei Liu <si-wei.liu@oracle.com>

Acked-by: Jason Wang <jasowang@redhat.com>

Signed-off-by: Eugenio Pérez <eperezma@redhat.com>

net/net.c | 13 ++++++++-----

1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/net/net.c b/net/net.c

--- a/net/net.c

+++ b/net/net.c

@@ -XXX,XX +XXX,XX @@ NetClientState *qemu_get_peer(NetClientState *nc, int queue_index)

return ncs->peer;

}

-static void qemu_cleanup_net_client(NetClientState *nc)

+static void qemu_cleanup_net_client(NetClientState *nc,

+ bool remove_from_net_clients)

- QTAILQ_REMOVE(&net_clients, nc, next);

+ if (remove_from_net_clients) {

+ QTAILQ_REMOVE(&net_clients, nc, next);

+ }

if (nc->info->cleanup) {

nc->info->cleanup(nc);

@@ -XXX,XX +XXX,XX @@ void qemu_del_net_client(NetClientState *nc)

}

for (i = 0; i < queues; i++) {

- qemu_cleanup_net_client(ncs[i]);

+ qemu_cleanup_net_client(ncs[i], true);

}

return;

}

for (i = 0; i < queues; i++) {

- qemu_cleanup_net_client(ncs[i]);

+ qemu_cleanup_net_client(ncs[i], true);

qemu_free_net_client(ncs[i]);

@@ -XXX,XX +XXX,XX @@ void qemu_del_nic(NICState *nic)

for (i = queues - 1; i >= 0; i--) {

NetClientState *nc = qemu_get_subqueue(nic, i);

- qemu_cleanup_net_client(nc);

+ qemu_cleanup_net_client(nc, true);

qemu_free_net_client(nc);

2.42.0

From: Eugenio Pérez <eperezma@redhat.com>

Commit a0d7215e33 ("vhost-vdpa: do not cleanup the vdpa/vhost-net

structures if peer nic is present") effectively delayed the backend

cleanup, allowing the frontend or the guest to access it resources as

long as the frontend is still visible to the guest.

However it does not clean up the resources until the qemu process is

over. This causes an effective leak if the device is deleted with

device_del, as there is no way to close the vdpa device. This makes

impossible to re-add that device to this or other QEMU instances until

the first instance of QEMU is finished.

Move the cleanup from qemu_cleanup to the NIC deletion and to

net_cleanup.

Fixes: a0d7215e33 ("vhost-vdpa: do not cleanup the vdpa/vhost-net structures if peer nic is present")

Reported-by: Lei Yang <leiyang@redhat.com>

Signed-off-by: Eugenio Pérez <eperezma@redhat.com>

Signed-off-by: Jonah Palmer <jonah.palmer@oracle.com>

net/net.c | 33 +++++++++++++++++++++++++++------

net/vhost-vdpa.c | 8 --------

2 files changed, 27 insertions(+), 14 deletions(-)

diff --git a/net/net.c b/net/net.c

--- a/net/net.c

+++ b/net/net.c

@@ -XXX,XX +XXX,XX @@ void qemu_del_net_client(NetClientState *nc)

object_unparent(OBJECT(nf));

}

- /* If there is a peer NIC, delete and cleanup client, but do not free. */

+ /*

+ * If there is a peer NIC, transfer ownership to it. Delete the client

+ * from net_client list but do not cleanup nor free. This way NIC can

+ * still access to members of the backend.

+ *

+ * The cleanup and free will be done when the NIC is free.

+ */

if (nc->peer && nc->peer->info->type == NET_CLIENT_DRIVER_NIC) {

NICState *nic = qemu_get_nic(nc->peer);

if (nic->peer_deleted) {

@@ -XXX,XX +XXX,XX @@ void qemu_del_net_client(NetClientState *nc)

for (i = 0; i < queues; i++) {

ncs[i]->peer->link_down = true;

+ QTAILQ_REMOVE(&net_clients, ncs[i], next);

}

if (nc->peer->info->link_status_changed) {

nc->peer->info->link_status_changed(nc->peer);

}

- for (i = 0; i < queues; i++) {

- qemu_cleanup_net_client(ncs[i], true);

- }

-

return;

}

@@ -XXX,XX +XXX,XX @@ void qemu_del_nic(NICState *nic)

for (i = 0; i < queues; i++) {

NetClientState *nc = qemu_get_subqueue(nic, i);

- /* If this is a peer NIC and peer has already been deleted, free it now. */

+ /*

+ * If this is a peer NIC and peer has already been deleted, clean it up

+ * and free it now.

+ */

if (nic->peer_deleted) {

+ qemu_cleanup_net_client(nc->peer, false);

qemu_free_net_client(nc->peer);

} else if (nc->peer) {

/* if there are RX packets pending, complete them */

@@ -XXX,XX +XXX,XX @@ void net_cleanup(void)

* of the latest NET_CLIENT_DRIVER_NIC, and operate on *p as we walk

* the list.

*

+ * However, the NIC may have peers that trust to be clean beyond this

+ * point. For example, if they have been removed with device_del.

+ *

* The 'nc' variable isn't part of the list traversal; it's purely

* for convenience as too much '(*p)->' has a tendency to make the

* readers' eyes bleed.

@@ -XXX,XX +XXX,XX @@ void net_cleanup(void)

while (*p) {

nc = *p;

if (nc->info->type == NET_CLIENT_DRIVER_NIC) {

+ NICState *nic = qemu_get_nic(nc);

+ if (nic->peer_deleted) {

+ int queues = MAX(nic->conf->peers.queues, 1);

+

+ for (int i = 0; i < queues; i++) {

+ nc = qemu_get_subqueue(nic, i);

+ qemu_cleanup_net_client(nc->peer, false);

+ }

+ }

+

/* Skip NET_CLIENT_DRIVER_NIC entries */

p = &QTAILQ_NEXT(nc, next);

} else {

diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c

index XXXXXXX..XXXXXXX 100644

--- a/net/vhost-vdpa.c

+++ b/net/vhost-vdpa.c

@@ -XXX,XX +XXX,XX @@ static void vhost_vdpa_cleanup(NetClientState *nc)

{

VhostVDPAState *s = DO_UPCAST(VhostVDPAState, nc, nc);

- /*

- * If a peer NIC is attached, do not cleanup anything.

- * Cleanup will happen as a part of qemu_cleanup() -> net_cleanup()

- * when the guest is shutting down.

- */

- if (nc->peer && nc->peer->info->type == NET_CLIENT_DRIVER_NIC) {

- return;

- }

munmap(s->cvq_cmd_out_buffer, vhost_vdpa_net_cvq_cmd_page_len());

munmap(s->status, vhost_vdpa_net_cvq_cmd_page_len());

if (s->vhost_net) {

2.42.0

From: Akihiko Odaki <akihiko.odaki@daynix.com>

iov_from_buf(), iov_to_buf(), iov_memset(), and iov_copy() asserts

that the given offset fits in the iov while tolerating the specified

number of bytes to operate with to be greater than the size of iov.

This is inconsistent so remove the assertions.

Asserting the offset fits in the iov makes sense if it is expected that

there are other operations that process the content before the offset

and the content is processed in order. Under this expectation, the

offset should point to the end of bytes that are previously processed

and fit in the iov. However, this expectation depends on the details of

the caller, and did not hold true at least one case and required code to

check iov_size(), which is added with commit 83ddb3dbba2e

("hw/net/net_tx_pkt: Fix overrun in update_sctp_checksum()").

Adding such a check is inefficient and error-prone. These functions

already tolerate the specified number of bytes to operate with to be

greater than the size of iov to avoid such checks so remove the

assertions to tolerate invalid offset as well. They return the number of

bytes they operated with so their callers can still check the returned

value to ensure there are sufficient space at the given offset.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>

include/qemu/iov.h | 5 +++--

util/iov.c | 5 -----

2 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/include/qemu/iov.h b/include/qemu/iov.h

--- a/include/qemu/iov.h

+++ b/include/qemu/iov.h

@@ -XXX,XX +XXX,XX @@ size_t iov_size(const struct iovec *iov, const unsigned int iov_cnt);

* only part of data will be copied, up to the end of the iovec.

* Number of bytes actually copied will be returned, which is

* min(bytes, iov_size(iov)-offset)

- * `Offset' must point to the inside of iovec.

+ * Returns 0 when `offset' points to the outside of iovec.

*/

size_t iov_from_buf_full(const struct iovec *iov, unsigned int iov_cnt,

size_t offset, const void *buf, size_t bytes);

@@ -XXX,XX +XXX,XX @@ iov_to_buf(const struct iovec *iov, const unsigned int iov_cnt,

/**

* Set data bytes pointed out by iovec `iov' of size `iov_cnt' elements,

* starting at byte offset `start', to value `fillc', repeating it

- * `bytes' number of times. `Offset' must point to the inside of iovec.

+ * `bytes' number of times.

* If `bytes' is large enough, only last bytes portion of iovec,

* up to the end of it, will be filled with the specified value.

* Function return actual number of bytes processed, which is

* min(size, iov_size(iov) - offset).

+ * Returns 0 when `offset' points to the outside of iovec.

*/

size_t iov_memset(const struct iovec *iov, const unsigned int iov_cnt,

size_t offset, int fillc, size_t bytes);

diff --git a/util/iov.c b/util/iov.c

index XXXXXXX..XXXXXXX 100644

--- a/util/iov.c

+++ b/util/iov.c

@@ -XXX,XX +XXX,XX @@ size_t iov_from_buf_full(const struct iovec *iov, unsigned int iov_cnt,

offset -= iov[i].iov_len;

- assert(offset == 0);

return done;

}

@@ -XXX,XX +XXX,XX @@ size_t iov_to_buf_full(const struct iovec *iov, const unsigned int iov_cnt,

offset -= iov[i].iov_len;

}

}

- assert(offset == 0);

return done;

}

@@ -XXX,XX +XXX,XX @@ size_t iov_memset(const struct iovec *iov, const unsigned int iov_cnt,

offset -= iov[i].iov_len;

}

}

- assert(offset == 0);

return done;

}

@@ -XXX,XX +XXX,XX @@ unsigned iov_copy(struct iovec *dst_iov, unsigned int dst_iov_cnt,

bytes -= len;

offset = 0;

}

- assert(offset == 0);

return j;

}

@@ -XXX,XX +XXX,XX @@ size_t qemu_iovec_concat_iov(QEMUIOVector *dst,

soffset -= src_iov[i].iov_len;

}

}

- assert(soffset == 0); /* offset beyond end of src */

return done;

2.42.0

From: Akihiko Odaki <akihiko.odaki@daynix.com>

This reverts commit 83ddb3dbba2ee0f1767442ae6ee665058aeb1093.

The added check is no longer necessary due to a change of

iov_from_buf().

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>

hw/net/net_tx_pkt.c | 4 ----

1 file changed, 4 deletions(-)

@@ -XXX,XX +XXX,XX @@ bool net_tx_pkt_update_sctp_checksum(struct NetTxPkt *pkt)

uint32_t csum = 0;

struct iovec *pl_start_frag = pkt->vec + NET_TX_PKT_PL_START_FRAG;

- if (iov_size(pl_start_frag, pkt->payload_frags) < 8 + sizeof(csum)) {

- return false;

- }

-

if (iov_from_buf(pl_start_frag, pkt->payload_frags, 8, &csum, sizeof(csum)) < sizeof(csum)) {

return false;

}

2.42.0

From: Akihiko Odaki <akihiko.odaki@daynix.com>

ipvtap and macvtap create a file for each interface unlike tuntap, which

creates one file shared by all interfaces. Try to open a file dedicated

to the interface first for ipvtap and macvtap.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>

net/tap-linux.c | 17 ++++++++++++++---

1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/net/tap-linux.c b/net/tap-linux.c

--- a/net/tap-linux.c

+++ b/net/tap-linux.c

@@ -XXX,XX +XXX,XX @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr,

int len = sizeof(struct virtio_net_hdr);

unsigned int features;

- fd = RETRY_ON_EINTR(open(PATH_NET_TUN, O_RDWR));

+

+ ret = if_nametoindex(ifname);

+ if (ret) {

+ g_autofree char *file = g_strdup_printf("/dev/tap%d", ret);

+ fd = open(file, O_RDWR);

+ } else {

+ fd = -1;

+ }

+

if (fd < 0) {

- error_setg_errno(errp, errno, "could not open %s", PATH_NET_TUN);

- return -1;

+ fd = RETRY_ON_EINTR(open(PATH_NET_TUN, O_RDWR));

+ if (fd < 0) {

+ error_setg_errno(errp, errno, "could not open %s", PATH_NET_TUN);

+ return -1;

+ }

}

memset(&ifr, 0, sizeof(ifr));

ifr.ifr_flags = IFF_TAP | IFF_NO_PI;

2.42.0