MFC r312604 and r312605:

Simplify the handling of small packets padding in cpsw:
 - Pad small packets to 60 bytes and not 64 (exclude the CRC bytes);
 - Pad the packet using m_append(9), if the packet has enough space for
   padding, which is usually true, it will not be necessary append a newly
   allocated mbuf to the chain.

Suggested by:	yongari

MFC r312608:

Handle the rx queue stall while reading the packets from NIC (when the
descriptor state will not change anymore).  This seems to eliminate the
race where we can miss a stalled queue under high load.

While here remove the unnecessary curly brackets.

Reported by:	Konstantin Kormashev <konstantin@netgate.com>

MFC r312636:

Properly assemble an mbuf chain out of received fragments.

Remove the rx_batch hack, it makes no difference now that most of bugs have
been sorted out.

MFC r312637:

Be a little more pedantic here, the TRM says the hardware is supposed to
only clean the OWNER bit on SOP descriptors.

Sponsored by:	Rubicon Communications, LLC (Netgate)

2 files changed, 73 insertions, 97 deletions

diff --git a/sys/arm/ti/cpsw/if_cpsw.c b/sys/arm/ti/cpsw/if_cpsw.c
index 0b47028..3feeddd 100644
--- a/sys/arm/ti/cpsw/if_cpsw.c
+++ b/sys/arm/ti/cpsw/if_cpsw.c
@@ -784,8 +784,7 @@ cpsw_get_fdt_data(struct cpsw_softc *sc, int port)
 static int
 cpsw_attach(device_t dev)
 {
-	bus_dma_segment_t segs[1];
-	int error, i, nsegs;
+	int error, i;
 	struct cpsw_softc *sc;
 	uint32_t reg;
 
@@ -860,15 +859,8 @@ cpsw_attach(device_t dev)
 		return (error);
 	}
 
-	/* Allocate the null mbuf and pre-sync it. */
-	sc->null_mbuf = m_getcl(M_NOWAIT, MT_DATA, M_PKTHDR);
-	memset(sc->null_mbuf->m_data, 0, sc->null_mbuf->m_ext.ext_size);
-	bus_dmamap_create(sc->mbuf_dtag, 0, &sc->null_mbuf_dmamap);
-	bus_dmamap_load_mbuf_sg(sc->mbuf_dtag, sc->null_mbuf_dmamap,
-	    sc->null_mbuf, segs, &nsegs, BUS_DMA_NOWAIT);
-	bus_dmamap_sync(sc->mbuf_dtag, sc->null_mbuf_dmamap,
-	    BUS_DMASYNC_PREWRITE);
-	sc->null_mbuf_paddr = segs[0].ds_addr;
+	/* Allocate a NULL buffer for padding. */
+	sc->nullpad = malloc(ETHER_MIN_LEN, M_DEVBUF, M_WAITOK | M_ZERO);
 
 	cpsw_init_slots(sc);
 
@@ -947,13 +939,9 @@ cpsw_detach(device_t dev)
 	for (i = 0; i < nitems(sc->_slots); ++i)
 		cpsw_free_slot(sc, &sc->_slots[i]);
 
