* [PATCH net-next v2 0/3] Minor cleanups to skb frag ref/unref
@ 2024-03-27 21:45 Mina Almasry
2024-03-27 21:45 ` [PATCH net-next v2 1/3] net: make napi_frag_unref reuse skb_page_unref Mina Almasry
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Mina Almasry @ 2024-03-27 21:45 UTC (permalink / raw)
To: netdev, linux-kernel, linux-rdma
Cc: Mina Almasry, Ayush Sawal, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Mirko Lindner, Stephen Hemminger,
Tariq Toukan, Steffen Klassert, Herbert Xu, David Ahern,
Boris Pismenny, John Fastabend, Dragos Tatulea
v2:
- Removed RFC tag.
- Rebased on net-next after the merge window opening.
- Added 1 patch at the beginning, "net: make napi_frag_unref reuse
skb_page_unref" because a recent patch introduced some code
duplication that can also be improved.
- Addressed feedback from Dragos & Yunsheng.
- Added Dragos's Reviewed-by.
This series is largely motivated by a recent discussion where there was
some confusion on how to properly ref/unref pp pages vs non pp pages:
https://lore.kernel.org/netdev/CAHS8izOoO-EovwMwAm9tLYetwikNPxC0FKyVGu1TPJWSz4bGoA@mail.gmail.com/T/#t
There is some subtely there because pp uses page->pp_ref_count for
refcounting, while non-pp uses get_page()/put_page() for ref counting.
Getting the refcounting pairs wrong can lead to kernel crash.
Additionally currently it may not be obvious to skb users unaware of
page pool internals how to properly acquire a ref on a pp frag. It
requires checking of skb->pp_recycle & is_pp_page() to make the correct
calls and may require some handling at the call site aware of arguable pp
internals.
This series is a minor refactor with a couple of goals:
1. skb users should be able to ref/unref a frag using
[__]skb_frag_[un]ref() functions without needing to understand pp
concepts and pp_ref_count vs get/put_page() differences.
2. reference counting functions should have a mirror opposite. I.e. there
should be a foo_unref() to every foo_ref() with a mirror opposite
implementation (as much as possible).
This is RFC to collect feedback if this change is desirable, but also so
that I don't race with the fix for the issue Dragos is seeing for his
crash.
https://lore.kernel.org/lkml/CAHS8izN436pn3SndrzsCyhmqvJHLyxgCeDpWXA4r1ANt3RCDLQ@mail.gmail.com/T/
Cc: Dragos Tatulea <dtatulea@nvidia.com>
Mina Almasry (3):
net: make napi_frag_unref reuse skb_page_unref
net: mirror skb frag ref/unref helpers
net: remove napi_frag_unref
.../chelsio/inline_crypto/ch_ktls/chcr_ktls.c | 2 +-
drivers/net/ethernet/marvell/sky2.c | 2 +-
drivers/net/ethernet/mellanox/mlx4/en_rx.c | 2 +-
drivers/net/ethernet/sun/cassini.c | 4 +-
include/linux/skbuff.h | 44 +++++++-------
net/core/skbuff.c | 58 ++++++-------------
net/ipv4/esp4.c | 2 +-
net/ipv6/esp6.c | 2 +-
net/tls/tls_device.c | 2 +-
net/tls/tls_strp.c | 2 +-
10 files changed, 52 insertions(+), 68 deletions(-)
--
2.44.0.396.g6e790dbe36-goog
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH net-next v2 1/3] net: make napi_frag_unref reuse skb_page_unref
2024-03-27 21:45 [PATCH net-next v2 0/3] Minor cleanups to skb frag ref/unref Mina Almasry
@ 2024-03-27 21:45 ` Mina Almasry
2024-03-27 21:45 ` [PATCH net-next v2 2/3] net: mirror skb frag ref/unref helpers Mina Almasry
2024-03-27 21:45 ` [PATCH net-next v2 3/3] net: remove napi_frag_unref Mina Almasry
2 siblings, 0 replies; 6+ messages in thread
From: Mina Almasry @ 2024-03-27 21:45 UTC (permalink / raw)
To: netdev, linux-kernel, linux-rdma
Cc: Mina Almasry, Ayush Sawal, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Mirko Lindner, Stephen Hemminger,
Tariq Toukan, Steffen Klassert, Herbert Xu, David Ahern,
Boris Pismenny, John Fastabend, Dragos Tatulea
The implementations of these 2 functions are almost identical. Remove
the implementation of napi_frag_unref, and make it a call into
skb_page_unref so we don't duplicate the implementation.
Signed-off-by: Mina Almasry <almasrymina@google.com>
---
include/linux/skbuff.h | 12 +++---------
net/ipv4/esp4.c | 2 +-
net/ipv6/esp6.c | 2 +-
3 files changed, 5 insertions(+), 11 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index b945af8a6208..bafa5c9ff59a 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -3524,10 +3524,10 @@ int skb_cow_data_for_xdp(struct page_pool *pool, struct sk_buff **pskb,
bool napi_pp_put_page(struct page *page, bool napi_safe);
static inline void
-skb_page_unref(const struct sk_buff *skb, struct page *page, bool napi_safe)
+skb_page_unref(struct page *page, bool recycle, bool napi_safe)
{
#ifdef CONFIG_PAGE_POOL
- if (skb->pp_recycle && napi_pp_put_page(page, napi_safe))
+ if (recycle && napi_pp_put_page(page, napi_safe))
return;
#endif
put_page(page);
@@ -3536,13 +3536,7 @@ skb_page_unref(const struct sk_buff *skb, struct page *page, bool napi_safe)
static inline void
napi_frag_unref(skb_frag_t *frag, bool recycle, bool napi_safe)
{
- struct page *page = skb_frag_page(frag);
-
-#ifdef CONFIG_PAGE_POOL
- if (recycle && napi_pp_put_page(page, napi_safe))
- return;
-#endif
- put_page(page);
+ skb_page_unref(skb_frag_page(frag), recycle, napi_safe);
}
/**
diff --git a/net/ipv4/esp4.c b/net/ipv4/esp4.c
index d33d12421814..3d2c252c5570 100644
--- a/net/ipv4/esp4.c
+++ b/net/ipv4/esp4.c
@@ -114,7 +114,7 @@ static void esp_ssg_unref(struct xfrm_state *x, void *tmp, struct sk_buff *skb)
*/
if (req->src != req->dst)
for (sg = sg_next(req->src); sg; sg = sg_next(sg))
- skb_page_unref(skb, sg_page(sg), false);
+ skb_page_unref(sg_page(sg), skb->pp_recycle, false);
}
#ifdef CONFIG_INET_ESPINTCP
diff --git a/net/ipv6/esp6.c b/net/ipv6/esp6.c
index 7371886d4f9f..4fe4f97f5420 100644
--- a/net/ipv6/esp6.c
+++ b/net/ipv6/esp6.c
@@ -131,7 +131,7 @@ static void esp_ssg_unref(struct xfrm_state *x, void *tmp, struct sk_buff *skb)
*/
if (req->src != req->dst)
for (sg = sg_next(req->src); sg; sg = sg_next(sg))
- skb_page_unref(skb, sg_page(sg), false);
+ skb_page_unref(sg_page(sg), skb->pp_recycle, false);
}
#ifdef CONFIG_INET6_ESPINTCP
--
2.44.0.396.g6e790dbe36-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH net-next v2 2/3] net: mirror skb frag ref/unref helpers
2024-03-27 21:45 [PATCH net-next v2 0/3] Minor cleanups to skb frag ref/unref Mina Almasry
2024-03-27 21:45 ` [PATCH net-next v2 1/3] net: make napi_frag_unref reuse skb_page_unref Mina Almasry
@ 2024-03-27 21:45 ` Mina Almasry
2024-03-28 17:07 ` kernel test robot
2024-03-28 17:18 ` kernel test robot
2024-03-27 21:45 ` [PATCH net-next v2 3/3] net: remove napi_frag_unref Mina Almasry
2 siblings, 2 replies; 6+ messages in thread
From: Mina Almasry @ 2024-03-27 21:45 UTC (permalink / raw)
To: netdev, linux-kernel, linux-rdma
Cc: Mina Almasry, Ayush Sawal, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Mirko Lindner, Stephen Hemminger,
Tariq Toukan, Steffen Klassert, Herbert Xu, David Ahern,
Boris Pismenny, John Fastabend, Dragos Tatulea
Refactor some of the skb frag ref/unref helpers for improved clarity.
Implement napi_pp_get_page() to be the mirror counterpart of
napi_pp_put_page().
Implement skb_page_ref() to be the mirror of skb_page_unref().
Improve __skb_frag_ref() to become a mirror counterpart of
__skb_frag_unref(). Previously unref could handle pp & non-pp pages,
while the ref could only handle non-pp pages. Now both the ref & unref
helpers can correctly handle both pp & non-pp pages.
Now that __skb_frag_ref() can handle both pp & non-pp pages, remove
skb_pp_frag_ref(), and use __skb_frag_ref() instead. This lets us
remove pp specific handling from skb_try_coalesce.
Signed-off-by: Mina Almasry <almasrymina@google.com>
Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>
---
.../chelsio/inline_crypto/ch_ktls/chcr_ktls.c | 2 +-
drivers/net/ethernet/sun/cassini.c | 4 +-
include/linux/skbuff.h | 22 ++++++--
net/core/skbuff.c | 54 ++++++-------------
4 files changed, 38 insertions(+), 44 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c b/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c
index 6482728794dd..f9b0a9533985 100644
--- a/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c
+++ b/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c
@@ -1658,7 +1658,7 @@ static void chcr_ktls_copy_record_in_skb(struct sk_buff *nskb,
for (i = 0; i < record->num_frags; i++) {
skb_shinfo(nskb)->frags[i] = record->frags[i];
/* increase the frag ref count */
- __skb_frag_ref(&skb_shinfo(nskb)->frags[i]);
+ __skb_frag_ref(&skb_shinfo(nskb)->frags[i], false);
}
skb_shinfo(nskb)->nr_frags = record->num_frags;
diff --git a/drivers/net/ethernet/sun/cassini.c b/drivers/net/ethernet/sun/cassini.c
index bfb903506367..fabba729e1b8 100644
--- a/drivers/net/ethernet/sun/cassini.c
+++ b/drivers/net/ethernet/sun/cassini.c
@@ -1999,7 +1999,7 @@ static int cas_rx_process_pkt(struct cas *cp, struct cas_rx_comp *rxc,
skb->len += hlen - swivel;
skb_frag_fill_page_desc(frag, page->buffer, off, hlen - swivel);
- __skb_frag_ref(frag);
+ __skb_frag_ref(frag, false);
/* any more data? */
if ((words[0] & RX_COMP1_SPLIT_PKT) && ((dlen -= hlen) > 0)) {
@@ -2023,7 +2023,7 @@ static int cas_rx_process_pkt(struct cas *cp, struct cas_rx_comp *rxc,
frag++;
skb_frag_fill_page_desc(frag, page->buffer, 0, hlen);
- __skb_frag_ref(frag);
+ __skb_frag_ref(frag, false);
RX_USED_ADD(page, hlen + cp->crc_size);
}
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index bafa5c9ff59a..058d72a2a250 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -3494,15 +3494,29 @@ static inline struct page *skb_frag_page(const skb_frag_t *frag)
return netmem_to_page(frag->netmem);
}
+bool napi_pp_get_page(struct page *page);
+
+static inline void skb_page_ref(struct page *page, bool recycle)
+{
+#ifdef CONFIG_PAGE_POOL
+ if (recycle && napi_pp_get_page(page))
+ return;
+#endif
+ get_page(page);
+}
+
/**
* __skb_frag_ref - take an addition reference on a paged fragment.
* @frag: the paged fragment
+ * @recycle: skb->pp_recycle param of the parent skb. False if no parent skb.
*
- * Takes an additional reference on the paged fragment @frag.
+ * Takes an additional reference on the paged fragment @frag. Obtains the
+ * correct reference count depending on whether skb->pp_recycle is set and
+ * whether the frag is a page pool frag.
*/
-static inline void __skb_frag_ref(skb_frag_t *frag)
+static inline void __skb_frag_ref(skb_frag_t *frag, bool recycle)
{
- get_page(skb_frag_page(frag));
+ skb_page_ref(skb_frag_page(frag), recycle);
}
/**
@@ -3514,7 +3528,7 @@ static inline void __skb_frag_ref(skb_frag_t *frag)
*/
static inline void skb_frag_ref(struct sk_buff *skb, int f)
{
- __skb_frag_ref(&skb_shinfo(skb)->frags[f]);
+ __skb_frag_ref(&skb_shinfo(skb)->frags[f], skb->pp_recycle);
}
int skb_pp_cow_data(struct page_pool *pool, struct sk_buff **pskb,
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 17617c29be2d..5c86ecaceb6c 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -1005,6 +1005,19 @@ int skb_cow_data_for_xdp(struct page_pool *pool, struct sk_buff **pskb,
EXPORT_SYMBOL(skb_cow_data_for_xdp);
#if IS_ENABLED(CONFIG_PAGE_POOL)
+bool napi_pp_get_page(struct page *page)
+{
+
+ page = compound_head(page);
+
+ if (!is_pp_page(page))
+ return false;
+
+ page_pool_ref_page(head_page);
+ return true;
+}
+EXPORT_SYMBOL(napi_pp_get_page);
+
bool napi_pp_put_page(struct page *page, bool napi_safe)
{
bool allow_direct = false;
@@ -1057,37 +1070,6 @@ static bool skb_pp_recycle(struct sk_buff *skb, void *data, bool napi_safe)
return napi_pp_put_page(virt_to_page(data), napi_safe);
}
-/**
- * skb_pp_frag_ref() - Increase fragment references of a page pool aware skb
- * @skb: page pool aware skb
- *
- * Increase the fragment reference count (pp_ref_count) of a skb. This is
- * intended to gain fragment references only for page pool aware skbs,
- * i.e. when skb->pp_recycle is true, and not for fragments in a
- * non-pp-recycling skb. It has a fallback to increase references on normal
- * pages, as page pool aware skbs may also have normal page fragments.
- */
-static int skb_pp_frag_ref(struct sk_buff *skb)
-{
- struct skb_shared_info *shinfo;
- struct page *head_page;
- int i;
-
- if (!skb->pp_recycle)
- return -EINVAL;
-
- shinfo = skb_shinfo(skb);
-
- for (i = 0; i < shinfo->nr_frags; i++) {
- head_page = compound_head(skb_frag_page(&shinfo->frags[i]));
- if (likely(is_pp_page(head_page)))
- page_pool_ref_page(head_page);
- else
- page_ref_inc(head_page);
- }
- return 0;
-}
-
static void skb_kfree_head(void *head, unsigned int end_offset)
{
if (end_offset == SKB_SMALL_HEAD_HEADROOM)
@@ -4196,7 +4178,7 @@ int skb_shift(struct sk_buff *tgt, struct sk_buff *skb, int shiftlen)
to++;
} else {
- __skb_frag_ref(fragfrom);
+ __skb_frag_ref(fragfrom, skb->pp_recycle);
skb_frag_page_copy(fragto, fragfrom);
skb_frag_off_copy(fragto, fragfrom);
skb_frag_size_set(fragto, todo);
@@ -4846,7 +4828,7 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
}
*nskb_frag = (i < 0) ? skb_head_frag_to_page_desc(frag_skb) : *frag;
- __skb_frag_ref(nskb_frag);
+ __skb_frag_ref(nskb_frag, nskb->pp_recycle);
size = skb_frag_size(nskb_frag);
if (pos < offset) {
@@ -5977,10 +5959,8 @@ bool skb_try_coalesce(struct sk_buff *to, struct sk_buff *from,
/* if the skb is not cloned this does nothing
* since we set nr_frags to 0.
*/
- if (skb_pp_frag_ref(from)) {
- for (i = 0; i < from_shinfo->nr_frags; i++)
- __skb_frag_ref(&from_shinfo->frags[i]);
- }
+ for (i = 0; i < from_shinfo->nr_frags; i++)
+ __skb_frag_ref(&from_shinfo->frags[i], from->pp_recycle);
to->truesize += delta;
to->len += len;
--
2.44.0.396.g6e790dbe36-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH net-next v2 3/3] net: remove napi_frag_unref
2024-03-27 21:45 [PATCH net-next v2 0/3] Minor cleanups to skb frag ref/unref Mina Almasry
2024-03-27 21:45 ` [PATCH net-next v2 1/3] net: make napi_frag_unref reuse skb_page_unref Mina Almasry
2024-03-27 21:45 ` [PATCH net-next v2 2/3] net: mirror skb frag ref/unref helpers Mina Almasry
@ 2024-03-27 21:45 ` Mina Almasry
2 siblings, 0 replies; 6+ messages in thread
From: Mina Almasry @ 2024-03-27 21:45 UTC (permalink / raw)
To: netdev, linux-kernel, linux-rdma
Cc: Mina Almasry, Ayush Sawal, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Mirko Lindner, Stephen Hemminger,
Tariq Toukan, Steffen Klassert, Herbert Xu, David Ahern,
Boris Pismenny, John Fastabend, Dragos Tatulea
With the changes in the last patches, napi_frag_unref() is now
reduandant. Remove it and use skb_page_unref directly.
Signed-off-by: Mina Almasry <almasrymina@google.com>
Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>
---
drivers/net/ethernet/marvell/sky2.c | 2 +-
drivers/net/ethernet/mellanox/mlx4/en_rx.c | 2 +-
include/linux/skbuff.h | 14 +++++---------
net/core/skbuff.c | 4 ++--
net/tls/tls_device.c | 2 +-
net/tls/tls_strp.c | 2 +-
6 files changed, 11 insertions(+), 15 deletions(-)
diff --git a/drivers/net/ethernet/marvell/sky2.c b/drivers/net/ethernet/marvell/sky2.c
index 07720841a8d7..8e00a5856856 100644
--- a/drivers/net/ethernet/marvell/sky2.c
+++ b/drivers/net/ethernet/marvell/sky2.c
@@ -2501,7 +2501,7 @@ static void skb_put_frags(struct sk_buff *skb, unsigned int hdr_space,
if (length == 0) {
/* don't need this page */
- __skb_frag_unref(frag, false);
+ __skb_frag_unref(frag, false, false);
--skb_shinfo(skb)->nr_frags;
} else {
size = min(length, (unsigned) PAGE_SIZE);
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_rx.c b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
index eac49657bd07..4dbf29b46979 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_rx.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
@@ -526,7 +526,7 @@ static int mlx4_en_complete_rx_desc(struct mlx4_en_priv *priv,
fail:
while (nr > 0) {
nr--;
- __skb_frag_unref(skb_shinfo(skb)->frags + nr, false);
+ __skb_frag_unref(skb_shinfo(skb)->frags + nr, false, false);
}
return 0;
}
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 058d72a2a250..c3edb4a3450a 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -3547,23 +3547,19 @@ skb_page_unref(struct page *page, bool recycle, bool napi_safe)
put_page(page);
}
-static inline void
-napi_frag_unref(skb_frag_t *frag, bool recycle, bool napi_safe)
-{
- skb_page_unref(skb_frag_page(frag), recycle, napi_safe);
-}
-
/**
* __skb_frag_unref - release a reference on a paged fragment.
* @frag: the paged fragment
* @recycle: recycle the page if allocated via page_pool
+ * @napi_safe: set to true if running in the same napi context as where the
+ * consumer would run.
*
* Releases a reference on the paged fragment @frag
* or recycles the page via the page_pool API.
*/
-static inline void __skb_frag_unref(skb_frag_t *frag, bool recycle)
+static inline void __skb_frag_unref(skb_frag_t *frag, bool recycle, bool napi_safe)
{
- napi_frag_unref(frag, recycle, false);
+ skb_page_unref(skb_frag_page(frag), recycle, napi_safe);
}
/**
@@ -3578,7 +3574,7 @@ static inline void skb_frag_unref(struct sk_buff *skb, int f)
struct skb_shared_info *shinfo = skb_shinfo(skb);
if (!skb_zcopy_managed(skb))
- __skb_frag_unref(&shinfo->frags[f], skb->pp_recycle);
+ __skb_frag_unref(&shinfo->frags[f], skb->pp_recycle, false);
}
/**
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 5c86ecaceb6c..a6dbba56e047 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -1109,7 +1109,7 @@ static void skb_release_data(struct sk_buff *skb, enum skb_drop_reason reason,
}
for (i = 0; i < shinfo->nr_frags; i++)
- napi_frag_unref(&shinfo->frags[i], skb->pp_recycle, napi_safe);
+ __skb_frag_unref(&shinfo->frags[i], skb->pp_recycle, napi_safe);
free_head:
if (shinfo->frag_list)
@@ -4200,7 +4200,7 @@ int skb_shift(struct sk_buff *tgt, struct sk_buff *skb, int shiftlen)
fragto = &skb_shinfo(tgt)->frags[merge];
skb_frag_size_add(fragto, skb_frag_size(fragfrom));
- __skb_frag_unref(fragfrom, skb->pp_recycle);
+ __skb_frag_unref(fragfrom, skb->pp_recycle, false);
}
/* Reposition in the original skb */
diff --git a/net/tls/tls_device.c b/net/tls/tls_device.c
index bf8ed36b1ad6..5dc6381f34fb 100644
--- a/net/tls/tls_device.c
+++ b/net/tls/tls_device.c
@@ -140,7 +140,7 @@ static void destroy_record(struct tls_record_info *record)
int i;
for (i = 0; i < record->num_frags; i++)
- __skb_frag_unref(&record->frags[i], false);
+ __skb_frag_unref(&record->frags[i], false, false);
kfree(record);
}
diff --git a/net/tls/tls_strp.c b/net/tls/tls_strp.c
index ca1e0e198ceb..85b41f226978 100644
--- a/net/tls/tls_strp.c
+++ b/net/tls/tls_strp.c
@@ -196,7 +196,7 @@ static void tls_strp_flush_anchor_copy(struct tls_strparser *strp)
DEBUG_NET_WARN_ON_ONCE(atomic_read(&shinfo->dataref) != 1);
for (i = 0; i < shinfo->nr_frags; i++)
- __skb_frag_unref(&shinfo->frags[i], false);
+ __skb_frag_unref(&shinfo->frags[i], false, false);
shinfo->nr_frags = 0;
if (strp->copy_mode) {
kfree_skb_list(shinfo->frag_list);
--
2.44.0.396.g6e790dbe36-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net-next v2 2/3] net: mirror skb frag ref/unref helpers
2024-03-27 21:45 ` [PATCH net-next v2 2/3] net: mirror skb frag ref/unref helpers Mina Almasry
@ 2024-03-28 17:07 ` kernel test robot
2024-03-28 17:18 ` kernel test robot
1 sibling, 0 replies; 6+ messages in thread
From: kernel test robot @ 2024-03-28 17:07 UTC (permalink / raw)
To: Mina Almasry, netdev, linux-kernel, linux-rdma
Cc: llvm, oe-kbuild-all, Mina Almasry, Ayush Sawal, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Mirko Lindner, Stephen Hemminger,
Tariq Toukan, Steffen Klassert, Herbert Xu, David Ahern,
Boris Pismenny, John Fastabend, Dragos Tatulea
Hi Mina,
kernel test robot noticed the following build errors:
[auto build test ERROR on net-next/main]
url: https://github.com/intel-lab-lkp/linux/commits/Mina-Almasry/net-make-napi_frag_unref-reuse-skb_page_unref/20240328-054816
base: net-next/main
patch link: https://lore.kernel.org/r/20240327214523.2182174-3-almasrymina%40google.com
patch subject: [PATCH net-next v2 2/3] net: mirror skb frag ref/unref helpers
config: x86_64-rhel-8.3-rust (https://download.01.org/0day-ci/archive/20240329/202403290006.WfusvToB-lkp@intel.com/config)
compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240329/202403290006.WfusvToB-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202403290006.WfusvToB-lkp@intel.com/
All errors (new ones prefixed by >>):
>> net/tls/tls_device_fallback.c:280:22: error: too few arguments to function call, expected 2, have 1
280 | __skb_frag_ref(frag);
| ~~~~~~~~~~~~~~ ^
include/linux/skbuff.h:3517:20: note: '__skb_frag_ref' declared here
3517 | static inline void __skb_frag_ref(skb_frag_t *frag, bool recycle)
| ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.
vim +280 net/tls/tls_device_fallback.c
e8f69799810c32 Ilya Lesokhin 2018-04-30 228
e8f69799810c32 Ilya Lesokhin 2018-04-30 229 /* This function may be called after the user socket is already
e8f69799810c32 Ilya Lesokhin 2018-04-30 230 * closed so make sure we don't use anything freed during
e8f69799810c32 Ilya Lesokhin 2018-04-30 231 * tls_sk_proto_close here
e8f69799810c32 Ilya Lesokhin 2018-04-30 232 */
e8f69799810c32 Ilya Lesokhin 2018-04-30 233
e8f69799810c32 Ilya Lesokhin 2018-04-30 234 static int fill_sg_in(struct scatterlist *sg_in,
e8f69799810c32 Ilya Lesokhin 2018-04-30 235 struct sk_buff *skb,
d80a1b9d186057 Boris Pismenny 2018-07-13 236 struct tls_offload_context_tx *ctx,
e8f69799810c32 Ilya Lesokhin 2018-04-30 237 u64 *rcd_sn,
e8f69799810c32 Ilya Lesokhin 2018-04-30 238 s32 *sync_size,
e8f69799810c32 Ilya Lesokhin 2018-04-30 239 int *resync_sgs)
e8f69799810c32 Ilya Lesokhin 2018-04-30 240 {
504148fedb8542 Eric Dumazet 2022-06-30 241 int tcp_payload_offset = skb_tcp_all_headers(skb);
e8f69799810c32 Ilya Lesokhin 2018-04-30 242 int payload_len = skb->len - tcp_payload_offset;
e8f69799810c32 Ilya Lesokhin 2018-04-30 243 u32 tcp_seq = ntohl(tcp_hdr(skb)->seq);
e8f69799810c32 Ilya Lesokhin 2018-04-30 244 struct tls_record_info *record;
e8f69799810c32 Ilya Lesokhin 2018-04-30 245 unsigned long flags;
e8f69799810c32 Ilya Lesokhin 2018-04-30 246 int remaining;
e8f69799810c32 Ilya Lesokhin 2018-04-30 247 int i;
e8f69799810c32 Ilya Lesokhin 2018-04-30 248
e8f69799810c32 Ilya Lesokhin 2018-04-30 249 spin_lock_irqsave(&ctx->lock, flags);
e8f69799810c32 Ilya Lesokhin 2018-04-30 250 record = tls_get_record(ctx, tcp_seq, rcd_sn);
e8f69799810c32 Ilya Lesokhin 2018-04-30 251 if (!record) {
e8f69799810c32 Ilya Lesokhin 2018-04-30 252 spin_unlock_irqrestore(&ctx->lock, flags);
e8f69799810c32 Ilya Lesokhin 2018-04-30 253 return -EINVAL;
e8f69799810c32 Ilya Lesokhin 2018-04-30 254 }
e8f69799810c32 Ilya Lesokhin 2018-04-30 255
e8f69799810c32 Ilya Lesokhin 2018-04-30 256 *sync_size = tcp_seq - tls_record_start_seq(record);
e8f69799810c32 Ilya Lesokhin 2018-04-30 257 if (*sync_size < 0) {
e8f69799810c32 Ilya Lesokhin 2018-04-30 258 int is_start_marker = tls_record_is_start_marker(record);
e8f69799810c32 Ilya Lesokhin 2018-04-30 259
e8f69799810c32 Ilya Lesokhin 2018-04-30 260 spin_unlock_irqrestore(&ctx->lock, flags);
e8f69799810c32 Ilya Lesokhin 2018-04-30 261 /* This should only occur if the relevant record was
e8f69799810c32 Ilya Lesokhin 2018-04-30 262 * already acked. In that case it should be ok
e8f69799810c32 Ilya Lesokhin 2018-04-30 263 * to drop the packet and avoid retransmission.
e8f69799810c32 Ilya Lesokhin 2018-04-30 264 *
e8f69799810c32 Ilya Lesokhin 2018-04-30 265 * There is a corner case where the packet contains
e8f69799810c32 Ilya Lesokhin 2018-04-30 266 * both an acked and a non-acked record.
e8f69799810c32 Ilya Lesokhin 2018-04-30 267 * We currently don't handle that case and rely
a0e128ef88e4a0 Yueh-Shun Li 2023-06-22 268 * on TCP to retransmit a packet that doesn't contain
e8f69799810c32 Ilya Lesokhin 2018-04-30 269 * already acked payload.
e8f69799810c32 Ilya Lesokhin 2018-04-30 270 */
e8f69799810c32 Ilya Lesokhin 2018-04-30 271 if (!is_start_marker)
e8f69799810c32 Ilya Lesokhin 2018-04-30 272 *sync_size = 0;
e8f69799810c32 Ilya Lesokhin 2018-04-30 273 return -EINVAL;
e8f69799810c32 Ilya Lesokhin 2018-04-30 274 }
e8f69799810c32 Ilya Lesokhin 2018-04-30 275
e8f69799810c32 Ilya Lesokhin 2018-04-30 276 remaining = *sync_size;
e8f69799810c32 Ilya Lesokhin 2018-04-30 277 for (i = 0; remaining > 0; i++) {
e8f69799810c32 Ilya Lesokhin 2018-04-30 278 skb_frag_t *frag = &record->frags[i];
e8f69799810c32 Ilya Lesokhin 2018-04-30 279
e8f69799810c32 Ilya Lesokhin 2018-04-30 @280 __skb_frag_ref(frag);
e8f69799810c32 Ilya Lesokhin 2018-04-30 281 sg_set_page(sg_in + i, skb_frag_page(frag),
b54c9d5bd6e38e Jonathan Lemon 2019-07-30 282 skb_frag_size(frag), skb_frag_off(frag));
e8f69799810c32 Ilya Lesokhin 2018-04-30 283
e8f69799810c32 Ilya Lesokhin 2018-04-30 284 remaining -= skb_frag_size(frag);
e8f69799810c32 Ilya Lesokhin 2018-04-30 285
e8f69799810c32 Ilya Lesokhin 2018-04-30 286 if (remaining < 0)
e8f69799810c32 Ilya Lesokhin 2018-04-30 287 sg_in[i].length += remaining;
e8f69799810c32 Ilya Lesokhin 2018-04-30 288 }
e8f69799810c32 Ilya Lesokhin 2018-04-30 289 *resync_sgs = i;
e8f69799810c32 Ilya Lesokhin 2018-04-30 290
e8f69799810c32 Ilya Lesokhin 2018-04-30 291 spin_unlock_irqrestore(&ctx->lock, flags);
e8f69799810c32 Ilya Lesokhin 2018-04-30 292 if (skb_to_sgvec(skb, &sg_in[i], tcp_payload_offset, payload_len) < 0)
e8f69799810c32 Ilya Lesokhin 2018-04-30 293 return -EINVAL;
e8f69799810c32 Ilya Lesokhin 2018-04-30 294
e8f69799810c32 Ilya Lesokhin 2018-04-30 295 return 0;
e8f69799810c32 Ilya Lesokhin 2018-04-30 296 }
e8f69799810c32 Ilya Lesokhin 2018-04-30 297
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next v2 2/3] net: mirror skb frag ref/unref helpers
2024-03-27 21:45 ` [PATCH net-next v2 2/3] net: mirror skb frag ref/unref helpers Mina Almasry
2024-03-28 17:07 ` kernel test robot
@ 2024-03-28 17:18 ` kernel test robot
1 sibling, 0 replies; 6+ messages in thread
From: kernel test robot @ 2024-03-28 17:18 UTC (permalink / raw)
To: Mina Almasry, netdev, linux-kernel, linux-rdma
Cc: llvm, oe-kbuild-all, Mina Almasry, Ayush Sawal, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Mirko Lindner, Stephen Hemminger,
Tariq Toukan, Steffen Klassert, Herbert Xu, David Ahern,
Boris Pismenny, John Fastabend, Dragos Tatulea
Hi Mina,
kernel test robot noticed the following build errors:
[auto build test ERROR on net-next/main]
url: https://github.com/intel-lab-lkp/linux/commits/Mina-Almasry/net-make-napi_frag_unref-reuse-skb_page_unref/20240328-054816
base: net-next/main
patch link: https://lore.kernel.org/r/20240327214523.2182174-3-almasrymina%40google.com
patch subject: [PATCH net-next v2 2/3] net: mirror skb frag ref/unref helpers
config: riscv-defconfig (https://download.01.org/0day-ci/archive/20240329/202403290103.BHENUIYW-lkp@intel.com/config)
compiler: clang version 19.0.0git (https://github.com/llvm/llvm-project 23de3862dce582ce91c1aa914467d982cb1a73b4)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240329/202403290103.BHENUIYW-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202403290103.BHENUIYW-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from net/core/skbuff.c:40:
In file included from include/linux/mm.h:2208:
include/linux/vmstat.h:522:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
522 | return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
| ~~~~~~~~~~~ ^ ~~~
>> net/core/skbuff.c:1016:21: error: use of undeclared identifier 'head_page'
1016 | page_pool_ref_page(head_page);
| ^
1 warning and 1 error generated.
vim +/head_page +1016 net/core/skbuff.c
1010
1011 page = compound_head(page);
1012
1013 if (!is_pp_page(page))
1014 return false;
1015
> 1016 page_pool_ref_page(head_page);
1017 return true;
1018 }
1019 EXPORT_SYMBOL(napi_pp_get_page);
1020
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-03-28 17:19 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-27 21:45 [PATCH net-next v2 0/3] Minor cleanups to skb frag ref/unref Mina Almasry
2024-03-27 21:45 ` [PATCH net-next v2 1/3] net: make napi_frag_unref reuse skb_page_unref Mina Almasry
2024-03-27 21:45 ` [PATCH net-next v2 2/3] net: mirror skb frag ref/unref helpers Mina Almasry
2024-03-28 17:07 ` kernel test robot
2024-03-28 17:18 ` kernel test robot
2024-03-27 21:45 ` [PATCH net-next v2 3/3] net: remove napi_frag_unref Mina Almasry
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.