linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net v2 0/3] net: lantiq_xrx200: fix errors under memory pressure
@ 2022-08-15 14:57 Aleksander Jan Bajkowski
  2022-08-15 14:57 ` [PATCH net v2 1/3] net: lantiq_xrx200: confirm skb is allocated before using Aleksander Jan Bajkowski
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Aleksander Jan Bajkowski @ 2022-08-15 14:57 UTC (permalink / raw)
  To: hauke, davem, edumazet, kuba, pabeni, olek2, netdev, linux-kernel

This series fixes issues that can occur in the driver under memory pressure.
Situations when the system cannot allocate memory are rare, so the mentioned bugs
have been fixed recently. The patches have been tested on a BT Home router with the
Lantiq xRX200 chipset.

Changelog:

  v2:
  - the second patch has been changed, so that under memory pressure situation
    the driver will not receive packets indefinitely regardless of the NAPI budget,
  - the third patch has been added.

Aleksander Jan Bajkowski (3):
  net: lantiq_xrx200: confirm skb is allocated before using
  net: lantiq_xrx200: fix lock under memory pressure
  net: lantiq_xrx200: restore buffer if memory allocation failed

 drivers/net/ethernet/lantiq_xrx200.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

-- 
2.30.2


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

* [PATCH net v2 1/3] net: lantiq_xrx200: confirm skb is allocated before using
  2022-08-15 14:57 [PATCH net v2 0/3] net: lantiq_xrx200: fix errors under memory pressure Aleksander Jan Bajkowski
@ 2022-08-15 14:57 ` Aleksander Jan Bajkowski
  2022-08-17  3:41   ` Jakub Kicinski
  2022-08-15 14:57 ` [PATCH net v2 2/3] net: lantiq_xrx200: fix lock under memory pressure Aleksander Jan Bajkowski
  2022-08-15 14:57 ` [PATCH net v2 3/3] net: lantiq_xrx200: restore buffer if memory allocation failed Aleksander Jan Bajkowski
  2 siblings, 1 reply; 5+ messages in thread
From: Aleksander Jan Bajkowski @ 2022-08-15 14:57 UTC (permalink / raw)
  To: hauke, davem, edumazet, kuba, pabeni, olek2, netdev, linux-kernel

xrx200_hw_receive() assumes build_skb() always works and goes straight
to skb_reserve(). However, build_skb() can fail under memory pressure.

Add a check in case build_skb() failed to allocate and return NULL.

Fixes: e015593573b3 ("net: lantiq_xrx200: convert to build_skb")
Reported-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Aleksander Jan Bajkowski <olek2@wp.pl>
---
 drivers/net/ethernet/lantiq_xrx200.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/ethernet/lantiq_xrx200.c b/drivers/net/ethernet/lantiq_xrx200.c
index 5edb68a8aab1..6a83a6c19484 100644
--- a/drivers/net/ethernet/lantiq_xrx200.c
+++ b/drivers/net/ethernet/lantiq_xrx200.c
@@ -239,6 +239,13 @@ static int xrx200_hw_receive(struct xrx200_chan *ch)
 	}
 
 	skb = build_skb(buf, priv->rx_skb_size);
+	if (!skb) {
+		skb_free_frag(buf);
+		net_dev->stats.rx_dropped++;
+		netdev_err(net_dev, "failed to build skb\n");
+		return -ENOMEM;
+	}
+
 	skb_reserve(skb, NET_SKB_PAD);
 	skb_put(skb, len);
 
-- 
2.30.2


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

* [PATCH net v2 2/3] net: lantiq_xrx200: fix lock under memory pressure
  2022-08-15 14:57 [PATCH net v2 0/3] net: lantiq_xrx200: fix errors under memory pressure Aleksander Jan Bajkowski
  2022-08-15 14:57 ` [PATCH net v2 1/3] net: lantiq_xrx200: confirm skb is allocated before using Aleksander Jan Bajkowski
@ 2022-08-15 14:57 ` Aleksander Jan Bajkowski
  2022-08-15 14:57 ` [PATCH net v2 3/3] net: lantiq_xrx200: restore buffer if memory allocation failed Aleksander Jan Bajkowski
  2 siblings, 0 replies; 5+ messages in thread
From: Aleksander Jan Bajkowski @ 2022-08-15 14:57 UTC (permalink / raw)
  To: hauke, davem, edumazet, kuba, pabeni, olek2, netdev, linux-kernel

When the xrx200_hw_receive() function returns -ENOMEM, the NAPI poll
function immediately returns an error.
This is incorrect for two reasons:
* the function terminates without enabling interrupts or scheduling NAPI,
* the error code (-ENOMEM) is returned instead of the number of received
packets.

After the first memory allocation failure occurs, packet reception is
locked due to disabled interrupts from DMA..

