source/pci: add iommu_group/type attribute #705

marquiz · 2021-12-21T13:23:10Z

Add iommu_group/type to the list of PCI device attributes that are
discovered. The value is the raw value from sysfs (i.e DMA, DMA-FQ or
identity).

No built-in (automatic) labels are generated based on this, but, the
attribute is available for custom label rules to use. Examples of custom
rules:

  - name: "iommu enabled rule"
    labels:
      iommu.enabled: "true"
    matchFeatures:
      - feature: pci.device
        matchExpressions:
          "iommu_group/type": {op: NotIn, value: ["unknown"]}

  - name: "iommu passthrough rule"
    labels:
      iommu.passthrough: "true"
    matchFeatures:
      - feature: pci.device
        matchExpressions:
          "iommu_group/type": {op: In, value: ["identity"]}

k8s-ci-robot · 2021-12-21T13:23:27Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: marquiz

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [marquiz]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

marquiz · 2021-12-21T13:23:32Z

Depends on #677
/hold

marquiz · 2021-12-21T13:24:31Z

/assign zvonkok
ping @mythi

mythi · 2022-01-07T08:39:17Z

"iommu_group/type": {op: Exists}

I have a VM without an IOMMU and it gives iommu_group/type as well.

matchFeatures:
      - feature: pci.device

does it mean there's also net.device with the same function after this PR?

Might be useful to have more capabilities supported too, e.g., amd vs intel IOMMU and IOMMU version

marquiz · 2022-01-07T10:09:56Z

I have a VM without an IOMMU and it gives iommu_group/type as well.

Hmm, interesting 🧐 Want to take a closer look at that, what is the value of the type then?

does it mean there's also net.device with the same function after this PR?

Yup, those will have the same property.

Might be useful to have more capabilities supported too, e.g., amd vs intel IOMMU and IOMMU version

We could add e.g. iommu/intel-iommu/version to address that. Do we have a use case for that, WDYT? I have no experience (or hw) on amd.

marquiz · 2022-01-07T10:24:27Z

Maybe it the instructions to see if iommu is enabled should then be

  - name: "iommu enabled rule"
    labels:
      iommu.enabled: "true"
    matchFeatures:
      - feature: pci.device
        matchExpressions:
          "iommu_group/type": {op: NotIn, value: ["unknown"]}

marquiz · 2022-01-07T11:56:03Z

Rebased. Changed the instructions in the first commit message

marquiz · 2022-01-07T12:01:48Z

/unhold

marquiz · 2022-01-10T12:42:41Z

@mythi any comments to the above?

ping @zvonkok, any feedback?

This is virtually the last open bit for v0.10.0 😊

ArangoGutierrez · 2022-01-10T14:03:28Z

/test all

ArangoGutierrez · 2022-01-10T15:48:06Z

tested with quay.io/eduardoarango/node-feature-discovery:705

