diff --git a/.github/workflows/buildtest.yml b/.github/workflows/buildtest.yml index d759bd5c0..30b93e483 100644 --- a/.github/workflows/buildtest.yml +++ b/.github/workflows/buildtest.yml @@ -9,7 +9,7 @@ jobs: build-test: strategy: matrix: - go-version: [1.19.x] + go-version: [1.21.x] os: [ubuntu-latest] goos: [linux] goarch: [amd64, arm64, ppc64le] @@ -18,12 +18,12 @@ jobs: GO111MODULE: on steps: - name: Set up Go - uses: actions/setup-go@v3 + uses: actions/setup-go@v4 with: go-version: ${{ matrix.go-version }} - name: Check out code into the Go module directory - uses: actions/checkout@v2 + uses: actions/checkout@v3 - name: Build test for ${{ matrix.goarch }} env: @@ -33,7 +33,7 @@ jobs: - name: Go test if: ${{ matrix.goarch }} == "amd64" - run: sudo go test ./... # sudo needed for netns change in test + run: sudo go test -race ./... # sudo needed for netns change in test coverage: runs-on: ubuntu-latest @@ -41,9 +41,9 @@ jobs: name: coverage steps: - name: Set up Go - uses: actions/setup-go@v1 + uses: actions/setup-go@v4 with: - go-version: 1.19.x + go-version: 1.21.x - name: Check out code into the Go module directory uses: actions/checkout@v2 @@ -52,7 +52,38 @@ jobs: run: sudo make test-coverage # sudo needed for netns change in test - name: Coveralls - uses: coverallsapp/github-action@1.1.3 + uses: coverallsapp/github-action@v2 with: github-token: ${{ secrets.GITHUB_TOKEN }} - path-to-lcov: test/coverage/lcov.info + file: test/coverage/lcov.info + + sriov-operator-e2e-test: + name: SR-IOV operator e2e tests + needs: [ build-test ] + runs-on: [ sriov ] + env: + TEST_REPORT_PATH: k8s-artifacts + steps: + - name: Check out the repo + uses: actions/checkout@v3 + + - name: build sriov-cni image + run: podman build -t ghaction-sriov-cni:pr-${{github.event.pull_request.number}} . + + - name: Check out sriov operator's code + uses: actions/checkout@v2 + with: + repository: k8snetworkplumbingwg/sriov-network-operator + path: sriov-network-operator-wc + + - name: run test + run: make test-e2e-conformance-virtual-k8s-cluster-ci + working-directory: sriov-network-operator-wc + env: + LOCAL_SRIOV_CNI_IMAGE: ghaction-sriov-cni:pr-${{github.event.pull_request.number}} + + - uses: actions/upload-artifact@v3 + if: always() + with: + name: ${{ env.TEST_REPORT_PATH }} + path: ./sriov-network-operator-wc/${{ env.TEST_REPORT_PATH }} diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml new file mode 100644 index 000000000..b6b516d00 --- /dev/null +++ b/.github/workflows/codeql.yml @@ -0,0 +1,41 @@ +name: "CodeQL" + +on: + push: + branches: [ "master" ] + pull_request: + branches: [ "master" ] + schedule: + - cron: "37 4 * * 0" + +jobs: + analyze: + name: Analyze + runs-on: ubuntu-latest + permissions: + actions: read + contents: read + security-events: write + + strategy: + fail-fast: false + matrix: + language: [ go ] + + steps: + - name: Checkout + uses: actions/checkout@v3 + + - name: Initialize CodeQL + uses: github/codeql-action/init@v2 + with: + languages: ${{ matrix.language }} + queries: +security-and-quality + + - name: Autobuild + uses: github/codeql-action/autobuild@v2 + + - name: Perform CodeQL Analysis + uses: github/codeql-action/analyze@v2 + with: + category: "/language:${{ matrix.language }}" diff --git a/.github/workflows/image-push-master.yml b/.github/workflows/image-push-master.yml index 47226edfd..0e01babc1 100644 --- a/.github/workflows/image-push-master.yml +++ b/.github/workflows/image-push-master.yml @@ -13,27 +13,27 @@ jobs: runs-on: ubuntu-20.04 steps: - name: Check out the repo - uses: actions/checkout@v2 + uses: actions/checkout@v3 - name: Set up Docker Buildx - uses: docker/setup-buildx-action@v1 + uses: docker/setup-buildx-action@v2 - name: Login to Docker - uses: docker/login-action@v1 + uses: docker/login-action@v2 with: registry: ghcr.io username: ${{ github.repository_owner }} password: ${{ secrets.GITHUB_TOKEN }} - name: Build and push sriov-cni - uses: docker/build-push-action@v2 + uses: docker/build-push-action@v4 with: context: . push: true platforms: linux/amd64 tags: | ${{ env.IMAGE_NAME }}:latest-amd64 - ${{ steps.docker_meta.outputs.tags }}:${{ github.sha }} + ${{ env.IMAGE_NAME }}:${{ github.sha }} file: ./Dockerfile build-and-push-arm64-sriov-cni: @@ -41,23 +41,23 @@ jobs: runs-on: ubuntu-20.04 steps: - name: Check out the repo - uses: actions/checkout@v2 + uses: actions/checkout@v3 - name: Set up QEMU - uses: docker/setup-qemu-action@v1 + uses: docker/setup-qemu-action@v2 - name: Set up Docker Buildx - uses: docker/setup-buildx-action@v1 + uses: docker/setup-buildx-action@v2 - name: Login to Docker - uses: docker/login-action@v1 + uses: docker/login-action@v2 with: registry: ghcr.io username: ${{ github.repository_owner }} password: ${{ secrets.GITHUB_TOKEN }} - name: Build and push sriov-cni - uses: docker/build-push-action@v2 + uses: docker/build-push-action@v4 with: context: . push: true @@ -71,23 +71,23 @@ jobs: runs-on: ubuntu-20.04 steps: - name: Check out the repo - uses: actions/checkout@v2 + uses: actions/checkout@v3 - name: Set up QEMU - uses: docker/setup-qemu-action@v1 + uses: docker/setup-qemu-action@v2 - name: Set up Docker Buildx - uses: docker/setup-buildx-action@v1 + uses: docker/setup-buildx-action@v2 - name: Login to Docker - uses: docker/login-action@v1 + uses: docker/login-action@v2 with: registry: ghcr.io username: ${{ github.repository_owner }} password: ${{ secrets.GITHUB_TOKEN }} - name: Build and push sriov-cni - uses: docker/build-push-action@v2 + uses: docker/build-push-action@v4 with: context: . push: true @@ -98,13 +98,13 @@ jobs: push-manifest: runs-on: ubuntu-20.04 - needs: [build-and-push-amd64-sriov-cni,build-and-push-amr64-sriov-cni,build-and-push-ppc64le-sriov-cni] + needs: [build-and-push-amd64-sriov-cni,build-and-push-arm64-sriov-cni,build-and-push-ppc64le-sriov-cni] steps: - name: Set up Docker Buildx - uses: docker/setup-buildx-action@v1 + uses: docker/setup-buildx-action@v2 - name: Login to GitHub Container Registry - uses: docker/login-action@v1 + uses: docker/login-action@v2 with: registry: ghcr.io username: ${{ github.repository_owner }} @@ -112,15 +112,7 @@ jobs: - name: Create manifest for multi-arch images run: | - # pull - docker pull ${{ env.IMAGE_NAME }}:latest-amd64 - docker pull ${{ env.IMAGE_NAME }}:latest-arm64 - docker pull ${{ env.IMAGE_NAME }}:latest-ppc64le - # create - docker manifest create ${{ env.IMAGE_NAME }}:latest ${{ env.IMAGE_NAME }}:latest-amd64 ${{ env.IMAGE_NAME }}:latest-arm64 ${{ env.IMAGE_NAME }}:latest-ppc64le - # annotate - docker manifest annotate ${{ env.IMAGE_NAME }}:latest ${{ env.IMAGE_NAME }}:latest-amd64 --arch amd64 - docker manifest annotate ${{ env.IMAGE_NAME }}:latest ${{ env.IMAGE_NAME }}:latest-arm64 --arch arm64 - docker manifest annotate ${{ env.IMAGE_NAME }}:latest ${{ env.IMAGE_NAME }}:latest-ppc64le --arch ppc64le - # push - docker manifest push ${{ env.IMAGE_NAME }}:latest \ No newline at end of file + docker buildx imagetools create -t ${{ env.IMAGE_NAME }}:latest \ + ${{ env.IMAGE_NAME }}:latest-amd64 \ + ${{ env.IMAGE_NAME }}:latest-arm64 \ + ${{ env.IMAGE_NAME }}:latest-ppc64le \ No newline at end of file diff --git a/.github/workflows/image-push-release.yml b/.github/workflows/image-push-release.yml index 9d91a0cc5..116c75159 100644 --- a/.github/workflows/image-push-release.yml +++ b/.github/workflows/image-push-release.yml @@ -13,13 +13,13 @@ jobs: name: Image push AMD64 steps: - name: Check out the repo - uses: actions/checkout@v2 + uses: actions/checkout@v3 - name: Set up Docker Buildx - uses: docker/setup-buildx-action@v1 + uses: docker/setup-buildx-action@v2 - name: Login to Docker - uses: docker/login-action@v1 + uses: docker/login-action@v2 with: registry: ghcr.io username: ${{ github.repository_owner }} @@ -27,20 +27,22 @@ jobs: - name: Docker meta id: docker_meta - uses: crazy-max/ghaction-docker-meta@v1 + uses: docker/metadata-action@v4 with: images: ${{ env.IMAGE_NAME }} - tag-latest: false + flavor: | + latest=false + tags: | + type=ref,event=tag - name: Build and push sriov-cni - uses: docker/build-push-action@v2 + uses: docker/build-push-action@v4 with: context: . push: true platforms: linux/amd64 tags: | ${{ steps.docker_meta.outputs.tags }}-amd64 - ${{ steps.docker_meta.outputs.tags }}:${{ github.sha }} file: ./Dockerfile build-and-push-arm64-sriov-cni: @@ -48,13 +50,13 @@ jobs: name: Image push ARM64 steps: - name: Check out the repo - uses: actions/checkout@v2 + uses: actions/checkout@v3 - name: Set up Docker Buildx - uses: docker/setup-buildx-action@v1 + uses: docker/setup-buildx-action@v2 - name: Login to Docker - uses: docker/login-action@v1 + uses: docker/login-action@v2 with: registry: ghcr.io username: ${{ github.repository_owner }} @@ -62,13 +64,16 @@ jobs: - name: Docker meta id: docker_meta - uses: crazy-max/ghaction-docker-meta@v1 + uses: docker/metadata-action@v4 with: images: ${{ env.IMAGE_NAME }} - tag-latest: false + flavor: | + latest=false + tags: | + type=ref,event=tag - name: Build and push sriov-cni - uses: docker/build-push-action@v2 + uses: docker/build-push-action@v4 with: context: . push: true @@ -82,13 +87,13 @@ jobs: name: Image push ppc64le steps: - name: Check out the repo - uses: actions/checkout@v2 + uses: actions/checkout@v3 - name: Set up Docker Buildx - uses: docker/setup-buildx-action@v1 + uses: docker/setup-buildx-action@v2 - name: Login to Docker - uses: docker/login-action@v1 + uses: docker/login-action@v2 with: registry: ghcr.io username: ${{ github.repository_owner }} @@ -96,37 +101,43 @@ jobs: - name: Docker meta id: docker_meta - uses: crazy-max/ghaction-docker-meta@v1 + uses: docker/metadata-action@v4 with: images: ${{ env.IMAGE_NAME }} - tag-latest: false + flavor: | + latest=false + tags: | + type=ref,event=tag - name: Build and push sriov-cni - uses: docker/build-push-action@v2 + uses: docker/build-push-action@v4 with: context: . push: true - platforms: linux/arm64 + platforms: linux/ppc64le tags: | ${{ steps.docker_meta.outputs.tags }}-ppc64le file: ./Dockerfile.ppc64le push-manifest: runs-on: ubuntu-20.04 - needs: [build-and-push-amd64-sriov-cni,build-and-push-amr64-sriov-cni,build-and-push-ppc64le-sriov-cni] + needs: [build-and-push-amd64-sriov-cni,build-and-push-arm64-sriov-cni,build-and-push-ppc64le-sriov-cni] steps: - name: Set up Docker Buildx - uses: docker/setup-buildx-action@v1 + uses: docker/setup-buildx-action@v2 - name: Docker meta id: docker_meta - uses: crazy-max/ghaction-docker-meta@v1 + uses: docker/metadata-action@v4 with: images: ${{ env.IMAGE_NAME }} - tag-latest: false + flavor: | + latest=false + tags: | + type=ref,event=tag - name: Login to GitHub Container Registry - uses: docker/login-action@v1 + uses: docker/login-action@v2 with: registry: ghcr.io username: ${{ github.repository_owner }} @@ -134,15 +145,7 @@ jobs: - name: Create manifest for multi-arch images run: | - # pull - docker pull ${{ steps.docker_meta.outputs.tags }}-amd64 - docker pull ${{ steps.docker_meta.outputs.tags }}-arm64 - docker pull ${{ steps.docker_meta.outputs.tags }}-ppc64le - # create - docker manifest create ${{ steps.docker_meta.outputs.tags }} ${{ steps.docker_meta.outputs.tags }}-amd64 ${{ steps.docker_meta.outputs.tags }}-arm64 ${{ steps.docker_meta.outputs.tags }}-ppc64le - # annotate - docker manifest annotate ${{ steps.docker_meta.outputs.tags }} ${{ steps.docker_meta.outputs.tags }}-amd64 --arch amd64 - docker manifest annotate ${{ steps.docker_meta.outputs.tags }} ${{ steps.docker_meta.outputs.tags }}-arm64 --arch arm64 - docker manifest annotate ${{ steps.docker_meta.outputs.tags }} ${{ steps.docker_meta.outputs.tags }}-ppc64le --arch ppc64le - # push - docker manifest push ${{ steps.docker_meta.outputs.tags }} + docker buildx imagetools create -t ${{ steps.docker_meta.outputs.tags }} \ + ${{ steps.docker_meta.outputs.tags }}-amd64 \ + ${{ steps.docker_meta.outputs.tags }}-arm64 \ + ${{ steps.docker_meta.outputs.tags }}-ppc64le diff --git a/.github/workflows/static-scan.yml b/.github/workflows/static-scan.yml index 42015c68a..b92b32a67 100644 --- a/.github/workflows/static-scan.yml +++ b/.github/workflows/static-scan.yml @@ -5,25 +5,26 @@ jobs: name: Lint runs-on: ubuntu-latest steps: - - uses: actions/checkout@v2 - - name: golangci-lint - uses: golangci/golangci-lint-action@v2 + - name: Set up Go + uses: actions/setup-go@v4 with: - # Required: the version of golangci-lint is required and must be specified without patch version: we always use the latest patch version. - version: v1.49 + go-version: "1.20" + - uses: actions/checkout@v3 + - name: run make lint + run: make lint shellcheck: name: Shellcheck runs-on: ubuntu-latest steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v3 - name: Run ShellCheck uses: ludeeus/action-shellcheck@master hadolint: runs-on: ubuntu-latest name: Hadolint steps: - - uses: actions/checkout@v2 - - uses: brpaz/hadolint-action@v1.2.1 + - uses: actions/checkout@v3 + - uses: hadolint/hadolint-action@v3.1.0 name: Run Hadolint with: dockerfile: Dockerfile diff --git a/.gitignore b/.gitignore index 090feba9b..d830af5c2 100644 --- a/.gitignore +++ b/.gitignore @@ -82,6 +82,9 @@ Temporary Items # Local History for Visual Studio Code .history/ +### IntelliJ ### +.idea + ### VisualStudioCode Patch ### # Ignore all local history of files .history diff --git a/Dockerfile.rhel7 b/Dockerfile.rhel7 deleted file mode 100644 index 824577d1b..000000000 --- a/Dockerfile.rhel7 +++ /dev/null @@ -1,17 +0,0 @@ -FROM registry.svc.ci.openshift.org/ocp/builder:golang-1.10 AS builder - -COPY . /usr/src/sriov-cni - -WORKDIR /usr/src/sriov-cni -RUN make clean && \ - make build - -FROM registry.svc.ci.openshift.org/ocp/4.0:base -COPY --from=builder /usr/src/sriov-cni/build/sriov /usr/bin/ -WORKDIR / - -LABEL io.k8s.display-name="SR-IOV CNI" - -COPY ./images/entrypoint.sh / - -ENTRYPOINT ["/entrypoint.sh"] diff --git a/Makefile b/Makefile index 6aa837008..73a27a024 100644 --- a/Makefile +++ b/Makefile @@ -14,6 +14,7 @@ BASE=$(GOPATH)/src/$(REPO_PATH) GOFILES = $(shell find . -name *.go | grep -vE "(\/vendor\/)|(_test.go)") PKGS = $(or $(PKG),$(shell cd $(BASE) && env GOPATH=$(GOPATH) $(GO) list ./... | grep -v "^$(PACKAGE)/vendor/")) TESTPKGS = $(shell env GOPATH=$(GOPATH) $(GO) list -f '{{ if or .TestGoFiles .XTestGoFiles }}{{ .ImportPath }}{{ end }}' $(PKGS)) +IMAGE_BUILDER ?= docker export GOPATH export GOBIN @@ -33,7 +34,6 @@ endif # Go tools GO = go -GODOC = godoc GOFMT = gofmt TIMEOUT = 15 V ?= 0 @@ -63,7 +63,7 @@ $(BUILDDIR)/$(BINARY_NAME): $(GOFILES) | $(BUILDDIR) GOLANGCILINT = $(GOBIN)/golangci-lint $(GOLANGCILINT): | $(BASE) ; $(info Installing golangci-lint...) - $Q go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.45 + $Q go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.52.2 GOCOVMERGE = $(GOBIN)/gocovmerge $(GOCOVMERGE): | $(BASE) ; $(info Building gocovmerge...) @@ -137,7 +137,10 @@ fmt: ; $(info Running gofmt...) @ ## Run gofmt on all source files # To pass proxy for Docker invoke it as 'make image HTTP_POXY=http://192.168.0.1:8080' .PHONY: image image: | $(BASE) ; $(info Building Docker image...) @ ## Build SR-IOV CNI docker image - @docker build -t $(TAG) -f $(DOCKERFILE) $(CURDIR) $(DOCKERARGS) + @$(IMAGE_BUILDER) build -t $(TAG) -f $(DOCKERFILE) $(CURDIR) $(DOCKERARGS) + +test-image: image + $Q $(BASE)/images/image_test.sh $(IMAGE_BUILDER) $(TAG) # Misc diff --git a/OWNERS b/OWNERS new file mode 100644 index 000000000..19c3d44a4 --- /dev/null +++ b/OWNERS @@ -0,0 +1,8 @@ +## ADMINS: People who control settings for the repo. +adrianchiris +dougbtv + +## Maintainers: People who can merge code in this repo. +zeeke +SchSeba +Eoghan1232 diff --git a/README.md b/README.md index 55edad110..6e8429500 100644 --- a/README.md +++ b/README.md @@ -44,7 +44,7 @@ To deploy SR-IOV CNI by itself on a Kubernetes 1.16+ cluster: ## Usage -SR-IOV CNI networks are commonly configured using Multus and SR-IOV Device Plugin using Network Attachment Definitions. More information about configuring Kubernetes networks using this pattern can be found in the [Multus configuration reference document.](https://intel.github.io/multus-cni/docs/configuration.html) +SR-IOV CNI networks are commonly configured using Multus and SR-IOV Device Plugin using Network Attachment Definitions. More information about configuring Kubernetes networks using this pattern can be found in the [Multus configuration reference document.](https://github.com/k8snetworkplumbingwg/multus-cni/blob/master/docs/configuration.md) A Network Attachment Definition for SR-IOV CNI takes the form: diff --git a/cmd/sriov/main.go b/cmd/sriov/main.go index 06dd05697..e844d5074 100644 --- a/cmd/sriov/main.go +++ b/cmd/sriov/main.go @@ -4,6 +4,7 @@ import ( "errors" "fmt" "runtime" + "strings" "github.com/containernetworking/cni/pkg/skel" "github.com/containernetworking/cni/pkg/types" @@ -12,6 +13,7 @@ import ( "github.com/containernetworking/plugins/pkg/ipam" "github.com/containernetworking/plugins/pkg/ns" "github.com/k8snetworkplumbingwg/sriov-cni/pkg/config" + "github.com/k8snetworkplumbingwg/sriov-cni/pkg/logging" "github.com/k8snetworkplumbingwg/sriov-cni/pkg/sriov" "github.com/k8snetworkplumbingwg/sriov-cni/pkg/utils" "github.com/vishvananda/netlink" @@ -42,7 +44,13 @@ func getEnvArgs(envArgsString string) (*envArgs, error) { } func cmdAdd(args *skel.CmdArgs) error { - var macAddr string + if err := config.SetLogging(args.StdinData, args.ContainerID, args.Netns, args.IfName); err != nil { + return err + } + logging.Debug("function called", + "func", "cmdAdd", + "args.Path", args.Path, "args.StdinData", string(args.StdinData), "args.Args", args.Args) + netConf, err := config.LoadConf(args.StdinData) if err != nil { return fmt.Errorf("SRIOV-CNI failed to load netconf: %v", err) @@ -67,6 +75,9 @@ func cmdAdd(args *skel.CmdArgs) error { netConf.MAC = netConf.RuntimeConfig.Mac } + // Always use lower case for mac address + netConf.MAC = strings.ToLower(netConf.MAC) + netns, err := ns.GetNS(args.Netns) if err != nil { return fmt.Errorf("failed to open netns %q: %v", netns, err) @@ -85,7 +96,7 @@ func cmdAdd(args *skel.CmdArgs) error { return err }) if err == nil { - _ = sm.ReleaseVF(netConf, args.IfName, args.ContainerID, netns) + _ = sm.ReleaseVF(netConf, args.IfName, netns) } // Reset the VF if failure occurs before the netconf is cached _ = sm.ResetVFConfig(netConf) @@ -102,14 +113,15 @@ func cmdAdd(args *skel.CmdArgs) error { }} if !netConf.DPDKMode { - macAddr, err = sm.SetupVF(netConf, args.IfName, args.ContainerID, netns) + err = sm.SetupVF(netConf, args.IfName, netns) if err != nil { return fmt.Errorf("failed to set up pod interface %q from the device %q: %v", args.IfName, netConf.Master, err) } - result.Interfaces[0].Mac = macAddr } + result.Interfaces[0].Mac = config.GetMacAddressForResult(netConf) + // run the IPAM plugin if netConf.IPAM.Type != "" { var r types.Result @@ -175,12 +187,20 @@ func cmdAdd(args *skel.CmdArgs) error { } // Cache NetConf for CmdDel + logging.Debug("Cache NetConf for CmdDel", + "func", "cmdAdd", + "config.DefaultCNIDir", config.DefaultCNIDir, + "netConf", netConf) if err = utils.SaveNetConf(args.ContainerID, config.DefaultCNIDir, args.IfName, netConf); err != nil { return fmt.Errorf("error saving NetConf %q", err) } + // Mark the pci address as in use. + logging.Debug("Mark the PCI address as in use", + "func", "cmdAdd", + "config.DefaultCNIDir", config.DefaultCNIDir, + "netConf.DeviceID", netConf.DeviceID) allocator := utils.NewPCIAllocator(config.DefaultCNIDir) - // Mark the pci address as in used if err = allocator.SaveAllocatedPCI(netConf.DeviceID, args.Netns); err != nil { return fmt.Errorf("error saving the pci allocation for vf pci address %s: %v", netConf.DeviceID, err) } @@ -189,6 +209,13 @@ func cmdAdd(args *skel.CmdArgs) error { } func cmdDel(args *skel.CmdArgs) error { + if err := config.SetLogging(args.StdinData, args.ContainerID, args.Netns, args.IfName); err != nil { + return err + } + logging.Debug("function called", + "func", "cmdDel", + "args.Path", args.Path, "args.StdinData", string(args.StdinData), "args.Args", args.Args) + netConf, cRefPath, err := config.LoadConfFromCache(args) if err != nil { // If cmdDel() fails, cached netconf is cleaned up by @@ -198,6 +225,9 @@ func cmdDel(args *skel.CmdArgs) error { // Return nil when LoadConfFromCache fails since the rest // of cmdDel() code relies on netconf as input argument // and there is no meaning to continue. + logging.Error("Cannot load config file from cache", + "func", "cmdDel", + "err", err) return nil } @@ -219,6 +249,11 @@ func cmdDel(args *skel.CmdArgs) error { return nil } + // Verify VF ID existence. + if _, err := utils.GetVfid(netConf.DeviceID, netConf.Master); err != nil { + return fmt.Errorf("cmdDel() error obtaining VF ID: %q", err) + } + sm := sriov.NewSriovManager() /* ResetVFConfig resets a VF administratively. We must run ResetVFConfig @@ -246,12 +281,16 @@ func cmdDel(args *skel.CmdArgs) error { } defer netns.Close() - if err = sm.ReleaseVF(netConf, args.IfName, args.ContainerID, netns); err != nil { + if err = sm.ReleaseVF(netConf, args.IfName, netns); err != nil { return err } } // Mark the pci address as released + logging.Debug("Mark the PCI address as released", + "func", "cmdDel", + "config.DefaultCNIDir", config.DefaultCNIDir, + "netConf.DeviceID", netConf.DeviceID) allocator := utils.NewPCIAllocator(config.DefaultCNIDir) if err = allocator.DeleteAllocatedPCI(netConf.DeviceID); err != nil { return fmt.Errorf("error cleaning the pci allocation for vf pci address %s: %v", netConf.DeviceID, err) @@ -260,7 +299,7 @@ func cmdDel(args *skel.CmdArgs) error { return nil } -func cmdCheck(args *skel.CmdArgs) error { +func cmdCheck(_ *skel.CmdArgs) error { return nil } diff --git a/docs/configuration-reference.md b/docs/configuration-reference.md index aebc9eb60..7f7904956 100644 --- a/docs/configuration-reference.md +++ b/docs/configuration-reference.md @@ -10,6 +10,7 @@ The SR-IOV CNI configures networks through a CNI spec configuration object. In a * `deviceID` (string, required): A valid pci address of an SRIOV NIC's VF. e.g. "0000:03:02.3" * `vlan` (int, optional): VLAN ID to assign for the VF. Value must be in the range 0-4094 (0 for disabled, 1-4094 for valid VLAN IDs). * `vlanQoS` (int, optional): VLAN QoS to assign for the VF. Value must be in the range 0-7. This option requires `vlan` field to be set to a non-zero value. Otherwise, the error will be returned. +* `vlanProto` (string, optional): VLAN protocol to assign for the VF. Allowed values: "802.1ad", "802.1q" (default). * `mac` (string, optional): MAC address to assign for the VF * `spoofchk` (string, optional): turn packet spoof checking on or off for the VF * `trust` (string, optional): turn trust setting on or off for the VF @@ -17,6 +18,9 @@ The SR-IOV CNI configures networks through a CNI spec configuration object. In a * `min_tx_rate` (int, optional): change the allowed minimum transmit bandwidth, in Mbps, for the VF. Setting this to 0 disables rate limiting. The min_tx_rate value should be <= max_tx_rate. Support of this feature depends on NICs and drivers. * `max_tx_rate` (int, optional): change the allowed maximum transmit bandwidth, in Mbps, for the VF. Setting this to 0 disables rate limiting. +* `logLevel` (string, optional): either of panic, error, warning, info, debug with a default of info. +* `logFile` (string, optional): path to file for log output. By default, this will log to stderr. Logging to stderr +means that the logs will show up in crio logs (in the journal in most configurations) and in multus pod logs. An SR-IOV CNI config with each field filled out looks like: @@ -27,14 +31,17 @@ An SR-IOV CNI config with each field filled out looks like: "name": "sriov-dpdk", "type": "sriovi-net", "deviceID": "0000:03:02.0", - "vlan": 1000, "mac": "CA:FE:C0:FF:EE:00", + "vlan": 1000, "vlanQoS": 4, + "vlanProto": "802.1ad", "min_tx_rate": 100, "max_tx_rate": 200, "spoofchk": "off", "trust": "on", - "link_state": "enable" + "link_state": "enable", + "logLevel": "debug", + "logFile": "/tmp/sriov.log" } ``` diff --git a/go.mod b/go.mod index a0f67238c..ae348dd14 100644 --- a/go.mod +++ b/go.mod @@ -1,28 +1,38 @@ module github.com/k8snetworkplumbingwg/sriov-cni -go 1.19 +go 1.21 require ( github.com/containernetworking/cni v1.1.2 - github.com/containernetworking/plugins v1.1.1 - github.com/onsi/ginkgo v1.16.4 - github.com/onsi/gomega v1.17.0 - github.com/stretchr/testify v1.5.1 - github.com/vishvananda/netlink v1.1.1-0.20210330154013-f5de75959ad5 - golang.org/x/net v0.7.0 + github.com/containernetworking/plugins v1.2.0 + github.com/k8snetworkplumbingwg/cni-log v0.0.0-20230801160229-b6e062c9e0f2 + github.com/onsi/ginkgo/v2 v2.9.2 + github.com/onsi/gomega v1.27.5 + github.com/stretchr/testify v1.6.1 + github.com/vishvananda/netlink v1.2.1-beta.2.0.20230905152006-63484bbf69f8 + golang.org/x/net v0.17.0 ) require ( github.com/coreos/go-iptables v0.6.0 // indirect github.com/davecgh/go-spew v1.1.1 // indirect - github.com/fsnotify/fsnotify v1.4.9 // indirect - github.com/nxadm/tail v1.4.8 // indirect + github.com/go-logr/logr v1.2.3 // indirect + github.com/go-task/slim-sprig v0.0.0-20230315185526-52ccab3ef572 // indirect + github.com/google/go-cmp v0.5.9 // indirect + github.com/google/pprof v0.0.0-20230323073829-e72429f035bd // indirect github.com/pmezard/go-difflib v1.0.0 // indirect - github.com/safchain/ethtool v0.0.0-20210803160452-9aa261dae9b1 // indirect + github.com/safchain/ethtool v0.2.0 // indirect github.com/stretchr/objx v0.1.0 // indirect - github.com/vishvananda/netns v0.0.0-20210104183010-2eb08e3e575f // indirect - golang.org/x/sys v0.5.0 // indirect - golang.org/x/text v0.7.0 // indirect - gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7 // indirect + github.com/vishvananda/netns v0.0.4 // indirect + golang.org/x/sys v0.13.0 // indirect + golang.org/x/text v0.13.0 // indirect + golang.org/x/tools v0.7.0 // indirect + gopkg.in/natefinch/lumberjack.v2 v2.0.0 // indirect gopkg.in/yaml.v2 v2.4.0 // indirect + gopkg.in/yaml.v3 v3.0.1 // indirect +) + +replace ( + github.com/onsi/ginkgo/v2 => github.com/onsi/ginkgo/v2 v2.9.2 + github.com/onsi/gomega => github.com/onsi/gomega v1.27.5 ) diff --git a/go.sum b/go.sum index a1f761015..a0d02566e 100644 --- a/go.sum +++ b/go.sum @@ -1,123 +1,111 @@ +github.com/BurntSushi/toml v1.1.0 h1:ksErzDEI1khOiGPgpwuI7x2ebx/uXQNw7xJpn9Eq1+I= github.com/chzyer/logex v1.1.10/go.mod h1:+Ywpsq7O8HXn0nuIou7OrIPyXbp3wmkHB+jjWRnGsAI= github.com/chzyer/readline v0.0.0-20180603132655-2972be24d48e/go.mod h1:nSuG5e5PlCu98SY8svDHJxuZscDgtXS6KTTbou5AhLI= github.com/chzyer/test v0.0.0-20180213035817-a1ea475d72b1/go.mod h1:Q3SI9o4m/ZMnBNeIyt5eFwwo7qiLfzFZmjNmxjkiQlU= github.com/containernetworking/cni v1.1.2 h1:wtRGZVv7olUHMOqouPpn3cXJWpJgM6+EUl31EQbXALQ= github.com/containernetworking/cni v1.1.2/go.mod h1:sDpYKmGVENF3s6uvMvGgldDWeG8dMxakj/u+i9ht9vw= -github.com/containernetworking/plugins v1.1.1 h1:+AGfFigZ5TiQH00vhR8qPeSatj53eNGz0C1d3wVYlHE= -github.com/containernetworking/plugins v1.1.1/go.mod h1:Sr5TH/eBsGLXK/h71HeLfX19sZPp3ry5uHSkI4LPxV8= +github.com/containernetworking/plugins v1.2.0 h1:SWgg3dQG1yzUo4d9iD8cwSVh1VqI+bP7mkPDoSfP9VU= +github.com/containernetworking/plugins v1.2.0/go.mod h1:/VjX4uHecW5vVimFa1wkG4s+r/s9qIfPdqlLF4TW8c4= github.com/coreos/go-iptables v0.6.0 h1:is9qnZMPYjLd8LYqmm/qlE+wwEgJIkTYdhV3rfZo4jk= github.com/coreos/go-iptables v0.6.0/go.mod h1:Qe8Bv2Xik5FyTXwgIbLAnv2sWSBmvWdFETJConOQ//Q= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= -github.com/fsnotify/fsnotify v1.4.7/go.mod h1:jwhsz4b93w/PPRr/qN1Yymfu8t87LnFCMoQvtojpjFo= github.com/fsnotify/fsnotify v1.4.9 h1:hsms1Qyu0jgnwNXIxa+/V/PDsU6CfLf6CNO8H7IWoS4= -github.com/fsnotify/fsnotify v1.4.9/go.mod h1:znqG4EE+3YCdAaPaxE2ZRY/06pZUdp0tY4IgpuI1SZQ= -github.com/go-task/slim-sprig v0.0.0-20210107165309-348f09dbbbc0/go.mod h1:fyg7847qk6SyHyPtNmDHnmrv/HOrqktSC+C9fM+CJOE= -github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= -github.com/golang/protobuf v1.4.0-rc.1/go.mod h1:ceaxUfeHdC40wWswd/P6IGgMaK3YpKi5j83Wpe3EHw8= -github.com/golang/protobuf v1.4.0-rc.1.0.20200221234624-67d41d38c208/go.mod h1:xKAWHe0F5eneWXFV3EuXVDTCmh+JuBKY0li0aMyXATA= -github.com/golang/protobuf v1.4.0-rc.2/go.mod h1:LlEzMj4AhA7rCAGe4KMBDvJI+AwstrUpVNzEA03Pprs= -github.com/golang/protobuf v1.4.0-rc.4.0.20200313231945-b860323f09d0/go.mod h1:WU3c8KckQ9AFe+yFwt9sWVRKCVIyN9cPHBJSNnbL67w= -github.com/golang/protobuf v1.4.0/go.mod h1:jodUvKwWbYaEsadDk5Fwe5c77LiNKVO9IDvqG2KuDX0= -github.com/golang/protobuf v1.4.2/go.mod h1:oDoupMAO8OvCJWAcko0GGGIgR6R6ocIYbsSw735rRwI= +github.com/go-logr/logr v1.2.3 h1:2DntVwHkVopvECVRSlL5PSo9eG+cAkDCuckLubN+rq0= +github.com/go-logr/logr v1.2.3/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A= +github.com/go-task/slim-sprig v0.0.0-20230315185526-52ccab3ef572 h1:tfuBGBXKqDEevZMzYi5KSi8KkcZtzBcTgAUUtapy0OI= +github.com/go-task/slim-sprig v0.0.0-20230315185526-52ccab3ef572/go.mod h1:9Pwr4B2jHnOSGXyyzV8ROjYa2ojvAY6HCGYYfMoC3Ls= github.com/golang/protobuf v1.5.0/go.mod h1:FsONVRAS9T7sI+LIUmWTfcYkHO4aIWwzhcaSAoJOfIk= -github.com/golang/protobuf v1.5.2 h1:ROPKBNFfQgOUMifHyP+KYbvpjbdoFNs+aK7DXlji0Tw= -github.com/golang/protobuf v1.5.2/go.mod h1:XVQd3VNwM+JqD3oG2Ue2ip4fOMUkwXdXDdiuN0vRsmY= -github.com/google/go-cmp v0.3.0/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU= -github.com/google/go-cmp v0.3.1/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU= -github.com/google/go-cmp v0.4.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= +github.com/golang/protobuf v1.5.3 h1:KhyjKVUg7Usr/dYsdSqoFveMYd5ko72D+zANwlG1mmg= +github.com/golang/protobuf v1.5.3/go.mod h1:XVQd3VNwM+JqD3oG2Ue2ip4fOMUkwXdXDdiuN0vRsmY= github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= +github.com/google/go-cmp v0.5.9 h1:O2Tfq5qg4qc4AmwVlvv0oLiVAGB7enBSJ2x2DqQFi38= +github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= github.com/google/pprof v0.0.0-20210407192527-94a9f03dee38/go.mod h1:kpwsk12EmLew5upagYY7GY0pfYCcupk39gWOCRROcvE= -github.com/hpcloud/tail v1.0.0/go.mod h1:ab1qPbhIpdTxEkNHXyeSf5vhxWSCs/tWer42PpOxQnU= +github.com/google/pprof v0.0.0-20230323073829-e72429f035bd h1:r8yyd+DJDmsUhGrRBxH5Pj7KeFK5l+Y3FsgT8keqKtk= +github.com/google/pprof v0.0.0-20230323073829-e72429f035bd/go.mod h1:79YE0hCXdHag9sBkw2o+N/YnZtTkXi0UT9Nnixa5eYk= github.com/ianlancetaylor/demangle v0.0.0-20200824232613-28f6c0f3b639/go.mod h1:aSSvb/t6k1mPoxDqO4vJh6VOCGPwU4O0C2/Eqndh1Sc= -github.com/nxadm/tail v1.4.4/go.mod h1:kenIhsEOeOJmVchQTgglprH7qJGnHDVpk1VPCcaMI8A= +github.com/k8snetworkplumbingwg/cni-log v0.0.0-20230801160229-b6e062c9e0f2 h1:KB8UPZQwLge4Abuk9tNmvzffdCJgqXSN341BX98QTHg= +github.com/k8snetworkplumbingwg/cni-log v0.0.0-20230801160229-b6e062c9e0f2/go.mod h1:/x45AlZDoJVSSV4ECDb5TcHLzrVRDllsCMDzMrtHKwk= github.com/nxadm/tail v1.4.8 h1:nPr65rt6Y5JFSKQO7qToXr7pePgD6Gwiw05lkbyAQTE= -github.com/nxadm/tail v1.4.8/go.mod h1:+ncqLTQzXmGhMZNUePPaPqPvBxHAIsmXswZKocGu+AU= -github.com/onsi/ginkgo v1.6.0/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE= -github.com/onsi/ginkgo v1.12.1/go.mod h1:zj2OWP4+oCPe1qIXoGWkgMRwljMUYCdkwsT2108oapk= -github.com/onsi/ginkgo v1.16.4 h1:29JGrr5oVBm5ulCWet69zQkzWipVXIol6ygQUe/EzNc= -github.com/onsi/ginkgo v1.16.4/go.mod h1:dX+/inL/fNMqNlz0e9LfyB9TswhZpCVdJM/Z6Vvnwo0= -github.com/onsi/ginkgo/v2 v2.1.3 h1:e/3Cwtogj0HA+25nMP1jCMDIf8RtRYbGwGGuBIFztkc= -github.com/onsi/ginkgo/v2 v2.1.3/go.mod h1:vw5CSIxN1JObi/U8gcbwft7ZxR2dgaR70JSE3/PpL4c= -github.com/onsi/gomega v1.7.1/go.mod h1:XdKZgCCFLUoM/7CFJVPcG8C1xQ1AJ0vpAezJrB7JYyY= -github.com/onsi/gomega v1.10.1/go.mod h1:iN09h71vgCQne3DLsj+A5owkum+a2tYe+TOCB1ybHNo= -github.com/onsi/gomega v1.17.0 h1:9Luw4uT5HTjHTN8+aNcSThgH1vdXnmdJ8xIfZ4wyTRE= -github.com/onsi/gomega v1.17.0/go.mod h1:HnhC7FXeEQY45zxNK3PPoIUhzk/80Xly9PcubAlGdZY= +github.com/onsi/ginkgo v1.16.5 h1:8xi0RTUf59SOSfEtZMvwTvXYMzG4gV23XVHOZiXNtnE= +github.com/onsi/ginkgo/v2 v2.9.2 h1:BA2GMJOtfGAfagzYtrAlufIP0lq6QERkFmHLMLPwFSU= +github.com/onsi/ginkgo/v2 v2.9.2/go.mod h1:WHcJJG2dIlcCqVfBAwUCrJxSPFb6v4azBwgxeMeDuts= +github.com/onsi/gomega v1.27.5 h1:T/X6I0RNFw/kTqgfkZPcQ5KU6vCnWNBGdtrIx2dpGeQ= +github.com/onsi/gomega v1.27.5/go.mod h1:PIQNjfQwkP3aQAH7lf7j87O/5FiNr+ZR8+ipb+qQlhg= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= -github.com/safchain/ethtool v0.0.0-20210803160452-9aa261dae9b1 h1:ZFfeKAhIQiiOrQaI3/znw0gOmYpO28Tcu1YaqMa/jtQ= -github.com/safchain/ethtool v0.0.0-20210803160452-9aa261dae9b1/go.mod h1:Z0q5wiBQGYcxhMZ6gUqHn6pYNLypFAvaL3UvgZLR0U4= +github.com/safchain/ethtool v0.2.0 h1:dILxMBqDnQfX192cCAPjZr9v2IgVXeElHPy435Z/IdE= +github.com/safchain/ethtool v0.2.0/go.mod h1:WkKB1DnNtvsMlDmQ50sgwowDJV/hGbJSOvJoEXs1AJQ= github.com/stretchr/objx v0.1.0 h1:4G4v2dO3VZwixGIRoQ5Lfboy6nUhCyYzaqnIAPPhYs4= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= -github.com/stretchr/testify v1.5.1 h1:nOGnQDM7FYENwehXlg/kFVnos3rEvtKTjRvOWSzb6H4= -github.com/stretchr/testify v1.5.1/go.mod h1:5W2xD1RspED5o8YsWQXVCued0rvSQ+mT+I5cxcmMvtA= -github.com/vishvananda/netlink v1.1.1-0.20210330154013-f5de75959ad5 h1:+UB2BJA852UkGH42H+Oee69djmxS3ANzl2b/JtT1YiA= -github.com/vishvananda/netlink v1.1.1-0.20210330154013-f5de75959ad5/go.mod h1:twkDnbuQxJYemMlGd4JFIcuhgX83tXhKS2B/PRMpOho= +github.com/stretchr/testify v1.6.1 h1:hDPOHmpOpP40lSULcqw7IrRb/u7w6RpDC9399XyoNd0= +github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= +github.com/vishvananda/netlink v1.2.1-beta.2.0.20230905152006-63484bbf69f8 h1:xxfANIR2aBrzBufGZZkbIg0N4V1AEmw0hKWlWv8eMYM= +github.com/vishvananda/netlink v1.2.1-beta.2.0.20230905152006-63484bbf69f8/go.mod h1:whJevzBpTrid75eZy99s3DqCmy05NfibNaF2Ol5Ox5A= github.com/vishvananda/netns v0.0.0-20200728191858-db3c7e526aae/go.mod h1:DD4vA1DwXk04H54A1oHXtwZmA0grkVMdPxx/VGLCah0= -github.com/vishvananda/netns v0.0.0-20210104183010-2eb08e3e575f h1:p4VB7kIXpOQvVn1ZaTIVp+3vuYAXFe3OJEvjbUYJLaA= -github.com/vishvananda/netns v0.0.0-20210104183010-2eb08e3e575f/go.mod h1:DD4vA1DwXk04H54A1oHXtwZmA0grkVMdPxx/VGLCah0= -github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= +github.com/vishvananda/netns v0.0.4 h1:Oeaw1EM2JMxD51g9uhtC0D7erkIjgmj8+JZc26m1YX8= +github.com/vishvananda/netns v0.0.4/go.mod h1:SpkAiCQRtJ6TvvxPnOSyH3BMl6unz3xZlaprSwhNNJM= +github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= -golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= -golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= -golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= -golang.org/x/net v0.0.0-20180906233101-161cd47e91fd/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= -golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= +golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc= +golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4= +golang.org/x/mod v0.8.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= +golang.org/x/mod v0.9.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= -golang.org/x/net v0.0.0-20200520004742-59133d7f0dd7/go.mod h1:qpuaurCH72eLCgpAm/N6yyVIVM9cpaDIP3A8BGJEC5A= -golang.org/x/net v0.0.0-20201021035429-f5854403a974/go.mod h1:sp8m0HH+o8qH0wwXwYZr8TS3Oi6o0r6Gce1SSxlDquU= -golang.org/x/net v0.0.0-20210428140749-89ef3d95e781/go.mod h1:OJAsFXCWl8Ukc7SiCT/9KSuxbyM7479/AVlXFRxuMCk= -golang.org/x/net v0.7.0 h1:rJrUqqhjsgNp7KqAIc25s9pZnjU7TUcSY7HcVZjdn1g= -golang.org/x/net v0.7.0/go.mod h1:2Tu9+aMcznHK/AK1HMvgo6xiTLG5rD5rZLDS+rp2Bjs= -golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= +golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg= +golang.org/x/net v0.0.0-20220722155237-a158d28d115b/go.mod h1:XRhObCWvk6IyKnWLug+ECip1KBveYUHfp+8e9klMJ9c= +golang.org/x/net v0.6.0/go.mod h1:2Tu9+aMcznHK/AK1HMvgo6xiTLG5rD5rZLDS+rp2Bjs= +golang.org/x/net v0.8.0/go.mod h1:QVkue5JL9kW//ek3r6jTKnTFis1tRmNAW2P1shuFdJc= +golang.org/x/net v0.17.0 h1:pVaXccu2ozPjCXewfr1S7xza/zcXTity9cCdXQYSjIM= +golang.org/x/net v0.17.0/go.mod h1:NxSsAGuq816PNPmqtQdLE42eU2Fs7NoRIZrHJAlaCOE= golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= -golang.org/x/sync v0.0.0-20201020160332-67f06af15bc9/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= -golang.org/x/sys v0.0.0-20180909124046-d0be0721c37e/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= +golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= +golang.org/x/sync v0.1.0/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= -golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.0.0-20190904154756-749cb33beabd/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.0.0-20191005200804-aed5e4c7ecf9/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.0.0-20191120155948-bd437916bb0e/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20191204072324-ce4227a45e2e/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200217220822-9197077df867/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.0.0-20200323222414-85ca7c5b95cd/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.0.0-20200728102440-3e129f6d46b1/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.0.0-20210112080510-489259a85091/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.0.0-20210423082822-04245dca01da/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.5.0 h1:MUK/U/4lj1t1oPg0HfuXDN/Z1wv31ZJ/YcPiGccS4DU= +golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.0.0-20210630005230-0f9fa26af87c/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.10.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.13.0 h1:Af8nKPmuFypiUBjVoU9V20FiaFXOcuZI21p0ycVYYGE= +golang.org/x/sys v0.13.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= +golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= +golang.org/x/term v0.5.0/go.mod h1:jMB1sMXY+tzblOD4FWmEbocvup2/aLOaQEp7JmGp78k= +golang.org/x/term v0.6.0/go.mod h1:m6U89DPEgQRMq3DNkDClhWw02AUbt2daBVO4cn4Hv9U= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= -golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= -golang.org/x/text v0.7.0 h1:4BRB4x83lYWy72KwLD/qYDuTu7q9PjSagHvijDw7cLo= +golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ= golang.org/x/text v0.7.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8= +golang.org/x/text v0.8.0/go.mod h1:e1OnstbJyHTd6l/uOt8jFFHp6TRDWZR/bV3emEE/zU8= +golang.org/x/text v0.13.0 h1:ablQoSUd0tRdKxZewP80B+BaqeKJuVhuRxj/dkrun3k= +golang.org/x/text v0.13.0/go.mod h1:TvPlkZtksWOMsz7fbANvkp4WM8x/WCo/om8BMLbz+aE= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= -golang.org/x/tools v0.0.0-20201224043029-2b0845dc783e/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA= +golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc= +golang.org/x/tools v0.6.0/go.mod h1:Xwgl3UAJ/d3gWutnCtw505GrjyAbvKui8lOU390QaIU= +golang.org/x/tools v0.7.0 h1:W4OVu8VVOaIO0yzWMNdepAulS7YfoS3Zabrm8DOXXU4= +golang.org/x/tools v0.7.0/go.mod h1:4pg6aUX35JBAogB10C9AtvVL+qowtN4pT3CGSQex14s= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= -golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= -golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= -google.golang.org/protobuf v0.0.0-20200109180630-ec00e32a8dfd/go.mod h1:DFci5gLYBciE7Vtevhsrf46CRTquxDuWsQurQQe4oz8= -google.golang.org/protobuf v0.0.0-20200221191635-4d8936d0db64/go.mod h1:kwYJMbMJ01Woi6D6+Kah6886xMZcty6N08ah7+eCXa0= -google.golang.org/protobuf v0.0.0-20200228230310-ab0ca4ff8a60/go.mod h1:cfTl7dwQJ+fmap5saPgwCLgHXTUD7jkjRqWcaiX5VyM= -google.golang.org/protobuf v1.20.1-0.20200309200217-e05f789c0967/go.mod h1:A+miEFZTKqfCUM6K7xSMQL9OKL/b6hQv+e19PK+JZNE= -google.golang.org/protobuf v1.21.0/go.mod h1:47Nbq4nVaFHyn7ilMalzfO3qCViNmqZ2kzikPIcrTAo= -google.golang.org/protobuf v1.23.0/go.mod h1:EGpADcykh3NcUnDUJcl1+ZksZNG86OlYog2l/sGQquU= google.golang.org/protobuf v1.26.0-rc.1/go.mod h1:jlhhOSvTdKEhbULTjvd4ARK9grFBp09yW+WbY/TyQbw= -google.golang.org/protobuf v1.26.0 h1:bxAC2xTBsZGibn2RTntX0oH50xLsqy1OxA9tTL3p/lk= google.golang.org/protobuf v1.26.0/go.mod h1:9q0QmTI4eRPtz6boOQmLYwt+qCgq0jsYwAQnmE0givc= +google.golang.org/protobuf v1.28.0 h1:w43yiav+6bVFTBQFZX0r7ipe9JQ1QsbMgHwbBziscLw= +google.golang.org/protobuf v1.28.0/go.mod h1:HV8QOd/L58Z+nl8r43ehVNZIU/HEI6OcFqwMG9pJV4I= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= -gopkg.in/fsnotify.v1 v1.4.7/go.mod h1:Tz8NjZHkW78fSQdbUxIjBTcgA1z1m8ZHf0WmKUhAMys= +gopkg.in/natefinch/lumberjack.v2 v2.0.0 h1:1Lc07Kr7qY4U2YPouBjpCLxpiyxIVoxqXgkXLknAOE8= +gopkg.in/natefinch/lumberjack.v2 v2.0.0/go.mod h1:l0ndWWf7gzL7RNwBG7wST/UCcT4T24xpD6X8LsfU/+k= gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7 h1:uRGJdciOHaEIrze2W8Q3AKkepLTh2hOroT7a+7czfdQ= -gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7/go.mod h1:dt/ZhP58zS4L8KSrWDmTeBkI65Dw0HsyUHuEVlX15mw= -gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= -gopkg.in/yaml.v2 v2.2.4/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= -gopkg.in/yaml.v2 v2.3.0/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.4.0 h1:D8xgwECY7CYvx+Y2n4sBz93Jn9JRvxdiyyo8CTfuKaY= gopkg.in/yaml.v2 v2.4.0/go.mod h1:RDklbk79AGWmwhnvt/jBztapEOGDOx6ZbXqjP6csGnQ= +gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= +gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= +gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/images/entrypoint.sh b/images/entrypoint.sh index fb2315c94..906917599 100755 --- a/images/entrypoint.sh +++ b/images/entrypoint.sh @@ -6,6 +6,7 @@ set -e # Set known directories. CNI_BIN_DIR="/host/opt/cni/bin" SRIOV_BIN_FILE="/usr/bin/sriov" +NO_SLEEP=0 # Give help text for parameters. usage() @@ -18,6 +19,7 @@ usage() printf "\t-h --help\n" printf "\t--cni-bin-dir=%s\n" "$CNI_BIN_DIR" printf "\t--sriov-bin-file=%s\n" "$SRIOV_BIN_FILE" + printf "\t--no-sleep\n" } # Parse parameters given as arguments to this script. @@ -35,6 +37,9 @@ while [ "$1" != "" ]; do --sriov-bin-file) SRIOV_BIN_FILE=$VALUE ;; + --no-sleep) + NO_SLEEP=1 + ;; *) /bin/echo "ERROR: unknown parameter \"$PARAM\"" usage @@ -57,6 +62,10 @@ done # Copy file into proper place. cp -f "$SRIOV_BIN_FILE" "$CNI_BIN_DIR" +if [ $NO_SLEEP -eq 1 ]; then + exit 0 +fi + echo "Entering sleep... (success)" trap : TERM INT diff --git a/images/image_test.sh b/images/image_test.sh new file mode 100755 index 000000000..a7adbbcec --- /dev/null +++ b/images/image_test.sh @@ -0,0 +1,22 @@ +#!/bin/sh + +set -x + +OCI_RUNTIME=$1 +IMAGE_UNDER_TEST=$2 + +OUTPUT_DIR=$(mktemp -d) + +"${OCI_RUNTIME}" run -v "${OUTPUT_DIR}:/out" "${IMAGE_UNDER_TEST}" --cni-bin-dir=/out --no-sleep + +if [ ! -e "${OUTPUT_DIR}/sriov" ]; then + echo "Output file ${OUTPUT_DIR}/sriov not found" + exit 1 +fi + +if [ ! -s "${OUTPUT_DIR}/sriov" ]; then + echo "Output file ${OUTPUT_DIR}/sriov is empty" + exit 1 +fi + +exit 0 diff --git a/pkg/config/config.go b/pkg/config/config.go index b2b6f706e..361fbbba8 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -7,6 +7,7 @@ import ( "strings" "github.com/containernetworking/cni/pkg/skel" + "github.com/k8snetworkplumbingwg/sriov-cni/pkg/logging" sriovtypes "github.com/k8snetworkplumbingwg/sriov-cni/pkg/types" "github.com/k8snetworkplumbingwg/sriov-cni/pkg/utils" ) @@ -16,6 +17,17 @@ var ( DefaultCNIDir = "/var/lib/cni/sriov" ) +// SetLogging sets global logging parameters. +func SetLogging(stdinData []byte, containerID, netns, ifName string) error { + n := &sriovtypes.NetConf{} + if err := json.Unmarshal(stdinData, n); err != nil { + return fmt.Errorf("SetLogging(): failed to load netconf: %v", err) + } + + logging.Init(n.LogLevel, n.LogFile, containerID, netns, ifName) + return nil +} + // LoadConf parses and validates stdin netconf and returns NetConf object func LoadConf(bytes []byte) (*sriovtypes.NetConf, error) { n := &sriovtypes.NetConf{} @@ -36,11 +48,15 @@ func LoadConf(bytes []byte) (*sriovtypes.NetConf, error) { return nil, fmt.Errorf("LoadConf(): VF pci addr is required") } - allocator := utils.NewPCIAllocator(DefaultCNIDir) // Check if the device is already allocated. // This is to prevent issues where kubelet request to delete a pod and in the same time a new pod using the same // vf is started. we can have an issue where the cmdDel of the old pod is called AFTER the cmdAdd of the new one // This will block the new pod creation until the cmdDel is done. + logging.Debug("Check if the device is already allocated", + "func", "LoadConf", + "DefaultCNIDir", DefaultCNIDir, + "n.DeviceID", n.DeviceID) + allocator := utils.NewPCIAllocator(DefaultCNIDir) isAllocated, err := allocator.IsAllocated(n.DeviceID) if err != nil { return n, err @@ -51,8 +67,8 @@ func LoadConf(bytes []byte) (*sriovtypes.NetConf, error) { } // Assuming VF is netdev interface; Get interface name(s) - hostIFNames, err := utils.GetVFLinkNames(n.DeviceID) - if err != nil || hostIFNames == "" { + hostIFName, err := utils.GetVFLinkName(n.DeviceID) + if err != nil || hostIFName == "" { // VF interface not found; check if VF has dpdk driver hasDpdkDriver, err := utils.HasDpdkDriver(n.DeviceID) if err != nil { @@ -61,38 +77,54 @@ func LoadConf(bytes []byte) (*sriovtypes.NetConf, error) { n.DPDKMode = hasDpdkDriver } - if hostIFNames != "" { - n.OrigVfState.HostIFName = hostIFNames + if hostIFName != "" { + n.OrigVfState.HostIFName = hostIFName } - if hostIFNames == "" && !n.DPDKMode { + if hostIFName == "" && !n.DPDKMode { return nil, fmt.Errorf("LoadConf(): the VF %s does not have a interface name or a dpdk driver", n.DeviceID) } - if n.Vlan != nil { - // validate vlan id range - if *n.Vlan < 0 || *n.Vlan > 4094 { - return nil, fmt.Errorf("LoadConf(): vlan id %d invalid: value must be in the range 0-4094", *n.Vlan) - } + if n.Vlan == nil { + vlan := 0 + n.Vlan = &vlan } - if n.VlanQoS != nil { - // validate that VLAN QoS is in the 0-7 range - if *n.VlanQoS < 0 || *n.VlanQoS > 7 { - return nil, fmt.Errorf("LoadConf(): vlan QoS PCP %d invalid: value must be in the range 0-7", *n.VlanQoS) - } + // validate vlan id range + if *n.Vlan < 0 || *n.Vlan > 4094 { + return nil, fmt.Errorf("LoadConf(): vlan id %d invalid: value must be in the range 0-4094", *n.Vlan) } - // validate that vlan id is set if vlan qos is set - if n.VlanQoS != nil && n.Vlan == nil { - return nil, fmt.Errorf(("LoadConf(): vlan id must be configured to set vlan QoS")) + if n.VlanQoS == nil { + qos := 0 + n.VlanQoS = &qos + } + + // validate that VLAN QoS is in the 0-7 range + if *n.VlanQoS < 0 || *n.VlanQoS > 7 { + return nil, fmt.Errorf("LoadConf(): vlan QoS PCP %d invalid: value must be in the range 0-7", *n.VlanQoS) } // validate non-zero value for vlan id if vlan qos is set to a non-zero value - if (n.VlanQoS != nil && *n.VlanQoS != 0) && *n.Vlan == 0 { + if *n.VlanQoS != 0 && *n.Vlan == 0 { return nil, fmt.Errorf("LoadConf(): non-zero vlan id must be configured to set vlan QoS to a non-zero value") } + if n.VlanProto == nil { + proto := sriovtypes.Proto8021q + n.VlanProto = &proto + } + + *n.VlanProto = strings.ToLower(*n.VlanProto) + if *n.VlanProto != sriovtypes.Proto8021ad && *n.VlanProto != sriovtypes.Proto8021q { + return nil, fmt.Errorf("LoadConf(): vlan Proto %s invalid: value must be '802.1Q' or '802.1ad'", *n.VlanProto) + } + + // validate non-zero value for vlan id if vlan proto is set to 802.1ad + if *n.VlanProto == sriovtypes.Proto8021ad && *n.Vlan == 0 { + return nil, fmt.Errorf("LoadConf(): non-zero vlan id must be configured to set vlan proto 802.1ad") + } + // validate that link state is one of supported values if n.LinkState != "" && n.LinkState != "auto" && n.LinkState != "enable" && n.LinkState != "disable" { return nil, fmt.Errorf("LoadConf(): invalid link_state value: %s", n.LinkState) @@ -136,3 +168,21 @@ func LoadConfFromCache(args *skel.CmdArgs) (*sriovtypes.NetConf, string, error) return netConf, cRefPath, nil } + +// GetMacAddressForResult return the mac address we should report to the CNI call return object +// if the device is on kernel mode we report that one back +// if not we check the administrative mac address on the PF +// if it is set and is not zero, report it. +func GetMacAddressForResult(netConf *sriovtypes.NetConf) string { + if netConf.MAC != "" { + return netConf.MAC + } + if !netConf.DPDKMode { + return netConf.OrigVfState.EffectiveMAC + } + if netConf.OrigVfState.AdminMAC != "00:00:00:00:00:00" { + return netConf.OrigVfState.AdminMAC + } + + return "" +} diff --git a/pkg/config/config_suite_test.go b/pkg/config/config_suite_test.go index 9d12c7b73..250602953 100644 --- a/pkg/config/config_suite_test.go +++ b/pkg/config/config_suite_test.go @@ -4,7 +4,7 @@ import ( "testing" "github.com/k8snetworkplumbingwg/sriov-cni/pkg/utils" - . "github.com/onsi/ginkgo" + . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" ) diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index 7194b837e..74d7179f8 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -1,11 +1,15 @@ package config import ( + "fmt" + "os" + "github.com/containernetworking/plugins/pkg/testutils" - "github.com/k8snetworkplumbingwg/sriov-cni/pkg/utils" - . "github.com/onsi/ginkgo" + . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" - "os" + + "github.com/k8snetworkplumbingwg/sriov-cni/pkg/types" + "github.com/k8snetworkplumbingwg/sriov-cni/pkg/utils" ) var _ = Describe("Config", func() { @@ -65,6 +69,57 @@ var _ = Describe("Config", func() { Expect(err).To(HaveOccurred()) }) + validVlanID := 100 + zeroVlanID := 0 + invalidVlanID := 5000 + validQoS := 1 + zeroQoS := 0 + invalidQoS := 10 + valid8021qProto := "802.1Q" + valid8021adProto := "802.1ad" + invalidProto := "802" + DescribeTable("Vlan ID, QoS and Proto", + func(vlanID *int, vlanQoS *int, vlanProto *string, failure bool) { + s := `{ + "name": "mynet", + "type": "sriov", + "deviceID": "0000:af:06.1", + "vf": 0` + if vlanID != nil { + s = fmt.Sprintf(`%s, + "vlan": %d`, s, *vlanID) + } + if vlanQoS != nil { + s = fmt.Sprintf(`%s, + "vlanQoS": %d`, s, *vlanQoS) + } + if vlanProto != nil { + s = fmt.Sprintf(`%s, + "vlanProto": "%s"`, s, *vlanProto) + } + s = fmt.Sprintf(`%s + }`, s) + conf := []byte(s) + _, err := LoadConf(conf) + if failure { + Expect(err).To(HaveOccurred()) + } else { + Expect(err).ToNot(HaveOccurred()) + } + }, + Entry("valid vlan ID", &validVlanID, nil, nil, false), + Entry("invalid vlan ID", &invalidVlanID, nil, nil, true), + Entry("vlan ID equal to zero and non-zero QoS set", &zeroVlanID, &validQoS, nil, true), + Entry("vlan ID equal to zero and 802.1ad Proto set", &zeroVlanID, nil, &valid8021adProto, true), + Entry("invalid QoS", &validVlanID, &invalidQoS, nil, true), + Entry("invalid Proto", &validVlanID, nil, &invalidProto, true), + Entry("valid 802.1q Proto", &validVlanID, nil, &valid8021qProto, false), + Entry("valid 802.1ad Proto", &validVlanID, nil, &valid8021adProto, false), + Entry("no vlan ID and non-zero QoS set", nil, &validQoS, nil, true), + Entry("no vlan ID and 802.1ad Proto set", nil, nil, &valid8021adProto, true), + Entry("default values for vlan, qos and proto", &zeroVlanID, &zeroQoS, &valid8021qProto, false), + ) + It("Assuming device is allocated", func() { conf := []byte(`{ "name": "mynet", @@ -118,4 +173,49 @@ var _ = Describe("Config", func() { Expect(err).To(HaveOccurred()) }) }) + Context("Checking GetMacAddressForResult function", func() { + It("Should return the mac address requested by the user", func() { + netconf := &types.NetConf{ + MAC: "MAC", + OrigVfState: types.VfState{ + EffectiveMAC: "EffectiveMAC", + AdminMAC: "AdminMAC", + }, + } + + Expect(GetMacAddressForResult(netconf)).To(Equal("MAC")) + }) + It("Should return the EffectiveMAC mac address if the user didn't request and the the driver is not DPDK", func() { + netconf := &types.NetConf{ + DPDKMode: false, + OrigVfState: types.VfState{ + EffectiveMAC: "EffectiveMAC", + AdminMAC: "AdminMAC", + }, + } + + Expect(GetMacAddressForResult(netconf)).To(Equal("EffectiveMAC")) + }) + It("Should return the AdminMAC mac address if the user didn't request and the the driver is DPDK", func() { + netconf := &types.NetConf{ + DPDKMode: true, + OrigVfState: types.VfState{ + EffectiveMAC: "EffectiveMAC", + AdminMAC: "AdminMAC", + }, + } + + Expect(GetMacAddressForResult(netconf)).To(Equal("AdminMAC")) + }) + It("Should return empty string if the user didn't request the the driver is DPDK and adminMac is 0", func() { + netconf := &types.NetConf{ + DPDKMode: true, + OrigVfState: types.VfState{ + AdminMAC: "00:00:00:00:00:00", + }, + } + + Expect(GetMacAddressForResult(netconf)).To(Equal("")) + }) + }) }) diff --git a/pkg/logging/logging.go b/pkg/logging/logging.go new file mode 100644 index 000000000..4d5e81fc4 --- /dev/null +++ b/pkg/logging/logging.go @@ -0,0 +1,93 @@ +// package logging is a small wrapper around github.com/k8snetworkplumbingwg/cni-log + +package logging + +import ( + cnilog "github.com/k8snetworkplumbingwg/cni-log" +) + +const ( + labelCNIName = "cniName" + labelContainerID = "containerID" + labelNetNS = "netns" + labelIFName = "ifname" + cniName = "sriov-cni" +) + +var ( + logLevelDefault = cnilog.InfoLevel + containerID = "" + netNS = "" + ifName = "" +) + +// Init initializes logging with the requested parameters in this order: log level, log file, container ID, +// network namespace and interface name. +func Init(logLevel, logFile, containerIdentification, networkNamespace, interfaceName string) { + setLogLevel(logLevel) + setLogFile(logFile) + containerID = containerIdentification + netNS = networkNamespace + ifName = interfaceName +} + +// setLogLevel sets the log level to either verbose, debug, info, warn, error or panic. If an invalid string is +// provided, it uses error. +func setLogLevel(l string) { + ll := cnilog.StringToLevel(l) + if ll == cnilog.InvalidLevel { + ll = logLevelDefault + } + cnilog.SetLogLevel(ll) +} + +// setLogFile sets the log file for logging. If the empty string is provided, it uses stderr. +func setLogFile(fileName string) { + if fileName == "" { + cnilog.SetLogStderr(true) + cnilog.SetLogFile("") + return + } + cnilog.SetLogFile(fileName) + cnilog.SetLogStderr(false) +} + +// Debug provides structured logging for log level >= debug. +func Debug(msg string, args ...interface{}) { + cnilog.DebugStructured(msg, prependArgs(args)...) +} + +// Info provides structured logging for log level >= info. +func Info(msg string, args ...interface{}) { + cnilog.InfoStructured(msg, prependArgs(args)...) +} + +// Warning provides structured logging for log level >= warning. +func Warning(msg string, args ...interface{}) { + cnilog.WarningStructured(msg, prependArgs(args)...) +} + +// Error provides structured logging for log level >= error. +func Error(msg string, args ...interface{}) { + _ = cnilog.ErrorStructured(msg, prependArgs(args)...) +} + +// Panic provides structured logging for log level >= panic. +func Panic(msg string, args ...interface{}) { + cnilog.PanicStructured(msg, prependArgs(args)...) +} + +// prependArgs prepends cniName, containerID, netNS and ifName to the args of every log message. +func prependArgs(args []interface{}) []interface{} { + if ifName != "" { + args = append([]interface{}{labelIFName, ifName}, args...) + } + if netNS != "" { + args = append([]interface{}{labelNetNS, netNS}, args...) + } + if containerID != "" { + args = append([]interface{}{labelContainerID, containerID}, args...) + } + args = append([]interface{}{labelCNIName, cniName}, args...) + return args +} diff --git a/pkg/logging/logging_suite_test.go b/pkg/logging/logging_suite_test.go new file mode 100644 index 000000000..6f85b94ef --- /dev/null +++ b/pkg/logging/logging_suite_test.go @@ -0,0 +1,19 @@ +package logging + +import ( + "testing" + + g "github.com/onsi/ginkgo/v2" + o "github.com/onsi/gomega" +) + +func TestConfig(t *testing.T) { + o.RegisterFailHandler(g.Fail) + g.RunSpecs(t, "Logging Suite") +} + +var _ = g.BeforeSuite(func() { +}) + +var _ = g.AfterSuite(func() { +}) diff --git a/pkg/logging/logging_test.go b/pkg/logging/logging_test.go new file mode 100644 index 000000000..495690c28 --- /dev/null +++ b/pkg/logging/logging_test.go @@ -0,0 +1,230 @@ +package logging + +import ( + "fmt" + "io" + "os" + + g "github.com/onsi/ginkgo/v2" + o "github.com/onsi/gomega" +) + +var _ = g.Describe("Logging", func() { + var origStderr *os.File + var stderrFile *os.File + + g.BeforeEach(func() { + var err error + stderrFile, err = os.CreateTemp("", "") + o.Expect(err).NotTo(o.HaveOccurred()) + origStderr = os.Stderr + os.Stderr = stderrFile + }) + + g.AfterEach(func() { + os.Stderr = origStderr + o.Expect(stderrFile.Close()).To(o.Succeed()) + o.Expect(os.RemoveAll(stderrFile.Name())).To(o.Succeed()) + }) + + g.Context("log argument prepender", func() { + g.When("none of netns, containerID, ifName are specified", func() { + g.BeforeEach(func() { + Init("", "", "", "", "") + }) + + g.It("should only prepend the cniName", func() { + Panic("test message", "a", "b") + _, _ = stderrFile.Seek(0, 0) + out, err := io.ReadAll(stderrFile) + o.Expect(err).NotTo(o.HaveOccurred()) + o.Expect(out).Should(o.ContainSubstring(fmt.Sprintf(`%s="%s"`, labelCNIName, cniName))) + o.Expect(out).ShouldNot(o.ContainSubstring(labelContainerID)) + o.Expect(out).ShouldNot(o.ContainSubstring(labelNetNS)) + o.Expect(out).ShouldNot(o.ContainSubstring(labelIFName)) + }) + }) + + g.When("netns, containerID and ifName are specified", func() { + const ( + testContainerID = "test-containerid" + testNetNS = "test-netns" + testIFName = "test-ifname" + ) + + g.BeforeEach(func() { + Init("", "", testContainerID, testNetNS, testIFName) + }) + + g.It("should log cniName, netns, containerID and ifName", func() { + Panic("test message", "a", "b") + _, _ = stderrFile.Seek(0, 0) + out, err := io.ReadAll(stderrFile) + o.Expect(err).NotTo(o.HaveOccurred()) + o.Expect(out).Should(o.ContainSubstring(fmt.Sprintf(`%s="%s"`, labelCNIName, cniName))) + o.Expect(out).Should(o.ContainSubstring(fmt.Sprintf(`%s="%s"`, labelContainerID, testContainerID))) + o.Expect(out).Should(o.ContainSubstring(fmt.Sprintf(`%s="%s"`, labelNetNS, testNetNS))) + o.Expect(out).Should(o.ContainSubstring(fmt.Sprintf(`%s="%s"`, labelIFName, testIFName))) + }) + }) + }) + + g.Context("log levels", func() { + g.When("the defaults are used", func() { + g.BeforeEach(func() { + Init("", "", "", "", "") + }) + + g.It("panic messages are logged to stderr", func() { + Panic("test message", "a", "b") + _, _ = stderrFile.Seek(0, 0) + out, err := io.ReadAll(stderrFile) + o.Expect(err).NotTo(o.HaveOccurred()) + o.Expect(out).Should(o.ContainSubstring("test message")) + }) + + g.It("info messages are logged to stderr and look as expected", func() { + Info("test message", "a", "b") + _, _ = stderrFile.Seek(0, 0) + out, err := io.ReadAll(stderrFile) + o.Expect(err).NotTo(o.HaveOccurred()) + o.Expect(out).Should(o.ContainSubstring(`msg="test message"`)) + o.Expect(out).Should(o.ContainSubstring(`a="b"`)) + o.Expect(out).Should(o.ContainSubstring(`level="info"`)) + }) + + g.It("debug messages are not logged to stderr", func() { + Debug("test message", "a", "b") + _, _ = stderrFile.Seek(0, 0) + out, err := io.ReadAll(stderrFile) + o.Expect(err).NotTo(o.HaveOccurred()) + o.Expect(out).ShouldNot(o.ContainSubstring("test message")) + }) + }) + + g.When("the log level is raised to warning", func() { + g.BeforeEach(func() { + Init("warning", "", "", "", "") + }) + + g.It("panic messages are logged to stderr", func() { + Panic("test message", "a", "b") + _, _ = stderrFile.Seek(0, 0) + out, err := io.ReadAll(stderrFile) + o.Expect(err).NotTo(o.HaveOccurred()) + o.Expect(out).Should(o.ContainSubstring("test message")) + }) + + g.It("error messages are logged to stderr", func() { + Error("test message", "a", "b") + _, _ = stderrFile.Seek(0, 0) + out, err := io.ReadAll(stderrFile) + o.Expect(err).NotTo(o.HaveOccurred()) + o.Expect(out).Should(o.ContainSubstring("test message")) + }) + + g.It("warning messages are logged to stderr", func() { + Warning("test message", "a", "b") + _, _ = stderrFile.Seek(0, 0) + out, err := io.ReadAll(stderrFile) + o.Expect(err).NotTo(o.HaveOccurred()) + o.Expect(out).Should(o.ContainSubstring("test message")) + }) + + g.It("info messages are not logged to stderr", func() { + Info("test message", "a", "b") + _, _ = stderrFile.Seek(0, 0) + out, err := io.ReadAll(stderrFile) + o.Expect(err).NotTo(o.HaveOccurred()) + o.Expect(out).ShouldNot(o.ContainSubstring("test message")) + }) + }) + + g.When("the log level is set to an invalid value", func() { + g.BeforeEach(func() { + Init("I'm invalid", "", "", "", "") + }) + + g.It("panic messages are logged to stderr", func() { + Panic("test message", "a", "b") + _, _ = stderrFile.Seek(0, 0) + out, err := io.ReadAll(stderrFile) + o.Expect(err).NotTo(o.HaveOccurred()) + o.Expect(out).Should(o.ContainSubstring("test message")) + }) + + g.It("info messages are logged to stderr", func() { + Info("test message", "a", "b") + _, _ = stderrFile.Seek(0, 0) + out, err := io.ReadAll(stderrFile) + o.Expect(err).NotTo(o.HaveOccurred()) + o.Expect(out).Should(o.ContainSubstring("test message")) + }) + + g.It("debug messages are not logged to stderr", func() { + Debug("test message", "a", "b") + _, _ = stderrFile.Seek(0, 0) + out, err := io.ReadAll(stderrFile) + o.Expect(err).NotTo(o.HaveOccurred()) + o.Expect(out).ShouldNot(o.ContainSubstring("test message")) + }) + }) + }) + + g.Context("log files", func() { + var logFile *os.File + + g.BeforeEach(func() { + var err error + logFile, err = os.CreateTemp("", "") + o.Expect(err).NotTo(o.HaveOccurred()) + }) + + g.AfterEach(func() { + o.Expect(logFile.Close()).To(o.Succeed()) + o.Expect(os.RemoveAll(logFile.Name())).To(o.Succeed()) + }) + + g.When("the log file is set", func() { + g.BeforeEach(func() { + Init("", logFile.Name(), "", "", "") + }) + + g.It("error messages are logged to log file but not to stderr", func() { + Error("test message", "a", "b") + _, _ = stderrFile.Seek(0, 0) + out, err := io.ReadAll(logFile) + o.Expect(err).NotTo(o.HaveOccurred()) + o.Expect(out).Should(o.ContainSubstring("test message")) + + _, _ = stderrFile.Seek(0, 0) + out, err = io.ReadAll(stderrFile) + o.Expect(err).NotTo(o.HaveOccurred()) + o.Expect(out).ShouldNot(o.ContainSubstring("test message")) + }) + }) + + g.When("the log file is set and then unset", func() { + g.BeforeEach(func() { + // TODO: This triggers a data race in github.com/k8snetworkplumbingwg/cni-log; fix the datarace in the + // logging component and then remove the skip. + g.Skip("https://github.com/k8snetworkplumbingwg/cni-log/issues/15") + Init("", logFile.Name(), "", "", "") + setLogFile("") + }) + + g.It("logs to stderr but not to file", func() { + Error("test message", "a", "b") + _, _ = stderrFile.Seek(0, 0) + out, err := io.ReadAll(logFile) + o.Expect(err).NotTo(o.HaveOccurred()) + o.Expect(out).ShouldNot(o.ContainSubstring("test message")) + + _, _ = stderrFile.Seek(0, 0) + out, err = io.ReadAll(stderrFile) + o.Expect(err).NotTo(o.HaveOccurred()) + o.Expect(out).Should(o.ContainSubstring("test message")) + }) + }) + }) +}) diff --git a/pkg/sriov/sriov.go b/pkg/sriov/sriov.go index deaad0b98..d4bbf5e08 100644 --- a/pkg/sriov/sriov.go +++ b/pkg/sriov/sriov.go @@ -2,11 +2,10 @@ package sriov import ( "fmt" - "net" - "time" "github.com/containernetworking/plugins/pkg/ns" + "github.com/k8snetworkplumbingwg/sriov-cni/pkg/logging" sriovtypes "github.com/k8snetworkplumbingwg/sriov-cni/pkg/types" "github.com/k8snetworkplumbingwg/sriov-cni/pkg/utils" "github.com/vishvananda/netlink" @@ -39,8 +38,8 @@ func (p *pciUtilsImpl) EnableArpAndNdiscNotify(ifName string) error { // Manager provides interface invoke sriov nic related operations type Manager interface { - SetupVF(conf *sriovtypes.NetConf, podifName string, cid string, netns ns.NetNS) (string, error) - ReleaseVF(conf *sriovtypes.NetConf, podifName string, cid string, netns ns.NetNS) error + SetupVF(conf *sriovtypes.NetConf, podifName string, netns ns.NetNS) error + ReleaseVF(conf *sriovtypes.NetConf, podifName string, netns ns.NetNS) error ResetVFConfig(conf *sriovtypes.NetConf) error ApplyVFConfig(conf *sriovtypes.NetConf) error FillOriginalVfInfo(conf *sriovtypes.NetConf) error @@ -60,128 +59,145 @@ func NewSriovManager() Manager { } // SetupVF sets up a VF in Pod netns -func (s *sriovManager) SetupVF(conf *sriovtypes.NetConf, podifName string, cid string, netns ns.NetNS) (string, error) { +func (s *sriovManager) SetupVF(conf *sriovtypes.NetConf, podifName string, netns ns.NetNS) error { linkName := conf.OrigVfState.HostIFName linkObj, err := s.nLink.LinkByName(linkName) if err != nil { - return "", fmt.Errorf("error getting VF netdevice with name %s", linkName) + return fmt.Errorf("error getting VF netdevice with name %s", linkName) } + // Save the original effective MAC address before overriding it + conf.OrigVfState.EffectiveMAC = linkObj.Attrs().HardwareAddr.String() + // tempName used as intermediary name to avoid name conflicts tempName := fmt.Sprintf("%s%d", "temp_", linkObj.Attrs().Index) // 1. Set link down + logging.Debug("1. Set link down", + "func", "SetupVF", + "linkObj", linkObj) if err := s.nLink.LinkSetDown(linkObj); err != nil { - return "", fmt.Errorf("failed to down vf device %q: %v", linkName, err) + return fmt.Errorf("failed to down vf device %q: %v", linkName, err) } // 2. Set temp name + logging.Debug("2. Set temp name", + "func", "SetupVF", + "linkObj", linkObj, + "tempName", tempName) if err := s.nLink.LinkSetName(linkObj, tempName); err != nil { - return "", fmt.Errorf("error setting temp IF name %s for %s", tempName, linkName) - } - - macAddress := linkObj.Attrs().HardwareAddr.String() - // 3. Set MAC address - if conf.MAC != "" { - hwaddr, err := net.ParseMAC(conf.MAC) - if err != nil { - return "", fmt.Errorf("failed to parse MAC address %s: %v", conf.MAC, err) - } - - // Save the original effective MAC address before overriding it - conf.OrigVfState.EffectiveMAC = linkObj.Attrs().HardwareAddr.String() - - /* Some NIC drivers (i.e. i40e/iavf) set VF MAC address asynchronously - via PF. This means that while the PF could already show the VF with - the desired MAC address, the netdev VF may still have the original - one. If in this window we issue a netdev VF MAC address set, the driver - will return an error and the pod will fail to create. - Other NICs (Mellanox) require explicit netdev VF MAC address so we - cannot skip this part. - Retry up to 5 times; wait 200 milliseconds between retries - */ - err = utils.Retry(5, 200*time.Millisecond, func() error { - return s.nLink.LinkSetHardwareAddr(linkObj, hwaddr) - }) - - if err != nil { - return "", fmt.Errorf("failed to set netlink MAC address to %s: %v", hwaddr, err) - } - macAddress = conf.MAC + return fmt.Errorf("error setting temp IF name %s for %s", tempName, linkName) } - // 4. Change netns + // 3. Change netns + logging.Debug("3. Change netns", + "func", "SetupVF", + "linkObj", linkObj, + "netns.Fd()", int(netns.Fd())) if err := s.nLink.LinkSetNsFd(linkObj, int(netns.Fd())); err != nil { - return "", fmt.Errorf("failed to move IF %s to netns: %q", tempName, err) + return fmt.Errorf("failed to move IF %s to netns: %q", tempName, err) } if err := netns.Do(func(_ ns.NetNS) error { - // 5. Set Pod IF name + // 4. Set Pod IF name + logging.Debug("4. Set Pod IF name", + "func", "SetupVF", + "linkObj", linkObj, + "podifName", podifName) if err := s.nLink.LinkSetName(linkObj, podifName); err != nil { return fmt.Errorf("error setting container interface name %s for %s", linkName, tempName) } - // 6. Enable IPv4 ARP notify and IPv6 Network Discovery notify + // 5. Enable IPv4 ARP notify and IPv6 Network Discovery notify // Error is ignored here because enabling this feature is only a performance enhancement. + logging.Debug("5. Enable IPv4 ARP notify and IPv6 Network Discovery notify", + "func", "SetupVF", + "podifName", podifName) _ = s.utils.EnableArpAndNdiscNotify(podifName) + // 6. Set MAC address + if conf.MAC != "" { + logging.Debug("6. Set MAC address", + "func", "SetupVF", + "s.nLink", s.nLink, + "podifName", podifName, + "conf.MAC", conf.MAC) + err = utils.SetVFEffectiveMAC(s.nLink, podifName, conf.MAC) + if err != nil { + return fmt.Errorf("failed to set netlink MAC address to %s: %v", conf.MAC, err) + } + } + // 7. Bring IF up in Pod netns + logging.Debug("7. Bring IF up in Pod netns", + "func", "SetupVF", + "linkObj", linkObj) if err := s.nLink.LinkSetUp(linkObj); err != nil { return fmt.Errorf("error bringing interface up in container ns: %q", err) } return nil }); err != nil { - return "", fmt.Errorf("error setting up interface in container namespace: %q", err) + return fmt.Errorf("error setting up interface in container namespace: %q", err) } - conf.ContIFNames = podifName - return macAddress, nil + return nil } // ReleaseVF reset a VF from Pod netns and return it to init netns -func (s *sriovManager) ReleaseVF(conf *sriovtypes.NetConf, podifName string, cid string, netns ns.NetNS) error { +func (s *sriovManager) ReleaseVF(conf *sriovtypes.NetConf, podifName string, netns ns.NetNS) error { initns, err := ns.GetCurrentNS() if err != nil { return fmt.Errorf("failed to get init netns: %v", err) } - if len(conf.ContIFNames) < 1 && len(conf.ContIFNames) != len(conf.OrigVfState.HostIFName) { - return fmt.Errorf("number of interface names mismatch ContIFNames: %d HostIFNames: %d", len(conf.ContIFNames), len(conf.OrigVfState.HostIFName)) - } - return netns.Do(func(_ ns.NetNS) error { // get VF device + logging.Debug("Get VF device", + "func", "ReleaseVF", + "podifName", podifName) linkObj, err := s.nLink.LinkByName(podifName) if err != nil { return fmt.Errorf("failed to get netlink device with name %s: %q", podifName, err) } // shutdown VF device + logging.Debug("Shutdown VF device", + "func", "ReleaseVF", + "linkObj", linkObj) if err = s.nLink.LinkSetDown(linkObj); err != nil { return fmt.Errorf("failed to set link %s down: %q", podifName, err) } // rename VF device + logging.Debug("Rename VF device", + "func", "ReleaseVF", + "linkObj", linkObj, + "conf.OrigVfState.HostIFName", conf.OrigVfState.HostIFName) err = s.nLink.LinkSetName(linkObj, conf.OrigVfState.HostIFName) if err != nil { return fmt.Errorf("failed to rename link %s to host name %s: %q", podifName, conf.OrigVfState.HostIFName, err) } - // reset effective MAC address if conf.MAC != "" { - hwaddr, err := net.ParseMAC(conf.OrigVfState.EffectiveMAC) + // reset effective MAC address + logging.Debug("Reset effective MAC address", + "func", "ReleaseVF", + "s.nLink", s.nLink, + "conf.OrigVfState.HostIFName", conf.OrigVfState.HostIFName, + "conf.OrigVfState.EffectiveMAC", conf.OrigVfState.EffectiveMAC) + err = utils.SetVFEffectiveMAC(s.nLink, conf.OrigVfState.HostIFName, conf.OrigVfState.EffectiveMAC) if err != nil { - return fmt.Errorf("failed to parse original effective MAC address %s: %v", conf.OrigVfState.EffectiveMAC, err) - } - - if err = s.nLink.LinkSetHardwareAddr(linkObj, hwaddr); err != nil { - return fmt.Errorf("failed to restore original effective netlink MAC address %s: %v", hwaddr, err) + return fmt.Errorf("failed to restore original effective netlink MAC address %s: %v", conf.OrigVfState.EffectiveMAC, err) } } // move VF device to init netns + logging.Debug("Move VF device to init netns", + "func", "ReleaseVF", + "linkObj", linkObj, + "initns.Fd()", int(initns.Fd())) if err = s.nLink.LinkSetNsFd(linkObj, int(initns.Fd())); err != nil { return fmt.Errorf("failed to move interface %s to init netns: %v", conf.OrigVfState.HostIFName, err) } @@ -207,32 +223,15 @@ func (s *sriovManager) ApplyVFConfig(conf *sriovtypes.NetConf) error { return fmt.Errorf("failed to lookup master %q: %v", conf.Master, err) } // 1. Set vlan - if conf.Vlan == nil { - vlan := new(int) - *vlan = 0 - conf.Vlan = vlan - } - // set vlan qos if present in the config - if conf.VlanQoS != nil { - if err = s.nLink.LinkSetVfVlanQos(pfLink, conf.VFID, *conf.Vlan, *conf.VlanQoS); err != nil { - return fmt.Errorf("failed to set vf %d vlan configuration: %v", conf.VFID, err) - } - } else { - // set vlan id field only - if err = s.nLink.LinkSetVfVlan(pfLink, conf.VFID, *conf.Vlan); err != nil { - return fmt.Errorf("failed to set vf %d vlan: %v", conf.VFID, err) - } + if err = s.nLink.LinkSetVfVlanQosProto(pfLink, conf.VFID, *conf.Vlan, *conf.VlanQoS, sriovtypes.VlanProtoInt[*conf.VlanProto]); err != nil { + return fmt.Errorf("failed to set vf %d vlan configuration - id %d, qos %d and proto %s: %v", conf.VFID, *conf.Vlan, *conf.VlanQoS, *conf.VlanProto, err) } // 2. Set mac address if conf.MAC != "" { - hwaddr, err := net.ParseMAC(conf.MAC) - if err != nil { - return fmt.Errorf("failed to parse MAC address %s: %v", conf.MAC, err) - } - - if err = s.nLink.LinkSetVfHardwareAddr(pfLink, conf.VFID, hwaddr); err != nil { - return fmt.Errorf("failed to set MAC address to %s: %v", hwaddr, err) + // when we restore the original hardware mac address we may get a device or resource busy. so we introduce retry + if err := utils.SetVFHardwareMAC(s.nLink, conf.Master, conf.VFID, conf.MAC); err != nil { + return fmt.Errorf("failed to set MAC address to %s: %v", conf.MAC, err) } } @@ -312,6 +311,7 @@ func (s *sriovManager) FillOriginalVfInfo(conf *sriovtypes.NetConf) error { return fmt.Errorf("failed to find vf %d", conf.VFID) } conf.OrigVfState.FillFromVfInfo(vfState) + return err } @@ -322,15 +322,13 @@ func (s *sriovManager) ResetVFConfig(conf *sriovtypes.NetConf) error { return fmt.Errorf("failed to lookup master %q: %v", conf.Master, err) } - // Restore VLAN - if conf.Vlan != nil { - if conf.VlanQoS != nil { - if err = s.nLink.LinkSetVfVlanQos(pfLink, conf.VFID, conf.OrigVfState.Vlan, conf.OrigVfState.VlanQoS); err != nil { - return fmt.Errorf("failed to restore vf %d vlan: %v", conf.VFID, err) - } - } else if err = s.nLink.LinkSetVfVlan(pfLink, conf.VFID, conf.OrigVfState.Vlan); err != nil { - return fmt.Errorf("failed to restore vf %d vlan: %v", conf.VFID, err) - } + // Set 802.1q as default in case cache config does not have a value for vlan proto. + if conf.OrigVfState.VlanProto == 0 { + conf.OrigVfState.VlanProto = sriovtypes.VlanProtoInt[sriovtypes.Proto8021q] + } + + if err = s.nLink.LinkSetVfVlanQosProto(pfLink, conf.VFID, conf.OrigVfState.Vlan, conf.OrigVfState.VlanQoS, conf.OrigVfState.VlanProto); err != nil { + return fmt.Errorf("failed to set vf %d vlan configuration - id %d, qos %d and proto %d: %v", conf.VFID, conf.OrigVfState.Vlan, conf.OrigVfState.VlanQoS, conf.OrigVfState.VlanProto, err) } // Restore spoofchk @@ -342,34 +340,16 @@ func (s *sriovManager) ResetVFConfig(conf *sriovtypes.NetConf) error { // Restore the original administrative MAC address if conf.MAC != "" { - hwaddr, err := net.ParseMAC(conf.OrigVfState.AdminMAC) - if err != nil { - return fmt.Errorf("failed to parse original administrative MAC address %s: %v", conf.OrigVfState.AdminMAC, err) - } - - /* Some NIC drivers (i.e. i40e/iavf) set VF MAC address asynchronously - via PF. This means that while the PF could already show the VF with - the desired MAC address, the netdev VF may still have the original - one. If in this window we issue a netdev VF MAC address set, the driver - will return an error and the pod will fail to create. - Other NICs (Mellanox) require explicit netdev VF MAC address so we - cannot skip this part. - Retry up to 5 times; wait 200 milliseconds between retries - */ - err = utils.Retry(5, 200*time.Millisecond, func() error { - return s.nLink.LinkSetVfHardwareAddr(pfLink, conf.VFID, hwaddr) - }) - if err != nil { - return fmt.Errorf("failed to restore original administrative MAC address %s: %v", hwaddr, err) + // when we restore the original hardware mac address we may get a device or resource busy. so we introduce retry + if err := utils.SetVFHardwareMAC(s.nLink, conf.Master, conf.VFID, conf.OrigVfState.AdminMAC); err != nil { + return fmt.Errorf("failed to restore original administrative MAC address %s: %v", conf.OrigVfState.AdminMAC, err) } } // Restore VF trust if conf.Trust != "" { - // TODO: netlink go implementation does not support getting VF trust, need to add support there first - // for now, just set VF trust to off if it was specified by the user in netconf - if err = s.nLink.LinkSetVfTrust(pfLink, conf.VFID, false); err != nil { - return fmt.Errorf("failed to disable trust for vf %d: %v", conf.VFID, err) + if err = s.nLink.LinkSetVfTrust(pfLink, conf.VFID, conf.OrigVfState.Trust); err != nil { + return fmt.Errorf("failed to set trust for vf %d: %v", conf.VFID, err) } } diff --git a/pkg/sriov/sriov_suite_test.go b/pkg/sriov/sriov_suite_test.go index 462bad8d7..4ee20377c 100644 --- a/pkg/sriov/sriov_suite_test.go +++ b/pkg/sriov/sriov_suite_test.go @@ -1,11 +1,12 @@ package sriov import ( - "github.com/k8snetworkplumbingwg/sriov-cni/pkg/utils" - . "github.com/onsi/ginkgo" + . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "testing" + + "github.com/k8snetworkplumbingwg/sriov-cni/pkg/utils" ) func TestConfig(t *testing.T) { diff --git a/pkg/sriov/sriov_test.go b/pkg/sriov/sriov_test.go index d8c90e24d..00f705f00 100644 --- a/pkg/sriov/sriov_test.go +++ b/pkg/sriov/sriov_test.go @@ -3,34 +3,19 @@ package sriov import ( "net" + "github.com/k8snetworkplumbingwg/sriov-cni/pkg/utils" + "github.com/containernetworking/plugins/pkg/ns" "github.com/containernetworking/plugins/pkg/testutils" "github.com/k8snetworkplumbingwg/sriov-cni/pkg/sriov/mocks" sriovtypes "github.com/k8snetworkplumbingwg/sriov-cni/pkg/types" mocks_utils "github.com/k8snetworkplumbingwg/sriov-cni/pkg/utils/mocks" - . "github.com/onsi/ginkgo" + . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "github.com/stretchr/testify/mock" "github.com/vishvananda/netlink" ) -// FakeLink is a dummy netlink struct used during testing -type FakeLink struct { - netlink.LinkAttrs -} - -// type FakeLink struct { -// linkAtrrs *netlink.LinkAttrs -// } - -func (l *FakeLink) Attrs() *netlink.LinkAttrs { - return &l.LinkAttrs -} - -func (l *FakeLink) Type() string { - return "FakeLink" -} - var _ = Describe("Sriov", func() { var ( t GinkgoTInterface @@ -42,18 +27,15 @@ var _ = Describe("Sriov", func() { Context("Checking SetupVF function", func() { var ( podifName string - contID string netconf *sriovtypes.NetConf ) BeforeEach(func() { podifName = "net1" - contID = "dummycid" netconf = &sriovtypes.NetConf{ - Master: "enp175s0f1", - DeviceID: "0000:af:06.0", - VFID: 0, - ContIFNames: "net1", + Master: "enp175s0f1", + DeviceID: "0000:af:06.0", + VFID: 0, OrigVfState: sriovtypes.VfState{ HostIFName: "enp175s6", }, @@ -76,7 +58,7 @@ var _ = Describe("Sriov", func() { Expect(err).NotTo(HaveOccurred()) - fakeLink := &FakeLink{netlink.LinkAttrs{ + fakeLink := &utils.FakeLink{LinkAttrs: netlink.LinkAttrs{ Index: 1000, Name: "dummylink", HardwareAddr: fakeMac, @@ -91,9 +73,9 @@ var _ = Describe("Sriov", func() { mocked.On("LinkSetVfVlanQos", mock.Anything, mock.AnythingOfType("int"), mock.AnythingOfType("int"), mock.AnythingOfType("int")).Return(nil) mockedPciUtils.On("EnableArpAndNdiscNotify", mock.AnythingOfType("string")).Return(nil) sm := sriovManager{nLink: mocked, utils: mockedPciUtils} - macAddr, err := sm.SetupVF(netconf, podifName, contID, targetNetNS) + err = sm.SetupVF(netconf, podifName, targetNetNS) Expect(err).NotTo(HaveOccurred()) - Expect(macAddr).To(Equal("6e:16:06:0e:b7:e9")) + Expect(netconf.OrigVfState.EffectiveMAC).To(Equal("6e:16:06:0e:b7:e9")) }) It("Setting VF's MAC address", func() { var targetNetNS ns.NetNS @@ -113,23 +95,29 @@ var _ = Describe("Sriov", func() { expMac, err := net.ParseMAC(netconf.MAC) Expect(err).NotTo(HaveOccurred()) - fakeLink := &FakeLink{netlink.LinkAttrs{ + fakeLink := &utils.FakeLink{LinkAttrs: netlink.LinkAttrs{ Index: 1000, Name: "dummylink", HardwareAddr: fakeMac, }} - mocked.On("LinkByName", mock.AnythingOfType("string")).Return(fakeLink, nil) + net1Link := &utils.FakeLink{LinkAttrs: netlink.LinkAttrs{ + Index: 1000, + Name: "net1", + HardwareAddr: expMac, + }} + + mocked.On("LinkByName", "enp175s6").Return(fakeLink, nil) + mocked.On("LinkByName", "net1").Return(net1Link, nil) mocked.On("LinkSetDown", fakeLink).Return(nil) mocked.On("LinkSetName", fakeLink, mock.Anything).Return(nil) - mocked.On("LinkSetHardwareAddr", fakeLink, expMac).Return(nil) + mocked.On("LinkSetHardwareAddr", net1Link, expMac).Return(nil) mocked.On("LinkSetNsFd", fakeLink, mock.AnythingOfType("int")).Return(nil) mocked.On("LinkSetUp", fakeLink).Return(nil) mockedPciUtils.On("EnableArpAndNdiscNotify", mock.AnythingOfType("string")).Return(nil) sm := sriovManager{nLink: mocked, utils: mockedPciUtils} - macAddr, err := sm.SetupVF(netconf, podifName, contID, targetNetNS) + err = sm.SetupVF(netconf, podifName, targetNetNS) Expect(err).NotTo(HaveOccurred()) - Expect(macAddr).To(Equal(netconf.MAC)) mocked.AssertExpectations(t) }) }) @@ -137,20 +125,18 @@ var _ = Describe("Sriov", func() { Context("Checking ReleaseVF function", func() { var ( podifName string - contID string netconf *sriovtypes.NetConf ) BeforeEach(func() { podifName = "net1" - contID = "dummycid" netconf = &sriovtypes.NetConf{ - Master: "enp175s0f1", - DeviceID: "0000:af:06.0", - VFID: 0, - ContIFNames: "net1", + Master: "enp175s0f1", + DeviceID: "0000:af:06.0", + VFID: 0, OrigVfState: sriovtypes.VfState{ - HostIFName: "enp175s6", + HostIFName: "enp175s6", + EffectiveMAC: "6e:16:06:0e:b7:e9", }, } }) @@ -164,14 +150,17 @@ var _ = Describe("Sriov", func() { }() Expect(err).NotTo(HaveOccurred()) mocked := &mocks_utils.NetlinkManager{} - fakeLink := &FakeLink{netlink.LinkAttrs{Index: 1000, Name: "dummylink"}} + fakeMac, err := net.ParseMAC("6e:16:06:0e:b7:e9") + Expect(err).NotTo(HaveOccurred()) + + fakeLink := &utils.FakeLink{LinkAttrs: netlink.LinkAttrs{Index: 1000, Name: "dummylink", HardwareAddr: fakeMac}} - mocked.On("LinkByName", netconf.ContIFNames).Return(fakeLink, nil) + mocked.On("LinkByName", podifName).Return(fakeLink, nil) mocked.On("LinkSetDown", fakeLink).Return(nil) mocked.On("LinkSetName", fakeLink, netconf.OrigVfState.HostIFName).Return(nil) mocked.On("LinkSetNsFd", fakeLink, mock.AnythingOfType("int")).Return(nil) sm := sriovManager{nLink: mocked} - err = sm.ReleaseVF(netconf, podifName, contID, targetNetNS) + err = sm.ReleaseVF(netconf, podifName, targetNetNS) Expect(err).NotTo(HaveOccurred()) mocked.AssertExpectations(t) }) @@ -179,26 +168,22 @@ var _ = Describe("Sriov", func() { Context("Checking ReleaseVF function - restore config", func() { var ( podifName string - contID string netconf *sriovtypes.NetConf ) BeforeEach(func() { podifName = "net1" - contID = "dummycid" netconf = &sriovtypes.NetConf{ - Master: "enp175s0f1", - DeviceID: "0000:af:06.0", - VFID: 0, - MAC: "aa:f3:8d:65:1b:d4", - ContIFNames: "net1", + Master: "enp175s0f1", + DeviceID: "0000:af:06.0", + VFID: 0, OrigVfState: sriovtypes.VfState{ HostIFName: "enp175s6", EffectiveMAC: "c6:c8:7f:1f:21:90", }, } }) - It("Restores Effective MAC address when provided in netconf", func() { + It("Should not restores Effective MAC address when it is not provided in netconf", func() { var targetNetNS ns.NetNS targetNetNS, err := testutils.NewNS() defer func() { @@ -207,18 +192,48 @@ var _ = Describe("Sriov", func() { } }() Expect(err).NotTo(HaveOccurred()) + fakeLink := &utils.FakeLink{LinkAttrs: netlink.LinkAttrs{Index: 1000, Name: "dummylink"}} mocked := &mocks_utils.NetlinkManager{} - fakeLink := &FakeLink{netlink.LinkAttrs{Index: 1000, Name: "dummylink"}} - mocked.On("LinkByName", netconf.ContIFNames).Return(fakeLink, nil) + mocked.On("LinkByName", podifName).Return(fakeLink, nil) mocked.On("LinkSetDown", fakeLink).Return(nil) mocked.On("LinkSetName", fakeLink, netconf.OrigVfState.HostIFName).Return(nil) mocked.On("LinkSetNsFd", fakeLink, mock.AnythingOfType("int")).Return(nil) - origEffMac, err := net.ParseMAC(netconf.OrigVfState.EffectiveMAC) + sm := sriovManager{nLink: mocked} + err = sm.ReleaseVF(netconf, podifName, targetNetNS) + Expect(err).NotTo(HaveOccurred()) + mocked.AssertExpectations(t) + }) + + It("Restores Effective MAC address when provided in netconf", func() { + netconf.MAC = "aa:f3:8d:65:1b:d4" + var targetNetNS ns.NetNS + targetNetNS, err := testutils.NewNS() + defer func() { + if targetNetNS != nil { + targetNetNS.Close() + } + }() Expect(err).NotTo(HaveOccurred()) - mocked.On("LinkSetHardwareAddr", fakeLink, origEffMac).Return(nil) + fakeLink := &utils.FakeLink{LinkAttrs: netlink.LinkAttrs{Index: 1000, Name: "dummylink"}} + mocked := &mocks_utils.NetlinkManager{} + + fakeMac, err := net.ParseMAC("c6:c8:7f:1f:21:90") + Expect(err).NotTo(HaveOccurred()) + tempLink := &utils.FakeLink{LinkAttrs: netlink.LinkAttrs{ + Index: 1000, + Name: "enp175s6", + HardwareAddr: fakeMac, + }} + + mocked.On("LinkByName", podifName).Return(fakeLink, nil) + mocked.On("LinkByName", netconf.OrigVfState.HostIFName).Return(tempLink, nil) + mocked.On("LinkSetDown", fakeLink).Return(nil) + mocked.On("LinkSetHardwareAddr", tempLink, fakeMac).Return(nil) + mocked.On("LinkSetName", fakeLink, netconf.OrigVfState.HostIFName).Return(nil) + mocked.On("LinkSetNsFd", fakeLink, mock.AnythingOfType("int")).Return(nil) sm := sriovManager{nLink: mocked} - err = sm.ReleaseVF(netconf, podifName, contID, targetNetNS) + err = sm.ReleaseVF(netconf, podifName, targetNetNS) Expect(err).NotTo(HaveOccurred()) mocked.AssertExpectations(t) }) @@ -230,10 +245,9 @@ var _ = Describe("Sriov", func() { BeforeEach(func() { netconf = &sriovtypes.NetConf{ - Master: "enp175s0f1", - DeviceID: "0000:af:06.0", - VFID: 0, - ContIFNames: "net1", + Master: "enp175s0f1", + DeviceID: "0000:af:06.0", + VFID: 0, OrigVfState: sriovtypes.VfState{ HostIFName: "enp175s6", }, @@ -245,7 +259,7 @@ var _ = Describe("Sriov", func() { fakeMac, err := net.ParseMAC("6e:16:06:0e:b7:e9") Expect(err).NotTo(HaveOccurred()) - fakeLink := &FakeLink{netlink.LinkAttrs{ + fakeLink := &utils.FakeLink{LinkAttrs: netlink.LinkAttrs{ Index: 1000, Name: "dummylink", HardwareAddr: fakeMac, @@ -270,10 +284,9 @@ var _ = Describe("Sriov", func() { BeforeEach(func() { netconf = &sriovtypes.NetConf{ - Master: "enp175s0f1", - DeviceID: "0000:af:06.0", - VFID: 0, - ContIFNames: "net1", + Master: "enp175s0f1", + DeviceID: "0000:af:06.0", + VFID: 0, OrigVfState: sriovtypes.VfState{ HostIFName: "enp175s6", }, @@ -281,9 +294,10 @@ var _ = Describe("Sriov", func() { }) It("Does not change VF config if it wasnt requested to be changed in netconf", func() { mocked := &mocks_utils.NetlinkManager{} - fakeLink := &FakeLink{netlink.LinkAttrs{Index: 1000, Name: "dummylink"}} + fakeLink := &utils.FakeLink{LinkAttrs: netlink.LinkAttrs{Index: 1000, Name: "dummylink"}} mocked.On("LinkByName", netconf.Master).Return(fakeLink, nil) + mocked.On("LinkSetVfVlanQosProto", fakeLink, netconf.VFID, netconf.OrigVfState.Vlan, netconf.OrigVfState.VlanQoS, sriovtypes.VlanProtoInt[sriovtypes.Proto8021q]).Return(nil) sm := sriovManager{nLink: mocked} err := sm.ResetVFConfig(netconf) Expect(err).NotTo(HaveOccurred()) @@ -302,18 +316,17 @@ var _ = Describe("Sriov", func() { minTxRate := 1000 netconf = &sriovtypes.NetConf{ - Master: "enp175s0f1", - DeviceID: "0000:af:06.0", - VFID: 3, - ContIFNames: "net1", - MAC: "d2:fc:22:a7:0d:e8", - Vlan: &vlan, - VlanQoS: &vlanQos, - SpoofChk: "on", - MaxTxRate: &maxTxRate, - MinTxRate: &minTxRate, - Trust: "on", - LinkState: "enable", + Master: "enp175s0f1", + DeviceID: "0000:af:06.0", + VFID: 0, + MAC: "d2:fc:22:a7:0d:e8", + Vlan: &vlan, + VlanQoS: &vlanQos, + SpoofChk: "on", + MaxTxRate: &maxTxRate, + MinTxRate: &minTxRate, + Trust: "on", + LinkState: "enable", OrigVfState: sriovtypes.VfState{ HostIFName: "enp175s6", SpoofChk: false, @@ -328,14 +341,16 @@ var _ = Describe("Sriov", func() { } }) It("Restores original VF configurations", func() { + origMac, err := net.ParseMAC(netconf.OrigVfState.AdminMAC) + Expect(err).NotTo(HaveOccurred()) mocked := &mocks_utils.NetlinkManager{} - fakeLink := &FakeLink{netlink.LinkAttrs{Index: 1000, Name: "dummylink"}} + fakeLink := &utils.FakeLink{LinkAttrs: netlink.LinkAttrs{Index: 1000, Name: "dummylink", Vfs: []netlink.VfInfo{ + {Mac: origMac}, + }}} mocked.On("LinkByName", netconf.Master).Return(fakeLink, nil) - mocked.On("LinkSetVfVlanQos", fakeLink, netconf.VFID, netconf.OrigVfState.Vlan, netconf.OrigVfState.VlanQoS).Return(nil) + mocked.On("LinkSetVfVlanQosProto", fakeLink, netconf.VFID, netconf.OrigVfState.Vlan, netconf.OrigVfState.VlanQoS, sriovtypes.VlanProtoInt[sriovtypes.Proto8021q]).Return(nil) mocked.On("LinkSetVfSpoofchk", fakeLink, netconf.VFID, netconf.OrigVfState.SpoofChk).Return(nil) - origMac, err := net.ParseMAC(netconf.OrigVfState.AdminMAC) - Expect(err).NotTo(HaveOccurred()) mocked.On("LinkSetVfHardwareAddr", fakeLink, netconf.VFID, origMac).Return(nil) mocked.On("LinkSetVfTrust", fakeLink, netconf.VFID, false).Return(nil) mocked.On("LinkSetVfRate", fakeLink, netconf.VFID, netconf.OrigVfState.MinTxRate, netconf.OrigVfState.MaxTxRate).Return(nil) diff --git a/pkg/types/types.go b/pkg/types/types.go index 96c4580a7..c8724f714 100644 --- a/pkg/types/types.go +++ b/pkg/types/types.go @@ -5,14 +5,23 @@ import ( "github.com/vishvananda/netlink" ) +const ( + Proto8021q = "802.1q" + Proto8021ad = "802.1ad" +) + +var VlanProtoInt = map[string]int{Proto8021q: 33024, Proto8021ad: 34984} + // VfState represents the state of the VF type VfState struct { HostIFName string SpoofChk bool + Trust bool AdminMAC string EffectiveMAC string Vlan int VlanQoS int + VlanProto int MinTxRate int MaxTxRate int LinkState uint32 @@ -26,21 +35,23 @@ func (vs *VfState) FillFromVfInfo(info *netlink.VfInfo) { vs.MinTxRate = int(info.MinTxRate) vs.Vlan = info.Vlan vs.VlanQoS = info.Qos + vs.VlanProto = info.VlanProto vs.SpoofChk = info.Spoofchk + vs.Trust = info.Trust != 0 } // NetConf extends types.NetConf for sriov-cni type NetConf struct { types.NetConf OrigVfState VfState // Stores the original VF state as it was prior to any operations done during cmdAdd flow - DPDKMode bool + DPDKMode bool `json:"-"` Master string MAC string - Vlan *int `json:"vlan"` - VlanQoS *int `json:"vlanQoS"` - DeviceID string `json:"deviceID"` // PCI address of a VF in valid sysfs format + Vlan *int `json:"vlan"` + VlanQoS *int `json:"vlanQoS"` + VlanProto *string `json:"vlanProto"` // 802.1ad|802.1q + DeviceID string `json:"deviceID"` // PCI address of a VF in valid sysfs format VFID int - ContIFNames string // VF names after in the container; used during deletion MinTxRate *int `json:"min_tx_rate"` // Mbps, 0 = disable rate limiting MaxTxRate *int `json:"max_tx_rate"` // Mbps, 0 = disable rate limiting SpoofChk string `json:"spoofchk,omitempty"` // on|off @@ -49,4 +60,6 @@ type NetConf struct { RuntimeConfig struct { Mac string `json:"mac,omitempty"` } `json:"runtimeConfig,omitempty"` + LogLevel string `json:"logLevel,omitempty"` + LogFile string `json:"logFile,omitempty"` } diff --git a/pkg/utils/mocks/netlink_manager_mock.go b/pkg/utils/mocks/netlink_manager_mock.go index 891a4e9d8..0e1fc4395 100644 --- a/pkg/utils/mocks/netlink_manager_mock.go +++ b/pkg/utils/mocks/netlink_manager_mock.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.14.0. DO NOT EDIT. +// Code generated by mockery v2.20.0. DO NOT EDIT. package mocks @@ -20,6 +20,10 @@ func (_m *NetlinkManager) LinkByName(_a0 string) (netlink.Link, error) { ret := _m.Called(_a0) var r0 netlink.Link + var r1 error + if rf, ok := ret.Get(0).(func(string) (netlink.Link, error)); ok { + return rf(_a0) + } if rf, ok := ret.Get(0).(func(string) netlink.Link); ok { r0 = rf(_a0) } else { @@ -28,7 +32,6 @@ func (_m *NetlinkManager) LinkByName(_a0 string) (netlink.Link, error) { } } - var r1 error if rf, ok := ret.Get(1).(func(string) error); ok { r1 = rf(_a0) } else { @@ -178,27 +181,13 @@ func (_m *NetlinkManager) LinkSetVfTrust(_a0 netlink.Link, _a1 int, _a2 bool) er return r0 } -// LinkSetVfVlan provides a mock function with given fields: _a0, _a1, _a2 -func (_m *NetlinkManager) LinkSetVfVlan(_a0 netlink.Link, _a1 int, _a2 int) error { - ret := _m.Called(_a0, _a1, _a2) +// LinkSetVfVlanQosProto provides a mock function with given fields: _a0, _a1, _a2, _a3, _a4 +func (_m *NetlinkManager) LinkSetVfVlanQosProto(_a0 netlink.Link, _a1 int, _a2 int, _a3 int, _a4 int) error { + ret := _m.Called(_a0, _a1, _a2, _a3, _a4) var r0 error - if rf, ok := ret.Get(0).(func(netlink.Link, int, int) error); ok { - r0 = rf(_a0, _a1, _a2) - } else { - r0 = ret.Error(0) - } - - return r0 -} - -// LinkSetVfVlanQos provides a mock function with given fields: _a0, _a1, _a2, _a3 -func (_m *NetlinkManager) LinkSetVfVlanQos(_a0 netlink.Link, _a1 int, _a2 int, _a3 int) error { - ret := _m.Called(_a0, _a1, _a2, _a3) - - var r0 error - if rf, ok := ret.Get(0).(func(netlink.Link, int, int, int) error); ok { - r0 = rf(_a0, _a1, _a2, _a3) + if rf, ok := ret.Get(0).(func(netlink.Link, int, int, int, int) error); ok { + r0 = rf(_a0, _a1, _a2, _a3, _a4) } else { r0 = ret.Error(0) } diff --git a/pkg/utils/netlink_manager.go b/pkg/utils/netlink_manager.go index cf681eee0..2eeb4c507 100644 --- a/pkg/utils/netlink_manager.go +++ b/pkg/utils/netlink_manager.go @@ -11,8 +11,7 @@ import ( // NetlinkManager is an interface to mock nelink library type NetlinkManager interface { LinkByName(string) (netlink.Link, error) - LinkSetVfVlan(netlink.Link, int, int) error - LinkSetVfVlanQos(netlink.Link, int, int, int) error + LinkSetVfVlanQosProto(netlink.Link, int, int, int, int) error LinkSetVfHardwareAddr(netlink.Link, int, net.HardwareAddr) error LinkSetHardwareAddr(netlink.Link, net.HardwareAddr) error LinkSetUp(netlink.Link) error @@ -35,14 +34,9 @@ func (n *MyNetlink) LinkByName(name string) (netlink.Link, error) { return netlink.LinkByName(name) } -// LinkSetVfVlan using NetlinkManager -func (n *MyNetlink) LinkSetVfVlan(link netlink.Link, vf, vlan int) error { - return netlink.LinkSetVfVlan(link, vf, vlan) -} - -// LinkSetVfVlanQos sets VLAN ID and QoS field for given VF using NetlinkManager -func (n *MyNetlink) LinkSetVfVlanQos(link netlink.Link, vf, vlan, qos int) error { - return netlink.LinkSetVfVlanQos(link, vf, vlan, qos) +// LinkSetVfVlanQosProto sets VLAN ID, QoS and Proto field for given VF using NetlinkManager +func (n *MyNetlink) LinkSetVfVlanQosProto(link netlink.Link, vf, vlan, qos, proto int) error { + return netlink.LinkSetVfVlanQosProto(link, vf, vlan, qos, proto) } // LinkSetVfHardwareAddr using NetlinkManager diff --git a/pkg/utils/pci_allocator.go b/pkg/utils/pci_allocator.go index 1dd7201ce..3bf6ff054 100644 --- a/pkg/utils/pci_allocator.go +++ b/pkg/utils/pci_allocator.go @@ -2,9 +2,11 @@ package utils import ( "fmt" - "github.com/containernetworking/plugins/pkg/ns" "os" "path/filepath" + + "github.com/containernetworking/plugins/pkg/ns" + "github.com/k8snetworkplumbingwg/sriov-cni/pkg/logging" ) type PCIAllocation interface { @@ -73,6 +75,9 @@ func (p *PCIAllocator) IsAllocated(pciAddress string) (bool, error) { // we release the PCI address networkNamespace, err := ns.GetNS(string(dat)) if err != nil { + logging.Debug("Mark the PCI address as released", + "func", "IsAllocated", + "pciAddress", pciAddress) err = p.DeleteAllocatedPCI(pciAddress) if err != nil { return false, fmt.Errorf("error deleting the pci allocation for vf pci address %s: %v", pciAddress, err) diff --git a/pkg/utils/pci_allocator_test.go b/pkg/utils/pci_allocator_test.go index 6437498f0..4160aa308 100644 --- a/pkg/utils/pci_allocator_test.go +++ b/pkg/utils/pci_allocator_test.go @@ -1,7 +1,7 @@ package utils import ( - . "github.com/onsi/ginkgo" + . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "github.com/containernetworking/plugins/pkg/ns" diff --git a/pkg/utils/testing.go b/pkg/utils/testing.go index eeb149b7e..7db0c19de 100644 --- a/pkg/utils/testing.go +++ b/pkg/utils/testing.go @@ -10,6 +10,8 @@ import ( "os" "path/filepath" "syscall" + + "github.com/vishvananda/netlink" ) type tmpSysFs struct { @@ -116,10 +118,8 @@ func createSymlinks(link, target string) error { if err := os.MkdirAll(target, 0755); err != nil { return err } - if err := os.Symlink(target, link); err != nil { - return err - } - return nil + + return os.Symlink(target, link) } // RemoveTmpSysFs removes mocked sysfs @@ -134,8 +134,23 @@ func RemoveTmpSysFs() error { if err = ts.originalRoot.Close(); err != nil { return err } - if err = os.RemoveAll(ts.dirRoot); err != nil { - return err - } - return nil + + return os.RemoveAll(ts.dirRoot) +} + +// FakeLink is a dummy netlink struct used during testing +type FakeLink struct { + netlink.LinkAttrs +} + +// type FakeLink struct { +// linkAtrrs *netlink.LinkAttrs +// } + +func (l *FakeLink) Attrs() *netlink.LinkAttrs { + return &l.LinkAttrs +} + +func (l *FakeLink) Type() string { + return "FakeLink" } diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go index 7f4b3c16a..81bcf70f3 100644 --- a/pkg/utils/utils.go +++ b/pkg/utils/utils.go @@ -170,8 +170,8 @@ func GetSharedPF(ifName string) (string, error) { return pfName, fmt.Errorf("Shared PF not found") } -// GetVFLinkNames returns VF's network interface name given it's PCI addr -func GetVFLinkNames(pciAddr string) (string, error) { +// GetVFLinkName returns VF's network interface name given it's PCI addr +func GetVFLinkName(pciAddr string) (string, error) { var names []string vfDir := filepath.Join(SysBusPci, pciAddr, "net") if _, err := os.Lstat(vfDir); err != nil { @@ -248,11 +248,7 @@ func SaveNetConf(cid, dataDir, podIfName string, conf interface{}) error { cRef := strings.Join(s, "-") // save the rendered netconf for cmdDel - if err = saveScratchNetConf(cRef, dataDir, netConfBytes); err != nil { - return err - } - - return nil + return saveScratchNetConf(cRef, dataDir, netConfBytes) } func saveScratchNetConf(containerID, dataDir string, netconf []byte) error { @@ -288,6 +284,83 @@ func CleanCachedNetConf(cRefPath string) error { return nil } +// SetVFEffectiveMAC will try to set the mac address on a specific VF interface +// +// the function will also validate that the mac address was configured as expect +// it will return an error if it didn't manage to configure the vf mac address +// or the mac is not equal to the expect one +// retries 20 times and wait 100 milliseconds +// +// Some NIC drivers (i.e. i40e/iavf) set VF MAC address asynchronously +// via PF. This means that while the PF could already show the VF with +// the desired MAC address, the netdev VF may still have the original +// one. If in this window we issue a netdev VF MAC address set, the driver +// will return an error and the pod will fail to create. +// Other NICs (Mellanox) require explicit netdev VF MAC address so we +// cannot skip this part. +// Retry up to 5 times; wait 200 milliseconds between retries +func SetVFEffectiveMAC(netLinkManager NetlinkManager, netDeviceName string, macAddress string) error { + hwaddr, err := net.ParseMAC(macAddress) + if err != nil { + return fmt.Errorf("failed to parse MAC address %s: %v", macAddress, err) + } + + orgLinkObj, err := netLinkManager.LinkByName(netDeviceName) + if err != nil { + return err + } + + return Retry(20, 100*time.Millisecond, func() error { + if err := netLinkManager.LinkSetHardwareAddr(orgLinkObj, hwaddr); err != nil { + return err + } + + linkObj, err := netLinkManager.LinkByName(netDeviceName) + if err != nil { + return fmt.Errorf("failed to get netlink device with name %s: %q", orgLinkObj.Attrs().Name, err) + } + if linkObj.Attrs().HardwareAddr.String() != macAddress { + return fmt.Errorf("effective mac address is different from requested one") + } + + return nil + }) +} + +// SetVFHardwareMAC will try to set the hardware mac address on a specific VF ID under a requested PF + +// the function will also validate that the mac address was configured as expect +// it will return an error if it didn't manage to configure the vf mac address +// or the mac is not equal to the expect one +// retries 20 times and wait 100 milliseconds +func SetVFHardwareMAC(netLinkManager NetlinkManager, pfDevice string, vfID int, macAddress string) error { + hwaddr, err := net.ParseMAC(macAddress) + if err != nil { + return fmt.Errorf("failed to parse MAC address %s: %v", macAddress, err) + } + + orgLinkObj, err := netLinkManager.LinkByName(pfDevice) + if err != nil { + return err + } + + return Retry(20, 100*time.Millisecond, func() error { + if err := netLinkManager.LinkSetVfHardwareAddr(orgLinkObj, vfID, hwaddr); err != nil { + return err + } + + linkObj, err := netLinkManager.LinkByName(pfDevice) + if err != nil { + return fmt.Errorf("failed to get netlink device with name %s: %q", orgLinkObj.Attrs().Name, err) + } + if linkObj.Attrs().Vfs[vfID].Mac.String() != macAddress { + return fmt.Errorf("hardware mac address is different from requested one") + } + + return nil + }) +} + // IsValidMACAddress checks if net.HardwareAddr is a valid MAC address. func IsValidMACAddress(addr net.HardwareAddr) bool { invalidMACAddresses := [][]byte{ diff --git a/pkg/utils/utils_suite_test.go b/pkg/utils/utils_suite_test.go index 82389173f..66433869c 100644 --- a/pkg/utils/utils_suite_test.go +++ b/pkg/utils/utils_suite_test.go @@ -3,7 +3,7 @@ package utils import ( "testing" - . "github.com/onsi/ginkgo" + . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" ) diff --git a/pkg/utils/utils_test.go b/pkg/utils/utils_test.go index 0bb159030..273f02e91 100644 --- a/pkg/utils/utils_test.go +++ b/pkg/utils/utils_test.go @@ -2,10 +2,15 @@ package utils import ( "errors" + "net" "time" - . "github.com/onsi/ginkgo" + . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + + "github.com/vishvananda/netlink" + + mocks_utils "github.com/k8snetworkplumbingwg/sriov-cni/pkg/utils/mocks" ) var _ = Describe("Utils", func() { @@ -70,7 +75,7 @@ var _ = Describe("Utils", func() { // Expect(err).To(HaveOccurred(), "Looking for shared PF for not supported NIC should return an error") // }) }) - Context("Checking GetVFLinkNames function", func() { + Context("Checking GetVFLinkName function", func() { It("Assuming existing vf", func() { result, err := GetVFLinkNamesFromVFID("enp175s0f1", 0) Expect(result).To(ContainElement("enp175s6"), "Existing PF should have at least one VF") @@ -81,7 +86,7 @@ var _ = Describe("Utils", func() { Expect(err).To(HaveOccurred(), "Not existing VF should return an error") }) }) - Context("Checking Retry functon", func() { + Context("Checking Retry function", func() { It("Assuming calling function fails", func() { err := Retry(5, 10*time.Millisecond, func() error { return errors.New("") }) Expect(err).To((HaveOccurred()), "Retry should return an error") @@ -91,4 +96,92 @@ var _ = Describe("Utils", func() { Expect(err).NotTo((HaveOccurred()), "Retry should not return an error") }) }) + Context("Checking SetVFEffectiveMAC function", func() { + It("assuming calling function fails", func() { + mocked := &mocks_utils.NetlinkManager{} + fakeMac, err := net.ParseMAC("6e:16:06:0e:b7:e9") + Expect(err).ToNot(HaveOccurred()) + fakeNewMac, err := net.ParseMAC("60:00:00:00:00:01") + Expect(err).ToNot(HaveOccurred()) + + fakeLink := &FakeLink{netlink.LinkAttrs{ + Index: 1000, + Name: "enp175s0f1", + HardwareAddr: fakeMac, + }} + + mocked.On("LinkByName", "enp175s0f1").Return(fakeLink, nil) + mocked.On("LinkSetHardwareAddr", fakeLink, fakeNewMac).Return(nil) + + err = SetVFEffectiveMAC(mocked, "enp175s0f1", "60:00:00:00:00:01") + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("effective mac address is different from requested one")) + }) + + It("assuming calling function does not fails", func() { + mocked := &mocks_utils.NetlinkManager{} + fakeMac, err := net.ParseMAC("60:00:00:00:00:01") + Expect(err).ToNot(HaveOccurred()) + fakeNewMac, err := net.ParseMAC("60:00:00:00:00:01") + Expect(err).ToNot(HaveOccurred()) + + fakeLink := &FakeLink{netlink.LinkAttrs{ + Index: 1000, + Name: "enp175s0f1", + HardwareAddr: fakeMac, + }} + + mocked.On("LinkByName", "enp175s0f1").Return(fakeLink, nil) + mocked.On("LinkSetHardwareAddr", fakeLink, fakeNewMac).Return(nil) + + err = SetVFEffectiveMAC(mocked, "enp175s0f1", "60:00:00:00:00:01") + Expect(err).ToNot(HaveOccurred()) + }) + }) + Context("Checking SetVFHardwareMAC function", func() { + It("assuming calling function fails", func() { + mocked := &mocks_utils.NetlinkManager{} + fakeMac, err := net.ParseMAC("6e:16:06:0e:b7:e9") + Expect(err).ToNot(HaveOccurred()) + fakeNewMac, err := net.ParseMAC("60:00:00:00:00:01") + Expect(err).ToNot(HaveOccurred()) + + fakeLink := &FakeLink{netlink.LinkAttrs{ + Index: 1000, + Name: "enp175s0f1", + Vfs: []netlink.VfInfo{ + {Mac: fakeMac}, + }, + }} + + mocked.On("LinkByName", "enp175s0f1").Return(fakeLink, nil) + mocked.On("LinkSetVfHardwareAddr", fakeLink, 0, fakeNewMac).Return(nil) + + err = SetVFHardwareMAC(mocked, "enp175s0f1", 0, "60:00:00:00:00:01") + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("hardware mac address is different from requested one")) + }) + + It("assuming calling function does not fails", func() { + mocked := &mocks_utils.NetlinkManager{} + fakeMac, err := net.ParseMAC("60:00:00:00:00:01") + Expect(err).ToNot(HaveOccurred()) + fakeNewMac, err := net.ParseMAC("60:00:00:00:00:01") + Expect(err).ToNot(HaveOccurred()) + + fakeLink := &FakeLink{netlink.LinkAttrs{ + Index: 1000, + Name: "enp175s0f1", + Vfs: []netlink.VfInfo{ + {Mac: fakeMac}, + }, + }} + + mocked.On("LinkByName", "enp175s0f1").Return(fakeLink, nil) + mocked.On("LinkSetVfHardwareAddr", fakeLink, 0, fakeNewMac).Return(nil) + + err = SetVFHardwareMAC(mocked, "enp175s0f1", 0, "60:00:00:00:00:01") + Expect(err).ToNot(HaveOccurred()) + }) + }) })