Repository: kubernetes-sigs/node-feature-discovery
Pull request #290 (2020-02-18 10:11:20)

Support custom features
This PR implements #287 

k8s-ci-robot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by: *<a href="https://github.com/kubernetes-sigs/node-feature-discovery/pull/290#" title="Author self-approved">adrianchiris</a>*
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please assign **balajismaniam**
You can assign the PR to them by writing `/assign @balajismaniam` in a comment when ready.

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=kubernetes-sigs%2Fnode-feature-discovery).

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/kubernetes-sigs/node-feature-discovery/blob/master/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["balajismaniam"]} -->
adrianchiris: @marquiz 
adrianchiris: > Put the common pci discovery library under pkg/

@marquiz the intention was to utilize golang [internal packages ](https://golang.org/doc/go1.4#internalpackages ) which will prevent other projects from importing the package.

/pkg usually is intended (i think...) for exposing stable API for other projects to consume.

anyway, if that's not a concern i can move no problem.

marquiz: > the intention was to utilize golang [internal packages ](https://golang.org/doc/go1.4#internalpackages) which will prevent other projects from importing the package.

@adrianchiris thanks for educating me :mortar_board: I wasn't even aware of this golang "kludge". In this  light the path is fine. We might consider moving some other stuff there, too...
k8s-ci-robot: @adrianchiris: PR needs rebase.

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
</details>
marquiz: @adrianchiris : needs rebase after #288 was merged
zvonkok: > Thanks @adrianchiris for the patchset!
> 
> I started to think that is _PciIdRegex_ in this form the sort of matching we want? I was thinking about #288 which adds detection of one new PCI device attribute, and, having a bit more "k8s-style" matcher something like:
> 
> ```
> PciDevAttrs:
> - vendor: ['15b3']
> - vendor: ['1234']
>   device: ['1000', '1010', '1020']
> ```
> 
> don't know if that's a good or bad idea, though. @zvonkok ?

@marquiz  @adrianchiris  I do not think we need a regex to match against specific PCI device. It looks cumbersome and people who do not understand regex, beware we are not all developers, may have trouble understanding what is this matched against. 

The `vendor: ['15b3']` has the same information content as `.*:15b3:.*`  is easier to read and one immediately sees what we're matching against. 

To understand  `.*:15b3:.*` one would have to know that 15b3 is the vendor id, and what `.*:`, `:.*` means in this context

We have the functions to dissect the PCI attributes, we can leverage that. 

The chaining of attributes to be more specific looks IMHO nicer when used e.g.:

```
PciDevAttrs:
- vendor: ['15b3']
  device:  ['1000', '1010', '1020']
```

Without thinking about we can see what we're matching against.



adrianchiris: @zvonkok @marquiz  thank you for providing feedback,  i see your point.
ill respin the PR and address the comments

Instead of a PciIdRegex rule we shall have a PciDevAttrs rule which will match on a list of PCI device attributes. each device attribute will be composed of list elements. 

Match operation: OR will be performed per element and AND will be performed per attribute.
An empty attribute will not be included in the matching process.

`
PciDevAttrs:
  - class : ["0200"]
    vendor : ["15b3"]
    device : ["1014", "1016"]
  - vendor : ["14e4"]
    device : ["165f"]
`
in the example above we will match on:
PciDevAttrs[0] : class 0200 AND vendor 15b3 AND device (1014 OR 1016)
PciDevAttrs[1] : vendor 14e4 AND device 165f

Sounds good ?
marquiz: > Sounds good ?
@adrianchiris thanks for excplicitly writing out the logic. Yes, that is how I envisioned it would work. @zvonkok ?

marquiz: But, of course, there are other alternatives for the interface, too. For example, a more flexible (and clumsy) would be to have something similar to `matchExpressions` used in k8s selectors
```
- vendor: {operator: In, values: ["15b3"]}
  device: {operator: In, values: ["1014", "1016"]}
```
That might be just overcomplication, though, because the only meaningful operators I can currently think of are `In` and `NotIn`. OTOH, also other operators (like `Lt`, `Gt`, `Exists`, `DoesNotExist`) could make sense in the future if more device attributes for matching are enabled in the future
adrianchiris: I think we should avoid over complication (thinking from the user perspective here)
Perhaps name the match rule to PciID to limit the scope ?

Then if/when there are more use cases have one that matches on other attributes.

marquiz: > I think we should avoid over complication (thinking from the user perspective here)

Agree

> Perhaps name the match rule to PciID to limit the scope ?

Mm, that might be better



> Then if/when there are more use cases have one that matches on other attributes.

We can do that
mythi: I've looked into `source/pci/` recently and found this PR interesting.

One question though: what are 'custom features'. 

My observations are: 1) the functionality does not bring sources as such (it re-uses what pci source does and adds a bit on top of `source/kernel`), 2) a new match needs new rules (that are built-in).