go test ./cmd/... ./pkg/... ./source/...
?       sigs.k8s.io/node-feature-discovery/cmd/nfd-master       [no test files]
ok      sigs.k8s.io/node-feature-discovery/cmd/nfd-topology-updater     0.010s
ok      sigs.k8s.io/node-feature-discovery/cmd/nfd-worker       0.009s
?       sigs.k8s.io/node-feature-discovery/pkg/api/feature      [no test files]
?       sigs.k8s.io/node-feature-discovery/pkg/apihelper        [no test files]
ok      sigs.k8s.io/node-feature-discovery/pkg/apis/nfd/v1alpha1        0.009s
?       sigs.k8s.io/node-feature-discovery/pkg/cpuid    [no test files]
?       sigs.k8s.io/node-feature-discovery/pkg/generated/clientset/versioned    [no test files]
?       sigs.k8s.io/node-feature-discovery/pkg/generated/clientset/versioned/fake       [no test files]
?       sigs.k8s.io/node-feature-discovery/pkg/generated/clientset/versioned/scheme     [no test files]
?       sigs.k8s.io/node-feature-discovery/pkg/generated/clientset/versioned/typed/nfd/v1alpha1 [no test files]
?       sigs.k8s.io/node-feature-discovery/pkg/generated/clientset/versioned/typed/nfd/v1alpha1/fake    [no test files]
?       sigs.k8s.io/node-feature-discovery/pkg/generated/informers/externalversions     [no test files]
?       sigs.k8s.io/node-feature-discovery/pkg/generated/informers/externalversions/internalinterfaces  [no test files]
?       sigs.k8s.io/node-feature-discovery/pkg/generated/informers/externalversions/nfd [no test files]
?       sigs.k8s.io/node-feature-discovery/pkg/generated/informers/externalversions/nfd/v1alpha1        [no test files]
?       sigs.k8s.io/node-feature-discovery/pkg/generated/listers/nfd/v1alpha1   [no test files]
ok      sigs.k8s.io/node-feature-discovery/pkg/kubeconf 0.015s
?       sigs.k8s.io/node-feature-discovery/pkg/labeler  [no test files]
?       sigs.k8s.io/node-feature-discovery/pkg/nfd-client       [no test files]
ok      sigs.k8s.io/node-feature-discovery/pkg/nfd-client/topology-updater      0.020s
ok      sigs.k8s.io/node-feature-discovery/pkg/nfd-client/worker        3.105s
ok      sigs.k8s.io/node-feature-discovery/pkg/nfd-master       0.021s
?       sigs.k8s.io/node-feature-discovery/pkg/podres   [no test files]
ok      sigs.k8s.io/node-feature-discovery/pkg/resourcemonitor  0.027s
?       sigs.k8s.io/node-feature-discovery/pkg/topologypolicy   [no test files]
?       sigs.k8s.io/node-feature-discovery/pkg/topologyupdater  [no test files]
ok      sigs.k8s.io/node-feature-discovery/pkg/utils    0.022s
?       sigs.k8s.io/node-feature-discovery/pkg/version  [no test files]
ok      sigs.k8s.io/node-feature-discovery/source       0.017s
ok      sigs.k8s.io/node-feature-discovery/source/cpu   0.009s
?       sigs.k8s.io/node-feature-discovery/source/custom        [no test files]
?       sigs.k8s.io/node-feature-discovery/source/custom/rules  [no test files]
?       sigs.k8s.io/node-feature-discovery/source/fake  [no test files]
?       sigs.k8s.io/node-feature-discovery/source/iommu [no test files]
ok      sigs.k8s.io/node-feature-discovery/source/kernel        0.005s
ok      sigs.k8s.io/node-feature-discovery/source/local 0.011s
ok      sigs.k8s.io/node-feature-discovery/source/memory        0.024s
ok      sigs.k8s.io/node-feature-discovery/source/network       0.045s
ok      sigs.k8s.io/node-feature-discovery/source/pci   0.020s
ok      sigs.k8s.io/node-feature-discovery/source/storage       0.020s
ok      sigs.k8s.io/node-feature-discovery/source/system        0.006s
ok      sigs.k8s.io/node-feature-discovery/source/usb   0.012s

-/-

Ran 3 of 230 Specs in 26.624 seconds
SUCCESS! -- 3 Passed | 0 Failed | 0 Pending | 227 Skipped
--- PASS: TestE2E (26.64s)
PASS
ok      sigs.k8s.io/node-feature-discovery/test/e2e     26.677s

Add "iommu_group/type" to the list of PCI device attributes that are discovered. The value is the raw value from sysfs (i.e DMA, DMA-FQ or identity). No built-in (automatic) labels are generated based on this, but, the attribute is available for custom label rules to use. Examples of custom rules: - name: "iommu enabled rule" labels: iommu.enabled: "true" matchFeatures: - feature: pci.device matchExpressions: "iommu_group/type": {op: NotIn, value: ["unknown"]} - name: "iommu passthrough rule" labels: iommu.passthrough: "true" matchFeatures: - feature: pci.device matchExpressions: "iommu_group/type": {op: In, value: ["identity"]}

marquiz · 2022-01-11T09:55:40Z

Rebased, updated docs

mythi · 2022-01-11T10:15:28Z

source/pci is OK but I feel source/network is unnecessary. with source/network today it's not possible to say to which device the attribute is linked. therefore, for better usability checking "PCI is a NIC and PCI has IOMMU" sounds more reasonable?

marquiz · 2022-01-11T12:39:07Z

I feel source/network is unnecessary. with source/network today it's not possible to say to which device the attribute is linked. therefore, for better usability checking "PCI is a NIC and PCI has IOMMU" sounds more reasonable?

