git: 318bbb6d5a1e - main - xen-netfront: attempt to make cleanup idempotent

From: Roger Pau Monné <royger_at_FreeBSD.org>
Date: Mon, 06 Nov 2023 10:17:48 UTC
The branch main has been updated by royger:

URL: https://cgit.FreeBSD.org/src/commit/?id=318bbb6d5a1eae77eb5dc687bcc63c0f99558e21

commit 318bbb6d5a1eae77eb5dc687bcc63c0f99558e21
Author:     Roger Pau Monné <royger@FreeBSD.org>
AuthorDate: 2023-11-03 09:28:16 +0000
Commit:     Roger Pau Monné <royger@FreeBSD.org>
CommitDate: 2023-11-06 10:17:40 +0000

    xen-netfront: attempt to make cleanup idempotent
    
    Current cleanup code assumes that all the fields are allocated and/or setup by
    the time cleanup is called, but this is not always true: a failure in mid-setup
    of the device will cause the functions to be called with possibly uninitialized
    fields.
    
    Fix the functions to cope with such sate, while also attempting to make the
    cleanup idempotent.
    
    Finally fix an error path during setup that would not mark the device as
    closed, and hence prevents the kernel from finishing booting.
    
    Fixes: 96375eac945c ("xen-netfront: add multiqueue support")
    Sponsored by: Citrix Systems R&D
---
 sys/dev/xen/netfront/netfront.c | 27 ++++++++++++++++++++++-----
 1 file changed, 22 insertions(+), 5 deletions(-)

diff --git a/sys/dev/xen/netfront/netfront.c b/sys/dev/xen/netfront/netfront.c
index dafb838cf328..6ac6ecc3bdb7 100644
--- a/sys/dev/xen/netfront/netfront.c
+++ b/sys/dev/xen/netfront/netfront.c
@@ -589,8 +589,10 @@ talk_to_backend(device_t dev, struct netfront_info *info)
 		num_queues = max_queues;
 
 	err = setup_device(dev, info, num_queues);
-	if (err != 0)
+	if (err != 0) {
+		xenbus_dev_fatal(dev, err, "setup device");
 		goto out;
+	}
 
  again:
 	err = xs_transaction_start(&xst);
@@ -717,7 +719,10 @@ disconnect_rxq(struct netfront_rxq *rxq)
 
 	xn_release_rx_bufs(rxq);
 	gnttab_free_grant_references(rxq->gref_head);
-	gnttab_end_foreign_access(rxq->ring_ref, NULL);
+	if (rxq->ring_ref != GRANT_REF_INVALID) {
+		gnttab_end_foreign_access(rxq->ring_ref, NULL);
+		rxq->ring_ref = GRANT_REF_INVALID;
+	}
 	/*
 	 * No split event channel support at the moment, handle will
 	 * be unbound in tx. So no need to call xen_intr_unbind here,
@@ -732,6 +737,7 @@ destroy_rxq(struct netfront_rxq *rxq)
 
 	callout_drain(&rxq->rx_refill);
 	free(rxq->ring.sring, M_DEVBUF);
+	rxq->ring.sring = NULL;
 }
 
 static void
@@ -763,6 +769,8 @@ setup_rxqs(device_t dev, struct netfront_info *info,
 
 		rxq->id = q;
 		rxq->info = info;
+
+		rxq->gref_head = GNTTAB_LIST_END;
 		rxq->ring_ref = GRANT_REF_INVALID;
 		rxq->ring.sring = NULL;
 		snprintf(rxq->name, XN_QUEUE_NAME_LEN, "xnrx_%u", q);
@@ -819,7 +827,10 @@ disconnect_txq(struct netfront_txq *txq)
 
 	xn_release_tx_bufs(txq);
 	gnttab_free_grant_references(txq->gref_head);
-	gnttab_end_foreign_access(txq->ring_ref, NULL);
+	if (txq->ring_ref != GRANT_REF_INVALID) {
+		gnttab_end_foreign_access(txq->ring_ref, NULL);
+		txq->ring_ref = GRANT_REF_INVALID;
+	}
 	xen_intr_unbind(&txq->xen_intr_handle);
 }
 
@@ -829,9 +840,14 @@ destroy_txq(struct netfront_txq *txq)
 	unsigned int i;
 
 	free(txq->ring.sring, M_DEVBUF);
+	txq->ring.sring = NULL;
 	buf_ring_free(txq->br, M_DEVBUF);
-	taskqueue_drain_all(txq->tq);
-	taskqueue_free(txq->tq);
+	txq->br = NULL;
+	if (txq->tq) {
+		taskqueue_drain_all(txq->tq);
+		taskqueue_free(txq->tq);
+		txq->tq = NULL;
+	}
 
 	for (i = 0; i <= NET_TX_RING_SIZE; i++) {
 		bus_dmamap_destroy(txq->info->dma_tag,
@@ -870,6 +886,7 @@ setup_txqs(device_t dev, struct netfront_info *info,
 		txq->id = q;
 		txq->info = info;
 
+		txq->gref_head = GNTTAB_LIST_END;
 		txq->ring_ref = GRANT_REF_INVALID;
 		txq->ring.sring = NULL;