bpf: Various fixes for MAX_*_OPTIONS and support for 5.10
#24122
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pchaigno
added
area/CI
pchaigno
force-pushed
the
pr/pchaigno/bpf-options-cover-510-515
branch
from
f8823c4
to
5e35a27
Compare
pchaigno
force-pushed
the
pr/pchaigno/bpf-options-cover-510-515
branch
15 times, most recently
from
9b59387
to
6f56e35
Compare
pchaigno
force-pushed
the
pr/pchaigno/bpf-options-cover-510-515
branch
3 times, most recently
from
b2c5bfb
to
f08fe78
Compare
gentoo-root
reviewed
Mar 13, 2023
pchaigno
force-pushed
the
pr/pchaigno/bpf-options-cover-510-515
branch
2 times, most recently
from
0006388
to
9a9f091
Compare
gentoo-root
requested changes
Mar 16, 2023
pchaigno
force-pushed
the
pr/pchaigno/bpf-options-cover-510-515
branch
from
9a9f091
to
0e92958
Compare
gentoo-root
approved these changes
Mar 24, 2023
pchaigno
changed the title
MAX_*_OPTIONS and support for 5.10MAX_*_OPTIONS and support for 5.10
pchaigno
force-pushed
the
pr/pchaigno/bpf-options-cover-510-515
branch
from
7e466fe
to
5e13a15
Compare
aanm
requested changes
Mar 27, 2023
aanm
approved these changes
Mar 27, 2023
tklauser
approved these changes
Mar 28, 2023
Co-authored-by: Tobias Klauser <tobias@cilium.io> Signed-off-by: Paul Chaignon <paul@cilium.io> Signed-off-by: Tobias Klauser <tobias@cilium.io>
Signed-off-by: Paul Chaignon <paul@cilium.io>
Running make -C bpf KERNEL=netnext currently fails with:
In file included from bpf_xdp.c:42:
In file included from /home/paul/cilium3/bpf/lib/nodeport.h:20:
In file included from /home/paul/cilium3/bpf/lib/encap.h:11:
/home/paul/cilium3/bpf/lib/wireguard.h:80:12: error: no member named 'mark' in 'struct xdp_md'
if ((ctx->mark & MARK_MAGIC_WG_ENCRYPTED) == MARK_MAGIC_WG_ENCRYPTED)
~~~ ^
1 error generated.
That is because ENABLE_WIREGUARD is enabled even for bpf_xdp. Instead of
enabling it in MAX_BASE_OPTIONS, we should enable it only for bpf_host,
bpf_lxc, and bpf_overlay.
Signed-off-by: Paul Chaignon <paul@cilium.io>
I tested with KERNEL=419 and KERNEL=54 and the ENABLE_EGRESS_GATEWAY macro would never be defined. In short, this is caused by an improper use of double quotes. See [1] for the longer explanation. We also update KERNEL's default value to be 'kernel' and not '"kernel"' (quotes were part of the variable). 1 - #24122 (comment) Fixes: d0390ff ("bpf: add wider compile testing for EgressGW") Signed-off-by: Maxim Mikityanskiy <maxim@isovalent.com> Signed-off-by: Paul Chaignon <paul@cilium.io>
By moving ENABLE_VTEP to MAX_BASE_OPTIONS, we simplify a bit the logic, but it also means we will now enable it for all BPF programs. That shouldn't be an issue given it's compatible with e.g. XDP. Signed-off-by: Paul Chaignon <paul@cilium.io>
No functional changes in this commit.
We make use of the filter function to simplify conditions into the
following pattern:
if $KERNEL one of 54, netnext; then
Enable any feature that isn't supported on 4.19
if $KERNEL one of netnext; then
Enable any feature that isn't supported on 4.19 and 5.4
(Remember that we currently only support 4.19, 5.4, and netnext as
KERNEL values.)
Signed-off-by: Paul Chaignon <paul@cilium.io>
This commit adds explicit coverage for 5.10 in complexity tests. I'm calling it 'explicit' because it doesn't just run on a 5.10 kernel but also tailors the datapath configuration to that kernel. The 5.15 CI run will also use the datapath configuration for 5.10. Until now, it used the 5.4 configuration which has less features enabled. The features we can enable on 5.10 are the same we can enable on net-next today. That is, we don't have any feature yet that requires 5.11 or newer. The exception would be IPv6 BIG TCP, but that doesn't impact the BPF code. 5.10 and netnext configuration are currently the same. That will change once we start having 5.11+ features. Signed-off-by: Paul Chaignon <paul@cilium.io>
All feature macros should follow the pattern ENABLE_XXX=1. Our is_defined macro in the datapath relies on that pattern so best to use it everywhere. Fixes: ae046b4 ("bpf: Add partial SCTP support.") Signed-off-by: Paul Chaignon <paul@cilium.io>
pchaigno
force-pushed
the
pr/pchaigno/bpf-options-cover-510-515
branch
from
5e13a15
to
d80e94a
Compare
pchaigno
force-pushed
the
pr/pchaigno/bpf-options-cover-510-515
branch
from
d80e94a
to
1aed520
Compare
|
The changes made here are only covered by the verifier workflow (updated link to passing run in OP) and the build datapath job. Both are passing. I don't think we need to wait for the cilium/loader review given we already have three reviews including Maxim from datapath and Tobias who originally ported this code to a workflow. |
pchaigno
added
the
ready-to-merge
Don't we need a focused Jenkins run to cover 4.19 ? |
I wondered the same (and even triggered the test 😅), but no, I don't think so. The 4.19 Jenkins run relies on the configs under |
👍 |
julianwiedmann
pushed a commit
that referenced
this pull request
Mar 29, 2023
I tested with KERNEL=419 and KERNEL=54 and the ENABLE_EGRESS_GATEWAY macro would never be defined. In short, this is caused by an improper use of double quotes. See [1] for the longer explanation. We also update KERNEL's default value to be 'kernel' and not '"kernel"' (quotes were part of the variable). 1 - #24122 (comment) Fixes: d0390ff ("bpf: add wider compile testing for EgressGW") Signed-off-by: Maxim Mikityanskiy <maxim@isovalent.com> Signed-off-by: Paul Chaignon <paul@cilium.io>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary of changes:
make -C bpf kernel=Xcommand.MAX_*_OPTIONSa bit.MAX_*_OPTIONSand in the complexity tests.See commits for details.
Successful run: https://github.com/cilium/cilium/actions/runs/4552745444?pr=24122.