This is a fair question to which I don't have an answer 🧐 I guess why I added this to network, too, was just following sr-iov. The sriov attributes are available for both pci and network devices. network can be easily dropped from this pr. Any thoughts @zvonkok ?

marquiz · 2022-01-11T12:51:15Z

Dropped the patch for source/network. Now only concerns source/pci.

/retitle source/pci: add iommu_group/type attribute

marquiz · 2022-01-11T12:58:14Z

Network part now submitted separately in #717

mythi · 2022-01-11T13:09:21Z

/lgtm

thanks!

k8s-ci-robot · 2022-01-11T13:09:31Z

@mythi: changing LGTM is restricted to collaborators

In response to this:

/lgtm

thanks!

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

ArangoGutierrez · 2022-01-11T13:18:39Z

/lgtm

thanks!

/lgtm

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Dec 21, 2021

k8s-ci-robot requested review from adrianchiris and kad December 21, 2021 13:23

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 21, 2021

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 21, 2021

marquiz mentioned this pull request Dec 21, 2021

Drop the iommu feature source #676

Closed

k8s-ci-robot assigned zvonkok Dec 21, 2021

marquiz added this to the v0.10.0 milestone Dec 22, 2021

marquiz force-pushed the devel/iommu-dev-attributes branch from 3260d62 to 3547670 Compare January 7, 2022 11:52

k8s-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jan 7, 2022

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 7, 2022

marquiz force-pushed the devel/iommu-dev-attributes branch from 3547670 to 865bc43 Compare January 11, 2022 09:55

marquiz mentioned this pull request Jan 11, 2022

source/pci: detect intel-iommu/version #716

Merged

marquiz force-pushed the devel/iommu-dev-attributes branch from 865bc43 to 152f353 Compare January 11, 2022 12:48

k8s-ci-robot changed the title ~~Add iommu_group/type attribute to pci and network device features~~ source/pci: add iommu_group/type attribute Jan 11, 2022

k8s-ci-robot assigned ArangoGutierrez Jan 11, 2022

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 11, 2022

k8s-ci-robot merged commit a1cb431 into kubernetes-sigs:master Jan 11, 2022

marquiz deleted the devel/iommu-dev-attributes branch January 11, 2022 13:27

marquiz mentioned this pull request Jan 11, 2022

Release v0.10.0 #678

Closed

22 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

source/pci: add iommu_group/type attribute #705

source/pci: add iommu_group/type attribute #705

marquiz commented Dec 21, 2021 •

edited

k8s-ci-robot commented Dec 21, 2021

marquiz commented Dec 21, 2021

marquiz commented Dec 21, 2021

mythi commented Jan 7, 2022

marquiz commented Jan 7, 2022

marquiz commented Jan 7, 2022

marquiz commented Jan 7, 2022

marquiz commented Jan 7, 2022

marquiz commented Jan 10, 2022

ArangoGutierrez commented Jan 10, 2022

ArangoGutierrez commented Jan 10, 2022

marquiz commented Jan 11, 2022

mythi commented Jan 11, 2022

marquiz commented Jan 11, 2022

marquiz commented Jan 11, 2022

marquiz commented Jan 11, 2022

mythi commented Jan 11, 2022

k8s-ci-robot commented Jan 11, 2022

ArangoGutierrez commented Jan 11, 2022

source/pci: add iommu_group/type attribute #705

source/pci: add iommu_group/type attribute #705

Conversation

marquiz commented Dec 21, 2021 • edited

k8s-ci-robot commented Dec 21, 2021

marquiz commented Dec 21, 2021

marquiz commented Dec 21, 2021

mythi commented Jan 7, 2022

marquiz commented Jan 7, 2022

marquiz commented Jan 7, 2022

marquiz commented Jan 7, 2022

marquiz commented Jan 7, 2022

marquiz commented Jan 10, 2022

ArangoGutierrez commented Jan 10, 2022

ArangoGutierrez commented Jan 10, 2022

marquiz commented Jan 11, 2022

mythi commented Jan 11, 2022

marquiz commented Jan 11, 2022

marquiz commented Jan 11, 2022

marquiz commented Jan 11, 2022

mythi commented Jan 11, 2022

k8s-ci-robot commented Jan 11, 2022

ArangoGutierrez commented Jan 11, 2022

marquiz commented Dec 21, 2021 •

edited