Merge pull request #660 from marquiz/devel/network-feature-source

source/network: implement FeatureSource

deployment/base/nfd-crds/cr-sample.yaml

@@ -51,6 +51,11 @@ spec:
             rotational: {op: In, value: ["0"]}
             dax: {op: In, value: ["0"]}
 
+        - feature: network.device
+          matchExpressions:
+            operstate: {op: In, value: ["up"]}
+            speed: {op: Gt, value: ["100"]}
+
         - feature: system.osrelease
           matchExpressions:
             ID: {op: In, value: ["fedora", "centos"]}

deployment/components/worker-config/nfd-worker.conf.example

@@ -150,10 +150,16 @@
 #            major: {op: In, value: ["5"]}
 #            minor: {op: Gt, value: ["10"]}
 #
-#        - storage.block:
+#        - feature: storage.block
+#          matchExpressions:
 #            rotational: {op: In, value: ["0"]}
 #            dax: {op: In, value: ["0"]}
 #
+#        - feature: network.device
+#          matchExpressions:
+#            operstate: {op: In, value: ["up"]}
+#            speed: {op: Gt, value: ["100"]}
+#
 #        - feature: system.osrelease
 #          matchExpressions:
 #            ID: {op: In, value: ["fedora", "centos"]}

deployment/helm/node-feature-discovery/values.yaml

@@ -239,10 +239,16 @@ worker:
     #            major: {op: In, value: ["5"]}
     #            minor: {op: Gt, value: ["10"]}
     #
-    #        - storage.block:
+    #        - feature: storage.block
+    #          matchExpressions:
     #            rotational: {op: In, value: ["0"]}
     #            dax: {op: In, value: ["0"]}
     #
+    #        - feature: network.device
+    #          matchExpressions:
+    #            operstate: {op: In, value: ["up"]}
+    #            speed: {op: Gt, value: ["100"]}
+    #
     #        - feature: system.osrelease
     #          matchExpressions:
     #            ID: {op: In, value: ["fedora", "centos"]}

source/network/network.go

@@ -17,37 +17,43 @@ limitations under the License.
 package network
 
 import (
-	"bytes"
 	"fmt"
 	"io/ioutil"
 	"os"
+	"path/filepath"
 	"strconv"
 	"strings"
 
 	"k8s.io/klog/v2"
 
+	"sigs.k8s.io/node-feature-discovery/pkg/api/feature"
+	"sigs.k8s.io/node-feature-discovery/pkg/utils"
 	"sigs.k8s.io/node-feature-discovery/source"
 )
 
 const Name = "network"
 
-// Linux net iface flags (we only specify the first few)
-const (
-	flagUp = 1 << iota
-	_      // flagBroadcast
-	_      // flagDebug
-	flagLoopback
-)
+const DeviceFeature = "device"
 
 const sysfsBaseDir = "class/net"
 
-// networkSource implements the LabelSource interface.
-type networkSource struct{}
+// networkSource implements the FeatureSource and LabelSource interfaces.
+type networkSource struct {
+	features *feature.DomainFeatures
+}
 
 // Singleton source instance
 var (
 	src networkSource
-	_   source.LabelSource = &src
+	_   source.FeatureSource = &src
+	_   source.LabelSource   = &src
+)
+
+var (
+	// ifaceAttrs is the list of files under /sys/class/net/<iface> that we're trying to read
+	ifaceAttrs = []string{"operstate", "speed"}
+	// devAttrs is the list of files under /sys/class/net/<iface>/device that we're trying to read
+	devAttrs = []string{"sriov_numvfs", "sriov_totalvfs"}
 )
 
 // Name returns an identifier string for this feature source.
