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 <[email protected]>
---
v2: replace linear == 0 with !linear
v1: https://lore.kernel.org/all/[email protected]/
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
On Tue, Aug 1, 2023 at 1:07 AM Tahsin Erdogan <[email protected]> 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 <[email protected]>
> ---
I will have to tweak one existing packetdrill test, nothing major.
Tested-by: Eric Dumazet <[email protected]>
Reviewed-by: Eric Dumazet <[email protected]>
Thanks.
On Tue, Aug 1, 2023 at 11:37 AM Eric Dumazet <[email protected]> wrote:
>
> On Tue, Aug 1, 2023 at 1:07 AM Tahsin Erdogan <[email protected]> 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 <[email protected]>
> > ---
>
> I will have to tweak one existing packetdrill test, nothing major.
>
> Tested-by: Eric Dumazet <[email protected]>
> Reviewed-by: Eric Dumazet <[email protected]>
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.
On Tue, Aug 1, 2023 at 3:14 PM Willem de Bruijn
<[email protected]> 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.
Eric Dumazet wrote:
> On Tue, Aug 1, 2023 at 11:37 AM Eric Dumazet <[email protected]> wrote:
> >
> > On Tue, Aug 1, 2023 at 1:07 AM Tahsin Erdogan <[email protected]> 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 <[email protected]>
> > > ---
> >
> > I will have to tweak one existing packetdrill test, nothing major.
> >
> > Tested-by: Eric Dumazet <[email protected]>
> > Reviewed-by: Eric Dumazet <[email protected]>
>
> 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.
On Tue, 2023-08-01 at 12:16 +0200, Eric Dumazet wrote:
> On Tue, Aug 1, 2023 at 11:37 AM Eric Dumazet <[email protected]>
> wrote:
> > On Tue, Aug 1, 2023 at 1:07 AM Tahsin Erdogan <[email protected]>
> > 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 <[email protected]>
> > > ---
> >
> > I will have to tweak one existing packetdrill test, nothing major.
> >
> > Tested-by: Eric Dumazet <[email protected]>
> > Reviewed-by: Eric Dumazet <[email protected]>
>
> 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?
On Tue, Aug 8, 2023 at 3:22 AM Erdogan, Tahsin <[email protected]> wrote:
> Hi Eric, I believe your changes are merged. Are we good to apply my
> patch next?
Sure thing, please rebase and resend.
Thanks.