diff --git a/cilium-dbg/cmd/post_uninstall_cleanup.go b/cilium-dbg/cmd/post_uninstall_cleanup.go index b18318725c85e..975787741f9be 100644 --- a/cilium-dbg/cmd/post_uninstall_cleanup.go +++ b/cilium-dbg/cmd/post_uninstall_cleanup.go @@ -555,7 +555,7 @@ func removeXDPAttachments(links []netlink.Link) error { }) for _, link := range links { - if err := loader.DetachXDP(link, bpf.CiliumPath(), "cil_xdp_entry"); err != nil { + if err := loader.DetachXDP(link.Attrs().Name, bpf.CiliumPath(), "cil_xdp_entry"); err != nil { return err } fmt.Printf("removed cilium xdp of %s\n", link.Attrs().Name) diff --git a/pkg/datapath/cells.go b/pkg/datapath/cells.go index f6a9c1bd0b463..b2933a1c138bb 100644 --- a/pkg/datapath/cells.go +++ b/pkg/datapath/cells.go @@ -26,7 +26,6 @@ import ( "github.com/cilium/cilium/pkg/datapath/linux/sysctl" "github.com/cilium/cilium/pkg/datapath/linux/utime" "github.com/cilium/cilium/pkg/datapath/loader" - loaderTypes "github.com/cilium/cilium/pkg/datapath/loader/types" "github.com/cilium/cilium/pkg/datapath/orchestrator" "github.com/cilium/cilium/pkg/datapath/prefilter" "github.com/cilium/cilium/pkg/datapath/tables" @@ -218,7 +217,7 @@ type datapathParams struct { TunnelConfig tunnel.Config - Loader loaderTypes.Loader + Loader types.Loader NodeManager nodeManager.NodeManager diff --git a/pkg/datapath/fake/cells.go b/pkg/datapath/fake/cells.go index 0c01c1f3b16f5..3ae8bcbe3e302 100644 --- a/pkg/datapath/fake/cells.go +++ b/pkg/datapath/fake/cells.go @@ -16,6 +16,7 @@ import ( "github.com/cilium/cilium/pkg/datapath/iptables/ipset" "github.com/cilium/cilium/pkg/datapath/linux/bigtcp" "github.com/cilium/cilium/pkg/datapath/linux/sysctl" + "github.com/cilium/cilium/pkg/datapath/loader" "github.com/cilium/cilium/pkg/datapath/tables" "github.com/cilium/cilium/pkg/datapath/tunnel" "github.com/cilium/cilium/pkg/datapath/types" @@ -54,6 +55,7 @@ var Cell = cell.Module( func() mtu.MTU { return &fakeTypes.MTU{} }, func() *wg.Agent { return nil }, func() types.Loader { return &fakeTypes.FakeLoader{} }, + loader.NewCompilationLock, func() sysctl.Sysctl { return &Sysctl{} }, tables.NewDeviceTable, diff --git a/pkg/datapath/fake/types/datapath.go b/pkg/datapath/fake/types/datapath.go index 9a371bf0e4ce1..63be677ceae7c 100644 --- a/pkg/datapath/fake/types/datapath.go +++ b/pkg/datapath/fake/types/datapath.go @@ -166,6 +166,10 @@ func (f *FakeLoader) RestoreTemplates(stateDir string) error { return nil } +func (f *FakeLoader) DetachXDP(ifaceName string, bpffsBase, progName string) error { + return nil +} + type FakeOrchestrator struct{} func (f *FakeOrchestrator) Reinitialize(ctx context.Context) error { diff --git a/pkg/datapath/linux/datapath.go b/pkg/datapath/linux/datapath.go index d7150449e5978..30ef257a1a5f8 100644 --- a/pkg/datapath/linux/datapath.go +++ b/pkg/datapath/linux/datapath.go @@ -6,7 +6,6 @@ package linux import ( "github.com/cilium/statedb" - loader "github.com/cilium/cilium/pkg/datapath/loader/types" "github.com/cilium/cilium/pkg/datapath/tables" datapath "github.com/cilium/cilium/pkg/datapath/types" "github.com/cilium/cilium/pkg/maps/lbmap" @@ -31,7 +30,7 @@ type linuxDatapath struct { nodeIDHandler datapath.NodeIDHandler nodeNeighbors datapath.NodeNeighbors nodeAddressing datapath.NodeAddressing - loader loader.Loader + loader datapath.Loader wgAgent datapath.WireguardAgent lbmap datapath.LBMap bwmgr datapath.BandwidthManager @@ -46,7 +45,7 @@ type DatapathParams struct { BWManager datapath.BandwidthManager NodeAddressing datapath.NodeAddressing MTU datapath.MTUConfiguration - Loader loader.Loader + Loader datapath.Loader NodeManager manager.NodeManager DB *statedb.DB Devices statedb.Table[*tables.Device] diff --git a/pkg/datapath/loader/cell.go b/pkg/datapath/loader/cell.go index a5482ae632291..eb1ff5c9d91a6 100644 --- a/pkg/datapath/loader/cell.go +++ b/pkg/datapath/loader/cell.go @@ -7,7 +7,7 @@ import ( "github.com/cilium/hive/cell" "github.com/spf13/pflag" - loaderTypes "github.com/cilium/cilium/pkg/datapath/loader/types" + datapath "github.com/cilium/cilium/pkg/datapath/types" ) var Cell = cell.Module( @@ -16,11 +16,11 @@ var Cell = cell.Module( cell.Config(DefaultConfig), cell.Provide(NewLoader), - cell.Provide(newCompilationLock), + cell.Provide(NewCompilationLock), ) // NewLoader returns a new loader. -func NewLoader(p Params) loaderTypes.Loader { +func NewLoader(p Params) datapath.Loader { return newLoader(p) } diff --git a/pkg/datapath/loader/compile.go b/pkg/datapath/loader/compile.go index 28c7074e44a50..4c5d857c0461e 100644 --- a/pkg/datapath/loader/compile.go +++ b/pkg/datapath/loader/compile.go @@ -394,6 +394,6 @@ type compilationLock struct { lock.RWMutex } -func newCompilationLock() types.CompilationLock { +func NewCompilationLock() types.CompilationLock { return &compilationLock{} } diff --git a/pkg/datapath/loader/types/types.go b/pkg/datapath/loader/types/types.go deleted file mode 100644 index 5437f14b2f687..0000000000000 --- a/pkg/datapath/loader/types/types.go +++ /dev/null @@ -1,27 +0,0 @@ -// SPDX-License-Identifier: Apache-2.0 -// Copyright Authors of Cilium - -package types - -import ( - "context" - - "github.com/vishvananda/netlink" - - "github.com/cilium/cilium/pkg/datapath/loader/metrics" - "github.com/cilium/cilium/pkg/datapath/tunnel" - "github.com/cilium/cilium/pkg/datapath/types" -) - -type Loader interface { - CallsMapPath(id uint16) string - CustomCallsMapPath(id uint16) string - DetachXDP(iface netlink.Link, bpffsBase, progName string) error - EndpointHash(cfg types.EndpointConfiguration) (string, error) - HostDatapathInitialized() <-chan struct{} - Reinitialize(ctx context.Context, tunnelConfig tunnel.Config, deviceMTU int, iptMgr types.IptablesManager, p types.Proxy) error - ReinitializeXDP(ctx context.Context, extraCArgs []string) error - ReloadDatapath(ctx context.Context, ep types.Endpoint, stats *metrics.SpanStat) (err error) - RestoreTemplates(stateDir string) error - Unload(ep types.Endpoint) -} diff --git a/pkg/datapath/loader/xdp.go b/pkg/datapath/loader/xdp.go index 443e89325d6f6..a262428f0b219 100644 --- a/pkg/datapath/loader/xdp.go +++ b/pkg/datapath/loader/xdp.go @@ -86,7 +86,7 @@ func (l *loader) maybeUnloadObsoleteXDPPrograms(xdpDevs []string, xdpMode, bpffs } } if !used { - if err := l.DetachXDP(link, bpffsBase, symbolFromHostNetdevXDP); err != nil { + if err := l.DetachXDP(link.Attrs().Name, bpffsBase, symbolFromHostNetdevXDP); err != nil { log.WithError(err).Warn("Failed to detach obsolete XDP program") } } @@ -276,9 +276,14 @@ func attachXDPProgram(iface netlink.Link, prog *ebpf.Program, progName, bpffsDir // // bpffsBase is typically /sys/fs/bpf/cilium, but can be overridden to a tempdir // during tests. -func (l *loader) DetachXDP(iface netlink.Link, bpffsBase, progName string) error { +func (l *loader) DetachXDP(ifaceName string, bpffsBase, progName string) error { + iface, err := netlink.LinkByName(ifaceName) + if err != nil { + return fmt.Errorf("getting link '%s' by name: %w", ifaceName, err) + } + pin := filepath.Join(bpffsDeviceLinksDir(bpffsBase, iface), progName) - err := bpf.UnpinLink(pin) + err = bpf.UnpinLink(pin) if err == nil { return nil } @@ -287,12 +292,6 @@ func (l *loader) DetachXDP(iface netlink.Link, bpffsBase, progName string) error return fmt.Errorf("unpinning XDP program using bpf_link: %w", err) } - // Refresh the link info since the caller may have constructed iface manually. - iface, err = netlink.LinkByName(iface.Attrs().Name) - if err != nil { - return fmt.Errorf("getting link '%s' by name: %w", iface.Attrs().Name, err) - } - xdp := iface.Attrs().Xdp if xdp == nil || !xdp.Attached { return nil diff --git a/pkg/datapath/loader/xdp_test.go b/pkg/datapath/loader/xdp_test.go index 29cab00c8d103..cd3a1c88d0c90 100644 --- a/pkg/datapath/loader/xdp_test.go +++ b/pkg/datapath/loader/xdp_test.go @@ -183,7 +183,7 @@ func TestAttachXDPWithExistingLink(t *testing.T) { require.NoError(t, err) // Detach the program. - err = newTestLoader(t).DetachXDP(veth, basePath, "test") + err = newTestLoader(t).DetachXDP(veth.Attrs().Name, basePath, "test") require.NoError(t, err) err = netlink.LinkDel(veth) @@ -214,11 +214,11 @@ func TestDetachXDPWithPreviousAttach(t *testing.T) { require.True(t, getLink(t, veth).Attrs().Xdp.Attached) // Detach with the wrong name, leaving the program attached. - err = newTestLoader(t).DetachXDP(veth, basePath, "foo") + err = newTestLoader(t).DetachXDP(veth.Attrs().Name, basePath, "foo") require.NoError(t, err) require.True(t, getLink(t, veth).Attrs().Xdp.Attached) - err = newTestLoader(t).DetachXDP(veth, basePath, "test") + err = newTestLoader(t).DetachXDP(veth.Attrs().Name, basePath, "test") require.NoError(t, err) require.False(t, getLink(t, veth).Attrs().Xdp.Attached) diff --git a/pkg/datapath/orchestrator/orchestrator.go b/pkg/datapath/orchestrator/orchestrator.go index e0cb72762bcc0..003ca058531b9 100644 --- a/pkg/datapath/orchestrator/orchestrator.go +++ b/pkg/datapath/orchestrator/orchestrator.go @@ -9,8 +9,8 @@ import ( "github.com/cilium/hive/cell" "github.com/cilium/cilium/pkg/datapath/iptables" - "github.com/cilium/cilium/pkg/datapath/loader/types" "github.com/cilium/cilium/pkg/datapath/tunnel" + "github.com/cilium/cilium/pkg/datapath/types" "github.com/cilium/cilium/pkg/mtu" "github.com/cilium/cilium/pkg/proxy" ) diff --git a/pkg/datapath/types/loader.go b/pkg/datapath/types/loader.go index 69f4d026897d6..5283c8f881fe1 100644 --- a/pkg/datapath/types/loader.go +++ b/pkg/datapath/types/loader.go @@ -24,6 +24,7 @@ type Loader interface { Reinitialize(ctx context.Context, tunnelConfig tunnel.Config, deviceMTU int, iptMgr IptablesManager, p Proxy) error HostDatapathInitialized() <-chan struct{} RestoreTemplates(stateDir string) error + DetachXDP(iface string, bpffsBase, progName string) error } // PreFilter an interface for an XDP pre-filter. diff --git a/test/controlplane/suite/agent.go b/test/controlplane/suite/agent.go index ea09f92e14456..509a9e6b8f040 100644 --- a/test/controlplane/suite/agent.go +++ b/test/controlplane/suite/agent.go @@ -18,7 +18,6 @@ import ( fakecni "github.com/cilium/cilium/daemon/cmd/cni/fake" fakeDatapath "github.com/cilium/cilium/pkg/datapath/fake" fakeTypes "github.com/cilium/cilium/pkg/datapath/fake/types" - "github.com/cilium/cilium/pkg/datapath/loader" "github.com/cilium/cilium/pkg/datapath/prefilter" datapathTables "github.com/cilium/cilium/pkg/datapath/tables" fqdnproxy "github.com/cilium/cilium/pkg/fqdn/proxy" @@ -78,7 +77,6 @@ func (h *agentHandle) setupCiliumAgentHive(clientset k8sClient.Clientset, extraC ), fakeDatapath.Cell, prefilter.Cell, - loader.Cell, monitorAgent.Cell, metrics.Cell, store.Cell,