Re: [RFC net-next] net: veth: reduce page_pool memory footprint using half page per-buffer

Lorenzo Bianconi
2023-05-12T14:14:52Z

[-- Attachment #1: Type: text/plain, Size: 3510 bytes --]

> From: Lorenzo Bianconi <lorenzo@kernel.org>
> Date: Fri, 12 May 2023 15:08:13 +0200
>
> > In order to reduce page_pool memory footprint, rely on
> > page_pool_dev_alloc_frag routine and reduce buffer size
> > (VETH_PAGE_POOL_FRAG_SIZE) to PAGE_SIZE / 2 in order to consume one page
> > for two 1500B frames. Reduce VETH_XDP_PACKET_HEADROOM to 192 from 256
> > (XDP_PACKET_HEADROOM) to fit max_head_size in VETH_PAGE_POOL_FRAG_SIZE.
> > Please note, using default values (CONFIG_MAX_SKB_FRAGS=17), maximum
> > supported MTU is now reduced to 36350B.
>
> I thought we're stepping away from page splitting bit by bit O_o

do you mean to driver private page_split implementation? AFAIK we are not
stepping away from page_pool page split implementation (or maybe I missed it :))

> Primarily for the reasons you mentioned / worked around here: it creates
> several significant limitations and at least on 64-bit systems it
> doesn't scale anymore. 192 bytes of headroom is less than what XDP
> expects (isn't it? Isn't 256 standard-standard, so that skb XDP path
> reallocates heads only to have 256+ there?), 384 bytes of shinfo can
> change anytime and even now page split simply blocks you from increasing
> MAX_SKB_FRAGS even by one. Not speaking of MTU limitations etc.
> BTW Intel drivers suffer from the very same things due solely to page
> split (and I'm almost done with converting at least some of them to Page
> Pool and 1 page per buffer model), I don't recommend deliberately
> falling into that pit =\ :D

I am not sure about the 192 vs 256 bytes of headroom (this is why I sent this
patch as RFC, my main goal is to discuss about this requirement). In the
previous discussion [0] we deferred this implementation since if we do not
reduce requested xdp headroom, we will not be able to fit two 1500B frames
into a single page (for skb_shared_info size [1]) and we introduce a performance
penalty.

Regards,
Lorenzo

[0] https://lore.kernel.org/netdev/6298f73f7cc7391c7c4a52a6a89b1ae21488bda1.1682188837.git.lorenzo@kernel.org/
[1] $ pahole -C skb_shared_info vmlinux.o
struct skb_shared_info {
__u8 flags; /* 0 1 */
__u8 meta_len; /* 1 1 */
__u8 nr_frags; /* 2 1 */
__u8 tx_flags; /* 3 1 */
unsigned short gso_size; /* 4 2 */
unsigned short gso_segs; /* 6 2 */
struct sk_buff * frag_list; /* 8 8 */
struct skb_shared_hwtstamps hwtstamps; /* 16 8 */
unsigned int gso_type; /* 24 4 */
u32 tskey; /* 28 4 */
atomic_t dataref; /* 32 4 */
unsigned int xdp_frags_size; /* 36 4 */
void * destructor_arg; /* 40 8 */
skb_frag_t frags[17]; /* 48 272 */

/* size: 320, cachelines: 5, members: 14 */
};

>
> >
> > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > ---
> > drivers/net/veth.c | 39 +++++++++++++++++++++++++--------------
> > 1 file changed, 25 insertions(+), 14 deletions(-)
> [...]
>
> Thanks,
> Olek

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]