svn commit: r354344 - head/sys/net

[Eric Joyner]

Author: erj
Date: Mon Nov  4 23:06:57 2019
New Revision: 354344
URL: https://svnweb.freebsd.org/changeset/base/354344

Log:
  iflib: properly release memory allocated for DMA
  
  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 at intel.com>
  
  Submitted by:	Jacob Keller <jacob.e.keller at intel.com>
  Reviewed by:	erj@, gallatin@
  MFC after:	1 week
  Sponsored by:	Intel Corporation
  Differential Revision:	https://reviews.freebsd.org/D22203

Modified:
  head/sys/net/iflib.c

Modified: head/sys/net/iflib.c
==============================================================================
--- head/sys/net/iflib.c	Mon Nov  4 22:57:36 2019	(r354343)
+++ head/sys/net/iflib.c	Mon Nov  4 23:06:57 2019	(r354344)
@@ -1696,20 +1696,16 @@ iflib_txsd_destroy(if_ctx_t ctx, iflib_txq_t txq, int 
 {
 	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)