Skip to contentSkip to conversation

bnxt: make sure we return pages to the pool

PATCH Applied Jakub Kicinski 3 participants 3 messages Jan 11, 2023 – Jan 12, 2023

Subsystems (3)
lore
Share & tools
lore

Jakub Kicinski

author maintainer
#0 lore raw

PATCH 1/1 bnxt: make sure we return pages to the pool

Before the commit under Fixes the page would have been released from the pool before the napi_alloc_skb() call, so normal page freeing was fine (released page == no longer in the pool).

After the change we just mark the page for recycling so it's still in the pool if the skb alloc fails, we need to recycle.

Same commit added the same bug in the new bnxt_rx_multi_page_skb().

Fixes 1dc4c557bfed ("bnxt: adding bnxt_xdp_build_skb to build skb from multibuffer xdp_buff")
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/net/ethernet/broadcom/bnxt/bnxt.c +2 -2
index 16ce7a90610c..240a7e8a7652 100644
@@ -993,7 +993,7 @@ static struct sk_buff *bnxt_rx_multi_page_skb(struct bnxt *bp,
 			     DMA_ATTR_WEAK_ORDERING);
 	skb = build_skb(page_address(page), PAGE_SIZE);
 	if (!skb) {
-		__free_page(page);
+		page_pool_recycle_direct(rxr->page_pool, page);
 		return NULL;
 	}
 	skb_mark_for_recycle(skb);
@@ -1031,7 +1031,7 @@ static struct sk_buff *bnxt_rx_page_skb(struct bnxt *bp,
 
 	skb = napi_alloc_skb(&rxr->bnapi->napi, payload);
 	if (!skb) {
-		__free_page(page);
+		page_pool_recycle_direct(rxr->page_pool, page);
 		return NULL;
 	}
 
Signature · 1 line
2.38.1
Reviewed-by on v1 1/1
#1 lore raw
Quoted 8 lines · Jakub Kicinski

Before the commit under Fixes the page would have been released from the pool before the napi_alloc_skb() call, so normal page freeing was fine (released page == no longer in the pool).

After the change we just mark the page for recycling so it's still in the pool if the skb alloc fails, we need to recycle.

Same commit added the same bug in the new bnxt_rx_multi_page_skb().

Good catch, thank you.

Quoted 2 lines
Fixes 1dc4c557bfed ("bnxt: adding bnxt_xdp_build_skb to build skb from multibuffer xdp_buff")
Quoted 29 lines
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/net/ethernet/broadcom/bnxt/bnxt.c +2 -2
index 16ce7a90610c..240a7e8a7652 100644
@@ -993,7 +993,7 @@ static struct sk_buff *bnxt_rx_multi_page_skb(struct bnxt *bp,
 			     DMA_ATTR_WEAK_ORDERING);
 	skb = build_skb(page_address(page), PAGE_SIZE);
 	if (!skb) {
-		__free_page(page);
+		page_pool_recycle_direct(rxr->page_pool, page);
 		return NULL;
 	}
 	skb_mark_for_recycle(skb);
@@ -1031,7 +1031,7 @@ static struct sk_buff *bnxt_rx_page_skb(struct bnxt *bp,
 
 	skb = napi_alloc_skb(&rxr->bnapi->napi, payload);
 	if (!skb) {
-		__free_page(page);
+		page_pool_recycle_direct(rxr->page_pool, page);
 		return NULL;
 	}
 
Signature · 1 line
2.38.1
#2 lore raw
In reply to Jakub Kicinski
This patch was applied to netdev/net.git (master) by Jakub Kicinski <[email protected]>:

Hello:

This patch was applied to netdev/net.git (master)
by Jakub Kicinski <[email protected]>:

Quoted 8 lines · On Tue, 10 Jan 2023 20:25:47 -0800 you wrote:

Before the commit under Fixes the page would have been released from the pool before the napi_alloc_skb() call, so normal page freeing was fine (released page == no longer in the pool).

After the change we just mark the page for recycling so it's still in the pool if the skb alloc fails, we need to recycle.

[...]

Here is the summary with links:
- [net] bnxt: make sure we return pages to the pool

    https://git.kernel.org/netdev/net/c/97f5e03a4a27

You are awesome, thank you!

Signature · 2 lines
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html