d/freebsd-skq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

iflib: properly release memory allocated for DMA

Browse Source 
DMA memory allocations using the bus_dma.h interface are not properly
released in all cases for both Tx and Rx. This causes ~448 bytes of
M_DEVBUF allocations to be leaked.

First, the DMA maps for Rx are not properly destroyed. A slight attempt
is made in iflib_fl_bufs_free to destroy the maps if we're detaching.
However, this function may not be reliably called during detach. Indeed,
there is a comment "asking" if this should be moved out.

Fix this by moving the bus_dmamap_destroy call into iflib_rx_sds_free,
where we already sync and unload the DMA.

Second, the DMA tag associated with the ifr_ifdi descriptor DMA is not
released properly anywhere. Add a call to iflib_dma_free in
iflib_rx_structures_free.

Third, use of NULL as a canary value on the map pointer returned by
bus_dmamap_create is not valid. On some platforms, notably x86, this
value may be NULL. In this case, we fail to properly release the related
resources.

Remove the NULL checks on map values in both iflib_fl_bufs_free and
iflib_txsd_destroy.

With all of these fixes applied, the leaks to M_DEVBUF are squelched,
and iflib drivers now seem to properly cleanup when detaching.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>

Submitted by:	Jacob Keller <jacob.e.keller@intel.com>
Reviewed by:	erj@, gallatin@
MFC after:	1 week
Sponsored by:	Intel Corporation
Differential Revision:	https://reviews.freebsd.org/D22203
This commit is contained in:
erj 2019-11-04 23:06:57 +00:00
parent d3f2d7a9be
commit 08b39c8928
1 changed files with 9 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
sys/net/iflib.c
View File

@ -1696,20 +1696,16 @@ iflib_txsd_destroy(if_ctx_t ctx, iflib_txq_t txq, int i)
{
bus_dmamap_t map;
map = NULL;
if (txq->ift_sds.ifsd_map != NULL)
if (txq->ift_sds.ifsd_map != NULL) {
map = txq->ift_sds.ifsd_map[i];
if (map != NULL) {
bus_dmamap_sync(txq->ift_buf_tag, map, BUS_DMASYNC_POSTWRITE);
bus_dmamap_unload(txq->ift_buf_tag, map);
bus_dmamap_destroy(txq->ift_buf_tag, map);
txq->ift_sds.ifsd_map[i] = NULL;
}
map = NULL;
if (txq->ift_sds.ifsd_tso_map != NULL)
if (txq->ift_sds.ifsd_tso_map != NULL) {
map = txq->ift_sds.ifsd_tso_map[i];
if (map != NULL) {
bus_dmamap_sync(txq->ift_tso_buf_tag, map,
BUS_DMASYNC_POSTWRITE);
bus_dmamap_unload(txq->ift_tso_buf_tag, map);
@ -2120,9 +2116,6 @@ iflib_fl_bufs_free(iflib_fl_t fl)
bus_dmamap_unload(fl->ifl_buf_tag, sd_map);
if (*sd_cl != NULL)
uma_zfree(fl->ifl_zone, *sd_cl);
// XXX: Should this get moved out?
if (iflib_in_detach(fl->ifl_rxq->ifr_ctx))
bus_dmamap_destroy(fl->ifl_buf_tag, sd_map);
if (*sd_m != NULL) {
m_init(*sd_m, M_NOWAIT, MT_DATA, 0);
uma_zfree(zone_mbuf, *sd_m);
@ -2210,9 +2203,6 @@ iflib_rx_sds_free(iflib_rxq_t rxq)
if (fl->ifl_buf_tag != NULL) {
if (fl->ifl_sds.ifsd_map != NULL) {
for (j = 0; j < fl->ifl_size; j++) {
if (fl->ifl_sds.ifsd_map[j] ==
NULL)
continue;
bus_dmamap_sync(
fl->ifl_buf_tag,
fl->ifl_sds.ifsd_map[j],
@ -2220,6 +2210,9 @@ iflib_rx_sds_free(iflib_rxq_t rxq)
bus_dmamap_unload(
fl->ifl_buf_tag,
fl->ifl_sds.ifsd_map[j]);
bus_dmamap_destroy(
fl->ifl_buf_tag,
fl->ifl_sds.ifsd_map[j]);
}
}
bus_dma_tag_destroy(fl->ifl_buf_tag);
@ -5735,9 +5728,12 @@ static void
iflib_rx_structures_free(if_ctx_t ctx)
{
iflib_rxq_t rxq = ctx->ifc_rxqs;
int i;
if_shared_ctx_t sctx = ctx->ifc_sctx;
int i, j;
for (i = 0; i < ctx->ifc_softc_ctx.isc_nrxqsets; i++, rxq++) {
for (j = 0; j < sctx->isc_nrxqs; j++)
iflib_dma_free(&rxq->ifr_ifdi[j]);
iflib_rx_sds_free(rxq);
#if defined(INET6) || defined(INET)
if (if_getcapabilities(ctx->ifc_ifp) & IFCAP_LRO)