linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/6] net: stmmac: fix handling of oversized frames
@ 2019-03-27 20:35 Aaro Koskinen
  2019-03-27 20:35 ` [PATCH 1/6] net: stmmac: use correct DMA buffer size in the RX descriptor Aaro Koskinen
                   ` (7 more replies)
  0 siblings, 8 replies; 11+ messages in thread
From: Aaro Koskinen @ 2019-03-27 20:35 UTC (permalink / raw)
  To: David S. Miller, Jose Abreu, Alexandre Torgue,
	Giuseppe Cavallaro, netdev
  Cc: linux-kernel, Aaro Koskinen

From: Aaro Koskinen <aaro.koskinen@nokia.com>

Hi,

I accidentally had MTU size mismatch (9000 vs. 1500) in my network,
and I noticed I could kill a system using stmmac & 1500 MTU simply
by pinging it with "ping -s 2000 ...".

While testing a fix I encountered also some other issues that need fixing.

I have tested these only with enhanced descriptors, so the normal
descriptor changes need a careful review.

A.

Aaro Koskinen (6):
  net: stmmac: use correct DMA buffer size in the RX descriptor
  net: stmmac: ratelimit RX error logs
  net: stmmac: don't stop NAPI processing when dropping a packet
  net: stmmac: don't overwrite discard_frame status
  net: stmmac: fix dropping of multi-descriptor RX frames
  net: stmmac: don't log oversized frames

 .../net/ethernet/stmicro/stmmac/descs_com.h   | 22 +++++++-----
 .../ethernet/stmicro/stmmac/dwmac4_descs.c    |  2 +-
 .../ethernet/stmicro/stmmac/dwxgmac2_descs.c  |  2 +-
 .../net/ethernet/stmicro/stmmac/enh_desc.c    | 22 ++++++++----
 drivers/net/ethernet/stmicro/stmmac/hwif.h    |  2 +-
 .../net/ethernet/stmicro/stmmac/norm_desc.c   | 12 ++++---
 .../net/ethernet/stmicro/stmmac/stmmac_main.c | 34 +++++++++++--------
 7 files changed, 59 insertions(+), 37 deletions(-)

-- 
2.17.0


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

* [PATCH 1/6] net: stmmac: use correct DMA buffer size in the RX descriptor
  2019-03-27 20:35 [PATCH 0/6] net: stmmac: fix handling of oversized frames Aaro Koskinen
