Skip to content

Commit d296e0d

Browse files
ti-moti-mo
committed
bpf: configure datapath using generated Go structs
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>
1 parent d0c87d2 commit d296e0d

9 files changed

Lines changed: 156 additions & 348 deletions

File tree

‎pkg/datapath/linux/config/config.go‎

Lines changed: 2 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -976,61 +976,7 @@ func (h *HeaderfileWriter) WriteNetdevConfig(w io.Writer, opts *option.IntOption
976976

977977
// writeStaticData writes the endpoint-specific static data defines to the
978978
// specified writer. This must be kept in sync with loader.ELFSubstitutions().
979-
func (h *HeaderfileWriter) writeStaticData(devices []string, fw io.Writer, e datapath.EndpointConfiguration) {
980-
if e.IsHost() {
981-
if option.Config.EnableBPFMasquerade {
982-
if option.Config.EnableIPv4Masquerade {
983-
// NodePort comment above applies to IPV4_MASQUERADE too
984-
placeholderIPv4 := []byte{1, 1, 1, 1}
985-
fmt.Fprint(fw, defineIPv4("IPV4_MASQUERADE", placeholderIPv4))
986-
}
987-
if option.Config.EnableIPv6Masquerade {
988-
// NodePort comment above applies to IPV6_MASQUERADE too
989-
placeholderIPv6 := []byte{1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1}
990-
fmt.Fprint(fw, defineIPv6("IPV6_MASQUERADE", placeholderIPv6))
991-
}
992-
}
993-
// Dummy value to avoid being optimized when 0
994-
fmt.Fprint(fw, defineUint32("SECCTX_FROM_IPCACHE", 1))
995-
996-
// Use templating for ETH_HLEN only if there is any L2-less device
997-
if !mac.HaveMACAddrs(devices) {
998-
// L2 hdr len (for L2-less devices it will be replaced with "0")
999-
fmt.Fprint(fw, defineUint16("ETH_HLEN", mac.EthHdrLen))
1000-
}
1001-
} else {
1002-
// We want to ensure that the template BPF program always has "LXC_IP"
1003-
// defined and present as a symbol in the resulting object file after
1004-
// compilation, regardless of whether IPv6 is disabled. Because the type
1005-
// templateCfg hardcodes a dummy IPv6 address (and adheres to the
1006-
// datapath.EndpointConfiguration interface), we can rely on it always
1007-
// having an IPv6 addr. Endpoints however may not have IPv6 addrs if IPv6
1008-
// is disabled. Hence this check prevents us from omitting the "LXC_IP"
1009-
// symbol from the template BPF program. Without this, the following
1010-
// scenario is possible:
1011-
// 1) Enable IPv6 in cilium
1012-
// 2) Create an endpoint (ensure endpoint has an IPv6 addr)
1013-
// 3) Disable IPv6 and restart cilium
1014-
// This results in a template BPF object without an "LXC_IP" defined,
1015-
// __but__ the endpoint still has "LXC_IP" defined. This causes a later
1016-
// call to loader.ELFSubstitutions() to fail on missing a symbol "LXC_IP".
1017-
if ipv6 := e.IPv6Address(); ipv6.IsValid() {
1018-
fmt.Fprint(fw, defineIPv6("LXC_IP", ipv6.AsSlice()))
1019-
}
1020-
1021-
fmt.Fprint(fw, defineIPv4("LXC_IPV4", e.IPv4Address().AsSlice()))
1022-
fmt.Fprint(fw, defineUint16("LXC_ID", uint16(e.GetID())))
1023-
}
1024-
1025-
fmt.Fprint(fw, defineMAC("THIS_INTERFACE_MAC", e.GetNodeMAC()))
1026-
fmt.Fprint(fw, defineUint64("ENDPOINT_NETNS_COOKIE", uint64(e.GetEndpointNetNsCookie())))
1027-
1028-
secID := e.GetIdentityLocked().Uint32()
1029-
fmt.Fprint(fw, defineUint32("SECLABEL", secID))
1030-
fmt.Fprintln(fw, "#define SECLABEL_IPV4 SECLABEL")
1031-
fmt.Fprintln(fw, "#define SECLABEL_IPV6 SECLABEL")
1032-
fmt.Fprint(fw, defineUint32("POLICY_VERDICT_LOG_FILTER", e.GetPolicyVerdictLogFilter()))
1033-
979+
func (h *HeaderfileWriter) writeStaticData(fw io.Writer, e datapath.EndpointConfiguration) {
1034980
epID := uint16(e.GetID())
1035981
fmt.Fprintf(fw, "#define POLICY_MAP %s\n", bpf.LocalMapName(policymap.MapName, epID))
1036982
callsMapName := callsmap.MapName
@@ -1055,7 +1001,7 @@ func (h *HeaderfileWriter) WriteEndpointConfig(w io.Writer, cfg *datapath.LocalN
10551001
}
10561002

10571003
writeIncludes(w)
1058-
h.writeStaticData(deviceNames, fw, e)
1004+
h.writeStaticData(fw, e)
10591005

10601006
return h.writeTemplateConfig(fw, deviceNames, cfg.HostEndpointID, e, cfg.DirectRoutingDevice)
10611007
}

‎pkg/datapath/linux/config/config_test.go‎

Lines changed: 1 addition & 105 deletions
Original file line numberDiff line numberDiff line change
@@ -125,111 +125,14 @@ func TestWriteNetdevConfig(t *testing.T) {
125125
})
126126
}
127127

