Skip to content

Commit e7891c5

Browse files
eugpermarjasowang
authored andcommitted
net: move backend cleanup to NIC cleanup
Commit a0d7215 ("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: a0d7215 ("vhost-vdpa: do not cleanup the vdpa/vhost-net structures if peer nic is present") Reported-by: Lei Yang <[email protected]> Signed-off-by: Eugenio Pérez <[email protected]> Signed-off-by: Jonah Palmer <[email protected]> Signed-off-by: Jason Wang <[email protected]>
1 parent db0d401 commit e7891c5

File tree

2 files changed

+27
-14
lines changed

2 files changed

+27
-14
lines changed

net/net.c

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -428,7 +428,13 @@ void qemu_del_net_client(NetClientState *nc)
428428
object_unparent(OBJECT(nf));
429429
}
430430

431-
/* If there is a peer NIC, delete and cleanup client, but do not free. */
431+
/*
432+
* If there is a peer NIC, transfer ownership to it. Delete the client
433+
* from net_client list but do not cleanup nor free. This way NIC can
434+
* still access to members of the backend.
435+
*
436+
* The cleanup and free will be done when the NIC is free.
437+
*/
432438
if (nc->peer && nc->peer->info->type == NET_CLIENT_DRIVER_NIC) {
433439
NICState *nic = qemu_get_nic(nc->peer);
434440
if (nic->peer_deleted) {
@@ -438,16 +444,13 @@ void qemu_del_net_client(NetClientState *nc)
438444

439445
for (i = 0; i < queues; i++) {
440446
ncs[i]->peer->link_down = true;
447+
QTAILQ_REMOVE(&net_clients, ncs[i], next);
441448
}
442449

443450
if (nc->peer->info->link_status_changed) {
444451
nc->peer->info->link_status_changed(nc->peer);
445452
}
446453

447-
for (i = 0; i < queues; i++) {
448-
qemu_cleanup_net_client(ncs[i], true);
449-
}
450-
451454
return;
452455
}
453456

@@ -465,8 +468,12 @@ void qemu_del_nic(NICState *nic)
465468

466469
for (i = 0; i < queues; i++) {
467470
NetClientState *nc = qemu_get_subqueue(nic, i);
468-
/* If this is a peer NIC and peer has already been deleted, free it now. */
471+
/*
472+
* If this is a peer NIC and peer has already been deleted, clean it up
473+
* and free it now.
474+
*/
469475
if (nic->peer_deleted) {
476+
qemu_cleanup_net_client(nc->peer, false);
470477
qemu_free_net_client(nc->peer);
471478
} else if (nc->peer) {
472479
/* if there are RX packets pending, complete them */
@@ -1681,13 +1688,27 @@ void net_cleanup(void)
16811688
* of the latest NET_CLIENT_DRIVER_NIC, and operate on *p as we walk
16821689
* the list.
16831690
*
1691+
* However, the NIC may have peers that trust to be clean beyond this
1692+
* point. For example, if they have been removed with device_del.
1693+
*
16841694
* The 'nc' variable isn't part of the list traversal; it's purely
16851695
* for convenience as too much '(*p)->' has a tendency to make the
16861696
* readers' eyes bleed.
16871697
*/
16881698
while (*p) {
16891699
nc = *p;
16901700
if (nc->info->type == NET_CLIENT_DRIVER_NIC) {
1701+
NICState *nic = qemu_get_nic(nc);
1702+
1703+
if (nic->peer_deleted) {
1704+
int queues = MAX(nic->conf->peers.queues, 1);
1705+
1706+
for (int i = 0; i < queues; i++) {
1707+
nc = qemu_get_subqueue(nic, i);
1708+
qemu_cleanup_net_client(nc->peer, false);
1709+
}
1710+
}
1711+
16911712
/* Skip NET_CLIENT_DRIVER_NIC entries */
16921713
p = &QTAILQ_NEXT(nc, next);
16931714
} else {

net/vhost-vdpa.c

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -224,14 +224,6 @@ static void vhost_vdpa_cleanup(NetClientState *nc)
224224
{
225225
VhostVDPAState *s = DO_UPCAST(VhostVDPAState, nc, nc);
226226

227-
/*
228-
* If a peer NIC is attached, do not cleanup anything.
229-
* Cleanup will happen as a part of qemu_cleanup() -> net_cleanup()
230-
* when the guest is shutting down.
231-
*/
232-
if (nc->peer && nc->peer->info->type == NET_CLIENT_DRIVER_NIC) {
233-
return;
234-
}
235227
munmap(s->cvq_cmd_out_buffer, vhost_vdpa_net_cvq_cmd_page_len());
236228
munmap(s->status, vhost_vdpa_net_cvq_cmd_page_len());
237229
if (s->vhost_net) {

0 commit comments

Comments
 (0)