-
Notifications
You must be signed in to change notification settings - Fork 3.4k
datapath: configuration using type-safe Go structs #36991
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.
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
Conversation
/test |
8153ffd
to
f742683
Compare
/test |
f742683
to
49bfbb7
Compare
/test |
dd77405
to
abb5c15
Compare
/test |
abb5c15
to
095e4bb
Compare
/test |
095e4bb
to
7ebf7a4
Compare
/test |
7ebf7a4
to
9f09023
Compare
/test |
9f09023
to
a5676e6
Compare
/test |
1 similar comment
/test |
2ce46df
to
c6de9dd
Compare
/test |
c6de9dd
to
af705fb
Compare
/test |
af705fb
to
db4dcfe
Compare
/test |
db4dcfe
to
5fce5b5
Compare
A follow-up commit will turn this constant into a runtime-provided constant. Remove it from complexity tests in a separate commit to make the diff a little easier to grok. Signed-off-by: Timo Beckers <timo@isovalent.com>
Moved the following variables: - SECCTX_FROM_IPCACHE: in bpf_host.c instead of conditionally in node_config.h, no longer defaults to 1 in some cases - ETH_HLEN: in bpf_host.c instead of writeStaticData() - LXC_IP: ep_config.h -> config/endpoint.h, no longer defaults to beef:.. - LXC_IPV4: ep_config.h -> config/endpoint.h, no longer defaults to 1.2.3.4 - LXC_ID: ep_config.h -> config/endpoint.h, no longer defaults to 0x2a - SECLABEL: ep_config.h -> config/endpoint.h, no longer defaults to 0xfffff - POLICY_VERDICT_LOG_FILTER: single definition in lib/policy_log.h, no longer defaults to 0xffff - ENDPOINT_NETNS_COOKIE: ep_config.h -> config/lxc.h, no longer defaults to maxuint32 - THIS_MTU: lib/common.h -> lib/nodeport.h Signed-off-by: Timo Beckers <timo@isovalent.com>
Moved the following variables: - THIS_INTERFACE_MAC: node_config.h -> config/global.h - THIS_INTERFACE_IFINDEX: node_config.h -> config/global.h - IPV4_MASQUERADE: lib/stubs.h -> lib/nat.h - IPV6_MASQUERADE: lib/stubs.h -> lib/nat.h - ROUTER_IP: temporary hack to prevent it from showing up in Go codegen, still rendered into node_config.h by the agent - HOST_IP: only used in tests, moved out of node_config.h and renamed to NODE_IPV6 to avoid confusion Signed-off-by: Timo Beckers <timo@isovalent.com>
…figs This commit makes CollectionOptions.Constants take an annotated struct generated by dpgen instead of the previous string map. Uses config.StructToMap to convert the struct to a map to apply to the CollectionSpec before loading. Signed-off-by: Timo Beckers <timo@isovalent.com>
This commit invokes dpgen during agent build to generate configuration scaffoldings for all of Cilium's bpf objects. It also sets up DECLARE_CONFIG to emit config values with the kind:object decl tag so dpgen can pick them out when generating structs. Signed-off-by: Timo Beckers <timo@isovalent.com>
This commit removes ELFVariableSubstitutions and finally chops up the pipeline into separate code paths for tailoring objects to specific devices and endpoints. Instead of the typical map[string]any, bpf.LoadAndAssign now takes an annotated struct of config values to apply to the object, making the provenance of a value easier to find by playing nice with editor tooling, as well as being completely type-safe from here on out. In the process, we figured out exactly which values need to be injected into which objects to avoid dead code and general surprising behaviour. Signed-off-by: Timo Beckers <timo@isovalent.com>
Signed-off-by: Timo Beckers <timo@isovalent.com>
A previous commit introduced the ciliumHostRewrites and endpointRewrites functions to obtain a configuration object to feed into the bpf loader at load time. With most of the ELF templating logic gone and compile-time values being migrated to load-time values, the endpoint regeneration logic was no longer responding to policy changes, specifically due to the security label now being a load-time config. This commit includes contents of host and workload endpoint rewrites in the endpoint hash. Since EndpointHash() now blocks when the endpoint write lock is held, its call had to be moved out of the locked context in runPreCompilationSteps, after the secID has been updated. Signed-off-by: Timo Beckers <timo@isovalent.com>
Prior commits made these helpers unused. Remove them. Signed-off-by: Timo Beckers <timo@isovalent.com>
b54cceb
to
b4abbf2
Compare
/test |
@smagnani96 Checkpatch complains about line continuations in a macro, that's hard to avoid, same for the long line warning, since we have some long strings in there. The sudo failure was due to #37736. |
return "", fmt.Errorf("hashing host rewrites: %w", err) | ||
} | ||
} else { | ||
cfg, _ := endpointRewrites(epCfg) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this generate a different hash for each Endpoint on the node, meaning that every hash is unique so we now have to compile the datapath once for every endpoint rather than once for all endpoints?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Short answer with recent versions is that we still compile the datapath once for all workload endpoints (+ once for host, as expected). I guess the abstraction has shifted recently. I wonder whether we can just completely remove the EndpointHash(..)
logic then.
This turned out to be a large patch set, but I've narrowed things down as much as I could and sent a few prerequisites in another PR. Please see individual commits for more detailed explanation and reasoning.
At its core, this PR removes
ELFVariableSubstitutions
and aims to establish a single declaration for each datapath config value, making them all runtime-provided in the process.Currently, this only addresses endpoint-specific configuration. A follow-up PR will introduce the equivalent for gradually replacing
node_config.h
with this mechanism by embedding astruct Node
into each configuration object.Fixes: #30893