128-
func TestWriteEndpointConfig(t *testing.T) {
129-
writeConfig(t, "endpoint", func(w io.Writer, dp datapath.ConfigWriter) error {
130-
return dp.WriteEndpointConfig(w, &dummyNodeCfg, &dummyEPCfg)
131-
})
132-
133-
// Create copy of config option so that it can be restored at the end of
134-
// this test. In the future, we'd like to parallelize running unit tests.
135-
// As it stands, this test would not be ready to parallelize until we
136-
// remove our dependency on globals (e.g. option.Config).
137-
oldEnableIPv6 := option.Config.EnableIPv6
138-
defer func() {
139-
option.Config.EnableIPv6 = oldEnableIPv6
140-
}()
141-
142-
testRun := func(te *testutils.TestEndpoint) ([]byte, map[string]uint64) {
143-
cfg := &HeaderfileWriter{}
144-
varSub := loader.ELFVariableSubstitutions(te)
145-
146-
var buf bytes.Buffer
147-
cfg.writeStaticData(nil, &buf, te)
148-
149-
return buf.Bytes(), varSub
150-
}
151-
152-
lxcIPs := []string{"LXC_IP_1", "LXC_IP_2"}
153-
154-
tests := []struct {
155-
description string
156-
template testutils.TestEndpoint // Represents template bpf prog
157-
endpoint testutils.TestEndpoint // Represents normal endpoint bpf prog
158-
preTestRun func(t *testutils.TestEndpoint, e *testutils.TestEndpoint)
159-
templateExp bool
160-
endpointExp bool
161-
}{
162-
{
163-
description: "IPv6 is disabled, endpoint does not have an IPv6 addr",
164-
template: testutils.NewTestEndpoint(),
165-
endpoint: testutils.NewTestEndpoint(),
166-
preTestRun: func(t *testutils.TestEndpoint, e *testutils.TestEndpoint) {
167-
option.Config.EnableIPv6 = false
168-
t.IPv6 = ipv6DummyAddr // Template bpf prog always has dummy IPv6
169-
e.IPv6 = netip.Addr{} // This endpoint does not have an IPv6 addr
170-
},
171-
templateExp: true,
172-
endpointExp: false,
173-
},
174-
{
175-
description: "IPv6 is disabled, endpoint does have an IPv6 addr",
176-
template: testutils.NewTestEndpoint(),
177-
endpoint: testutils.NewTestEndpoint(),
178-
preTestRun: func(t *testutils.TestEndpoint, e *testutils.TestEndpoint) {
179-
option.Config.EnableIPv6 = false
180-
t.IPv6 = ipv6DummyAddr // Template bpf prog always has dummy IPv6
181-
e.IPv6 = ipv6DummyAddr // This endpoint does have an IPv6 addr
182-
},
183-
templateExp: true,
184-
endpointExp: true,
185-
},
186-
{
187-
description: "IPv6 is enabled",
188-
template: testutils.NewTestEndpoint(),
189-
endpoint: testutils.NewTestEndpoint(),
190-
preTestRun: func(t *testutils.TestEndpoint, e *testutils.TestEndpoint) {
191-
option.Config.EnableIPv6 = true
192-
t.IPv6 = ipv6DummyAddr
193-
e.IPv6 = ipv6DummyAddr
194-
},
195-
templateExp: true,
196-
endpointExp: true,
197-
},
198-
{
199-
description: "IPv6 is enabled, endpoint does not have IPv6 address",
200-
template: testutils.NewTestEndpoint(),
201-
endpoint: testutils.NewTestEndpoint(),
202-
preTestRun: func(t *testutils.TestEndpoint, e *testutils.TestEndpoint) {
203-
option.Config.EnableIPv6 = true
204-
t.IPv6 = ipv6DummyAddr
205-
e.IPv6 = netip.Addr{}
206-
},
207-
templateExp: true,
208-
endpointExp: false,
209-
},
210-
}
211-
for _, test := range tests {
212-
t.Logf("Testing %s", test.description)
213-
test.preTestRun(&test.template, &test.endpoint)
214-
215-
b, vsub := testRun(&test.template)
216-
require.Equal(t, test.templateExp, bytes.Contains(b, []byte("DEFINE_IPV6")))
217-
assertKeysInsideMap(t, vsub, lxcIPs, test.templateExp)
218-
219-
b, vsub = testRun(&test.endpoint)
220-
require.Equal(t, test.endpointExp, bytes.Contains(b, []byte("DEFINE_IPV6")))
221-
assertKeysInsideMap(t, vsub, lxcIPs, test.endpointExp)
222-
}
223-
}
224-
225128
func TestWriteStaticData(t *testing.T) {
226129
cfg := &HeaderfileWriter{}
227130
ep := &dummyEPCfg
228131

229132
mapSub := loader.ELFMapSubstitutions(ep)
230133

231134
var buf bytes.Buffer
232-
cfg.writeStaticData(nil, &buf, ep)
135+
cfg.writeStaticData(&buf, ep)
233136
b := buf.Bytes()
234137

235138
for _, v := range mapSub {
@@ -238,13 +141,6 @@ func TestWriteStaticData(t *testing.T) {
238141
}
239142
}
240143

241-
func assertKeysInsideMap(t *testing.T, m map[string]uint64, keys []string, want bool) {
242-
for _, v := range keys {
243-
_, ok := m[v]
244-
require.Equal(t, want, ok)
245-
}
246-
}
247-
248144
func createMainLink(name string, t *testing.T) *netlink.Dummy {
249145
link := &netlink.Dummy{
250146
LinkAttrs: netlink.LinkAttrs{

‎pkg/datapath/linux/config/utils.go‎

Lines changed: 0 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import (
77
"fmt"
88
"net"
99

10-
"github.com/cilium/cilium/pkg/byteorder"
1110
"github.com/cilium/cilium/pkg/common"
1211
)
1312

@@ -18,33 +17,6 @@ func FmtDefineAddress(name string, addr []byte) string {
1817
return fmt.Sprintf("#define %s { .addr = { %s } }\n", name, common.GoArray2C(addr))
1918
}
2019

21-
// defineUint16 writes the C definition for an unsigned 16-bit value.
22-
func defineUint16(name string, value uint16) string {
23-
return fmt.Sprintf("DEFINE_U16(%s, %#04x);\t/* %d */\n#define %s fetch_u16(%s)\n",
24-
name, value, value, name, name)
25-
}
26-
27-
// defineUint32 writes the C definition for an unsigned 32-bit value.
28-
func defineUint32(name string, value uint32) string {
29-
return fmt.Sprintf("DEFINE_U32(%s, %#08x);\t/* %d */\n#define %s fetch_u32(%s)\n",
30-
name, value, value, name, name)
31-
}
32-
33-
// defineUint64 writes the C definition for an unsigned 64-bit value.
34-
func defineUint64(name string, value uint64) string {
35-
return fmt.Sprintf("DEFINE_U64(%s, %#016x);\t/* %d */\n#define %s fetch_u64(%s)\n",
36-
name, value, value, name, name)
37-
}
38-
39-
// defineIPv4 writes the C definition for the given IPv4 address.
40-
func defineIPv4(name string, addr []byte) string {
41-
if len(addr) != net.IPv4len {
42-
return fmt.Sprintf("/* BUG: bad ip define %s %s */\n", name, common.GoArray2C(addr))
43-
}
44-
nboAddr := byteorder.NetIPv4ToHost32(addr)
45-
return defineUint32(name, nboAddr)
46-
}
47-
4820
// defineIPv6 writes the C definition for the given IPv6 address.
4921
func defineIPv6(name string, addr []byte) string {
5022
if len(addr) != net.IPv6len {
@@ -58,15 +30,6 @@ func dumpRaw(name string, addr []byte) string {
5830
return fmt.Sprintf(" %s%s\n", name, common.GoArray2C(addr))
5931
}
6032

61-
// defineMAC writes the C definition for the given MAC name and addr.
62-
func defineMAC(name string, addr []byte) string {
63-
if len(addr) != 6 { /* MAC len */
64-
return fmt.Sprintf("/* BUG: bad mac define %s %s */\n", name, common.GoArray2C(addr))
65-
}
66-
return fmt.Sprintf("DEFINE_MAC(%s, %s);\n#define %s fetch_mac(%s)\n",
67-
name, common.GoArray2C(addr), name, name)
68-
}
69-
7033
// declareConfig writes the C macro for declaring a global configuration variable that can be
7134
// modified at runtime.
7235
func declareConfig(name string, value any, description string) string {

‎pkg/datapath/linux/config/utils_test.go‎

Lines changed: 0 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -39,28 +39,3 @@ func TestDefineIPv6(t *testing.T) {
3939
require.Equal(t, test.output, defineIPv6("foo", test.input))
4040
}
4141
}
42-
43-
func TestDefineMAC(t *testing.T) {
44-
tests := []formatTestCase{
45-
{
46-
input: nil,
47-
output: "/* BUG: bad mac define foo */\n",
48-
},
49-
{
50-
input: []byte{},
51-
output: "/* BUG: bad mac define foo */\n",
52-
},
53-
{
54-
input: []byte{1, 2, 3},
55-
output: "/* BUG: bad mac define foo 0x1, 0x2, 0x3 */\n",
56-
},
57-
{
58-
input: []byte{1, 2, 3, 4, 5, 6},
59-
output: "DEFINE_MAC(foo, 0x1, 0x2, 0x3, 0x4, 0x5, 0x6);\n" +
60-
"#define foo fetch_mac(foo)\n",
61-
},
62-
}
63-
for _, test := range tests {
64-
require.Equal(t, test.output, defineMAC("foo", test.input))
65-
}
66-
}

‎pkg/datapath/loader/base.go‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ import (
3030
"github.com/cilium/cilium/pkg/identity"
3131
ipamOption "github.com/cilium/cilium/pkg/ipam/option"
3232
"github.com/cilium/cilium/pkg/logging/logfields"
33-
"github.com/cilium/cilium/pkg/mac"
3433
"github.com/cilium/cilium/pkg/option"
3534
"github.com/cilium/cilium/pkg/socketlb"
3635
wgTypes "github.com/cilium/cilium/pkg/wireguard/types"
@@ -265,7 +264,6 @@ func reinitializeOverlay(ctx context.Context, tunnelConfig tunnel.Config) error
265264

266265
// gather compile options for bpf_overlay.c
267266
opts := []string{
268-
fmt.Sprintf("-DTHIS_INTERFACE_MAC={.addr=%s}", mac.CArrayString(link.Attrs().HardwareAddr)),
269267
fmt.Sprintf("-DCALLS_MAP=cilium_calls_overlay_%d", identity.ReservedIdentityWorld),
270268
}
271269
if option.Config.EnableNodePort {
@@ -295,7 +293,6 @@ func reinitializeWireguard(ctx context.Context) (err error) {
295293
}
296294

297295
opts := []string{
298-
fmt.Sprintf("-DTHIS_INTERFACE_MAC={.addr=%s}", mac.CArrayString(link.Attrs().HardwareAddr)),
299296
fmt.Sprintf("-DCALLS_MAP=cilium_calls_wireguard_%d", identity.ReservedIdentityWorld),
300297
}
301298

0 commit comments

Comments
 (0)