netdev - Re: [PATCH v4 net-next 02/14] 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: <CANn89iLXnt+xav7RaEm=8zuaLpOwO1xwnQtCiqN36iSV36Fk9g@mail.gmail.com>
Date:   Tue, 15 Mar 2022 08:57:18 -0700
From:   Eric Dumazet <edumazet@...gle.com>
To:     Alexander H Duyck <alexander.duyck@...il.com>
Cc:     Eric Dumazet <eric.dumazet@...il.com>,
        "David S . Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        netdev <netdev@...r.kernel.org>,
        Alexander Duyck <alexanderduyck@...com>,
        Coco Li <lixiaoyan@...gle.com>
Subject: Re: [PATCH v4 net-next 02/14] ipv6: add dev->gso_ipv6_max_size

On Fri, Mar 11, 2022 at 8:22 AM Alexander H Duyck
<alexander.duyck@...il.com> wrote:
>
> On Wed, 2022-03-09 at 21:46 -0800, Eric Dumazet wrote:
> > From: Coco Li <lixiaoyan@...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 (tso_ipv6_max_size)
> > added in previous patch.
> >
> > 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 61db67222c47664c179b6a5d3b6f15fdf8a02bdd..9ed348d8b6f1195514c3b5f85fbe2c45b3fa997f 100644
> > --- a/include/linux/netdevice.h
> > +++ b/include/linux/netdevice.h
> > @@ -1952,6 +1952,7 @@ enum netdev_ml_priv_type {
> >   *                                   registered
> >   *   @offload_xstats_l3:     L3 HW stats for this netdevice.
> >   *   @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.
> > @@ -2291,6 +2292,7 @@ struct net_device {
> >       netdevice_tracker       dev_registered_tracker;
> >       struct rtnl_hw_stats64  *offload_xstats_l3;
> >       unsigned int            tso_ipv6_max_size;
> > +     unsigned int            gso_ipv6_max_size;
> >  };
> >  #define to_net_dev(d) container_of(d, struct net_device, dev)
> >
>
> Rather than have this as a device specific value would it be
> advantageous to consider making this a namespace specific sysctl value
> instead? Something along the lines of:
>   net.ipv6.conf.*.max_jumbogram_size
>
> It could also be applied generically to the GSO/GRO as the upper limit
> for any frame assembled by the socket or GRO.
>
> The general idea is that might be desirable for admins to be able to
> basically just set the maximum size they want to see for IPv6 frames
> and if we could combine the GRO/GSO logic into a single sysctl that
> could be set on a namespace basis instead of a device basis which would
> be more difficult to track down. We already have the per-device limits
> in the tso_ipv6_max_size for the outgoing frames so it seems like it
> might make sense to make this per network namespace and defaultable
> rather than per device and requiring an update for each device
> instance.
>

At least Google found it was easier to have per device controls, in
terms of testing the feature,
and gradually deploying it.

We have hosts with multiple NIC, of different types. We want to be
able to control BIG TCP on a per device basis.
For instance I had a bug in one of the implementation for one (non
upstream) driver, that I could mitigate
by setting a different limit only for this NIC, until the host can
boot with a fixed kernel.

We use ipvlan, with one private net-ns and IPv6 address per job, we
wanted to deploy BIG TCP on a per job basis

I guess that if you want to add a sysctl, automatically overriding the
per device setting, this could be done later ?