Skip to content

Commit 5584d9e

Browse files
author
Alexei Starovoitov
committed
Merge branch 'xdp: recycle Page Pool backed skbs built from XDP frames'
Alexander Lobakin says: ==================== Yeah, I still remember that "Who needs cpumap nowadays" (c), but anyway. __xdp_build_skb_from_frame() missed the moment when the networking stack became able to recycle skb pages backed by a page_pool. This was making e.g. cpumap redirect even less effective than simple %XDP_PASS. veth was also affected in some scenarios. A lot of drivers use skb_mark_for_recycle() already, it's been almost two years and seems like there are no issues in using it in the generic code too. {__,}xdp_release_frame() can be then removed as it losts its last user. Page Pool becomes then zero-alloc (or almost) in the abovementioned cases, too. Other memory type models (who needs them at this point) have no changes. Some numbers on 1 Xeon Platinum core bombed with 27 Mpps of 64-byte IPv6 UDP, iavf w/XDP[0] (CONFIG_PAGE_POOL_STATS is enabled): Plain %XDP_PASS on baseline, Page Pool driver: src cpu Rx drops dst cpu Rx 2.1 Mpps N/A 2.1 Mpps cpumap redirect (cross-core, w/o leaving its NUMA node) on baseline: 6.8 Mpps 5.0 Mpps 1.8 Mpps cpumap redirect with skb PP recycling: 7.9 Mpps 5.7 Mpps 2.2 Mpps +22% (from cpumap redir on baseline) [0] https://github.com/alobakin/linux/commits/iavf-xdp ==================== Signed-off-by: Alexei Starovoitov <[email protected]>
2 parents 283b40c + d4e4923 commit 5584d9e

File tree

4 files changed

+30
-58
lines changed

4 files changed

+30
-58
lines changed

include/linux/skbuff.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5069,12 +5069,12 @@ static inline u64 skb_get_kcov_handle(struct sk_buff *skb)
50695069
#endif
50705070
}
50715071

5072-
#ifdef CONFIG_PAGE_POOL
50735072
static inline void skb_mark_for_recycle(struct sk_buff *skb)
50745073
{
5074+
#ifdef CONFIG_PAGE_POOL
50755075
skb->pp_recycle = 1;
5076-
}
50775076
#endif
5077+
}
50785078

50795079
#endif /* __KERNEL__ */
50805080
#endif /* _LINUX_SKBUFF_H */

include/net/xdp.h

Lines changed: 0 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -317,35 +317,6 @@ void xdp_flush_frame_bulk(struct xdp_frame_bulk *bq);
317317
void xdp_return_frame_bulk(struct xdp_frame *xdpf,
318318
struct xdp_frame_bulk *bq);
319319

320-
/* When sending xdp_frame into the network stack, then there is no
321-
* return point callback, which is needed to release e.g. DMA-mapping
322-
* resources with page_pool. Thus, have explicit function to release
323-
* frame resources.
324-
*/
325-
void __xdp_release_frame(void *data, struct xdp_mem_info *mem);
326-
static inline void xdp_release_frame(struct xdp_frame *xdpf)
327-
{
328-
struct xdp_mem_info *mem = &xdpf->mem;
329-
struct skb_shared_info *sinfo;
330-
int i;
331-
332-
/* Curr only page_pool needs this */
333-
if (mem->type != MEM_TYPE_PAGE_POOL)
334-
return;
335-
336-
if (likely(!xdp_frame_has_frags(xdpf)))
337-
goto out;
338-
339-
sinfo = xdp_get_shared_info_from_frame(xdpf);
340-
for (i = 0; i < sinfo->nr_frags; i++) {
341-
struct page *page = skb_frag_page(&sinfo->frags[i]);
342-
343-
__xdp_release_frame(page_address(page), mem);
344-
}
345-
out:
346-
__xdp_release_frame(xdpf->data, mem);
347-
}
348-
349320
static __always_inline unsigned int xdp_get_frame_len(struct xdp_frame *xdpf)
350321
{
351322
struct skb_shared_info *sinfo;

net/core/xdp.c

Lines changed: 2 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -531,21 +531,6 @@ void xdp_return_buff(struct xdp_buff *xdp)
531531
}
532532
EXPORT_SYMBOL_GPL(xdp_return_buff);
533533

