Skip to content

Commit 8369fd5

Browse files
committed
skbuff: Optimization of SKB coalescing for page pool
JIRA: https://issues.redhat.com/browse/RHEL-91107 Conflicts: - using page_pool_page_is_pp() due to existing backport of cd3c931 ("page_pool: Move pp_magic check into helper functions") commit f7dc324 Author: Liang Chen <liangchen.linux@gmail.com> Date: Fri Dec 15 11:30:11 2023 +0800 skbuff: Optimization of SKB coalescing for page pool In order to address the issues encountered with commit 1effe8c ("skbuff: fix coalescing for page_pool fragment recycling"), the combination of the following condition was excluded from skb coalescing: from->pp_recycle = 1 from->cloned = 1 to->pp_recycle = 1 However, with page pool environments, the aforementioned combination can be quite common(ex. NetworkMananger may lead to the additional packet_type being registered, thus the cloning). In scenarios with a higher number of small packets, it can significantly affect the success rate of coalescing. For example, considering packets of 256 bytes size, our comparison of coalescing success rate is as follows: Without page pool: 70% With page pool: 13% Consequently, this has an impact on performance: Without page pool: 2.57 Gbits/sec With page pool: 2.26 Gbits/sec Therefore, it seems worthwhile to optimize this scenario and enable coalescing of this particular combination. To achieve this, we need to ensure the correct increment of the "from" SKB page's page pool reference count (pp_ref_count). Following this optimization, the success rate of coalescing measured in our environment has improved as follows: With page pool: 60% This success rate is approaching the rate achieved without using page pool, and the performance has also been improved: With page pool: 2.52 Gbits/sec Below is the performance comparison for small packets before and after this optimization. We observe no impact to packets larger than 4K. packet size before after improved (bytes) (Gbits/sec) (Gbits/sec) 128 1.19 1.27 7.13% 256 2.26 2.52 11.75% 512 4.13 4.81 16.50% 1024 6.17 6.73 9.05% 2048 14.54 15.47 6.45% 4096 25.44 27.87 9.52% Signed-off-by: Liang Chen <liangchen.linux@gmail.com> Reviewed-by: Yunsheng Lin <linyunsheng@huawei.com> Suggested-by: Jason Wang <jasowang@redhat.com> Reviewed-by: Mina Almasry <almasrymina@google.com> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Ivan Vecera <ivecera@redhat.com>
1 parent 8de2979 commit 8369fd5

File tree

2 files changed

+45
-12
lines changed

2 files changed

+45
-12
lines changed

include/net/page_pool/helpers.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,11 @@ static inline long page_pool_unref_page(struct page *page, long nr)
279279
return ret;
280280
}
281281

282+
static inline void page_pool_ref_page(struct page *page)
283+
{
284+
atomic_long_inc(&page->pp_ref_count);
285+
}
286+
282287
static inline bool page_pool_is_last_ref(struct page *page)
283288
{
284289
/* If page_pool_unref_page() returns 0, we were the last user */

net/core/skbuff.c

Lines changed: 40 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -877,6 +877,37 @@ static bool skb_pp_recycle(struct sk_buff *skb, void *data)
877877
return napi_pp_put_page(virt_to_page(data));
878878
}
879879

880+
/**
881+
* skb_pp_frag_ref() - Increase fragment references of a page pool aware skb
882+
* @skb: page pool aware skb
883+
*
884+
* Increase the fragment reference count (pp_ref_count) of a skb. This is
885+
* intended to gain fragment references only for page pool aware skbs,
886+
* i.e. when skb->pp_recycle is true, and not for fragments in a
887+
* non-pp-recycling skb. It has a fallback to increase references on normal
888+
* pages, as page pool aware skbs may also have normal page fragments.
889+
*/
890+
static int skb_pp_frag_ref(struct sk_buff *skb)
891+
{
892+
struct skb_shared_info *shinfo;
893+
struct page *head_page;
894+
int i;
895+
896+
if (!skb->pp_recycle)
897+
return -EINVAL;
898+
899+
shinfo = skb_shinfo(skb);
900+
901+
for (i = 0; i < shinfo->nr_frags; i++) {
902+
head_page = compound_head(skb_frag_page(&shinfo->frags[i]));
903+
if (likely(page_pool_page_is_pp(head_page)))
904+
page_pool_ref_page(head_page);
905+
else
906+
page_ref_inc(head_page);
907+
}
908+
return 0;
909+
}
910+
880911
static void skb_free_head(struct sk_buff *skb)
881912
{
882913
unsigned char *head = skb->head;
@@ -5654,17 +5685,12 @@ bool skb_try_coalesce(struct sk_buff *to, struct sk_buff *from,
56545685
return false;
56555686

56565687
/* In general, avoid mixing page_pool and non-page_pool allocated
5657-
* pages within the same SKB. Additionally avoid dealing with clones
5658-
* with page_pool pages, in case the SKB is using page_pool fragment
5659-
* references (page_pool_alloc_frag()). Since we only take full page
5660-
* references for cloned SKBs at the moment that would result in
5661-
* inconsistent reference counts.
5662-
* In theory we could take full references if @from is cloned and
5663-
* !@to->pp_recycle but its tricky (due to potential race with
5664-
* the clone disappearing) and rare, so not worth dealing with.
5688+
* pages within the same SKB. In theory we could take full
5689+
* references if @from is cloned and !@to->pp_recycle but its
5690+
* tricky (due to potential race with the clone disappearing) and
5691+
* rare, so not worth dealing with.
56655692
*/
5666-
if (to->pp_recycle != from->pp_recycle ||
5667-
(from->pp_recycle && skb_cloned(from)))
5693+
if (to->pp_recycle != from->pp_recycle)
56685694
return false;
56695695

56705696
if (len <= skb_tailroom(to)) {
@@ -5721,8 +5747,10 @@ bool skb_try_coalesce(struct sk_buff *to, struct sk_buff *from,
57215747
/* if the skb is not cloned this does nothing
57225748
* since we set nr_frags to 0.
57235749
*/
5724-
for (i = 0; i < from_shinfo->nr_frags; i++)
5725-
__skb_frag_ref(&from_shinfo->frags[i]);
5750+
if (skb_pp_frag_ref(from)) {
5751+
for (i = 0; i < from_shinfo->nr_frags; i++)
5752+
__skb_frag_ref(&from_shinfo->frags[i]);
5753+
}
57265754

57275755
to->truesize += delta;
57285756
to->len += len;

0 commit comments

Comments
 (0)