Date:   Wed, 7 Sep 2022 15:43:00 -0700
From:   Kuniyuki Iwashima <kuniyu@...zon.com>
To:     <edumazet@...gle.com>
CC:     <davem@...emloft.net>, <kuba@...nel.org>, <kuni1840@...il.com>,
        <kuniyu@...zon.com>, <netdev@...r.kernel.org>, <pabeni@...hat.com>
Subject: Re: [PATCH v5 net-next 6/6] tcp: Introduce optional per-netns ehash.

From:   Eric Dumazet <edumazet@...gle.com>
Date:   Wed, 7 Sep 2022 14:57:14 -0700
> On Wed, Sep 7, 2022 at 2:47 PM Kuniyuki Iwashima <kuniyu@...zon.com> wrote:
> >
> > From:   Eric Dumazet <edumazet@...gle.com>
> > Date:   Wed, 7 Sep 2022 13:55:08 -0700
> > > On Tue, Sep 6, 2022 at 5:57 PM Kuniyuki Iwashima <kuniyu@...zon.com> wrote:
> > > >
> > > > The more sockets we have in the hash table, the longer we spend looking
> > > > up the socket.  While running a number of small workloads on the same
> > > > host, they penalise each other and cause performance degradation.
> > > >
> > > > The root cause might be a single workload that consumes much more
> > > > resources than the others.  It often happens on a cloud service where
> > > > different workloads share the same computing resource.
> > > >
> > > > On EC2 c5.24xlarge instance (196 GiB memory and 524288 (1Mi / 2) ehash
> > > > entries), after running iperf3 in different netns, creating 24Mi sockets
> > > > without data transfer in the root netns causes about 10% performance
> > > > regression for the iperf3's connection.
> > > >
> > > >  thash_entries          sockets         length          Gbps
> > > >         524288                1              1          50.7
> > > >                            24Mi             48          45.1
> > > >
> > > > It is basically related to the length of the list of each hash bucket.
> > > > For testing purposes to see how performance drops along the length,
> > > > I set 131072 (1Mi / 8) to thash_entries, and here's the result.
> > > >
> > > >  thash_entries          sockets         length          Gbps
> > > >         131072                1              1          50.7
> > > >                             1Mi              8          49.9
> > > >                             2Mi             16          48.9
> > > >                             4Mi             32          47.3
> > > >                             8Mi             64          44.6
> > > >                            16Mi            128          40.6
> > > >                            24Mi            192          36.3
> > > >                            32Mi            256          32.5
> > > >                            40Mi            320          27.0
> > > >                            48Mi            384          25.0
> > > >
> > > > To resolve the socket lookup degradation, we introduce an optional
> > > > per-netns hash table for TCP, but it's just ehash, and we still share
> > > > the global bhash, bhash2 and lhash2.
> > > >
> > > > With a smaller ehash, we can look up non-listener sockets faster and
> > > > isolate such noisy neighbours.  In addition, we can reduce lock contention.
> > > >
> > > > We can control the ehash size by a new sysctl knob.  However, depending
> > > > on workloads, it will require very sensitive tuning, so we disable the
> > > > feature by default (net.ipv4.tcp_child_ehash_entries == 0).  Moreover,
> > > > we can fall back to using the global ehash in case we fail to allocate
> > > > enough memory for a new ehash.  The maximum size is 16Mi, which is large
> > > > enough that even if we have 48Mi sockets, the average list length is 3,
> > > > and regression would be less than 1%.
> > > >
> > > > We can check the current ehash size by another read-only sysctl knob,
> > > > net.ipv4.tcp_ehash_entries.  A negative value means the netns shares
> > > > the global ehash (per-netns ehash is disabled or failed to allocate
> > > > memory).
> > > >
> > > >   # dmesg | cut -d ' ' -f 5- | grep "established hash"
> > > >   TCP established hash table entries: 524288 (order: 10, 4194304 bytes, vmalloc hugepage)
> > > >
> > > >   # sysctl net.ipv4.tcp_ehash_entries
> > > >   net.ipv4.tcp_ehash_entries = 524288  # can be changed by thash_entries
> > > >
> > > >   # sysctl net.ipv4.tcp_child_ehash_entries
> > > >   net.ipv4.tcp_child_ehash_entries = 0  # disabled by default
> > > >
> > > >   # ip netns add test1
> > > >   # ip netns exec test1 sysctl net.ipv4.tcp_ehash_entries
> > > >   net.ipv4.tcp_ehash_entries = -524288  # share the global ehash
> > > >
> > > >   # sysctl -w net.ipv4.tcp_child_ehash_entries=100
> > > >   net.ipv4.tcp_child_ehash_entries = 100
> > > >
> > > >   # ip netns add test2
> > > >   # ip netns exec test2 sysctl net.ipv4.tcp_ehash_entries
> > > >   net.ipv4.tcp_ehash_entries = 128  # own a per-netns ehash with 2^n buckets
> > > >
> > > > When more than two processes in the same netns create per-netns ehash
> > > > concurrently with different sizes, we need to guarantee the size in
> > > > one of the following ways:
> > > >
> > > >   1) Share the global ehash and create per-netns ehash
> > > >
> > > >   First, unshare() with tcp_child_ehash_entries==0.  It creates dedicated
> > > >   netns sysctl knobs where we can safely change tcp_child_ehash_entries
> > > >   and clone()/unshare() to create a per-netns ehash.
> > > >
> > > >   2) Control write on sysctl by BPF
> > > >
> > > >   We can use BPF_PROG_TYPE_CGROUP_SYSCTL to allow/deny read/write on
> > > >   sysctl knobs.
> > > >
> > > > Note the default values of two sysctl knobs depend on the ehash size and
> > > > should be tuned carefully:
> > > >
> > > >   tcp_max_tw_buckets  : tcp_child_ehash_entries / 2
> > > >   tcp_max_syn_backlog : max(128, tcp_child_ehash_entries / 128)
> > > >
> > > > As a bonus, we can dismantle netns faster.  Currently, while destroying
> > > > netns, we call inet_twsk_purge(), which walks through the global ehash.
> > > > It can be potentially big because it can have many sockets other than
> > > > TIME_WAIT in all netns.  Splitting ehash changes that situation, where
> > > > it's only necessary for inet_twsk_purge() to clean up TIME_WAIT sockets
> > > > in each netns.
> > > >
> > > > With regard to this, we do not free the per-netns ehash in inet_twsk_kill()
> > > > to avoid UAF while iterating the per-netns ehash in inet_twsk_purge().
> > > > Instead, we do it in tcp_sk_exit_batch() after calling tcp_twsk_purge() to
> > > > keep it protocol-family-independent.
> > > >
> > > > In the future, we could optimise ehash lookup/iteration further by removing
> > > > netns comparison for the per-netns ehash.
> > > >
> > > > Signed-off-by: Kuniyuki Iwashima <kuniyu@...zon.com>
> > >
> > > ...
> > >
> > > > diff --git a/net/ipv4/inet_hashtables.c b/net/ipv4/inet_hashtables.c
> > > > index c440de998910..e94e1316fcc3 100644
> > > > --- a/net/ipv4/inet_hashtables.c
> > > > +++ b/net/ipv4/inet_hashtables.c
> > > > @@ -1145,3 +1145,60 @@ int inet_ehash_locks_alloc(struct inet_hashinfo *hashinfo)
> > > >         return 0;
> > > >  }
> > > >  EXPORT_SYMBOL_GPL(inet_ehash_locks_alloc);
> > > > +
> > > > +struct inet_hashinfo *inet_pernet_hashinfo_alloc(struct inet_hashinfo *hashinfo,
> > > > +                                                unsigned int ehash_entries)
> > > > +{
> > > > +       struct inet_hashinfo *new_hashinfo;
> > > > +       int i;
> > > > +
> > > > +       new_hashinfo = kmalloc(sizeof(*new_hashinfo), GFP_KERNEL);
> > > > +       if (!new_hashinfo)
> > > > +               goto err;
> > > > +
> > > > +       new_hashinfo->ehash = kvmalloc_array(ehash_entries,
> > > > +                                            sizeof(struct inet_ehash_bucket),
> > > > +                                            GFP_KERNEL_ACCOUNT);
> > >
> > > Note that in current kernel,  init_net ehash table is using hugepages:
> > >
> > > # dmesg | grep "TCP established hash table"
> > > [   17.512756] TCP established hash table entries: 524288 (order: 10,
> > > 4194304 bytes, vmalloc hugepage)
> > >
> > > As this is very desirable, I would suggest using the following to
> > > avoid possible performance regression,
> > > especially for workload wanting a big ehash, as hinted by your changelog.
> > >
> > > new_hashinfo->ehash = vmalloc_huge(ehash_entries * sizeof(struct
> > > inet_ehash_bucket), GFP_KERNEL_ACCOUNT);
> > >
> > > (No overflow can happen in the multiply, as ehash_entries < 16M)
> >
> > Do we need 'get_order(size) >= MAX_ORDER' check or just use it?
> 
> No need, just use it.
> 
> If you happen to allocate 1MB, vmalloc_huge() will simply use 256 4k
> pages, not 2MB.
> 
> > Due to the test in alloc_large_system_hash(), on a machine where the
> > calculted bucket size is not large enough, we don't use hugepages for
> > init_net.
> 
> I can tell that even my laptop allocates hugepage...
> 
> And it will all depend on the user-configured ehash table size.

Ok, I'll use vmalloc_huge().


> > > Another point is that on NUMA, init_net ehash table is spread over
> > > available NUMA nodes.
> > >
> > > While net_pernet_hashinfo_alloc() will allocate pages depending on
> > > current process NUMA policy.
> > >
> > > Maybe worth noting this in the changelog, because it is very possible
> > > that new nets
> > > is created with default NUMA policy, and depending on which cpu
> > > current thread is
> > > running, hash table will fully reside on a 'random' node, with very
> > > different performance
> > > results for highly optimized networking applications.
> >
> > Sounds great!
> > But I'm not familiar with mm, so let me confirm a bit more.
> >
> > It seems vmalloc_huge() always pass NUMA_NO_NODE to __vmalloc_node_range(),
> > so if we use vmalloc_huge(), the per-net ehash will be spread on each NUMA
> > nodes unless vmap_allow_huge is disabled in the kernel parameters, right?
> 
> No, it depends on current NUMA policy
> 
> "man numa"
> 
> By default, at system init time, NUMA policy spreads allocations on
> all memory nodes.
> 
> After system has booted, default NUMA policy allocates memory on your
> local node only.
> 
> You can check on a NUMA host how vmalloc regions are spread
> 
> grep N0= /proc/vmallocinfo
> grep N1= /proc/vmallocinfo
> 
> 
> For instance, all large system hashes are evenly spread:
> 
> grep alloc_large_system_hash /proc/vmallocinfo
> ...
> 0x00000000c13cf72b-0x0000000068e51b86 4198400
> alloc_large_system_hash+0x160/0x3aa pages=1024 vmalloc vpages N0=512
> N1=512
> ...
> 
> while:
> # echo 200000 >/proc/sys/net/ipv4/tcp_child_ehash_entries
> # unshare -n
> # grep  inet_pernet_hashinfo_alloc /proc/vmallocinfo
> 0x00000000980d41fd-0x00000000ea510502 2101248
> inet_pernet_hashinfo_alloc+0x79/0x910 pages=512 vmalloc N1=512
> 
> (everything has been allocated into N1, because "unshare -n" probably
> was scheduled on a cpu in NUMA node1)

Thanks for explanation!
I'll note the difference with the global ehash and NUMA policy in the
changelog.


> > Or, even if we use vmalloc_huge(), the ehash could be controlled by the
> > current process's NUMA policy?  (Sorry I'm not sure where the policy is
> > applied..)
> >
> >
> > > > +       if (!new_hashinfo->ehash)
> > > > +               goto free_hashinfo;
> > > > +
> > > > +       new_hashinfo->ehash_mask = ehash_entries - 1;
> > > > +
> > > > +       if (inet_ehash_locks_alloc(new_hashinfo))
> > > > +               goto free_ehash;
> > > > +
> > > > +       for (i = 0; i < ehash_entries; i++)
> > > > +               INIT_HLIST_NULLS_HEAD(&new_hashinfo->ehash[i].chain, i);
> > > > +
> > > > +       new_hashinfo->bind_bucket_cachep = hashinfo->bind_bucket_cachep;
> > > > +       new_hashinfo->bhash = hashinfo->bhash;
> > > > +       new_hashinfo->bind2_bucket_cachep = hashinfo->bind2_bucket_cachep;
> > > > +       new_hashinfo->bhash2 = hashinfo->bhash2;
> > > > +       new_hashinfo->bhash_size = hashinfo->bhash_size;
> > > > +
> > > > +       new_hashinfo->lhash2_mask = hashinfo->lhash2_mask;
> > > > +       new_hashinfo->lhash2 = hashinfo->lhash2;
> > > > +

Powered by blists - more mailing lists