netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 net] lan743x: Fix RX Kernel Panic
@ 2019-03-11 17:39 Bryan Whitehead
  2019-03-11 18:36 ` David Miller
  0 siblings, 1 reply; 6+ messages in thread
From: Bryan Whitehead @ 2019-03-11 17:39 UTC (permalink / raw)
  To: davem; +Cc: netdev, UNGLinuxDriver

It has been noticed that running the speed test at
www.speedtest.net occasionally causes a kernel panic.

Investigation revealed that under this test RX buffer allocation
sometimes fails and returns NULL. But the lan743x driver did
not handle this case.

This patch fixes this issue by attempting to allocate a buffer
before sending the new rx packet to the OS. If the allocation
fails then the new rx packet is dropped and the existing buffer
is reused in the DMA ring.

Updates for v2:
    Additional 2 locations where allocation was not checked,
        has been changed to reuse existing buffer.

Fixes: 23f0703c125b ("lan743x: Add main source files for new lan743x driver")
Signed-off-by: Bryan Whitehead <Bryan.Whitehead@microchip.com>
---
 drivers/net/ethernet/microchip/lan743x_main.c | 46 +++++++++++++++++++--------
 1 file changed, 32 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/microchip/lan743x_main.c b/drivers/net/ethernet/microchip/lan743x_main.c
index 4d1b4a2..2876bab 100644
--- a/drivers/net/ethernet/microchip/lan743x_main.c
+++ b/drivers/net/ethernet/microchip/lan743x_main.c
@@ -1902,7 +1902,17 @@ static int lan743x_rx_next_index(struct lan743x_rx *rx, int index)
 	return ((++index) % rx->ring_size);
 }
 
-static int lan743x_rx_allocate_ring_element(struct lan743x_rx *rx, int index)
+static struct sk_buff *lan743x_rx_allocate_skb(struct lan743x_rx *rx)
+{
+	int length = 0;
+
+	length = (LAN743X_MAX_FRAME_SIZE + ETH_HLEN + 4 + RX_HEAD_PADDING);
+	return __netdev_alloc_skb(rx->adapter->netdev,
+				  length, GFP_ATOMIC | GFP_DMA);
+}
+
+static int lan743x_rx_init_ring_element(struct lan743x_rx *rx, int index,
+					struct sk_buff *skb)
 {
 	struct lan743x_rx_buffer_info *buffer_info;
 	struct lan743x_rx_descriptor *descriptor;
@@ -1911,9 +1921,7 @@ static int lan743x_rx_allocate_ring_element(struct lan743x_rx *rx, int index)
 	length = (LAN743X_MAX_FRAME_SIZE + ETH_HLEN + 4 + RX_HEAD_PADDING);
 	descriptor = &rx->ring_cpu_ptr[index];
 	buffer_info = &rx->buffer_info[index];
-	buffer_info->skb = __netdev_alloc_skb(rx->adapter->netdev,
-					      length,
-					      GFP_ATOMIC | GFP_DMA);
+	buffer_info->skb = skb;
 	if (!(buffer_info->skb))
 		return -ENOMEM;
 	buffer_info->dma_ptr = dma_map_single(&rx->adapter->pdev->dev,
@@ -2060,8 +2068,19 @@ static int lan743x_rx_process_packet(struct lan743x_rx *rx)
 		/* packet is available */
 		if (first_index == last_index) {
 			/* single buffer packet */
+			struct sk_buff *new_skb = NULL;
 			int packet_length;
 
+			new_skb = lan743x_rx_allocate_skb(rx);
+			if (!new_skb) {
+				/* failed to allocate next skb.
+				 * Memory is very low.
+				 * Drop this packet and reuse buffer.
+				 */
+				lan743x_rx_reuse_ring_element(rx, first_index);
+				goto process_extension;
+			}
+
 			buffer_info = &rx->buffer_info[first_index];
 			skb = buffer_info->skb;
 			descriptor = &rx->ring_cpu_ptr[first_index];
@@ -2081,7 +2100,7 @@ static int lan743x_rx_process_packet(struct lan743x_rx *rx)
 			skb_put(skb, packet_length - 4);
 			skb->protocol = eth_type_trans(skb,
 						       rx->adapter->netdev);
-			lan743x_rx_allocate_ring_element(rx, first_index);
+			lan743x_rx_init_ring_element(rx, first_index, new_skb);
 		} else {
 			int index = first_index;
 
@@ -2094,26 +2113,23 @@ static int lan743x_rx_process_packet(struct lan743x_rx *rx)
 			if (first_index <= last_index) {
 				while ((index >= first_index) &&
 				       (index <= last_index)) {
-					lan743x_rx_release_ring_element(rx,
-									index);
-					lan743x_rx_allocate_ring_element(rx,
-									 index);
+					lan743x_rx_reuse_ring_element(rx,
+								      index);
 					index = lan743x_rx_next_index(rx,
 								      index);
 				}
 			} else {
 				while ((index >= first_index) ||
 				       (index <= last_index)) {
-					lan743x_rx_release_ring_element(rx,
-									index);
-					lan743x_rx_allocate_ring_element(rx,
-									 index);
+					lan743x_rx_reuse_ring_element(rx,
+								      index);
 					index = lan743x_rx_next_index(rx,
 								      index);
 				}
 			}
 		}
 
+process_extension:
 		if (extension_index >= 0) {
 			descriptor = &rx->ring_cpu_ptr[extension_index];
 			buffer_info = &rx->buffer_info[extension_index];
@@ -2290,7 +2306,9 @@ static int lan743x_rx_ring_init(struct lan743x_rx *rx)
 
 	rx->last_head = 0;
 	for (index = 0; index < rx->ring_size; index++) {
-		ret = lan743x_rx_allocate_ring_element(rx, index);
+		struct sk_buff *new_skb = lan743x_rx_allocate_skb(rx);
+
+		ret = lan743x_rx_init_ring_element(rx, index, new_skb);
 		if (ret)
 			goto cleanup;
 	}
-- 
2.7.4


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

* Re: [PATCH v2 net] lan743x: Fix RX Kernel Panic
  2019-03-11 17:39 [PATCH v2 net] lan743x: Fix RX Kernel Panic Bryan Whitehead
@ 2019-03-11 18:36 ` David Miller
  2019-03-11 18:50   ` Bryan.Whitehead
  0 siblings, 1 reply; 6+ messages in thread
From: David Miller @ 2019-03-11 18:36 UTC (permalink / raw)
  To: Bryan.Whitehead; +Cc: netdev, UNGLinuxDriver

From: Bryan Whitehead <Bryan.Whitehead@microchip.com>
Date: Mon, 11 Mar 2019 13:39:39 -0400

> @@ -2290,7 +2306,9 @@ static int lan743x_rx_ring_init(struct lan743x_rx *rx)
>  
>  	rx->last_head = 0;
>  	for (index = 0; index < rx->ring_size; index++) {
> -		ret = lan743x_rx_allocate_ring_element(rx, index);
> +		struct sk_buff *new_skb = lan743x_rx_allocate_skb(rx);
> +
> +		ret = lan743x_rx_init_ring_element(rx, index, new_skb);

Need to check new_skb for NULL.

Again....

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

* RE: [PATCH v2 net] lan743x: Fix RX Kernel Panic
  2019-03-11 18:36 ` David Miller
