From 720d92b5cf88cfeac731d0f201cc6ebb8470b3bb Mon Sep 17 00:00:00 2001 From: Maryam Tahhan Date: Tue, 28 Nov 2023 15:19:47 +0000 Subject: [PATCH] multiple device request issue (#81) * multiple device request issue the issue is if a single pod requests different devices from different pools it results in multiple uds servers serving the same container and all attempt to mount their uds to the pod as /tmp/afxdp.sock. A similar issue exists for the AFXDP_DEVICES env var that's set in each container. This patch fixes the first issue by mounting the xsksocket at /tmp/afxdp_dp//afxdp.sock, a similar issue also existed for the bpf map pinning support. --------- Signed-off-by: Maryam Tahhan --- Makefile | 4 +- constants/constants.go | 23 ++++---- internal/deviceplugin/poolManager.go | 27 +++++---- internal/deviceplugin/poolManager_test.go | 67 ++++++++++++++++------- 4 files changed, 77 insertions(+), 44 deletions(-) diff --git a/Makefile b/Makefile index e0de3507..336d4180 100644 --- a/Makefile +++ b/Makefile @@ -73,14 +73,14 @@ build: builddp buildcni docker: ## Build docker image @echo "****** Docker Image ******" @echo - docker build -t localhost:5000/afxdp-device-plugin -f images/amd64.dockerfile . + docker build -t afxdp-device-plugin -f images/amd64.dockerfile . @echo @echo podman: ## Build podman image @echo "****** Podman Image ******" @echo - podman build -t localhost:5000/afxdp-device-plugin -f images/amd64.dockerfile . + podman build -t afxdp-device-plugin -f images/amd64.dockerfile . @echo @echo diff --git a/constants/constants.go b/constants/constants.go index 3ae67af0..32f17c8b 100644 --- a/constants/constants.go +++ b/constants/constants.go @@ -40,7 +40,7 @@ var ( /* Devices */ devicesProhibited = []string{"eno", "eth", "lo", "docker", "flannel", "cni"} // interfaces we never add to a pool - devicesEnvVar = "AFXDP_DEVICES" // env var set in the end user application pod, lists AF_XDP devices attached + devicesEnvVarPrefix = "AFXDP_DEVICES_" // env var set in the end user application pod, lists AF_XDP devices attached deviceValidNameRegex = `^[a-zA-Z0-9_-]+$` // regex to check if a string is a valid device name deviceValidNameMin = 1 // minimum length of a device name deviceValidNameMax = 50 // maximum length of a device name @@ -74,16 +74,17 @@ var ( afxdpMinimumLinux = "4.18.0" // minimum Linux version for AF_XDP support /* UDS*/ - udsMaxTimeout = 300 // maximum configurable uds timeout in seconds - udsMinTimeout = 30 // minimum (and default) uds timeout in seconds - udsMsgBufSize = 64 // uds message buffer size - udsCtlBufSize = 4 // uds control buffer size - udsProtocol = "unixpacket" // uds protocol: "unix"=SOCK_STREAM, "unixdomain"=SOCK_DGRAM, "unixpacket"=SOCK_SEQPACKET - udsSockDir = "/tmp/afxdp_dp/" // host location where we place our uds sockets. If changing location remember to update daemonset mount point - udsPodPath = "/tmp/afxdp.sock" // the uds filepath as it will appear in the end user application pod + udsMaxTimeout = 300 // maximum configurable uds timeout in seconds + udsMinTimeout = 30 // minimum (and default) uds timeout in seconds + udsMsgBufSize = 64 // uds message buffer size + udsCtlBufSize = 4 // uds control buffer size + udsProtocol = "unixpacket" // uds protocol: "unix"=SOCK_STREAM, "unixdomain"=SOCK_DGRAM, "unixpacket"=SOCK_SEQPACKET + udsSockDir = "/tmp/afxdp_dp/" // host location where we place our uds sockets. If changing location remember to update daemonset mount point + udsPodPath = "/tmp/afxdp_dp/" // the uds filepath as it will appear in the end user application pod + udsPodSock = "/afxdp.sock" /* BPF*/ - bpfMapPodPath = "/tmp/xsks_map" + bpfMapPodPath = "/tmp/afxdp_dp/" xsk_map = "/xsks_map" udsDirFileMode = 0700 // permissions for the directory in which we create our uds sockets @@ -216,6 +217,7 @@ type uds struct { SockDir string DirFileMode int PodPath string + SockName string Handshake handshake } @@ -284,7 +286,7 @@ func init() { Devices = devices{ Prohibited: devicesProhibited, - EnvVarList: devicesEnvVar, + EnvVarList: devicesEnvVarPrefix, ValidNameRegex: deviceValidNameRegex, ValidNameMin: deviceValidNameMin, ValidNameMax: deviceValidNameMax, @@ -326,6 +328,7 @@ func init() { SockDir: udsSockDir, DirFileMode: udsDirFileMode, PodPath: udsPodPath, + SockName: udsPodSock, Handshake: handshake{ Version: handshakeHandshakeVersion, RequestVersion: handshakeRequestVersion, diff --git a/internal/deviceplugin/poolManager.go b/internal/deviceplugin/poolManager.go index 15414765..60990180 100644 --- a/internal/deviceplugin/poolManager.go +++ b/internal/deviceplugin/poolManager.go @@ -196,20 +196,22 @@ func (pm *PoolManager) Allocate(ctx context.Context, cresp := new(pluginapi.ContainerAllocateResponse) envs := make(map[string]string) - if !pm.UdsServerDisable { - cresp.Mounts = append(cresp.Mounts, &pluginapi.Mount{ - HostPath: udsPath, - ContainerPath: constants.Uds.PodPath, - ReadOnly: false, - }) - } - //loop each device request per container for _, devName := range crqt.DevicesIDs { device := pm.Devices[devName] pretty, _ := tools.PrettyString(device.Public()) logging.Debugf("Device: %s", pretty) + containerSockPath := constants.Uds.PodPath + device.Name() + constants.Uds.SockName + + if !pm.UdsServerDisable { + cresp.Mounts = append(cresp.Mounts, &pluginapi.Mount{ + HostPath: udsPath, + ContainerPath: containerSockPath, + ReadOnly: false, + }) + } + if device.Mode() != pm.Mode { err := fmt.Errorf("pool mode %s does not match device mode %s", pm.Mode, device.Mode()) logging.Errorf("%v", err) @@ -265,16 +267,18 @@ func (pm *PoolManager) Allocate(ctx context.Context, //FULL PATH WILL INCLUDE THE XSKMAP... fullPath := pinPath + constants.Bpf.Xsk_map - logging.Debugf("mapping %s to %s", fullPath, constants.Bpf.BpfMapPodPath) + containerMapPath := constants.Bpf.BpfMapPodPath + device.Name() + constants.Bpf.Xsk_map + logging.Debugf("mapping %s to %s", fullPath, containerMapPath) cresp.Mounts = append(cresp.Mounts, &pluginapi.Mount{ HostPath: fullPath, - ContainerPath: constants.Bpf.BpfMapPodPath, + ContainerPath: containerMapPath, ReadOnly: false, }) } } - envs[constants.Devices.EnvVarList] = strings.Join(crqt.DevicesIDs, " ") + envVar := constants.Devices.EnvVarList + strings.ToUpper(pm.Name) + envs[envVar] = strings.Join(crqt.DevicesIDs, " ") envsPrint, err := tools.PrettyString(envs) if err != nil { logging.Errorf("Error printing container environment variables: %v", err) @@ -283,7 +287,6 @@ func (pm *PoolManager) Allocate(ctx context.Context, } cresp.Envs = envs response.ContainerResponses = append(response.ContainerResponses, cresp) - } if !pm.UdsServerDisable { diff --git a/internal/deviceplugin/poolManager_test.go b/internal/deviceplugin/poolManager_test.go index 3e6be008..2e556f62 100644 --- a/internal/deviceplugin/poolManager_test.go +++ b/internal/deviceplugin/poolManager_test.go @@ -18,6 +18,7 @@ package deviceplugin import ( "context" "encoding/json" + "strings" "testing" "github.com/intel/afxdp-plugins-for-kubernetes/constants" @@ -57,6 +58,8 @@ func TestAllocate(t *testing.T) { pm.ServerFactory = udsserver.NewFakeServerFactory() pm.BpfHandler = bpf.NewFakeHandler() + envVar := constants.Devices.EnvVarList + strings.ToUpper(pm.Name) + testCases := []struct { name string containerRequests []*pluginapi.ContainerAllocateRequest @@ -69,10 +72,10 @@ func TestAllocate(t *testing.T) { }, expContainerResponses: []*pluginapi.ContainerAllocateResponse{ { - Envs: map[string]string{constants.Devices.EnvVarList: "dev_1"}, + Envs: map[string]string{envVar: "dev_1"}, Mounts: []*pluginapi.Mount{ { - ContainerPath: constants.Uds.PodPath, + ContainerPath: constants.Uds.PodPath + "dev_1" + constants.Uds.SockName, HostPath: "/tmp/fake-socket.sock", ReadOnly: false, }, @@ -90,10 +93,20 @@ func TestAllocate(t *testing.T) { }, expContainerResponses: []*pluginapi.ContainerAllocateResponse{ { - Envs: map[string]string{constants.Devices.EnvVarList: "dev_1 dev_2 dev_3"}, + Envs: map[string]string{envVar: "dev_1 dev_2 dev_3"}, Mounts: []*pluginapi.Mount{ { - ContainerPath: constants.Uds.PodPath, + ContainerPath: constants.Uds.PodPath + "dev_1" + constants.Uds.SockName, + HostPath: "/tmp/fake-socket.sock", + ReadOnly: false, + }, + { + ContainerPath: constants.Uds.PodPath + "dev_2" + constants.Uds.SockName, + HostPath: "/tmp/fake-socket.sock", + ReadOnly: false, + }, + { + ContainerPath: constants.Uds.PodPath + "dev_3" + constants.Uds.SockName, HostPath: "/tmp/fake-socket.sock", ReadOnly: false, }, @@ -112,10 +125,10 @@ func TestAllocate(t *testing.T) { }, expContainerResponses: []*pluginapi.ContainerAllocateResponse{ { - Envs: map[string]string{constants.Devices.EnvVarList: "dev_1"}, + Envs: map[string]string{envVar: "dev_1"}, Mounts: []*pluginapi.Mount{ { - ContainerPath: constants.Uds.PodPath, + ContainerPath: constants.Uds.PodPath + "dev_1" + constants.Uds.SockName, HostPath: "/tmp/fake-socket.sock", ReadOnly: false, }, @@ -124,10 +137,10 @@ func TestAllocate(t *testing.T) { Annotations: map[string]string{}, }, { - Envs: map[string]string{constants.Devices.EnvVarList: "dev_2"}, + Envs: map[string]string{envVar: "dev_2"}, Mounts: []*pluginapi.Mount{ { - ContainerPath: constants.Uds.PodPath, + ContainerPath: constants.Uds.PodPath + "dev_2" + constants.Uds.SockName, HostPath: "/tmp/fake-socket.sock", ReadOnly: false, }, @@ -146,10 +159,20 @@ func TestAllocate(t *testing.T) { }, expContainerResponses: []*pluginapi.ContainerAllocateResponse{ { - Envs: map[string]string{constants.Devices.EnvVarList: "dev_1 dev_2 dev_3"}, + Envs: map[string]string{envVar: "dev_1 dev_2 dev_3"}, Mounts: []*pluginapi.Mount{ { - ContainerPath: constants.Uds.PodPath, + ContainerPath: constants.Uds.PodPath + "dev_1" + constants.Uds.SockName, + HostPath: "/tmp/fake-socket.sock", + ReadOnly: false, + }, + { + ContainerPath: constants.Uds.PodPath + "dev_2" + constants.Uds.SockName, + HostPath: "/tmp/fake-socket.sock", + ReadOnly: false, + }, + { + ContainerPath: constants.Uds.PodPath + "dev_3" + constants.Uds.SockName, HostPath: "/tmp/fake-socket.sock", ReadOnly: false, }, @@ -158,10 +181,20 @@ func TestAllocate(t *testing.T) { Annotations: map[string]string{}, }, { - Envs: map[string]string{constants.Devices.EnvVarList: "dev_4 dev_5 dev_6"}, + Envs: map[string]string{envVar: "dev_4 dev_5 dev_6"}, Mounts: []*pluginapi.Mount{ { - ContainerPath: constants.Uds.PodPath, + ContainerPath: constants.Uds.PodPath + "dev_4" + constants.Uds.SockName, + HostPath: "/tmp/fake-socket.sock", + ReadOnly: false, + }, + { + ContainerPath: constants.Uds.PodPath + "dev_5" + constants.Uds.SockName, + HostPath: "/tmp/fake-socket.sock", + ReadOnly: false, + }, + { + ContainerPath: constants.Uds.PodPath + "dev_6" + constants.Uds.SockName, HostPath: "/tmp/fake-socket.sock", ReadOnly: false, }, @@ -179,14 +212,8 @@ func TestAllocate(t *testing.T) { }, expContainerResponses: []*pluginapi.ContainerAllocateResponse{ { - Envs: map[string]string{constants.Devices.EnvVarList: ""}, - Mounts: []*pluginapi.Mount{ - { - ContainerPath: constants.Uds.PodPath, - HostPath: "/tmp/fake-socket.sock", - ReadOnly: false, - }, - }, + Envs: map[string]string{envVar: ""}, + Mounts: []*pluginapi.Mount{}, Devices: []*pluginapi.DeviceSpec{}, Annotations: map[string]string{}, },