netdev - Re: [PATCH net-next 02/15] ipv6: add dev->gso_ipv6_max

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <CANn89i+-R11duRgVHoi=sJRJQ3FUVf-oruOzLbjpKAXaatK41A@mail.gmail.com>
Date:   Thu, 3 Feb 2022 07:34:08 -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 02/15] ipv6: add dev->gso_ipv6_max_size

On Thu, Feb 3, 2022 at 12:57 AM Paolo Abeni <pabeni@...hat.com> wrote:
>
> Hello,
>
> On Wed, 2022-02-02 at 17:51 -0800, Eric Dumazet wrote:
> > From: Eric Dumazet <edumazet@...gle.com>
> >
> > This enable TCP stack to build TSO packets bigger than
> > 64KB if the driver is LSOv2 compatible.
> >
> > This patch introduces new variable gso_ipv6_max_size
> > that is modifiable through ip link.
> >
> > ip link set dev eth0 gso_ipv6_max_size 185000
> >
> > User input is capped by driver limit.
> >
> > Signed-off-by: Coco Li <lixiaoyan@...gle.com>
> > Signed-off-by: Eric Dumazet <edumazet@...gle.com>
> > ---
> >  include/linux/netdevice.h          | 12 ++++++++++++
> >  include/uapi/linux/if_link.h       |  1 +
> >  net/core/dev.c                     |  1 +
> >  net/core/rtnetlink.c               | 15 +++++++++++++++
> >  net/core/sock.c                    |  6 ++++++
> >  tools/include/uapi/linux/if_link.h |  1 +
> >  6 files changed, 36 insertions(+)
> >
> > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> > index b1f68df2b37bc4b623f61cc2c6f0c02ba2afbe02..2a563869ba44f7d48095d36b1395e3fbd8cfff87 100644
> > --- a/include/linux/netdevice.h
> > +++ b/include/linux/netdevice.h
> > @@ -1949,6 +1949,7 @@ enum netdev_ml_priv_type {
> >   *   @linkwatch_dev_tracker: refcount tracker used by linkwatch.
> >   *   @watchdog_dev_tracker:  refcount tracker used by watchdog.
> >   *   @tso_ipv6_max_size:     Maximum size of IPv6 TSO packets (driver/NIC limit)
> > + *   @gso_ipv6_max_size:     Maximum size of IPv6 GSO packets (user/admin limit)
> >   *
> >   *   FIXME: cleanup struct net_device such that network protocol info
> >   *   moves out.
> > @@ -2284,6 +2285,7 @@ struct net_device {
> >       netdevice_tracker       linkwatch_dev_tracker;
> >       netdevice_tracker       watchdog_dev_tracker;
> >       unsigned int            tso_ipv6_max_size;
> > +     unsigned int            gso_ipv6_max_size;
> >  };
> >  #define to_net_dev(d) container_of(d, struct net_device, dev)
> >
> > @@ -4804,6 +4806,10 @@ static inline void netif_set_gso_max_size(struct net_device *dev,
> >  {
> >       /* dev->gso_max_size is read locklessly from sk_setup_caps() */
> >       WRITE_ONCE(dev->gso_max_size, size);
> > +
> > +     /* legacy drivers want to lower gso_max_size, regardless of family. */
> > +     size = min(size, dev->gso_ipv6_max_size);
> > +     WRITE_ONCE(dev->gso_ipv6_max_size, size);
> >  }
> >
> >  static inline void netif_set_gso_max_segs(struct net_device *dev,
> > @@ -4827,6 +4833,12 @@ static inline void netif_set_tso_ipv6_max_size(struct net_device *dev,
> >       dev->tso_ipv6_max_size = size;
> >  }
> >
> > +static inline void netif_set_gso_ipv6_max_size(struct net_device *dev,
> > +                                            unsigned int size)
> > +{
> > +     size = min(size, dev->tso_ipv6_max_size);
> > +     WRITE_ONCE(dev->gso_ipv6_max_size, size);
>
> Dumb questions on my side: should the above be limited to
> tso_ipv6_max_size ? or increasing gso_ipv6_max_size helps even if the
> egress NIC does not support LSOv2?

I thought that " size = min(size, dev->tso_ipv6_max_size);" was doing
exactly that ?

I  will fix the From: tag because patch autor is Coco Li

>
> Should gso_ipv6_max_size be capped to some reasonable value (well lower
> than 4G), to avoid the stack building very complex skbs?
>

Drivers are responsible for choosing the max value, then admins choose
optimal operational values based on their constraints (like device MTU)

Typical LSOv2 values are 256K or 512KB, but we really tested BIG TCP
with 45 4K segments per packet.

> Thanks!
>
> Paolo
>