@ 2019-03-27 20:35 ` Aaro Koskinen
  2019-03-29 10:02   ` Jose Abreu
  2019-03-27 20:35 ` [PATCH 2/6] net: stmmac: ratelimit RX error logs Aaro Koskinen
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 11+ messages in thread
From: Aaro Koskinen @ 2019-03-27 20:35 UTC (permalink / raw)
  To: David S. Miller, Jose Abreu, Alexandre Torgue,
	Giuseppe Cavallaro, netdev
  Cc: linux-kernel, Aaro Koskinen

From: Aaro Koskinen <aaro.koskinen@nokia.com>

We always program the maximum DMA buffer size into the receive descriptor,
although the allocated size may be less. E.g. with the default MTU size
we allocate only 1536 bytes. If somebody sends us a bigger frame, then
memory may get corrupted.

Fix by using exact buffer sizes.

Signed-off-by: Aaro Koskinen <aaro.koskinen@nokia.com>
---
 .../net/ethernet/stmicro/stmmac/descs_com.h   | 22 ++++++++++++-------
 .../ethernet/stmicro/stmmac/dwmac4_descs.c    |  2 +-
 .../ethernet/stmicro/stmmac/dwxgmac2_descs.c  |  2 +-
 .../net/ethernet/stmicro/stmmac/enh_desc.c    | 10 ++++++---
 drivers/net/ethernet/stmicro/stmmac/hwif.h    |  2 +-
 .../net/ethernet/stmicro/stmmac/norm_desc.c   | 10 ++++++---
 .../net/ethernet/stmicro/stmmac/stmmac_main.c |  6 +++--
 7 files changed, 35 insertions(+), 19 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/descs_com.h b/drivers/net/ethernet/stmicro/stmmac/descs_com.h
index 40d6356a7e73..3dfb07a78952 100644
--- a/drivers/net/ethernet/stmicro/stmmac/descs_com.h
+++ b/drivers/net/ethernet/stmicro/stmmac/descs_com.h
@@ -29,11 +29,13 @@
 /* Specific functions used for Ring mode */
 
 /* Enhanced descriptors */
-static inline void ehn_desc_rx_set_on_ring(struct dma_desc *p, int end)
+static inline void ehn_desc_rx_set_on_ring(struct dma_desc *p, int end,
+					   int bfsize)
 {
-	p->des1 |= cpu_to_le32((BUF_SIZE_8KiB
-			<< ERDES1_BUFFER2_SIZE_SHIFT)
-		   & ERDES1_BUFFER2_SIZE_MASK);
+	if (bfsize == BUF_SIZE_16KiB)
+		p->des1 |= cpu_to_le32((BUF_SIZE_8KiB
+				<< ERDES1_BUFFER2_SIZE_SHIFT)
+			   & ERDES1_BUFFER2_SIZE_MASK);
 
 	if (end)
 		p->des1 |= cpu_to_le32(ERDES1_END_RING);
@@ -59,11 +61,15 @@ static inline void enh_set_tx_desc_len_on_ring(struct dma_desc *p, int len)
 }
 
 /* Normal descriptors */
-static inline void ndesc_rx_set_on_ring(struct dma_desc *p, int end)
+static inline void ndesc_rx_set_on_ring(struct dma_desc *p, int end, int bfsize)
 {
-	p->des1 |= cpu_to_le32(((BUF_SIZE_2KiB - 1)
-				<< RDES1_BUFFER2_SIZE_SHIFT)
-		    & RDES1_BUFFER2_SIZE_MASK);
+	if (bfsize >= BUF_SIZE_2KiB) {
+		int bfsize2;
+
+		bfsize2 = min(bfsize - BUF_SIZE_2KiB + 1, BUF_SIZE_2KiB - 1);
+		p->des1 |= cpu_to_le32((bfsize2 << RDES1_BUFFER2_SIZE_SHIFT)
+			    & RDES1_BUFFER2_SIZE_MASK);
+	}
 
 	if (end)
 		p->des1 |= cpu_to_le32(RDES1_END_RING);
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
index 7fbb6a4dbf51..e061e9f5fad7 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
@@ -296,7 +296,7 @@ static int dwmac4_wrback_get_rx_timestamp_status(void *desc, void *next_desc,
 }
 
 static void dwmac4_rd_init_rx_desc(struct dma_desc *p, int disable_rx_ic,
-				   int mode, int end)
+				   int mode, int end, int bfsize)
 {
 	dwmac4_set_rx_owner(p, disable_rx_ic);
 }
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c
index 1d858fdec997..98fa471da7c0 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c
@@ -123,7 +123,7 @@ static int dwxgmac2_get_rx_timestamp_status(void *desc, void *next_desc,
 }
 
 static void dwxgmac2_init_rx_desc(struct dma_desc *p, int disable_rx_ic,
-				  int mode, int end)
+				  int mode, int end, int bfsize)
 {
 	dwxgmac2_set_rx_owner(p, disable_rx_ic);
 }
diff --git a/drivers/net/ethernet/stmicro/stmmac/enh_desc.c b/drivers/net/ethernet/stmicro/stmmac/enh_desc.c
index 5ef91a790f9d..e8855e6adb48 100644
--- a/drivers/net/ethernet/stmicro/stmmac/enh_desc.c
+++ b/drivers/net/ethernet/stmicro/stmmac/enh_desc.c
@@ -259,15 +259,19 @@ static int enh_desc_get_rx_status(void *data, struct stmmac_extra_stats *x,
 }
 
 static void enh_desc_init_rx_desc(struct dma_desc *p, int disable_rx_ic,
-				  int mode, int end)
+				  int mode, int end, int bfsize)
 {
+	int bfsize1;
+
 	p->des0 |= cpu_to_le32(RDES0_OWN);
-	p->des1 |= cpu_to_le32(BUF_SIZE_8KiB & ERDES1_BUFFER1_SIZE_MASK);
+
+	bfsize1 = min(bfsize, BUF_SIZE_8KiB);
+	p->des1 |= cpu_to_le32(bfsize1 & ERDES1_BUFFER1_SIZE_MASK);
 
 	if (mode == STMMAC_CHAIN_MODE)
 		ehn_desc_rx_set_on_chain(p);
 	else
-		ehn_desc_rx_set_on_ring(p, end);
+		ehn_desc_rx_set_on_ring(p, end, bfsize);
 
 	if (disable_rx_ic)
 		p->des1 |= cpu_to_le32(ERDES1_DISABLE_IC);
diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.h b/drivers/net/ethernet/stmicro/stmmac/hwif.h
index 92b8944f26e3..5bb00234d961 100644
--- a/drivers/net/ethernet/stmicro/stmmac/hwif.h
+++ b/drivers/net/ethernet/stmicro/stmmac/hwif.h
@@ -33,7 +33,7 @@ struct dma_extended_desc;
 struct stmmac_desc_ops {
 	/* DMA RX descriptor ring initialization */
 	void (*init_rx_desc)(struct dma_desc *p, int disable_rx_ic, int mode,
-			int end);
+			int end, int bfsize);
 	/* DMA TX descriptor ring initialization */
 	void (*init_tx_desc)(struct dma_desc *p, int mode, int end);
 	/* Invoked by the xmit function to prepare the tx descriptor */
diff --git a/drivers/net/ethernet/stmicro/stmmac/norm_desc.c b/drivers/net/ethernet/stmicro/stmmac/norm_desc.c
index de65bb29feba..c55a9815b394 100644
--- a/drivers/net/ethernet/stmicro/stmmac/norm_desc.c
+++ b/drivers/net/ethernet/stmicro/stmmac/norm_desc.c
@@ -135,15 +135,19 @@ static int ndesc_get_rx_status(void *data, struct stmmac_extra_stats *x,
 }
 
 static void ndesc_init_rx_desc(struct dma_desc *p, int disable_rx_ic, int mode,
-			       int end)
+			       int end, int bfsize)
 {
+	int bfsize1;
+
 	p->des0 |= cpu_to_le32(RDES0_OWN);
-	p->des1 |= cpu_to_le32((BUF_SIZE_2KiB - 1) & RDES1_BUFFER1_SIZE_MASK);
+
+	bfsize1 = min(bfsize, BUF_SIZE_2KiB - 1);
+	p->des1 |= cpu_to_le32(bfsize & RDES1_BUFFER1_SIZE_MASK);
 
 	if (mode == STMMAC_CHAIN_MODE)
 		ndesc_rx_set_on_chain(p, end);
 	else
-		ndesc_rx_set_on_ring(p, end);
+		ndesc_rx_set_on_ring(p, end, bfsize);
 
 	if (disable_rx_ic)
 		p->des1 |= cpu_to_le32(RDES1_DISABLE_IC);
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 6a2e1031a62a..4e496cf655f2 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -1136,11 +1136,13 @@ static void stmmac_clear_rx_descriptors(struct stmmac_priv *priv, u32 queue)
 		if (priv->extend_desc)
 			stmmac_init_rx_desc(priv, &rx_q->dma_erx[i].basic,
 					priv->use_riwt, priv->mode,
-					(i == DMA_RX_SIZE - 1));
+					(i == DMA_RX_SIZE - 1),
+					priv->dma_buf_sz);
 		else
 			stmmac_init_rx_desc(priv, &rx_q->dma_rx[i],
 					priv->use_riwt, priv->mode,
-					(i == DMA_RX_SIZE - 1));
+					(i == DMA_RX_SIZE - 1),
+					priv->dma_buf_sz);
 }
 
 /**
-- 
2.17.0


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

* [PATCH 2/6] net: stmmac: ratelimit RX error logs
  2019-03-27 20:35 [PATCH 0/6] net: stmmac: fix handling of oversized frames Aaro Koskinen
  2019-03-27 20:35 ` [PATCH 1/6] net: stmmac: use correct DMA buffer size in the RX descriptor Aaro Koskinen
