netdev - Re: [PATCH net-next 06/15] ipv6/gro: insert temporary HBH/jumbo header

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CANn89i+ZN=Gvn93pCaF=f4Q5X9vyzS24CpevtWpaD0Zjb7OGHg@mail.gmail.com>
Date:   Thu, 3 Feb 2022 07:48:10 -0800
From:   Eric Dumazet <edumazet@...gle.com>
To:     Paolo Abeni <pabeni@...hat.com>
Cc:     Eric Dumazet <eric.dumazet@...il.com>,
        "David S . Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        netdev <netdev@...r.kernel.org>, Coco Li <lixiaoyan@...gle.com>
Subject: Re: [PATCH net-next 06/15] ipv6/gro: insert temporary HBH/jumbo header

On Thu, Feb 3, 2022 at 1:20 AM Paolo Abeni <pabeni@...hat.com> wrote:
>
> On Wed, 2022-02-02 at 17:51 -0800, Eric Dumazet wrote:
> > From: Eric Dumazet <edumazet@...gle.com>
> >
> > Following patch will add GRO_IPV6_MAX_SIZE, allowing gro to build
> > BIG TCP ipv6 packets (bigger than 64K).
> >
> > This patch changes ipv6_gro_complete() to insert a HBH/jumbo header
> > so that resulting packet can go through IPv6/TCP stacks.
> >
> > Signed-off-by: Eric Dumazet <edumazet@...gle.com>
> > ---
> >  net/ipv6/ip6_offload.c | 32 ++++++++++++++++++++++++++++++--
> >  1 file changed, 30 insertions(+), 2 deletions(-)
> >
> > diff --git a/net/ipv6/ip6_offload.c b/net/ipv6/ip6_offload.c
> > index d37a79a8554e92a1dcaa6fd023cafe2114841ece..dac6f60436e167a3d979fef02f25fc039c6ed37d 100644
> > --- a/net/ipv6/ip6_offload.c
> > +++ b/net/ipv6/ip6_offload.c
> > @@ -318,15 +318,43 @@ static struct sk_buff *ip4ip6_gro_receive(struct list_head *head,
> >  INDIRECT_CALLABLE_SCOPE int ipv6_gro_complete(struct sk_buff *skb, int nhoff)
> >  {
> >       const struct net_offload *ops;
> > -     struct ipv6hdr *iph = (struct ipv6hdr *)(skb->data + nhoff);
> > +     struct ipv6hdr *iph;
> >       int err = -ENOSYS;
> > +     u32 payload_len;
> >
> >       if (skb->encapsulation) {
> >               skb_set_inner_protocol(skb, cpu_to_be16(ETH_P_IPV6));
> >               skb_set_inner_network_header(skb, nhoff);
> >       }
> >
> > -     iph->payload_len = htons(skb->len - nhoff - sizeof(*iph));
> > +     payload_len = skb->len - nhoff - sizeof(*iph);
> > +     if (unlikely(payload_len > IPV6_MAXPLEN)) {
> > +             struct hop_jumbo_hdr *hop_jumbo;
> > +             int hoplen = sizeof(*hop_jumbo);
> > +
> > +             /* Move network header left */
> > +             memmove(skb_mac_header(skb) - hoplen, skb_mac_header(skb),
> > +                     skb->transport_header - skb->mac_header);
>
> I was wondering if we should check for enough headroom and what about
> TCP over UDP tunnel, then I read the next patch ;)

The check about headroom is provided in the following patch (ipv6: add
GRO_IPV6_MAX_SIZE),
which allows GRO stack to build packets bigger than 64KB,
if drivers provided enough headroom (8 bytes).
They usually provide NET_SKB_PAD (64 bytes or more)

Before the next patch, this code is dead.

Also current patch set does not cook BIG TCP packets for tunneled traffic
(look at skb_gro_receive() changes in following patch)


>
> I think a comment here referring to the constraint enforced by
> skb_gro_receive() could help, or perhaps squashing the 2 patches?!?

Well no, we spent time making small patches to ease review, and these patches
have different authors anyway.

>
> Thanks!
>
> Paolo
>