Most users use __skb_frag_set_page()/skb_frag_off_set()/
skb_frag_size_set() to fill the page desc for a skb frag.
It does not make much sense to calling __skb_frag_set_page()
without calling skb_frag_off_set(), as the offset may depend
on whether the page is head page or tail page, so add
skb_frag_fill_page_desc() to fill the page desc for a skb
frag.
In the future, we can make sure the page in the frag is
head page of compound page or a base page, if not, we
may warn about that and convert the tail page to head
page and update the offset accordingly, if we see a warning
about that, we also fix the caller to fill the head page
in the frag. when the fixing is done, we may remove the
warning and converting.
In this way, we can remove the compound_head() or use
page_ref_*() like the below case:
https://elixir.bootlin.com/linux/latest/source/net/core/page_pool.c#L881
https://elixir.bootlin.com/linux/latest/source/include/linux/skbuff.h#L3383
It may also convert net stack to use the folio easier.
V1: repost with all the ack/review tags included.
RFC: remove a local variable as pointed out by Simon.
Yunsheng Lin (2):
net: introduce and use skb_frag_fill_page_desc()
net: remove __skb_frag_set_page()
.../net/ethernet/aquantia/atlantic/aq_ring.c | 6 +--
drivers/net/ethernet/broadcom/bnx2.c | 1 -
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 10 ++---
drivers/net/ethernet/chelsio/cxgb3/sge.c | 5 +--
drivers/net/ethernet/emulex/benet/be_main.c | 32 ++++++++-------
drivers/net/ethernet/freescale/enetc/enetc.c | 5 +--
.../net/ethernet/fungible/funeth/funeth_rx.c | 5 +--
drivers/net/ethernet/marvell/mvneta.c | 5 +--
.../net/ethernet/mellanox/mlx5/core/en_rx.c | 4 +-
drivers/net/ethernet/sun/cassini.c | 8 +---
drivers/net/virtio_net.c | 4 +-
drivers/net/vmxnet3/vmxnet3_drv.c | 4 +-
drivers/net/xen-netback/netback.c | 4 +-
include/linux/skbuff.h | 39 +++++--------------
net/bpf/test_run.c | 3 +-
net/core/gro.c | 4 +-
net/core/pktgen.c | 13 ++++---
net/core/skbuff.c | 7 ++--
net/tls/tls_device.c | 10 ++---
net/xfrm/xfrm_ipcomp.c | 5 +--
20 files changed, 65 insertions(+), 109 deletions(-)
--
2.33.0
The remaining users calling __skb_frag_set_page() with
page being NULL seems to be doing defensive programming,
as shinfo->nr_frags is already decremented, so remove
them.
Signed-off-by: Yunsheng Lin <[email protected]>
Reviewed-by: Leon Romanovsky <[email protected]>
Reviewed-by: Michael Chan <[email protected]>
Reviewed-by: Jesse Brandeburg <[email protected]>
Reviewed-by: Simon Horman <[email protected]>
---
RFC: remove a local variable as pointed out by Simon.
---
drivers/net/ethernet/broadcom/bnx2.c | 1 -
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 5 +----
include/linux/skbuff.h | 12 ------------
3 files changed, 1 insertion(+), 17 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnx2.c b/drivers/net/ethernet/broadcom/bnx2.c
index 466e1d62bcf6..0d917a9699c5 100644
--- a/drivers/net/ethernet/broadcom/bnx2.c
+++ b/drivers/net/ethernet/broadcom/bnx2.c
@@ -2955,7 +2955,6 @@ bnx2_reuse_rx_skb_pages(struct bnx2 *bp, struct bnx2_rx_ring_info *rxr,
shinfo = skb_shinfo(skb);
shinfo->nr_frags--;
page = skb_frag_page(&shinfo->frags[shinfo->nr_frags]);
- __skb_frag_set_page(&shinfo->frags[shinfo->nr_frags], NULL);
cons_rx_pg->page = page;
dev_kfree_skb(skb);
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index efaff5018af8..f42e51bd3e42 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -1102,10 +1102,7 @@ static u32 __bnxt_rx_agg_pages(struct bnxt *bp,
xdp_buff_set_frag_pfmemalloc(xdp);
if (bnxt_alloc_rx_page(bp, rxr, prod, GFP_ATOMIC) != 0) {
- unsigned int nr_frags;
-
- nr_frags = --shinfo->nr_frags;
- __skb_frag_set_page(&shinfo->frags[nr_frags], NULL);
+ --shinfo->nr_frags;
cons_rx_buf->page = page;
/* Update prod since possibly some pages have been
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 30be21c7d05f..00e8c435fa1a 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -3491,18 +3491,6 @@ static inline void skb_frag_page_copy(skb_frag_t *fragto,
fragto->bv_page = fragfrom->bv_page;
}
-/**
- * __skb_frag_set_page - sets the page contained in a paged fragment
- * @frag: the paged fragment
- * @page: the page to set
- *
- * Sets the fragment @frag to contain @page.
- */
-static inline void __skb_frag_set_page(skb_frag_t *frag, struct page *page)
-{
- frag->bv_page = page;
-}
-
bool skb_page_frag_refill(unsigned int sz, struct page_frag *pfrag, gfp_t prio);
/**
--
2.33.0
Hello:
This series was applied to netdev/net-next.git (main)
by David S. Miller <[email protected]>:
On Thu, 11 May 2023 09:12:11 +0800 you wrote:
> Most users use __skb_frag_set_page()/skb_frag_off_set()/
> skb_frag_size_set() to fill the page desc for a skb frag.
> It does not make much sense to calling __skb_frag_set_page()
> without calling skb_frag_off_set(), as the offset may depend
> on whether the page is head page or tail page, so add
> skb_frag_fill_page_desc() to fill the page desc for a skb
> frag.
>
> [...]
Here is the summary with links:
- [net-next,v2,1/2] net: introduce and use skb_frag_fill_page_desc()
https://git.kernel.org/netdev/net-next/c/b51f4113ebb0
- [net-next,v2,2/2] net: remove __skb_frag_set_page()
https://git.kernel.org/netdev/net-next/c/278fda0d52f6
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html