@ 2019-03-27 20:35 ` Aaro Koskinen
  2019-03-27 20:35 ` [PATCH 3/6] net: stmmac: don't stop NAPI processing when dropping a packet Aaro Koskinen
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Aaro Koskinen @ 2019-03-27 20:35 UTC (permalink / raw)
  To: David S. Miller, Jose Abreu, Alexandre Torgue,
	Giuseppe Cavallaro, netdev
  Cc: linux-kernel, Aaro Koskinen

From: Aaro Koskinen <aaro.koskinen@nokia.com>

Ratelimit RX error logs.

Signed-off-by: Aaro Koskinen <aaro.koskinen@nokia.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 4e496cf655f2..392d94cede17 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -3433,9 +3433,10 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
 			 *  ignored
 			 */
 			if (frame_len > priv->dma_buf_sz) {
-				netdev_err(priv->dev,
-					   "len %d larger than size (%d)\n",
-					   frame_len, priv->dma_buf_sz);
+				if (net_ratelimit())
+					netdev_err(priv->dev,
+						   "len %d larger than size (%d)\n",
+						   frame_len, priv->dma_buf_sz);
 				priv->dev->stats.rx_length_errors++;
 				break;
 			}
@@ -3492,9 +3493,10 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
 			} else {
 				skb = rx_q->rx_skbuff[entry];
 				if (unlikely(!skb)) {
-					netdev_err(priv->dev,
-						   "%s: Inconsistent Rx chain\n",
-						   priv->dev->name);
+					if (net_ratelimit())
+						netdev_err(priv->dev,
+							   "%s: Inconsistent Rx chain\n",
+							   priv->dev->name);
 					priv->dev->stats.rx_dropped++;
 					break;
 				}
-- 
2.17.0


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

* [PATCH 3/6] net: stmmac: don't stop NAPI processing when dropping a packet
  2019-03-27 20:35 [PATCH 0/6] net: stmmac: fix handling of oversized frames Aaro Koskinen
  2019-03-27 20:35 ` [PATCH 1/6] net: stmmac: use correct DMA buffer size in the RX descriptor Aaro Koskinen
  2019-03-27 20:35 ` [PATCH 2/6] net: stmmac: ratelimit RX error logs Aaro Koskinen
@ 2019-03-27 20:35 ` Aaro Koskinen
  2019-03-27 20:35 ` [PATCH 4/6] net: stmmac: don't overwrite discard_frame status Aaro Koskinen
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Aaro Koskinen @ 2019-03-27 20:35 UTC (permalink / raw)
  To: David S. Miller, Jose Abreu, Alexandre Torgue,
	Giuseppe Cavallaro, netdev
  Cc: linux-kernel, Aaro Koskinen

From: Aaro Koskinen <aaro.koskinen@nokia.com>

Currently, if we drop a packet, we exit from NAPI loop before the budget
is consumed. In some situations this will make the RX processing stall
e.g. when flood pinging the system with oversized packets, as the
errorneous packets are not dropped efficiently.

If we drop a packet, we should just continue to the next one as long as
the budget allows.

Signed-off-by: Aaro Koskinen <aaro.koskinen@nokia.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 392d94cede17..a26e36dbb5df 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -3354,9 +3354,8 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
 {
 	struct stmmac_rx_queue *rx_q = &priv->rx_queue[queue];
 	struct stmmac_channel *ch = &priv->channel[queue];
-	unsigned int entry = rx_q->cur_rx;
+	unsigned int next_entry = rx_q->cur_rx;
 	int coe = priv->hw->rx_csum;
-	unsigned int next_entry;
 	unsigned int count = 0;
 	bool xmac;
 
@@ -3374,10 +3373,12 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
 		stmmac_display_ring(priv, rx_head, DMA_RX_SIZE, true);
 	}
 	while (count < limit) {
-		int status;
+		int entry, status;
 		struct dma_desc *p;
 		struct dma_desc *np;
 
+		entry = next_entry;
+
 		if (priv->extend_desc)
 			p = (struct dma_desc *)(rx_q->dma_erx + entry);
 		else
@@ -3438,7 +3439,7 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
 						   "len %d larger than size (%d)\n",
 						   frame_len, priv->dma_buf_sz);
 				priv->dev->stats.rx_length_errors++;
-				break;
+				continue;
 			}
 
 			/* ACS is set; GMAC core strips PAD/FCS for IEEE 802.3
@@ -3473,7 +3474,7 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
 						dev_warn(priv->device,
 							 "packet dropped\n");
 					priv->dev->stats.rx_dropped++;
-					break;
+					continue;
 				}
 
 				dma_sync_single_for_cpu(priv->device,
@@ -3498,7 +3499,7 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
 							   "%s: Inconsistent Rx chain\n",
 							   priv->dev->name);
 					priv->dev->stats.rx_dropped++;
-					break;
+					continue;
 				}
 				prefetch(skb->data - NET_IP_ALIGN);
 				rx_q->rx_skbuff[entry] = NULL;
@@ -3533,7 +3534,6 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
 			priv->dev->stats.rx_packets++;
 			priv->dev->stats.rx_bytes += frame_len;
 		}
-		entry = next_entry;
 	}
 
 	stmmac_rx_refill(priv, queue);
-- 
2.17.0


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

* [PATCH 4/6] net: stmmac: don't overwrite discard_frame status
  2019-03-27 20:35 [PATCH 0/6] net: stmmac: fix handling of oversized frames Aaro Koskinen
                   ` (2 preceding siblings ...)
  2019-03-27 20:35 ` [PATCH 3/6] net: stmmac: don't stop NAPI processing when dropping a packet Aaro Koskinen
@ 2019-03-27 20:35 ` Aaro Koskinen
  2019-03-27 20:35 ` [PATCH 5/6] net: stmmac: fix dropping of multi-descriptor RX frames Aaro Koskinen
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Aaro Koskinen @ 2019-03-27 20:35 UTC (permalink / raw)
  To: David S. Miller, Jose Abreu, Alexandre Torgue,
	Giuseppe Cavallaro, netdev
  Cc: linux-kernel, Aaro Koskinen

From: Aaro Koskinen <aaro.koskinen@nokia.com>

If we have error bits set, the discard_frame status will get overwritten
by checksum bit checks, which might set the status back to good one.
Fix by checking the COE status only if the frame is good.

Signed-off-by: Aaro Koskinen <aaro.koskinen@nokia.com>
---
 drivers/net/ethernet/stmicro/stmmac/enh_desc.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/enh_desc.c b/drivers/net/ethernet/stmicro/stmmac/enh_desc.c
index e8855e6adb48..c42ef6c729c0 100644
--- a/drivers/net/ethernet/stmicro/stmmac/enh_desc.c
+++ b/drivers/net/ethernet/stmicro/stmmac/enh_desc.c
@@ -231,9 +231,10 @@ static int enh_desc_get_rx_status(void *data, struct stmmac_extra_stats *x,
 	 * It doesn't match with the information reported into the databook.
 	 * At any rate, we need to understand if the CSUM hw computation is ok
 	 * and report this info to the upper layers. */