534-
/* Only called for MEM_TYPE_PAGE_POOL see xdp.h */
535-
void __xdp_release_frame(void *data, struct xdp_mem_info *mem)
536-
{
537-
struct xdp_mem_allocator *xa;
538-
struct page *page;
539-
540-
rcu_read_lock();
541-
xa = rhashtable_lookup(mem_id_ht, &mem->id, mem_id_rht_params);
542-
page = virt_to_head_page(data);
543-
if (xa)
544-
page_pool_release_page(xa->page_pool, page);
545-
rcu_read_unlock();
546-
}
547-
EXPORT_SYMBOL_GPL(__xdp_release_frame);
548-
549534
void xdp_attachment_setup(struct xdp_attachment_info *info,
550535
struct netdev_bpf *bpf)
551536
{
@@ -658,8 +643,8 @@ struct sk_buff *__xdp_build_skb_from_frame(struct xdp_frame *xdpf,
658643
* - RX ring dev queue index (skb_record_rx_queue)
659644
*/
660645

661-
/* Until page_pool get SKB return path, release DMA here */
662-
xdp_release_frame(xdpf);
646+
if (xdpf->mem.type == MEM_TYPE_PAGE_POOL)
647+
skb_mark_for_recycle(skb);
663648

664649
/* Allow SKB to reuse area used by xdp_frame */
665650
xdp_scrub_frame(xdpf);

tools/testing/selftests/bpf/progs/test_xdp_do_redirect.c

Lines changed: 26 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,19 @@
44

55
#define ETH_ALEN 6
66
#define HDR_SZ (sizeof(struct ethhdr) + sizeof(struct ipv6hdr) + sizeof(struct udphdr))
7+
8+
/**
9+
* enum frame_mark - magics to distinguish page/packet paths
10+
* @MARK_XMIT: page was recycled due to the frame being "xmitted" by the NIC.
11+
* @MARK_IN: frame is being processed by the input XDP prog.
12+
* @MARK_SKB: frame did hit the TC ingress hook as an skb.
13+
*/
14+
enum frame_mark {
15+
MARK_XMIT = 0U,
16+
MARK_IN = 0x42,
17+
MARK_SKB = 0x45,
18+
};
19+
720
const volatile int ifindex_out;
821
const volatile int ifindex_in;
922
const volatile __u8 expect_dst[ETH_ALEN];
@@ -34,10 +47,10 @@ int xdp_redirect(struct xdp_md *xdp)
3447
if (*metadata != 0x42)
3548
return XDP_ABORTED;
3649

37-
if (*payload == 0) {
38-
*payload = 0x42;
50+
if (*payload == MARK_XMIT)
3951
pkts_seen_zero++;
40-
}
52+
53+
*payload = MARK_IN;
4154

4255
if (bpf_xdp_adjust_meta(xdp, 4))
4356
return XDP_ABORTED;
@@ -51,21 +64,21 @@ int xdp_redirect(struct xdp_md *xdp)
5164
return ret;
5265
}
5366

54-
static bool check_pkt(void *data, void *data_end)
67+
static bool check_pkt(void *data, void *data_end, const __u32 mark)
5568
{
5669
struct ipv6hdr *iph = data + sizeof(struct ethhdr);
5770
__u8 *payload = data + HDR_SZ;
5871

5972
if (payload + 1 > data_end)
6073
return false;
6174

62-
if (iph->nexthdr != IPPROTO_UDP || *payload != 0x42)
75+
if (iph->nexthdr != IPPROTO_UDP || *payload != MARK_IN)
6376
return false;
6477

6578
/* reset the payload so the same packet doesn't get counted twice when
6679
* it cycles back through the kernel path and out the dst veth
6780
*/
68-
*payload = 0;
81+
*payload = mark;
6982
return true;
7083
}
7184

@@ -75,11 +88,11 @@ int xdp_count_pkts(struct xdp_md *xdp)
7588
void *data = (void *)(long)xdp->data;
7689
void *data_end = (void *)(long)xdp->data_end;
7790

78-
if (check_pkt(data, data_end))
91+
if (check_pkt(data, data_end, MARK_XMIT))
7992
pkts_seen_xdp++;
8093

81-
/* Return XDP_DROP to make sure the data page is recycled, like when it
82-
* exits a physical NIC. Recycled pages will be counted in the
94+
/* Return %XDP_DROP to recycle the data page with %MARK_XMIT, like
95+
* it exited a physical NIC. Those pages will be counted in the
8396
* pkts_seen_zero counter above.
8497
*/
8598
return XDP_DROP;
@@ -91,9 +104,12 @@ int tc_count_pkts(struct __sk_buff *skb)
91104
void *data = (void *)(long)skb->data;
92105
void *data_end = (void *)(long)skb->data_end;
93106

94-
if (check_pkt(data, data_end))
107+
if (check_pkt(data, data_end, MARK_SKB))
95108
pkts_seen_tc++;
96109

110+
/* Will be either recycled or freed, %MARK_SKB makes sure it won't
111+
* hit any of the counters above.
112+
*/
97113
return 0;
98114
}
99115

0 commit comments

Comments
 (0)