Re: [PATCH net] net: avoid 32 x truesize under-estimation for tiny skbs

On Wed, Mar 31, 2021 at 1:34 PM Xuan Zhuo [off-list ref] wrote:

On Wed, 31 Mar 2021 04:11:12 -0400, Michael S. Tsirkin [off-list ref] wrote:

quoted

On Mon, Mar 29, 2021 at 11:06:09AM +0200, Eric Dumazet wrote:

quoted

On Mon, Mar 29, 2021 at 10:52 AM Xuan Zhuo [off-list ref] wrote:

quoted

On Wed, 13 Jan 2021 08:18:19 -0800, Eric Dumazet [off-list ref] wrote:

quoted

From: Eric Dumazet <redacted>

Both virtio net and napi_get_frags() allocate skbs
with a very small skb->head

While using page fragments instead of a kmalloc backed skb->head might give
a small performance improvement in some cases, there is a huge risk of
under estimating memory usage.

For both GOOD_COPY_LEN and GRO_MAX_HEAD, we can fit at least 32 allocations
per page (order-3 page in x86), or even 64 on PowerPC

We have been tracking OOM issues on GKE hosts hitting tcp_mem limits
but consuming far more memory for TCP buffers than instructed in tcp_mem[2]

Even if we force napi_alloc_skb() to only use order-0 pages, the issue
would still be there on arches with PAGE_SIZE >= 32768

This patch makes sure that small skb head 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.

Note that we might in the future use the sk_buff napi cache,
instead of going through a more expensive __alloc_skb()

Another idea would be to use separate page sizes depending
on the allocated length (to never have more than 4 frags per page)

I would like to thank Greg Thelen for his precious help on this matter,
analysing crash dumps is always a time consuming task.

This patch causes a performance degradation of about 10% in the scenario of
virtio-net + GRO.

For GRO, there is no way to merge skbs based on frags with this patch, only
frag_list can be used to link skbs. The problem that this cause are that compared
to the GRO package merged into the frags way, the current skb needs to call
kfree_skb_list to release each skb, resulting in performance degradation.

virtio-net will store some data onto the linear space after receiving it. In
addition to the header, there are also some payloads, so "headlen <= offset"
fails. And skb->head_frag is failing when use kmalloc() for skb->head allocation.

Thanks for the report.

There is no way we can make things both fast for existing strategies
used by _insert_your_driver
and malicious usages of data that can sit for seconds/minutes in socket queues.

I think that if you want to gain this 10% back, you have to change
virtio_net to meet optimal behavior.

Normal drivers make sure to not pull payload in skb->head, only headers.

Hmm we do have hdr_len field, but seem to ignore it on RX.
Jason do you see any issues with using it for the head len?

Why not add a struct skb_shared_info space when adding merge/big buffer?
So that we can use build_skb to create skb. For skb with little data, we can
copy directly to save pages.

This does not matter. build_skb() is a distraction here.

Ultimately, you want to receive fat GRO packets with 17 MSS per sk_buff

The patch I gave earlier will combine :

1) Advantage of having skb->head being backed by slab allocations,
   to solve the OOM issue my patch already coped with.

2)  Up to 17 page frags attached to the sk_buff, storing 17 MSS.

In the past, virtio_net had to use 2 slots in shared_info per MSS
  -One slot for the first ~200 bytes of payload
 - One slot for the last part of the MSS.

Please try my patch, so that we confirm the issue is really in page_to_skb()

Thanks