-	ret = enh_desc_coe_rdes0(!!(rdes0 & RDES0_IPC_CSUM_ERROR),
-				 !!(rdes0 & RDES0_FRAME_TYPE),
-				 !!(rdes0 & ERDES0_RX_MAC_ADDR));
+	if (likely(ret == good_frame))
+		ret = enh_desc_coe_rdes0(!!(rdes0 & RDES0_IPC_CSUM_ERROR),
+					 !!(rdes0 & RDES0_FRAME_TYPE),
+					 !!(rdes0 & ERDES0_RX_MAC_ADDR));
 
 	if (unlikely(rdes0 & RDES0_DRIBBLING))
 		x->dribbling_bit++;
-- 
2.17.0


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

* [PATCH 5/6] net: stmmac: fix dropping of multi-descriptor RX frames
  2019-03-27 20:35 [PATCH 0/6] net: stmmac: fix handling of oversized frames Aaro Koskinen
                   ` (3 preceding siblings ...)
  2019-03-27 20:35 ` [PATCH 4/6] net: stmmac: don't overwrite discard_frame status Aaro Koskinen
@ 2019-03-27 20:35 ` Aaro Koskinen
  2019-03-27 20:35 ` [PATCH 6/6] net: stmmac: don't log oversized frames Aaro Koskinen
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Aaro Koskinen @ 2019-03-27 20:35 UTC (permalink / raw)
  To: David S. Miller, Jose Abreu, Alexandre Torgue,
	Giuseppe Cavallaro, netdev
  Cc: linux-kernel, Aaro Koskinen

