[PATCH v2] tun: avoid high-order page allocation for packet header

[PATCH v2] tun: avoid high-order page allocation for packet header

[Tahsin Erdogan]

When GSO is not enabled and a packet is transmitted via writev(), all
payload is treated as header which requires a contiguous memory allocation.
This allocation request is harder to satisfy, and may even fail if there is
enough fragmentation.

Note that sendmsg() code path limits the linear copy length, so this change
makes writev() and sendmsg() more consistent.

Signed-off-by: Tahsin Erdogan <trdgn@amazon.com>
---
v2: replace linear == 0 with !linear
v1: https://lore.kernel.org/all/20230726030936.1587269-1-trdgn@amazon.com/
 drivers/net/tun.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index d75456adc62a..4c57804f4cbd 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -1523,7 +1523,7 @@ static struct sk_buff *tun_alloc_skb(struct tun_file *tfile,
 	int err;
 
 	/* Under a page?  Don't bother with paged skb. */
-	if (prepad + len < PAGE_SIZE || !linear)
+	if (prepad + len < PAGE_SIZE)
 		linear = len;
 
 	skb = sock_alloc_send_pskb(sk, prepad + linear, len - linear, noblock,
@@ -1838,6 +1838,9 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
 			 */
 			zerocopy = false;
 		} else {
+			if (!linear)
+				linear = min_t(size_t, good_linear, copylen);
+
 			skb = tun_alloc_skb(tfile, align, copylen, linear,
 					    noblock);
 		}
-- 
2.41.0

Re: [PATCH v2] tun: avoid high-order page allocation for packet header

[Eric Dumazet]

On Tue, Aug 1, 2023 at 1:07 AM Tahsin Erdogan <trdgn@amazon.com> wrote:
>
> When GSO is not enabled and a packet is transmitted via writev(), all
> payload is treated as header which requires a contiguous memory allocation.
> This allocation request is harder to satisfy, and may even fail if there is
> enough fragmentation.
>
> Note that sendmsg() code path limits the linear copy length, so this change
> makes writev() and sendmsg() more consistent.
>
> Signed-off-by: Tahsin Erdogan <trdgn@amazon.com>
> ---

I will have to tweak one existing packetdrill test, nothing major.

Tested-by: Eric Dumazet <edumazet@google.com>
Reviewed-by: Eric Dumazet <edumazet@google.com>

Thanks.

Re: [PATCH v2] tun: avoid high-order page allocation for packet header

[Eric Dumazet]

On Tue, Aug 1, 2023 at 11:37 AM Eric Dumazet <edumazet@google.com> wrote:
>
> On Tue, Aug 1, 2023 at 1:07 AM Tahsin Erdogan <trdgn@amazon.com> wrote:
> >
> > When GSO is not enabled and a packet is transmitted via writev(), all
> > payload is treated as header which requires a contiguous memory allocation.
> > This allocation request is harder to satisfy, and may even fail if there is
> > enough fragmentation.
> >
> > Note that sendmsg() code path limits the linear copy length, so this change
> > makes writev() and sendmsg() more consistent.
> >
> > Signed-off-by: Tahsin Erdogan <trdgn@amazon.com>
> > ---
>
> I will have to tweak one existing packetdrill test, nothing major.
>
> Tested-by: Eric Dumazet <edumazet@google.com>
> Reviewed-by: Eric Dumazet <edumazet@google.com>

I have to take this back, sorry.

We need to change alloc_skb_with_frags() and tun.c to attempt
high-order allocations,
otherwise tun users sending very large buffers will regress.
(Even if this _could_ fail as you pointed out if memory is tight/fragmented)

I am working to make the change in alloc_skb_with_frags() and in tun,
we can apply your patch after this prereq.

Re: [PATCH v2] tun: avoid high-order page allocation for packet header

[Willem de Bruijn]

Eric Dumazet wrote:
> On Tue, Aug 1, 2023 at 11:37 AM Eric Dumazet <edumazet@google.com> wrote:
> >
> > On Tue, Aug 1, 2023 at 1:07 AM Tahsin Erdogan <trdgn@amazon.com> wrote:
> > >
> > > When GSO is not enabled and a packet is transmitted via writev(), all
> > > payload is treated as header which requires a contiguous memory allocation.
> > > This allocation request is harder to satisfy, and may even fail if there is
> > > enough fragmentation.
> > >
> > > Note that sendmsg() code path limits the linear copy length, so this change
> > > makes writev() and sendmsg() more consistent.
> > >
> > > Signed-off-by: Tahsin Erdogan <trdgn@amazon.com>
> > > ---
> >
> > I will have to tweak one existing packetdrill test, nothing major.
> >
> > Tested-by: Eric Dumazet <edumazet@google.com>
> > Reviewed-by: Eric Dumazet <edumazet@google.com>
> 
> I have to take this back, sorry.
> 
> We need to change alloc_skb_with_frags() and tun.c to attempt
> high-order allocations,
> otherwise tun users sending very large buffers will regress.
> (Even if this _could_ fail as you pointed out if memory is tight/fragmented)
> 
> I am working to make the change in alloc_skb_with_frags() and in tun,
> we can apply your patch after this prereq.

This exactly same allocation logic also exists in packet_alloc_skb and
tap_alloc_skb. If changing one of them, perhaps should address convert
all at the same time, to keep behavior consistent.

Re: [PATCH v2] tun: avoid high-order page allocation for packet header

[Eric Dumazet]

On Tue, Aug 1, 2023 at 3:14 PM Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
>
>> This exactly same allocation logic also exists in packet_alloc_skb and
> tap_alloc_skb. If changing one of them, perhaps should address convert
> all at the same time, to keep behavior consistent.

Sure, I can take care of them as well.

Re: [PATCH v2] tun: avoid high-order page allocation for packet header

[Erdogan, Tahsin]

On Tue, 2023-08-01 at 12:16 +0200, Eric Dumazet wrote:
> On Tue, Aug 1, 2023 at 11:37 AM Eric Dumazet <edumazet@google.com>
> wrote:
> > On Tue, Aug 1, 2023 at 1:07 AM Tahsin Erdogan <trdgn@amazon.com>
> > wrote:
> > > When GSO is not enabled and a packet is transmitted via writev(),
> > > all
> > > payload is treated as header which requires a contiguous memory
> > > allocation.
> > > This allocation request is harder to satisfy, and may even fail
> > > if there is
> > > enough fragmentation.
> > > 
> > > Note that sendmsg() code path limits the linear copy length, so
> > > this change
> > > makes writev() and sendmsg() more consistent.
> > > 
> > > Signed-off-by: Tahsin Erdogan <trdgn@amazon.com>
> > > ---
> > 
> > I will have to tweak one existing packetdrill test, nothing major.
> > 
> > Tested-by: Eric Dumazet <edumazet@google.com>
> > Reviewed-by: Eric Dumazet <edumazet@google.com>
> 
> I have to take this back, sorry.
> 
> We need to change alloc_skb_with_frags() and tun.c to attempt
> high-order allocations,
> otherwise tun users sending very large buffers will regress.
> (Even if this _could_ fail as you pointed out if memory is
> tight/fragmented)
> 
> I am working to make the change in alloc_skb_with_frags() and in tun,
> we can apply your patch after this prereq.

Hi Eric, I believe your changes are merged. Are we good to apply my
patch next?

Re: [PATCH v2] tun: avoid high-order page allocation for packet header

[Eric Dumazet]

On Tue, Aug 8, 2023 at 3:22 AM Erdogan, Tahsin <trdgn@amazon.com> wrote:

> Hi Eric, I believe your changes are merged. Are we good to apply my
> patch next?

Sure thing, please rebase and resend.

Thanks.