datapath: configuration using type-safe Go structs#36991
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.
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.
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
ELFVariableSubstitutionsand 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.hwith this mechanism by embedding astruct Nodeinto each configuration object.Fixes: #30893