PERFORCE change 127403 for review

[Kip Macy]

http://perforce.freebsd.org/chv.cgi?CH=127403

Change 127403 by kmacy at kmacy_home:ethng on 2007/10/12 03:06:45

	fix handling of case where packet header mbuf contained zero bytes of data
	- don't modify initial mbuf chain while iterating through it
	- when collapsing mbufs do it from the array and not the chain
	- when freeing mbufs do it from the chain and not the array
	
	skip special handling of mbuf chain with only buffer but multiple mbufs
	probably not common enough to special case and the way it was being handled
	could leak memory

Affected files ...

.. //depot/projects/ethng/src/sys/dev/cxgb/sys/uipc_mvec.c#15 edit

Differences ...

==== //depot/projects/ethng/src/sys/dev/cxgb/sys/uipc_mvec.c#15 (text+ko) ====

@@ -125,7 +125,6 @@
 
 	mi->mi_flags = m->m_flags;
 	mi->mi_len = m->m_len;
-
 	
 	if (m->m_flags & M_PKTHDR) {
 		mi->mi_ether_vtag = m->m_pkthdr.ether_vtag;
@@ -141,6 +140,7 @@
 		mi->mi_size = (m->m_type == EXT_CLIOVEC) ? MCLBYTES : MIOVBYTES;
 		mi->mi_type = m->m_type;
 		mi->mi_len = m->m_pkthdr.len;
+		KASSERT(mi->mi_len, ("empty packet"));
 		mi->mi_refcnt = NULL;
 	} else if (m->m_flags & M_EXT) {
 		memcpy(&mi->mi_ext, &m->m_ext, sizeof(struct m_ext_));
@@ -171,11 +171,12 @@
 int
 busdma_map_sg_collapse(struct mbuf **m, bus_dma_segment_t *segs, int *nsegs)
 {
-	struct mbuf *m0, *n = *m;
+	struct mbuf *m0, *n = *m, *mhead;
 	struct mbuf_iovec *mi;
 	struct mbuf *marray[TX_MAX_SEGS];
 	int i, type, seg_count, defragged = 0, err = 0;
 	struct mbuf_vec *mv;
+	uma_zone_t zone;
 
 	if (n->m_flags & M_PKTHDR && !SLIST_EMPTY(&n->m_pkthdr.tags)) 
 		m_tag_delete_chain(n, NULL);
@@ -221,20 +222,23 @@
 		/*
 		 * firmware doesn't like empty segments
 		 */
-		if (__predict_false(n->m_len == 0)) {
-			n = m_free(n);
-			if (seg_count)
-				marray[seg_count - 1]->m_next = n;
-			continue;
-		}
-		seg_count++;
+		if (__predict_true(n->m_len != 0)) 
+			seg_count++;
+
 		n = n->m_next;
 	}
+#if 0
+	/*
+	 * XXX needs more careful consideration
+	 */
 	if (__predict_false(seg_count == 1)) {
-		n = *m;
+		n = marray[0];
+		if (n != *m)
+			
 		/* XXX */
 		goto retry;
 	}
+#endif	
 	if (seg_count == 0) {
 		if (cxgb_debug)
 			printf("empty segment chain\n");
@@ -257,43 +261,41 @@
 		goto err_out;
 	}
 	if (seg_count > MAX_CL_IOV) {
-		if ((m0 = uma_zalloc_arg(zone_jumbop, NULL, M_NOWAIT)) == NULL) {
-			err = ENOMEM;
-			goto err_out;
-		}
+		zone = zone_jumbop;
 		type = EXT_JMPIOVEC;
 	} else if (seg_count > MAX_MIOVEC_IOV) {
-		DPRINTF("seg count=%d ", seg_count);
-		if ((m0 = uma_zalloc_arg(zone_clust, NULL, M_NOWAIT)) == NULL) {
-			err = ENOMEM;
-			goto err_out;
-		}
+		zone = zone_clust;
 		type = EXT_CLIOVEC;
 	} else {
-		if ((m0 = uma_zalloc_arg(zone_miovec, NULL, M_NOWAIT)) == NULL) {
-			err = ENOMEM;
-			goto err_out;
-		}
 		type = EXT_IOVEC;
+		zone = zone_miovec;
 	}
+	if ((m0 = uma_zalloc_arg(zone, NULL, M_NOWAIT)) == NULL) {
+		err = ENOMEM;
+		goto err_out;
+	}
 	
-	n = marray[0];
-	memcpy(m0, n, sizeof(struct m_hdr) + sizeof(struct pkthdr));
+	memcpy(m0, *m, sizeof(struct m_hdr) + sizeof(struct pkthdr));
 	m0->m_type = type;
 	mv = mtomv(m0);
 	mv->mv_count = seg_count;
 	mv->mv_first = 0;
-
-	for (mi = mv->mv_vec; n; mi++, segs++) {
+	for (i = 0, mi = mv->mv_vec; i < seg_count; mi++, segs++, i++) {
+		n = marray[i];
 		busdma_map_mbuf_fast(n, segs);
-		n = _mcl_collapse_mbuf(mi, n);
+		_mcl_collapse_mbuf(mi, n);
 	}
-	for (i = 0; i < seg_count; i++) {
-		marray[i]->m_next = marray[i]->m_nextpkt = NULL;
-		if ((marray[i]->m_flags & (M_EXT|M_NOFREE)) == M_EXT) {
-			marray[i]->m_flags &= ~M_EXT; 
-			m_free(marray[i]);
+	n = *m;
+	while (n) {
+		if (((n->m_flags & (M_EXT|M_NOFREE)) == M_EXT) && (n->m_len > 0))
+			n->m_flags &= ~M_EXT; 
+		else if (n->m_len > 0) {
+			n = n->m_next;
+			continue;
 		}
+		mhead = n->m_next;
+		m_free(n);
+		n = mhead;
 	}
 	*nsegs = seg_count;
 	*m = m0;