@@ -58,75 +64,104 @@ func (s *networkSource) Priority() int { return 0 }
 
 // GetLabels method of the LabelSource interface
 func (s *networkSource) GetLabels() (source.FeatureLabels, error) {
-	features := source.FeatureLabels{}
+	labels := source.FeatureLabels{}
+	features := s.GetFeatures()
 
-	netInterfaces, err := ioutil.ReadDir(source.SysfsDir.Path(sysfsBaseDir))
-	if err != nil {
-		return nil, fmt.Errorf("failed to list network interfaces: %s", err.Error())
-	}
-
-	// iterating through network interfaces to obtain their respective number of virtual functions
-	for _, netInterface := range netInterfaces {
-		name := netInterface.Name()
-		flags, err := readIfFlags(name)
-		if err != nil {
-			klog.Error(err)
+	for _, dev := range features.Instances[DeviceFeature].Elements {
+		attrs := dev.Attributes
+		if attrs["operstate"] != "up" {
 			continue
 		}
+		for attr, feature := range map[string]string{
+			"sriov_totalvfs": "sriov.capable",
+			"sriov_numvfs":   "sriov.configured"} {
 
-		if flags&flagUp != 0 && flags&flagLoopback == 0 {
-			totalBytes, err := ioutil.ReadFile(source.SysfsDir.Path(sysfsBaseDir, name, "device/sriov_totalvfs"))
-			if err != nil {
-				if !os.IsNotExist(err) {
-					klog.Errorf("failed to determine SR-IOV support for network interface: %s: %v", name, err)
-				}
-				continue
-			}
-			total := bytes.TrimSpace(totalBytes)
-			t, err := strconv.Atoi(string(total))
-			if err != nil {
-				klog.Errorf("error in obtaining maximum supported number of virtual functions for network interface: %s: %v", name, err)
-				continue
-			}
-			if t > 0 {
-				klog.V(1).Infof("SR-IOV capability is detected on the network interface: %s", name)
-				klog.V(1).Infof("%d maximum supported number of virtual functions on network interface: %s", t, name)
-				features["sriov.capable"] = true
-				numBytes, err := ioutil.ReadFile(source.SysfsDir.Path(sysfsBaseDir, name, "device/sriov_numvfs"))
+			if v, ok := attrs[attr]; ok {
+				t, err := strconv.Atoi(v)
 				if err != nil {
-					klog.V(1).Infof("SR-IOV not configured for network interface: %s: %s", name, err)
+					klog.Errorf("failed to parse %s of %s: %v", attr, attrs["name"])
 					continue
 				}
-				num := bytes.TrimSpace(numBytes)
-				n, err := strconv.Atoi(string(num))
-				if err != nil {
-					klog.Errorf("error in obtaining the configured number of virtual functions for network interface: %s: %v", name, err)
-					continue
-				}
-				if n > 0 {
-					klog.V(1).Infof("%d virtual functions configured on network interface: %s", n, name)
-					features["sriov.configured"] = true
-					break
-				} else if n == 0 {
-					klog.V(1).Infof("SR-IOV not configured on network interface: %s", name)
+				if t > 0 {
+					labels[feature] = true
 				}
 			}
 		}
 	}
-	return features, nil
+	return labels, nil
 }
 
-func readIfFlags(name string) (uint64, error) {
-	raw, err := ioutil.ReadFile(source.SysfsDir.Path(sysfsBaseDir, name, "flags"))
+// Discover method of the FeatureSource interface.
+func (s *networkSource) Discover() error {
+	s.features = feature.NewDomainFeatures()
+
+	devs, err := detectNetDevices()
 	if err != nil {
-		return 0, fmt.Errorf("failed to read flags for interface %q: %v", name, err)
+		return fmt.Errorf("failed to detect network devices: %w", err)
 	}
-	flags, err := strconv.ParseUint(strings.TrimSpace(string(raw)), 0, 64)
+	s.features.Instances[DeviceFeature] = feature.InstanceFeatureSet{Elements: devs}
+
+	utils.KlogDump(3, "discovered network features:", "  ", s.features)
+
+	return nil
+}
+
+// GetFeatures method of the FeatureSource Interface.
+func (s *networkSource) GetFeatures() *feature.DomainFeatures {
+	if s.features == nil {
+		s.features = feature.NewDomainFeatures()
+	}
+	return s.features
+}
+
+func detectNetDevices() ([]feature.InstanceFeature, error) {
+	sysfsBasePath := source.SysfsDir.Path(sysfsBaseDir)
+
+	ifaces, err := ioutil.ReadDir(sysfsBasePath)
 	if err != nil {
-		return 0, fmt.Errorf("failed to parse flags for interface %q: %v", name, err)
+		return nil, fmt.Errorf("failed to list network interfaces: %w", err)
 	}
 
-	return flags, nil
+	// Iterate over devices
+	info := make([]feature.InstanceFeature, 0, len(ifaces))
+	for _, iface := range ifaces {
+		name := iface.Name()
+		if _, err := os.Stat(filepath.Join(sysfsBasePath, name, "device")); err == nil {
+			info = append(info, readIfaceInfo(filepath.Join(sysfsBasePath, name)))
+		} else if klog.V(3).Enabled() {
+			klog.Infof("skipping non-device iface %q", name)
+		}
+	}
+
+	return info, nil
+}
+
+func readIfaceInfo(path string) feature.InstanceFeature {
+	attrs := map[string]string{"name": filepath.Base(path)}
+	for _, attrName := range ifaceAttrs {
+		data, err := ioutil.ReadFile(filepath.Join(path, attrName))
+		if err != nil {
+			if !os.IsNotExist(err) {
+				klog.Errorf("failed to read net iface attribute %s: %v", attrName, err)
+			}
+			continue
+		}
+		attrs[attrName] = strings.TrimSpace(string(data))
+	}
+
+	for _, attrName := range devAttrs {
+		data, err := ioutil.ReadFile(filepath.Join(path, "device", attrName))
+		if err != nil {
+			if !os.IsNotExist(err) {
+				klog.Errorf("failed to read net device attribute %s: %v", attrName, err)
+			}
+			continue
+		}
+		attrs[attrName] = strings.TrimSpace(string(data))
+	}
+
+	return *feature.NewInstanceFeature(attrs)
+
 }
 
 func init() {

source/network/network_test.go

@@ -0,0 +1,36 @@
+/*
+Copyright 2021 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 network
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"sigs.k8s.io/node-feature-discovery/pkg/api/feature"
+)
+
+func TestNetworkSource(t *testing.T) {
+	assert.Equal(t, src.Name(), Name)
+
+	// Check that GetLabels works with empty features
+	src.features = feature.NewDomainFeatures()
+	l, err := src.GetLabels()
+
+	assert.Nil(t, err, err)
+	assert.Empty(t, l)
+
+}