@ 2019-03-11 18:50   ` Bryan.Whitehead
  2019-03-11 19:18     ` David Miller
  0 siblings, 1 reply; 6+ messages in thread
From: Bryan.Whitehead @ 2019-03-11 18:50 UTC (permalink / raw)
  To: davem; +Cc: netdev, UNGLinuxDriver

> -----Original Message-----
> From: David Miller <davem@davemloft.net>
> Sent: Monday, March 11, 2019 2:37 PM
> To: Bryan Whitehead - C21958 <Bryan.Whitehead@microchip.com>
> Cc: netdev@vger.kernel.org; UNGLinuxDriver
> <UNGLinuxDriver@microchip.com>
> Subject: Re: [PATCH v2 net] lan743x: Fix RX Kernel Panic
> 
> From: Bryan Whitehead <Bryan.Whitehead@microchip.com>
> Date: Mon, 11 Mar 2019 13:39:39 -0400
> 
> > @@ -2290,7 +2306,9 @@ static int lan743x_rx_ring_init(struct lan743x_rx
> *rx)
> >
> >  	rx->last_head = 0;
> >  	for (index = 0; index < rx->ring_size; index++) {
> > -		ret = lan743x_rx_allocate_ring_element(rx, index);
> > +		struct sk_buff *new_skb = lan743x_rx_allocate_skb(rx);
> > +
> > +		ret = lan743x_rx_init_ring_element(rx, index, new_skb);
> 
> Need to check new_skb for NULL.
> 
> Again....

David,

In this case the function lan743x_rx_init_ring_element does not assume that
new_skb is not NULL. It does check for NULL and returns error in that case.

It does this because allocation use to be inside lan743x_rx_init_ring_element,
when its name was lan743x_rx_allocate_ring_element.

I pulled the allocation part out to solve the other problem, but functionally it
Is exactly the same and will clean up appropriately if allocation returns NULL.

Regards,
Bryan

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

* Re: [PATCH v2 net] lan743x: Fix RX Kernel Panic
  2019-03-11 18:50   ` Bryan.Whitehead
@ 2019-03-11 19:18     ` David Miller
  2019-03-11 19:20       ` David Miller
  0 siblings, 1 reply; 6+ messages in thread
From: David Miller @ 2019-03-11 19:18 UTC (permalink / raw)
  To: Bryan.Whitehead; +Cc: netdev, UNGLinuxDriver

From: <Bryan.Whitehead@microchip.com>
Date: Mon, 11 Mar 2019 18:50:27 +0000

> In this case the function lan743x_rx_init_ring_element does not assume that
> new_skb is not NULL. It does check for NULL and returns error in that case.

Aha, I see, thanks for pointing that out and correcting me.  Let me
review this some more.

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

* Re: [PATCH v2 net] lan743x: Fix RX Kernel Panic
  2019-03-11 19:18     ` David Miller
@ 2019-03-11 19:20       ` David Miller
  2019-03-11 19:22         ` Bryan.Whitehead
  0 siblings, 1 reply; 6+ messages in thread
From: David Miller @ 2019-03-11 19:20 UTC (permalink / raw)
  To: Bryan.Whitehead; +Cc: netdev, UNGLinuxDriver

From: David Miller <davem@davemloft.net>
Date: Mon, 11 Mar 2019 12:18:12 -0700 (PDT)

> From: <Bryan.Whitehead@microchip.com>
> Date: Mon, 11 Mar 2019 18:50:27 +0000
> 
>> In this case the function lan743x_rx_init_ring_element does not assume that
>> new_skb is not NULL. It does check for NULL and returns error in that case.
> 
> Aha, I see, thanks for pointing that out and correcting me.  Let me
> review this some more.

Applied and queued up for -stable, thanks.

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

* RE: [PATCH v2 net] lan743x: Fix RX Kernel Panic
  2019-03-11 19:20       ` David Miller
@ 2019-03-11 19:22         ` Bryan.Whitehead
  0 siblings, 0 replies; 6+ messages in thread
From: Bryan.Whitehead @ 2019-03-11 19:22 UTC (permalink / raw)
  To: davem; +Cc: netdev, UNGLinuxDriver

> Applied and queued up for -stable, thanks.
Thanks David

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

end of thread, other threads:[~2019-03-11 19:22 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-11 17:39 [PATCH v2 net] lan743x: Fix RX Kernel Panic Bryan Whitehead
2019-03-11 18:36 ` David Miller
2019-03-11 18:50   ` Bryan.Whitehead
2019-03-11 19:18     ` David Miller
2019-03-11 19:20       ` David Miller
2019-03-11 19:22         ` Bryan.Whitehead

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).