OCPBUGS-43442: Verify HW configuration to enable or disable T-GM Holdover

addons/generic/generic.go

@@ -2,15 +2,18 @@ package generic
 
 import (
 	"encoding/json"
+
 	"github.com/golang/glog"
 	"github.com/openshift/linuxptp-daemon/pkg/plugin"
 	ptpv1 "github.com/openshift/ptp-operator/api/v1"
 )
 
+// GenericPluginData is a struct to hold the reference string
 type GenericPluginData struct {
 	referenceString *string
 }
 
+// OnPTPConfigChangeGeneric is a generic function to handle PTPConfigChange
 func OnPTPConfigChangeGeneric(data *interface{}, nodeProfile *ptpv1.PtpProfile) error {
 	var pluginOpts string = ""
 	var err error
@@ -25,22 +28,21 @@ func OnPTPConfigChangeGeneric(data *interface{}, nodeProfile *ptpv1.PtpProfile)
 
 	if data != nil && pluginOpts != "" {
 		_data := *data
-		glog.Infof("Saving status to hwconfig: %s", string(pluginOpts))
-		var pluginData *GenericPluginData = _data.(*GenericPluginData)
+		glog.Infof("Saving status to hwconfig: %s", pluginOpts)
+		var pluginData = _data.(*GenericPluginData)
 		_pluginData := *pluginData
 		*_pluginData.referenceString = pluginOpts
 	}
 	glog.Infof("OnPTPConfigChangeGeneric: (%s)", pluginOpts)
 	return err
-
 }
 
 func PopulateHwConfigGeneric(data *interface{}, hwconfigs *[]ptpv1.HwConfig) error {
 	glog.Infof("PopulateHwConfigGeneric")
 	var referenceString string = ""
 	if data != nil {
 		_data := *data
-		var pluginData *GenericPluginData = _data.(*GenericPluginData)
+		var pluginData = _data.(*GenericPluginData)
 		_pluginData := *pluginData
 		if _pluginData.referenceString != nil {
 			referenceString = *_pluginData.referenceString

addons/intel/e810.go

@@ -5,6 +5,7 @@ import (
 	"encoding/binary"
 	"encoding/json"
 	"fmt"
+	hwplugin "github.com/openshift/linuxptp-daemon/addons/plugin"
 	"os"
 	"os/exec"
 	"reflect"
@@ -95,7 +96,7 @@ func OnPTPConfigChangeE810(data *interface{}, nodeProfile *ptpv1.PtpProfile) err
 	e810Opts.EnableDefaultConfig = false
 
 	for name, opts := range (*nodeProfile).Plugins {
-		if name == "e810" {
+		if name == hwplugin.E810 {
 			optsByteArray, _ = json.Marshal(opts)
 			err = json.Unmarshal(optsByteArray, &e810Opts)
 			if err != nil {
@@ -121,9 +122,9 @@ func OnPTPConfigChangeE810(data *interface{}, nodeProfile *ptpv1.PtpProfile) err
 					(*nodeProfile).PtpSettings[dpllClockIdStr] = strconv.FormatUint(getClockIdE810(device), 10)
 					for pin, value := range pins {
 						deviceDir := fmt.Sprintf("/sys/class/net/%s/device/ptp/", device)
-						phcs, err := os.ReadDir(deviceDir)
-						if err != nil {
-							glog.Error("e810 failed to read " + deviceDir + ": " + err.Error())
+						phcs, err2 := os.ReadDir(deviceDir)
+						if err2 != nil {
+							glog.Error("e810 failed to read " + deviceDir + ": " + err2.Error())
 							continue
 						}
 
@@ -134,22 +135,22 @@ func OnPTPConfigChangeE810(data *interface{}, nodeProfile *ptpv1.PtpProfile) err
 								pinPath = fmt.Sprintf("/sys/class/net/%s/device/ptp/%s/pins/%s", device, phc.Name(), pin)
 							}
 							glog.Infof("echo %s > %s", value, pinPath)
-							err = os.WriteFile(pinPath, []byte(value), 0666)
-							if err != nil {
-								glog.Error("e810 failed to write " + value + " to " + pinPath + ": " + err.Error())
+							err2 = os.WriteFile(pinPath, []byte(value), 0666)
+							if err2 != nil {
+								glog.Error("e810 failed to write " + value + " to " + pinPath + ": " + err2.Error())
 							}
 						}
 					}
 				}
 			}
 
-			comps, err := findDelayCompensation(e810Opts, nodeProfile)
-			if err != nil {
-				glog.Errorf("fail to get delay compensations, %s", err)
+			comps, err2 := findDelayCompensation(e810Opts, nodeProfile)
+			if err2 != nil {
+				glog.Errorf("fail to get delay compensations, %s", err2)
 			}
-			err = sendDelayCompensation(comps)
-			if err != nil {
-				glog.Errorf("fail to send delay compensations, %s", err)
+			err2 = sendDelayCompensation(comps)
+			if err2 != nil {
+				glog.Errorf("fail to send delay compensations, %s", err2)
 			}
 
 			for k, v := range e810Opts.DpllSettings {
@@ -190,7 +191,7 @@ func AfterRunPTPCommandE810(data *interface{}, nodeProfile *ptpv1.PtpProfile, co
 	e810Opts.EnableDefaultConfig = false
 
 	for name, opts := range (*nodeProfile).Plugins {
-		if name == "e810" {
+		if name == hwplugin.E810 {
 			optsByteArray, _ = json.Marshal(opts)
 			err = json.Unmarshal(optsByteArray, &e810Opts)
 			if err != nil {
@@ -201,17 +202,19 @@ func AfterRunPTPCommandE810(data *interface{}, nodeProfile *ptpv1.PtpProfile, co
 				for _, ublxOpt := range append(e810Opts.UblxCmds, getDefaultUblxCmds()...) {
 					ublxArgs := ublxOpt.Args
 					glog.Infof("Running /usr/bin/ubxtool with args %s", strings.Join(ublxArgs, ", "))
-					stdout, err = exec.Command("/usr/local/bin/ubxtool", ublxArgs...).CombinedOutput()
-					//stdout, err = exec.Command("/usr/local/bin/ubxtool", "-p", "STATUS").CombinedOutput()
-					_data := *data
-					if data != nil && ublxOpt.ReportOutput {
-						glog.Infof("Saving status to hwconfig: %s", string(stdout))
-						var pluginData *E810PluginData = _data.(*E810PluginData)
-						_pluginData := *pluginData
-						statusString := fmt.Sprintf("ublx data: %s", string(stdout))
-						*_pluginData.hwplugins = append(*_pluginData.hwplugins, statusString)
+					if stdout, err = exec.Command("/usr/local/bin/ubxtool", ublxArgs...).CombinedOutput(); err == nil {
+						if data != nil && ublxOpt.ReportOutput {
+							_data := *data
+							glog.Infof("Saving status to hwconfig: %s", string(stdout))
+							var pluginData *E810PluginData = _data.(*E810PluginData)
+							_pluginData := *pluginData
+							statusString := fmt.Sprintf("ublx data: %s", string(stdout))
+							*_pluginData.hwplugins = append(*_pluginData.hwplugins, statusString)
+						} else {
+							glog.Infof("Not saving status to hwconfig: %s", string(stdout))
+						}
 					} else {
-						glog.Infof("Not saving status to hwconfig: %s", string(stdout))
+						glog.Errorf("Failed to run ubxtool with args %s: %s", strings.Join(ublxArgs, ", "), err)
 					}
 				}
 			} else {
@@ -243,14 +246,14 @@ func PopulateHwConfigE810(data *interface{}, hwconfigs *[]ptpv1.HwConfig) error
 }
 
 func E810(name string) (*plugin.Plugin, *interface{}) {
-	if name != "e810" {
+	if name != hwplugin.E810 {
 		glog.Errorf("Plugin must be initialized as 'e810'")
 		return nil, nil
 	}
-	glog.Infof("registering e810 plugin")
+	glog.Infof("registering all hw plugins")
 	hwplugins := []string{}
 	pluginData := E810PluginData{hwplugins: &hwplugins}
-	_plugin := plugin.Plugin{Name: "e810",
+	_plugin := plugin.Plugin{Name: hwplugin.E810,
 		OnPTPConfigChange:  OnPTPConfigChangeE810,
 		AfterRunPTPCommand: AfterRunPTPCommandE810,
 		PopulateHwConfig:   PopulateHwConfigE810,

addons/intel/phaseAdjust.go

@@ -106,7 +106,6 @@ func sendDelayCompensation(comp *[]delayCompensation) error {
 		return fmt.Errorf("failed to dump DPLL pins: %v", err)
 	}
 	for _, pin := range pinReplies {
-
 		for _, dc := range *comp {
 			desiredClockId, err := strconv.ParseUint(dc.clockId, 10, 64)
 			if err != nil {
@@ -145,9 +144,9 @@ func findDelayCompensation(e810Opts E810Opts, nodeProfile *ptpv1.PtpProfile) (*[
 
 			pinLabel = link.Pin
 			internalDelay = link.DelayPs
-			clockId, err := addClockId(card.Id, nodeProfile)
-			if err != nil {
-				return nil, err
+			clockId, err2 := addClockId(card.Id, nodeProfile)
+			if err2 != nil {
+				return nil, err2
 			}
 			compensations = append(compensations, delayCompensation{
 				DelayPs:   int32(externalDelay) + internalDelay,
@@ -159,12 +158,12 @@ func findDelayCompensation(e810Opts E810Opts, nodeProfile *ptpv1.PtpProfile) (*[
 		}
 		if card.GnssInput {
 			gnssLink := &delays.GnssInput
-			if gnssLink == nil {
+			if gnssLink.Connector == "" && gnssLink.Pin == "" && gnssLink.DelayPs == 0 && gnssLink.DelayVariationPs == 0 {
 				return nil, fmt.Errorf("plugin E810 error: can't identify GNSS link in the %s data", card.Part)
 			}
-			clockId, err := addClockId(card.Id, nodeProfile)
-			if err != nil {
-				return nil, err
+			clockId, err2 := addClockId(card.Id, nodeProfile)
+			if err2 != nil {
+				return nil, err2
 			}
 			compensations = append(compensations, delayCompensation{
 				DelayPs:   gnssLink.DelayPs,
@@ -179,9 +178,9 @@ func findDelayCompensation(e810Opts E810Opts, nodeProfile *ptpv1.PtpProfile) (*[
 			if link == nil {
 				return nil, fmt.Errorf("plugin E810 error: can't find connector %s in the card %s spec", outputConn, card.Part)
 			}
-			clockId, err := addClockId(card.Id, nodeProfile)
-			if err != nil {
-				return nil, err
+			clockId, err2 := addClockId(card.Id, nodeProfile)
+			if err2 != nil {
+				return nil, err2
 			}
 			compensations = append(compensations, delayCompensation{
 				DelayPs:   link.DelayPs,
@@ -290,7 +289,7 @@ func parseVpdBlock(block []byte) *map[string]string {
 // matching to the correct internal delay profile
 // Currently the fingerprint is extracted from the "Vendor Information V1"
 // in the hardware Vital Product Data (VPD). With more cards with different
-// delay profiles are avaliable, this function might need to change depending on
+// delay profiles are available, this function might need to change depending on
 // how manufacturers expose data relevant for delay profiles in the VPD file
 func GetHardwareFingerprint(device string) string {
 	b, err := os.ReadFile(fmt.Sprintf("/sys/class/net/%s/device/vpd", device))

addons/mapping.go

@@ -3,10 +3,12 @@ package mapping
 import (
 	"github.com/openshift/linuxptp-daemon/addons/generic"
 	"github.com/openshift/linuxptp-daemon/addons/intel"
+	plugin2 "github.com/openshift/linuxptp-daemon/addons/plugin"
 	"github.com/openshift/linuxptp-daemon/pkg/plugin"
 )
 
+// PluginMapping is a map of plugin names to their respective constructors
 var PluginMapping = map[string]plugin.New{
-	"reference": generic.Reference,
-	"e810":      intel.E810,
+	"reference":  generic.Reference,
+	plugin2.E810: intel.E810,
 }

addons/plugin/naming.go

@@ -0,0 +1,8 @@
+package plugin
+
+const (
+	// E810 is the name of the Intel E810 plugin
+	E810 = "e810"
+	// Gen is the name of the generic plugin
+	Gen = "generic"
+)

cmd/main.go

@@ -80,7 +80,7 @@ func main() {
 	// The name of NodePtpDevice CR for this node is equal to the node name
 	var stdoutToSocket = false
 	if val, ok := os.LookupEnv("LOGS_TO_SOCKET"); ok && val != "" {
-		if ret, err := strconv.ParseBool(val); err == nil {
+		if ret, err2 := strconv.ParseBool(val); err2 == nil {
 			stdoutToSocket = ret
 		}
 	}
@@ -154,12 +154,12 @@ func main() {
 			}
 
 			nodeProfile := filepath.Join(cp.profileDir, nodeName)
-			if _, err := os.Stat(nodeProfile); err != nil {
-				if os.IsNotExist(err) {
+			if _, err2 := os.Stat(nodeProfile); err2 != nil {
+				if os.IsNotExist(err2) {
 					glog.Infof("ptp profile doesn't exist for node: %v", nodeName)
 					continue
 				} else {
-					glog.Errorf("error stating node profile %v: %v", nodeName, err)
+					glog.Errorf("error stating node profile %v: %v", nodeName, err2)
 					continue
 				}
 			}

pkg/config/config.go

@@ -94,8 +94,8 @@ func GetKubeConfig() (*rest.Config, error) {
 	}
 	// If no in-cluster config, try the default location in the user's home directory
 	if usr, err := user.Current(); err == nil {
-		kubeConfig := filepath.Join(usr.HomeDir, ".kube", "config")
-		return configFromFlags(kubeConfig)
+		config := filepath.Join(usr.HomeDir, ".kube", "config")
+		return configFromFlags(config)
 	}
 
 	return nil, fmt.Errorf("could not locate a kubeconfig")