Fixes: fe1a56420cf2 ("net: lantiq: Add Lantiq / Intel VRX200 Ethernet driver")
Signed-off-by: Aleksander Jan Bajkowski <olek2@wp.pl>
---
 drivers/net/ethernet/lantiq_xrx200.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/lantiq_xrx200.c b/drivers/net/ethernet/lantiq_xrx200.c
index 6a83a6c19484..8f9155eacdb3 100644
--- a/drivers/net/ethernet/lantiq_xrx200.c
+++ b/drivers/net/ethernet/lantiq_xrx200.c
@@ -295,7 +295,7 @@ static int xrx200_poll_rx(struct napi_struct *napi, int budget)
 			if (ret == XRX200_DMA_PACKET_IN_PROGRESS)
 				continue;
 			if (ret != XRX200_DMA_PACKET_COMPLETE)
-				return ret;
+				break;
 			rx++;
 		} else {
 			break;
-- 
2.30.2


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

* [PATCH net v2 3/3] net: lantiq_xrx200: restore buffer if memory allocation failed
  2022-08-15 14:57 [PATCH net v2 0/3] net: lantiq_xrx200: fix errors under memory pressure Aleksander Jan Bajkowski
  2022-08-15 14:57 ` [PATCH net v2 1/3] net: lantiq_xrx200: confirm skb is allocated before using Aleksander Jan Bajkowski
  2022-08-15 14:57 ` [PATCH net v2 2/3] net: lantiq_xrx200: fix lock under memory pressure Aleksander Jan Bajkowski
@ 2022-08-15 14:57 ` Aleksander Jan Bajkowski
  2 siblings, 0 replies; 5+ messages in thread
From: Aleksander Jan Bajkowski @ 2022-08-15 14:57 UTC (permalink / raw)
  To: hauke, davem, edumazet, kuba, pabeni, olek2, netdev, linux-kernel

In a situation where memory allocation fails, an invalid buffer address
is stored. When this descriptor is used again, the system panics in the
build_skb() function when accessing memory.

Fixes: 7ea6cd16f159 ("lantiq: net: fix duplicated skb in rx descriptor ring")
Signed-off-by: Aleksander Jan Bajkowski <olek2@wp.pl>
---
 drivers/net/ethernet/lantiq_xrx200.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/lantiq_xrx200.c b/drivers/net/ethernet/lantiq_xrx200.c
index 8f9155eacdb3..e817bc545d81 100644
--- a/drivers/net/ethernet/lantiq_xrx200.c
+++ b/drivers/net/ethernet/lantiq_xrx200.c
@@ -193,6 +193,7 @@ static int xrx200_alloc_buf(struct xrx200_chan *ch, void *(*alloc)(unsigned int
 
 	ch->rx_buff[ch->dma.desc] = alloc(priv->rx_skb_size);
 	if (!ch->rx_buff[ch->dma.desc]) {
+		ch->rx_buff[ch->dma.desc] = buf;
 		ret = -ENOMEM;
 		goto skip;
 	}
-- 
2.30.2


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

* Re: [PATCH net v2 1/3] net: lantiq_xrx200: confirm skb is allocated before using
  2022-08-15 14:57 ` [PATCH net v2 1/3] net: lantiq_xrx200: confirm skb is allocated before using Aleksander Jan Bajkowski
@ 2022-08-17  3:41   ` Jakub Kicinski
  0 siblings, 0 replies; 5+ messages in thread
From: Jakub Kicinski @ 2022-08-17  3:41 UTC (permalink / raw)
  To: Aleksander Jan Bajkowski
  Cc: hauke, davem, edumazet, pabeni, netdev, linux-kernel

On Mon, 15 Aug 2022 16:57:38 +0200 Aleksander Jan Bajkowski wrote:
> +		netdev_err(net_dev, "failed to build skb\n");

I don't thinkg this is needed, is build_skb() using GFP_NOWARN?
Otherwise there will be an OOM splat anyway. Do check, I'm not sure.

If there won't be an OOM splat this line should be rate limited.

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

end of thread, other threads:[~2022-08-17  3:42 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-15 14:57 [PATCH net v2 0/3] net: lantiq_xrx200: fix errors under memory pressure Aleksander Jan Bajkowski
2022-08-15 14:57 ` [PATCH net v2 1/3] net: lantiq_xrx200: confirm skb is allocated before using Aleksander Jan Bajkowski
2022-08-17  3:41   ` Jakub Kicinski
2022-08-15 14:57 ` [PATCH net v2 2/3] net: lantiq_xrx200: fix lock under memory pressure Aleksander Jan Bajkowski
2022-08-15 14:57 ` [PATCH net v2 3/3] net: lantiq_xrx200: restore buffer if memory allocation failed Aleksander Jan Bajkowski

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