All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v5 1/9] bgmac: simplify tx ring index handling
@ 2015-04-13 17:27 Felix Fietkau
  2015-04-13 17:27 ` [PATCH v5 2/9] bgmac: leave interrupts disabled as long as there is work to do Felix Fietkau
                   ` (7 more replies)
  0 siblings, 8 replies; 13+ messages in thread
From: Felix Fietkau @ 2015-04-13 17:27 UTC (permalink / raw)
  To: netdev; +Cc: zajec5, hauke

Keep incrementing ring->start and ring->end instead of pointing it to
the actual ring slot entry. This simplifies the calculation of the
number of free slots.

Signed-off-by: Felix Fietkau <nbd@openwrt.org>
---
 drivers/net/ethernet/broadcom/bgmac.c | 44 ++++++++++++++---------------------
 drivers/net/ethernet/broadcom/bgmac.h |  6 ++---
 2 files changed, 20 insertions(+), 30 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
index fa8f9e1..73374b4 100644
--- a/drivers/net/ethernet/broadcom/bgmac.c
+++ b/drivers/net/ethernet/broadcom/bgmac.c
@@ -142,11 +142,10 @@ static netdev_tx_t bgmac_dma_tx_add(struct bgmac *bgmac,
 {
 	struct device *dma_dev = bgmac->core->dma_dev;
 	struct net_device *net_dev = bgmac->net_dev;
-	struct bgmac_slot_info *slot = &ring->slots[ring->end];
-	int free_slots;
+	int index = ring->end % BGMAC_TX_RING_SLOTS;
+	struct bgmac_slot_info *slot = &ring->slots[index];
 	int nr_frags;
 	u32 flags;
-	int index = ring->end;
 	int i;
 
 	if (skb->len > BGMAC_DESC_CTL1_LEN) {
@@ -158,13 +157,7 @@ static netdev_tx_t bgmac_dma_tx_add(struct bgmac *bgmac,
 		skb_checksum_help(skb);
 
 	nr_frags = skb_shinfo(skb)->nr_frags;
-
-	if (ring->start <= ring->end)
-		free_slots = ring->start - ring->end + BGMAC_TX_RING_SLOTS;
-	else
-		free_slots = ring->start - ring->end;
-
-	if (free_slots <= nr_frags + 1) {
+	if (ring->end - ring->start + nr_frags + 1 >= BGMAC_TX_RING_SLOTS) {
 		bgmac_err(bgmac, "TX ring is full, queue should be stopped!\n");
 		netif_stop_queue(net_dev);
 		return NETDEV_TX_BUSY;
@@ -200,7 +193,7 @@ static netdev_tx_t bgmac_dma_tx_add(struct bgmac *bgmac,
 	}
 
 	slot->skb = skb;
-
+	ring->end += nr_frags + 1;
 	netdev_sent_queue(net_dev, skb->len);
 
 	wmb();
@@ -208,13 +201,12 @@ static netdev_tx_t bgmac_dma_tx_add(struct bgmac *bgmac,
 	/* Increase ring->end to point empty slot. We tell hardware the first
 	 * slot it should *not* read.
 	 */
-	ring->end = (index + 1) % BGMAC_TX_RING_SLOTS;
 	bgmac_write(bgmac, ring->mmio_base + BGMAC_DMA_TX_INDEX,
 		    ring->index_base +
-		    ring->end * sizeof(struct bgmac_dma_desc));
+		    (ring->end % BGMAC_TX_RING_SLOTS) *
+		    sizeof(struct bgmac_dma_desc));
 
-	free_slots -= nr_frags + 1;
-	if (free_slots < 8)
+	if (ring->end - ring->start >= BGMAC_TX_RING_SLOTS - 8)
 		netif_stop_queue(net_dev);
 
 	return NETDEV_TX_OK;
@@ -256,17 +248,17 @@ static void bgmac_dma_tx_free(struct bgmac *bgmac, struct bgmac_dma_ring *ring)
 	empty_slot &= BGMAC_DMA_TX_STATDPTR;
 	empty_slot /= sizeof(struct bgmac_dma_desc);
 
-	while (ring->start != empty_slot) {
-		struct bgmac_slot_info *slot = &ring->slots[ring->start];
-		u32 ctl1 = le32_to_cpu(ring->cpu_base[ring->start].ctl1);
-		int len = ctl1 & BGMAC_DESC_CTL1_LEN;
+	while (ring->start != ring->end) {
+		int slot_idx = ring->start % BGMAC_TX_RING_SLOTS;
+		struct bgmac_slot_info *slot = &ring->slots[slot_idx];
+		u32 ctl1;
+		int len;
 
-		if (!slot->dma_addr) {
-			bgmac_err(bgmac, "Hardware reported transmission for empty TX ring slot %d! End of ring: %d\n",
-				  ring->start, ring->end);
-			goto next;
-		}
+		if (slot_idx == empty_slot)
+			break;
 
+		ctl1 = le32_to_cpu(ring->cpu_base[slot_idx].ctl1);
+		len = ctl1 & BGMAC_DESC_CTL1_LEN;
 		if (ctl1 & BGMAC_DESC_CTL0_SOF)
 			/* Unmap no longer used buffer */
 			dma_unmap_single(dma_dev, slot->dma_addr, len,
@@ -284,10 +276,8 @@ static void bgmac_dma_tx_free(struct bgmac *bgmac, struct bgmac_dma_ring *ring)
 			slot->skb = NULL;
 		}
 
-next:
 		slot->dma_addr = 0;
-		if (++ring->start >= BGMAC_TX_RING_SLOTS)
-			ring->start = 0;
+		ring->start++;
 		freed = true;
 	}
 
diff --git a/drivers/net/ethernet/broadcom/bgmac.h b/drivers/net/ethernet/broadcom/bgmac.h
index 3ad965f..5a198d5 100644
--- a/drivers/net/ethernet/broadcom/bgmac.h
+++ b/drivers/net/ethernet/broadcom/bgmac.h
@@ -414,10 +414,10 @@ enum bgmac_dma_ring_type {
  * empty.
  */
 struct bgmac_dma_ring {
-	u16 num_slots;
-	u16 start;
-	u16 end;
+	u32 start;
+	u32 end;
 
+	u16 num_slots;
 	u16 mmio_base;
 	struct bgmac_dma_desc *cpu_base;
 	dma_addr_t dma_base;
-- 
2.2.2

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v5 2/9] bgmac: leave interrupts disabled as long as there is work to do
  2015-04-13 17:27 [PATCH v5 1/9] bgmac: simplify tx ring index handling Felix Fietkau
@ 2015-04-13 17:27 ` Felix Fietkau
  2015-04-13 17:27 ` [PATCH v5 3/9] bgmac: set received skb headroom to NET_SKB_PAD Felix Fietkau
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: Felix Fietkau @ 2015-04-13 17:27 UTC (permalink / raw)
  To: netdev; +Cc: zajec5, hauke

Always poll rx and tx during NAPI poll instead of relying on the status
of the first interrupt. This prevents bgmac_poll from leaving unfinished
work around until the next IRQ.
In my tests this makes bridging/routing throughput under heavy load more
stable and ensures that no new IRQs arrive as long as bgmac_poll uses up
the entire budget.

Signed-off-by: Felix Fietkau <nbd@openwrt.org>
---
 drivers/net/ethernet/broadcom/bgmac.c | 31 ++++++++++---------------------
 drivers/net/ethernet/broadcom/bgmac.h |  1 -
 2 files changed, 10 insertions(+), 22 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
index 73374b4..30464d0 100644
--- a/drivers/net/ethernet/broadcom/bgmac.c
+++ b/drivers/net/ethernet/broadcom/bgmac.c
@@ -1105,8 +1105,6 @@ static void bgmac_chip_reset(struct bgmac *bgmac)
 	bgmac_phy_init(bgmac);
 
 	netdev_reset_queue(bgmac->net_dev);
-
-	bgmac->int_status = 0;
 }
 
 static void bgmac_chip_intrs_on(struct bgmac *bgmac)
@@ -1221,14 +1219,13 @@ static irqreturn_t bgmac_interrupt(int irq, void *dev_id)
 	if (!int_status)
 		return IRQ_NONE;
 
-	/* Ack */
-	bgmac_write(bgmac, BGMAC_INT_STATUS, int_status);
+	int_status &= ~(BGMAC_IS_TX0 | BGMAC_IS_RX);
+	if (int_status)
+		bgmac_err(bgmac, "Unknown IRQs: 0x%08X\n", int_status);
 
 	/* Disable new interrupts until handling existing ones */
 	bgmac_chip_intrs_off(bgmac);
 
-	bgmac->int_status = int_status;
-
 	napi_schedule(&bgmac->napi);
 
 	return IRQ_HANDLED;
@@ -1237,25 +1234,17 @@ static irqreturn_t bgmac_interrupt(int irq, void *dev_id)
 static int bgmac_poll(struct napi_struct *napi, int weight)
 {
 	struct bgmac *bgmac = container_of(napi, struct bgmac, napi);
-	struct bgmac_dma_ring *ring;
 	int handled = 0;
 
-	if (bgmac->int_status & BGMAC_IS_TX0) {
-		ring = &bgmac->tx_ring[0];
-		bgmac_dma_tx_free(bgmac, ring);
-		bgmac->int_status &= ~BGMAC_IS_TX0;
-	}
+	/* Ack */
+	bgmac_write(bgmac, BGMAC_INT_STATUS, ~0);
 
-	if (bgmac->int_status & BGMAC_IS_RX) {
-		ring = &bgmac->rx_ring[0];
-		handled += bgmac_dma_rx_read(bgmac, ring, weight);
-		bgmac->int_status &= ~BGMAC_IS_RX;
-	}
+	bgmac_dma_tx_free(bgmac, &bgmac->tx_ring[0]);
+	handled += bgmac_dma_rx_read(bgmac, &bgmac->rx_ring[0], weight);
 
-	if (bgmac->int_status) {
-		bgmac_err(bgmac, "Unknown IRQs: 0x%08X\n", bgmac->int_status);
-		bgmac->int_status = 0;
-	}
+	/* Poll again if more events arrived in the meantime */
+	if (bgmac_read(bgmac, BGMAC_INT_STATUS) & (BGMAC_IS_TX0 | BGMAC_IS_RX))
+		return handled;
 
 	if (handled < weight) {
 		napi_complete(napi);
diff --git a/drivers/net/ethernet/broadcom/bgmac.h b/drivers/net/ethernet/broadcom/bgmac.h
index 5a198d5..abd50d1 100644
--- a/drivers/net/ethernet/broadcom/bgmac.h
+++ b/drivers/net/ethernet/broadcom/bgmac.h
@@ -452,7 +452,6 @@ struct bgmac {
 
 	/* Int */
 	u32 int_mask;
-	u32 int_status;
 
 	/* Current MAC state */
 	int mac_speed;
-- 
2.2.2

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v5 3/9] bgmac: set received skb headroom to NET_SKB_PAD
  2015-04-13 17:27 [PATCH v5 1/9] bgmac: simplify tx ring index handling Felix Fietkau
  2015-04-13 17:27 ` [PATCH v5 2/9] bgmac: leave interrupts disabled as long as there is work to do Felix Fietkau
@ 2015-04-13 17:27 ` Felix Fietkau
  2015-04-13 17:27 ` [PATCH v5 4/9] bgmac: simplify rx DMA error handling Felix Fietkau
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: Felix Fietkau @ 2015-04-13 17:27 UTC (permalink / raw)
  To: netdev; +Cc: zajec5, hauke

A packet buffer offset of 30 bytes is inefficient, because the first 2
bytes end up in a different cacheline.

Signed-off-by: Felix Fietkau <nbd@openwrt.org>
---
 drivers/net/ethernet/broadcom/bgmac.c | 14 ++++++++------
 drivers/net/ethernet/broadcom/bgmac.h |  4 +++-
 2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
index 30464d0..9a6742e 100644
--- a/drivers/net/ethernet/broadcom/bgmac.c
+++ b/drivers/net/ethernet/broadcom/bgmac.c
@@ -342,13 +342,13 @@ static int bgmac_dma_rx_skb_for_slot(struct bgmac *bgmac,
 		return -ENOMEM;
 
 	/* Poison - if everything goes fine, hardware will overwrite it */
-	rx = buf;
+	rx = buf + BGMAC_RX_BUF_OFFSET;
 	rx->len = cpu_to_le16(0xdead);
 	rx->flags = cpu_to_le16(0xbeef);
 
 	/* Map skb for the DMA */
-	dma_addr = dma_map_single(dma_dev, buf, BGMAC_RX_BUF_SIZE,
-				  DMA_FROM_DEVICE);
+	dma_addr = dma_map_single(dma_dev, buf + BGMAC_RX_BUF_OFFSET,
+				  BGMAC_RX_BUF_SIZE, DMA_FROM_DEVICE);
 	if (dma_mapping_error(dma_dev, dma_addr)) {
 		bgmac_err(bgmac, "DMA mapping error\n");
 		put_page(virt_to_head_page(buf));
@@ -399,7 +399,7 @@ static int bgmac_dma_rx_read(struct bgmac *bgmac, struct bgmac_dma_ring *ring,
 	while (ring->start != ring->end) {
 		struct device *dma_dev = bgmac->core->dma_dev;
 		struct bgmac_slot_info *slot = &ring->slots[ring->start];
-		struct bgmac_rx_header *rx = slot->buf;
+		struct bgmac_rx_header *rx = slot->buf + BGMAC_RX_BUF_OFFSET;
 		struct sk_buff *skb;
 		void *buf = slot->buf;
 		u16 len, flags;
@@ -450,8 +450,10 @@ static int bgmac_dma_rx_read(struct bgmac *bgmac, struct bgmac_dma_ring *ring,
 					 BGMAC_RX_BUF_SIZE, DMA_FROM_DEVICE);
 
 			skb = build_skb(buf, BGMAC_RX_ALLOC_SIZE);
-			skb_put(skb, BGMAC_RX_FRAME_OFFSET + len);
-			skb_pull(skb, BGMAC_RX_FRAME_OFFSET);
+			skb_put(skb, BGMAC_RX_FRAME_OFFSET +
+				BGMAC_RX_BUF_OFFSET + len);
+			skb_pull(skb, BGMAC_RX_FRAME_OFFSET +
+				 BGMAC_RX_BUF_OFFSET);
 
 			skb_checksum_none_assert(skb);
 			skb->protocol = eth_type_trans(skb, bgmac->net_dev);
diff --git a/drivers/net/ethernet/broadcom/bgmac.h b/drivers/net/ethernet/broadcom/bgmac.h
index abd50d1..3db0d52 100644
--- a/drivers/net/ethernet/broadcom/bgmac.h
+++ b/drivers/net/ethernet/broadcom/bgmac.h
@@ -360,9 +360,11 @@
 
 #define BGMAC_RX_HEADER_LEN			28		/* Last 24 bytes are unused. Well... */
 #define BGMAC_RX_FRAME_OFFSET			30		/* There are 2 unused bytes between header and real data */
+#define BGMAC_RX_BUF_OFFSET			(NET_SKB_PAD + NET_IP_ALIGN - \
+						 BGMAC_RX_FRAME_OFFSET)
 #define BGMAC_RX_MAX_FRAME_SIZE			1536		/* Copied from b44/tg3 */
 #define BGMAC_RX_BUF_SIZE			(BGMAC_RX_FRAME_OFFSET + BGMAC_RX_MAX_FRAME_SIZE)
-#define BGMAC_RX_ALLOC_SIZE			(SKB_DATA_ALIGN(BGMAC_RX_BUF_SIZE) + \
+#define BGMAC_RX_ALLOC_SIZE			(SKB_DATA_ALIGN(BGMAC_RX_BUF_SIZE + BGMAC_RX_BUF_OFFSET) + \
 						 SKB_DATA_ALIGN(sizeof(struct skb_shared_info)))
 
 #define BGMAC_BFL_ENETROBO			0x0010		/* has ephy roboswitch spi */
-- 
2.2.2

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v5 4/9] bgmac: simplify rx DMA error handling
  2015-04-13 17:27 [PATCH v5 1/9] bgmac: simplify tx ring index handling Felix Fietkau
  2015-04-13 17:27 ` [PATCH v5 2/9] bgmac: leave interrupts disabled as long as there is work to do Felix Fietkau
  2015-04-13 17:27 ` [PATCH v5 3/9] bgmac: set received skb headroom to NET_SKB_PAD Felix Fietkau
@ 2015-04-13 17:27 ` Felix Fietkau
  2015-04-13 19:47   ` Felix Fietkau
  2015-04-13 23:20   ` Francois Romieu
  2015-04-13 17:27 ` [PATCH v5 5/9] bgmac: add check for oversized packets Felix Fietkau
                   ` (4 subsequent siblings)
  7 siblings, 2 replies; 13+ messages in thread
From: Felix Fietkau @ 2015-04-13 17:27 UTC (permalink / raw)
  To: netdev; +Cc: zajec5, hauke

Unmap the DMA buffer before checking it. If an error occurs, free the
buffer and allocate a new one. If allocation or mapping fails, retry as
long as there is NAPI poll budget left (count every attempt instead of
every frame).

Signed-off-by: Felix Fietkau <nbd@openwrt.org>
---
 drivers/net/ethernet/broadcom/bgmac.c | 56 +++++++++++++----------------------
 1 file changed, 20 insertions(+), 36 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
index 9a6742e..1916ebc 100644
--- a/drivers/net/ethernet/broadcom/bgmac.c
+++ b/drivers/net/ethernet/broadcom/bgmac.c
@@ -404,51 +404,33 @@ static int bgmac_dma_rx_read(struct bgmac *bgmac, struct bgmac_dma_ring *ring,
 		void *buf = slot->buf;
 		u16 len, flags;
 
-		/* Unmap buffer to make it accessible to the CPU */
-		dma_sync_single_for_cpu(dma_dev, slot->dma_addr,
-					BGMAC_RX_BUF_SIZE, DMA_FROM_DEVICE);
-
-		/* Get info from the header */
-		len = le16_to_cpu(rx->len);
-		flags = le16_to_cpu(rx->flags);
+		if (++handled >= weight - 1) /* Should never be greater */
+			break;
 
 		do {
-			dma_addr_t old_dma_addr = slot->dma_addr;
-			int err;
+			if (!slot->dma_addr)
+				break;
+
+			/* Unmap buffer to make it accessible to the CPU */
+			dma_unmap_single(dma_dev, slot->dma_addr,
+					 BGMAC_RX_BUF_SIZE, DMA_FROM_DEVICE);
+			slot->dma_addr = 0;
+
+			/* Get info from the header */
+			len = le16_to_cpu(rx->len);
+			flags = le16_to_cpu(rx->flags);
 
 			/* Check for poison and drop or pass the packet */
 			if (len == 0xdead && flags == 0xbeef) {
 				bgmac_err(bgmac, "Found poisoned packet at slot %d, DMA issue!\n",
 					  ring->start);
-				dma_sync_single_for_device(dma_dev,
-							   slot->dma_addr,
-							   BGMAC_RX_BUF_SIZE,
-							   DMA_FROM_DEVICE);
+				kfree(buf);
 				break;
 			}
 
 			/* Omit CRC. */
 			len -= ETH_FCS_LEN;
 
-			/* Prepare new skb as replacement */
-			err = bgmac_dma_rx_skb_for_slot(bgmac, slot);
-			if (err) {
-				/* Poison the old skb */
-				rx->len = cpu_to_le16(0xdead);
-				rx->flags = cpu_to_le16(0xbeef);
-
-				dma_sync_single_for_device(dma_dev,
-							   slot->dma_addr,
-							   BGMAC_RX_BUF_SIZE,
-							   DMA_FROM_DEVICE);
-				break;
-			}
-			bgmac_dma_rx_setup_desc(bgmac, ring, ring->start);
-
-			/* Unmap old skb, we'll pass it to the netfif */
-			dma_unmap_single(dma_dev, old_dma_addr,
-					 BGMAC_RX_BUF_SIZE, DMA_FROM_DEVICE);
-
 			skb = build_skb(buf, BGMAC_RX_ALLOC_SIZE);
 			skb_put(skb, BGMAC_RX_FRAME_OFFSET +
 				BGMAC_RX_BUF_OFFSET + len);
@@ -458,14 +440,16 @@ static int bgmac_dma_rx_read(struct bgmac *bgmac, struct bgmac_dma_ring *ring,
 			skb_checksum_none_assert(skb);
 			skb->protocol = eth_type_trans(skb, bgmac->net_dev);
 			napi_gro_receive(&bgmac->napi, skb);
-			handled++;
 		} while (0);
 
+		/* Prepare new skb as replacement */
+		if (bgmac_dma_rx_skb_for_slot(bgmac, slot))
+			continue;
+
+		bgmac_dma_rx_setup_desc(bgmac, ring, ring->start);
+
 		if (++ring->start >= BGMAC_RX_RING_SLOTS)
 			ring->start = 0;
-
-		if (handled >= weight) /* Should never be greater */
-			break;
 	}
 
 	return handled;
-- 
2.2.2

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v5 5/9] bgmac: add check for oversized packets
  2015-04-13 17:27 [PATCH v5 1/9] bgmac: simplify tx ring index handling Felix Fietkau
                   ` (2 preceding siblings ...)
  2015-04-13 17:27 ` [PATCH v5 4/9] bgmac: simplify rx DMA error handling Felix Fietkau
@ 2015-04-13 17:27 ` Felix Fietkau
  2015-04-13 17:27 ` [PATCH v5 6/9] bgmac: increase rx ring size from 511 to 512 Felix Fietkau
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: Felix Fietkau @ 2015-04-13 17:27 UTC (permalink / raw)
  To: netdev; +Cc: zajec5, hauke

In very rare cases, the MAC can catch an internal buffer that is bigger
than it's supposed to be. Instead of crashing the kernel, simply pass
the buffer back to the hardware

Signed-off-by: Felix Fietkau <nbd@openwrt.org>
---
 drivers/net/ethernet/broadcom/bgmac.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
index 1916ebc..eca063d 100644
--- a/drivers/net/ethernet/broadcom/bgmac.c
+++ b/drivers/net/ethernet/broadcom/bgmac.c
@@ -428,6 +428,13 @@ static int bgmac_dma_rx_read(struct bgmac *bgmac, struct bgmac_dma_ring *ring,
 				break;
 			}
 
+			if (len > BGMAC_RX_ALLOC_SIZE) {
+				bgmac_err(bgmac, "Found oversized packet at slot %d, DMA issue!\n",
+					  ring->start);
+				kfree(buf);
+				break;
+			}
+
 			/* Omit CRC. */
 			len -= ETH_FCS_LEN;
 
-- 
2.2.2

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v5 6/9] bgmac: increase rx ring size from 511 to 512
  2015-04-13 17:27 [PATCH v5 1/9] bgmac: simplify tx ring index handling Felix Fietkau
                   ` (3 preceding siblings ...)
  2015-04-13 17:27 ` [PATCH v5 5/9] bgmac: add check for oversized packets Felix Fietkau
@ 2015-04-13 17:27 ` Felix Fietkau
  2015-04-13 17:27 ` [PATCH v5 7/9] bgmac: simplify dma init/cleanup Felix Fietkau
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: Felix Fietkau @ 2015-04-13 17:27 UTC (permalink / raw)
  To: netdev; +Cc: zajec5, hauke

Limiting it to 511 looks like a failed attempt at leaving one descriptor
empty to allow the hardware to stop processing a buffer that has not
been prepared yet. However, this doesn't work because this affects the
total ring size as well

Signed-off-by: Felix Fietkau <nbd@openwrt.org>
---
 drivers/net/ethernet/broadcom/bgmac.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/broadcom/bgmac.h b/drivers/net/ethernet/broadcom/bgmac.h
index 3db0d52..e45e303 100644
--- a/drivers/net/ethernet/broadcom/bgmac.h
+++ b/drivers/net/ethernet/broadcom/bgmac.h
@@ -356,7 +356,7 @@
 #define BGMAC_MAX_RX_RINGS			1
 
 #define BGMAC_TX_RING_SLOTS			128
-#define BGMAC_RX_RING_SLOTS			512 - 1		/* Why -1? Well, Broadcom does that... */
+#define BGMAC_RX_RING_SLOTS			512
 
 #define BGMAC_RX_HEADER_LEN			28		/* Last 24 bytes are unused. Well... */
 #define BGMAC_RX_FRAME_OFFSET			30		/* There are 2 unused bytes between header and real data */
-- 
2.2.2

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v5 7/9] bgmac: simplify dma init/cleanup
  2015-04-13 17:27 [PATCH v5 1/9] bgmac: simplify tx ring index handling Felix Fietkau
                   ` (4 preceding siblings ...)
  2015-04-13 17:27 ` [PATCH v5 6/9] bgmac: increase rx ring size from 511 to 512 Felix Fietkau
@ 2015-04-13 17:27 ` Felix Fietkau
  2015-04-13 17:27 ` [PATCH v5 8/9] bgmac: fix DMA rx corruption Felix Fietkau
  2015-04-13 17:27 ` [PATCH v5 9/9] bgmac: drop ring->num_slots Felix Fietkau
  7 siblings, 0 replies; 13+ messages in thread
From: Felix Fietkau @ 2015-04-13 17:27 UTC (permalink / raw)
  To: netdev; +Cc: zajec5, hauke

Instead of allocating buffers at device init time and initializing
descriptors at device open, do both at the same time (during open).
Free all buffers when closing the device.

Signed-off-by: Felix Fietkau <nbd@openwrt.org>
---
 drivers/net/ethernet/broadcom/bgmac.c | 69 +++++++++++++++++------------------
 1 file changed, 34 insertions(+), 35 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
index eca063d..457eac8 100644
--- a/drivers/net/ethernet/broadcom/bgmac.c
+++ b/drivers/net/ethernet/broadcom/bgmac.c
@@ -545,18 +545,26 @@ static void bgmac_dma_ring_desc_free(struct bgmac *bgmac,
 			  ring->dma_base);
 }
 
-static void bgmac_dma_free(struct bgmac *bgmac)
+static void bgmac_dma_cleanup(struct bgmac *bgmac)
 {
 	int i;
 
-	for (i = 0; i < BGMAC_MAX_TX_RINGS; i++) {
+	for (i = 0; i < BGMAC_MAX_TX_RINGS; i++)
 		bgmac_dma_tx_ring_free(bgmac, &bgmac->tx_ring[i]);
-		bgmac_dma_ring_desc_free(bgmac, &bgmac->tx_ring[i]);
-	}
-	for (i = 0; i < BGMAC_MAX_RX_RINGS; i++) {
+
+	for (i = 0; i < BGMAC_MAX_RX_RINGS; i++)
 		bgmac_dma_rx_ring_free(bgmac, &bgmac->rx_ring[i]);
+}
+
+static void bgmac_dma_free(struct bgmac *bgmac)
+{
+	int i;
+
+	for (i = 0; i < BGMAC_MAX_TX_RINGS; i++)
+		bgmac_dma_ring_desc_free(bgmac, &bgmac->tx_ring[i]);
+
+	for (i = 0; i < BGMAC_MAX_RX_RINGS; i++)
 		bgmac_dma_ring_desc_free(bgmac, &bgmac->rx_ring[i]);
-	}
 }
 
 static int bgmac_dma_alloc(struct bgmac *bgmac)
@@ -604,8 +612,6 @@ static int bgmac_dma_alloc(struct bgmac *bgmac)
 	}
 
 	for (i = 0; i < BGMAC_MAX_RX_RINGS; i++) {
-		int j;
-
 		ring = &bgmac->rx_ring[i];
 		ring->num_slots = BGMAC_RX_RING_SLOTS;
 		ring->mmio_base = ring_base[i];
@@ -628,15 +634,6 @@ static int bgmac_dma_alloc(struct bgmac *bgmac)
 			ring->index_base = lower_32_bits(ring->dma_base);
 		else
 			ring->index_base = 0;
-
-		/* Alloc RX slots */
-		for (j = 0; j < ring->num_slots; j++) {
-			err = bgmac_dma_rx_skb_for_slot(bgmac, &ring->slots[j]);
-			if (err) {
-				bgmac_err(bgmac, "Can't allocate skb for slot in RX ring\n");
-				goto err_dma_free;
-			}
-		}
 	}
 
 	return 0;
@@ -646,10 +643,10 @@ err_dma_free:
 	return -ENOMEM;
 }
 
-static void bgmac_dma_init(struct bgmac *bgmac)
+static int bgmac_dma_init(struct bgmac *bgmac)
 {
 	struct bgmac_dma_ring *ring;
-	int i;
+	int i, err;
 
 	for (i = 0; i < BGMAC_MAX_TX_RINGS; i++) {
 		ring = &bgmac->tx_ring[i];
@@ -681,8 +678,13 @@ static void bgmac_dma_init(struct bgmac *bgmac)
 		if (ring->unaligned)
 			bgmac_dma_rx_enable(bgmac, ring);
 
-		for (j = 0; j < ring->num_slots; j++)
+		for (j = 0; j < ring->num_slots; j++) {
+			err = bgmac_dma_rx_skb_for_slot(bgmac, &ring->slots[j]);
+			if (err)
+				return err;
+
 			bgmac_dma_rx_setup_desc(bgmac, ring, j);
+		}
 
 		bgmac_write(bgmac, ring->mmio_base + BGMAC_DMA_RX_INDEX,
 			    ring->index_base +
@@ -691,6 +693,8 @@ static void bgmac_dma_init(struct bgmac *bgmac)
 		ring->start = 0;
 		ring->end = 0;
 	}
+
+	return 0;
 }
 
 /**************************************************
@@ -1166,11 +1170,8 @@ static void bgmac_enable(struct bgmac *bgmac)
 }
 
 /* http://bcm-v4.sipsolutions.net/mac-gbit/gmac/chipinit */
-static void bgmac_chip_init(struct bgmac *bgmac, bool full_init)
+static void bgmac_chip_init(struct bgmac *bgmac)
 {
-	struct bgmac_dma_ring *ring;
-	int i;
-
 	/* 1 interrupt per received frame */
 	bgmac_write(bgmac, BGMAC_INT_RECV_LAZY, 1 << BGMAC_IRL_FC_SHIFT);
 
@@ -1188,16 +1189,7 @@ static void bgmac_chip_init(struct bgmac *bgmac, bool full_init)
 
 	bgmac_write(bgmac, BGMAC_RXMAX_LENGTH, 32 + ETHER_MAX_LEN);
 
-	if (full_init) {
-		bgmac_dma_init(bgmac);
-		if (1) /* FIXME: is there any case we don't want IRQs? */
-			bgmac_chip_intrs_on(bgmac);
-	} else {
-		for (i = 0; i < BGMAC_MAX_RX_RINGS; i++) {
-			ring = &bgmac->rx_ring[i];
-			bgmac_dma_rx_enable(bgmac, ring);
-		}
-	}
+	bgmac_chip_intrs_on(bgmac);
 
 	bgmac_enable(bgmac);
 }
@@ -1257,8 +1249,13 @@ static int bgmac_open(struct net_device *net_dev)
 	int err = 0;
 
 	bgmac_chip_reset(bgmac);
+
+	err = bgmac_dma_init(bgmac);
+	if (err)
+		goto err_out;
+
 	/* Specs say about reclaiming rings here, but we do that in DMA init */
-	bgmac_chip_init(bgmac, true);
+	bgmac_chip_init(bgmac);
 
 	err = request_irq(bgmac->core->irq, bgmac_interrupt, IRQF_SHARED,
 			  KBUILD_MODNAME, net_dev);
@@ -1273,6 +1270,7 @@ static int bgmac_open(struct net_device *net_dev)
 	netif_carrier_on(net_dev);
 
 err_out:
+	bgmac_dma_cleanup(bgmac);
 	return err;
 }
 
@@ -1289,6 +1287,7 @@ static int bgmac_stop(struct net_device *net_dev)
 	free_irq(bgmac->core->irq, net_dev);
 
 	bgmac_chip_reset(bgmac);
+	bgmac_dma_cleanup(bgmac);
 
 	return 0;
 }
-- 
2.2.2

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v5 8/9] bgmac: fix DMA rx corruption
  2015-04-13 17:27 [PATCH v5 1/9] bgmac: simplify tx ring index handling Felix Fietkau
                   ` (5 preceding siblings ...)
  2015-04-13 17:27 ` [PATCH v5 7/9] bgmac: simplify dma init/cleanup Felix Fietkau
@ 2015-04-13 17:27 ` Felix Fietkau
  2015-04-13 17:27 ` [PATCH v5 9/9] bgmac: drop ring->num_slots Felix Fietkau
  7 siblings, 0 replies; 13+ messages in thread
From: Felix Fietkau @ 2015-04-13 17:27 UTC (permalink / raw)
  To: netdev; +Cc: zajec5, hauke

The driver needs to inform the hardware about the first invalid (not yet
filled) rx slot, by writing its DMA descriptor pointer offset to the
BGMAC_DMA_RX_INDEX register.

This register was set to a value exceeding the rx ring size, effectively
allowing the hardware constant access to the full ring, regardless of
which slots are initialized.

To fix this issue, always mark the last filled rx slot as invalid.

Signed-off-by: Felix Fietkau <nbd@openwrt.org>
---
 drivers/net/ethernet/broadcom/bgmac.c | 27 ++++++++++++++++++---------
 1 file changed, 18 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
index 457eac8..dd3bfad 100644
--- a/drivers/net/ethernet/broadcom/bgmac.c
+++ b/drivers/net/ethernet/broadcom/bgmac.c
@@ -362,6 +362,16 @@ static int bgmac_dma_rx_skb_for_slot(struct bgmac *bgmac,
 	return 0;
 }
 
+static void bgmac_dma_rx_update_index(struct bgmac *bgmac,
+				      struct bgmac_dma_ring *ring)
+{
+	dma_wmb();
+
+	bgmac_write(bgmac, ring->mmio_base + BGMAC_DMA_RX_INDEX,
+		    ring->index_base +
+		    ring->end * sizeof(struct bgmac_dma_desc));
+}
+
 static void bgmac_dma_rx_setup_desc(struct bgmac *bgmac,
 				    struct bgmac_dma_ring *ring, int desc_idx)
 {
@@ -380,6 +390,8 @@ static void bgmac_dma_rx_setup_desc(struct bgmac *bgmac,
 	dma_desc->addr_high = cpu_to_le32(upper_32_bits(ring->slots[desc_idx].dma_addr));
 	dma_desc->ctl0 = cpu_to_le32(ctl0);
 	dma_desc->ctl1 = cpu_to_le32(ctl1);
+
+	ring->end = desc_idx;
 }
 
 static int bgmac_dma_rx_read(struct bgmac *bgmac, struct bgmac_dma_ring *ring,
@@ -394,9 +406,7 @@ static int bgmac_dma_rx_read(struct bgmac *bgmac, struct bgmac_dma_ring *ring,
 	end_slot &= BGMAC_DMA_RX_STATDPTR;
 	end_slot /= sizeof(struct bgmac_dma_desc);
 
-	ring->end = end_slot;
-
-	while (ring->start != ring->end) {
+	while (ring->start != end_slot) {
 		struct device *dma_dev = bgmac->core->dma_dev;
 		struct bgmac_slot_info *slot = &ring->slots[ring->start];
 		struct bgmac_rx_header *rx = slot->buf + BGMAC_RX_BUF_OFFSET;
@@ -459,6 +469,8 @@ static int bgmac_dma_rx_read(struct bgmac *bgmac, struct bgmac_dma_ring *ring,
 			ring->start = 0;
 	}
 
+	bgmac_dma_rx_update_index(bgmac, ring);
+
 	return handled;
 }
 
@@ -678,6 +690,8 @@ static int bgmac_dma_init(struct bgmac *bgmac)
 		if (ring->unaligned)
 			bgmac_dma_rx_enable(bgmac, ring);
 
+		ring->start = 0;
+		ring->end = 0;
 		for (j = 0; j < ring->num_slots; j++) {
 			err = bgmac_dma_rx_skb_for_slot(bgmac, &ring->slots[j]);
 			if (err)
@@ -686,12 +700,7 @@ static int bgmac_dma_init(struct bgmac *bgmac)
 			bgmac_dma_rx_setup_desc(bgmac, ring, j);
 		}
 
-		bgmac_write(bgmac, ring->mmio_base + BGMAC_DMA_RX_INDEX,
-			    ring->index_base +
-			    ring->num_slots * sizeof(struct bgmac_dma_desc));
-
-		ring->start = 0;
-		ring->end = 0;
+		bgmac_dma_rx_update_index(bgmac, ring);
 	}
 
 	return 0;
-- 
2.2.2

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v5 9/9] bgmac: drop ring->num_slots
  2015-04-13 17:27 [PATCH v5 1/9] bgmac: simplify tx ring index handling Felix Fietkau
                   ` (6 preceding siblings ...)
  2015-04-13 17:27 ` [PATCH v5 8/9] bgmac: fix DMA rx corruption Felix Fietkau
@ 2015-04-13 17:27 ` Felix Fietkau
  7 siblings, 0 replies; 13+ messages in thread
From: Felix Fietkau @ 2015-04-13 17:27 UTC (permalink / raw)
  To: netdev; +Cc: zajec5, hauke

The ring size is always known at compile time, so make the code a bit
more efficient

Signed-off-by: Felix Fietkau <nbd@openwrt.org>
---
 drivers/net/ethernet/broadcom/bgmac.c | 27 ++++++++++++++-------------
 drivers/net/ethernet/broadcom/bgmac.h |  3 +--
 2 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
index dd3bfad..92d1e7e 100644
--- a/drivers/net/ethernet/broadcom/bgmac.c
+++ b/drivers/net/ethernet/broadcom/bgmac.c
@@ -123,7 +123,7 @@ bgmac_dma_tx_add_buf(struct bgmac *bgmac, struct bgmac_dma_ring *ring,
 	struct bgmac_dma_desc *dma_desc;
 	u32 ctl1;
 
-	if (i == ring->num_slots - 1)
+	if (i == BGMAC_TX_RING_SLOTS - 1)
 		ctl0 |= BGMAC_DESC_CTL0_EOT;
 
 	ctl1 = len & BGMAC_DESC_CTL1_LEN;
@@ -378,7 +378,7 @@ static void bgmac_dma_rx_setup_desc(struct bgmac *bgmac,
 	struct bgmac_dma_desc *dma_desc = ring->cpu_base + desc_idx;
 	u32 ctl0 = 0, ctl1 = 0;
 
-	if (desc_idx == ring->num_slots - 1)
+	if (desc_idx == BGMAC_RX_RING_SLOTS - 1)
 		ctl0 |= BGMAC_DESC_CTL0_EOT;
 	ctl1 |= BGMAC_RX_BUF_SIZE & BGMAC_DESC_CTL1_LEN;
 	/* Is there any BGMAC device that requires extension? */
@@ -504,7 +504,7 @@ static void bgmac_dma_tx_ring_free(struct bgmac *bgmac,
 	struct bgmac_slot_info *slot;
 	int i;
 
-	for (i = 0; i < ring->num_slots; i++) {
+	for (i = 0; i < BGMAC_TX_RING_SLOTS; i++) {
 		int len = dma_desc[i].ctl1 & BGMAC_DESC_CTL1_LEN;
 
 		slot = &ring->slots[i];
@@ -529,7 +529,7 @@ static void bgmac_dma_rx_ring_free(struct bgmac *bgmac,
 	struct bgmac_slot_info *slot;
 	int i;
 
-	for (i = 0; i < ring->num_slots; i++) {
+	for (i = 0; i < BGMAC_RX_RING_SLOTS; i++) {
 		slot = &ring->slots[i];
 		if (!slot->buf)
 			continue;
@@ -543,7 +543,8 @@ static void bgmac_dma_rx_ring_free(struct bgmac *bgmac,
 }
 
 static void bgmac_dma_ring_desc_free(struct bgmac *bgmac,
-				     struct bgmac_dma_ring *ring)
+				     struct bgmac_dma_ring *ring,
+				     int num_slots)
 {
 	struct device *dma_dev = bgmac->core->dma_dev;
 	int size;
@@ -552,7 +553,7 @@ static void bgmac_dma_ring_desc_free(struct bgmac *bgmac,
 	    return;
 
 	/* Free ring of descriptors */
-	size = ring->num_slots * sizeof(struct bgmac_dma_desc);
+	size = num_slots * sizeof(struct bgmac_dma_desc);
 	dma_free_coherent(dma_dev, size, ring->cpu_base,
 			  ring->dma_base);
 }
@@ -573,10 +574,12 @@ static void bgmac_dma_free(struct bgmac *bgmac)
 	int i;
 
 	for (i = 0; i < BGMAC_MAX_TX_RINGS; i++)
-		bgmac_dma_ring_desc_free(bgmac, &bgmac->tx_ring[i]);
+		bgmac_dma_ring_desc_free(bgmac, &bgmac->tx_ring[i],
+					 BGMAC_TX_RING_SLOTS);
 
 	for (i = 0; i < BGMAC_MAX_RX_RINGS; i++)
-		bgmac_dma_ring_desc_free(bgmac, &bgmac->rx_ring[i]);
+		bgmac_dma_ring_desc_free(bgmac, &bgmac->rx_ring[i],
+					 BGMAC_RX_RING_SLOTS);
 }
 
 static int bgmac_dma_alloc(struct bgmac *bgmac)
@@ -599,11 +602,10 @@ static int bgmac_dma_alloc(struct bgmac *bgmac)
 
 	for (i = 0; i < BGMAC_MAX_TX_RINGS; i++) {
 		ring = &bgmac->tx_ring[i];
-		ring->num_slots = BGMAC_TX_RING_SLOTS;
 		ring->mmio_base = ring_base[i];
 
 		/* Alloc ring of descriptors */
-		size = ring->num_slots * sizeof(struct bgmac_dma_desc);
+		size = BGMAC_TX_RING_SLOTS * sizeof(struct bgmac_dma_desc);
 		ring->cpu_base = dma_zalloc_coherent(dma_dev, size,
 						     &ring->dma_base,
 						     GFP_KERNEL);
@@ -625,11 +627,10 @@ static int bgmac_dma_alloc(struct bgmac *bgmac)
 
 	for (i = 0; i < BGMAC_MAX_RX_RINGS; i++) {
 		ring = &bgmac->rx_ring[i];
-		ring->num_slots = BGMAC_RX_RING_SLOTS;
 		ring->mmio_base = ring_base[i];
 
 		/* Alloc ring of descriptors */
-		size = ring->num_slots * sizeof(struct bgmac_dma_desc);
+		size = BGMAC_RX_RING_SLOTS * sizeof(struct bgmac_dma_desc);
 		ring->cpu_base = dma_zalloc_coherent(dma_dev, size,
 						     &ring->dma_base,
 						     GFP_KERNEL);
@@ -692,7 +693,7 @@ static int bgmac_dma_init(struct bgmac *bgmac)
 
 		ring->start = 0;
 		ring->end = 0;
-		for (j = 0; j < ring->num_slots; j++) {
+		for (j = 0; j < BGMAC_RX_RING_SLOTS; j++) {
 			err = bgmac_dma_rx_skb_for_slot(bgmac, &ring->slots[j]);
 			if (err)
 				return err;
diff --git a/drivers/net/ethernet/broadcom/bgmac.h b/drivers/net/ethernet/broadcom/bgmac.h
index e45e303..db27feb 100644
--- a/drivers/net/ethernet/broadcom/bgmac.h
+++ b/drivers/net/ethernet/broadcom/bgmac.h
@@ -419,11 +419,10 @@ struct bgmac_dma_ring {
 	u32 start;
 	u32 end;
 
-	u16 num_slots;
-	u16 mmio_base;
 	struct bgmac_dma_desc *cpu_base;
 	dma_addr_t dma_base;
 	u32 index_base; /* Used for unaligned rings only, otherwise 0 */
+	u16 mmio_base;
 	bool unaligned;
 
 	struct bgmac_slot_info slots[BGMAC_RX_RING_SLOTS];
-- 
2.2.2

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v5 4/9] bgmac: simplify rx DMA error handling
  2015-04-13 17:27 ` [PATCH v5 4/9] bgmac: simplify rx DMA error handling Felix Fietkau
@ 2015-04-13 19:47   ` Felix Fietkau
  2015-04-13 23:20   ` Francois Romieu
  1 sibling, 0 replies; 13+ messages in thread
From: Felix Fietkau @ 2015-04-13 19:47 UTC (permalink / raw)
  To: netdev; +Cc: zajec5, hauke

On 2015-04-13 19:27, Felix Fietkau wrote:
> Unmap the DMA buffer before checking it. If an error occurs, free the
> buffer and allocate a new one. If allocation or mapping fails, retry as
> long as there is NAPI poll budget left (count every attempt instead of
> every frame).
> 
> Signed-off-by: Felix Fietkau <nbd@openwrt.org>
Sorry, have to send v6. There are some more issues in this patch
regarding the use of kfree().

- Felix

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v5 4/9] bgmac: simplify rx DMA error handling
  2015-04-13 17:27 ` [PATCH v5 4/9] bgmac: simplify rx DMA error handling Felix Fietkau
  2015-04-13 19:47   ` Felix Fietkau
@ 2015-04-13 23:20   ` Francois Romieu
  2015-04-14  0:04     ` David Miller
  1 sibling, 1 reply; 13+ messages in thread
From: Francois Romieu @ 2015-04-13 23:20 UTC (permalink / raw)
  To: Felix Fietkau; +Cc: netdev, zajec5, hauke

Felix Fietkau <nbd@openwrt.org> :
> Unmap the DMA buffer before checking it. If an error occurs, free the
> buffer and allocate a new one. If allocation or mapping fails, retry as
> long as there is NAPI poll budget left (count every attempt instead of
> every frame).

The driver is not supposed to use the NAPI budget this way.

If the driver can't reserve resources to handle the packet, it should
drop the data, keep everything in place for the hardware and move to the
next slot.

-- 
Ueimor

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v5 4/9] bgmac: simplify rx DMA error handling
  2015-04-13 23:20   ` Francois Romieu
@ 2015-04-14  0:04     ` David Miller
  2015-04-14  0:05       ` Felix Fietkau
  0 siblings, 1 reply; 13+ messages in thread
From: David Miller @ 2015-04-14  0:04 UTC (permalink / raw)
  To: romieu; +Cc: nbd, netdev, zajec5, hauke

From: Francois Romieu <romieu@fr.zoreil.com>
Date: Tue, 14 Apr 2015 01:20:40 +0200

> Felix Fietkau <nbd@openwrt.org> :
>> Unmap the DMA buffer before checking it. If an error occurs, free the
>> buffer and allocate a new one. If allocation or mapping fails, retry as
>> long as there is NAPI poll budget left (count every attempt instead of
>> every frame).
> 
> The driver is not supposed to use the NAPI budget this way.
> 
> If the driver can't reserve resources to handle the packet, it should
> drop the data, keep everything in place for the hardware and move to the
> next slot.

+1

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v5 4/9] bgmac: simplify rx DMA error handling
  2015-04-14  0:04     ` David Miller
@ 2015-04-14  0:05       ` Felix Fietkau
  0 siblings, 0 replies; 13+ messages in thread
From: Felix Fietkau @ 2015-04-14  0:05 UTC (permalink / raw)
  To: David Miller, romieu; +Cc: netdev, zajec5, hauke

On 2015-04-14 02:04, David Miller wrote:
> From: Francois Romieu <romieu@fr.zoreil.com>
> Date: Tue, 14 Apr 2015 01:20:40 +0200
> 
>> Felix Fietkau <nbd@openwrt.org> :
>>> Unmap the DMA buffer before checking it. If an error occurs, free the
>>> buffer and allocate a new one. If allocation or mapping fails, retry as
>>> long as there is NAPI poll budget left (count every attempt instead of
>>> every frame).
>> 
>> The driver is not supposed to use the NAPI budget this way.
>> 
>> If the driver can't reserve resources to handle the packet, it should
>> drop the data, keep everything in place for the hardware and move to the
>> next slot.
> 
> +1
Fixed in v6.

- Felix

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2015-04-14  0:05 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-13 17:27 [PATCH v5 1/9] bgmac: simplify tx ring index handling Felix Fietkau
2015-04-13 17:27 ` [PATCH v5 2/9] bgmac: leave interrupts disabled as long as there is work to do Felix Fietkau
2015-04-13 17:27 ` [PATCH v5 3/9] bgmac: set received skb headroom to NET_SKB_PAD Felix Fietkau
2015-04-13 17:27 ` [PATCH v5 4/9] bgmac: simplify rx DMA error handling Felix Fietkau
2015-04-13 19:47   ` Felix Fietkau
2015-04-13 23:20   ` Francois Romieu
2015-04-14  0:04     ` David Miller
2015-04-14  0:05       ` Felix Fietkau
2015-04-13 17:27 ` [PATCH v5 5/9] bgmac: add check for oversized packets Felix Fietkau
2015-04-13 17:27 ` [PATCH v5 6/9] bgmac: increase rx ring size from 511 to 512 Felix Fietkau
2015-04-13 17:27 ` [PATCH v5 7/9] bgmac: simplify dma init/cleanup Felix Fietkau
2015-04-13 17:27 ` [PATCH v5 8/9] bgmac: fix DMA rx corruption Felix Fietkau
2015-04-13 17:27 ` [PATCH v5 9/9] bgmac: drop ring->num_slots Felix Fietkau

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.