Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

bpf: Inline assembly for packet context access #25336

Merged
merged 2 commits into from
May 10, 2023
Merged

bpf: Inline assembly for packet context access #25336

merged 2 commits into from
May 10, 2023

Commits on May 9, 2023

  1. bpf: tests: Minimise calls to ctx_data{,_end}() in prep for inline asm 

    The BPF unit tests currently pass the verifier just fine. "If it ain't
broke, don't fix it." Oh but of course, that would be too easy.

In a future commit, we will implement BPF-based IPv6 masquerading, for
which we face an issue with the verifier: when accessing the context,
LLVM sometimes generates 32-bit assignments, leading to the verifier
being unable to track the packet pointers.

We have a fix for that: we can modify the way we do packet context
accesses, preventing LLVM to perform optimisations by using inline
assembly.

But the "asm volatile()" calls come at a cost: if called multiple times
ctx_data() and ctx_data_end() will no longer be optimised out, and
instead will reset the tracking of packet pointers.

To fix the fix for what's not broken yet, in other words: in preparation
for inline assembly context access, make sure we do not call ctx_data()
and ctx_data_end() more than once in the BPF unit tests, unless the
calls are separated by a call to a helper function that resets tracking
anyway.

Suggested-by: Yonghong Song <yhs@fb.com>
Suggested-by: Maxim Mikityanskiy <maxim@isovalent.com>
Signed-off-by: Quentin Monnet <quentin@isovalent.com>
    @qmonnet
    qmonnet committed May 9, 2023
    Configuration menu
    Copy the full SHA
    f37ad28 View commit details
    Browse the repository at this point in the history

  2. bpf: Avoid 32bit assignment of packet pointer 

    Since ctx->{data,data_end,data_meta} are 32-bits fields, LLVM sometimes
generates 32-bit assignments for those pointers. This leads to verifier
errors as the verifier is unable to track the packet pointers through
the 32-bit assignments, as show below.

    ; return (void *)(unsigned long)ctx->data;
    2: (61) r9 = *(u32 *)(r7 +76)
    ; R7_w=ctx(id=0,off=0,imm=0) R9_w=pkt(id=0,off=0,r=0,imm=0)
    ; return (void *)(unsigned long)ctx->data;
    3: (bc) w6 = w9
    ; R6_w=inv(id=0,umax_value=4294967295,var_off=(0x0; 0xffffffff)) R9_w=pkt(id=0,off=0,r=0,imm=0)
    ; if (data + tot_len > data_end)
    4: (bf) r2 = r6
    ; R2_w=inv(id=1,umax_value=4294967295,var_off=(0x0; 0xffffffff)) R6_w=inv(id=1,umax_value=4294967295,var_off=(0x0; 0xffffffff))
    5: (07) r2 += 54
    ; R2_w=inv(id=0,umin_value=54,umax_value=4294967349,var_off=(0x0; 0x1ffffffff))
    ; if (data + tot_len > data_end)
    6: (2d) if r2 > r1 goto pc+466
    ; R1_w=pkt_end(id=0,off=0,imm=0) R2_w=inv(id=0,umin_value=54,umax_value=4294967349,var_off=(0x0; 0x1ffffffff))
    ; tmp = a->d1 - b->d1;
    7: (71) r2 = *(u8 *)(r6 +22)
    R6 invalid mem access 'inv'

As a workaround, Yonghong Song suggested to read those fields using asm
bytecode. With that trick, LLVM is unaware that the original field is
actually on 32 bits and can't perform the above "optimization".

This workaround is needed only now because a subsequent commit
introduces some BPF C code that causes this bytecode to be generated
(snat_v6_needed).

[ Quentin: Add #undef DEFINE_FUNC_CTX_POINTER. ]

Related: https://lore.kernel.org/bpf/c5a76b4d-abed-51f6-bf16-040eb0baf290@fb.com/T/#m933626f7404562a6e98af78a8f44c30977681ffa
Suggested-by: Yonghong Song <yhs@fb.com>
Signed-off-by: Paul Chaignon <paul@cilium.io>
    @pchaigno @qmonnet
    pchaigno authored and qmonnet committed May 9, 2023
    Configuration menu
    Copy the full SHA
    278058f View commit details
    Browse the repository at this point in the history