bpf-map-pinning: fixes a few issues

* Update the documentation to reflect configuration
changes needed.
* Update the program to pin the xsk map to just use
xdp-loader. The previous c implementation had issues
and the version of libxdp in the container images is
too old for libxdp pinning support. Reverting to bpf
(for pinning support) would break the libxdp based
program unloading.
* Don't return an error from the bpf clean up function
when there's no program on the interface.

Signed-off-by: Maryam Tahhan <mtahhan@redhat.com>

README.md

@@ -411,6 +411,8 @@ UdsServerDisable is a Boolean configuration. If set to true, devices in this poo
 
 BpfMapPinningEnable is a Boolean configuration. If set to true, will use BPF map pinning instead of a UDS to share an XSK map with a pod. By default, this is set to false. Should set UdsServerDisable to true when using this configuration.
 
+> **_NOTE:_**  When using this setting it's important to adjust the securityContext for the DP Daemonset to `privileged: true` to allow for bidirectional propagation of the volume where the BPF maps are pinned. this will no longer be needed when the DP is integrated with bpfman.
+
 #### UdsTimeout
 
 UdsTimeout is an integer configuration. This value sets the amount of time, in seconds, that the UDS server will wait while there is no activity on the UDS. When this timeout limit is reached, the UDS server terminates and the UDS is deleted from the filesystem. This can be a useful setting, for example, in scenarios where large batches of pods are created together. Large batches of pods tend to take some time to spin up, so it might be beneficial to have the UDS server sit waiting a little longer for the pod to start. The maximum allowed value is 300 seconds (5 min). The minimum and default value is 30 seconds.

cmd/deviceplugin/main.go

@@ -143,7 +143,6 @@ func main() {
 			logging.Errorf("Termination error: %v", err)
 		}
 	}