-	/* Free null mbuf. */
-	if (sc->null_mbuf_dmamap) {
-		bus_dmamap_unload(sc->mbuf_dtag, sc->null_mbuf_dmamap);
-		error = bus_dmamap_destroy(sc->mbuf_dtag, sc->null_mbuf_dmamap);
-		KASSERT(error == 0, ("Mapping still active"));
-		m_freem(sc->null_mbuf);
-	}
+	/* Free null padding buffer. */
+	if (sc->nullpad)
+		free(sc->nullpad, M_DEVBUF);
 
 	/* Free DMA tag */
 	if (sc->mbuf_dtag) {
@@ -1595,14 +1583,19 @@ cpsw_intr_rx(void *arg)
 static struct mbuf *
 cpsw_rx_dequeue(struct cpsw_softc *sc)
 {
+	int nsegs, port, removed;
 	struct cpsw_cpdma_bd bd;
 	struct cpsw_slot *last, *slot;
 	struct cpswp_softc *psc;
-	struct mbuf *mb_head, *mb_tail;
-	int port, removed = 0;
+	struct mbuf *m, *m0, *mb_head, *mb_tail;
+	uint16_t m0_flags;
 
+	nsegs = 0;
+	m0 = NULL;
 	last = NULL;
-	mb_head = mb_tail = NULL;
+	mb_head = NULL;
+	mb_tail = NULL;
+	removed = 0;
 
 	/* Pull completed packets off hardware RX queue. */
 	while ((slot = STAILQ_FIRST(&sc->rx.active)) != NULL) {
@@ -1625,10 +1618,12 @@ cpsw_rx_dequeue(struct cpsw_softc *sc)
 		bus_dmamap_sync(sc->mbuf_dtag, slot->dmamap, BUS_DMASYNC_POSTREAD);
 		bus_dmamap_unload(sc->mbuf_dtag, slot->dmamap);
 
+		m = slot->mbuf;
+		slot->mbuf = NULL;
+
 		if (bd.flags & CPDMA_BD_TDOWNCMPLT) {
 			CPSW_DEBUGF(sc, ("RX teardown is complete"));
-			m_freem(slot->mbuf);
-			slot->mbuf = NULL;
+			m_freem(m);
 			sc->rx.running = 0;
 			sc->rx.teardown = 0;
 			break;
@@ -1640,41 +1635,63 @@ cpsw_rx_dequeue(struct cpsw_softc *sc)
 		psc = device_get_softc(sc->port[port].dev);
 
 		/* Set up mbuf */
-		/* TODO: track SOP/EOP bits to assemble a full mbuf
-		   out of received fragments. */
-		slot->mbuf->m_data += bd.bufoff;
-		slot->mbuf->m_len = bd.buflen;
+		m->m_data += bd.bufoff;
+		m->m_len = bd.buflen;
 		if (bd.flags & CPDMA_BD_SOP) {
-			slot->mbuf->m_pkthdr.len = bd.pktlen;
-			slot->mbuf->m_pkthdr.rcvif = psc->ifp;
-			slot->mbuf->m_flags |= M_PKTHDR;
+			m->m_pkthdr.len = bd.pktlen;
+			m->m_pkthdr.rcvif = psc->ifp;
+			m->m_flags |= M_PKTHDR;
+			m0_flags = bd.flags;
+			m0 = m;
+		}
+		nsegs++;
+		m->m_next = NULL;
+		m->m_nextpkt = NULL;
+		if (bd.flags & CPDMA_BD_EOP && m0 != NULL) {
+			if (m0_flags & CPDMA_BD_PASS_CRC)
+				m_adj(m0, -ETHER_CRC_LEN);
+			m0_flags = 0;
+			m0 = NULL;
+			if (nsegs > sc->rx.longest_chain)
+				sc->rx.longest_chain = nsegs;
+			nsegs = 0;
 		}
-		slot->mbuf->m_next = NULL;
-		slot->mbuf->m_nextpkt = NULL;
-		if (bd.flags & CPDMA_BD_PASS_CRC)
-			m_adj(slot->mbuf, -ETHER_CRC_LEN);
 
 		if ((psc->ifp->if_capenable & IFCAP_RXCSUM) != 0) {
 			/* check for valid CRC by looking into pkt_err[5:4] */
 			if ((bd.flags &
 			    (CPDMA_BD_SOP | CPDMA_BD_PKT_ERR_MASK)) ==
 			    CPDMA_BD_SOP) {
-				slot->mbuf->m_pkthdr.csum_flags |= CSUM_IP_CHECKED;
-				slot->mbuf->m_pkthdr.csum_flags |= CSUM_IP_VALID;
-				slot->mbuf->m_pkthdr.csum_data = 0xffff;
+				m->m_pkthdr.csum_flags |= CSUM_IP_CHECKED;
+				m->m_pkthdr.csum_flags |= CSUM_IP_VALID;
+				m->m_pkthdr.csum_data = 0xffff;
 			}
 		}
 
+		if (STAILQ_FIRST(&sc->rx.active) != NULL &&
+		    (bd.flags & (CPDMA_BD_EOP | CPDMA_BD_EOQ)) ==
+		    (CPDMA_BD_EOP | CPDMA_BD_EOQ)) {
+			cpsw_write_hdp_slot(sc, &sc->rx,
+			    STAILQ_FIRST(&sc->rx.active));
+			sc->rx.queue_restart++;
+		}
+
 		/* Add mbuf to packet list to be returned. */
-		if (mb_tail) {
-			mb_tail->m_nextpkt = slot->mbuf;
+		if (mb_tail != NULL && (bd.flags & CPDMA_BD_SOP)) {
+			mb_tail->m_nextpkt = m;
+		} else if (mb_tail != NULL) {
+			mb_tail->m_next = m;
+		} else if (mb_tail == NULL && (bd.flags & CPDMA_BD_SOP) == 0) {
+			if (bootverbose)
+				printf(
+				    "%s: %s: discanding fragment packet w/o header\n",
+				    __func__, psc->ifp->if_xname);
+			m_freem(m);
+			continue;
 		} else {
-			mb_head = slot->mbuf;
+			mb_head = m;
 		}
-		mb_tail = slot->mbuf;
-		slot->mbuf = NULL;
-		if (sc->rx_batch > 0 && sc->rx_batch == removed)
-			break;
+		mb_tail = m;
 	}
 
 	if (removed != 0) {
@@ -1697,7 +1714,6 @@ cpsw_rx_enqueue(struct cpsw_softc *sc)
 	struct cpsw_cpdma_bd bd;
 	struct cpsw_slot *first_new_slot, *last_old_slot, *next, *slot;
 	int error, nsegs, added = 0;
-	uint32_t flags;
 
 	/* Register new mbufs with hardware. */
 	first_new_slot = NULL;
@@ -1763,22 +1779,13 @@ cpsw_rx_enqueue(struct cpsw_softc *sc)
 	} else {
 		/* Add buffers to end of current queue. */
 		cpsw_cpdma_write_bd_next(sc, last_old_slot, first_new_slot);
-		/* If underrun, restart queue. */
-		if ((flags = cpsw_cpdma_read_bd_flags(sc, last_old_slot)) &
-		    CPDMA_BD_EOQ) {
-			flags &= ~CPDMA_BD_EOQ;
-			cpsw_cpdma_write_bd_flags(sc, last_old_slot, flags);
-			cpsw_write_hdp_slot(sc, &sc->rx, first_new_slot);
-			sc->rx.queue_restart++;
-		}
 	}
 	sc->rx.queue_adds += added;
 	sc->rx.avail_queue_len -= added;
 	sc->rx.active_queue_len += added;
 	cpsw_write_4(sc, CPSW_CPDMA_RX_FREEBUFFER(0), added);
-	if (sc->rx.active_queue_len > sc->rx.max_active_queue_len) {
+	if (sc->rx.active_queue_len > sc->rx.max_active_queue_len)
 		sc->rx.max_active_queue_len = sc->rx.active_queue_len;
-	}
 }
 
 static void
@@ -1830,21 +1837,19 @@ cpswp_tx_enqueue(struct cpswp_softc *sc)
 			break;
 
 		slot->mbuf = m0;
-		padlen = ETHER_MIN_LEN - slot->mbuf->m_pkthdr.len;
+		padlen = ETHER_MIN_LEN - ETHER_CRC_LEN - m0->m_pkthdr.len;
 		if (padlen < 0)
 			padlen = 0;
+		else if (padlen > 0)
+			m_append(slot->mbuf, padlen, sc->swsc->nullpad);
 
 		/* Create mapping in DMA memory */
 		error = bus_dmamap_load_mbuf_sg(sc->swsc->mbuf_dtag,
 		    slot->dmamap, slot->mbuf, segs, &nsegs, BUS_DMA_NOWAIT);
 		/* If the packet is too fragmented, try to simplify. */
 		if (error == EFBIG ||
-		    (error == 0 &&
-		    nsegs + (padlen > 0 ? 1 : 0) > sc->swsc->tx.avail_queue_len)) {
+		    (error == 0 && nsegs > sc->swsc->tx.avail_queue_len)) {
 			bus_dmamap_unload(sc->swsc->mbuf_dtag, slot->dmamap);
-			if (padlen > 0) /* May as well add padding. */
-				m_append(slot->mbuf, padlen,
-				    sc->swsc->null_mbuf->m_data);
 			m0 = m_defrag(slot->mbuf, M_NOWAIT);
 			if (m0 == NULL) {
 				device_printf(sc->dev,
@@ -1896,7 +1901,7 @@ cpswp_tx_enqueue(struct cpswp_softc *sc)
 		bd.bufptr = segs[0].ds_addr;
 		bd.bufoff = 0;
 		bd.buflen = segs[0].ds_len;
-		bd.pktlen = m_length(slot->mbuf, NULL) + padlen;
+		bd.pktlen = m_length(slot->mbuf, NULL);
 		bd.flags =  CPDMA_BD_SOP | CPDMA_BD_OWNER;
 		if (sc->swsc->dualemac) {
 			bd.flags |= CPDMA_BD_TO_PORT;
@@ -1921,42 +1926,18 @@ cpswp_tx_enqueue(struct cpswp_softc *sc)
 			bd.pktlen = 0;
 			bd.flags = CPDMA_BD_OWNER;
 		}
+
 		/* Save the final buffer. */
-		if (padlen <= 0)
-			bd.flags |= CPDMA_BD_EOP;
-		else {
-			next = STAILQ_NEXT(slot, next);
-			bd.next = cpsw_cpdma_bd_paddr(sc->swsc, next);
-		}
+		bd.flags |= CPDMA_BD_EOP;
 		cpsw_cpdma_write_bd(sc->swsc, slot, &bd);
 		STAILQ_REMOVE_HEAD(&sc->swsc->tx.avail, next);
 		STAILQ_INSERT_TAIL(&sc->swsc->tx.active, slot, next);
 
-		if (padlen > 0) {
-			slot = STAILQ_FIRST(&sc->swsc->tx.avail);
-
-			/* Setup buffer of null pad bytes (definitely EOP). */
-			bd.next = 0;
-			bd.bufptr = sc->swsc->null_mbuf_paddr;
-			bd.bufoff = 0;
-			bd.buflen = padlen;
-			bd.pktlen = 0;
-			bd.flags = CPDMA_BD_EOP | CPDMA_BD_OWNER;
-			cpsw_cpdma_write_bd(sc->swsc, slot, &bd);
-			++nsegs;
-
-			STAILQ_REMOVE_HEAD(&sc->swsc->tx.avail, next);
-			STAILQ_INSERT_TAIL(&sc->swsc->tx.active, slot, next);
-		}
-
 		last = slot;
-
 		added += nsegs;
 		if (nsegs > sc->swsc->tx.longest_chain)
 			sc->swsc->tx.longest_chain = nsegs;
 
-		// TODO: Should we defer the BPF tap until
-		// after all packets are queued?
 		BPF_MTAP(sc->ifp, m0);
 	}
 
@@ -2001,7 +1982,8 @@ cpsw_tx_dequeue(struct cpsw_softc *sc)
 			sc->tx.teardown = 1;
 		}
 
-		if ((flags & CPDMA_BD_OWNER) != 0 && sc->tx.teardown == 0)
+		if ((flags & (CPDMA_BD_SOP | CPDMA_BD_OWNER)) ==
+		    (CPDMA_BD_SOP | CPDMA_BD_OWNER) && sc->tx.teardown == 0)
 			break; /* Hardware is still using this packet. */
 
 		bus_dmamap_sync(sc->mbuf_dtag, slot->dmamap, BUS_DMASYNC_POSTWRITE);
@@ -2727,9 +2709,6 @@ cpsw_add_sysctls(struct cpsw_softc *sc)
 	SYSCTL_ADD_INT(ctx, parent, OID_AUTO, "debug",
 	    CTLFLAG_RW, &sc->debug, 0, "Enable switch debug messages");
 
-	SYSCTL_ADD_INT(ctx, parent, OID_AUTO, "rx_batch",
-	    CTLFLAG_RW, &sc->rx_batch, 0, "Set the rx batch size");
-
 	SYSCTL_ADD_PROC(ctx, parent, OID_AUTO, "attachedSecs",
 	    CTLTYPE_UINT | CTLFLAG_RD, sc, 0, cpsw_stat_attached, "IU",
 	    "Time since driver attach");
diff --git a/sys/arm/ti/cpsw/if_cpswvar.h b/sys/arm/ti/cpsw/if_cpswvar.h
index f037dd5..003af22 100644
--- a/sys/arm/ti/cpsw/if_cpswvar.h
+++ b/sys/arm/ti/cpsw/if_cpswvar.h
@@ -89,7 +89,6 @@ struct cpsw_softc {
 	int		active_slave;
 	int		debug;
 	int		dualemac;
-	int		rx_batch;
 	phandle_t	node;
 	struct bintime	attach_uptime; /* system uptime when attach happened. */
 	struct cpsw_port port[2];
@@ -104,10 +103,8 @@ struct cpsw_softc {
 	struct resource	*irq_res[CPSW_INTR_COUNT];
 	void		*ih_cookie[CPSW_INTR_COUNT];
 
-	/* An mbuf full of nulls for TX padding. */
-	bus_dmamap_t null_mbuf_dmamap;
-	struct mbuf *null_mbuf;
-	bus_addr_t null_mbuf_paddr;
+	/* A buffer full of nulls for TX padding. */
+	void		*nullpad;
 
 	bus_dma_tag_t	mbuf_dtag;