Add Peer2Peer P4 rules independant of order of deployment, and other …

…fixes. (#286)

* Changes to ensure that Peer2Peer P4 rules, will get added, independant
of order of deployment of VSP(ipu-plugin) on x86-host and ACC.
If ipu-plugin's Init function gets invoked on ACC, prior to getting
invoked on x86, then host VFs will not be setup yet. In that case, peer2peer
rules can get added in CreateBridgePort or CreateNetworkFunction.
Moved Deletion of P4 rules(such as PeerToPeer) at the point,
where pod gets deleted, this increases the timeout period to
greater than default 30 seconds, so updated vsp-ds.yaml file with
increased timeout. And few misc fixes part of this PR.

e2e/artefacts/k8s/vsp-ds.yaml

@@ -22,6 +22,7 @@ spec:
                 operator: DoesNotExist
       hostNetwork: true
       automountServiceAccountToken: false
+      terminationGracePeriodSeconds: 180
       containers:
       - name: appcntr1
         image: silpixa00400458:5000/intel-ipuplugin:latest
@@ -31,15 +32,24 @@ spec:
         command: [ "/usr/bin/ipuplugin" ]
         args: [ "-v=debug"]
         volumeMounts:
-        - name: vendor-plugin-sock
-          mountPath: /var/run/dpu-daemon/
-        - name: dbus-socket
-          mountPath: /var/run/dbus
+        - mountPath: /var/run/
+          name: vendor-plugin-sock
+        - mountPath: /opt/p4/p4-cp-nws/var
+          mountPropagation: Bidirectional
+          name: host-opt
+        - mountPath: /proc
+          mountPropagation: Bidirectional
+          name: host-proc
       volumes:
-      - name: vendor-plugin-sock
-        hostPath:
-          path: /var/run/dpu-daemon/
-      - name: dbus-socket
-        hostPath:
-          path: /var/run/dbus/
-
+      - hostPath:
+          path: /proc
+          type: ""
+        name: host-proc
+      - hostPath:
+          path: /opt/p4/p4-cp-nws/var
+          type: ""
+        name: host-opt
+      - hostPath:
+          path: /var/run/
+          type: ""
+        name: vendor-plugin-sock

ipu-plugin/pkg/ipuplugin/bridgeport.go

@@ -125,6 +125,8 @@ func (s *server) CreateBridgePort(_ context.Context, in *pb.CreateBridgePortRequ
 		return s.Ports[in.BridgePort.Name].PbBrPort, nil
 	}
 
+	CheckAndAddPeerToPeerP4Rules(s.p4rtbin)
+
 	err, intfName := allocateAccInterface()
 	if err != nil {
 		return nil, fmt.Errorf("error from allocateAccInterface->%v", err)

ipu-plugin/pkg/ipuplugin/ipuplugin.go

@@ -22,7 +22,9 @@ import (
 	"path/filepath"
 	"syscall"
 
+	"github.com/intel/ipu-opi-plugins/ipu-plugin/pkg/p4rtclient"
 	"github.com/intel/ipu-opi-plugins/ipu-plugin/pkg/types"
+	"github.com/intel/ipu-opi-plugins/ipu-plugin/pkg/utils"
 	pb2 "github.com/openshift/dpu-operator/dpu-api/gen"
 
 	pb "github.com/opiproject/opi-api/network/evpn-gw/v1alpha1/gen/go"
@@ -86,14 +88,8 @@ func (s *server) Run() error {
 	if s.mode == types.IpuMode {
 		if err := s.bridgeCtlr.EnsureBridgeExists(); err != nil {
 			log.Errorf("error while checking host bridge existance: %v", err)
-			//log.Fatalf("error while checking host bridge existance: %v", err)
-			//return fmt.Errorf("host bridge error")
-		}
-		if err := ExecutableHandlerGlobal.SetupAccApfs(); err != nil {
-			log.Errorf("error from  SetupAccApfs %v", err)
-			return fmt.Errorf("error from  SetupAccApfs %v", err)
-		} else {
-			log.Info("ipuplugin: setup ACC APFs")
+			log.Fatalf("error while checking host bridge existance: %v", err)
+			return fmt.Errorf("host bridge error")
 		}
 	}
 	pb2.RegisterLifeCycleServiceServer(s.grpcSrvr, NewLifeCycleService(s.daemonHostIp, s.daemonIpuIp, s.daemonPort, s.mode, s.p4rtbin, s.bridgeCtlr))
@@ -124,6 +120,24 @@ func (s *server) Stop() {
 		//Note: Deletes bridge created in EnsureBridgeExists in  Run api.
 		s.bridgeCtlr.DeleteBridges()
 	}
+
+	log.Infof("DeletePhyPortRules, path->%s, 1->%v, 2->%v", s.p4rtbin, AccApfMacList[PHY_PORT0_INTF_INDEX], AccApfMacList[PHY_PORT1_INTF_INDEX])
+	p4rtclient.DeletePhyPortRules(s.p4rtbin, AccApfMacList[PHY_PORT0_INTF_INDEX], AccApfMacList[PHY_PORT1_INTF_INDEX])
+
+	vfMacList, err := utils.GetVfMacList()
+	if err != nil {
+		log.Errorf("Unable to reach the IMC %v", err)
+	}
+	if len(vfMacList) == 0 || (len(vfMacList) == 1 && vfMacList[0] == "") {
+		log.Errorf("No VFs initialized on the host")
+	} else {
+		log.Infof("DeletePeerToPeerP4Rules, path->%s, vfMacList->%v", s.p4rtbin, vfMacList)
+		p4rtclient.DeletePeerToPeerP4Rules(s.p4rtbin, vfMacList)
+	}
+
+	log.Infof("DeleteLAGP4Rules, path->%s", s.p4rtbin)
+	p4rtclient.DeleteLAGP4Rules(s.p4rtbin)
+
 	s.grpcSrvr.GracefulStop()
 	if s.listener != nil {
 		s.listener.Close()

ipu-plugin/pkg/ipuplugin/lifecycleservice.go

@@ -25,6 +25,7 @@ import (
 	math_rand "math/rand"
 	"net"
 	"os"
+	"strconv"
 	"strings"
 	"time"
 
@@ -62,6 +63,7 @@ const (
 
 var InitAccApfMacs = false
 var AccApfMacList []string
+var PeerToPeerP4RulesAdded = false
 
 // Reserved ACC interfaces(using vport_id or last digit of interface name, like 4 represents-> enp0s1f0d4)
 const (
@@ -369,28 +371,6 @@ func GetFilteredPFs(pfList *[]netlink.Link) error {
 	return nil
 }
 
-func FindInterfaceIdForGivenMac(macAddr string) (int, error) {
-	intfIndex := 0
-	found := false
-	if !InitAccApfMacs {
-		log.Errorf("FindInterfaceIdForGivenMac: AccApfs info not set, thro-> SetupAccApfs")
-		return 0, fmt.Errorf("FindInterfaceIdForGivenMac: AccApfs info not set, thro-> SetupAccApfs")
-	}
-	for i := 0; i < len(AccApfMacList); i++ {
-		if AccApfMacList[i] == macAddr {
-			intfIndex = i
-			log.Debugf("found intfIndex->%v for mac->%v\n", intfIndex, macAddr)
-			found = true
-			break
-		}
-	}
-	if found == true {
-		return intfIndex, nil
-	}
-	log.Errorf("Couldnt find intfIndex for mac->%v\n", macAddr)
-	return 0, fmt.Errorf("Couldnt find intfIndex for mac->%v\n", macAddr)
-}
-
 func FindInterfaceForGivenMac(macAddr string) (string, error) {
 	var pfList []netlink.Link
 	InitHandlers()
@@ -679,15 +659,15 @@ if [ -e %s ]; then
     sed -i 's/mod_num_pages = 1;/mod_num_pages = 2;/g' $CP_INIT_CFG
     sed -i 's/cxp_num_pages = 1;/cxp_num_pages = 6;/g' $CP_INIT_CFG
     sed -i 's/pf_mac_address = "00:00:00:00:03:14";/pf_mac_address = "%s";/g' $CP_INIT_CFG
-    sed -i 's/acc_apf = 4;/acc_apf = 16;/g' $CP_INIT_CFG
+    sed -i 's/acc_apf = 4;/acc_apf = %s;/g' $CP_INIT_CFG
     sed -i 's/comm_vports = .*/comm_vports = (([5,0],[4,0]),([0,3],[5,3]),([0,2],[4,3]));/g' $CP_INIT_CFG
     sed -i 's/uplink_vports = .*/uplink_vports = ([0,0,0],[0,1,1],[4,1,0],[4,5,1],[5,1,0],[5,2,1]);/g' $CP_INIT_CFG
     sed -i 's/rep_vports = .*/rep_vports = ([0,0,0],[4,5,1]);/g' $CP_INIT_CFG
     sed -i 's/exception_vports = .*/exception_vports = ([0,0,0],[4,5,1]); /g' $CP_INIT_CFG
 else
     echo "No custom package found. Continuing with default package"
 fi
-`, p4PkgName, p4PkgName, p4PkgName, p4PkgName, macAddress)
+`, p4PkgName, p4PkgName, p4PkgName, p4PkgName, macAddress, strconv.Itoa(ApfNumber))
 
 	return shellScript
 
@@ -832,10 +812,10 @@ func skipIMCReboot() (bool, string) {
 
 func (e *ExecutableHandlerImpl) validate() bool {
 
-	/*Note: Num of APFs gets validated early on,
-	in SetupAccApfs, prior to 1 interface(for Phy Port),
-	getting moved to p4 container in configureFxp */
-
+	if numAPFs := countAPFDevices(); numAPFs < ApfNumber {
+		log.Errorf("Not enough APFs %v, expected->%v", numAPFs, ApfNumber)
+		return false
+	}
 	if noReboot, infoStr := skipIMCReboot(); !noReboot {
 		fmt.Printf("IMC reboot required : %v\n", infoStr)
 		return false
@@ -868,14 +848,28 @@ func (e *ExecutableHandlerImpl) SetupAccApfs() error {
 	return nil
 }
 
-func (s *FXPHandlerImpl) configureFXP(p4rtbin string, brCtlr types.BridgeController) error {
-	vfMacList, err := utils.GetVfMacList()
-	if err != nil {
-		return fmt.Errorf("Unable to reach the IMC %v", err)
-	}
-	if len(vfMacList) == 0 {
-		return fmt.Errorf("No NFs initialized on the host")
+// If ipu-plugin's Init function gets invoked on ACC, prior to getting invoked
+// on x86, then host VFs will not be setup yet. In that case, peer2peer rules
+// will get added in CreateBridgePort or CreateNetworkFunction.
+func CheckAndAddPeerToPeerP4Rules(p4rtbin string) {
+	if !PeerToPeerP4RulesAdded {
+		vfMacList, err := utils.GetVfMacList()
+		if err != nil {
+			log.Errorf("CheckAndAddPeerToPeerP4Rules: unable to reach the IMC %v", err)
+			return
+		}
+		//with use of strings.split in utils, we can get list of length 1 with empty string.
+		if len(vfMacList) == 0 || (len(vfMacList) == 1 && vfMacList[0] == "") {
+			log.Infof("No VFs initialized on the host yet")
+		} else {
+			log.Infof("AddPeerToPeerP4Rules, path->%s, vfMacList->%v", p4rtbin, vfMacList)
+			p4rtclient.AddPeerToPeerP4Rules(p4rtbin, vfMacList)
+			PeerToPeerP4RulesAdded = true
+		}
 	}
+}
+
+func (s *FXPHandlerImpl) configureFXP(p4rtbin string, brCtlr types.BridgeController) error {
 	if !InitAccApfMacs {
 		log.Errorf("configureFXP: AccApfs info not set, thro-> SetupAccApfs")
 		return fmt.Errorf("configureFXP: AccApfs info not set, thro-> SetupAccApfs")
@@ -884,21 +878,14 @@ func (s *FXPHandlerImpl) configureFXP(p4rtbin string, brCtlr types.BridgeControl
 	//Note: Per current design, Phy Port1 is added to a different bridge(through P4 rules).
 	if err := brCtlr.AddPort(AccIntfNames[PHY_PORT0_INTF_INDEX]); err != nil {
 		log.Errorf("failed to add port to bridge: %v, for interface->%v", err, AccIntfNames[PHY_PORT0_INTF_INDEX])
-		//return fmt.Errorf("failed to add port to bridge: %v, for interface->%v", err, AccIntfNames[PHY_PORT0_INTF_INDEX])
+		return fmt.Errorf("failed to add port to bridge: %v, for interface->%v", err, AccIntfNames[PHY_PORT0_INTF_INDEX])
 	}
 	//Add P4 rules for phy ports
-	log.Infof("DeletePhyPortRules, path->%s, 1->%v, 2->%v", p4rtbin, AccApfMacList[PHY_PORT0_INTF_INDEX], AccApfMacList[PHY_PORT1_INTF_INDEX])
-	p4rtclient.DeletePhyPortRules(p4rtbin, AccApfMacList[PHY_PORT0_INTF_INDEX], AccApfMacList[PHY_PORT1_INTF_INDEX])
 	log.Infof("AddPhyPortRules, path->%s, 1->%v, 2->%v", p4rtbin, AccApfMacList[PHY_PORT0_INTF_INDEX], AccApfMacList[PHY_PORT1_INTF_INDEX])
 	p4rtclient.AddPhyPortRules(p4rtbin, AccApfMacList[PHY_PORT0_INTF_INDEX], AccApfMacList[PHY_PORT1_INTF_INDEX])
 
-	log.Infof("DeletePeerToPeerP4Rules, path->%s, vfMacList->%v", p4rtbin, vfMacList)
-	p4rtclient.DeletePeerToPeerP4Rules(p4rtbin, vfMacList)
-	log.Infof("AddPeerToPeerP4Rules, path->%s, vfMacList->%v", p4rtbin, vfMacList)
-	p4rtclient.AddPeerToPeerP4Rules(p4rtbin, vfMacList)
+	CheckAndAddPeerToPeerP4Rules(p4rtbin)
 
-	log.Infof("DeleteLAGP4Rules, path->%s", p4rtbin)
-	p4rtclient.DeleteLAGP4Rules(p4rtbin)
 	log.Infof("AddLAGP4Rules, path->%v", p4rtbin)
 	p4rtclient.AddLAGP4Rules(p4rtbin)
 

ipu-plugin/pkg/ipuplugin/networkfunctionservice.go

@@ -54,6 +54,8 @@ func (s *NetworkFunctionServiceServer) CreateNetworkFunction(ctx context.Context
 		return nil, status.Error(codes.Internal, "No NFs initialized on the host")
 	}
 
+	CheckAndAddPeerToPeerP4Rules(s.p4rtbin)
+
 	if err := s.bridgeCtlr.AddPort(AccIntfNames[NF_IN_PR_INTF_INDEX]); err != nil {
 		log.Errorf("failed to add port to bridge: %v, for interface->%v", err, AccIntfNames[NF_IN_PR_INTF_INDEX])
 		return nil, fmt.Errorf("failed to add port to bridge: %v, for interface->%v", err, AccIntfNames[NF_IN_PR_INTF_INDEX])

ipu-plugin/pkg/ipuplugin/ovsbridge.go

@@ -31,7 +31,7 @@ func createDbParam(ovsDbPath string) string {
 	return "--db=unix:" + ovsDbPath
 }
 
-func getInfrapodNamespace() string {
+func getInfrapodNamespace() (string, error) {
 
 	nsCmdParams := []string{"ps", "-a",
 		"|", "grep", "entrypoint.sh",
@@ -42,9 +42,9 @@ func getInfrapodNamespace() string {
 	ret, err := utils.ExecuteScript(strings.Join(nsCmdParams, " "))
 	if err != nil {
 		log.Errorf("unable to get Namespace of infrapod: %v", err)
-		return ret
+		return "", fmt.Errorf("unable to get Namespace of infrapod: %v", err)
 	}
-	return ret
+	return ret, nil
 }
 
 func (b *ovsBridge) EnsureBridgeExists() error {
@@ -53,14 +53,19 @@ func (b *ovsBridge) EnsureBridgeExists() error {
 	if err := utils.ExecOsCommand(b.ovsCliDir+"/ovs-vsctl", createBrParams...); err != nil {
 		return fmt.Errorf("error creating ovs bridge %s with ovs-vsctl command %s", b.bridgeName, err.Error())
 	}
+	netNs, err := getInfrapodNamespace()
+	if err != nil {
+		log.Errorf("EnsureBridgeExists: error->%v from getInfrapodNamespace", err)
+		return err
+	}
 	//assigning IP for bridge interface.
 	ipAddr := ACC_VM_PR_IP
-	cmdParams := []string{"net", "exec", getInfrapodNamespace(), "ip", "addr", "add", "dev", b.bridgeName, ipAddr}
+	cmdParams := []string{"net", "exec", netNs, "ip", "addr", "add", "dev", b.bridgeName, ipAddr}
 	if err := utils.ExecOsCommand("ip", cmdParams...); err != nil {
 		return fmt.Errorf("error->%v, assigning IP->%v to ovs bridge %s", err.Error(), ipAddr, b.bridgeName)
 	}
 	//bring the interface up.
-	cmdParams = []string{"net", "exec", getInfrapodNamespace(), "ip", "link", "set", "dev", b.bridgeName, "up"}
+	cmdParams = []string{"net", "exec", netNs, "ip", "link", "set", "dev", b.bridgeName, "up"}
 	if err := utils.ExecOsCommand("ip", cmdParams...); err != nil {
 		return fmt.Errorf("error->%v, bringing UP bridge interface->%v", err.Error(), b.bridgeName)
 	}
@@ -79,9 +84,14 @@ func (b *ovsBridge) DeleteBridges() error {
 }
 
 func (b *ovsBridge) AddPort(portName string) error {
+	netNs, err := getInfrapodNamespace()
+	if err != nil {
+		log.Errorf("AddPort: error->%v from getInfrapodNamespace", err)
+		return err
+	}
 	// Move interface to the infrapod namespace
-	ipParams := []string{"link", "set", portName, "netns", getInfrapodNamespace()}
-	err := utils.ExecOsCommand("ip", ipParams...)
+	ipParams := []string{"link", "set", portName, "netns", netNs}
+	err = utils.ExecOsCommand("ip", ipParams...)
 	if err != nil {
 		log.Errorf("error moving interface %s to infra namespace with error %s", portName, err.Error())
 	}
@@ -92,7 +102,7 @@ func (b *ovsBridge) AddPort(portName string) error {
 		return fmt.Errorf("unable to add port to the bridge: %w", err)
 	}
 	//bring the interface up.
-	cmdParams := []string{"net", "exec", getInfrapodNamespace(), "ip", "link", "set", "dev", portName, "up"}
+	cmdParams := []string{"net", "exec", netNs, "ip", "link", "set", "dev", portName, "up"}
 	if err := utils.ExecOsCommand("ip", cmdParams...); err != nil {
 		return fmt.Errorf("error->%v, bringing UP interface->%v", err.Error(), portName)
 	}
@@ -101,9 +111,14 @@ func (b *ovsBridge) AddPort(portName string) error {
 }
 
 func (b *ovsBridge) DeletePort(portName string) error {
+	netNs, err := getInfrapodNamespace()
+	if err != nil {
+		log.Errorf("DeletePort: error->%v from getInfrapodNamespace", err)
+		return err
+	}
 	// Move interface out of the infrapod namespace
-	ipParams := []string{"net", "exec", getInfrapodNamespace(), "ip", "link", "set", "dev", portName, "netns", "1"}
-	err := utils.ExecOsCommand("ip", ipParams...)
+	ipParams := []string{"net", "exec", netNs, "ip", "link", "set", "dev", portName, "netns", "1"}
+	err = utils.ExecOsCommand("ip", ipParams...)
 	if err != nil {
 		log.Errorf("error moving interface %s to infra namespace with error %s", portName, err.Error())
 	}

p4sdk/Makefile

@@ -41,7 +41,7 @@ PLATFORMS ?= linux/arm64,linux/amd64
 imagex: ## Build and push docker image for the manager for cross-platform support
 	# copy existing Dockerfile and insert --platform=${BUILDPLATFORM} into Dockerfile.cross, and preserve the original Dockerfile
 	sed -e '1 s/\(^FROM\)/FROM --platform=\$$\{TARGETPLATFORM\}/; t' -e ' 1,// s//FROM --platform=\$$\{TARGETPLATFORM\}/' $(DOCKERFILE) > $(DOCKERFILE).cross
-	- $(IMGTOOL) buildx create --name image-builder-p4 --use --buildkitd-flags '--allow-insecure-entitlement security.insecure' --driver-opt env.http_proxy=$(HTTP_PROXY) --driver-opt env.https_proxy=$(HTTPS_PROXY) --driver-opt '"env.no_proxy='$(NO_PROXY)'"'
+	- $(IMGTOOL) buildx create --name image-builder-p4  --use --config=buildkit.toml --buildkitd-flags '--allow-insecure-entitlement security.insecure' --driver-opt env.http_proxy=$(HTTP_PROXY) --driver-opt env.https_proxy=$(HTTPS_PROXY) --driver-opt '"env.no_proxy='$(NO_PROXY)'"'
 	$(IMGTOOL) buildx use image-builder-p4
 	- $(IMGTOOL) buildx build --allow security.insecure --push --platform=$(PLATFORMS) --tag ${IMAGE_TAG_VERSION} -f $(DOCKERFILE).cross  $(CURDIR) $(DOCKERARGS)
 	- $(IMGTOOL) buildx rm image-builder-p4

p4sdk/buildkit.toml

@@ -0,0 +1,2 @@
+[registry."localhost:5000"]
+  http = true