| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <20240419222328.3231075-1-dwilder@us.ibm.com>
Date: Fri, 19 Apr 2024 15:23:28 -0700
From: David J Wilder <dwilder@...ibm.com>
To: netdev@...r.kernel.org
Subject: [RFC PATCH] net: skb: Increasing allocation in __napi_alloc_skb() to 2k when needed.
When testing CONFIG_MAX_SKB_FRAGS=45 on ppc64le and x86_64 I ran into a
couple of issues.
__napi_alloc_skb() assumes its smallest fragment allocations will fit in
1K. When CONFIG_MAX_SKB_FRAGS is increased this may no longer be true
resulting in __napi_alloc_skb() reverting to using page_frag_alloc().
This results in the return of the bug fixed in:
Commit 3226b158e67c ("net: avoid 32 x truesize under-estimation for
tiny skbs")
That commit insured that "small skb head fragments are kmalloc backed,
so that other objects in the slab page can be reused instead of being held
as long as skbs are sitting in socket queues."
On ppc64le the warning from napi_get_frags_check() is displayed when
CONFIG_MAX_SKB_FRAGS is set to 45. The purpose of the warning is to detect
when an increase of MAX_SKB_FRAGS has reintroduced the aforementioned bug.
Unfortunately on x86_64 this warning is not seen, even though it should be.
I found the warning was disabled by:
commit dbae2b062824 ("net: skb: introduce and use a single page frag
cache")
This RFC patch to __napi_alloc_skb() determines if an skbuff allocation
with a head fragment of size GRO_MAX_HEAD will fit in a 1k allocation,
increasing the allocation to 2k if needed.
I have functionally tested this patch, performance testing is still needed.
TBD: Remove the limitation on 4k page size from the single page frag cache
allowing ppc64le (64K page size) to benefit from this change.
TBD: I have not address the warning in napi_get_frags_check() on x86_64.
Will the warning still be needed once the other changes are completed?
Signed-off-by: David J Wilder <dwilder@...ibm.com>
---
net/core/skbuff.c | 34 +++++++++++++++++++---------------
1 file changed, 19 insertions(+), 15 deletions(-)
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index b99127712e67..e3b6115a2edc 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -232,18 +232,18 @@ static void skb_under_panic(struct sk_buff *skb, unsigned int sz, void *addr)
* page - to avoid excessive truesize underestimation
*/
-struct page_frag_1k {
+struct page_frag_small {
void *va;
u16 offset;
bool pfmemalloc;
};
-static void *page_frag_alloc_1k(struct page_frag_1k *nc, gfp_t gfp)
+static void *page_frag_alloc_small(struct page_frag_small *nc, gfp_t gfp, unsigned int len)
{
struct page *page;
int offset;
- offset = nc->offset - SZ_1K;
+ offset = nc->offset - len;
if (likely(offset >= 0))
goto use_frag;
@@ -253,8 +253,8 @@ static void *page_frag_alloc_1k(struct page_frag_1k *nc, gfp_t gfp)
nc->va = page_address(page);
nc->pfmemalloc = page_is_pfmemalloc(page);
- offset = PAGE_SIZE - SZ_1K;
- page_ref_add(page, offset / SZ_1K);
+ offset = PAGE_SIZE - len;
+ page_ref_add(page, offset / len);
use_frag:
nc->offset = offset;
@@ -268,10 +268,10 @@ static void *page_frag_alloc_1k(struct page_frag_1k *nc, gfp_t gfp)
#define NAPI_HAS_SMALL_PAGE_FRAG 0
#define NAPI_SMALL_PAGE_PFMEMALLOC(nc) false
-struct page_frag_1k {
+struct page_frag_small {
};
-static void *page_frag_alloc_1k(struct page_frag_1k *nc, gfp_t gfp_mask)
+static void *page_frag_alloc_small(struct page_frag_small *nc, gfp_t gfp_mask, unsigned int len)
{
return NULL;
}
@@ -280,7 +280,7 @@ static void *page_frag_alloc_1k(struct page_frag_1k *nc, gfp_t gfp_mask)
struct napi_alloc_cache {
struct page_frag_cache page;
- struct page_frag_1k page_small;
+ struct page_frag_small page_small;
unsigned int skb_count;
void *skb_cache[NAPI_SKB_CACHE_SIZE];
};
@@ -787,6 +787,7 @@ EXPORT_SYMBOL(__netdev_alloc_skb);
*
* %NULL is returned if there is no free memory.
*/
+
struct sk_buff *__napi_alloc_skb(struct napi_struct *napi, unsigned int len,
gfp_t gfp_mask)
{
@@ -794,6 +795,7 @@ struct sk_buff *__napi_alloc_skb(struct napi_struct *napi, unsigned int len,
struct sk_buff *skb;
bool pfmemalloc;
void *data;
+ unsigned int size = SZ_1K;
DEBUG_NET_WARN_ON_ONCE(!in_softirq());
len += NET_SKB_PAD + NET_IP_ALIGN;
@@ -801,9 +803,14 @@ struct sk_buff *__napi_alloc_skb(struct napi_struct *napi, unsigned int len,
/* If requested length is either too small or too big,
* we use kmalloc() for skb->head allocation.
* When the small frag allocator is available, prefer it over kmalloc
- * for small fragments
+ * for small fragments. Larger MAX_SKB_FRAGS values may require more
+ * than a 1K allocation found by testing SKB_WITH_OVERHEAD(GRO_MAX_HEAD).
*/
- if ((!NAPI_HAS_SMALL_PAGE_FRAG && len <= SKB_WITH_OVERHEAD(1024)) ||
+
+ if (SKB_WITH_OVERHEAD(1024) < (MAX_HEADER + 128 + NET_SKB_PAD + NET_IP_ALIGN))
+ size = SZ_2K;
+
+ if ((!NAPI_HAS_SMALL_PAGE_FRAG && len <= SKB_WITH_OVERHEAD(size)) ||
len > SKB_WITH_OVERHEAD(PAGE_SIZE) ||
(gfp_mask & (__GFP_DIRECT_RECLAIM | GFP_DMA))) {
skb = __alloc_skb(len, gfp_mask, SKB_ALLOC_RX | SKB_ALLOC_NAPI,
@@ -818,7 +825,7 @@ struct sk_buff *__napi_alloc_skb(struct napi_struct *napi, unsigned int len,
if (sk_memalloc_socks())
gfp_mask |= __GFP_MEMALLOC;
- if (NAPI_HAS_SMALL_PAGE_FRAG && len <= SKB_WITH_OVERHEAD(1024)) {
+ if (NAPI_HAS_SMALL_PAGE_FRAG && len <= SKB_WITH_OVERHEAD(size)) {
/* we are artificially inflating the allocation size, but
* that is not as bad as it may look like, as:
* - 'len' less than GRO_MAX_HEAD makes little sense
@@ -829,13 +836,10 @@ struct sk_buff *__napi_alloc_skb(struct napi_struct *napi, unsigned int len,
* little networking, as that implies no WiFi and no
* tunnels support, and 32 bits arches.
*/
- len = SZ_1K;
-
- data = page_frag_alloc_1k(&nc->page_small, gfp_mask);
+ data = page_frag_alloc_small(&nc->page_small, gfp_mask, size);
pfmemalloc = NAPI_SMALL_PAGE_PFMEMALLOC(nc->page_small);
} else {
len = SKB_HEAD_ALIGN(len);
-
data = page_frag_alloc(&nc->page, len, gfp_mask);
pfmemalloc = nc->page.pfmemalloc;
}
--
2.39.3
Powered by blists - more mailing lists