netdev - Re: [PATCH net-next 01/15] net: add netdev->tso_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: <CANn89iKd-M6Ry+K7m+n5Voo641K7S24qm27SwrP4VAAchVPT4A@mail.gmail.com>
Date:   Thu, 3 Feb 2022 08:56:56 -0800
From:   Eric Dumazet <edumazet@...gle.com>
To:     Jakub Kicinski <kuba@...nel.org>
Cc:     Eric Dumazet <eric.dumazet@...il.com>,
        "David S . Miller" <davem@...emloft.net>,
        netdev <netdev@...r.kernel.org>, Coco Li <lixiaoyan@...gle.com>
Subject: Re: [PATCH net-next 01/15] net: add netdev->tso_ipv6_max_size attribute

On Thu, Feb 3, 2022 at 8:34 AM Jakub Kicinski <kuba@...nel.org> wrote:
>
> On Wed,  2 Feb 2022 17:51:26 -0800 Eric Dumazet wrote:
> > From: Eric Dumazet <edumazet@...gle.com>
> >
> > Some NIC (or virtual devices) are LSOv2 compatible.
> >
> > BIG TCP plans using the large LSOv2 feature for IPv6.
> >
> > New netlink attribute IFLA_TSO_IPV6_MAX_SIZE is defined.
> >
> > Drivers should use netif_set_tso_ipv6_max_size() to advertize their limit.
> >
> > Unchanged drivers are not allowing big TSO packets to be sent.
>
> Many drivers will have a limit on how many buffer descriptors they
> can chain, not the size of the super frame, I'd think. Is that not
> the case? We can't assume all pages but the first and last are full,
> right?

In our case, we have a 100Gbit Google NIC which has these limits:

- TX descriptor has a 16bit field filled with skb->len
- No more than 21 frags per 'packet'

In order to support BIG TCP on it, we had to split the bigger TCP packets
into smaller chunks, to satisfy both constraints (even if the second
constraint is hardly hit once you chop to ~60KB packets, given our 4K
MTU)

ndo_features_check() might help to take care of small oddities.

For instance I will insert the following in the next version of the series:

commit 26644be08edc2f14f6ec79f650cc4a5d380df498
Author: Eric Dumazet <edumazet@...gle.com>
Date:   Wed Feb 2 23:22:01 2022 -0800

    net: typhoon: implement ndo_features_check method

    Instead of disabling TSO if MAX_SKB_FRAGS > 32, implement
    ndo_features_check() method for this driver.

    If skb has more than 32 frags, use the following heuristic:

    1) force GSO for gso packets
    2) Otherwise force linearization.

    Most locally generated TCP packets will use a small number of fragments
    anyway.

    Signed-off-by: Eric Dumazet <edumazet@...gle.com>

diff --git a/drivers/net/ethernet/3com/typhoon.c
b/drivers/net/ethernet/3com/typhoon.c
index 8aec5d9fbfef2803c181387537300502a937caf0..216e26a49e9c272ba7483bfa06941ff11ea40e3c
100644
--- a/drivers/net/ethernet/3com/typhoon.c
+++ b/drivers/net/ethernet/3com/typhoon.c
@@ -138,11 +138,6 @@ MODULE_PARM_DESC(use_mmio, "Use MMIO (1) or
PIO(0) to access the NIC. "
 module_param(rx_copybreak, int, 0);
 module_param(use_mmio, int, 0);

-#if defined(NETIF_F_TSO) && MAX_SKB_FRAGS > 32
-#warning Typhoon only supports 32 entries in its SG list for TSO, disabling TSO
-#undef NETIF_F_TSO
-#endif
-
 #if TXLO_ENTRIES <= (2 * MAX_SKB_FRAGS)
 #error TX ring too small!
 #endif
@@ -2261,9 +2256,23 @@ typhoon_test_mmio(struct pci_dev *pdev)
        return mode;
 }

+static netdev_features_t typhoon_features_check(struct sk_buff *skb,
+                                               struct net_device *dev,
+                                               netdev_features_t features)
+{
+       if (skb_shinfo(skb)->nr_frags > 32) {
+               if (skb_is_gso(skb))
+                       features &= ~NETIF_F_GSO_MASK;
+               else
+                       features &= ~NETIF_F_SG;
+       }
+       return features;
+}
+
 static const struct net_device_ops typhoon_netdev_ops = {
        .ndo_open               = typhoon_open,
        .ndo_stop               = typhoon_close,
+       .ndo_features_check     = typhoon_features_check,
        .ndo_start_xmit         = typhoon_start_tx,
        .ndo_set_rx_mode        = typhoon_set_rx_mode,
        .ndo_tx_timeout         = typhoon_tx_timeout,