From: Aaro Koskinen <aaro.koskinen@nokia.com>

Packets without the last descriptor set should be dropped early. If we
receive a frame larger than the DMA buffer, the HW will continue using the
next descriptor. Driver mistakes these as individual frames, and sometimes
a truncated frame (without the LD set) may look like a valid packet.

This fixes a strange issue where the system replies to 4098-byte ping
although the MTU/DMA buffer size is set to 4096, and yet at the same
time it's logging an oversized packet.

Signed-off-by: Aaro Koskinen <aaro.koskinen@nokia.com>
---
 drivers/net/ethernet/stmicro/stmmac/enh_desc.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/enh_desc.c b/drivers/net/ethernet/stmicro/stmmac/enh_desc.c
index c42ef6c729c0..5202d6ad7919 100644
--- a/drivers/net/ethernet/stmicro/stmmac/enh_desc.c
+++ b/drivers/net/ethernet/stmicro/stmmac/enh_desc.c
@@ -201,6 +201,11 @@ static int enh_desc_get_rx_status(void *data, struct stmmac_extra_stats *x,
 	if (unlikely(rdes0 & RDES0_OWN))
 		return dma_own;
 
+	if (unlikely(!(rdes0 & RDES0_LAST_DESCRIPTOR))) {
+		stats->rx_length_errors++;
+		return discard_frame;
+	}
+
 	if (unlikely(rdes0 & RDES0_ERROR_SUMMARY)) {
 		if (unlikely(rdes0 & RDES0_DESCRIPTOR_ERROR)) {
 			x->rx_desc++;
-- 
2.17.0


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

* [PATCH 6/6] net: stmmac: don't log oversized frames
  2019-03-27 20:35 [PATCH 0/6] net: stmmac: fix handling of oversized frames Aaro Koskinen
                   ` (4 preceding siblings ...)
  2019-03-27 20:35 ` [PATCH 5/6] net: stmmac: fix dropping of multi-descriptor RX frames Aaro Koskinen
@ 2019-03-27 20:35 ` Aaro Koskinen
  2019-03-29  0:10 ` [PATCH 0/6] net: stmmac: fix handling of " David Miller
  2019-03-31 21:01 ` David Miller
  7 siblings, 0 replies; 11+ messages in thread
From: Aaro Koskinen @ 2019-03-27 20:35 UTC (permalink / raw)
  To: David S. Miller, Jose Abreu, Alexandre Torgue,
	Giuseppe Cavallaro, netdev
  Cc: linux-kernel, Aaro Koskinen

From: Aaro Koskinen <aaro.koskinen@nokia.com>

This is log is harmful as it can trigger multiple times per packet. Delete
it.

Signed-off-by: Aaro Koskinen <aaro.koskinen@nokia.com>
---
 drivers/net/ethernet/stmicro/stmmac/norm_desc.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/norm_desc.c b/drivers/net/ethernet/stmicro/stmmac/norm_desc.c
index c55a9815b394..b7dd4e3c760d 100644
--- a/drivers/net/ethernet/stmicro/stmmac/norm_desc.c
+++ b/drivers/net/ethernet/stmicro/stmmac/norm_desc.c
@@ -91,8 +91,6 @@ static int ndesc_get_rx_status(void *data, struct stmmac_extra_stats *x,
 		return dma_own;
 
 	if (unlikely(!(rdes0 & RDES0_LAST_DESCRIPTOR))) {
-		pr_warn("%s: Oversized frame spanned multiple buffers\n",
-			__func__);
 		stats->rx_length_errors++;
 		return discard_frame;
 	}
-- 
2.17.0


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

* Re: [PATCH 0/6] net: stmmac: fix handling of oversized frames
  2019-03-27 20:35 [PATCH 0/6] net: stmmac: fix handling of oversized frames Aaro Koskinen
                   ` (5 preceding siblings ...)
  2019-03-27 20:35 ` [PATCH 6/6] net: stmmac: don't log oversized frames Aaro Koskinen
@ 2019-03-29  0:10 ` David Miller
  2019-03-31 21:01 ` David Miller
  7 siblings, 0 replies; 11+ messages in thread
From: David Miller @ 2019-03-29  0:10 UTC (permalink / raw)
  To: aaro.koskinen
  Cc: joabreu, alexandre.torgue, peppe.cavallaro, netdev, linux-kernel,
	aaro.koskinen

From: Aaro Koskinen <aaro.koskinen@iki.fi>
Date: Wed, 27 Mar 2019 22:35:34 +0200

> I accidentally had MTU size mismatch (9000 vs. 1500) in my network,
> and I noticed I could kill a system using stmmac & 1500 MTU simply
> by pinging it with "ping -s 2000 ...".
> 
> While testing a fix I encountered also some other issues that need fixing.
> 
> I have tested these only with enhanced descriptors, so the normal
> descriptor changes need a careful review.

I really want to see some stmmac developers review and/or test these
changes.

Thanks.

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

* Re: [PATCH 1/6] net: stmmac: use correct DMA buffer size in the RX descriptor
  2019-03-27 20:35 ` [PATCH 1/6] net: stmmac: use correct DMA buffer size in the RX descriptor Aaro Koskinen
@ 2019-03-29 10:02   ` Jose Abreu
  2019-03-29 10:48     ` Aaro Koskinen
  0 siblings, 1 reply; 11+ messages in thread
From: Jose Abreu @ 2019-03-29 10:02 UTC (permalink / raw)
  To: Aaro Koskinen, David S. Miller, Jose Abreu, Alexandre Torgue,
	Giuseppe Cavallaro, netdev
  Cc: linux-kernel, Aaro Koskinen

On 3/27/2019 8:35 PM, Aaro Koskinen wrote:
> From: Aaro Koskinen <aaro.koskinen@nokia.com>
> 
> We always program the maximum DMA buffer size into the receive descriptor,
> although the allocated size may be less. E.g. with the default MTU size
> we allocate only 1536 bytes. If somebody sends us a bigger frame, then
> memory may get corrupted.
> 
> Fix by using exact buffer sizes.
> 
> Signed-off-by: Aaro Koskinen <aaro.koskinen@nokia.com>

So, I'm seeing that the maximum allowed buffer size that can be
put in the descriptor changes between enhanced descriptors to
normal descriptors (8KB vs. 2KB right ?).

Shouldn't stmmac_main know about this limit before trying to init
descriptors ?

We do limit the MTU according to HW version but I would rather
prefer not having to need to calculate min() values in the
descriptor code and just use the value as is ...

Thanks,
Jose Miguel Abreu

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

* Re: [PATCH 1/6] net: stmmac: use correct DMA buffer size in the RX descriptor
  2019-03-29 10:02   ` Jose Abreu
@ 2019-03-29 10:48     ` Aaro Koskinen
  0 siblings, 0 replies; 11+ messages in thread
From: Aaro Koskinen @ 2019-03-29 10:48 UTC (permalink / raw)
  To: Jose Abreu
  Cc: David S. Miller, Alexandre Torgue, Giuseppe Cavallaro, netdev,
	linux-kernel, Aaro Koskinen

Hi,

On Fri, Mar 29, 2019 at 10:02:20AM +0000, Jose Abreu wrote:
> On 3/27/2019 8:35 PM, Aaro Koskinen wrote:
> > From: Aaro Koskinen <aaro.koskinen@nokia.com>
> > 
> > We always program the maximum DMA buffer size into the receive descriptor,
> > although the allocated size may be less. E.g. with the default MTU size
> > we allocate only 1536 bytes. If somebody sends us a bigger frame, then
> > memory may get corrupted.
> > 
> > Fix by using exact buffer sizes.
> > 
> > Signed-off-by: Aaro Koskinen <aaro.koskinen@nokia.com>
> 
> So, I'm seeing that the maximum allowed buffer size that can be
> put in the descriptor changes between enhanced descriptors to
> normal descriptors (8KB vs. 2KB right ?).

There are two size fields, so it's 16KB vs 4 KB.

> Shouldn't stmmac_main know about this limit before trying to init
> descriptors ?

Yes, and that is not a problem.

> We do limit the MTU according to HW version but I would rather
> prefer not having to need to calculate min() values in the
> descriptor code and just use the value as is ...

I don't think the calculation can be avoided, as the passed value can
be bigger than a single field can hold, or less than the maximum.

A.

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

* Re: [PATCH 0/6] net: stmmac: fix handling of oversized frames
  2019-03-27 20:35 [PATCH 0/6] net: stmmac: fix handling of oversized frames Aaro Koskinen
                   ` (6 preceding siblings ...)
  2019-03-29  0:10 ` [PATCH 0/6] net: stmmac: fix handling of " David Miller
@ 2019-03-31 21:01 ` David Miller
  7 siblings, 0 replies; 11+ messages in thread
From: David Miller @ 2019-03-31 21:01 UTC (permalink / raw)
  To: aaro.koskinen
  Cc: joabreu, alexandre.torgue, peppe.cavallaro, netdev, linux-kernel,
	aaro.koskinen

From: Aaro Koskinen <aaro.koskinen@iki.fi>
Date: Wed, 27 Mar 2019 22:35:34 +0200

> I accidentally had MTU size mismatch (9000 vs. 1500) in my network,
> and I noticed I could kill a system using stmmac & 1500 MTU simply
> by pinging it with "ping -s 2000 ...".
> 
> While testing a fix I encountered also some other issues that need fixing.
> 
> I have tested these only with enhanced descriptors, so the normal
> descriptor changes need a careful review.

Series applied, thanks Aaro.

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

end of thread, other threads:[~2019-03-31 21:01 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-27 20:35 [PATCH 0/6] net: stmmac: fix handling of oversized frames Aaro Koskinen
2019-03-27 20:35 ` [PATCH 1/6] net: stmmac: use correct DMA buffer size in the RX descriptor Aaro Koskinen
2019-03-29 10:02   ` Jose Abreu
2019-03-29 10:48     ` Aaro Koskinen
2019-03-27 20:35 ` [PATCH 2/6] net: stmmac: ratelimit RX error logs Aaro Koskinen
2019-03-27 20:35 ` [PATCH 3/6] net: stmmac: don't stop NAPI processing when dropping a packet Aaro Koskinen
2019-03-27 20:35 ` [PATCH 4/6] net: stmmac: don't overwrite discard_frame status Aaro Koskinen
2019-03-27 20:35 ` [PATCH 5/6] net: stmmac: fix dropping of multi-descriptor RX frames Aaro Koskinen
2019-03-27 20:35 ` [PATCH 6/6] net: stmmac: don't log oversized frames Aaro Koskinen
2019-03-29  0:10 ` [PATCH 0/6] net: stmmac: fix handling of " David Miller
2019-03-31 21:01 ` David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).