Changeset View
Standalone View
sys/net/iflib.c
Show First 20 Lines • Show All 4,894 Lines • ▼ Show 20 Lines | if (msix > 1) { | ||||
if ((err = iflib_legacy_setup(ctx, ctx->isc_legacy_intr, ctx->ifc_softc, &rid, "irq0")) != 0) { | if ((err = iflib_legacy_setup(ctx, ctx->isc_legacy_intr, ctx->ifc_softc, &rid, "irq0")) != 0) { | ||||
device_printf(dev, "iflib_legacy_setup failed %d\n", err); | device_printf(dev, "iflib_legacy_setup failed %d\n", err); | ||||
goto fail_queues; | goto fail_queues; | ||||
} | } | ||||
} else { | } else { | ||||
device_printf(dev, | device_printf(dev, | ||||
"Cannot use iflib with only 1 MSI-X interrupt!\n"); | "Cannot use iflib with only 1 MSI-X interrupt!\n"); | ||||
err = ENODEV; | err = ENODEV; | ||||
goto fail_intr_free; | goto fail_queues; | ||||
} | } | ||||
ether_ifattach(ctx->ifc_ifp, ctx->ifc_mac.octet); | ether_ifattach(ctx->ifc_ifp, ctx->ifc_mac.octet); | ||||
if ((err = IFDI_ATTACH_POST(ctx)) != 0) { | if ((err = IFDI_ATTACH_POST(ctx)) != 0) { | ||||
device_printf(dev, "IFDI_ATTACH_POST failed %d\n", err); | device_printf(dev, "IFDI_ATTACH_POST failed %d\n", err); | ||||
goto fail_detach; | goto fail_detach; | ||||
} | } | ||||
Show All 19 Lines | #endif | ||||
iflib_add_pfil(ctx); | iflib_add_pfil(ctx); | ||||
ctx->ifc_flags |= IFC_INIT_DONE; | ctx->ifc_flags |= IFC_INIT_DONE; | ||||
CTX_UNLOCK(ctx); | CTX_UNLOCK(ctx); | ||||
return (0); | return (0); | ||||
fail_detach: | fail_detach: | ||||
ether_ifdetach(ctx->ifc_ifp); | ether_ifdetach(ctx->ifc_ifp); | ||||
fail_intr_free: | |||||
iflib_free_intr_mem(ctx); | |||||
fail_queues: | fail_queues: | ||||
iflib_tqg_detach(ctx); | |||||
iflib_tx_structures_free(ctx); | iflib_tx_structures_free(ctx); | ||||
iflib_rx_structures_free(ctx); | iflib_rx_structures_free(ctx); | ||||
iflib_tqg_detach(ctx); | |||||
IFDI_DETACH(ctx); | IFDI_DETACH(ctx); | ||||
IFDI_QUEUES_FREE(ctx); | |||||
fail_intr_free: | |||||
iflib_free_intr_mem(ctx); | |||||
fail_unlock: | fail_unlock: | ||||
CTX_UNLOCK(ctx); | CTX_UNLOCK(ctx); | ||||
iflib_deregister(ctx); | iflib_deregister(ctx); | ||||
fail_ctx_free: | fail_ctx_free: | ||||
device_set_softc(ctx->ifc_dev, NULL); | device_set_softc(ctx->ifc_dev, NULL); | ||||
if (ctx->ifc_flags & IFC_SC_ALLOCATED) | if (ctx->ifc_flags & IFC_SC_ALLOCATED) | ||||
free(ctx->ifc_softc, M_IFLIB); | free(ctx->ifc_softc, M_IFLIB); | ||||
free(ctx, M_IFLIB); | free(ctx, M_IFLIB); | ||||
▲ Show 20 Lines • Show All 180 Lines • ▼ Show 20 Lines | #endif | ||||
iflib_add_device_sysctl_post(ctx); | iflib_add_device_sysctl_post(ctx); | ||||
ctx->ifc_flags |= IFC_INIT_DONE; | ctx->ifc_flags |= IFC_INIT_DONE; | ||||
CTX_UNLOCK(ctx); | CTX_UNLOCK(ctx); | ||||
return (0); | return (0); | ||||
fail_detach: | fail_detach: | ||||
ether_ifdetach(ctx->ifc_ifp); | ether_ifdetach(ctx->ifc_ifp); | ||||
fail_queues: | fail_queues: | ||||
iflib_tqg_detach(ctx); | |||||
iflib_tx_structures_free(ctx); | iflib_tx_structures_free(ctx); | ||||
iflib_rx_structures_free(ctx); | iflib_rx_structures_free(ctx); | ||||
iflib_tqg_detach(ctx); | |||||
fail_iflib_detach: | fail_iflib_detach: | ||||
IFDI_DETACH(ctx); | IFDI_DETACH(ctx); | ||||
IFDI_QUEUES_FREE(ctx); | |||||
fail_unlock: | fail_unlock: | ||||
CTX_UNLOCK(ctx); | CTX_UNLOCK(ctx); | ||||
iflib_deregister(ctx); | iflib_deregister(ctx); | ||||
fail_ctx_free: | fail_ctx_free: | ||||
free(ctx->ifc_softc, M_IFLIB); | free(ctx->ifc_softc, M_IFLIB); | ||||
free(ctx, M_IFLIB); | free(ctx, M_IFLIB); | ||||
return (err); | return (err); | ||||
} | } | ||||
Show All 13 Lines | if ((sctx->isc_flags & IFLIB_PSEUDO) && | ||||
if_detach(ifp); | if_detach(ifp); | ||||
} else { | } else { | ||||
ether_ifdetach(ifp); | ether_ifdetach(ifp); | ||||
} | } | ||||
iflib_tqg_detach(ctx); | iflib_tqg_detach(ctx); | ||||
iflib_tx_structures_free(ctx); | iflib_tx_structures_free(ctx); | ||||
iflib_rx_structures_free(ctx); | iflib_rx_structures_free(ctx); | ||||
IFDI_DETACH(ctx); | |||||
IFDI_QUEUES_FREE(ctx); | |||||
markj: It looks like we're missing a call to IFDI_QUEUES_FREE here. | |||||
Done Inline ActionsThanks for catching this. Also we are missing IFDI_DETACH(ctx) too. Will add that too and refresh the review. stallamr_netapp.com: Thanks for catching this.
Also we are missing IFDI_DETACH(ctx) too. Will add that too and… | |||||
iflib_deregister(ctx); | iflib_deregister(ctx); | ||||
if (ctx->ifc_flags & IFC_SC_ALLOCATED) | if (ctx->ifc_flags & IFC_SC_ALLOCATED) | ||||
free(ctx->ifc_softc, M_IFLIB); | free(ctx->ifc_softc, M_IFLIB); | ||||
free(ctx, M_IFLIB); | free(ctx, M_IFLIB); | ||||
return (0); | return (0); | ||||
} | } | ||||
▲ Show 20 Lines • Show All 42 Lines • ▼ Show 20 Lines | #endif | ||||
CTX_LOCK(ctx); | CTX_LOCK(ctx); | ||||
iflib_stop(ctx); | iflib_stop(ctx); | ||||
CTX_UNLOCK(ctx); | CTX_UNLOCK(ctx); | ||||
iflib_rem_pfil(ctx); | iflib_rem_pfil(ctx); | ||||
if (ctx->ifc_led_dev != NULL) | if (ctx->ifc_led_dev != NULL) | ||||
led_destroy(ctx->ifc_led_dev); | led_destroy(ctx->ifc_led_dev); | ||||
iflib_tqg_detach(ctx); | iflib_tqg_detach(ctx); | ||||
iflib_tx_structures_free(ctx); | |||||
iflib_rx_structures_free(ctx); | |||||
CTX_LOCK(ctx); | CTX_LOCK(ctx); | ||||
IFDI_DETACH(ctx); | IFDI_DETACH(ctx); | ||||
IFDI_QUEUES_FREE(ctx); | |||||
CTX_UNLOCK(ctx); | CTX_UNLOCK(ctx); | ||||
Not Done Inline ActionsShould this move down to just before the iflib_tx_structures_free() call, for consistency with other teardown paths? markj: Should this move down to just before the iflib_tx_structures_free() call, for consistency with… | |||||
Done Inline Actionsagree. Will make the change. stallamr_netapp.com: agree. Will make the change. | |||||
/* ether_ifdetach calls if_qflush - lock must be destroy afterwards*/ | /* ether_ifdetach calls if_qflush - lock must be destroy afterwards*/ | ||||
iflib_free_intr_mem(ctx); | iflib_free_intr_mem(ctx); | ||||
Done Inline ActionsHmm, sorry for not noticing sooner, but there is an ordering issue here. IFDI_DETACH releases the irq resources for the driver, but iflib_free_intr_mem() is responsible for releasing MSI-X vectors back to the bus. This is happening in the wrong order now, so I believe the pci_release_msi() call will fail. markj: Hmm, sorry for not noticing sooner, but there is an ordering issue here. IFDI_DETACH releases… | |||||
Done Inline ActionsThat is very true. Thanks for noticing this. pci_release_msi() was returning 16 i.,e EBUSY prior to the fix. stallamr_netapp.com: That is very true. Thanks for noticing this.
pci_release_msi() was returning 16 i.,e EBUSY… | |||||
bus_generic_detach(dev); | bus_generic_detach(dev); | ||||
iflib_tx_structures_free(ctx); | |||||
iflib_rx_structures_free(ctx); | |||||
iflib_deregister(ctx); | iflib_deregister(ctx); | ||||
device_set_softc(ctx->ifc_dev, NULL); | device_set_softc(ctx->ifc_dev, NULL); | ||||
Not Done Inline ActionsNow I'm wondering why we need separate DETACH and QUEUES_FREE methods... that's outside the scope of this change though. markj: Now I'm wondering why we need separate DETACH and QUEUES_FREE methods... that's outside the… | |||||
Done Inline ActionsYou are right. We can absorb IFDI_QUEUES_FREE() into IFDI_DETACH(). May be the real intention to keep separate is to handle iflib_queues_alloc() failure. But iflib_queues_alloc() failure trickle down and eventually becomes iflib_device_register() failure, where we do IFDI_DETACH() anyway. stallamr_netapp.com: You are right. We can absorb IFDI_QUEUES_FREE() into IFDI_DETACH().
May be the real intention… | |||||
if (ctx->ifc_flags & IFC_SC_ALLOCATED) | if (ctx->ifc_flags & IFC_SC_ALLOCATED) | ||||
free(ctx->ifc_softc, M_IFLIB); | free(ctx->ifc_softc, M_IFLIB); | ||||
unref_ctx_core_offset(ctx); | unref_ctx_core_offset(ctx); | ||||
free(ctx, M_IFLIB); | free(ctx, M_IFLIB); | ||||
return (0); | return (0); | ||||
} | } | ||||
static void | static void | ||||
▲ Show 20 Lines • Show All 564 Lines • ▼ Show 20 Lines | iflib_tx_structures_free(if_ctx_t ctx) | ||||
for (i = 0; i < NTXQSETS(ctx); i++, txq++) { | for (i = 0; i < NTXQSETS(ctx); i++, txq++) { | ||||
for (j = 0; j < sctx->isc_ntxqs; j++) | for (j = 0; j < sctx->isc_ntxqs; j++) | ||||
iflib_dma_free(&txq->ift_ifdi[j]); | iflib_dma_free(&txq->ift_ifdi[j]); | ||||
iflib_txq_destroy(txq); | iflib_txq_destroy(txq); | ||||
} | } | ||||
free(ctx->ifc_txqs, M_IFLIB); | free(ctx->ifc_txqs, M_IFLIB); | ||||
ctx->ifc_txqs = NULL; | ctx->ifc_txqs = NULL; | ||||
IFDI_QUEUES_FREE(ctx); | |||||
} | } | ||||
/********************************************************************* | /********************************************************************* | ||||
* | * | ||||
* Initialize all receive rings. | * Initialize all receive rings. | ||||
* | * | ||||
**********************************************************************/ | **********************************************************************/ | ||||
static int | static int | ||||
▲ Show 20 Lines • Show All 1,150 Lines • Show Last 20 Lines |
It looks like we're missing a call to IFDI_QUEUES_FREE here.