Should this feature be turned to a configurable label-filter where user specified filters are run for each source instead?
adrianchiris: ```
My observations are: 1) the functionality does not bring sources as such (it re-uses what pci source does and adds a bit on top of source/kernel), 2) a new match needs new rules (that are built-in).

Should this feature be turned to a configurable label-filter where user specified filters are run for each source instead?
```

The intention is not to limit the functionality to other sources.
There are bits of code that's going to be reused across sources (hence i also move some code to a common place).

But rather to define framework and an expandable set of match rules which users can (simply) compose to trigger NFD to label a node with a label of their choosing given the rules match.
marquiz: Yes, in the first phase we would have pci and kmod matchesrs, butm the idea is that we would be able to add new matchers when needed.

And, there would be a set of built-in rules (to satisfy the `rdma.capable` use case that @adrianchiris  presented in #261. In addition, there would be a set of configurable user-defined rules.
mythi: > Yes, in the first phase we would have pci and kmod matchesrs, butm the idea is that we would be able to add new matchers when needed.

My comment was that why not to have a generic matcher that filters source plugins' output.
marquiz: Matchers might not have any "source plugin". An example is the mkod matcher
zvonkok: > Sounds good ?

Yes, sounds good. This makes it fairly easy to add some "simple" rules to detect a specific device. 

Thinking of the operators I might have a use-case where a `Gt` could help but than we would need to rely on the vendor to populate the PCI class ID with ascending numbers, which is not a guarantee, there is no PCI class ID naming spec. e.g NVIDIAs M60 is a 0x300 class id and the V100 and T4 are 0x302. To cover only the "latest and greatest" detection of cards one could write

```
- vendor: {operator: In, values: ["10de"]}
  class: {operator: Gt, values: ["300"]}
```

But this can be also easily be done by the current approach. Lets start with the simple config and extend if we need it. 

Configs from customers and internal POCs will tell us what we need and which pattern we would need to look for. 











mythi: > Matchers might not have any "source plugin". An example is the mkod matcher

My concern is that conceptually the matchers become data sources too.
marquiz: > My concern is that conceptually the matchers become data sources too.

Well yes, they are. But not feature sources (as specified in the nfd internal api)
adrianchiris: @marquiz @zvonkok inputs appreciated :)
marquiz: > @marquiz @zvonkok inputs appreciated :)

Oops, sorry. I missed the new version of the patchset, I'll take a closer look soon
zvonkok: @marquiz Any additional comments? 
marquiz: > @marquiz @zvonkok inputs appreciated :)

Thanks for the update and apologies for the delay.

Some comments above, but, overall, I start to like this 😉 


Patch:
diff --git a/README.md b/README.md
index cd4789b13..e755fa0f6 100644
--- a/README.md
+++ b/README.md
@@ -150,6 +150,7 @@ for up-to-date information about the required volume mounts.
 The current set of feature sources are the following:
 
 - CPU
+- Custom
 - IOMMU
 - Kernel
 - Memory
