Pr/jwi/v1.19/ipv6 hdrlen test - #47764

[ upstream commit bbf7f18 ]

We have added a number of per-CPU maps recently which are used to store
variables outside of the stack. These maps are a bit cumbersome to use,
and require a lot of boilerplate code. This commit replaces the per-CPU
maps used in the NAT code with `DEFINE_AUX` and `AUX`

In this commit we also combine the `ipv6_ct_tuple`, `ipv6_nat_target`,
and `trace_ctx` into a single struct `snat_v6_args` so its easier to
pass around together instead of via separate arguments/parameters.

Signed-off-by: Dylan Reimerink <[email protected]>
Signed-off-by: Julian Wiedmann <[email protected]>

[ upstream commit 524a591 ]

The ct_buffer object is used in a BPF map to pass lots of information
across tail calls after the ct lookup is done. As a result, it's a huge
object of 96 bytes (measured on the stack). We should never allocate it
on the stack or we'll quickly run out of space.

There's also no need to allocate it on the stack because it's already in
a BPF array map. So we can simply lookup the single-entry from the map
and populate that instead of doing a map update from a stack object.

This change reduces the stack sizes of tail_handle_ipv{4,6}_from_netdev
from 296 and 352 bytes to 232 and 256 bytes respectively.

Fixes: 1085ae2 ("bpf: Split handle_ipv6 in bpf_host.c after ct_lookup6")
Fixes: 3f356b0 ("bpf: Split handle_ipv4 in bpf_host.c after ct_lookup4")
Signed-off-by: Paul Chaignon <[email protected]>
Signed-off-by: Julian Wiedmann <[email protected]>

[ upstream commit 0be0d4e ]

In commit 524a591 ("bpf: Never allocate ct_buffers on stack"), I
missed that the same issue affects bpf_lxc. So let's apply the same
here. From commit 524a591:

  The ct_buffer object is used in a BPF map to pass lots of information
  across tail calls after the ct lookup is done. As a result, it's a
  huge object of 96 bytes (measured on the stack). We should never
  allocate it on the stack or we'll quickly run out of space.

Reported-by: Jordan Rife <[email protected]>
Signed-off-by: Paul Chaignon <[email protected]>
Signed-off-by: Julian Wiedmann <[email protected]>

[ upstream commit 1a013c0 ]

Currently, `go get github.com/cilium/[email protected]` fails because
bpf/lib/aux.h is a disallowed file name on Windows:

    $ go get github.com/cilium/[email protected]
    go: downloading github.com/cilium/cilium v1.20.0-pre.2
    go: github.com/cilium/[email protected]: create zip: bpf/lib/aux.h: malformed file path "bpf/lib/aux.h": "aux" disallowed as path element component on Windows

This currently blocks any Go module (e.g. `github.com/cilium/cilium-cli`)
from depending on a version of the `github.com/cilium/cilium` module
after commit d81d1e8 ("bpf: Auxiliary variables").

It seems the `go` command disallows[^1] some file names that are special
files on Windows[^2]. Thus, rename `bpf/lib/aux.h` to `bpf/lib/auxvar.h`
to work around this.

[^1]: https://go.googlesource.com/mod/+/refs/tags/v0.35.0/module/module.go#505
[^2]: https://docs.microsoft.com/en-us/windows/desktop/fileio/naming-a-file

Signed-off-by: Tobias Klauser <[email protected]>
Signed-off-by: Julian Wiedmann <[email protected]>

[ upstream commit 91fc5ff ]

Commit 57d6364 ("bpf: Reduce stack usage") moves two objects off
the stack and into per-CPU array maps to reduce stack sizes. That was
prior to the introduction of the DEFINE_AUX and AUX macros and therefore
didn't use them. Let's now switch this over to these macros.

Signed-off-by: Paul Chaignon <[email protected]>
Signed-off-by: Julian Wiedmann <[email protected]>

[ upstream commit 7ac6ed5 ]

Moving the IPv6 CT tuple from function snat_v6_new_mapping() off the
stack should reduce the maximum stack size of global function
snat_v6_nat (and thus of BPF program tail_handle_snat_fwd_ipv6) by 32
bytes.

Signed-off-by: Paul Chaignon <[email protected]>
Signed-off-by: Julian Wiedmann <[email protected]>

[ upstream commit 8952164 ]

Once we make ipv6_hdrlen_with_fraginfo into a global function in the
next commit, the verifier will complain with the following error on
v5.15 and v6.1 kernels.

  26: (bf) r3 = r10       ; R3_w=fp0 R10=fp0
  27: (07) r3 += -120     ; R3_w=fp-120
  ; ret = ipv6_hdrlen_with_fraginfo(ctx, &tuple.nexthdr, &fraginfo);
  28: (bf) r1 = r6        ; R1=ctx(off=0,imm=0) R6=ctx(off=0,imm=0)
  29: (85) call pc+811
  invalid indirect read from stack R3 off -120+0 size 8
  Caller passes invalid args into func#1
  verification time 1847 usec
  stack depth 112+32
  processed 586 insns (limit 1000000) max_states_per_insn 1 total_states 50 peak_states 50 mark_read 7

When reaching the global function call, the verifier checks that pointer
arguments point to initialized memory. In our case, the third argument
points to fp-120 which, according to the stack depths, is beyond the
allocated stack size for our program at the time (112B).

An easy solution to this issue is to always initialize fraginfo before
passing it via pointer to ipv6_hdrlen_with_fraginfo.

This is only happening in kernels v5.15 and v6.1 because it was fixed in
commit [1] upstream. That coming was released in v6.8 and backported to
v6.6.

1: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6b4a64bafd10
Signed-off-by: Paul Chaignon <[email protected]>
Signed-off-by: Julian Wiedmann <[email protected]>

[ upstream commit e9cd917 ]

That function is at the top of several BPF program and contains a loop
and a few conditions. Moving it to a global function should help a lot
with complexity.

Pointers to the stack given in arguments to global functions are seen as
mem_or_null by the verifier, even if they are always non-null in the
caller. To allow derefencing those pointers, we need to null-check them.
That is already done for fraginfo in ipv6_hdrlen_offset, so that only
leaves nexthdr to null-check.

Signed-off-by: Paul Chaignon <[email protected]>
Signed-off-by: Julian Wiedmann <[email protected]>

[ upstream commit 7e629c0 ]

The combined stack size for tail_nodeport_nat_egress_ipv6, which now
includes the stack size of global function ipv6_hdrlen_with_fraginfo
(48B), is currently at 464, so 90.6% of the limit. Such a large stack
size is rejected by our CI so we need to reduce it.

A good candidate is the struct bpf_fib_lookup_padded object used in the
nodeport logic. Moving this to an aux global variable frees 64B, with
the combined stack size now at 400B (78% of the limit).

On its own, this change increases complexity a lot for the
tail_nodeport_nat_egress_ipv6 BPF program, putting us very close to the
1M limit for v5.15. Thankfully, with the previous commit that reduces
complexity, the overall impact on complexity is very positive.

Signed-off-by: Paul Chaignon <[email protected]>
Signed-off-by: Julian Wiedmann <[email protected]>

[ upstream commit d5c1536 ]

When creating a new connection tracking entry we allocate the full key
and value on the stack. Both are pretty large, forcing the allocation
of two large chunks of contiguous stack memory. By moving these to
aux variables, we reduce the stack usage of multiple programs.

Signed-off-by: Dylan Reimerink <[email protected]>
Signed-off-by: Julian Wiedmann <[email protected]>

Signed-off-by: Julian Wiedmann <[email protected]>