-
 }
 
 func configureLogging(cfg deviceplugin.PluginConfig) error {

constants/constants.go

@@ -84,6 +84,7 @@ var (
 	udsPodSock    = "/afxdp.sock"
 
 	/* BPF*/
+	pinMapBaseDir = "/var/run/afxdp_dp/"
 	bpfMapPodPath = "/tmp/afxdp_dp/"
 	xsk_map       = "/xsks_map"
 
@@ -222,6 +223,7 @@ type uds struct {
 }
 
 type bpf struct {
+	PinMapBaseDir string
 	BpfMapPodPath string
 	Xsk_map       string
 }
@@ -349,6 +351,7 @@ func init() {
 	}
 
 	Bpf = bpf{
+		PinMapBaseDir: pinMapBaseDir,
 		BpfMapPodPath: bpfMapPodPath,
 		Xsk_map:       xsk_map,
 	}

deployments/daemonset.yml

@@ -62,6 +62,7 @@ spec:
           image: intel/afxdp-plugins-for-kubernetes:latest
           imagePullPolicy: IfNotPresent
           securityContext:
+            #privileged: true # NEEDED WHEN USING BPF MAP PINNING FOR BIDIR PROPAGATION
             capabilities:
               drop:
                 - all
@@ -78,6 +79,9 @@ spec:
           volumeMounts:
             - name: unixsock
               mountPath: /tmp/afxdp_dp/
+            - name: bpfmappinning
+              mountPath: /var/run/afxdp_dp/
+              #mountPropagation: Bidirectional
             - name: devicesock
               mountPath: /var/lib/kubelet/device-plugins/
             - name: resources
@@ -92,6 +96,9 @@ spec:
         - name: unixsock
           hostPath:
             path: /tmp/afxdp_dp/
+        - name: bpfmappinning
+          hostPath:
+            path: /var/run/afxdp_dp/
         - name: devicesock
           hostPath:
             path: /var/lib/kubelet/device-plugins/

images/amd64.dockerfile

@@ -28,13 +28,14 @@ WORKDIR /usr/src/afxdp_k8s_plugins
 RUN apk add --no-cache build-base~=0.5-r3 \
 && apk add --no-cache libbsd-dev~=0.11.7 \
 && apk add --no-cache libxdp-dev~=1.2.10-r0 \
+&& apk add --no-cache libbpf-dev~=1.0.1-r0 \
 && apk add --no-cache llvm15~=15.0.7-r0 \
 && apk add --no-cache clang15~=15.0.7-r0 \
 && make builddp
 
 FROM amd64/alpine:3.18@sha256:25fad2a32ad1f6f510e528448ae1ec69a28ef81916a004d3629874104f8a7f70
 RUN apk --no-cache -U add iproute2-rdma~=6.3.0-r0 acl~=2.3 \
-      && apk add --no-cache libxdp~=1.2.10-r0
+      && apk add --no-cache xdp-tools~=1.2.10-r0
 COPY --from=cnibuilder /usr/src/afxdp_k8s_plugins/bin/afxdp /afxdp/afxdp
 COPY --from=dpbuilder /usr/src/afxdp_k8s_plugins/bin/afxdp-dp /afxdp/afxdp-dp
 COPY --from=dpbuilder /usr/src/afxdp_k8s_plugins/images/entrypoint.sh /afxdp/entrypoint.sh

internal/bpf/bpfWrapper.c

@@ -158,13 +158,13 @@ int Clean_bpf(char *ifname) {
 
 	mp = xdp_multiprog__get_from_ifindex(if_index);
 	if (!mp) {
-		Log_Error("%s: unable to receive correct multi_prog reference : %s", __FUNCTION__,
-			  mp);
-		return -1;
+		Log_Info("%s: No programs loaded on : %s", __FUNCTION__,
+			  ifname);
+		return 0;
 	}
 
 	err = xdp_multiprog__detach(mp);
-	if (err) {
+	if (err && err != -EINVAL) { // -EINVAL == No program attached
 		Log_Error("%s: Removal of xdp program failed, returned: "
 			  "returned: %d",
 			  __FUNCTION__, err);
@@ -180,7 +180,6 @@ int Load_attach_bpf_xdp_pass(char *ifname) {
 	char *filename = "/afxdp/xdp_pass.o";
 	struct bpf_object *obj;
 	struct xdp_program *prog;
-	__u32 xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST | XDP_FLAGS_DRV_MODE;
 
 	Log_Info("%s: disovering if_index for interface %s", __FUNCTION__, ifname);
 
@@ -212,7 +211,7 @@ int Load_attach_bpf_xdp_pass(char *ifname) {
 	}
 
 	/* Attach the program to the interface at the xdp hook */
-	err = xdp_program__attach(prog, ifindex, XDP_FLAGS_UPDATE_IF_NOEXIST, 0);
+	err = xdp_program__attach(prog, ifindex, XDP_MODE_NATIVE, 0);
 	if (err) {
 		libxdp_strerror(err, "Couldn't attach the xdp pass program",
 				sizeof("Couldn't attach the xdp pass program"));
@@ -225,51 +224,3 @@ int Load_attach_bpf_xdp_pass(char *ifname) {
 	return 0;
 }
 
-int Load_bpf_pin_xsk_map(char *ifname, char *pin_path) {
-	struct bpf_object *obj;
-	struct xdp_program *prog;
-	struct bpf_link *link;
-	int ifindex, map_fd = -1;
-	int err;
-	const char *prog_name = "xdp_afxdp_redirect";
-	char *filename = "/afxdp/xdp_afxdp_redirect.o";
-	DECLARE_LIBBPF_OPTS(bpf_object_open_opts, bpf_opts, .pin_root_path = pin_path);
-
-	ifindex = if_nametoindex(ifname);
-	if (!ifindex) {
-		Log_Error("%s: if_index not valid: %s", __FUNCTION__, ifname);
-		return -1;
-	}
-	Log_Info("%s: if_index for interface %s is %d", __FUNCTION__, ifname, ifindex);
-
-	if (access(filename, O_RDONLY) < 0) {
-		Log_Error("%s:error accessing file %s: %s\n", __FUNCTION__, filename,
-			  strerror(errno));
-		return err;
-	}
-
-	Log_Info("%s: starting setup of xdp-redirect program on "
-		 "interface %s (%d)",
-		 __FUNCTION__, ifname, ifindex);
-
-	/* Load the BPF program */
-	prog = xdp_program__open_file(filename, NULL, NULL);
-	err = libxdp_get_error(prog);
-	if (err) {
-		libxdp_strerror(err, "Couldn’t load XDP program",
-				sizeof("Couldn’t load XDP program"));
-		Log_Error("%s: Couldn’t load XDP program\n", __FUNCTION__, filename);
-		return err;
-	}
-
-	/* Attach the program to the interface at the xdp hook */
-	err = xdp_program__attach(prog, ifindex, XDP_FLAGS_UPDATE_IF_NOEXIST, 0);
-	if (err) {
-		libxdp_strerror(err, "Couldn't attach the xdp pass program",
-				sizeof("Couldn't attach the xdp pass program"));
-		Log_Error("%s: Couldn't attach the XDP PASS PROGRAM TO %s\n", __FUNCTION__, ifname);
-		return err;
-	}
-
-	return 0;
-}

internal/bpf/bpfWrapper.go

@@ -26,6 +26,8 @@ import "C"
 
 import (
 	"errors"
+	"fmt"
+	"os/exec"
 
 	logging "github.com/sirupsen/logrus"
 )
@@ -85,10 +87,24 @@ func (r *handler) LoadAttachBpfXdpPass(ifname string) error {
 LoadBpfPinXskMap is the GoLang wrapper for the C function Load_bpf_send_xsk_map
 */
 func (r *handler) LoadBpfPinXskMap(ifname, pin_path string) error {
-	err := int(C.Load_bpf_pin_xsk_map(C.CString(ifname), C.CString(pin_path)))
 
-	if err < 0 {
-		return errors.New("error loading BPF program onto interface")
+	bpfProg := "/afxdp/xdp_afxdp_redirect.o"
+
+	loaderCmd := "xdp-loader load " + ifname + " " + bpfProg + " -p " + pin_path
+
+	if err := r.Cleanbpf(ifname); err != nil {
+		err = fmt.Errorf("cmdDel(): error removing BPF program from device: %w", err)
+		logging.Errorf(err.Error())
+
+		return err
+	}
+
+	logging.Infof("Loading XDP program using: %s", loaderCmd)
+
+	cmd := exec.Command("xdp-loader", "load", ifname, bpfProg, "-p", pin_path)
+	if err := cmd.Run(); err != nil {
+		logging.Errorf("error loading and pinning BPF program onto interface %v", err)
+		return errors.New("error loading and pinning BPF program onto interface")
 	}
 
 	return nil
@@ -111,6 +127,7 @@ func (r *handler) ConfigureBusyPoll(fd int, busyTimeout int, busyBudget int) err
 Cleanbpf is the GoLang wrapper for the C function Clean_bpf
 */
 func (r *handler) Cleanbpf(ifname string) error {
+
 	ret := C.Clean_bpf(C.CString(ifname))
 
 	if ret != 0 {

internal/bpf/bpfWrapper.h

@@ -19,7 +19,6 @@
 
 int Load_bpf_send_xsk_map(char *ifname);
 int Load_attach_bpf_xdp_pass(char *ifname);
-int Load_bpf_pin_xsk_map(char *ifname, char *pin_path);
 int Configure_busy_poll(int fd, int busy_timeout, int busy_budget);
 int Clean_bpf(char *ifname);
 

internal/bpf/mapManager.go

@@ -21,14 +21,14 @@ import (
 	"syscall"
 
 	"github.com/google/uuid"
+	"github.com/intel/afxdp-plugins-for-kubernetes/constants"
 	"github.com/intel/afxdp-plugins-for-kubernetes/internal/host"
 	"github.com/moby/sys/mount"
 	"github.com/pkg/errors"
 	logging "github.com/sirupsen/logrus"
 )
 
 const (
-	pinnedMapBaseDir     = "/var/run/afxdp_dp/"
 	pinnedMapDirFileMode = os.FileMode(0755)
 	bpffsDirFileMode     = os.FileMode(0755)
 )
@@ -38,7 +38,7 @@ MapManager is the interface defining the MAP MANAGER.
 Implementations of this interface are the main type of this MapManager package. TODO UPDATE
 */
 type MapManager interface {
-	CreateBPFFS(dev, path string) (string, error)
+	CreateBPFFS() (string, error)
 	DeleteBPFFS(dev string) error
 	AddMap(dev, path string)
 	GetMaps() (map[string]string, error)
@@ -48,7 +48,6 @@ type MapManager interface {
 
 type PoolBpfMapManager struct {
 	Manager MapManager
-	Path    string
 }
 
 /*
@@ -58,7 +57,7 @@ container is created the factory will create a MapManager to serve the
 associated pinned BPF Map. TODO UPDATE THIS....
 */
 type MapManagerFactory interface {
-	CreateMapManager(poolName, user string) (MapManager, string, error)
+	CreateMapManager(poolName, user string) (MapManager, error)
 }
 
 /*
@@ -89,15 +88,15 @@ func NewMapMangerFactory() MapManagerFactory {
 CreateMapManager creates, initialises, and returns an implementation of the MapManager interface.
 It also returns the filepath for bpf maps to be pinned.
 */
-func (f *mapManagerFactory) CreateMapManager(poolName, user string) (MapManager, string, error) {
+func (f *mapManagerFactory) CreateMapManager(poolName, user string) (MapManager, error) {
 
-	logging.Debugf("	  CreateMapManager	  ")
+	logging.Debugf("	  CreateMapManager %s	  ", poolName)
 	if poolName == "" || user == "" {
-		return nil, "", errors.New("Error poolname or user not set")
+		return nil, errors.New("Error poolname or user not set")
 	}
 	p, err := createBPFFSBaseDirectory(poolName, user)
 	if err != nil {
-		return nil, "", errors.Wrapf(err, "Error creating BPFFS base directory %v", err.Error())
+		return nil, errors.Wrapf(err, "Error creating BPFFS base directory %v", err.Error())
 	}
 	logging.Infof("Created BPFFS Base directory %s", p)
 
@@ -108,7 +107,7 @@ func (f *mapManagerFactory) CreateMapManager(poolName, user string) (MapManager,
 		name:      poolName,
 	}
 
-	return manager, p, nil
+	return manager, nil
 }
 
 func giveBpffsBasePermissions(path, user string) error {
@@ -125,40 +124,40 @@ func giveBpffsBasePermissions(path, user string) error {
 
 func createBPFFSBaseDirectory(p, user string) (string, error) {
 
-	logging.Infof("Creating BPFFS Base directory %s", p)
+	path := constants.Bpf.PinMapBaseDir + p + "/"
 
-	path := pinnedMapBaseDir + p
+	logging.Infof("Creating BPFFS Base directory %s", path)
 	if _, err := os.Stat(path); os.IsNotExist(err) {
 		//create base directory if it not exists, with correct file permissions
 		if err = os.MkdirAll(path, pinnedMapDirFileMode); err != nil {
-			return "", errors.Wrapf(err, "Error creating BPFFS base directory %s: %v", pinnedMapBaseDir, err.Error())
+			return "", errors.Wrapf(err, "Error creating BPFFS base directory %s: %v", constants.Bpf.PinMapBaseDir, err.Error())
 		}
 
 		if err = giveBpffsBasePermissions(path, user); err != nil {
-			return "", errors.Wrapf(err, "Error creating BPFFS base directory %s: %v", pinnedMapBaseDir, err.Error())
+			return "", errors.Wrapf(err, "Error creating BPFFS base directory %s: %v", constants.Bpf.PinMapBaseDir, err.Error())
 		}
 	}
 
 	logging.Infof("Created Base BPFFS directory %s", path)
 	return path, nil
 }
 
-func (m mapManager) CreateBPFFS(device, path string) (string, error) {
-	if _, err := os.Stat(path); os.IsNotExist(err) {
-		return "", errors.Wrapf(err, "Error creating BPFFS mount point base directory %s doesn't exist: %v", pinnedMapBaseDir, err.Error())
+func (m mapManager) CreateBPFFS() (string, error) {
+	if _, err := os.Stat(m.bpffsPath); os.IsNotExist(err) {
+		return "", errors.Wrapf(err, "Error creating BPFFS mount point base directory %s doesn't exist: %v", m.bpffsPath, err.Error())
 	}
 
 	bpffsPath, err := generateRandomBpffsName(m.bpffsPath)
 	if err != nil {
-		return "", errors.Wrapf(err, "Error generating BPFFS path: %s: %v", pinnedMapBaseDir, err.Error())
+		return "", errors.Wrapf(err, "Error generating BPFFS path: %s: %v", bpffsPath, err.Error())
 	}
 
 	if err = os.MkdirAll(bpffsPath, bpffsDirFileMode); err != nil {
-		return "", errors.Wrapf(err, "Error creating BPFFS base directory %s: %v", pinnedMapBaseDir, err.Error())
+		return "", errors.Wrapf(err, "Error creating BPFFS base directory %s: %v", bpffsPath, err.Error())
 	}
 
 	if err = giveBpffsBasePermissions(bpffsPath, m.uid); err != nil {
-		return "", errors.Wrapf(err, "Error creating BPFFS base directory %s: %v", pinnedMapBaseDir, err.Error())
+		return "", errors.Wrapf(err, "Error creating BPFFS base directory %s: %v", bpffsPath, err.Error())
 	}
 	logging.Infof("created a directory %s", bpffsPath)
 

internal/cni/cni.go

@@ -338,6 +338,14 @@ func CmdDel(args *skel.CmdArgs) error {
 		}
 	}
 
+	if cfg.DPSyncer {
+		logging.Infof("cmdDel(): Asking Device Plugin to delete any BPF maps for %s", cfg.Device)
+		err := dpcnisyncer.DeleteNetDev(cfg.Device)
+		if err != nil {
+			logging.Errorf("cmdDel(): DeleteNetDev from Syncer Server Failed for %s: %v", cfg.Device, err)
+		}
+	}
+
 	if !cfg.SkipUnloadBpf {
 		logging.Infof("cmdDel(): removing BPF program from device")
 		if err := bpfHandler.Cleanbpf(cfg.Device); err != nil {
@@ -365,14 +373,6 @@ func CmdDel(args *skel.CmdArgs) error {
 		}
 	}
 
-	if cfg.DPSyncer {
-		logging.Infof("cmdDel(): Asking Device Plugin to delete any BPF maps for %s", cfg.Device)
-		err := dpcnisyncer.DeleteNetDev(cfg.Device)
-		if err != nil {
-			logging.Errorf("cmdDel(): DeleteNetDev from Syncer Server Failed for %s: %v", cfg.Device, err)
-		}
-	}
-
 	if cfg.Mode == "cdq" {
 		isSf, err := netHandler.IsCdqSubfunction(cfg.Device)
 		if err != nil {

internal/deviceplugin/poolManager.go

@@ -107,8 +107,8 @@ func (pm *PoolManager) Init(config PoolConfig) error {
 	if pm.BpfMapPinningEnable {
 		var err error
 
-		logging.Infof("Creating new BPF Map manager %s %s", pm.DevicePrefix+"-maps/", pm.UID)
-		pm.Pbm.Manager, pm.Pbm.Path, err = pm.MapManagerFactory.CreateMapManager(pm.DevicePrefix+"-maps/", pm.UID)
+		logging.Infof("Creating new BPF Map manager %s %s", pm.Name, pm.UID)
+		pm.Pbm.Manager, err = pm.MapManagerFactory.CreateMapManager(pm.Name, pm.UID)
 		if err != nil {
 			logging.Errorf("Error new BPF Map manager: %v", err)
 			return err
@@ -251,7 +251,7 @@ func (pm *PoolManager) Allocate(ctx context.Context,
 
 			if pm.BpfMapPinningEnable {
 				logging.Infof("Loading BPF program on device: %s and pinning the map", device.Name())
-				pinPath, err := pm.Pbm.Manager.CreateBPFFS(device.Name(), pm.Pbm.Path)
+				pinPath, err := pm.Pbm.Manager.CreateBPFFS()
 				if err != nil {
 					logging.Errorf("Error Creating the BPFFS: %v", err)
 					return &response, err