loader: aggregate replaceDatapath arguments

The arguments to the replaceDatapath functions are already quite numerous
and make the function signature hard to read. In preparation for future
commits, this patch aggregates almost all arguments to the function into
one option parameter.

Signed-off-by: Robin Gögge <r.goegge@isovalent.com>

pkg/datapath/loader/base.go

@@ -209,7 +209,13 @@ func (l *loader) reinitializeIPSec(ctx context.Context) error {
 			log.WithError(err).WithField(logfields.Interface, iface).Warn("Rpfilter could not be disabled, node to node encryption may fail")
 		}
 
-		finalize, err := replaceDatapath(ctx, iface, networkObj, progs, "")
+		finalize, err := replaceDatapath(ctx,
+			replaceDatapathOptions{
+				device:   iface,
+				elf:      networkObj,
+				programs: progs,
+			},
+		)
 		if err != nil {
 			log.WithField(logfields.Interface, iface).WithError(err).Error("Load encryption network failed")
 			// collect errors, but keep trying replacing other interfaces.

pkg/datapath/loader/loader.go

@@ -388,7 +388,13 @@ func (l *loader) reloadHostDatapath(ctx context.Context, ep datapath.Endpoint, o
 		{progName: symbolToHostEp, direction: dirIngress},
 		{progName: symbolFromHostEp, direction: dirEgress},
 	}
-	finalize, err := replaceDatapath(ctx, ep.InterfaceName(), objPath, progs, "")
+	finalize, err := replaceDatapath(ctx,
+		replaceDatapathOptions{
+			device:   ep.InterfaceName(),
+			elf:      objPath,
+			programs: progs,
+		},
+	)
 	if err != nil {
 		scopedLog := ep.Logger(subsystem).WithFields(logrus.Fields{
 			logfields.Path: objPath,
@@ -420,7 +426,13 @@ func (l *loader) reloadHostDatapath(ctx context.Context, ep datapath.Endpoint, o
 		{progName: symbolToHostEp, direction: dirIngress},
 	}
 
-	finalize, err = replaceDatapath(ctx, defaults.SecondHostDevice, secondDevObjPath, progs, "")
+	finalize, err = replaceDatapath(ctx,
+		replaceDatapathOptions{
+			device:   defaults.SecondHostDevice,
+			elf:      secondDevObjPath,
+			programs: progs,
+		},
+	)
 	if err != nil {
 		scopedLog := ep.Logger(subsystem).WithFields(logrus.Fields{
 			logfields.Path: objPath,
@@ -465,7 +477,13 @@ func (l *loader) reloadHostDatapath(ctx context.Context, ep datapath.Endpoint, o
 			}
 		}
 
-		finalize, err := replaceDatapath(ctx, device, netdevObjPath, progs, "")
+		finalize, err := replaceDatapath(ctx,
+			replaceDatapathOptions{
+				device:   device,
+				elf:      netdevObjPath,
+				programs: progs,
+			},
+		)
 		if err != nil {
 			scopedLog := ep.Logger(subsystem).WithFields(logrus.Fields{
 				logfields.Path: objPath,
@@ -514,7 +532,13 @@ func (l *loader) reloadDatapath(ctx context.Context, ep datapath.Endpoint, dirs
 			}
 		}
 
-		finalize, err := replaceDatapath(ctx, ep.InterfaceName(), objPath, progs, "")
+		finalize, err := replaceDatapath(ctx,
+			replaceDatapathOptions{
+				device:   ep.InterfaceName(),
+				elf:      objPath,
+				programs: progs,
+			},
+		)
 		if err != nil {
 			scopedLog := ep.Logger(subsystem).WithFields(logrus.Fields{
 				logfields.Path: objPath,
@@ -560,7 +584,13 @@ func (l *loader) replaceOverlayDatapath(ctx context.Context, cArgs []string, ifa
 		{progName: symbolToOverlay, direction: dirEgress},
 	}
 
-	finalize, err := replaceDatapath(ctx, iface, overlayObj, progs, "")
+	finalize, err := replaceDatapath(ctx,
+		replaceDatapathOptions{
+			device:   iface,
+			elf:      overlayObj,
+			programs: progs,
+		},
+	)
 	if err != nil {
 		log.WithField(logfields.Interface, iface).WithError(err).Fatal("Load overlay network failed")
 	}

pkg/datapath/loader/loader_test.go

@@ -184,11 +184,24 @@ func (s *LoaderTestSuite) TestReload(c *C) {
 		{progName: symbolFromEndpoint, direction: dirIngress},
 		{progName: symbolToEndpoint, direction: dirEgress},
 	}
-	finalize, err := replaceDatapath(ctx, ep.InterfaceName(), objPath, progs, "")
+	finalize, err := replaceDatapath(ctx,
+		replaceDatapathOptions{
+			device:   ep.InterfaceName(),
+			elf:      objPath,
+			programs: progs,
+		},
+	)
 	c.Assert(err, IsNil)
 	finalize()
 
-	finalize, err = replaceDatapath(ctx, ep.InterfaceName(), objPath, progs, "")
+	finalize, err = replaceDatapath(ctx,
+		replaceDatapathOptions{
+			device:   ep.InterfaceName(),
+			elf:      objPath,
+			programs: progs,
+		},
+	)
+
 	c.Assert(err, IsNil)
 	finalize()
 }
@@ -333,7 +346,13 @@ func BenchmarkReplaceDatapath(b *testing.B) {
 	progs := []progDefinition{{progName: symbolFromEndpoint, direction: dirIngress}}
 	b.ResetTimer()
 	for i := 0; i < b.N; i++ {
-		finalize, err := replaceDatapath(ctx, ep.InterfaceName(), objPath, progs, "")
+		finalize, err := replaceDatapath(ctx,
+			replaceDatapathOptions{
+				device:   ep.InterfaceName(),
+				elf:      objPath,
+				programs: progs,
+			},
+		)
 		if err != nil {
 			b.Fatal(err)
 		}

pkg/datapath/loader/netlink.go

@@ -61,6 +61,13 @@ type progDefinition struct {
 	direction string
 }
 
+type replaceDatapathOptions struct {
+	device   string           // name of the netlink interface we attach to
+	elf      string           // path to object file
+	programs []progDefinition // programs that we want to attach/replace
+	xdpMode  string           // XDP driver mode, only applies when attaching XDP programs
+}
+
 // replaceDatapath replaces the qdisc and BPF program for an endpoint or XDP program.
 //
 // When successful, returns a finalizer to allow the map cleanup operation to be
@@ -74,25 +81,25 @@ type progDefinition struct {
 // For example, this is the case with from-netdev and to-netdev. If eth0:to-netdev
 // gets its program and maps replaced and unpinned, its eth0:from-netdev counterpart
 // will miss tail calls (and drop packets) until it has been replaced as well.
-func replaceDatapath(ctx context.Context, ifName, objPath string, progs []progDefinition, xdpMode string) (_ func(), err error) {
+func replaceDatapath(ctx context.Context, opts replaceDatapathOptions) (_ func(), err error) {
 	// Avoid unnecessarily loading a prog.
 	if err := ctx.Err(); err != nil {
 		return nil, err
 	}
 
-	link, err := netlink.LinkByName(ifName)
+	link, err := netlink.LinkByName(opts.device)
 	if err != nil {
-		return nil, fmt.Errorf("getting interface %s by name: %w", ifName, err)
+		return nil, fmt.Errorf("getting interface %s by name: %w", opts.device, err)
 	}
 
-	l := log.WithField("device", ifName).WithField("objPath", objPath).
+	l := log.WithField("device", opts.device).WithField("objPath", opts.elf).
 		WithField("ifindex", link.Attrs().Index)
 
 	// Load the ELF from disk.
 	l.Debug("Loading CollectionSpec from ELF")
-	spec, err := bpf.LoadCollectionSpec(objPath)
+	spec, err := bpf.LoadCollectionSpec(opts.elf)
 	if err != nil {
-		return nil, fmt.Errorf("loading eBPF ELF: %w", err)
+		return nil, fmt.Errorf("loading eBPF ELF %s: %w", opts.elf, err)
 	}
 
 	revert := func() {
@@ -103,7 +110,7 @@ func replaceDatapath(ctx context.Context, ifName, objPath string, progs []progDe
 		}
 	}
 
-	for _, prog := range progs {
+	for _, prog := range opts.programs {
 		if spec.Programs[prog.progName] == nil {
 			return nil, fmt.Errorf("no program %s found in eBPF ELF", prog.progName)
 		}
@@ -154,14 +161,14 @@ func replaceDatapath(ctx context.Context, ifName, objPath string, progs []progDe
 	// bpffs in the process.
 	finalize := func() {}
 	pinPath := bpf.TCGlobalsPath()
-	opts := ebpf.CollectionOptions{
+	collOpts := ebpf.CollectionOptions{
 		Maps: ebpf.MapOptions{PinPath: pinPath},
 	}
 	if err := bpf.MkdirBPF(pinPath); err != nil {
 		return nil, fmt.Errorf("creating bpffs pin path: %w", err)
 	}
 	l.Debug("Loading Collection into kernel")
-	coll, err := bpf.LoadCollection(spec, opts)
+	coll, err := bpf.LoadCollection(spec, collOpts)
 	if errors.Is(err, ebpf.ErrMapIncompatible) {
 		// Temporarily rename bpffs pins of maps whose definitions have changed in
 		// a new version of a datapath ELF.
@@ -179,7 +186,7 @@ func replaceDatapath(ctx context.Context, ifName, objPath string, progs []progDe
 
 		// Retry loading the Collection after starting map migration.
 		l.Debug("Retrying loading Collection into kernel after map migration")
-		coll, err = bpf.LoadCollection(spec, opts)
+		coll, err = bpf.LoadCollection(spec, collOpts)
 	}
 	var ve *ebpf.VerifierError
 	if errors.As(err, &ve) {
@@ -221,16 +228,16 @@ func replaceDatapath(ctx context.Context, ifName, objPath string, progs []progDe
 
 	// Finally, attach the endpoint's tc or xdp entry points to allow traffic to
 	// flow in.
-	for _, prog := range progs {
+	for _, prog := range opts.programs {
 		scopedLog := l.WithField("progName", prog.progName).WithField("direction", prog.direction)
-		if xdpMode != "" {
+		if opts.xdpMode != "" {
 			linkDir := bpffsDeviceLinksDir(bpf.CiliumPath(), link)
 			if err := bpf.MkdirBPF(linkDir); err != nil {
 				return nil, fmt.Errorf("creating bpffs link dir for device %s: %w", link.Attrs().Name, err)
 			}
 
 			scopedLog.Debug("Attaching XDP program to interface")
-			err = attachXDPProgram(link, coll.Programs[prog.progName], prog.progName, linkDir, xdpConfigModeToFlag(xdpMode))
+			err = attachXDPProgram(link, coll.Programs[prog.progName], prog.progName, linkDir, xdpConfigModeToFlag(opts.xdpMode))
 		} else {
 			scopedLog.Debug("Attaching TC program to interface")
 			err = attachTCProgram(link, coll.Programs[prog.progName], prog.progName, directionToParent(prog.direction))

pkg/datapath/loader/xdp.go

@@ -155,7 +155,14 @@ func compileAndLoadXDPProg(ctx context.Context, xdpDev, xdpMode string, extraCAr
 	}
 
 	progs := []progDefinition{{progName: symbolFromHostNetdevXDP, direction: ""}}
-	finalize, err := replaceDatapath(ctx, xdpDev, objPath, progs, xdpMode)
+	finalize, err := replaceDatapath(ctx,
+		replaceDatapathOptions{
+			device:   xdpDev,
+			elf:      objPath,
+			programs: progs,
+			xdpMode:  xdpMode,
+		},
+	)
 	if err != nil {
 		return err
 	}