@@ -181,6 +182,7 @@ feature logically has sub-hierarchy, e.g. `sriov.capable` and
 ```json
 {
   "feature.node.kubernetes.io/cpu-<feature-name>": "true",
+  "feature.node.kubernetes.io/custom-<feature-name>": "true",
   "feature.node.kubernetes.io/iommu-<feature-name>": "true",
   "feature.node.kubernetes.io/kernel-<feature name>": "<feature value>",
   "feature.node.kubernetes.io/memory-<feature-name>": "true",
@@ -251,6 +253,127 @@ capability might be supported but not enabled.
 | JSCVT     | Perform Conversion to Match Javascript
 | DCPOP     | Persistent Memory Support
 
+### Custom Features
+The Custom feature source allows the user to define features based on a mix of predefined rules.
+A rule is provided input witch affects its process of matching for a defined feature.
+
+To aid in making Custom Features clearer, we define a general and a per rule nomenclature, keeping things as
+consistent as possible.
+
+#### General Nomenclature & Definitions
+```
+Rule        :Represents a matching logic that is used to match on a feature.
+Rule Input  :The input a Rule is provided. This determines how a Rule performs the match operation.
+Matcher     :A composition of Rules, each Matcher may be composed of at most one instance of each Rule.
+```
+
+#### Custom Features Format (using the Nomenclature defined above)
+```yaml
+- name: <feature name>
+  matchOn:
+  - <Rule-1>: <Rule-1 Input>
+    [<Rule-2>: <Rule-2 Input>]
+  - <Matcher-2>
+  - ...
+  - ...
+  - <Matcher-N>
+- <custom feature 2>
+- ...
+- ...
+- <custom feature M>
+```
+
+#### Matching process
+Specifying Rules to match on a feature is done by providing a list of Matchers.
+Each Matcher contains one or more Rules.
+
+Logical _OR_ is performed between Matchers and logical _AND_ is performed between Rules
+of a given Matcher.
+
+#### Rules
+##### PciId Rule
+###### Nomenclature
+```
+Attribute   :A PCI attribute.
+Element     :An identifier of the PCI attribute.
+```
+
+The PciId Rule allows matching the PCI devices in the system on the following Attributes: `class`,`vendor` and
+`device`. A list of Elements is provided for each Attribute.
+
+###### Format
+```yaml
+pciId :
+  class: [<class id>, ...]
+  vendor: [<vendor id>,  ...]
+  device: [<device id>, ...]
+```
+
+Matching is done by performing a logical _OR_ between Elements of an Attribute and logical _AND_ between the specified Attributes for
+each PCI device in the system.
+At least one Attribute must be specified. Missing attributes will not partake in the matching process.
+
+##### LoadedKMod Rule
+###### Nomenclature
+```
+Element     :A kernel module
+```
+
+The LoadedKMod Rule allows matching the loaded kernel modules in the system against a provided list of Elements.
+
+###### Format
+```yaml
+loadedKMod : [<kernel module>, ...]
+```
+ Matching is done by performing logical _AND_ for each provided Element, i.e the Rule will match if all provided Elements (kernel modules) are loaded
+ in the system.
+
+#### Example
+```yaml
+custom:
+  - name: "my.kernel.feature"
+    matchOn:
+      - loadedKMod: ["kmod1", "kmod2"]
+  - name: "my.pci.feature"
+    matchOn:
+      - pciId:
+          vendor: ["15b3"]
+          device: ["1014", "1017"]
+  - name: "my.combined.feature"
+    matchOn:
+      - loadedKMod : ["vendor_kmod1", "vendor_kmod2"]
+        pciId:
+          vendor: ["15b3"]
+          device: ["1014", "1017"] 
+  - name: "my.accumulated.feature"
+    matchOn:
+      - loadedKMod : ["some_kmod1", "some_kmod2"]
+      - pciId:
+          vendor: ["15b3"]
+          device: ["1014", "1017"]
+```
+
+__In the example above:__
+- A node would contain the label: `feature.node.kubernetes.io/custom-my.kernel.feature=true`
+if the node has `kmod1` _AND_ `kmod2` kernel modules loaded. 
+- A node would contain the label: `feature.node.kubernetes.io/custom-my.pci.feature=true`
+if the node contains a PCI device with a PCI vendor ID of `15b3` _AND_ PCI device ID of `1014` _OR_ `1017`.
+- A node would contain the label: `feature.node.kubernetes.io/custom-my.combined.feature=true`
+if `vendor_kmod1` _AND_ `vendor_kmod2` kernel modules are loaded __AND__ the node contains a PCI device
+with a PCI vendor ID of `15b3` _AND_ PCI device ID of `1014` _or_ `1017`.
+- A node would contain the label: `feature.node.kubernetes.io/custom-my.accumulated.feature=true`
+if `some_kmod1` _AND_ `some_kmod2` kernel modules are loaded __OR__ the node contains a PCI device
+with a PCI vendor ID of `15b3` _AND_ PCI device ID of `1014` _OR_ `1017`.
+
+#### Statically defined features
+Some feature labels which are common and generic are defined statically in the `custom` feature source.
+A user may add additional Matchers to these feature labels by defining them in the `nfd-worker` configuration file.
+
+| Feature | Attribute | Description |
+| ------- | --------- | -----------|
+| rdma  | capable | The node has an RDMA capable Network adapter |
+| rdma | enabled | The node has the needed RDMA modules loaded to run RDMA traffic |
+
 ### IOMMU Features
 
 | Feature name   | Description                                                                         |
diff --git a/cmd/nfd-worker/main.go b/cmd/nfd-worker/main.go
index b11b2f4c9..48a8ac3a7 100644
--- a/cmd/nfd-worker/main.go
+++ b/cmd/nfd-worker/main.go
@@ -91,7 +91,7 @@ func argsParse(argv []string) (worker.Args, error) {
                               in testing
                               [Default: ]
   --sources=<sources>         Comma separated list of feature sources.
-                              [Default: cpu,iommu,kernel,local,memory,network,pci,storage,system]
+                              [Default: cpu,custom,iommu,kernel,local,memory,network,pci,storage,system]
   --no-publish                Do not publish discovered features to the
                               cluster-local Kubernetes API server.
   --label-whitelist=<pattern> Regular expression to filter label names to
diff --git a/nfd-worker.conf.example b/nfd-worker.conf.example
index f52acb342..591385874 100644
--- a/nfd-worker.conf.example
+++ b/nfd-worker.conf.example
@@ -44,3 +44,22 @@
 #      - "device"
 #      - "subsystem_vendor"
 #      - "subsystem_device"
+#  custom:
+#    - name: "my.kernel.feature"
+#      matchOn:
+#        - loadedKMod: ["example_kmod1", "example_kmod2"]
+#    - name: "my.pci.feature"
+#      matchOn:
+#        - pciId:
+#            class: ["0200"]
+#            vendor: ["15b3"]
+#            device: ["1014", "1017"]
+#        - pciId :
+#            vendor: ["8086"]
+#            device: ["1000", "1100"]
+#    - name: "my.combined.feature"
+#      matchOn:
+#        - pciId:
+#            vendor: ["15b3"]
+#            device: ["1014", "1017"]
+#          loadedKMod : ["vendor_kmod1", "vendor_kmod2"]
diff --git a/pkg/nfd-worker/nfd-worker.go b/pkg/nfd-worker/nfd-worker.go
index e46e1421e..a353f8585 100644
--- a/pkg/nfd-worker/nfd-worker.go
+++ b/pkg/nfd-worker/nfd-worker.go
@@ -36,6 +36,7 @@ import (
 	"sigs.k8s.io/node-feature-discovery/pkg/version"
 	"sigs.k8s.io/node-feature-discovery/source"
 	"sigs.k8s.io/node-feature-discovery/source/cpu"
+	"sigs.k8s.io/node-feature-discovery/source/custom"
 	"sigs.k8s.io/node-feature-discovery/source/fake"
 	"sigs.k8s.io/node-feature-discovery/source/iommu"
 	"sigs.k8s.io/node-feature-discovery/source/kernel"
@@ -61,6 +62,7 @@ type NFDConfig struct {
 		Cpu    *cpu.NFDConfig    `json:"cpu,omitempty"`
 		Kernel *kernel.NFDConfig `json:"kernel,omitempty"`
 		Pci    *pci.NFDConfig    `json:"pci,omitempty"`
+		Custom *custom.NFDConfig `json:"custom,omitempty"`
 	} `json:"sources,omitempty"`
 }
 
@@ -234,6 +236,7 @@ func configParse(filepath string, overrides string) error {
 	config.Sources.Cpu = &cpu.Config
 	config.Sources.Kernel = &kernel.Config
 	config.Sources.Pci = &pci.Config
+	config.Sources.Custom = &custom.Config
 
 	data, err := ioutil.ReadFile(filepath)
 	if err != nil {
@@ -276,6 +279,7 @@ func configureParameters(sourcesWhiteList []string, labelWhiteListStr string) (e
 		pci.Source{},
 		storage.Source{},
 		system.Source{},
+		custom.Source{},
 		// local needs to be the last source so that it is able to override
 		// labels from other sources
 		local.Source{},
diff --git a/source/custom/custom.go b/source/custom/custom.go
new file mode 100644
index 000000000..300f79a47
--- /dev/null
+++ b/source/custom/custom.go
@@ -0,0 +1,93 @@
+/*
+Copyright 2020 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package custom
+
+import (
+	"fmt"
+	"log"
+
+	"sigs.k8s.io/node-feature-discovery/source"
+	"sigs.k8s.io/node-feature-discovery/source/custom/rules"
+)
+
+// Custom Features Configurations
+type MatchRule struct {
+	PciId *rules.PciIdRule `json:"pciId,omitempty""`
+	LoadedKMod *rules.LoadedKModRule `json:"loadedKMod,omitempty""`
+}
+
+type CustomFeature struct {
+	Name    string      `json:"name"`
+	MatchOn []MatchRule `json:"matchOn"`
+}
+
+type NFDConfig []CustomFeature
+
+var Config = NFDConfig{}
+
+// Implements FeatureSource Interface
+type Source struct{}
+
+// Return name of the feature source
+func (s Source) Name() string { return "custom" }
+
+// Discover features
+func (s Source) Discover() (source.Features, error) {
+	features := source.Features{}
+	allFeatureConfig := append(getStaticFeatureConfig(), Config...)
+	log.Printf("INFO: Custom features: %+v", allFeatureConfig)
+	// Iterate over features
+	for _, customFeature := range allFeatureConfig {
+		featureExist, err := s.discoverFeature(customFeature)
+		if err != nil {
+			return features, fmt.Errorf("failed to discover feature: %s. %s", customFeature.Name, err.Error())
+		}
+		if featureExist {
+			features[customFeature.Name] = true
+		}
+	}
+	return features, nil
+}
+
+// Process a single feature by Matching on the defined rules.
+// A feature is present if all defined Rules in a MatchRule return a match.
+func (s Source) discoverFeature(feature CustomFeature) (bool, error) {
+	for _, rule := range feature.MatchOn {
+		// PCI ID rule
+		if rule.PciId != nil {
+			match, err := rule.PciId.Match()
+			if err != nil {
+				return false, err
+			}
+			if !match {
+				continue
+			}
+		}
+		// Loaded kernel module rule
+		if rule.LoadedKMod != nil {
+			match, err := rule.LoadedKMod.Match()
+			if err != nil {
+				return false, err
+			}
+			if !match {
+				continue
+			}
+		}
+		return true, nil
+	}
+	return false, nil
+}
diff --git a/source/custom/rules/loaded_kmod_rule.go b/source/custom/rules/loaded_kmod_rule.go
new file mode 100644
index 000000000..2bfb8c32f
--- /dev/null
+++ b/source/custom/rules/loaded_kmod_rule.go
@@ -0,0 +1,61 @@
+/*
+Copyright 2020 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package rules
+
+import (
+	"fmt"
+	"io/ioutil"
+	"strings"
+)
+
+// Rule that matches on loaded kernel modules in the system
+type LoadedKModRule []string
+
+const kmodProcfsPath = "/proc/modules"
+
+// Match loaded kernel modules on provided list of kernel modules
+func (kmods *LoadedKModRule) Match() (bool, error) {
+	loadedModules, err := kmods.getLoadedModules()
+	if err != nil {
+		return false, fmt.Errorf("failed to get loaded kernel modules. %s", err.Error())
+	}
+	for _, kmod := range *kmods {
+		if _, ok := loadedModules[kmod]; !ok {
+			// kernel module not loaded
+			return false, nil
+		}
+	}
+	return true, nil
+}
+
+func (kmods *LoadedKModRule) getLoadedModules() (map[string]struct{}, error) {
+	out, err := ioutil.ReadFile(kmodProcfsPath)
+	if err != nil {
+		return nil, fmt.Errorf("failed to read file %s: %s", kmodProcfsPath, err.Error())
+	}
+
+	loadedMods := make(map[string]struct{})
+	for _, line := range strings.Split(string(out), "\n") {
+		// skip empty lines
+		if len(line) == 0 {
+			continue
+		}
+		// append loaded module
+		loadedMods[strings.Fields(line)[0]] = struct{}{}
+	}
+	return loadedMods, nil
+}
diff --git a/source/custom/rules/pci_id_rule.go b/source/custom/rules/pci_id_rule.go
new file mode 100644
index 000000000..45931cddd
--- /dev/null
+++ b/source/custom/rules/pci_id_rule.go
@@ -0,0 +1,87 @@
+/*
+Copyright 2020 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package rules
+
+import (
+	"fmt"
+	pciutils "sigs.k8s.io/node-feature-discovery/source/internal"
+)
+
+// Rule that matches on the following PCI device attributes: <class, vendor, device>
+// each device attribute will be a list elements(strings).
+// Match operation: OR will be performed per element and AND will be performed per attribute.
+// An empty attribute will not be included in the matching process.
+type PciIdRuleInput struct {
+	Class  []string `json:"class,omitempty"`
+	Vendor []string `json:"vendor,omitempty"`
+	Device []string `json:"device,omitempty"`
+}
+
+type PciIdRule struct {
+	PciIdRuleInput
+}
+
+// Match PCI devices on provided PCI device attributes
+func (r *PciIdRule) Match() (bool, error) {
+	devAttr := map[string]bool{}
+	for _, attr := range []string{"class", "vendor", "device"} {
+		devAttr[attr] = true
+	}
+	allDevs, err := pciutils.DetectPci(devAttr)
+	if err != nil {
+		return false, fmt.Errorf("failed to detect PCI devices: %s", err.Error())
+	}
+
+	for _, classDevs := range allDevs {
+		for _, dev := range classDevs {
+			// match rule on a single device
+			if r.matchDevOnRule(dev) {
+				return true, nil
+			}
+		}
+	}
+	return false, nil
+}
+
+func (r *PciIdRule) matchDevOnRule(dev pciutils.PciDeviceInfo) bool {
+	if len(r.Class) == 0 && len(r.Vendor) == 0 && len(r.Device) == 0 {
+		return false
+	}
+
+	if len(r.Class) > 0 && !in(dev["class"], r.Class) {
+		return false
+	}
+
+	if len(r.Vendor) > 0 && !in(dev["vendor"], r.Vendor) {
+		return false
+	}
+
+	if len(r.Device) > 0 && !in(dev["device"], r.Device) {
+		return false
+	}
+
+	return true
+}
+
+func in(item string, arr []string) bool {
+	for _, val := range arr {
+		if val == item {
+			return true
+		}
+	}
+	return false
+}
diff --git a/source/custom/rules/rule.go b/source/custom/rules/rule.go
new file mode 100644
index 000000000..5632fccd0
--- /dev/null
+++ b/source/custom/rules/rule.go
@@ -0,0 +1,22 @@
+/*
+Copyright 2020 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package rules
+
+type Rule interface {
+	// Match on rule
+	Match() (bool, error)
+}
diff --git a/source/custom/static_features.go b/source/custom/static_features.go
new file mode 100644
index 000000000..8796380af
--- /dev/null
+++ b/source/custom/static_features.go
@@ -0,0 +1,46 @@
+/*
+Copyright 2020 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package custom
+
+import (
+	"sigs.k8s.io/node-feature-discovery/source/custom/rules"
+)
+
+// getStaticFeatures returns statically configured custom features to discover
+// e.g RMDA related features. NFD configuration file may extend these custom features by adding rules.
+func getStaticFeatureConfig() []CustomFeature {
+	return []CustomFeature{
+		CustomFeature{
+			Name: "rdma.capable",
+			MatchOn: []MatchRule{
+				MatchRule{
+					PciId: &rules.PciIdRule{
+						rules.PciIdRuleInput{Vendor: []string{"15b3"}},
+					},
+				},
+			},
+		},
+		CustomFeature{
+			Name: "rdma.available",
+			MatchOn: []MatchRule{
+				MatchRule{
+					LoadedKMod: &rules.LoadedKModRule{"ib_uverbs", "rdma_ucm"},
+				},
+			},
+		},
+	}
+}
diff --git a/source/internal/pci_utils.go b/source/internal/pci_utils.go
new file mode 100644
index 000000000..4caec5687
--- /dev/null
+++ b/source/internal/pci_utils.go
@@ -0,0 +1,98 @@
+/*
+Copyright 2020 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package pciutils
+
+import (
+	"fmt"
+	"io/ioutil"
+	"log"
+	"path"
+	"strings"
+)
+
+type PciDeviceInfo map[string]string
+
+var DefaultPciDevAttrs = []string{"class", "vendor", "device", "subsystem_vendor", "subsystem_device"}
+var ExtraPciDevAttrs = []string{"sriov_totalvfs"}
+
+// Read a single PCI device attribute
+// A PCI attribute in this context, maps to the corresponding sysfs file
+func readSingleAttribute(devPath string, attrName string) (string, error) {
+	data, err := ioutil.ReadFile(path.Join(devPath, attrName))
+	if err != nil {
+		return "", fmt.Errorf("failed to read device attribute %s: %v", attrName, err)
+	}
+	// Strip whitespace and '0x' prefix
+	attrVal := strings.TrimSpace(strings.TrimPrefix(string(data), "0x"))
+
+	if attrName == "class" && len(attrVal) > 4 {
+		// Take four first characters, so that the programming
+		// interface identifier gets stripped from the raw class code
+		attrVal = attrVal[0:4]
+	}
+	return attrVal, nil
+}
+
+// Read information of one PCI device
+func readDevInfo(devPath string, deviceAttrSpec map[string]bool) (PciDeviceInfo, error) {
+	info := PciDeviceInfo{}
+
+	for attr, must := range deviceAttrSpec {
+		attrVal, err := readSingleAttribute(devPath, attr)
+		if err != nil {
+			if must {
+				return info, fmt.Errorf("Failed to read device %s: %s", attr, err)
+			} else {
+				continue
+			}
+
+		}
+		info[attr] = attrVal
+	}
+	return info, nil
+}
+
+// List available PCI devices and retrieve device attributes.
+// deviceAttrSpec is a map which specifies which attributes to retrieve.
+// a false value for a specific attribute marks the attribute as optional.
+// a true value for a specific attribute marks the attribute as mandatory.
+// "class" attribute is considered mandatory.
+// DetectPci() will fail if the retrieval of a mandatory attribute fails.
+func DetectPci(deviceAttrSpec map[string]bool) (map[string][]PciDeviceInfo, error) {
+	const basePath = "/sys/bus/pci/devices/"
+	devInfo := make(map[string][]PciDeviceInfo)
+
+	devices, err := ioutil.ReadDir(basePath)
+	if err != nil {
+		return nil, err
+	}
+	// "class" is a mandatory attribute, inject it to spec if needed.
+	deviceAttrSpec["class"] = true
+
+	// Iterate over devices
+	for _, device := range devices {
+		info, err := readDevInfo(path.Join(basePath, device.Name()), deviceAttrSpec)
+		if err != nil {
+			log.Print(err)
+			continue
+		}
+		class := info["class"]
+		devInfo[class] = append(devInfo[class], info)
+	}
+
+	return devInfo, nil
+}
diff --git a/source/pci/pci.go b/source/pci/pci.go
index ed70f95a7..f1c337cbb 100644
--- a/source/pci/pci.go
+++ b/source/pci/pci.go
@@ -18,16 +18,13 @@ package pci
 
 import (
 	"fmt"
-	"io/ioutil"
 	"log"
-	"path"
 	"strings"
 
 	"sigs.k8s.io/node-feature-discovery/source"
+	"sigs.k8s.io/node-feature-discovery/source/internal"
 )
 
-type pciDeviceInfo map[string]string
-
 type NFDConfig struct {
 	DeviceClassWhitelist []string `json:"deviceClassWhitelist,omitempty"`
 	DeviceLabelFields    []string `json:"deviceLabelFields,omitempty"`
@@ -38,9 +35,6 @@ var Config = NFDConfig{
 	DeviceLabelFields:    []string{"class", "vendor"},
 }
 
-var devLabelAttrs = []string{"class", "vendor", "device", "subsystem_vendor", "subsystem_device"}
-var extraDevAttrs = []string{"sriov_totalvfs"}
-
 // Implement FeatureSource interface
 type Source struct{}
 
@@ -58,7 +52,7 @@ func (s Source) Discover() (source.Features, error) {
 		configLabelFields[field] = true
 	}
 
-	for _, attr := range devLabelAttrs {
+	for _, attr := range pciutils.DefaultPciDevAttrs {
 		if _, ok := configLabelFields[attr]; ok {
 			deviceLabelFields = append(deviceLabelFields, attr)
 			delete(configLabelFields, attr)
@@ -79,14 +73,14 @@ func (s Source) Discover() (source.Features, error) {
 	// Read extraDevAttrs + configured or default labels. Attributes
 	// set to 'true' are considered must-have.
 	deviceAttrs := map[string]bool{}
-	for _, label := range extraDevAttrs {
+	for _, label := range pciutils.ExtraPciDevAttrs {
 		deviceAttrs[label] = false
 	}
 	for _, label := range deviceLabelFields {
 		deviceAttrs[label] = true
 	}
 
-	devs, err := detectPci(deviceAttrs)
+	devs, err := pciutils.DetectPci(deviceAttrs)
 	if err != nil {
 		return nil, fmt.Errorf("Failed to detect PCI devices: %s", err.Error())
 	}
@@ -112,54 +106,5 @@ func (s Source) Discover() (source.Features, error) {
 			}
 		}
 	}
-
 	return features, nil
 }
-
-// Read information of one PCI device
-func readDevInfo(devPath string, devAttrs map[string]bool) (pciDeviceInfo, error) {
-	info := pciDeviceInfo{}
-	for attr, must := range devAttrs {
-		data, err := ioutil.ReadFile(path.Join(devPath, attr))
-		if err != nil {
-			if must {
-				return info, fmt.Errorf("Failed to read device %s: %s", attr, err)
-			} else {
-				continue
-			}
-		}
-		// Strip whitespace and '0x' prefix
-		info[attr] = strings.TrimSpace(strings.TrimPrefix(string(data), "0x"))
-
-		if attr == "class" && len(info[attr]) > 4 {
-			// Take four first characters, so that the programming
-			// interface identifier gets stripped from the raw class code
-			info[attr] = info[attr][0:4]
-		}
-	}
-	return info, nil
-}
-
-// List available PCI devices
-func detectPci(devAttrs map[string]bool) (map[string][]pciDeviceInfo, error) {
-	const basePath = "/sys/bus/pci/devices/"
-	devInfo := make(map[string][]pciDeviceInfo)
-
-	devices, err := ioutil.ReadDir(basePath)
-	if err != nil {
-		return nil, err
-	}
-
-	// Iterate over devices
-	for _, device := range devices {
-		info, err := readDevInfo(path.Join(basePath, device.Name()), devAttrs)
-		if err != nil {
-			log.Print(err)
-			continue
-		}
-		class := info["class"]
-		devInfo[class] = append(devInfo[class], info)
-	}
-
-	return devInfo, nil
-}


Test patch:
diff --git a/cmd/nfd-worker/main_test.go b/cmd/nfd-worker/main_test.go
index 05824fa8d..90b51727c 100644
--- a/cmd/nfd-worker/main_test.go
+++ b/cmd/nfd-worker/main_test.go
@@ -23,6 +23,8 @@ import (
 	. "github.com/smartystreets/goconvey/convey"
 )
 
+var allSources = []string{"cpu", "custom", "iommu", "kernel", "local", "memory", "network", "pci", "storage", "system"}
+
 func TestArgsParse(t *testing.T) {
 	Convey("When parsing command line arguments", t, func() {
 		Convey("When --no-publish and --oneshot flags are passed", func() {
@@ -32,7 +34,7 @@ func TestArgsParse(t *testing.T) {
 				So(args.SleepInterval, ShouldEqual, 60*time.Second)
 				So(args.NoPublish, ShouldBeTrue)
 				So(args.Oneshot, ShouldBeTrue)
-				So(args.Sources, ShouldResemble, []string{"cpu", "iommu", "kernel", "local", "memory", "network", "pci", "storage", "system"})
+				So(args.Sources, ShouldResemble, allSources)
 				So(len(args.LabelWhiteList), ShouldEqual, 0)
 				So(err, ShouldBeNil)
 			})
@@ -56,7 +58,7 @@ func TestArgsParse(t *testing.T) {
 
 			Convey("args.labelWhiteList is set to appropriate value and args.sources is set to default value", func() {
 				So(args.NoPublish, ShouldBeFalse)
-				So(args.Sources, ShouldResemble, []string{"cpu", "iommu", "kernel", "local", "memory", "network", "pci", "storage", "system"})
+				So(args.Sources, ShouldResemble, allSources)
 				So(args.LabelWhiteList, ShouldResemble, ".*rdt.*")
 				So(err, ShouldBeNil)
 			})