From 540b6b8cb00fcde8ada0f9ed76dce530744a3cab Mon Sep 17 00:00:00 2001 From: sudhar-krishnakumar Date: Thu, 5 Dec 2024 13:07:11 -0800 Subject: [PATCH] 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 | 32 +++++--- ipu-plugin/pkg/ipuplugin/bridgeport.go | 2 + ipu-plugin/pkg/ipuplugin/ipuplugin.go | 30 ++++++-- ipu-plugin/pkg/ipuplugin/lifecycleservice.go | 75 ++++++++----------- .../pkg/ipuplugin/networkfunctionservice.go | 2 + ipu-plugin/pkg/ipuplugin/ovsbridge.go | 35 ++++++--- p4sdk/Makefile | 2 +- p4sdk/buildkit.toml | 2 + 8 files changed, 106 insertions(+), 74 deletions(-) create mode 100644 p4sdk/buildkit.toml diff --git a/e2e/artefacts/k8s/vsp-ds.yaml b/e2e/artefacts/k8s/vsp-ds.yaml index 5c71ac40..80fa4687 100644 --- a/e2e/artefacts/k8s/vsp-ds.yaml +++ b/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 diff --git a/ipu-plugin/pkg/ipuplugin/bridgeport.go b/ipu-plugin/pkg/ipuplugin/bridgeport.go index ad36cde1..a8ab89cf 100644 --- a/ipu-plugin/pkg/ipuplugin/bridgeport.go +++ b/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) diff --git a/ipu-plugin/pkg/ipuplugin/ipuplugin.go b/ipu-plugin/pkg/ipuplugin/ipuplugin.go index dc79cd20..51e3245c 100644 --- a/ipu-plugin/pkg/ipuplugin/ipuplugin.go +++ b/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() diff --git a/ipu-plugin/pkg/ipuplugin/lifecycleservice.go b/ipu-plugin/pkg/ipuplugin/lifecycleservice.go index 6019cfe4..c6284c91 100644 --- a/ipu-plugin/pkg/ipuplugin/lifecycleservice.go +++ b/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,7 +659,7 @@ 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 @@ -687,7 +667,7 @@ if [ -e %s ]; then 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) diff --git a/ipu-plugin/pkg/ipuplugin/networkfunctionservice.go b/ipu-plugin/pkg/ipuplugin/networkfunctionservice.go index e5314ac5..1b5dad05 100644 --- a/ipu-plugin/pkg/ipuplugin/networkfunctionservice.go +++ b/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]) diff --git a/ipu-plugin/pkg/ipuplugin/ovsbridge.go b/ipu-plugin/pkg/ipuplugin/ovsbridge.go index 4c87cb60..33159ad7 100644 --- a/ipu-plugin/pkg/ipuplugin/ovsbridge.go +++ b/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()) } diff --git a/p4sdk/Makefile b/p4sdk/Makefile index 5a3b8616..7ad71fd6 100644 --- a/p4sdk/Makefile +++ b/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 diff --git a/p4sdk/buildkit.toml b/p4sdk/buildkit.toml new file mode 100644 index 00000000..4226e840 --- /dev/null +++ b/p4sdk/buildkit.toml @@ -0,0 +1,2 @@ +[registry."localhost:5000"] + http = true