From f5a75b1430d21659dc55a0086d8fecdcc176fbf7 Mon Sep 17 00:00:00 2001 From: Vitaly Grinberg Date: Mon, 30 Mar 2026 17:05:11 +0300 Subject: [PATCH 1/9] Reject phase offsets of inactive input pins The DPLL FFO feature introduced additional pin reports. These additional reports are generated because the pin FFO changed, but the phase offset is indicated as 0. Such reports can be misinterpreted and acted upon in the clock state decision function. Ignore DPLL pin replies of inactive pins and exclude them from the phase offset decision chain. Only allow pin replays matched by clock ID and parent device of type "PPS" in the connected state. Signed-off-by: Vitaly Grinberg --- pkg/dpll/dpll.go | 40 ++++---- pkg/dpll/dpll_internal_test.go | 172 +++++++++++++++++++++++++++++++++ 2 files changed, 193 insertions(+), 19 deletions(-) create mode 100644 pkg/dpll/dpll_internal_test.go diff --git a/pkg/dpll/dpll.go b/pkg/dpll/dpll.go index b6f6a56e..072ee225 100644 --- a/pkg/dpll/dpll.go +++ b/pkg/dpll/dpll.go @@ -127,6 +127,9 @@ type DpllConfig struct { phaseOffsetPinFilter map[string]map[string]string inSyncConditionThreshold uint64 inSyncConditionTimes uint64 + + // devices holds the cache of DPLL device replies + devices []*nl.DoDeviceGetReply } func (d *DpllConfig) InSpec() bool { @@ -341,26 +344,23 @@ func (d *DpllConfig) Timer() int64 { return d.timer } -func (d *DpllConfig) PhaseOffsetPin(pin *nl.PinInfo) bool { - - if pin.ClockID == d.clockId && len(pin.ParentDevice) > PPS_PIN_INDEX && pin.ParentDevice[PPS_PIN_INDEX].PhaseOffset != math.MaxInt64 { - for k, v := range d.phaseOffsetPinFilter[strconv.FormatUint(d.clockId, 10)] { - switch k { - case "boardLabel": - if strings.Compare(pin.BoardLabel, v) != 0 { - return false - } - case "panelLabel": - if strings.Compare(pin.PanelLabel, v) != 0 { - return false - } - default: - glog.Warningf("unsupported phase offset pin filter key: %s", k) +// ActivePhaseOffsetPin checks whether the given pin is actively connected +// and feeds the relevant PPS DPLL matched by clock ID +func (d *DpllConfig) ActivePhaseOffsetPin(pin *nl.PinInfo) (int, bool) { + if pin.ClockID != d.clockId { + return -1, false + } + for i, p := range pin.ParentDevice { + if p.State != nl.PinStateConnected { + continue + } + for _, dev := range d.devices { + if dev.ID == p.ParentID && dev.ClockID == d.clockId && nl.GetDpllType(dev.Type) == "pps" { + return i, true } } - return true } - return false + return -1, false } // nlUpdateState updates DPLL state in the DpllConfig structure. @@ -386,8 +386,8 @@ func (d *DpllConfig) nlUpdateState(devices []*nl.DoDeviceGetReply, pins []*nl.Pi } } for _, pin := range pins { - if d.PhaseOffsetPin(pin) { - d.SetPhaseOffset(pin.ParentDevice[PPS_PIN_INDEX].PhaseOffset) + if index, ok := d.ActivePhaseOffsetPin(pin); ok { + d.SetPhaseOffset(pin.ParentDevice[index].PhaseOffset) glog.Info("setting phase offset to ", d.phaseOffset, " ns for clock id ", d.clockId, " iface ", d.iface) valid = true } @@ -506,6 +506,8 @@ func (d *DpllConfig) MonitorDpllNetlink() { goto abort } + d.devices = replies + if d.nlUpdateState(replies, []*nl.PinInfo{}) { d.stateDecision() } diff --git a/pkg/dpll/dpll_internal_test.go b/pkg/dpll/dpll_internal_test.go new file mode 100644 index 00000000..58f54886 --- /dev/null +++ b/pkg/dpll/dpll_internal_test.go @@ -0,0 +1,172 @@ +package dpll + +import ( + "testing" + + nl "github.com/k8snetworkplumbingwg/linuxptp-daemon/pkg/dpll-netlink" + "github.com/stretchr/testify/assert" +) + +func TestActivePhaseOffsetPin(t *testing.T) { + const ( + testClockID uint64 = 0xAABBCCDD + otherClockID uint64 = 0x11223344 + ppsDeviceID uint32 = 10 + eecDeviceID uint32 = 20 + otherDeviceID uint32 = 30 + ) + + ppsDevice := &nl.DoDeviceGetReply{ + ID: ppsDeviceID, + ClockID: testClockID, + Type: nl.DpllTypePPS, + } + eecDevice := &nl.DoDeviceGetReply{ + ID: eecDeviceID, + ClockID: testClockID, + Type: nl.DpllTypeEEC, + } + otherClockPPS := &nl.DoDeviceGetReply{ + ID: otherDeviceID, + ClockID: otherClockID, + Type: nl.DpllTypePPS, + } + + tests := []struct { + name string + clockID uint64 + devices []*nl.DoDeviceGetReply + pin *nl.PinInfo + expectedIndex int + expectedOk bool + }{ + { + name: "pin clock ID mismatch", + clockID: testClockID, + devices: []*nl.DoDeviceGetReply{ppsDevice}, + pin: &nl.PinInfo{ + ClockID: otherClockID, + ParentDevice: []nl.PinParentDevice{ + {ParentID: ppsDeviceID, State: nl.PinStateConnected}, + }, + }, + expectedIndex: -1, + expectedOk: false, + }, + { + name: "connected to PPS device with matching clock", + clockID: testClockID, + devices: []*nl.DoDeviceGetReply{ppsDevice, eecDevice}, + pin: &nl.PinInfo{ + ClockID: testClockID, + ParentDevice: []nl.PinParentDevice{ + {ParentID: ppsDeviceID, State: nl.PinStateConnected}, + }, + }, + expectedIndex: 0, + expectedOk: true, + }, + { + name: "disconnected from PPS device", + clockID: testClockID, + devices: []*nl.DoDeviceGetReply{ppsDevice}, + pin: &nl.PinInfo{ + ClockID: testClockID, + ParentDevice: []nl.PinParentDevice{ + {ParentID: ppsDeviceID, State: nl.PinStateDisconnected}, + }, + }, + expectedIndex: -1, + expectedOk: false, + }, + { + name: "connected to EEC device only", + clockID: testClockID, + devices: []*nl.DoDeviceGetReply{eecDevice}, + pin: &nl.PinInfo{ + ClockID: testClockID, + ParentDevice: []nl.PinParentDevice{ + {ParentID: eecDeviceID, State: nl.PinStateConnected}, + }, + }, + expectedIndex: -1, + expectedOk: false, + }, + { + name: "connected to PPS device but different clock ID in device", + clockID: testClockID, + devices: []*nl.DoDeviceGetReply{otherClockPPS}, + pin: &nl.PinInfo{ + ClockID: testClockID, + ParentDevice: []nl.PinParentDevice{ + {ParentID: otherDeviceID, State: nl.PinStateConnected}, + }, + }, + expectedIndex: -1, + expectedOk: false, + }, + { + name: "multiple parents, second is connected PPS", + clockID: testClockID, + devices: []*nl.DoDeviceGetReply{ppsDevice, eecDevice}, + pin: &nl.PinInfo{ + ClockID: testClockID, + ParentDevice: []nl.PinParentDevice{ + {ParentID: eecDeviceID, State: nl.PinStateConnected}, + {ParentID: ppsDeviceID, State: nl.PinStateConnected}, + }, + }, + expectedIndex: 1, + expectedOk: true, + }, + { + name: "selectable state is not connected", + clockID: testClockID, + devices: []*nl.DoDeviceGetReply{ppsDevice}, + pin: &nl.PinInfo{ + ClockID: testClockID, + ParentDevice: []nl.PinParentDevice{ + {ParentID: ppsDeviceID, State: nl.PinStateSelectable}, + }, + }, + expectedIndex: -1, + expectedOk: false, + }, + { + name: "no parent devices", + clockID: testClockID, + devices: []*nl.DoDeviceGetReply{ppsDevice}, + pin: &nl.PinInfo{ + ClockID: testClockID, + ParentDevice: []nl.PinParentDevice{}, + }, + expectedIndex: -1, + expectedOk: false, + }, + { + name: "no cached devices", + clockID: testClockID, + devices: nil, + pin: &nl.PinInfo{ + ClockID: testClockID, + ParentDevice: []nl.PinParentDevice{ + {ParentID: ppsDeviceID, State: nl.PinStateConnected}, + }, + }, + expectedIndex: -1, + expectedOk: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + d := &DpllConfig{ + clockId: tt.clockID, + devices: tt.devices, + } + index, ok := d.ActivePhaseOffsetPin(tt.pin) + assert.Equal(t, tt.expectedIndex, index, "device index") + assert.Equal(t, tt.expectedOk, ok, "match result") + }) + } +} From cebbf7128bd1ac2f0ac2a2e511b3cbf0ecd5ec2c Mon Sep 17 00:00:00 2001 From: Vitaly Grinberg Date: Mon, 9 Mar 2026 22:44:28 +0200 Subject: [PATCH 2/9] Ignore phase offsets from non-input pins Signed-off-by: Vitaly Grinberg --- pkg/dpll/dpll.go | 2 +- pkg/dpll/dpll_internal_test.go | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/dpll/dpll.go b/pkg/dpll/dpll.go index 072ee225..a8b42da0 100644 --- a/pkg/dpll/dpll.go +++ b/pkg/dpll/dpll.go @@ -351,7 +351,7 @@ func (d *DpllConfig) ActivePhaseOffsetPin(pin *nl.PinInfo) (int, bool) { return -1, false } for i, p := range pin.ParentDevice { - if p.State != nl.PinStateConnected { + if p.State != nl.PinStateConnected || p.Direction != nl.PinDirectionInput { continue } for _, dev := range d.devices { diff --git a/pkg/dpll/dpll_internal_test.go b/pkg/dpll/dpll_internal_test.go index 58f54886..319256d8 100644 --- a/pkg/dpll/dpll_internal_test.go +++ b/pkg/dpll/dpll_internal_test.go @@ -60,7 +60,7 @@ func TestActivePhaseOffsetPin(t *testing.T) { pin: &nl.PinInfo{ ClockID: testClockID, ParentDevice: []nl.PinParentDevice{ - {ParentID: ppsDeviceID, State: nl.PinStateConnected}, + {ParentID: ppsDeviceID, State: nl.PinStateConnected, Direction: nl.PinDirectionInput}, }, }, expectedIndex: 0, @@ -112,8 +112,8 @@ func TestActivePhaseOffsetPin(t *testing.T) { pin: &nl.PinInfo{ ClockID: testClockID, ParentDevice: []nl.PinParentDevice{ - {ParentID: eecDeviceID, State: nl.PinStateConnected}, - {ParentID: ppsDeviceID, State: nl.PinStateConnected}, + {ParentID: eecDeviceID, State: nl.PinStateConnected, Direction: nl.PinDirectionInput}, + {ParentID: ppsDeviceID, State: nl.PinStateConnected, Direction: nl.PinDirectionInput}, }, }, expectedIndex: 1, From f9d57d0680f8265c9a02d5f3862a7daf0d25dfbf Mon Sep 17 00:00:00 2001 From: Vitaly Grinberg Date: Tue, 31 Mar 2026 11:32:18 +0300 Subject: [PATCH 3/9] Create DPLLPins abstraction and use them as fall back for SMA/U.FL pins Assisted-By: cursor Signed-off-by: Vitaly Grinberg --- addons/intel/clock-chain.go | 373 +++++++------- addons/intel/clock-chain_test.go | 195 ++++++++ addons/intel/clockID.go | 122 +++++ addons/intel/clockID_test.go | 144 ++++++ addons/intel/common.go | 93 ++++ addons/intel/dpllPins.go | 143 ++++++ addons/intel/dpllPins_test.go | 463 ++++++++++++++++++ addons/intel/e810.go | 284 +++++------ addons/intel/e810_test.go | 418 +++++++++++++++- addons/intel/e825.go | 148 +----- addons/intel/e825_test.go | 24 +- addons/intel/e830.go | 77 +-- addons/intel/e830_test.go | 56 ++- addons/intel/intel_test.go | 433 ---------------- addons/intel/mock.go | 12 - addons/intel/mock_test.go | 391 +++++++++++++++ addons/intel/phaseAdjust.go | 43 +- addons/intel/pinConfig.go | 11 + addons/intel/pinConfig_test.go | 27 + addons/intel/testdata/profile-t-tsc.yaml | 1 - .../testdata/profile-tbc-no-input-delays.yaml | 1 - addons/intel/testdata/profile-tbc.yaml | 1 - addons/intel/testdata/profile-tgm-old.yaml | 1 - addons/intel/testdata/profile-tgm.yaml | 1 - pkg/daemon/daemon.go | 2 +- pkg/dpll-netlink/dpll-uapi.go | 2 +- pkg/dpll-netlink/dpll.go | 5 +- pkg/dpll/dpll.go | 172 +++++-- pkg/dpll/dpll_test.go | 6 +- 29 files changed, 2584 insertions(+), 1065 deletions(-) create mode 100644 addons/intel/clock-chain_test.go create mode 100644 addons/intel/clockID.go create mode 100644 addons/intel/clockID_test.go create mode 100644 addons/intel/common.go create mode 100644 addons/intel/dpllPins.go create mode 100644 addons/intel/dpllPins_test.go delete mode 100644 addons/intel/intel_test.go delete mode 100644 addons/intel/mock.go create mode 100644 addons/intel/mock_test.go diff --git a/addons/intel/clock-chain.go b/addons/intel/clock-chain.go index 1a6e1e25..7c9b4ba4 100644 --- a/addons/intel/clock-chain.go +++ b/addons/intel/clock-chain.go @@ -3,9 +3,6 @@ package intel import ( "errors" "fmt" - "os" - "slices" - "strconv" "time" "github.com/golang/glog" @@ -14,54 +11,49 @@ import ( ptpv1 "github.com/k8snetworkplumbingwg/ptp-operator/api/v1" ) -// FileSystemInterface defines the interface for filesystem operations to enable mocking -type FileSystemInterface interface { - ReadDir(dirname string) ([]os.DirEntry, error) - WriteFile(filename string, data []byte, perm os.FileMode) error -} - -// RealFileSystem implements FileSystemInterface using real OS operations -type RealFileSystem struct{} - -// ReadDir reads the contents of the directory specified by dirname -func (fs *RealFileSystem) ReadDir(dirname string) ([]os.DirEntry, error) { - return os.ReadDir(dirname) -} +type ( + // ClockChainType represents the type of clock chain + ClockChainType int -// WriteFile writes the data to the file specified by filename -func (fs *RealFileSystem) WriteFile(filename string, data []byte, perm os.FileMode) error { - return os.WriteFile(filename, data, perm) -} + // ClockChain represents a set of interrelated clocks + ClockChain struct { + Type ClockChainType `json:"clockChainType"` + LeadingNIC CardInfo `json:"LeadingNIC"` + OtherNICs []CardInfo `json:"otherNICs,omitempty"` + DpllPins DPLLPins `json:"dpllPins"` + } -// Default filesystem implementation -var filesystem FileSystemInterface = &RealFileSystem{} + // ClockChainInterface is the mockable public interface of the ClockChain + ClockChainInterface interface { + EnterNormalTBC() error + EnterHoldoverTBC() error + SetPinDefaults() error + GetLeadingNIC() CardInfo + } -type ClockChainType int -type ClockChain struct { - Type ClockChainType `json:"clockChainType"` - LeadingNIC CardInfo `json:"leadingNIC"` - OtherNICs []CardInfo `json:"otherNICs,omitempty"` - DpllPins []*dpll.PinInfo `json:"dpllPins"` -} -type CardInfo struct { - Name string `json:"name"` - DpllClockID string `json:"dpllClockId"` - // upstreamPort specifies the slave port in the T-BC case. For example, if the "name" - // is ens4f0, the "upstreamPort" could be ens4f1, depending on ptp4l config - UpstreamPort string `json:"upstreamPort"` - Pins map[string]dpll.PinInfo `json:"pins"` -} + // CardInfo represents an individual card in the clock chain + CardInfo struct { + Name string `json:"name"` + DpllClockID uint64 `json:"dpllClockId"` + // upstreamPort specifies the slave port in the T-BC case. For example, if the "name" + // is ens4f0, the "upstreamPort" could be ens4f1, depending on ptp4l config + UpstreamPort string `json:"upstreamPort"` + // Pins map[string]dpll.PinInfo `json:"pins"` + } -// PhaseInputsProvider abstracts access to PhaseInputs so InitClockChain can -// accept different option structs (e.g., E810Opts, E825Opts) -type PhaseInputsProvider interface { - GetPhaseInputs() []PhaseInputs -} + // PhaseInputsProvider abstracts access to PhaseInputs so InitClockChain can + // accept different option structs (e.g., E810Opts, E825Opts) + PhaseInputsProvider interface { + GetPhaseInputs() []PhaseInputs + } +) const ( ClockTypeUnset ClockChainType = iota ClockTypeTGM ClockTypeTBC +) +const ( PriorityEnabled = 0 PriorityDisabled = 255 ) @@ -94,41 +86,29 @@ type PinParentControl struct { PpsPriority uint8 EecOutputState uint8 PpsOutputState uint8 + EecDirection *uint8 + PpsDirection *uint8 } type PinControl struct { Label string ParentControl PinParentControl } -var configurablePins = []string{sdp20, sdp21, sdp22, sdp23, gnss, sma1Input, sma2Input, c8270Rclka, c8270Rclkb} - -func (c *ClockChain) getLiveDpllPinsInfo() error { - if !unitTest { - conn, err := dpll.Dial(nil) - if err != nil { - return fmt.Errorf("failed to dial DPLL: %v", err) - } - //nolint:errcheck - defer conn.Close() - c.DpllPins, err = conn.DumpPinGet() - if err != nil { - return fmt.Errorf("failed to dump DPLL pins: %v", err) - } - } else { - c.DpllPins = DpllPins - } - return nil +// GetLeadingNIC returns the leading NIC from the clock chain +func (c *ClockChain) GetLeadingNIC() CardInfo { + return c.LeadingNIC } func (c *ClockChain) resolveInterconnections(opts PhaseInputsProvider, nodeProfile *ptpv1.PtpProfile) (*[]delayCompensation, error) { compensations := []delayCompensation{} - var clockID *string for _, card := range opts.GetPhaseInputs() { delays, err := InitInternalDelays(card.Part) if err != nil { return nil, err } - glog.Infof("card: %%v", card) + glog.Infof("card: %+v", card) + var clockID uint64 + if !card.GnssInput && card.UpstreamPort == "" { externalDelay := card.Input.DelayPs connector := card.Input.Connector @@ -145,29 +125,26 @@ func (c *ClockChain) resolveInterconnections(opts PhaseInputsProvider, nodeProfi if err != nil { return nil, err } - compensations = append(compensations, delayCompensation{ DelayPs: int32(externalDelay) + internalDelay, pinLabel: pinLabel, iface: card.ID, direction: "input", - clockID: *clockID, + clockID: clockID, }) // Track non-leading NICs c.OtherNICs = append(c.OtherNICs, CardInfo{ Name: card.ID, - DpllClockID: *clockID, + DpllClockID: clockID, UpstreamPort: card.UpstreamPort, - Pins: make(map[string]dpll.PinInfo, 0), }) } else { c.LeadingNIC.Name = card.ID c.LeadingNIC.UpstreamPort = card.UpstreamPort - clockID, err = addClockID(card.ID, nodeProfile) + c.LeadingNIC.DpllClockID, err = addClockID(card.ID, nodeProfile) if err != nil { return nil, err } - c.LeadingNIC.DpllClockID = *clockID if card.GnssInput { c.Type = ClockTypeTGM gnssLink := &delays.GnssInput @@ -176,7 +153,7 @@ func (c *ClockChain) resolveInterconnections(opts PhaseInputsProvider, nodeProfi pinLabel: gnssLink.Pin, iface: card.ID, direction: "input", - clockID: *clockID, + clockID: c.LeadingNIC.DpllClockID, }) } else { // if no GNSS and no external, then ptp4l input @@ -197,7 +174,7 @@ func (c *ClockChain) resolveInterconnections(opts PhaseInputsProvider, nodeProfi pinLabel: link.Pin, iface: card.ID, direction: "output", - clockID: *clockID, + clockID: clockID, }) } } @@ -206,14 +183,13 @@ func (c *ClockChain) resolveInterconnections(opts PhaseInputsProvider, nodeProfi // InitClockChain initializes the ClockChain struct based on live DPLL pin info func InitClockChain(opts PhaseInputsProvider, nodeProfile *ptpv1.PtpProfile) (*ClockChain, error) { - var chain = &ClockChain{ - LeadingNIC: CardInfo{ - Pins: make(map[string]dpll.PinInfo, 0), - }, - OtherNICs: make([]CardInfo, 0), + chain := &ClockChain{ + LeadingNIC: CardInfo{}, + OtherNICs: make([]CardInfo, 0), + DpllPins: DpllPins, } - err := chain.getLiveDpllPinsInfo() + err := chain.DpllPins.FetchPins() if err != nil { return chain, err } @@ -222,35 +198,24 @@ func InitClockChain(opts PhaseInputsProvider, nodeProfile *ptpv1.PtpProfile) (*C if err != nil { glog.Errorf("fail to get delay compensations, %s", err) } - if !unitTest { - err = sendDelayCompensation(comps, chain.DpllPins) - if err != nil { - glog.Errorf("fail to send delay compensations, %s", err) - } - } - err = chain.getLeadingCardSDP() + err = SendDelayCompensation(comps, chain.DpllPins) if err != nil { - return chain, err - } - // Populate pins for other NICs, if any - err = chain.getOtherCardsSDP() - if err != nil { - return chain, err + glog.Errorf("fail to send delay compensations, %s", err) } glog.Info("about to set DPLL pin priorities to defaults") - _, err = chain.SetPinDefaults() + err = chain.SetPinDefaults() if err != nil { return chain, err } if chain.Type == ClockTypeTBC { (*nodeProfile).PtpSettings["clockType"] = "T-BC" glog.Info("about to init TBC pins") - _, err = chain.InitPinsTBC() + err = chain.InitPinsTBC() if err != nil { return chain, fmt.Errorf("failed to initialize pins for T-BC operation: %s", err.Error()) } glog.Info("about to enter TBC Normal mode") - _, err = chain.EnterNormalTBC() + err = chain.EnterNormalTBC() if err != nil { return chain, fmt.Errorf("failed to enter T-BC normal mode: %s", err.Error()) } @@ -258,44 +223,9 @@ func InitClockChain(opts PhaseInputsProvider, nodeProfile *ptpv1.PtpProfile) (*C return chain, err } -func (c *ClockChain) getLeadingCardSDP() error { - clockID, err := strconv.ParseUint(c.LeadingNIC.DpllClockID, 10, 64) - if err != nil { - return err - } - for _, pin := range c.DpllPins { - if pin.ClockID == clockID && slices.Contains(configurablePins, pin.BoardLabel) { - c.LeadingNIC.Pins[pin.BoardLabel] = *pin - } - } - return nil -} - -// getOtherCardsSDP populates configurable pins for all non-leading NICs -func (c *ClockChain) getOtherCardsSDP() error { - if len(c.OtherNICs) == 0 { - return nil - } - for idx := range c.OtherNICs { - clockID, err := strconv.ParseUint(c.OtherNICs[idx].DpllClockID, 10, 64) - if err != nil { - return err - } - for _, pin := range c.DpllPins { - if pin.ClockID == clockID && slices.Contains(configurablePins, pin.BoardLabel) { - if c.OtherNICs[idx].Pins == nil { - c.OtherNICs[idx].Pins = make(map[string]dpll.PinInfo, 0) - } - c.OtherNICs[idx].Pins[pin.BoardLabel] = *pin - } - } - } - return nil -} - func writeSysFs(path string, val string) error { glog.Infof("writing " + val + " to " + path) - err := filesystem.WriteFile(path, []byte(val), 0666) + err := filesystem.WriteFile(path, []byte(val), 0o666) if err != nil { return fmt.Errorf("e810 failed to write " + val + " to " + path + ": " + err.Error()) } @@ -305,13 +235,12 @@ func writeSysFs(path string, val string) error { func (c *ClockChain) SetPinsControl(pins []PinControl) (*[]dpll.PinParentDeviceCtl, error) { pinCommands := []dpll.PinParentDeviceCtl{} for _, pinCtl := range pins { - dpllPin, found := c.LeadingNIC.Pins[pinCtl.Label] - if !found { - glog.Errorf("%s pin not found in the card", pinCtl.Label) + dpllPin := c.DpllPins.GetByLabel(pinCtl.Label, c.LeadingNIC.DpllClockID) + if dpllPin == nil { + glog.Errorf("pin not found with label %s for clockID %d", pinCtl.Label, c.LeadingNIC.DpllClockID) continue } - pinCommand := SetPinControlData(dpllPin, pinCtl.ParentControl) - pinCommands = append(pinCommands, *pinCommand) + pinCommands = append(pinCommands, SetPinControlData(*dpllPin, pinCtl.ParentControl)...) } return &pinCommands, nil } @@ -320,47 +249,68 @@ func (c *ClockChain) SetPinsControl(pins []PinControl) (*[]dpll.PinParentDeviceC // This is used specifically for initialization functions like SetPinDefaults func (c *ClockChain) SetPinsControlForAllNICs(pins []PinControl) (*[]dpll.PinParentDeviceCtl, error) { pinCommands := []dpll.PinParentDeviceCtl{} + errs := make([]error, 0) for _, pinCtl := range pins { - // Search for the pin across all NICs - found := false - - // Check leading NIC first - if pin, exists := c.LeadingNIC.Pins[pinCtl.Label]; exists { - pinCommand := SetPinControlData(pin, pinCtl.ParentControl) - pinCommands = append(pinCommands, *pinCommand) - found = true + foundPins := c.DpllPins.GetAllPinsByLabel(pinCtl.Label) + if len(foundPins) == 0 && pinCtl.Label == sma2Input { + pinCtl.Label = sma2 + foundPins = c.DpllPins.GetAllPinsByLabel(pinCtl.Label) } - - // Check all other NICs - for _, nic := range c.OtherNICs { - if pin, exists := nic.Pins[pinCtl.Label]; exists { - pinCommand := SetPinControlData(pin, pinCtl.ParentControl) - pinCommands = append(pinCommands, *pinCommand) - found = true - } + if len(foundPins) == 0 { + errs = append(errs, fmt.Errorf("pin %s not found on any nic", pinCtl.Label)) + continue } - - if !found { - return nil, fmt.Errorf("%s pin not found in any NIC", pinCtl.Label) + for _, pin := range foundPins { + pinCommands = append(pinCommands, SetPinControlData(*pin, pinCtl.ParentControl)...) } } - return &pinCommands, nil + return &pinCommands, errors.Join(errs...) } -func SetPinControlData(pin dpll.PinInfo, control PinParentControl) *dpll.PinParentDeviceCtl { - Pin := dpll.PinParentDeviceCtl{ - ID: pin.ID, - PinParentCtl: make([]dpll.PinControl, 0), +// buildDirectionCmd checks if any parent device direction is changing. +// If so, it returns a direction-only command and updates pin.ParentDevice +// directions in place. Returns nil if no direction change is needed. +// The kernel rejects combining direction changes with prio/state. +func buildDirectionCmd(pin *dpll.PinInfo, control PinParentControl) *dpll.PinParentDeviceCtl { + var cmd *dpll.PinParentDeviceCtl + + for i, parentDevice := range pin.ParentDevice { + var direction *uint32 + switch i { + case eecDpllIndex: + if control.EecDirection != nil { + v := uint32(*control.EecDirection) + direction = &v + } + case ppsDpllIndex: + if control.PpsDirection != nil { + v := uint32(*control.PpsDirection) + direction = &v + } + } + if direction != nil && *direction != parentDevice.Direction && pin.Capabilities&dpll.PinCapDir != 0 { + if cmd == nil { + cmd = &dpll.PinParentDeviceCtl{ID: pin.ID} + } + cmd.PinParentCtl = append(cmd.PinParentCtl, + dpll.PinControl{PinParentID: parentDevice.ParentID, Direction: direction}) + pin.ParentDevice[i].Direction = *direction + } } + return cmd +} + +// SetPinControlData builds DPLL netlink commands to configure a pin's parent devices. +func SetPinControlData(pin dpll.PinInfo, control PinParentControl) []dpll.PinParentDeviceCtl { + dirCmd := buildDirectionCmd(&pin, control) - for deviceIndex, parentDevice := range pin.ParentDevice { - var prio uint32 - var outputState uint32 - pc := dpll.PinControl{} - pc.PinParentID = parentDevice.ParentID - switch deviceIndex { + cmd := dpll.PinParentDeviceCtl{ID: pin.ID} + for i, parentDevice := range pin.ParentDevice { + var prio, outputState uint32 + + switch i { case eecDpllIndex: prio = uint32(control.EecPriority) outputState = uint32(control.EecOutputState) @@ -368,14 +318,26 @@ func SetPinControlData(pin dpll.PinInfo, control PinParentControl) *dpll.PinPare prio = uint32(control.PpsPriority) outputState = uint32(control.PpsOutputState) } + + pc := dpll.PinControl{PinParentID: parentDevice.ParentID} if parentDevice.Direction == dpll.PinDirectionInput { - pc.Prio = &prio - } else { + if pin.Capabilities&dpll.PinCapState != 0 { + selectable := uint32(dpll.PinStateSelectable) + pc.State = &selectable + } + if parentDevice.Prio != nil && pin.Capabilities&dpll.PinCapPrio != 0 { + pc.Prio = &prio + } + } else if pin.Capabilities&dpll.PinCapState != 0 { pc.State = &outputState } - Pin.PinParentCtl = append(Pin.PinParentCtl, pc) + cmd.PinParentCtl = append(cmd.PinParentCtl, pc) } - return &Pin + + if dirCmd != nil { + return []dpll.PinParentDeviceCtl{*dirCmd, cmd} + } + return []dpll.PinParentDeviceCtl{cmd} } func (c *ClockChain) EnableE810Outputs() error { @@ -398,13 +360,19 @@ func (c *ClockChain) EnableE810Outputs() error { glog.Error(e) return errors.New(e) } - // Enable SMA2 output as a workaround for https://issues.redhat.com/browse/RHEL-110297 - smaPath := fmt.Sprintf("%s%s/pins/%s", deviceDir, phcs[0].Name(), sma2) - err = writeSysFs(smaPath, "2 2") - if err != nil { - glog.Errorf("failed to write 2 2 to %s: %v", smaPath, err) - return err + if hasSysfsSMAPins(c.LeadingNIC.Name) { + err = pinConfig.applyPinSet(c.LeadingNIC.Name, pinSet{"SMA2": "2 2"}) + if err != nil { + glog.Errorf("failed to set SMA2 pin via sysfs: %s", err) + } + } else { + sma2Cmds := c.DpllPins.GetCommandsForPluginPinSet(c.LeadingNIC.DpllClockID, map[string]string{"SMA2": "2 2"}) + err = c.DpllPins.ApplyPinCommands(sma2Cmds) + if err != nil { + glog.Errorf("failed to set SMA2 pin to output: %s", err) + } } + pinPath = fmt.Sprintf("%s%s/period", deviceDir, phcs[0].Name()) err = writeSysFs(pinPath, sdp22PpsEnable) if err != nil { @@ -415,12 +383,12 @@ func (c *ClockChain) EnableE810Outputs() error { } // InitPinsTBC initializes the leading card E810 and DPLL pins for T-BC operation -func (c *ClockChain) InitPinsTBC() (*[]dpll.PinParentDeviceCtl, error) { +func (c *ClockChain) InitPinsTBC() error { // Enable 1PPS output on SDP22 // (To synchronize the DPLL1 to the E810 PHC synced by ptp4l): err := c.EnableE810Outputs() if err != nil { - glog.Error("failed to enable E810 outputs: ", err) + return fmt.Errorf("failed to enable E810 outputs: %w", err) } // Disable GNSS-1PPS (all cards), SDP20 and SDP21 commandsGnss, err := c.SetPinsControlForAllNICs([]PinControl{ @@ -470,11 +438,15 @@ func (c *ClockChain) InitPinsTBC() (*[]dpll.PinParentDeviceCtl, error) { glog.Error("failed to set pins control: ", err) } *commands = append(*commands, *commandsGnss...) - return commands, BatchPinSet(commands) + + err = BatchPinSet(commands) + // even if there was an error we still need to refresh the pin state. + fetchErr := c.DpllPins.FetchPins() + return errors.Join(err, fetchErr) } // EnterHoldoverTBC configures the leading card DPLL pins for T-BC holdover -func (c *ClockChain) EnterHoldoverTBC() (*[]dpll.PinParentDeviceCtl, error) { +func (c *ClockChain) EnterHoldoverTBC() error { // Disable DPLL inputs from e810 (SDP22) // Enable DPLL Outputs to e810 (SDP21, SDP23) commands, err := c.SetPinsControl([]PinControl{ @@ -494,13 +466,16 @@ func (c *ClockChain) EnterHoldoverTBC() (*[]dpll.PinParentDeviceCtl, error) { }, }) if err != nil { - return nil, err + return err } - return commands, BatchPinSet(commands) + err = BatchPinSet(commands) + // even if there was an error we still need to refresh the pin state. + fetchErr := c.DpllPins.FetchPins() + return errors.Join(err, fetchErr) } // EnterNormalTBC configures the leading card DPLL pins for regular T-BC operation -func (c *ClockChain) EnterNormalTBC() (*[]dpll.PinParentDeviceCtl, error) { +func (c *ClockChain) EnterNormalTBC() error { // Disable DPLL Outputs to e810 (SDP23, SDP21) // Enable DPLL inputs from e810 (SDP22) commands, err := c.SetPinsControl([]PinControl{ @@ -520,13 +495,16 @@ func (c *ClockChain) EnterNormalTBC() (*[]dpll.PinParentDeviceCtl, error) { }, }) if err != nil { - return nil, err + return err } - return commands, BatchPinSet(commands) + err = BatchPinSet(commands) + // even if there was an error we still need to refresh the pin state. + fetchErr := c.DpllPins.FetchPins() + return errors.Join(err, fetchErr) } // SetPinDefaults initializes DPLL pins to default recommended values -func (c *ClockChain) SetPinDefaults() (*[]dpll.PinParentDeviceCtl, error) { +func (c *ClockChain) SetPinDefaults() error { // DPLL Priority List: // // Recommended | Pin Index | EEC-DPLL0 | PPS-DPLL1 @@ -542,7 +520,7 @@ func (c *ClockChain) SetPinDefaults() (*[]dpll.PinParentDeviceCtl, error) { // 8 | 2 | Recovered CLK1 (C827_0-RCLKA) | Recovered CLK1 (C827_0-RCLKA) // 9 | 3 | Recovered CLK2 (C827_0-RCLKB) | Recovered CLK2 (C827_0-RCLKB) // 10 | -- | OCXO | OCXO - + d := uint8(dpll.PinDirectionInput) // Also, Enable DPLL Outputs to e810 (SDP21, SDP23) commands, err := c.SetPinsControlForAllNICs([]PinControl{ { @@ -555,15 +533,19 @@ func (c *ClockChain) SetPinDefaults() (*[]dpll.PinParentDeviceCtl, error) { { Label: sma1Input, ParentControl: PinParentControl{ - EecPriority: 3, - PpsPriority: 3, + EecPriority: 3, + PpsPriority: 3, + EecDirection: &d, + PpsDirection: &d, }, }, { Label: sma2Input, ParentControl: PinParentControl{ - EecPriority: 2, - PpsPriority: 2, + EecPriority: 2, + PpsPriority: 2, + EecDirection: &d, + PpsDirection: &d, }, }, { @@ -610,15 +592,18 @@ func (c *ClockChain) SetPinDefaults() (*[]dpll.PinParentDeviceCtl, error) { }, }) if err != nil { - return nil, err + return err } - return commands, BatchPinSet(commands) + err = BatchPinSet(commands) + // even if there was an error we still need to refresh the pin state. + fetchErr := c.DpllPins.FetchPins() + return errors.Join(err, fetchErr) } -func BatchPinSet(commands *[]dpll.PinParentDeviceCtl) error { - if unitTest { - return nil - } +// BatchPinSet function pointer allows mocking of BatchPinSet +var BatchPinSet = batchPinSet + +func batchPinSet(commands *[]dpll.PinParentDeviceCtl) error { conn, err := dpll.Dial(nil) if err != nil { return fmt.Errorf("failed to dial DPLL: %v", err) diff --git a/addons/intel/clock-chain_test.go b/addons/intel/clock-chain_test.go new file mode 100644 index 00000000..f37b361e --- /dev/null +++ b/addons/intel/clock-chain_test.go @@ -0,0 +1,195 @@ +package intel + +import ( + "os" + "testing" + + "github.com/stretchr/testify/assert" +) + +func Test_ProcessProfileTbcClockChain(t *testing.T) { + _, restoreDPLLPins := setupMockDPLLPinsFromJSON("./testdata/dpll-pins.json") + defer restoreDPLLPins() + restoreDelay := setupMockDelayCompensation() + defer restoreDelay() + mockPinSet, restorePinSet := setupBatchPinSetMock() + defer restorePinSet() + // Setup filesystem mock for TBC profile (3 devices with pins) + mockFS, restoreFs := setupMockFS() + defer restoreFs() + phcEntries := []os.DirEntry{MockDirEntry{name: "ptp0", isDir: true}} + + // EnableE810Outputs is called for the leading NIC (ens4f0) - needs specific paths + mockFS.AllowReadDir("/sys/class/net/ens4f0/device/ptp/", phcEntries, nil) + mockFS.AllowReadFile("/sys/class/net/ens4f0/device/ptp/ptp0/pins/SMA1", nil, os.ErrNotExist) + mockFS.ExpectWriteFile("/sys/class/net/ens4f0/device/ptp/ptp0/period", []byte("2 0 0 1 0"), os.FileMode(0o666), nil) + + mockPinConfig, restorePins := setupMockPinConfig() + defer restorePins() + + // Can read test profile + profile, err := loadProfile("./testdata/profile-tbc.yaml") + assert.NoError(t, err) + + mockClockIDsFromProfile(mockFS, profile) + + // Can run PTP config change handler without errors + p, d := E810("e810") + err = p.OnPTPConfigChange(d, profile) + assert.NoError(t, err) + ccData := clockChain.(*ClockChain) + assert.Equal(t, ClockTypeTBC, ccData.Type, "identified a wrong clock type") + assert.Equal(t, uint64(5799633565432596414), ccData.LeadingNIC.DpllClockID, "identified a wrong clock ID ") + assert.Equal(t, "ens4f1", ccData.LeadingNIC.UpstreamPort, "wrong upstream port") + assert.Equal(t, 0, mockPinConfig.actualPinSetCount) + assert.Equal(t, 0, mockPinConfig.actualPinFrqCount) + assert.NotNil(t, mockPinSet.commands, "DPLL commands should have been issued") + assert.Greater(t, len(*mockPinSet.commands), 0, "should have DPLL pin commands") + + // Test holdover entry + mockPinSet.reset() + err = clockChain.EnterHoldoverTBC() + assert.NoError(t, err) + assert.Equal(t, 2, len(*mockPinSet.commands)) + + // Test holdover exit + mockPinSet.reset() + err = clockChain.EnterNormalTBC() + assert.NoError(t, err) + assert.Equal(t, 2, len(*mockPinSet.commands)) + + // Ensure switching back to TGM resets any pins + mockPinSet.reset() + tgmProfile, err := loadProfile("./testdata/profile-tgm.yaml") + assert.NoError(t, err) + err = OnPTPConfigChangeE810(nil, tgmProfile) + assert.NoError(t, err) + assert.NotNil(t, mockPinSet.commands, "Ensure clockChain.SetPinDefaults was called") +} + +func Test_ProcessProfileTtscClockChain(t *testing.T) { + _, restoreDPLLPins := setupMockDPLLPinsFromJSON("./testdata/dpll-pins.json") + defer restoreDPLLPins() + restoreDelay := setupMockDelayCompensation() + defer restoreDelay() + mockPinSet, restorePinSet := setupBatchPinSetMock() + defer restorePinSet() + // Setup filesystem mock for T-TSC profile (1 device with pins) + mockFS, restoreFs := setupMockFS() + defer restoreFs() + phcEntries := []os.DirEntry{MockDirEntry{name: "ptp0", isDir: true}} + + // EnableE810Outputs is called for the leading NIC (ens4f0) - needs specific paths + mockFS.AllowReadDir("/sys/class/net/ens4f0/device/ptp/", phcEntries, nil) + mockFS.AllowReadFile("/sys/class/net/ens4f0/device/ptp/ptp0/pins/SMA1", nil, os.ErrNotExist) + mockFS.ExpectWriteFile("/sys/class/net/ens4f0/device/ptp/ptp0/period", []byte("2 0 0 1 0"), os.FileMode(0o666), nil) + + mockPinConfig, restorePins := setupMockPinConfig() + defer restorePins() + + // Can read test profile + profile, err := loadProfile("./testdata/profile-t-tsc.yaml") + assert.NoError(t, err) + + mockClockIDsFromProfile(mockFS, profile) + + // Can run PTP config change handler without errors + p, d := E810("e810") + err = p.OnPTPConfigChange(d, profile) + assert.NoError(t, err) + ccData := clockChain.(*ClockChain) + assert.Equal(t, ClockTypeTBC, ccData.Type, "identified a wrong clock type") + assert.Equal(t, uint64(5799633565432596414), ccData.LeadingNIC.DpllClockID, "identified a wrong clock ID ") + assert.Equal(t, "ens4f1", ccData.LeadingNIC.UpstreamPort, "wrong upstream port") + assert.NotNil(t, mockPinSet.commands, "Ensure some pins were set") + assert.Equal(t, 0, mockPinConfig.actualPinSetCount) + assert.Equal(t, 0, mockPinConfig.actualPinFrqCount) + + // Test holdover entry + mockPinSet.reset() + err = clockChain.EnterHoldoverTBC() + assert.NoError(t, err) + assert.Equal(t, 2, len(*mockPinSet.commands)) + + // Test holdover exit + mockPinSet.reset() + err = clockChain.EnterNormalTBC() + assert.NoError(t, err) + assert.Equal(t, 2, len(*mockPinSet.commands)) +} + +func Test_SetPinDefaults_AllNICs(t *testing.T) { + _, restoreDPLLPins := setupMockDPLLPinsFromJSON("./testdata/dpll-pins.json") + defer restoreDPLLPins() + restoreDelay := setupMockDelayCompensation() + defer restoreDelay() + mockPinSet, restorePinSet := setupBatchPinSetMock() + defer restorePinSet() + + mockPinConfig, restorePinConfig := setupMockPinConfig() + defer restorePinConfig() + + // Setup filesystem mock for EnableE810Outputs + mockFS, restoreFs := setupMockFS() + defer restoreFs() + phcEntries := []os.DirEntry{MockDirEntry{name: "ptp0", isDir: true}} + mockFS.AllowReadDir("/sys/class/net/ens4f0/device/ptp/", phcEntries, nil) + mockFS.AllowReadFile("/sys/class/net/ens4f0/device/ptp/ptp0/pins/SMA1", nil, os.ErrNotExist) + mockFS.ExpectWriteFile("/sys/class/net/ens4f0/device/ptp/ptp0/period", []byte("2 0 0 1 0"), os.FileMode(0o666), nil) + + // Load a profile with multiple NICs (leading + other NICs) + profile, err := loadProfile("./testdata/profile-tbc.yaml") + assert.NoError(t, err) + + mockClockIDsFromProfile(mockFS, profile) + + // Initialize the clock chain with multiple NICs + err = OnPTPConfigChangeE810(nil, profile) + assert.NoError(t, err) + assert.Equal(t, 0, mockPinConfig.actualPinSetCount) + assert.Equal(t, 0, mockPinConfig.actualPinFrqCount) + + // Verify we have the expected clock chain structure + ccData := clockChain.(*ClockChain) + assert.Equal(t, ClockTypeTBC, ccData.Type) + assert.Equal(t, "ens4f0", ccData.LeadingNIC.Name) + assert.Equal(t, 2, len(ccData.OtherNICs), "should have 2 other NICs (ens5f0, ens8f0)") + + // Reset to only capture commands from the explicit SetPinDefaults call + mockPinSet.reset() + err = clockChain.SetPinDefaults() + assert.NoError(t, err) + assert.NotNil(t, mockPinSet.commands) + + // SetPinDefaults configures 9 different pin types, and we have 3 NICs total + // Each pin type should have a command for each NIC that has that pin + assert.Equal(t, len(*mockPinSet.commands), 27, "should have exactly 27 pin commands") + + // Verify that commands include pins from multiple clock IDs + clockIDsSeen := make(map[uint64]bool) + pinLabelsSeen := make(map[string]bool) + + mockPins := ccData.DpllPins.(*mockedDPLLPins) + for _, cmd := range *mockPinSet.commands { + // Find which pin this command refers to by searching all pins + for _, pin := range mockPins.pins { + if pin.ID == cmd.ID { + clockIDsSeen[pin.ClockID] = true + pinLabelsSeen[pin.BoardLabel] = true + break + } + } + } + + // We should see commands for multiple clock IDs (multiple NICs) + assert.GreaterOrEqual(t, len(clockIDsSeen), 2, "should have commands for at least 2 different clock IDs") + + // We should see commands for the standard configurable pin types + expectedPins := []string{ + "GNSS-1PPS", "SMA1", "SMA2/U.FL2", "CVL-SDP20", "CVL-SDP22", + "CVL-SDP21", "CVL-SDP23", "C827_0-RCLKA", "C827_0-RCLKB", + } + for _, expectedPin := range expectedPins { + assert.True(t, pinLabelsSeen[expectedPin], "should have command for pin %s", expectedPin) + } +} diff --git a/addons/intel/clockID.go b/addons/intel/clockID.go new file mode 100644 index 00000000..e96cc774 --- /dev/null +++ b/addons/intel/clockID.go @@ -0,0 +1,122 @@ +package intel + +import ( + "encoding/binary" + "fmt" + "os/exec" + "path/filepath" + "strconv" + "strings" + + "github.com/golang/glog" + dpll "github.com/k8snetworkplumbingwg/linuxptp-daemon/pkg/dpll-netlink" +) + +const ( + pciConfigSpaceSize = 256 + pciExtendedCapabilityDsnID = 3 + pciExtendedCapabilityNextOffset = 2 + pciExtendedCapabilityOffsetShift = 4 + pciExtendedCapabilityDataOffset = 4 +) + +// getClockID returns the DPLL clock ID for a network device. +func getClockID(device string) uint64 { + clockID := getPCIClockID(device) + if clockID != 0 { + return clockID + } + return getDevlinkClockID(device) +} + +func getPCIClockID(device string) uint64 { + b, err := filesystem.ReadFile(fmt.Sprintf("/sys/class/net/%s/device/config", device)) + if err != nil { + glog.Error(err) + return 0 + } + var offset uint16 = pciConfigSpaceSize + var id uint16 + for { + // TODO: Add test for == case + if len(b) <= int(offset) { + glog.Errorf("PCI config space too short (%d bytes) for device %s", len(b), device) + return 0 + } + id = binary.LittleEndian.Uint16(b[offset:]) + if id != pciExtendedCapabilityDsnID { + if id == 0 { + glog.Errorf("DSN capability not found for device %s", device) + return 0 + } + offset = binary.LittleEndian.Uint16(b[offset+pciExtendedCapabilityNextOffset:]) >> pciExtendedCapabilityOffsetShift + continue + } + break + } + return binary.LittleEndian.Uint64(b[offset+pciExtendedCapabilityDataOffset:]) +} + +func getDevlinkClockID(device string) uint64 { + devicePath, err := filesystem.ReadLink(fmt.Sprintf("/sys/class/net/%s/device", device)) + if err != nil { + glog.Errorf("failed to resolve PCI address for %s: %v", device, err) + return 0 + } + pciAddr := filepath.Base(devicePath) + + out, err := exec.Command("devlink", "dev", "info", "pci/"+pciAddr).Output() + if err != nil { + glog.Errorf("getDevlinkClockID: devlink failed for %s (pci/%s): %v", device, pciAddr, err) + return 0 + } + + for _, line := range strings.Split(string(out), "\n") { + fields := strings.Fields(strings.TrimSpace(line)) + if len(fields) != 2 || fields[0] != "serial_number" { + continue + } + var clockID uint64 + // "50-7c-6f-ff-ff-1f-b5-80" -> "507c6fffff1fb580" -> 0x507c6fffff1fb580 + clockID, err = strconv.ParseUint(strings.ReplaceAll(fields[1], "-", ""), 16, 64) + if err != nil { + glog.Errorf("getDevlinkClockID: failed to parse serial '%s': %v", fields[1], err) + return 0 + } + return clockID + } + glog.Errorf("getDevlinkClockID: serial_number not found in devlink output for %s (pci/%s)", device, pciAddr) + return 0 +} + +// Using a named anonymous function to allow mocking +var getAllDpllDevices = func() ([]*dpll.DoDeviceGetReply, error) { + conn, err := dpll.Dial(nil) + if err != nil { + return nil, err + } + //nolint:errcheck + defer conn.Close() + return conn.DumpDeviceGet() +} + +// getClockIDByModule returns ClockID for a given DPLL module name, preferring PPS type if present +func getClockIDByModule(module string) (uint64, error) { + devices, err := getAllDpllDevices() + if err != nil { + return 0, err + } + var anyID uint64 + for _, d := range devices { + if strings.EqualFold(d.ModuleName, module) { + if d.Type == 1 { // PPS + return d.ClockID, nil + } + anyID = d.ClockID + } + } + if anyID != 0 { + return anyID, nil + } + return 0, fmt.Errorf("module %s DPLL not found", module) +} diff --git a/addons/intel/clockID_test.go b/addons/intel/clockID_test.go new file mode 100644 index 00000000..a3e21ed4 --- /dev/null +++ b/addons/intel/clockID_test.go @@ -0,0 +1,144 @@ +package intel + +import ( + "encoding/binary" + "fmt" + "testing" + + dpll "github.com/k8snetworkplumbingwg/linuxptp-daemon/pkg/dpll-netlink" + "github.com/stretchr/testify/assert" +) + +func generatePCIDataForClockID(id uint64) []byte { + return generateTestPCIDataForClockID(id, 0) +} + +func generateTestPCIDataForClockID(id uint64, extraSections int) []byte { + result := make([]byte, pciConfigSpaceSize) + nextSectionOffset := pciConfigSpaceSize + for i := range extraSections { + id := i + pciExtendedCapabilityDsnID + 1 // Anything != pciExtendedCapabilityDsnID + sectionSize := (1 << pciExtendedCapabilityOffsetShift) * (i + 1) + dataSize := sectionSize - pciExtendedCapabilityDataOffset + nextSectionOffset = nextSectionOffset + sectionSize + shiftedOffset := nextSectionOffset << pciExtendedCapabilityOffsetShift + result = binary.LittleEndian.AppendUint16(result, uint16(id)) + result = binary.LittleEndian.AppendUint16(result, uint16(shiftedOffset)) + result = append(result, make([]byte, dataSize)...) + } + result = binary.LittleEndian.AppendUint16(result, uint16(pciExtendedCapabilityDsnID)) // 16-bit capability id + result = binary.LittleEndian.AppendUint16(result, uint16(1)) // 16-bit capability size (shiftedl + result = binary.LittleEndian.AppendUint64(result, id) + return result +} + +func Test_getPCIClockID(t *testing.T) { + mfs, restore := setupMockFS() + defer restore() + + notFound := uint64(0) + + // No such file + mfs.ExpectReadFile("/sys/class/net/missing/device/config", []byte{}, fmt.Errorf("No such file")) + clockID := getPCIClockID("missing") + assert.Equal(t, notFound, clockID) + mfs.VerifyAllCalls(t) + + // Config empty + mfs.ExpectReadFile("/sys/class/net/short/device/config", []byte{}, nil) + clockID = getPCIClockID("short") + assert.Equal(t, notFound, clockID) + mfs.VerifyAllCalls(t) + + // No DSN + mfs.ExpectReadFile("/sys/class/net/empty/device/config", make([]byte, pciConfigSpaceSize+64), nil) + clockID = getPCIClockID("empty") + assert.Equal(t, notFound, clockID) + mfs.VerifyAllCalls(t) + + // DSN at start of space + expectedID := uint64(1111222233334) + mfs.ExpectReadFile("/sys/class/net/one/device/config", generatePCIDataForClockID(expectedID), nil) + clockID = getPCIClockID("one") + assert.Equal(t, expectedID, clockID) + mfs.VerifyAllCalls(t) + + // DSN after a few other sections + expectedID = uint64(5555666677778) + mfs.ExpectReadFile("/sys/class/net/two/device/config", generateTestPCIDataForClockID(expectedID, 3), nil) + clockID = getPCIClockID("two") + assert.Equal(t, expectedID, clockID) + mfs.VerifyAllCalls(t) + + // Config space truncated + expectedID = uint64(7777888899990) + fullData := generateTestPCIDataForClockID(expectedID, 2) + mfs.ExpectReadFile("/sys/class/net/truncated/device/config", fullData[:len(fullData)-16], nil) + clockID = getPCIClockID("truncated") + assert.Equal(t, notFound, clockID) + mfs.VerifyAllCalls(t) +} + +func Test_getClockIDByModule(t *testing.T) { + notFound := uint64(0) + + getAllDpllDevices = func() ([]*dpll.DoDeviceGetReply, error) { + return nil, fmt.Errorf("Fake error") + } + clockID, err := getClockIDByModule("module") + assert.Error(t, err) + assert.Equal(t, notFound, clockID) + + getAllDpllDevices = func() ([]*dpll.DoDeviceGetReply, error) { + return []*dpll.DoDeviceGetReply{}, nil + } + clockID, err = getClockIDByModule("module") + assert.Error(t, err) + assert.Equal(t, notFound, clockID) + + getAllDpllDevices = func() ([]*dpll.DoDeviceGetReply, error) { + return []*dpll.DoDeviceGetReply{ + { + ID: 0, + ModuleName: "other", + Type: 1, + ClockID: 1, + }, + { + ID: 1, + ModuleName: "module", + Type: 2, + ClockID: 2, + }, + { + ID: 2, + ModuleName: "module", + Type: 1, + ClockID: 42, + }, + }, nil + } + clockID, err = getClockIDByModule("module") + assert.NoError(t, err) + assert.Equal(t, uint64(42), clockID) + + getAllDpllDevices = func() ([]*dpll.DoDeviceGetReply, error) { + return []*dpll.DoDeviceGetReply{ + { + ID: 0, + ModuleName: "other", + Type: 1, + ClockID: 1, + }, + { + ID: 1, + ModuleName: "module", + Type: 2, + ClockID: 2, + }, + }, nil + } + clockID, err = getClockIDByModule("module") + assert.NoError(t, err) + assert.Equal(t, uint64(2), clockID) +} diff --git a/addons/intel/common.go b/addons/intel/common.go new file mode 100644 index 00000000..994b9527 --- /dev/null +++ b/addons/intel/common.go @@ -0,0 +1,93 @@ +// Package intel contains plugins for all supported Intel NICs +package intel + +import ( + "os" + "slices" + + ptpv1 "github.com/k8snetworkplumbingwg/ptp-operator/api/v1" +) + +// PluginOpts contains all configuration data common to all addons/intel NIC plugins +type PluginOpts struct { + Devices []string `json:"devices"` + DevicePins map[string]pinSet `json:"pins"` + DeviceFreqencies map[string]frqSet `json:"frequencies"` + DpllSettings map[string]uint64 `json:"settings"` + PhaseOffsetPins map[string]map[string]string `json:"phaseOffsetPins"` +} + +// PluginData contains all persistent data commont to all addons/intel NIC plugins +type PluginData struct { + name string + hwplugins []string +} + +// PopulateHwConfig populates hwconfig for all intel plugins +func (data *PluginData) PopulateHwConfig(_ *interface{}, hwconfigs *[]ptpv1.HwConfig) error { + for _, _hwconfig := range data.hwplugins { + hwConfig := ptpv1.HwConfig{} + hwConfig.DeviceID = data.name + hwConfig.Status = _hwconfig + *hwconfigs = append(*hwconfigs, hwConfig) + } + return nil +} + +func extendWithKeys[T any](s []string, m map[string]T) []string { + for key := range m { + if !slices.Contains(s, key) { + s = append(s, key) + } + } + return s +} + +// allDevices enumerates all defined devices (Devices/DevicePins/DeviceFrequencies/PhaseOffsets) +func (opts *PluginOpts) allDevices() []string { + // Enumerate all defined devices (Devices/DevicePins/DeviceFrequencies) + allDevices := opts.Devices + allDevices = extendWithKeys(allDevices, opts.DevicePins) + allDevices = extendWithKeys(allDevices, opts.DeviceFreqencies) + allDevices = extendWithKeys(allDevices, opts.PhaseOffsetPins) + return allDevices +} + +// GnssOptions defines GNSS-specific options common to Intel NIC plugins +type GnssOptions struct { + Disabled bool `json:"disabled"` +} + +// FileSystemInterface defines the interface for filesystem operations to enable mocking +type FileSystemInterface interface { + ReadDir(dirname string) ([]os.DirEntry, error) + WriteFile(filename string, data []byte, perm os.FileMode) error + ReadFile(filename string) ([]byte, error) + ReadLink(filename string) (string, error) +} + +// RealFileSystem implements FileSystemInterface using real OS operations +type RealFileSystem struct{} + +// ReadDir reads the contents of the directory specified by dirname +func (fs *RealFileSystem) ReadDir(dirname string) ([]os.DirEntry, error) { + return os.ReadDir(dirname) +} + +// WriteFile writes the data to the file specified by filename +func (fs *RealFileSystem) WriteFile(filename string, data []byte, perm os.FileMode) error { + return os.WriteFile(filename, data, perm) +} + +// ReadFile reads the data from the file specified by the filename +func (fs *RealFileSystem) ReadFile(filename string) ([]byte, error) { + return os.ReadFile(filename) +} + +// ReadLink returns the destination of a symbolic link. +func (fs *RealFileSystem) ReadLink(filename string) (string, error) { + return os.Readlink(filename) +} + +// Default filesystem implementation +var filesystem FileSystemInterface = &RealFileSystem{} diff --git a/addons/intel/dpllPins.go b/addons/intel/dpllPins.go new file mode 100644 index 00000000..1220e7d8 --- /dev/null +++ b/addons/intel/dpllPins.go @@ -0,0 +1,143 @@ +package intel + +import ( + "errors" + "fmt" + "strings" + + "github.com/golang/glog" + dpll "github.com/k8snetworkplumbingwg/linuxptp-daemon/pkg/dpll-netlink" +) + +// DPLLPins abstracts DPLL pin operations for mocking in tests. +type DPLLPins interface { + ApplyPinCommands(commands []dpll.PinParentDeviceCtl) error + FetchPins() error + GetByLabel(label string, clockID uint64) *dpll.PinInfo + GetAllPinsByLabel(label string) []*dpll.PinInfo + GetCommandsForPluginPinSet(clockID uint64, pinset pinSet) []dpll.PinParentDeviceCtl +} + +type dpllPins []*dpll.PinInfo + +// DpllPins is the package-level DPLL pin accessor, replaceable for testing. +var DpllPins DPLLPins = &dpllPins{} + +func (d *dpllPins) FetchPins() error { + var pins []*dpll.PinInfo + + conn, err := dpll.Dial(nil) + if err != nil { + return fmt.Errorf("failed to dial DPLL: %v", err) + } + //nolint:errcheck + defer conn.Close() + pins, err = conn.DumpPinGet() + if err != nil { + return fmt.Errorf("failed to dump DPLL pins: %v", err) + } + *d = dpllPins(pins) + return nil +} + +func (d *dpllPins) GetByLabel(label string, clockID uint64) *dpll.PinInfo { + for _, pin := range *d { + if pin.BoardLabel == label && pin.ClockID == clockID { + return pin + } + } + return nil +} + +func (d *dpllPins) GetAllPinsByLabel(label string) []*dpll.PinInfo { + result := make([]*dpll.PinInfo, 0) + + for _, pin := range *d { + if pin.BoardLabel == label { + result = append(result, pin) + } + } + return result +} + +func (d *dpllPins) GetCommandsForPluginPinSet(clockID uint64, pinset pinSet) []dpll.PinParentDeviceCtl { + pinCommands := make([]dpll.PinParentDeviceCtl, 0) + + for label, valueStr := range pinset { + // TODO: Move label checks to a higher level function + // if label == "U.FL1" || label == "U.FL2" { + // glog.Warningf("%s can not longer be set via the pins on the plugin; values ignored", label) + // continue + // } + + valueStr = strings.TrimSpace(valueStr) + values := strings.Fields(valueStr) + if len(values) != 2 { + glog.Errorf("Failed to unpack values for pin %s of clockID %d from '%s'", label, clockID, valueStr) + continue + } + + pinInfo := d.GetByLabel(label, clockID) + if pinInfo == nil { + glog.Errorf("not found pin with label %s for clockID %d", label, clockID) + continue + } + if len(pinInfo.ParentDevice) == 0 { + glog.Errorf("Unable to configure: No parent devices for pin %s for clockID %s", label, clockID) + continue + } + + ppCtrl := PinParentControl{} + + // For now we are ignoring the second value and setting both parent devices the same. + for i, parentDev := range pinInfo.ParentDevice { + var state uint8 + var priority uint8 + var direction *uint8 + + // TODO add checks for capabilies such as direction changes. + switch values[0] { + case "0": + if parentDev.Direction == dpll.PinDirectionInput { + state = dpll.PinStateSelectable + } else { + state = dpll.PinStateDisconnected + } + priority = PriorityDisabled + case "1": + state = dpll.PinStateConnected + v := uint8(dpll.PinDirectionInput) + direction = &v + priority = PriorityEnabled + case "2": + state = dpll.PinStateConnected + v := uint8(dpll.PinDirectionOutput) + direction = &v + priority = PriorityEnabled + default: + glog.Errorf("invalid initial value in pin config for clock id %s pin %s: '%s'", clockID, label, values[0]) + continue + } + + switch i { + case eecDpllIndex: + ppCtrl.EecOutputState = state + ppCtrl.EecDirection = direction + ppCtrl.EecPriority = priority + case ppsDpllIndex: + ppCtrl.PpsOutputState = state + ppCtrl.PpsDirection = direction + ppCtrl.PpsPriority = priority + } + } + pinCommands = append(pinCommands, SetPinControlData(*pinInfo, ppCtrl)...) + } + return pinCommands +} + +func (d *dpllPins) ApplyPinCommands(commands []dpll.PinParentDeviceCtl) error { + err := BatchPinSet(&commands) + // event if there was an error we still need to refresh the pin state. + fetchErr := d.FetchPins() + return errors.Join(err, fetchErr) +} diff --git a/addons/intel/dpllPins_test.go b/addons/intel/dpllPins_test.go new file mode 100644 index 00000000..880b7cd4 --- /dev/null +++ b/addons/intel/dpllPins_test.go @@ -0,0 +1,463 @@ +package intel + +import ( + "testing" + + dpll "github.com/k8snetworkplumbingwg/linuxptp-daemon/pkg/dpll-netlink" + "github.com/stretchr/testify/assert" +) + +const allPinCaps = dpll.PinCapDir | dpll.PinCapPrio | dpll.PinCapState + +func makeTwoParentPin(id uint32, label string, clockID uint64, eecDir, ppsDir uint32) *dpll.PinInfo { + return &dpll.PinInfo{ + ID: id, + BoardLabel: label, + ClockID: clockID, + Type: dpll.PinTypeEXT, + Capabilities: allPinCaps, + ParentDevice: []dpll.PinParentDevice{ + {ParentID: 100, Direction: eecDir, Prio: toPtr[uint32](0), State: dpll.PinStateDisconnected}, + {ParentID: 200, Direction: ppsDir, Prio: toPtr[uint32](0), State: dpll.PinStateDisconnected}, + }, + } +} + +func makePins(pins ...*dpll.PinInfo) dpllPins { + return dpllPins(pins) +} + +func TestGetByLabel(t *testing.T) { + pins := makePins( + makeTwoParentPin(1, "SMA1", 1000, dpll.PinDirectionInput, dpll.PinDirectionInput), + makeTwoParentPin(2, "SMA2", 1000, dpll.PinDirectionOutput, dpll.PinDirectionOutput), + makeTwoParentPin(3, "SMA1", 2000, dpll.PinDirectionInput, dpll.PinDirectionInput), + ) + + t.Run("found", func(t *testing.T) { + pin := pins.GetByLabel("SMA1", 1000) + assert.NotNil(t, pin) + assert.Equal(t, uint32(1), pin.ID) + }) + + t.Run("found_different_clockID", func(t *testing.T) { + pin := pins.GetByLabel("SMA1", 2000) + assert.NotNil(t, pin) + assert.Equal(t, uint32(3), pin.ID) + }) + + t.Run("wrong_label", func(t *testing.T) { + pin := pins.GetByLabel("GNSS_1PPS_IN", 1000) + assert.Nil(t, pin) + }) + + t.Run("wrong_clockID", func(t *testing.T) { + pin := pins.GetByLabel("SMA1", 9999) + assert.Nil(t, pin) + }) +} + +func TestGetCommandsForPluginPinSet_BadFormat(t *testing.T) { + pins := makePins( + makeTwoParentPin(1, "SMA1", 1000, dpll.PinDirectionInput, dpll.PinDirectionInput), + ) + + t.Run("single_value", func(t *testing.T) { + cmds := pins.GetCommandsForPluginPinSet(1000, pinSet{"SMA1": "1"}) + assert.Empty(t, cmds) + }) + + t.Run("three_values", func(t *testing.T) { + cmds := pins.GetCommandsForPluginPinSet(1000, pinSet{"SMA1": "1 1 1"}) + assert.Empty(t, cmds) + }) + + t.Run("empty_value", func(t *testing.T) { + cmds := pins.GetCommandsForPluginPinSet(1000, pinSet{"SMA1": ""}) + assert.Empty(t, cmds) + }) +} + +func TestGetCommandsForPluginPinSet_PinNotFound(t *testing.T) { + pins := makePins( + makeTwoParentPin(1, "SMA1", 1000, dpll.PinDirectionInput, dpll.PinDirectionInput), + ) + + cmds := pins.GetCommandsForPluginPinSet(1000, pinSet{"SMA2": "1 1"}) + assert.Empty(t, cmds) +} + +func TestGetCommandsForPluginPinSet_NoParentDevices(t *testing.T) { + pins := makePins(&dpll.PinInfo{ + ID: 1, + BoardLabel: "SMA1", + ClockID: 1000, + ParentDevice: []dpll.PinParentDevice{}, + }) + + cmds := pins.GetCommandsForPluginPinSet(1000, pinSet{"SMA1": "1 1"}) + assert.Empty(t, cmds) +} + +func TestGetCommandsForPluginPinSet_InvalidValue(t *testing.T) { + pins := makePins( + makeTwoParentPin(1, "SMA1", 1000, dpll.PinDirectionInput, dpll.PinDirectionInput), + ) + + // The default branch's `continue` only skips the inner parent-device loop, + // so SetPinControlData is still called with a zero-valued PinParentControl. + cmds := pins.GetCommandsForPluginPinSet(1000, pinSet{"SMA1": "9 9"}) + assert.Len(t, cmds, 1) + cmd := cmds[0] + assert.Len(t, cmd.PinParentCtl, 2) + for _, pc := range cmd.PinParentCtl { + assert.Nil(t, pc.Direction) + assert.NotNil(t, pc.Prio) + assert.Equal(t, uint32(0), *pc.Prio) + assert.NotNil(t, pc.State, "input pins always get state=selectable") + assert.Equal(t, uint32(dpll.PinStateSelectable), *pc.State) + } +} + +func toPtr[V any](v V) *V { return &v } + +func TestGetCommandsForPluginPinSet_Value0_FromConnected(t *testing.T) { + pins := makePins(&dpll.PinInfo{ + ID: 1, + BoardLabel: "SMA1", + ClockID: 1000, + Capabilities: allPinCaps, + ParentDevice: []dpll.PinParentDevice{ + {ParentID: 100, Direction: dpll.PinDirectionInput, Prio: toPtr[uint32](0), State: dpll.PinStateConnected}, + {ParentID: 200, Direction: dpll.PinDirectionInput, Prio: toPtr[uint32](0), State: dpll.PinStateConnected}, + }, + }) + + cmds := pins.GetCommandsForPluginPinSet(1000, pinSet{"SMA1": "0 0"}) + assert.Len(t, cmds, 1) + cmd := cmds[0] + assert.Equal(t, uint32(1), cmd.ID) + assert.Len(t, cmd.PinParentCtl, 2) + + for _, pc := range cmd.PinParentCtl { + assert.Nil(t, pc.Direction, "direction should be nil for value 0") + assert.NotNil(t, pc.Prio) + assert.Equal(t, uint32(PriorityDisabled), *pc.Prio) + assert.NotNil(t, pc.State, "input pins always get state=selectable") + assert.Equal(t, uint32(dpll.PinStateSelectable), *pc.State) + } +} + +func TestGetCommandsForPluginPinSet_Value0_FromDisconnected(t *testing.T) { + pins := makePins(&dpll.PinInfo{ + ID: 1, + BoardLabel: "SMA1", + ClockID: 1000, + Capabilities: allPinCaps, + ParentDevice: []dpll.PinParentDevice{ + {ParentID: 100, Direction: dpll.PinDirectionOutput, Prio: toPtr[uint32](0), State: dpll.PinStateDisconnected}, + {ParentID: 200, Direction: dpll.PinDirectionOutput, Prio: toPtr[uint32](0), State: dpll.PinStateDisconnected}, + }, + }) + + cmds := pins.GetCommandsForPluginPinSet(1000, pinSet{"SMA1": "0 0"}) + assert.Len(t, cmds, 1) + cmd := cmds[0] + assert.Len(t, cmd.PinParentCtl, 2) + + for _, pc := range cmd.PinParentCtl { + assert.Nil(t, pc.Direction, "direction should be nil for value 0") + // parentDevice.Direction is Output, so State is set + assert.NotNil(t, pc.State) + assert.Equal(t, uint32(dpll.PinStateDisconnected), *pc.State) + assert.Nil(t, pc.Prio, "prio should not be set for output direction") + } +} + +func TestGetCommandsForPluginPinSet_Value1_SetInput(t *testing.T) { + pins := makePins(&dpll.PinInfo{ + ID: 5, + BoardLabel: "SMA1", + ClockID: 1000, + Capabilities: allPinCaps, + ParentDevice: []dpll.PinParentDevice{ + {ParentID: 100, Direction: dpll.PinDirectionOutput, Prio: toPtr[uint32](0), State: dpll.PinStateDisconnected}, + {ParentID: 200, Direction: dpll.PinDirectionOutput, Prio: toPtr[uint32](0), State: dpll.PinStateDisconnected}, + }, + }) + + cmds := pins.GetCommandsForPluginPinSet(1000, pinSet{"SMA1": "1 1"}) + assert.Len(t, cmds, 2, "direction change should produce two commands") + + dirCmd := cmds[0] + assert.Equal(t, uint32(5), dirCmd.ID) + assert.Len(t, dirCmd.PinParentCtl, 2) + for _, pc := range dirCmd.PinParentCtl { + assert.NotNil(t, pc.Direction, "first command sets direction") + assert.Equal(t, uint32(dpll.PinDirectionInput), *pc.Direction) + assert.Nil(t, pc.Prio, "first command has no prio") + assert.Nil(t, pc.State, "first command has no state") + } + + dataCmd := cmds[1] + assert.Equal(t, uint32(5), dataCmd.ID) + assert.Len(t, dataCmd.PinParentCtl, 2) + for _, pc := range dataCmd.PinParentCtl { + assert.Nil(t, pc.Direction, "second command has no direction") + assert.NotNil(t, pc.Prio, "second command sets prio for input") + assert.Equal(t, uint32(PriorityEnabled), *pc.Prio) + assert.NotNil(t, pc.State, "input pins always get state=selectable") + assert.Equal(t, uint32(dpll.PinStateSelectable), *pc.State) + } +} + +func TestGetCommandsForPluginPinSet_Value2_SetOutput(t *testing.T) { + pins := makePins(&dpll.PinInfo{ + ID: 7, + BoardLabel: "SMA2", + ClockID: 1000, + Capabilities: allPinCaps, + ParentDevice: []dpll.PinParentDevice{ + {ParentID: 100, Direction: dpll.PinDirectionInput, Prio: toPtr[uint32](0), State: dpll.PinStateConnected}, + {ParentID: 200, Direction: dpll.PinDirectionInput, Prio: toPtr[uint32](0), State: dpll.PinStateConnected}, + }, + }) + + cmds := pins.GetCommandsForPluginPinSet(1000, pinSet{"SMA2": "2 2"}) + assert.Len(t, cmds, 2, "direction change should produce two commands") + + dirCmd := cmds[0] + assert.Equal(t, uint32(7), dirCmd.ID) + assert.Len(t, dirCmd.PinParentCtl, 2) + for _, pc := range dirCmd.PinParentCtl { + assert.NotNil(t, pc.Direction, "first command sets direction") + assert.Equal(t, uint32(dpll.PinDirectionOutput), *pc.Direction) + assert.Nil(t, pc.Prio, "first command has no prio") + assert.Nil(t, pc.State, "first command has no state") + } + + dataCmd := cmds[1] + assert.Equal(t, uint32(7), dataCmd.ID) + assert.Len(t, dataCmd.PinParentCtl, 2) + for _, pc := range dataCmd.PinParentCtl { + assert.Nil(t, pc.Direction, "second command has no direction") + assert.NotNil(t, pc.State, "second command sets state for output") + assert.Equal(t, uint32(dpll.PinStateConnected), *pc.State) + assert.Nil(t, pc.Prio, "second command has no prio for output") + } +} + +func TestGetCommandsForPluginPinSet_MultiplePins(t *testing.T) { + pins := makePins( + makeTwoParentPin(1, "SMA1", 1000, dpll.PinDirectionInput, dpll.PinDirectionInput), + makeTwoParentPin(2, "SMA2", 1000, dpll.PinDirectionOutput, dpll.PinDirectionOutput), + ) + + ps := pinSet{ + "SMA1": "1 1", + "SMA2": "2 2", + } + cmds := pins.GetCommandsForPluginPinSet(1000, ps) + assert.Len(t, cmds, 2) + + cmdByID := map[uint32]dpll.PinParentDeviceCtl{} + for _, c := range cmds { + cmdByID[c.ID] = c + } + + sma1 := cmdByID[1] + assert.Len(t, sma1.PinParentCtl, 2) + for _, pc := range sma1.PinParentCtl { + assert.Nil(t, pc.Direction, "no direction change for Input->Input") + assert.NotNil(t, pc.Prio) + assert.NotNil(t, pc.State, "input pins always get state=selectable") + assert.Equal(t, uint32(dpll.PinStateSelectable), *pc.State) + } + + sma2 := cmdByID[2] + assert.Len(t, sma2.PinParentCtl, 2) + for _, pc := range sma2.PinParentCtl { + assert.Nil(t, pc.Direction, "no direction change for Output->Output") + assert.NotNil(t, pc.State) + } +} + +func TestGetCommandsForPluginPinSet_ParentIDs(t *testing.T) { + pins := makePins(&dpll.PinInfo{ + ID: 10, + BoardLabel: "SMA1", + ClockID: 5000, + Capabilities: allPinCaps, + ParentDevice: []dpll.PinParentDevice{ + {ParentID: 42, Direction: dpll.PinDirectionInput, Prio: toPtr[uint32](0)}, + {ParentID: 99, Direction: dpll.PinDirectionInput, Prio: toPtr[uint32](0)}, + }, + }) + + cmds := pins.GetCommandsForPluginPinSet(5000, pinSet{"SMA1": "1 1"}) + assert.Len(t, cmds, 1) + assert.Equal(t, uint32(42), cmds[0].PinParentCtl[0].PinParentID) + assert.Equal(t, uint32(99), cmds[0].PinParentCtl[1].PinParentID) +} + +func TestGetCommandsForPluginPinSet_WhitespaceHandling(t *testing.T) { + pins := makePins( + makeTwoParentPin(1, "SMA1", 1000, dpll.PinDirectionInput, dpll.PinDirectionInput), + ) + + t.Run("leading_trailing_spaces", func(t *testing.T) { + cmds := pins.GetCommandsForPluginPinSet(1000, pinSet{"SMA1": " 1 1 "}) + assert.Len(t, cmds, 1) + }) + + t.Run("extra_internal_spaces", func(t *testing.T) { + cmds := pins.GetCommandsForPluginPinSet(1000, pinSet{"SMA1": "1 1"}) + assert.Len(t, cmds, 1) + }) +} + +func TestSetPinControlData_StateOnlyPin(t *testing.T) { + pin := dpll.PinInfo{ + ID: 15, + BoardLabel: "U.FL1", + Capabilities: dpll.PinCapState, + ParentDevice: []dpll.PinParentDevice{ + {ParentID: 0, Direction: dpll.PinDirectionOutput, State: dpll.PinStateConnected}, + {ParentID: 1, Direction: dpll.PinDirectionOutput, State: dpll.PinStateConnected}, + }, + } + + control := PinParentControl{ + EecOutputState: dpll.PinStateDisconnected, + PpsOutputState: dpll.PinStateDisconnected, + } + cmds := SetPinControlData(pin, control) + assert.Len(t, cmds, 1) + for _, pc := range cmds[0].PinParentCtl { + assert.NotNil(t, pc.State, "state-can-change should allow State") + assert.Equal(t, uint32(dpll.PinStateDisconnected), *pc.State) + assert.Nil(t, pc.Prio, "no priority-can-change capability") + assert.Nil(t, pc.Direction, "no direction-can-change capability") + } +} + +func TestSetPinControlData_StateOnlyPin_InputDirection(t *testing.T) { + pin := dpll.PinInfo{ + ID: 37, + BoardLabel: "U.FL2", + Capabilities: dpll.PinCapState, + ParentDevice: []dpll.PinParentDevice{ + {ParentID: 2, Direction: dpll.PinDirectionInput, State: dpll.PinStateDisconnected}, + {ParentID: 3, Direction: dpll.PinDirectionInput, State: dpll.PinStateDisconnected}, + }, + } + + control := PinParentControl{ + EecPriority: PriorityDisabled, + PpsPriority: PriorityDisabled, + EecOutputState: dpll.PinStateDisconnected, + PpsOutputState: dpll.PinStateDisconnected, + } + cmds := SetPinControlData(pin, control) + assert.Len(t, cmds, 1) + for _, pc := range cmds[0].PinParentCtl { + assert.Nil(t, pc.Prio, "no priority-can-change: prio must not be set") + assert.Nil(t, pc.Direction, "no direction-can-change: direction must not be set") + assert.NotNil(t, pc.State, "input pin with state-can-change gets state=selectable") + assert.Equal(t, uint32(dpll.PinStateSelectable), *pc.State) + } +} + +func TestSetPinControlData_NoCaps(t *testing.T) { + pin := dpll.PinInfo{ + ID: 99, + Capabilities: dpll.PinCapNone, + ParentDevice: []dpll.PinParentDevice{ + {ParentID: 0, Direction: dpll.PinDirectionInput}, + {ParentID: 1, Direction: dpll.PinDirectionInput}, + }, + } + + control := PinParentControl{ + EecPriority: 0, + PpsPriority: 0, + EecOutputState: dpll.PinStateConnected, + PpsOutputState: dpll.PinStateConnected, + } + cmds := SetPinControlData(pin, control) + assert.Len(t, cmds, 1) + for _, pc := range cmds[0].PinParentCtl { + assert.Nil(t, pc.Prio, "no capabilities: prio must not be set") + assert.Nil(t, pc.State, "no capabilities: state must not be set") + assert.Nil(t, pc.Direction, "no capabilities: direction must not be set") + } +} + +func TestSetPinControlData_PrioCapButNilPrio(t *testing.T) { + pin := dpll.PinInfo{ + ID: 58, + BoardLabel: "U.FL2", + Capabilities: dpll.PinCapPrio | dpll.PinCapState, + ParentDevice: []dpll.PinParentDevice{ + {ParentID: 4, Direction: dpll.PinDirectionInput, State: dpll.PinStateDisconnected}, + {ParentID: 5, Direction: dpll.PinDirectionInput, State: dpll.PinStateDisconnected}, + }, + } + + control := PinParentControl{ + EecPriority: PriorityDisabled, + PpsPriority: PriorityDisabled, + } + cmds := SetPinControlData(pin, control) + assert.Len(t, cmds, 1) + for _, pc := range cmds[0].PinParentCtl { + assert.Nil(t, pc.Prio, "kernel reports no prio on parent device: prio must not be set") + assert.NotNil(t, pc.State, "input pin with state-can-change gets state=selectable") + assert.Equal(t, uint32(dpll.PinStateSelectable), *pc.State) + } +} + +func TestBuildDirectionCmd_NoDirCap(t *testing.T) { + eecDir := uint8(dpll.PinDirectionOutput) + ppsDir := uint8(dpll.PinDirectionOutput) + pin := dpll.PinInfo{ + ID: 10, + Capabilities: dpll.PinCapPrio | dpll.PinCapState, + ParentDevice: []dpll.PinParentDevice{ + {ParentID: 0, Direction: dpll.PinDirectionInput}, + {ParentID: 1, Direction: dpll.PinDirectionInput}, + }, + } + control := PinParentControl{ + EecDirection: &eecDir, + PpsDirection: &ppsDir, + } + cmd := buildDirectionCmd(&pin, control) + assert.Nil(t, cmd, "direction change should be skipped without PinCapDir") + assert.Equal(t, uint32(dpll.PinDirectionInput), pin.ParentDevice[0].Direction, "direction should not be updated") + assert.Equal(t, uint32(dpll.PinDirectionInput), pin.ParentDevice[1].Direction, "direction should not be updated") +} + +func TestApplyPinCommands(t *testing.T) { + _, restorePins := setupMockDPLLPins( + makeTwoParentPin(1, "SMA1", 1000, dpll.PinDirectionInput, dpll.PinDirectionInput), + ) + defer restorePins() + + mockPinSet, restore := setupBatchPinSetMock() + defer restore() + + cmds := []dpll.PinParentDeviceCtl{ + { + ID: 1, + PinParentCtl: []dpll.PinControl{ + {PinParentID: 100, Prio: toPtr[uint32](0)}, + }, + }, + } + + err := DpllPins.ApplyPinCommands(cmds) + assert.NoError(t, err) + assert.NotNil(t, mockPinSet.commands) + assert.Len(t, *mockPinSet.commands, 1) +} diff --git a/addons/intel/e810.go b/addons/intel/e810.go index d80a6d0a..18e64a78 100644 --- a/addons/intel/e810.go +++ b/addons/intel/e810.go @@ -1,70 +1,45 @@ package intel import ( - "encoding/binary" "encoding/json" "fmt" - "os" - "os/exec" - "reflect" "strconv" "strings" "github.com/golang/glog" "github.com/k8snetworkplumbingwg/linuxptp-daemon/pkg/dpll" - dpll_netlink "github.com/k8snetworkplumbingwg/linuxptp-daemon/pkg/dpll-netlink" "github.com/k8snetworkplumbingwg/linuxptp-daemon/pkg/plugin" ptpv1 "github.com/k8snetworkplumbingwg/ptp-operator/api/v1" ) +var pluginNameE810 = "e810" + type E810Opts struct { - EnableDefaultConfig bool `json:"enableDefaultConfig"` - UblxCmds UblxCmdList `json:"ublxCmds"` - DevicePins map[string]map[string]string `json:"pins"` - DpllSettings map[string]uint64 `json:"settings"` - PhaseOffsetPins map[string]map[string]string `json:"phaseOffsetPins"` - PhaseInputs []PhaseInputs `json:"interconnections"` + PluginOpts + EnableDefaultConfig bool `json:"enableDefaultConfig"` + UblxCmds UblxCmdList `json:"ublxCmds"` + PhaseInputs []PhaseInputs `json:"interconnections"` } // GetPhaseInputs implements PhaseInputsProvider func (o E810Opts) GetPhaseInputs() []PhaseInputs { return o.PhaseInputs } -type E810UblxCmds struct { - ReportOutput bool `json:"reportOutput"` - Args []string `json:"args"` -} - type E810PluginData struct { - hwplugins *[]string + PluginData } -// Sourced from https://github.com/RHsyseng/oot-ice/blob/main/ptp-config.sh -var EnableE810PTPConfig = ` -#!/bin/bash -set -eu - -ETH=$(grep -e 000e -e 000f /sys/class/net/*/device/subsystem_device | awk -F"/" '{print $5}') - -for DEV in $ETH; do - if [ -f /sys/class/net/$DEV/device/ptp/ptp*/pins/U.FL2 ]; then - echo 0 2 > /sys/class/net/$DEV/device/ptp/ptp*/pins/U.FL2 - echo 0 1 > /sys/class/net/$DEV/device/ptp/ptp*/pins/U.FL1 - echo 0 2 > /sys/class/net/$DEV/device/ptp/ptp*/pins/SMA2 - echo 0 1 > /sys/class/net/$DEV/device/ptp/ptp*/pins/SMA1 - fi -done - -echo "Disabled all SMA and U.FL Connections" -` - var ( - unitTest bool - clockChain = &ClockChain{} + clockChain ClockChainInterface = &ClockChain{DpllPins: DpllPins} + + // defaultE810PinConfig -> All outputs disabled + defaultE810PinConfig = pinSet{ + "SMA1": "0 1", + "SMA2": "0 2", + "U.FL1": "0 1", + "U.FL2": "0 2", + } ) -// For mocking DPLL pin info -var DpllPins = []*dpll_netlink.PinInfo{} - func OnPTPConfigChangeE810(data *interface{}, nodeProfile *ptpv1.PtpProfile) error { glog.Info("calling onPTPConfigChange for e810 plugin") @@ -72,92 +47,120 @@ func OnPTPConfigChangeE810(data *interface{}, nodeProfile *ptpv1.PtpProfile) err var e810Opts E810Opts var err error - var optsByteArray []byte - var stdout []byte - var pinPath string e810Opts.EnableDefaultConfig = false + err = DpllPins.FetchPins() + if err != nil { + return err + } + for name, opts := range (*nodeProfile).Plugins { - if name == "e810" { - optsByteArray, _ = json.Marshal(opts) + if name == pluginNameE810 { + optsByteArray, _ := json.Marshal(opts) err = json.Unmarshal(optsByteArray, &e810Opts) if err != nil { glog.Error("e810 failed to unmarshal opts: " + err.Error()) } - // for unit testing only, PtpSettings may include "unitTest" key. The value is - // the path where resulting configuration files will be written, instead of /var/run - _, unitTest = (*nodeProfile).PtpSettings["unitTest"] - if unitTest { - MockPins() - } - if e810Opts.EnableDefaultConfig { - stdout, _ = exec.Command("/usr/bin/bash", "-c", EnableE810PTPConfig).Output() - glog.Infof(string(stdout)) - } + allDevices := e810Opts.allDevices() + + clockIDs := make(map[string]uint64) + if (*nodeProfile).PtpSettings == nil { (*nodeProfile).PtpSettings = make(map[string]string) } - for device, pins := range e810Opts.DevicePins { - dpllClockIdStr := fmt.Sprintf("%s[%s]", dpll.ClockIdStr, device) - if !unitTest { - (*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()) - continue - } - for _, phc := range phcs { - 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), 0o666) - if err != nil { - glog.Error("e810 failed to write " + value + " to " + pinPath + ": " + err.Error()) - } - } - } + + glog.Infof("Initializing e810 plugin for profile %s and devices %v", *nodeProfile.Name, allDevices) + for _, device := range allDevices { + dpllClockIDStr := fmt.Sprintf("%s[%s]", dpll.ClockIdStr, device) + clkID := getClockID(device) + if clkID == 0 { + glog.Errorf("failed to get clockID for device %s; pins for this device will not be configured", device) + } + clockIDs[device] = clkID + (*nodeProfile).PtpSettings[dpllClockIDStr] = strconv.FormatUint(clkID, 10) + } + + for device, frequencies := range e810Opts.DeviceFreqencies { + err = pinConfig.applyPinFrq(device, frequencies) + if err != nil { + glog.Errorf("e825 failed to set PHC frequencies for %s: %s", device, err) } } + // Copy DPLL Settings from plugin config to PtpSettings for k, v := range e810Opts.DpllSettings { if _, ok := (*nodeProfile).PtpSettings[k]; !ok { (*nodeProfile).PtpSettings[k] = strconv.FormatUint(v, 10) } } + + // Copy PhaseOffsetPins settings from plugin config to PtpSettings for iface, properties := range e810Opts.PhaseOffsetPins { - ifaceFound := false - for dev := range e810Opts.DevicePins { - if strings.Compare(iface, dev) == 0 { - ifaceFound = true - break - } - } - if !ifaceFound { - glog.Errorf("e810 phase offset pin filter initialization failed: interface %s not found among %v", - iface, reflect.ValueOf(e810Opts.DevicePins).MapKeys()) - break - } for pinProperty, value := range properties { - key := strings.Join([]string{iface, "phaseOffsetFilter", strconv.FormatUint(getClockIDE810(iface), 10), pinProperty}, ".") + key := strings.Join([]string{iface, "phaseOffsetFilter", strconv.FormatUint(getClockID(iface), 10), pinProperty}, ".") (*nodeProfile).PtpSettings[key] = value } } + + // Initialize clockChain if e810Opts.PhaseInputs != nil { - if unitTest { - // Mock clock chain DPLL pins in unit test - clockChain.DpllPins = DpllPins - } clockChain, err = InitClockChain(e810Opts, nodeProfile) if err != nil { return err } - (*nodeProfile).PtpSettings["leadingInterface"] = clockChain.LeadingNIC.Name - (*nodeProfile).PtpSettings["upstreamPort"] = clockChain.LeadingNIC.UpstreamPort + (*nodeProfile).PtpSettings["leadingInterface"] = clockChain.GetLeadingNIC().Name + (*nodeProfile).PtpSettings["upstreamPort"] = clockChain.GetLeadingNIC().UpstreamPort } else { - glog.Error("no clock chain set") + glog.Infof("No clock chain set: Restoring any previous pin state changes") + err = clockChain.SetPinDefaults() + if err != nil { + glog.Errorf("Could not restore clockChain pin defaults: %s", err) + } + clockChain = &ClockChain{DpllPins: DpllPins} + err = DpllPins.FetchPins() + if err != nil { + glog.Errorf("Could not determine the current state of the dpll pins: %s", err) + } + } + + if e810Opts.EnableDefaultConfig { + for _, device := range allDevices { + if hasSysfsSMAPins(device) { + err = pinConfig.applyPinSet(device, defaultE810PinConfig) + } else { + err = DpllPins.ApplyPinCommands(DpllPins.GetCommandsForPluginPinSet(clockIDs[device], defaultE810PinConfig)) + } + if err != nil { + glog.Errorf("e810 failed to set default Pin configuration for %s: %s", device, err) + } + } + } + + // Initialize all user-specified phc pins and frequencies + for device, pins := range e810Opts.DevicePins { + if hasSysfsSMAPins(device) { + err = pinConfig.applyPinSet(device, pins) + } else { + commands := DpllPins.GetCommandsForPluginPinSet(clockIDs[device], pins) + if pinSetHasSMAInput(pins) { + gnssPin := DpllPins.GetByLabel(gnss, clockIDs[device]) + if gnssPin != nil { + gnssCommands := SetPinControlData(*gnssPin, PinParentControl{ + EecPriority: 4, + PpsPriority: 4, + }) + commands = append(commands, gnssCommands...) + } else { + glog.Warningf("SMA input detected but GNSS-1PPS pin not found for clockID %d", clockIDs[device]) + } + } + err = DpllPins.ApplyPinCommands(commands) + } + if err != nil { + glog.Errorf("e810 failed to set Pin configuration for %s: %s", device, err) + } } } } @@ -169,13 +172,12 @@ func AfterRunPTPCommandE810(data *interface{}, nodeProfile *ptpv1.PtpProfile, co glog.Info("calling AfterRunPTPCommandE810 for e810 plugin") var e810Opts E810Opts var err error - var optsByteArray []byte e810Opts.EnableDefaultConfig = false for name, opts := range (*nodeProfile).Plugins { - if name == "e810" { - optsByteArray, _ = json.Marshal(opts) + if name == pluginNameE810 { + optsByteArray, _ := json.Marshal(opts) err = json.Unmarshal(optsByteArray, &e810Opts) if err != nil { glog.Error("e810 failed to unmarshal opts: " + err.Error()) @@ -184,23 +186,23 @@ func AfterRunPTPCommandE810(data *interface{}, nodeProfile *ptpv1.PtpProfile, co case "gpspipe": glog.Infof("AfterRunPTPCommandE810 doing ublx config for command: %s", command) // Execute user-supplied UblxCmds first: - *pluginData.hwplugins = append(*pluginData.hwplugins, e810Opts.UblxCmds.runAll()...) + pluginData.hwplugins = append(pluginData.hwplugins, e810Opts.UblxCmds.runAll()...) // Finish with the default commands: - *pluginData.hwplugins = append(*pluginData.hwplugins, defaultUblxCmds().runAll()...) + pluginData.hwplugins = append(pluginData.hwplugins, defaultUblxCmds().runAll()...) case "tbc-ho-exit": - _, err = clockChain.EnterNormalTBC() + err = clockChain.EnterNormalTBC() if err != nil { return fmt.Errorf("e810: failed to enter T-BC normal mode") } glog.Info("e810: enter T-BC normal mode") case "tbc-ho-entry": - _, err = clockChain.EnterHoldoverTBC() + err = clockChain.EnterHoldoverTBC() if err != nil { return fmt.Errorf("e810: failed to enter T-BC holdover") } glog.Info("e810: enter T-BC holdover") case "reset-to-default": - _, err = clockChain.SetPinDefaults() + err = clockChain.SetPinDefaults() if err != nil { return fmt.Errorf("e810: failed to reset pins to default") } @@ -213,81 +215,31 @@ func AfterRunPTPCommandE810(data *interface{}, nodeProfile *ptpv1.PtpProfile, co return nil } -func PopulateHwConfigE810(data *interface{}, hwconfigs *[]ptpv1.HwConfig) error { - //hwConfig := ptpv1.HwConfig{} - //hwConfig.DeviceID = "e810" - //*hwconfigs = append(*hwconfigs, hwConfig) - if data != nil { - _data := *data - pluginData := _data.(*E810PluginData) - _pluginData := *pluginData - if _pluginData.hwplugins != nil { - for _, _hwconfig := range *_pluginData.hwplugins { - hwConfig := ptpv1.HwConfig{} - hwConfig.DeviceID = "e810" - hwConfig.Status = _hwconfig - *hwconfigs = append(*hwconfigs, hwConfig) - } - } - } - return nil -} - func E810(name string) (*plugin.Plugin, *interface{}) { - if name != "e810" { + if name != pluginNameE810 { glog.Errorf("Plugin must be initialized as 'e810'") return nil, nil } glog.Infof("registering e810 plugin") - hwplugins := []string{} - pluginData := E810PluginData{hwplugins: &hwplugins} + pluginData := E810PluginData{ + PluginData: PluginData{name: pluginNameE810}, + } _plugin := plugin.Plugin{ - Name: "e810", + Name: pluginNameE810, OnPTPConfigChange: OnPTPConfigChangeE810, AfterRunPTPCommand: AfterRunPTPCommandE810, - PopulateHwConfig: PopulateHwConfigE810, + PopulateHwConfig: pluginData.PopulateHwConfig, } var iface interface{} = &pluginData return &_plugin, &iface } -func getClockIDE810(device string) uint64 { - const ( - PCI_EXT_CAP_ID_DSN = 3 - PCI_CFG_SPACE_SIZE = 256 - PCI_EXT_CAP_NEXT_OFFSET = 2 - PCI_EXT_CAP_OFFSET_SHIFT = 4 - PCI_EXT_CAP_DATA_OFFSET = 4 - ) - b, err := os.ReadFile(fmt.Sprintf("/sys/class/net/%s/device/config", device)) - if err != nil { - glog.Error(err) - return 0 - } - // Extended capability space starts right on PCI_CFG_SPACE - var offset uint16 = PCI_CFG_SPACE_SIZE - var id uint16 - for { - id = binary.LittleEndian.Uint16(b[offset:]) - if id != PCI_EXT_CAP_ID_DSN { - if id == 0 { - glog.Errorf("can't find DSN for device %s", device) - return 0 - } - offset = binary.LittleEndian.Uint16(b[offset+PCI_EXT_CAP_NEXT_OFFSET:]) >> PCI_EXT_CAP_OFFSET_SHIFT - continue +func pinSetHasSMAInput(pins pinSet) bool { + for label, value := range pins { + if (label == "SMA1" || label == "SMA2") && + strings.HasPrefix(strings.TrimSpace(value), "1") { + return true } - break - } - return binary.LittleEndian.Uint64(b[offset+PCI_EXT_CAP_DATA_OFFSET:]) -} - -func loadPins(path string) (*[]dpll_netlink.PinInfo, error) { - pins := &[]dpll_netlink.PinInfo{} - ptext, err := os.ReadFile(path) - if err != nil { - return pins, err } - err = json.Unmarshal([]byte(ptext), pins) - return pins, err + return false } diff --git a/addons/intel/e810_test.go b/addons/intel/e810_test.go index ea052b15..0903bfe4 100644 --- a/addons/intel/e810_test.go +++ b/addons/intel/e810_test.go @@ -1,9 +1,13 @@ package intel import ( + "errors" + "fmt" + "os" "slices" "testing" + dpll "github.com/k8snetworkplumbingwg/linuxptp-daemon/pkg/dpll-netlink" ptpv1 "github.com/k8snetworkplumbingwg/ptp-operator/api/v1" "github.com/stretchr/testify/assert" ) @@ -19,7 +23,6 @@ func Test_E810(t *testing.T) { } func Test_AfterRunPTPCommandE810(t *testing.T) { - unitTest = true profile, err := loadProfile("./testdata/profile-tgm.yaml") assert.NoError(t, err) p, d := E810("e810") @@ -55,7 +58,416 @@ func Test_AfterRunPTPCommandE810(t *testing.T) { } assert.Equal(t, requiredUblxCmds, found) // And expect 3 of them to have produced output (as specified in the profile) - assert.Equal(t, 3, len(*data.hwplugins)) + assert.Equal(t, 3, len(data.hwplugins)) +} + +func Test_initInternalDelays(t *testing.T) { + delays, err := InitInternalDelays("E810-XXVDA4T") + assert.NoError(t, err) + assert.Equal(t, "E810-XXVDA4T", delays.PartType) + assert.Len(t, delays.ExternalInputs, 3) + assert.Len(t, delays.ExternalOutputs, 3) +} + +func Test_initInternalDelays_BadPart(t *testing.T) { + _, err := InitInternalDelays("Dummy") + assert.Error(t, err) +} + +func Test_ProcessProfileTGMNew(t *testing.T) { + _, restorePins := setupMockDPLLPinsFromJSON("./testdata/dpll-pins.json") + defer restorePins() + restoreDelay := setupMockDelayCompensation() + defer restoreDelay() + mockPinSet, restorePinSet := setupBatchPinSetMock() + defer restorePinSet() + profile, err := loadProfile("./testdata/profile-tgm.yaml") + assert.NoError(t, err) + p, d := E810("e810") + + mockFs, restoreFs := setupMockFS() + defer restoreFs() + mockClockIDsFromProfile(mockFs, profile) + + err = p.OnPTPConfigChange(d, profile) + assert.NoError(t, err) + assert.NotNil(t, mockPinSet.commands, "Ensure clockChain.SetPinDefaults was called") +} + +// Test that the profile with no phase inputs is processed correctly +func Test_ProcessProfileTBCNoPhaseInputs(t *testing.T) { + _, restoreDPLLPins := setupMockDPLLPinsFromJSON("./testdata/dpll-pins.json") + defer restoreDPLLPins() + restoreDelay := setupMockDelayCompensation() + defer restoreDelay() + mockPinSet, restorePinSet := setupBatchPinSetMock() + defer restorePinSet() + + // Setup filesystem mock for TBC profile - EnableE810Outputs needs this + mockFS, restoreFs := setupMockFS() + defer restoreFs() + + // mockPins + mockPinConfig, restorePins := setupMockPinConfig() + defer restorePins() + + phcEntries := []os.DirEntry{MockDirEntry{name: "ptp0", isDir: true}} + + // EnableE810Outputs reads the ptp directory and writes period (SMA2 is now via DPLL) + mockFS.AllowReadDir("/sys/class/net/ens4f0/device/ptp/", phcEntries, nil) + mockFS.AllowReadFile("/sys/class/net/ens4f0/device/ptp/ptp0/pins/SMA1", nil, os.ErrNotExist) + mockFS.ExpectWriteFile("/sys/class/net/ens4f0/device/ptp/ptp0/period", []byte("2 0 0 1 0"), os.FileMode(0o666), nil) + + profile, err := loadProfile("./testdata/profile-tbc-no-input-delays.yaml") + assert.NoError(t, err) + p, d := E810("e810") + + mockClockIDsFromProfile(mockFS, profile) + + err = p.OnPTPConfigChange(d, profile) + assert.NoError(t, err) + assert.Equal(t, 0, mockPinConfig.actualPinSetCount) + assert.Equal(t, 0, mockPinConfig.actualPinFrqCount) + + // Verify that clockChain was initialized (SetPinDefaults is called as part of InitClockChain) + // If SetPinDefaults wasn't called, InitClockChain would have failed + assert.NotNil(t, clockChain, "clockChain should be initialized") + ccData := clockChain.(*ClockChain) + assert.Equal(t, ClockTypeTBC, ccData.Type, "clockChain should be T-BC type") + assert.NotNil(t, mockPinSet.commands, "Ensure clockChain.SetPinDefaults was called") + + // Verify all expected filesystem calls were made + mockFS.VerifyAllCalls(t) +} + +func Test_ProcessProfileTGMOld(t *testing.T) { + _, restorePins := setupMockDPLLPinsFromJSON("./testdata/dpll-pins.json") + defer restorePins() + restoreDelay := setupMockDelayCompensation() + defer restoreDelay() + mockPinSet, restorePinSet := setupBatchPinSetMock() + defer restorePinSet() + profile, err := loadProfile("./testdata/profile-tgm-old.yaml") + assert.NoError(t, err) + p, d := E810("e810") + + mockFS, restoreFs := setupMockFS() + defer restoreFs() + mockClockIDsFromProfile(mockFS, profile) + + err = p.OnPTPConfigChange(d, profile) + assert.NoError(t, err) + assert.NotNil(t, mockPinSet.commands, "Ensure some pins were set") +} + +func TestEnableE810Outputs(t *testing.T) { + mockPinSet, restorePinSet := setupBatchPinSetMock() + defer restorePinSet() + + sma2Pin := dpll.PinInfo{ + ID: 10, + ClockID: 1000, + BoardLabel: "SMA2", + Type: dpll.PinTypeEXT, + Capabilities: dpll.PinCapDir | dpll.PinCapPrio | dpll.PinCapState, + ParentDevice: []dpll.PinParentDevice{ + {ParentID: 1, Direction: dpll.PinDirectionOutput}, + {ParentID: 2, Direction: dpll.PinDirectionOutput}, + }, + } + + tests := []struct { + name string + setupMock func(*MockFileSystem) + clockChain *ClockChain + expectedError string + }{ + { + name: "DPLL path - no sysfs SMA pins", + clockChain: &ClockChain{ + LeadingNIC: CardInfo{Name: "ens4f0", DpllClockID: 1000}, + DpllPins: &mockedDPLLPins{pins: dpllPins{&sma2Pin}}, + }, + setupMock: func(m *MockFileSystem) { + phcEntries := []os.DirEntry{MockDirEntry{name: "ptp0", isDir: true}} + m.AllowReadDir("/sys/class/net/ens4f0/device/ptp/", phcEntries, nil) + m.ExpectReadFile("/sys/class/net/ens4f0/device/ptp/ptp0/pins/SMA1", nil, os.ErrNotExist) + m.ExpectWriteFile("/sys/class/net/ens4f0/device/ptp/ptp0/period", []byte("2 0 0 1 0"), os.FileMode(0o666), nil) + }, + expectedError: "", + }, + { + name: "Sysfs path - SMA pins available", + clockChain: &ClockChain{ + LeadingNIC: CardInfo{Name: "ens4f0", DpllClockID: 1000}, + DpllPins: &mockedDPLLPins{pins: dpllPins{&sma2Pin}}, + }, + setupMock: func(m *MockFileSystem) { + phcEntries := []os.DirEntry{MockDirEntry{name: "ptp0", isDir: true}} + m.AllowReadDir("/sys/class/net/ens4f0/device/ptp/", phcEntries, nil) + m.ExpectReadFile("/sys/class/net/ens4f0/device/ptp/ptp0/pins/SMA1", []byte("0 1"), nil) + m.ExpectWriteFile("/sys/class/net/ens4f0/device/ptp/ptp0/pins/SMA2", []byte{}, os.FileMode(0o666), nil) + m.ExpectWriteFile("/sys/class/net/ens4f0/device/ptp/ptp0/period", []byte("2 0 0 1 0"), os.FileMode(0o666), nil) + }, + expectedError: "", + }, + { + name: "Sysfs path - SMA2 write fails", + clockChain: &ClockChain{ + LeadingNIC: CardInfo{Name: "ens4f0", DpllClockID: 1000}, + DpllPins: &mockedDPLLPins{pins: dpllPins{&sma2Pin}}, + }, + setupMock: func(m *MockFileSystem) { + phcEntries := []os.DirEntry{MockDirEntry{name: "ptp0", isDir: true}} + m.AllowReadDir("/sys/class/net/ens4f0/device/ptp/", phcEntries, nil) + m.ExpectReadFile("/sys/class/net/ens4f0/device/ptp/ptp0/pins/SMA1", []byte("0 1"), nil) + m.ExpectWriteFile("/sys/class/net/ens4f0/device/ptp/ptp0/pins/SMA2", []byte{}, os.FileMode(0o666), errors.New("SMA2 write failed")) + m.ExpectWriteFile("/sys/class/net/ens4f0/device/ptp/ptp0/period", []byte("2 0 0 1 0"), os.FileMode(0o666), nil) + }, + expectedError: "", + }, + { + name: "ReadDir fails", + clockChain: &ClockChain{ + LeadingNIC: CardInfo{Name: "ens4f0", DpllClockID: 1000}, + DpllPins: &mockedDPLLPins{pins: dpllPins{&sma2Pin}}, + }, + setupMock: func(m *MockFileSystem) { + m.ExpectReadDir("/sys/class/net/ens4f0/device/ptp/", []os.DirEntry{}, errors.New("permission denied")) + }, + expectedError: "e810 failed to read /sys/class/net/ens4f0/device/ptp/: permission denied", + }, + { + name: "No PHC directories found", + clockChain: &ClockChain{ + LeadingNIC: CardInfo{Name: "ens4f0", DpllClockID: 1000}, + DpllPins: &mockedDPLLPins{pins: dpllPins{&sma2Pin}}, + }, + setupMock: func(m *MockFileSystem) { + m.ExpectReadDir("/sys/class/net/ens4f0/device/ptp/", []os.DirEntry{}, nil) + }, + expectedError: "e810 cards should have one PHC per NIC, but ens4f0 has 0", + }, + { + name: "Multiple PHC directories (warning case)", + clockChain: &ClockChain{ + LeadingNIC: CardInfo{Name: "ens4f0", DpllClockID: 1000}, + DpllPins: &mockedDPLLPins{pins: dpllPins{&sma2Pin}}, + }, + setupMock: func(m *MockFileSystem) { + phcEntries := []os.DirEntry{ + MockDirEntry{name: "ptp0", isDir: true}, + MockDirEntry{name: "ptp1", isDir: true}, + } + m.AllowReadDir("/sys/class/net/ens4f0/device/ptp/", phcEntries, nil) + m.ExpectReadFile("/sys/class/net/ens4f0/device/ptp/ptp0/pins/SMA1", nil, os.ErrNotExist) + m.ExpectWriteFile("/sys/class/net/ens4f0/device/ptp/ptp0/period", []byte("2 0 0 1 0"), os.FileMode(0o666), nil) + }, + expectedError: "", + }, + { + name: "Period write fails - should not return error but log", + clockChain: &ClockChain{ + LeadingNIC: CardInfo{Name: "ens4f0", DpllClockID: 1000}, + DpllPins: &mockedDPLLPins{pins: dpllPins{&sma2Pin}}, + }, + setupMock: func(m *MockFileSystem) { + phcEntries := []os.DirEntry{MockDirEntry{name: "ptp0", isDir: true}} + m.AllowReadDir("/sys/class/net/ens4f0/device/ptp/", phcEntries, nil) + m.ExpectReadFile("/sys/class/net/ens4f0/device/ptp/ptp0/pins/SMA1", nil, os.ErrNotExist) + m.ExpectWriteFile("/sys/class/net/ens4f0/device/ptp/ptp0/period", []byte("2 0 0 1 0"), os.FileMode(0o666), errors.New("period write failed")) + }, + expectedError: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mockPinSet.reset() + DpllPins = &mockedDPLLPins{pins: dpllPins{&sma2Pin}} + + // Setup mock filesystem + mockFS, restoreFs := setupMockFS() + defer restoreFs() + tt.setupMock(mockFS) + + // Execute function + err := tt.clockChain.EnableE810Outputs() + + // Check error + if tt.expectedError != "" { + assert.Error(t, err) + assert.Contains(t, err.Error(), tt.expectedError) + } else { + assert.NoError(t, err) + } + + // Verify all expected calls were made + mockFS.VerifyAllCalls(t) + }) + } +} + +func Test_AfterRunPTPCommandE810ClockChain(t *testing.T) { + profile, err := loadProfile("./testdata/profile-tgm.yaml") + assert.NoError(t, err) + p, d := E810("e810") + + err = p.AfterRunPTPCommand(d, profile, "bad command") + assert.NoError(t, err) + + mClockChain := &mockClockChain{} + clockChain = mClockChain + err = p.AfterRunPTPCommand(d, profile, "reset-to-default") + assert.NoError(t, err) + mClockChain.assertCallCounts(t, 0, 0, 1) + + mClockChain.returnErr = fmt.Errorf("Fake error") + err = p.AfterRunPTPCommand(d, profile, "reset-to-default") + assert.Error(t, err) + mClockChain.assertCallCounts(t, 0, 0, 2) + + mClockChain = &mockClockChain{} + clockChain = mClockChain + err = p.AfterRunPTPCommand(d, profile, "tbc-ho-entry") + assert.NoError(t, err) + mClockChain.assertCallCounts(t, 0, 1, 0) + mClockChain.returnErr = fmt.Errorf("Fake error") + err = p.AfterRunPTPCommand(d, profile, "tbc-ho-entry") + assert.Error(t, err) + mClockChain.assertCallCounts(t, 0, 2, 0) + + mClockChain = &mockClockChain{} + clockChain = mClockChain + err = p.AfterRunPTPCommand(d, profile, "tbc-ho-exit") + assert.NoError(t, err) + mClockChain.assertCallCounts(t, 1, 0, 0) + mClockChain.returnErr = fmt.Errorf("Fake error") + err = p.AfterRunPTPCommand(d, profile, "tbc-ho-exit") + assert.Error(t, err) + mClockChain.assertCallCounts(t, 2, 0, 0) +} + +func TestPinSetHasSMAInput(t *testing.T) { + tests := []struct { + name string + pins pinSet + expected bool + }{ + { + name: "SMA1 input", + pins: pinSet{"SMA1": "1 1"}, + expected: true, + }, + { + name: "SMA2 input", + pins: pinSet{"SMA2": "1 2"}, + expected: true, + }, + { + name: "SMA1 input with leading spaces", + pins: pinSet{"SMA1": " 1 1 "}, + expected: true, + }, + { + name: "SMA1 disabled", + pins: pinSet{"SMA1": "0 1"}, + expected: false, + }, + { + name: "SMA2 output", + pins: pinSet{"SMA2": "2 2"}, + expected: false, + }, + { + name: "no SMA pins", + pins: pinSet{"U.FL1": "1 1"}, + expected: false, + }, + { + name: "empty pinset", + pins: pinSet{}, + expected: false, + }, + { + name: "both SMA disabled", + pins: pinSet{"SMA1": "0 1", "SMA2": "0 2"}, + expected: false, + }, + { + name: "SMA1 disabled but SMA2 input", + pins: pinSet{"SMA1": "0 1", "SMA2": "1 2"}, + expected: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.expected, pinSetHasSMAInput(tt.pins)) + }) + } +} + +func TestDevicePins_DPLL_SMAInput_SetsGNSSPriority(t *testing.T) { + sma1Pin := makeTwoParentPin(1, "SMA1", 1000, + dpll.PinDirectionInput, dpll.PinDirectionInput) + gnssPin := makeTwoParentPin(2, "GNSS-1PPS", 1000, + dpll.PinDirectionInput, dpll.PinDirectionInput) + pins := makePins(sma1Pin, gnssPin) + + ps := pinSet{"SMA1": "1 1"} + commands := pins.GetCommandsForPluginPinSet(1000, ps) + + if pinSetHasSMAInput(ps) { + gnssPinInfo := pins.GetByLabel("GNSS-1PPS", 1000) + if gnssPinInfo != nil { + gnssCommands := SetPinControlData(*gnssPinInfo, PinParentControl{ + EecPriority: 4, + PpsPriority: 4, + }) + commands = append(commands, gnssCommands...) + } + } + + gnssFound := false + for _, cmd := range commands { + if cmd.ID == 2 { + gnssFound = true + assert.Len(t, cmd.PinParentCtl, 2) + for _, pc := range cmd.PinParentCtl { + assert.NotNil(t, pc.Prio) + assert.Equal(t, uint32(4), *pc.Prio) + } + } + } + assert.True(t, gnssFound, "GNSS-1PPS command should be present") +} + +func TestDevicePins_DPLL_NoSMAInput_NoGNSSCommand(t *testing.T) { + sma2Pin := makeTwoParentPin(1, "SMA2", 1000, + dpll.PinDirectionOutput, dpll.PinDirectionOutput) + gnssPin := makeTwoParentPin(2, "GNSS-1PPS", 1000, + dpll.PinDirectionInput, dpll.PinDirectionInput) + pins := makePins(sma2Pin, gnssPin) + + ps := pinSet{"SMA2": "2 2"} + commands := pins.GetCommandsForPluginPinSet(1000, ps) + + if pinSetHasSMAInput(ps) { + gnssPinInfo := pins.GetByLabel("GNSS-1PPS", 1000) + if gnssPinInfo != nil { + gnssCommands := SetPinControlData(*gnssPinInfo, PinParentControl{ + EecPriority: 4, + PpsPriority: 4, + }) + commands = append(commands, gnssCommands...) + } + } + + for _, cmd := range commands { + assert.NotEqual(t, uint32(2), cmd.ID, + "GNSS-1PPS command should NOT be present when SMA is output") + } } func Test_PopulateHwConfdigE810(t *testing.T) { @@ -69,7 +481,7 @@ func Test_PopulateHwConfdigE810(t *testing.T) { assert.NoError(t, err) assert.Equal(t, 0, len(output)) - data.hwplugins = &[]string{"A", "B", "C"} + data.hwplugins = []string{"A", "B", "C"} err = p.PopulateHwConfig(d, &output) assert.NoError(t, err) assert.Equal(t, []ptpv1.HwConfig{ diff --git a/addons/intel/e825.go b/addons/intel/e825.go index 6d91059d..09ffa69e 100644 --- a/addons/intel/e825.go +++ b/addons/intel/e825.go @@ -1,12 +1,9 @@ package intel import ( - "encoding/binary" "encoding/json" "errors" "fmt" - "os" - "slices" "strconv" "strings" @@ -45,49 +42,21 @@ var bcDpllPeriods = frqSet{ // E825Opts is the options structure for e825 plugin type E825Opts struct { - UblxCmds UblxCmdList `json:"ublxCmds"` - Devices []string `json:"devices"` - DevicePins map[string]pinSet `json:"pins"` - DeviceFreqencies map[string]frqSet `json:"frequencies"` - DpllSettings map[string]uint64 `json:"settings"` - PhaseOffsetPins map[string]map[string]string `json:"phaseOffsetPins"` - Gnss GnssOptions `json:"gnss"` -} - -// GnssOptions defines GNSS-specific options for the e825 -type GnssOptions struct { - Disabled bool `json:"disabled"` -} - -// allDevices enumerates all defined devices (Devices/DevicePins/DeviceFrequencies/PhaseOffsets) -func (opts *E825Opts) allDevices() []string { - // Enumerate all defined devices (Devices/DevicePins/DeviceFrequencies) - allDevices := opts.Devices - allDevices = extendWithKeys(allDevices, opts.DevicePins) - allDevices = extendWithKeys(allDevices, opts.DeviceFreqencies) - allDevices = extendWithKeys(allDevices, opts.PhaseOffsetPins) - return allDevices + PluginOpts + UblxCmds UblxCmdList `json:"ublxCmds"` + Gnss GnssOptions `json:"gnss"` } // E825PluginData is the data structure for e825 plugin type E825PluginData struct { - hwplugins *[]string - dpllPins []*dpll_netlink.PinInfo + PluginData + dpllPins []*dpll_netlink.PinInfo } func tbcConfigured(nodeProfile *ptpv1.PtpProfile) bool { return nodeProfile.PtpSettings["clockType"] == "T-BC" } -func extendWithKeys[T any](s []string, m map[string]T) []string { - for key := range m { - if !slices.Contains(s, key) { - s = append(s, key) - } - } - return s -} - // OnPTPConfigChangeE825 performs actions on PTP config change for e825 plugin func OnPTPConfigChangeE825(data *interface{}, nodeProfile *ptpv1.PtpProfile) error { pluginData := (*data).(*E825PluginData) @@ -113,7 +82,6 @@ func OnPTPConfigChangeE825(data *interface{}, nodeProfile *ptpv1.PtpProfile) err if err != nil { glog.Error("e825 failed to unmarshal opts: " + err.Error()) } - allDevices := e825Opts.allDevices() glog.Infof("Initializing e825 plugin for profile %s and devices %v", *nodeProfile.Name, allDevices) @@ -126,7 +94,7 @@ func OnPTPConfigChangeE825(data *interface{}, nodeProfile *ptpv1.PtpProfile) err dpllClockIDStr := fmt.Sprintf("%s[%s]", dpll.ClockIdStr, device) clkID := zlClockID if zlErr != nil { - clkID = getClockIDE825(device) + clkID = getClockID(device) } (*nodeProfile).PtpSettings[dpllClockIDStr] = strconv.FormatUint(clkID, 10) glog.Infof("Detected %s=%d (%x)", dpllClockIDStr, clkID, clkID) @@ -160,7 +128,7 @@ func OnPTPConfigChangeE825(data *interface{}, nodeProfile *ptpv1.PtpProfile) err if zlErr == nil { clockIDUsed = zlClockID } else { - clockIDUsed = getClockIDE825(iface) + clockIDUsed = getClockID(iface) } key := strings.Join([]string{iface, "phaseOffsetFilter", strconv.FormatUint(clockIDUsed, 10), pinProperty}, ".") (*nodeProfile).PtpSettings[key] = value @@ -192,9 +160,6 @@ func OnPTPConfigChangeE825(data *interface{}, nodeProfile *ptpv1.PtpProfile) err // populateDpllPins creates a list of all known DPLL pins func (d *E825PluginData) populateDpllPins() error { - if unitTest { - return nil - } conn, err := dpll_netlink.Dial(nil) if err != nil { return fmt.Errorf("failed to dial DPLL: %w", err) @@ -207,9 +172,6 @@ func (d *E825PluginData) populateDpllPins() error { return nil } -// Setup mockable pin setting function -var e825DoPinSet = BatchPinSet - // pinCmdSetState sets the state of an individual DPLL pin func pinCmdSetState(pin *dpll_netlink.PinInfo, connectable bool) dpll_netlink.PinParentDeviceCtl { newState := uint32(dpll_netlink.PinStateSelectable) @@ -257,7 +219,7 @@ func (d *E825PluginData) setupGnss(gnss GnssOptions) error { return errors.New("no GNSS pins found") } glog.Infof("Will %s %d GNSS pins: %v", action, len(commands), affectedPins) - return e825DoPinSet(&commands) + return BatchPinSet(&commands) } // AfterRunPTPCommandE825 performs actions after certain PTP commands for e825 plugin @@ -280,9 +242,9 @@ func AfterRunPTPCommandE825(data *interface{}, nodeProfile *ptpv1.PtpProfile, co case "gpspipe": glog.Infof("AfterRunPTPCommandE825 doing ublx config for command: %s", command) // Execute user-supplied UblxCmds first: - *pluginData.hwplugins = append(*pluginData.hwplugins, e825Opts.UblxCmds.runAll()...) + pluginData.hwplugins = append(pluginData.hwplugins, e825Opts.UblxCmds.runAll()...) // Finish with the default commands: - *pluginData.hwplugins = append(*pluginData.hwplugins, defaultUblxCmds().runAll()...) + pluginData.hwplugins = append(pluginData.hwplugins, defaultUblxCmds().runAll()...) // "tbc-ho-exit" is called when ptp4l sync is achieved on the T-BC upstreamPort case "tbc-ho-exit": if tbcConfigured(nodeProfile) { @@ -317,100 +279,22 @@ func AfterRunPTPCommandE825(data *interface{}, nodeProfile *ptpv1.PtpProfile, co return nil } -// PopulateHwConfigE825 populates hwconfig for e825 plugin -func PopulateHwConfigE825(data *interface{}, hwconfigs *[]ptpv1.HwConfig) error { - if data != nil { - _data := *data - pluginData := _data.(*E825PluginData) - _pluginData := *pluginData - if _pluginData.hwplugins != nil { - for _, _hwconfig := range *_pluginData.hwplugins { - hwConfig := ptpv1.HwConfig{} - hwConfig.DeviceID = "e825" - hwConfig.Status = _hwconfig - *hwconfigs = append(*hwconfigs, hwConfig) - } - } - } - return nil -} - // E825 initializes the e825 plugin func E825(name string) (*plugin.Plugin, *interface{}) { - if name != "e825" { + if name != pluginNameE825 { glog.Errorf("Plugin must be initialized as 'e825'") return nil, nil } glog.Infof("registering e825 plugin") - hwplugins := []string{} - pluginData := E825PluginData{hwplugins: &hwplugins} + pluginData := E825PluginData{ + PluginData: PluginData{name: pluginNameE825}, + } _plugin := plugin.Plugin{ - Name: "e825", + Name: pluginNameE825, OnPTPConfigChange: OnPTPConfigChangeE825, AfterRunPTPCommand: AfterRunPTPCommandE825, - PopulateHwConfig: PopulateHwConfigE825, + PopulateHwConfig: pluginData.PopulateHwConfig, } var iface interface{} = &pluginData return &_plugin, &iface } - -func getClockIDE825(device string) uint64 { - const ( - PCI_EXT_CAP_ID_DSN = 3 //nolint - PCI_CFG_SPACE_SIZE = 256 //nolint - PCI_EXT_CAP_NEXT_OFFSET = 2 //nolint - PCI_EXT_CAP_OFFSET_SHIFT = 4 //nolint - PCI_EXT_CAP_DATA_OFFSET = 4 //nolint - ) - b, err := os.ReadFile(fmt.Sprintf("/sys/class/net/%s/device/config", device)) - if err != nil { - glog.Error(err) - return 0 - } - // Extended capability space starts right on PCI_CFG_SPACE - var offset uint16 = PCI_CFG_SPACE_SIZE - var id uint16 - for { - id = binary.LittleEndian.Uint16(b[offset:]) - if id != PCI_EXT_CAP_ID_DSN { - if id == 0 { - glog.Errorf("can't find DSN for device %s", device) - return 0 - } - offset = binary.LittleEndian.Uint16(b[offset+PCI_EXT_CAP_NEXT_OFFSET:]) >> PCI_EXT_CAP_OFFSET_SHIFT - continue - } - break - } - return binary.LittleEndian.Uint64(b[offset+PCI_EXT_CAP_DATA_OFFSET:]) -} - -// getClockIDByModule returns ClockID for a given DPLL module name, preferring PPS type if present -func getClockIDByModule(module string) (uint64, error) { - if unitTest { - return 0, fmt.Errorf("netlink disabled in unit test") - } - conn, err := dpll_netlink.Dial(nil) - if err != nil { - return 0, err - } - //nolint:errcheck - defer conn.Close() - devices, err := conn.DumpDeviceGet() - if err != nil { - return 0, err - } - var anyID uint64 - for _, d := range devices { - if strings.EqualFold(d.ModuleName, module) { - if d.Type == 1 { // PPS - return d.ClockID, nil - } - anyID = d.ClockID - } - } - if anyID != 0 { - return anyID, nil - } - return 0, fmt.Errorf("module %s DPLL not found", module) -} diff --git a/addons/intel/e825_test.go b/addons/intel/e825_test.go index 3badc837..d6cc180a 100644 --- a/addons/intel/e825_test.go +++ b/addons/intel/e825_test.go @@ -43,7 +43,6 @@ func Test_E825(t *testing.T) { } func Test_AfterRunPTPCommandE825(t *testing.T) { - unitTest = true profile, err := loadProfile("./testdata/e825-tgm.yaml") assert.NoError(t, err) p, d := E825("e825") @@ -79,7 +78,7 @@ func Test_AfterRunPTPCommandE825(t *testing.T) { } assert.Equal(t, requiredUblxCmds, found) // And expect 3 of them to have produced output (as specified in the profile) - assert.Equal(t, 3, len(*data.hwplugins)) + assert.Equal(t, 3, len(data.hwplugins)) } func Test_AfterRunPTPCommandE825_TBC(t *testing.T) { @@ -141,7 +140,7 @@ func Test_PopulateHwConfdigE825(t *testing.T) { assert.NoError(t, err) assert.Equal(t, 0, len(output)) - data.hwplugins = &[]string{"A", "B", "C"} + data.hwplugins = []string{"A", "B", "C"} err = p.PopulateHwConfig(d, &output) assert.NoError(t, err) assert.Equal(t, []ptpv1.HwConfig{ @@ -161,17 +160,7 @@ func Test_PopulateHwConfdigE825(t *testing.T) { output) } -type mockBatchPinSet struct { - commands *[]dpll.PinParentDeviceCtl -} - -func (m *mockBatchPinSet) mock(commands *[]dpll.PinParentDeviceCtl) error { - m.commands = commands - return nil -} - func Test_setupGnss(t *testing.T) { - unitTest = true tcs := []struct { name string gnss GnssOptions @@ -265,8 +254,8 @@ func Test_setupGnss(t *testing.T) { } for _, tc := range tcs { t.Run(tc.name, func(tt *testing.T) { - mockPinSet := mockBatchPinSet{} - e825DoPinSet = mockPinSet.mock + mockPinSet, restorePinSet := setupBatchPinSetMock() + defer restorePinSet() data := E825PluginData{ dpllPins: tc.dpll, } @@ -327,7 +316,6 @@ func Test_OnPTPConfigChangeE825(t *testing.T) { } for _, tc := range tcs { t.Run(tc.name, func(tt *testing.T) { - unitTest = true mockPins, restorePins := setupMockPinConfig() defer restorePins() profile, err := loadProfile(tc.profile) @@ -336,6 +324,9 @@ func Test_OnPTPConfigChangeE825(t *testing.T) { } assert.NoError(tt, err) p, d := E825("e825") + data := (*d).(*E825PluginData) + mockDpllPinset, restoreDpllPins := setupGNSSMocks(data) + defer restoreDpllPins() err = p.OnPTPConfigChange(d, profile) if tc.expectError { assert.Error(tt, err) @@ -343,6 +334,7 @@ func Test_OnPTPConfigChangeE825(t *testing.T) { assert.NoError(tt, err) assert.Equal(tt, tc.expectedPinSets, mockPins.actualPinSetCount) assert.Equal(tt, tc.expectedPinFrqs, mockPins.actualPinFrqCount) + assert.Equal(tt, 1, len(*mockDpllPinset.commands)) } }) } diff --git a/addons/intel/e830.go b/addons/intel/e830.go index c7420107..7975174c 100644 --- a/addons/intel/e830.go +++ b/addons/intel/e830.go @@ -9,6 +9,7 @@ import ( "github.com/golang/glog" "github.com/k8snetworkplumbingwg/linuxptp-daemon/pkg/dpll" + dpll_netlink "github.com/k8snetworkplumbingwg/linuxptp-daemon/pkg/dpll-netlink" "github.com/k8snetworkplumbingwg/linuxptp-daemon/pkg/plugin" ptpv1 "github.com/k8snetworkplumbingwg/ptp-operator/api/v1" ) @@ -17,27 +18,46 @@ var pluginNameE830 = "e830" // E830Opts is the options for e830 plugin type E830Opts struct { - Devices []string `json:"devices"` - DevicePins map[string]pinSet `json:"pins"` - DeviceFreqencies map[string]frqSet `json:"frequencies"` - DpllSettings map[string]uint64 `json:"settings"` - PhaseOffsetPins map[string]map[string]string `json:"phaseOffsetPins"` -} - -// allDevices enumerates all defined devices (Devices/DevicePins/DeviceFrequencies/PhaseOffsets) -func (opts *E830Opts) allDevices() []string { - allDevices := opts.Devices - allDevices = extendWithKeys(allDevices, opts.DevicePins) - allDevices = extendWithKeys(allDevices, opts.DeviceFreqencies) - allDevices = extendWithKeys(allDevices, opts.PhaseOffsetPins) - return allDevices + PluginOpts } // E830PluginData is the plugin data for e830 plugin type E830PluginData struct { - hwplugins *[]string + PluginData +} + +func _hasDpllForClockID(clockID uint64) bool { + if clockID == 0 { + return false + } + conn, err := dpll_netlink.Dial(nil) + if err != nil { + glog.Warningf("failed to dial DPLL: %s", err) + return false + } + defer conn.Close() + + devices, err := conn.DumpDeviceGet() + if err != nil || devices == nil { + glog.Infof("No DPLLs found on this system") + return false + } + found := false + for _, device := range devices { + if device.ClockID == clockID { + glog.Infof("Detected %s DPLL for clock ID %#x", dpll_netlink.GetDpllType(device.Type), clockID) + found = true + } + } + if !found { + glog.Infof("No DPLL detected for clock ID %#x", clockID) + } + return found } +// Function pointer for mocking +var hasDpllForClockID = _hasDpllForClockID + // OnPTPConfigChangeE830 is called on PTP config change for e830 plugin func OnPTPConfigChangeE830(_ *interface{}, nodeProfile *ptpv1.PtpProfile) error { glog.Infof("calling onPTPConfigChange for e830 plugin (%s)", *nodeProfile.Name) @@ -62,8 +82,10 @@ func OnPTPConfigChangeE830(_ *interface{}, nodeProfile *ptpv1.PtpProfile) error glog.Infof("Initializing e830 plugin for profile %s and devices %v", *nodeProfile.Name, allDevices) // Setup clockID (prefer ice modue clock ID for e830) + clockIDs := make(map[string]uint64) for _, device := range allDevices { - clockID := getClockIDE810(device) + clockID := getClockID(device) + clockIDs[device] = clockID dpllClockIDStr := fmt.Sprintf("%s[%s]", dpll.ClockIdStr, device) nodeProfile.PtpSettings[dpllClockIDStr] = strconv.FormatUint(clockID, 10) glog.Infof("e830: Detected %s=%d (%x)", dpllClockIDStr, clockID, clockID) @@ -111,15 +133,14 @@ func OnPTPConfigChangeE830(_ *interface{}, nodeProfile *ptpv1.PtpProfile) error } } - // e830 DPLL is inaccessible to software, so ensure the daemon ignores all e830 DPLLs for now: + // Setup DPLL flags for all e830 cards for _, device := range allDevices { - // Note: Only set to "true" if it's unset; This allows overriding this by explicitly setting dpll.$iface.ignore = "false" in the PtpConfig section - key := dpll.PtpSettingsDpllIgnoreKey(device) - if value, ok := nodeProfile.PtpSettings[key]; ok { - glog.Infof("Not setting %s (already \"%s\")", key, value) + if hasDpllForClockID(clockIDs[device]) { + // CF DPLL only provides PhaseStatus, not Phase Offset or Frequency Status: + nodeProfile.PtpSettings[dpll.PtpSettingsDpllFlagsKey(device)] = strconv.FormatUint(uint64(dpll.FlagOnlyPhaseStatus), 10) } else { - nodeProfile.PtpSettings[key] = "true" - glog.Infof("Setting %s = \"true\"", key) + // No DPLL found: Mark this device to be ignored + nodeProfile.PtpSettings[dpll.PtpSettingsDpllIgnoreKey(device)] = "true" } } } @@ -130,9 +151,6 @@ func OnPTPConfigChangeE830(_ *interface{}, nodeProfile *ptpv1.PtpProfile) error // AfterRunPTPCommandE830 is called after running ptp command for e830 plugin func AfterRunPTPCommandE830(_ *interface{}, _ *ptpv1.PtpProfile, _ string) error { return nil } -// PopulateHwConfigE830 populates hwconfig for e830 plugin -func PopulateHwConfigE830(_ *interface{}, _ *[]ptpv1.HwConfig) error { return nil } - // E830 initializes the e830 plugin func E830(name string) (*plugin.Plugin, *interface{}) { if name != pluginNameE830 { @@ -140,13 +158,14 @@ func E830(name string) (*plugin.Plugin, *interface{}) { return nil, nil } glog.Infof("registering e830 plugin") - hwplugins := []string{} - pluginData := E830PluginData{hwplugins: &hwplugins} + pluginData := E830PluginData{ + PluginData: PluginData{name: pluginNameE830}, + } _plugin := plugin.Plugin{ Name: pluginNameE830, OnPTPConfigChange: OnPTPConfigChangeE830, AfterRunPTPCommand: AfterRunPTPCommandE830, - PopulateHwConfig: PopulateHwConfigE830, + PopulateHwConfig: pluginData.PopulateHwConfig, } var iface interface{} = &pluginData return &_plugin, &iface diff --git a/addons/intel/e830_test.go b/addons/intel/e830_test.go index 2a665b45..5ed35ae3 100644 --- a/addons/intel/e830_test.go +++ b/addons/intel/e830_test.go @@ -22,6 +22,7 @@ func Test_OnPTPConfigChangeE830(t *testing.T) { tcs := []struct { name string profile string + foundDpll bool editProfile func(*ptpv1.PtpProfile) expectError bool expectedPinSets int @@ -29,44 +30,58 @@ func Test_OnPTPConfigChangeE830(t *testing.T) { expectedPtpSettings map[string]string }{ { - name: "TGM Profile", - profile: "./testdata/e825-tgm.yaml", + name: "TGM Profile", + profile: "./testdata/e825-tgm.yaml", + foundDpll: true, expectedPtpSettings: map[string]string{ - "dpll.enp108s0f0.ignore": "true", + "dpll.enp108s0f0.ignore": "", "clockId[enp108s0f0]": "0", + "dpll.enp108s0f0.flags": "5", }, }, { - name: "TBC Profile", - profile: "./testdata/e825-tbc.yaml", + name: "TBC Profile", + profile: "./testdata/e825-tbc.yaml", + foundDpll: true, expectedPtpSettings: map[string]string{ - "dpll.enp108s0f0.ignore": "true", + "dpll.enp108s0f0.ignore": "", "clockId[enp108s0f0]": "0", + "dpll.enp108s0f0.flags": "5", }, }, { - name: "TBC Profile with dpll-ignore-override", - profile: "./testdata/e825-tbc.yaml", - editProfile: func(p *ptpv1.PtpProfile) { - // Inject dpll.$iface.ignore to ensure it's not reset to "true" - p.PtpSettings["dpll.enp108s0f0.ignore"] = "false" + name: "TGM Profile (No DPLL)", + profile: "./testdata/e825-tgm.yaml", + foundDpll: false, + expectedPtpSettings: map[string]string{ + "dpll.enp108s0f0.ignore": "true", + "clockId[enp108s0f0]": "0", + "dpll.enp108s0f0.flags": "", }, + }, + { + name: "TBC Profile (No DPLL)", + profile: "./testdata/e825-tbc.yaml", + foundDpll: false, expectedPtpSettings: map[string]string{ - "dpll.enp108s0f0.ignore": "false", + "dpll.enp108s0f0.ignore": "true", "clockId[enp108s0f0]": "0", + "dpll.enp108s0f0.flags": "", }, }, { - name: "TBC with no leadingInterface", - profile: "./testdata/e825-tbc.yaml", + name: "TBC with no leadingInterface", + profile: "./testdata/e825-tbc.yaml", + foundDpll: true, editProfile: func(p *ptpv1.PtpProfile) { delete(p.PtpSettings, "leadingInterface") }, expectError: true, }, { - name: "TBC with no upstreamPort", - profile: "./testdata/e825-tbc.yaml", + name: "TBC with no upstreamPort", + profile: "./testdata/e825-tbc.yaml", + foundDpll: true, editProfile: func(p *ptpv1.PtpProfile) { delete(p.PtpSettings, "upstreamPort") }, @@ -75,9 +90,16 @@ func Test_OnPTPConfigChangeE830(t *testing.T) { } for _, tc := range tcs { t.Run(tc.name, func(tt *testing.T) { - unitTest = true + // Mock pin setup mockPins, restorePins := setupMockPinConfig() defer restorePins() + + // Mock DPLL detection + hasDpllForClockID = func(_ uint64) bool { + return tc.foundDpll + } + defer func() { hasDpllForClockID = _hasDpllForClockID }() + profile, err := loadProfile(tc.profile) if tc.editProfile != nil { tc.editProfile(profile) diff --git a/addons/intel/intel_test.go b/addons/intel/intel_test.go deleted file mode 100644 index 23d58447..00000000 --- a/addons/intel/intel_test.go +++ /dev/null @@ -1,433 +0,0 @@ -package intel - -import ( - "errors" - "os" - "testing" - - ptpv1 "github.com/k8snetworkplumbingwg/ptp-operator/api/v1" - "github.com/stretchr/testify/assert" - "sigs.k8s.io/yaml" -) - -// MockFileSystem is a simple mock implementation of FileSystemInterface -type MockFileSystem struct { - // Expected calls and responses - readDirCalls []ReadDirCall - writeFileCalls []WriteFileCall - currentReadDir int - currentWriteFile int -} - -type ReadDirCall struct { - expectedPath string - returnDirs []os.DirEntry - returnError error -} - -type WriteFileCall struct { - expectedPath string - expectedData []byte - expectedPerm os.FileMode - returnError error -} - -func (m *MockFileSystem) ExpectReadDir(path string, dirs []os.DirEntry, err error) { - m.readDirCalls = append(m.readDirCalls, ReadDirCall{ - expectedPath: path, - returnDirs: dirs, - returnError: err, - }) -} - -func (m *MockFileSystem) ExpectWriteFile(path string, data []byte, perm os.FileMode, err error) { - m.writeFileCalls = append(m.writeFileCalls, WriteFileCall{ - expectedPath: path, - expectedData: data, - expectedPerm: perm, - returnError: err, - }) -} - -func (m *MockFileSystem) ReadDir(dirname string) ([]os.DirEntry, error) { - if m.currentReadDir >= len(m.readDirCalls) { - return nil, errors.New("unexpected ReadDir call") - } - call := m.readDirCalls[m.currentReadDir] - m.currentReadDir++ - // Allow wildcard matching - if expectedPath is empty, accept any path - if call.expectedPath != "" && call.expectedPath != dirname { - return nil, errors.New("ReadDir called with unexpected path") - } - return call.returnDirs, call.returnError -} - -func (m *MockFileSystem) WriteFile(filename string, _ []byte, _ os.FileMode) error { - if m.currentWriteFile >= len(m.writeFileCalls) { - return errors.New("unexpected WriteFile call") - } - call := m.writeFileCalls[m.currentWriteFile] - m.currentWriteFile++ - if call.expectedPath != filename { - return errors.New("WriteFile called with unexpected path") - } - return call.returnError -} - -func (m *MockFileSystem) VerifyAllCalls(t *testing.T) { - assert.Equal(t, len(m.readDirCalls), m.currentReadDir, "Not all expected ReadDir calls were made") - assert.Equal(t, len(m.writeFileCalls), m.currentWriteFile, "Not all expected WriteFile calls were made") -} - -// MockDirEntry implements os.DirEntry for testing -type MockDirEntry struct { - name string - isDir bool -} - -func (m MockDirEntry) Name() string { return m.name } -func (m MockDirEntry) IsDir() bool { return m.isDir } -func (m MockDirEntry) Type() os.FileMode { return 0 } -func (m MockDirEntry) Info() (os.FileInfo, error) { return nil, nil } - -func setupMockFS() (*MockFileSystem, func()) { - mockFs := MockFileSystem{} - originalFs := filesystem - filesystem = &mockFs - return &mockFs, func() { filesystem = originalFs } -} - -func loadProfile(path string) (*ptpv1.PtpProfile, error) { - profileData, err := os.ReadFile(path) - if err != nil { - return &ptpv1.PtpProfile{}, err - } - profile := ptpv1.PtpProfile{} - err = yaml.Unmarshal(profileData, &profile) - if err != nil { - return &ptpv1.PtpProfile{}, err - } - return &profile, nil -} - -func Test_initInternalDelays(t *testing.T) { - delays, err := InitInternalDelays("E810-XXVDA4T") - assert.NoError(t, err) - assert.Equal(t, "E810-XXVDA4T", delays.PartType) - assert.Len(t, delays.ExternalInputs, 3) - assert.Len(t, delays.ExternalOutputs, 3) -} - -func Test_initInternalDelays_BadPart(t *testing.T) { - _, err := InitInternalDelays("Dummy") - assert.Error(t, err) -} -func Test_ParseVpd(t *testing.T) { - b, err := os.ReadFile("./testdata/vpd.bin") - assert.NoError(t, err) - vpd := ParseVpd(b) - assert.Equal(t, "Intel(R) Ethernet Network Adapter E810-XXVDA4T", vpd.VendorSpecific1) - assert.Equal(t, "2422", vpd.VendorSpecific2) - assert.Equal(t, "M56954-005", vpd.PartNumber) - assert.Equal(t, "507C6F1FB174", vpd.SerialNumber) -} - -func Test_ProcessProfileTGMNew(t *testing.T) { - unitTest = true - profile, err := loadProfile("./testdata/profile-tgm.yaml") - assert.NoError(t, err) - err = OnPTPConfigChangeE810(nil, profile) - assert.NoError(t, err) -} - -// Test that the profile with no phase inputs is processed correctly -func Test_ProcessProfileTBCNoPhaseInputs(t *testing.T) { - unitTest = true - profile, err := loadProfile("./testdata/profile-tbc-no-input-delays.yaml") - assert.NoError(t, err) - err = OnPTPConfigChangeE810(nil, profile) - assert.NoError(t, err) -} -func Test_ProcessProfileTbc(t *testing.T) { - // Setup filesystem mock for TBC profile (3 devices with pins) - mockFS := &MockFileSystem{} - phcEntries := []os.DirEntry{MockDirEntry{name: "ptp0", isDir: true}} - - // profile-tbc.yaml has pins for ens4f0, ens5f0, ens8f0 (3 devices) - for i := 0; i < 3; i++ { - // Each device needs ReadDir + 4 pin writes (SMA1, SMA2, U.FL1, U.FL2) - mockFS.ExpectReadDir("", phcEntries, nil) // Wildcard path - for j := 0; j < 4; j++ { - mockFS.ExpectWriteFile("", []byte(""), os.FileMode(0666), nil) - } - } - - // Add extra operations for EnableE810Outputs and other calls - for i := 0; i < 10; i++ { - mockFS.ExpectReadDir("", phcEntries, nil) // Extra ReadDir calls - mockFS.ExpectWriteFile("", []byte(""), os.FileMode(0666), nil) // Extra WriteFile calls - } - - // Replace global filesystem with mock - originalFS := filesystem - filesystem = mockFS - defer func() { filesystem = originalFS }() - - // Set unitTest for MockPins() call - unitTest = true - defer func() { unitTest = false }() - - // Can read test profile - profile, err := loadProfile("./testdata/profile-tbc.yaml") - assert.NoError(t, err) - - // Can run PTP config change handler without errors - err = OnPTPConfigChangeE810(nil, profile) - assert.NoError(t, err) - assert.Equal(t, ClockTypeTBC, clockChain.Type, "identified a wrong clock type") - assert.Equal(t, "5799633565432596414", clockChain.LeadingNIC.DpllClockID, "identified a wrong clock ID ") - assert.Equal(t, 9, len(clockChain.LeadingNIC.Pins), "wrong number of configurable pins") - assert.Equal(t, "ens4f1", clockChain.LeadingNIC.UpstreamPort, "wrong upstream port") - - // Test holdover entry - commands, err := clockChain.EnterHoldoverTBC() - assert.NoError(t, err) - assert.Equal(t, 2, len(*commands)) - - // Test holdover exit - commands, err = clockChain.EnterNormalTBC() - assert.NoError(t, err) - assert.Equal(t, 2, len(*commands)) -} - -func Test_ProcessProfileTtsc(t *testing.T) { - // Setup filesystem mock for T-TSC profile (1 device with pins) - mockFS := &MockFileSystem{} - phcEntries := []os.DirEntry{MockDirEntry{name: "ptp0", isDir: true}} - - // profile-t-tsc.yaml has pins for ens4f0 only - mockFS.ExpectReadDir("", phcEntries, nil) // One ReadDir - for i := 0; i < 4; i++ { // 4 pin writes - mockFS.ExpectWriteFile("", []byte(""), os.FileMode(0666), nil) - } - - // Add extra operations for EnableE810Outputs and other calls - for i := 0; i < 10; i++ { - mockFS.ExpectReadDir("", phcEntries, nil) // Extra ReadDir calls - mockFS.ExpectWriteFile("", []byte(""), os.FileMode(0666), nil) // Extra WriteFile calls - } - - // Replace global filesystem with mock - originalFS := filesystem - filesystem = mockFS - defer func() { filesystem = originalFS }() - - // Set unitTest for MockPins() call - unitTest = true - defer func() { unitTest = false }() - - // Can read test profile - profile, err := loadProfile("./testdata/profile-t-tsc.yaml") - assert.NoError(t, err) - - // Can run PTP config change handler without errors - err = OnPTPConfigChangeE810(nil, profile) - assert.NoError(t, err) - assert.Equal(t, ClockTypeTBC, clockChain.Type, "identified a wrong clock type") - assert.Equal(t, "5799633565432596414", clockChain.LeadingNIC.DpllClockID, "identified a wrong clock ID ") - assert.Equal(t, 9, len(clockChain.LeadingNIC.Pins), "wrong number of configurable pins") - assert.Equal(t, "ens4f1", clockChain.LeadingNIC.UpstreamPort, "wrong upstream port") - - // Test holdover entry - commands, err := clockChain.EnterHoldoverTBC() - assert.NoError(t, err) - assert.Equal(t, 2, len(*commands)) - - // Test holdover exit - commands, err = clockChain.EnterNormalTBC() - assert.NoError(t, err) - assert.Equal(t, 2, len(*commands)) - -} - -func Test_ProcessProfileTGMOld(t *testing.T) { - unitTest = true - profile, err := loadProfile("./testdata/profile-tgm-old.yaml") - assert.NoError(t, err) - err = OnPTPConfigChangeE810(nil, profile) - assert.NoError(t, err) -} - -func Test_SetPinDefaults_AllNICs(t *testing.T) { - unitTest = true - - // Load a profile with multiple NICs (leading + other NICs) - profile, err := loadProfile("./testdata/profile-tbc.yaml") - assert.NoError(t, err) - - // Initialize the clock chain with multiple NICs - err = OnPTPConfigChangeE810(nil, profile) - assert.NoError(t, err) - - // Verify we have the expected clock chain structure - assert.Equal(t, ClockTypeTBC, clockChain.Type) - assert.Equal(t, "ens4f0", clockChain.LeadingNIC.Name) - assert.Equal(t, 2, len(clockChain.OtherNICs), "should have 2 other NICs (ens5f0, ens8f0)") - - // Verify each NIC has pins populated - assert.Greater(t, len(clockChain.LeadingNIC.Pins), 0, "leading NIC should have pins") - for i, nic := range clockChain.OtherNICs { - assert.Greater(t, len(nic.Pins), 0, "other NIC %d should have pins", i) - } - - // Call SetPinDefaults and verify it works with all NICs - commands, err := clockChain.SetPinDefaults() - assert.NoError(t, err) - assert.NotNil(t, commands) - - // SetPinDefaults configures 9 different pin types, and we have 3 NICs total - // Each pin type should have a command for each NIC that has that pin - assert.Equal(t, len(*commands), 27, "should have exactly 27 pin commands") - - // Verify that commands include pins from multiple clock IDs - clockIDsSeen := make(map[uint64]bool) - pinLabelsSeen := make(map[string]bool) - - for _, cmd := range *commands { - // Find which pin this command refers to by searching all pins - for _, pin := range clockChain.DpllPins { - if pin.ID == cmd.ID { - clockIDsSeen[pin.ClockID] = true - pinLabelsSeen[pin.BoardLabel] = true - break - } - } - } - - // We should see commands for multiple clock IDs (multiple NICs) - assert.GreaterOrEqual(t, len(clockIDsSeen), 2, "should have commands for at least 2 different clock IDs") - - // We should see commands for the standard configurable pin types - expectedPins := []string{"GNSS-1PPS", "SMA1", "SMA2/U.FL2", "CVL-SDP20", "CVL-SDP22", - "CVL-SDP21", "CVL-SDP23", "C827_0-RCLKA", "C827_0-RCLKB"} - for _, expectedPin := range expectedPins { - assert.True(t, pinLabelsSeen[expectedPin], "should have command for pin %s", expectedPin) - } -} - -func TestEnableE810Outputs(t *testing.T) { - tests := []struct { - name string - setupMock func(*MockFileSystem) - clockChain *ClockChain - expectedError string - }{ - { - name: "Successful execution - single PHC", - clockChain: &ClockChain{ - LeadingNIC: CardInfo{Name: "ens4f0"}, - }, - setupMock: func(m *MockFileSystem) { - phcEntries := []os.DirEntry{ - MockDirEntry{name: "ptp0", isDir: true}, - } - m.ExpectReadDir("/sys/class/net/ens4f0/device/ptp/", phcEntries, nil) - m.ExpectWriteFile("/sys/class/net/ens4f0/device/ptp/ptp0/pins/SMA2", []byte("2 2"), os.FileMode(0666), nil) - m.ExpectWriteFile("/sys/class/net/ens4f0/device/ptp/ptp0/period", []byte("2 0 0 1 0"), os.FileMode(0666), nil) - }, - expectedError: "", - }, - { - name: "ReadDir fails", - clockChain: &ClockChain{ - LeadingNIC: CardInfo{Name: "ens4f0"}, - }, - setupMock: func(m *MockFileSystem) { - m.ExpectReadDir("/sys/class/net/ens4f0/device/ptp/", []os.DirEntry{}, errors.New("permission denied")) - }, - expectedError: "e810 failed to read /sys/class/net/ens4f0/device/ptp/: permission denied", - }, - { - name: "No PHC directories found", - clockChain: &ClockChain{ - LeadingNIC: CardInfo{Name: "ens4f0"}, - }, - setupMock: func(m *MockFileSystem) { - m.ExpectReadDir("/sys/class/net/ens4f0/device/ptp/", []os.DirEntry{}, nil) - }, - expectedError: "e810 cards should have one PHC per NIC, but ens4f0 has 0", - }, - { - name: "Multiple PHC directories found (warning case)", - clockChain: &ClockChain{ - LeadingNIC: CardInfo{Name: "ens4f0"}, - }, - setupMock: func(m *MockFileSystem) { - phcEntries := []os.DirEntry{ - MockDirEntry{name: "ptp0", isDir: true}, - MockDirEntry{name: "ptp1", isDir: true}, - } - m.ExpectReadDir("/sys/class/net/ens4f0/device/ptp/", phcEntries, nil) - m.ExpectWriteFile("/sys/class/net/ens4f0/device/ptp/ptp0/pins/SMA2", []byte("2 2"), os.FileMode(0666), nil) - m.ExpectWriteFile("/sys/class/net/ens4f0/device/ptp/ptp0/period", []byte("2 0 0 1 0"), os.FileMode(0666), nil) - }, - expectedError: "", - }, - { - name: "SMA2 write fails", - clockChain: &ClockChain{ - LeadingNIC: CardInfo{Name: "ens4f0"}, - }, - setupMock: func(m *MockFileSystem) { - phcEntries := []os.DirEntry{ - MockDirEntry{name: "ptp0", isDir: true}, - } - m.ExpectReadDir("/sys/class/net/ens4f0/device/ptp/", phcEntries, nil) - m.ExpectWriteFile("/sys/class/net/ens4f0/device/ptp/ptp0/pins/SMA2", []byte("2 2"), os.FileMode(0666), errors.New("write failed")) - }, - expectedError: "e810 failed to write 2 2 to /sys/class/net/ens4f0/device/ptp/ptp0/pins/SMA2: write failed", - }, - { - name: "Period write fails - should not return error but log", - clockChain: &ClockChain{ - LeadingNIC: CardInfo{Name: "ens4f0"}, - }, - setupMock: func(m *MockFileSystem) { - phcEntries := []os.DirEntry{ - MockDirEntry{name: "ptp0", isDir: true}, - } - m.ExpectReadDir("/sys/class/net/ens4f0/device/ptp/", phcEntries, nil) - m.ExpectWriteFile("/sys/class/net/ens4f0/device/ptp/ptp0/pins/SMA2", []byte("2 2"), os.FileMode(0666), nil) - m.ExpectWriteFile("/sys/class/net/ens4f0/device/ptp/ptp0/period", []byte("2 0 0 1 0"), os.FileMode(0666), errors.New("period write failed")) - }, - expectedError: "", // Function doesn't return error for period write failure - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - // Setup mock filesystem - mockFS := &MockFileSystem{} - tt.setupMock(mockFS) - - // Replace global filesystem with mock - originalFS := filesystem - filesystem = mockFS - defer func() { filesystem = originalFS }() - - // Execute function - err := tt.clockChain.EnableE810Outputs() - - // Check error - if tt.expectedError != "" { - assert.Error(t, err) - assert.Contains(t, err.Error(), tt.expectedError) - } else { - assert.NoError(t, err) - } - - // Verify all expected calls were made - mockFS.VerifyAllCalls(t) - }) - } -} diff --git a/addons/intel/mock.go b/addons/intel/mock.go deleted file mode 100644 index 56a9e894..00000000 --- a/addons/intel/mock.go +++ /dev/null @@ -1,12 +0,0 @@ -package intel - -func MockPins() { - pins, err := loadPins("./testdata/dpll-pins.json") - if err != nil { - panic(err) - } - // Mock DPLL pins - for _, pin := range *pins { - DpllPins = append(DpllPins, &pin) - } -} diff --git a/addons/intel/mock_test.go b/addons/intel/mock_test.go new file mode 100644 index 00000000..aba203c4 --- /dev/null +++ b/addons/intel/mock_test.go @@ -0,0 +1,391 @@ +package intel + +import ( + "encoding/json" + "fmt" + "os" + "strconv" + "strings" + "testing" + + dpll "github.com/k8snetworkplumbingwg/linuxptp-daemon/pkg/dpll-netlink" + ptpv1 "github.com/k8snetworkplumbingwg/ptp-operator/api/v1" + "github.com/stretchr/testify/assert" + "sigs.k8s.io/yaml" +) + +// mockBatchPinSet is a simple mock to unit-test pin set operations +type mockBatchPinSet struct { + commands *[]dpll.PinParentDeviceCtl +} + +func (m *mockBatchPinSet) mock(commands *[]dpll.PinParentDeviceCtl) error { + if m.commands == nil { + cmds := make([]dpll.PinParentDeviceCtl, 0) + m.commands = &cmds + } + *m.commands = append(*m.commands, *commands...) + return nil +} + +func (m *mockBatchPinSet) reset() { + m.commands = nil +} + +func setupBatchPinSetMock() (*mockBatchPinSet, func()) { + originalBatchPinset := BatchPinSet + mock := &mockBatchPinSet{} + BatchPinSet = mock.mock + return mock, func() { BatchPinSet = originalBatchPinset } +} + +// MockFileSystem is a simple mock implementation of FileSystemInterface +type MockFileSystem struct { + // Expected calls and responses + readDirCalls []ReadDirCall + writeFileCalls []WriteFileCall + readFileCalls []ReadFileCall + readLinkCalls []ReadLinkCall + + currentReadDir int + currentWriteFile int + currentReadFile int + currentReadLink int + // Allowed (but not verified) calls and responses + allowedReadDir map[string]ReadDirCall + allowedWriteFile map[string]WriteFileCall + allowedReadFile map[string]ReadFileCall + allowedReadLink map[string]ReadLinkCall +} + +func setupMockFS() (*MockFileSystem, func()) { + originalFilesystem := filesystem + mock := &MockFileSystem{} + filesystem = mock + return mock, func() { filesystem = originalFilesystem } +} + +type ReadDirCall struct { + expectedPath string + returnDirs []os.DirEntry + returnError error +} + +type WriteFileCall struct { + expectedPath string + expectedData []byte + expectedPerm os.FileMode + returnError error +} + +type ReadFileCall struct { + expectedPath string + returnData []byte + returnError error +} + +type ReadLinkCall struct { + expectedPath string + returnData string + returnError error +} + +func (m *MockFileSystem) ExpectReadDir(path string, dirs []os.DirEntry, err error) { + m.readDirCalls = append(m.readDirCalls, ReadDirCall{ + expectedPath: path, + returnDirs: dirs, + returnError: err, + }) +} + +func (m *MockFileSystem) AllowReadDir(path string, dirs []os.DirEntry, err error) { + if m.allowedReadDir == nil { + m.allowedReadDir = make(map[string]ReadDirCall) + } + m.allowedReadDir[path] = ReadDirCall{ + expectedPath: path, + returnDirs: dirs, + returnError: err, + } +} + +func (m *MockFileSystem) ExpectWriteFile(path string, data []byte, perm os.FileMode, err error) { + m.writeFileCalls = append(m.writeFileCalls, WriteFileCall{ + expectedPath: path, + expectedData: data, + expectedPerm: perm, + returnError: err, + }) +} + +func (m *MockFileSystem) AllowWriteFile(path string) { + if m.allowedWriteFile == nil { + m.allowedWriteFile = make(map[string]WriteFileCall) + } + m.allowedWriteFile[path] = WriteFileCall{ + expectedPath: path, + } +} + +func (m *MockFileSystem) ExpectReadFile(path string, data []byte, err error) { + m.readFileCalls = append(m.readFileCalls, ReadFileCall{ + expectedPath: path, + returnData: data, + returnError: err, + }) +} + +func (m *MockFileSystem) AllowReadFile(path string, data []byte, err error) { + if m.allowedReadFile == nil { + m.allowedReadFile = make(map[string]ReadFileCall) + } + m.allowedReadFile[path] = ReadFileCall{ + expectedPath: path, + returnData: data, + returnError: err, + } +} + +func (m *MockFileSystem) ExpectReadLink(path string, data string, err error) { + m.readLinkCalls = append(m.readLinkCalls, ReadLinkCall{ + expectedPath: path, + returnData: data, + returnError: err, + }) +} + +func (m *MockFileSystem) AllowReadLink(path string, data string, err error) { + if m.allowedReadLink == nil { + m.allowedReadLink = make(map[string]ReadLinkCall) + } + m.allowedReadLink[path] = ReadLinkCall{ + expectedPath: path, + returnData: data, + returnError: err, + } +} + +func (m *MockFileSystem) ReadDir(dirname string) ([]os.DirEntry, error) { + if allowed, ok := m.allowedReadDir[dirname]; ok { + return allowed.returnDirs, allowed.returnError + } + if m.currentReadDir >= len(m.readDirCalls) { + return nil, fmt.Errorf("unexpected ReadDir call (%s)", dirname) + } + call := m.readDirCalls[m.currentReadDir] + m.currentReadDir++ + // Allow wildcard matching - if expectedPath is empty, accept any path + if call.expectedPath != "" && call.expectedPath != dirname { + return nil, fmt.Errorf("ReadDir called with unexpected path (%s), was expecting %s", dirname, call.expectedPath) + } + return call.returnDirs, call.returnError +} + +func (m *MockFileSystem) WriteFile(filename string, data []byte, _ os.FileMode) error { + if _, ok := m.allowedWriteFile[filename]; ok { + m.AllowReadFile(filename, data, nil) + return nil + } + if m.currentWriteFile >= len(m.writeFileCalls) { + return fmt.Errorf("unexpected WriteFile call (%s)", filename) + } + call := m.writeFileCalls[m.currentWriteFile] + m.currentWriteFile++ + if call.expectedPath != "" && call.expectedPath != filename { + return fmt.Errorf("WriteFile called with unexpected path (%s), was expecting %s", filename, call.expectedPath) + } + return call.returnError +} + +func (m *MockFileSystem) ReadFile(filename string) ([]byte, error) { + if allowed, ok := m.allowedReadFile[filename]; ok { + return allowed.returnData, allowed.returnError + } + if m.currentReadFile >= len(m.readFileCalls) { + return nil, fmt.Errorf("Unexpected ReadFile call (%s)", filename) + } + call := m.readFileCalls[m.currentReadFile] + m.currentReadFile++ + if call.expectedPath != "" && call.expectedPath != filename { + return nil, fmt.Errorf("ReadFile called with unexpected filename (%s), was expecting %s", filename, call.expectedPath) + } + return call.returnData, call.returnError +} + +func (m *MockFileSystem) ReadLink(filename string) (string, error) { + if allowed, ok := m.allowedReadLink[filename]; ok { + return allowed.returnData, allowed.returnError + } + if m.currentReadLink >= len(m.readLinkCalls) { + return "", fmt.Errorf("Unexpected ReadLink call (%s)", filename) + } + call := m.readLinkCalls[m.currentReadLink] + m.currentReadLink++ + if call.expectedPath != "" && call.expectedPath != filename { + return "", fmt.Errorf("ReadLink called with unexpected filename (%s), was expecting %s", filename, call.expectedPath) + } + return call.returnData, call.returnError +} + +func (m *MockFileSystem) VerifyAllCalls(t *testing.T) { + assert.Equal(t, len(m.readDirCalls), m.currentReadDir, "Not all expected ReadDir calls were made") + assert.Equal(t, len(m.writeFileCalls), m.currentWriteFile, "Not all expected WriteFile calls were made") + assert.Equal(t, len(m.readFileCalls), m.currentReadFile, "Not all expected ReadFile calls were made") + assert.Equal(t, len(m.readLinkCalls), m.currentReadLink, "Not all expected ReadLink calls were made") +} + +// MockDirEntry implements os.DirEntry for testing +type MockDirEntry struct { + name string + isDir bool +} + +func (m MockDirEntry) Name() string { return m.name } +func (m MockDirEntry) IsDir() bool { return m.isDir } +func (m MockDirEntry) Type() os.FileMode { return 0 } +func (m MockDirEntry) Info() (os.FileInfo, error) { return nil, nil } + +func loadProfile(path string) (*ptpv1.PtpProfile, error) { + profileData, err := os.ReadFile(path) + if err != nil { + return &ptpv1.PtpProfile{}, err + } + profile := ptpv1.PtpProfile{} + err = yaml.Unmarshal(profileData, &profile) + if err != nil { + return &ptpv1.PtpProfile{}, err + } + if profile.Name == nil { + return &profile, fmt.Errorf("Could not parse profile") + } + return &profile, nil +} + +type mockClockChain struct { + returnErr error + enterNormalTBCCount int + enterHoldoverTBCCount int + setPinDefaultsCount int +} + +func (m *mockClockChain) EnterNormalTBC() error { + m.enterNormalTBCCount++ + return m.returnErr +} + +func (m *mockClockChain) EnterHoldoverTBC() error { + m.enterHoldoverTBCCount++ + return m.returnErr +} + +func (m *mockClockChain) SetPinDefaults() error { + m.setPinDefaultsCount++ + return m.returnErr +} + +func (m *mockClockChain) GetLeadingNIC() CardInfo { + return CardInfo{} +} + +func (m *mockClockChain) assertCallCounts(t *testing.T, expectedNormalTBC, expectedHoldoverTBC, expectedSetPinDefaults int) { + assert.Equal(t, expectedNormalTBC, m.enterNormalTBCCount, "Expected enterNormalTBCCount") + assert.Equal(t, expectedHoldoverTBC, m.enterHoldoverTBCCount, "Expected enterHoldoverTBCCount") + assert.Equal(t, expectedSetPinDefaults, m.setPinDefaultsCount, "Expected setPinDefaultsCount") +} + +func mockClockIDsFromProfile(mfs *MockFileSystem, profile *ptpv1.PtpProfile) { + for key, val := range profile.PtpSettings { + var iface string + if strings.HasPrefix(key, "clockId[") && strings.HasSuffix(key, "]") { + iface = strings.TrimSuffix(strings.TrimPrefix(key, "clockId["), "]") + id, err := strconv.ParseUint(val, 10, 64) + if err != nil { + continue + } + mfs.AllowReadFile(fmt.Sprintf("/sys/class/net/%s/device/config", iface), generatePCIDataForClockID(id), nil) + } + } +} + +func setupGNSSMocks(data *E825PluginData) (*mockBatchPinSet, func()) { + // Setup Mock gnss dpll pin data + data.dpllPins = []*dpll.PinInfo{ + { + ID: 1, + BoardLabel: "SkipMe", + Type: dpll.PinTypeEXT, + Capabilities: dpll.PinCapPrio, + }, + { + BoardLabel: "GNSS_1PPS_IN", + ID: 2, + Type: dpll.PinTypeGNSS, + Capabilities: dpll.PinCapPrio | dpll.PinCapState, + ParentDevice: []dpll.PinParentDevice{ + { + ParentID: uint32(1), + Direction: dpll.PinDirectionInput, + }, + { + ParentID: uint32(2), + Direction: dpll.PinDirectionInput, + }, + }, + }, + } + // Mock pin-set logic + return setupBatchPinSetMock() +} + +type mockedDPLLPins struct { + pins dpllPins +} + +func (m *mockedDPLLPins) FetchPins() error { return nil } + +func (m *mockedDPLLPins) GetByLabel(label string, clockID uint64) *dpll.PinInfo { + return m.pins.GetByLabel(label, clockID) +} + +func (m *mockedDPLLPins) GetAllPinsByLabel(label string) []*dpll.PinInfo { + return m.pins.GetAllPinsByLabel(label) +} + +func (m *mockedDPLLPins) GetCommandsForPluginPinSet(clockID uint64, pinset pinSet) []dpll.PinParentDeviceCtl { + return m.pins.GetCommandsForPluginPinSet(clockID, pinset) +} + +func (m *mockedDPLLPins) ApplyPinCommands(commands []dpll.PinParentDeviceCtl) error { + return BatchPinSet(&commands) +} + +func setupMockDPLLPins(pins ...*dpll.PinInfo) (*mockedDPLLPins, func()) { + orig := DpllPins + mock := &mockedDPLLPins{pins: dpllPins(pins)} + DpllPins = mock + return mock, func() { DpllPins = orig } +} + +func setupMockDPLLPinsFromJSON(path string) (*mockedDPLLPins, func()) { //nolint: unparam // it may be used for other pin files in the future it doesn't make the code overly complex + pins := []dpll.PinInfo{} + data, err := os.ReadFile(path) + if err != nil { + panic(fmt.Sprintf("failed to read pins from %s: %v", path, err)) + } + if err = json.Unmarshal(data, &pins); err != nil { + panic(fmt.Sprintf("failed to unmarshal pins from %s: %v", path, err)) + } + ptrs := make([]*dpll.PinInfo, len(pins)) + for i := range pins { + ptrs[i] = &pins[i] + } + return setupMockDPLLPins(ptrs...) +} + +func setupMockDelayCompensation() func() { + orig := SendDelayCompensation + SendDelayCompensation = func(_ *[]delayCompensation, _ DPLLPins) error { + return nil + } + return func() { SendDelayCompensation = orig } +} diff --git a/addons/intel/phaseAdjust.go b/addons/intel/phaseAdjust.go index 2d6eb74a..5ad7ee25 100644 --- a/addons/intel/phaseAdjust.go +++ b/addons/intel/phaseAdjust.go @@ -51,7 +51,7 @@ type delayCompensation struct { pinLabel string iface string direction string - clockID string + clockID uint64 } var hardware = map[string]string{ @@ -99,7 +99,10 @@ func InitInternalDelays(part string) (*InternalDelays, error) { return nil, fmt.Errorf("can't find delays for %s", part) } -func sendDelayCompensation(comp *[]delayCompensation, DpllPins []*dpll.PinInfo) error { +// SendDelayCompensation is a function variable for mocking in tests +var SendDelayCompensation = sendDelayCompensation + +func sendDelayCompensation(comp *[]delayCompensation, pins DPLLPins) error { glog.Info(comp) conn, err := dpll.Dial(nil) if err != nil { @@ -108,33 +111,29 @@ func sendDelayCompensation(comp *[]delayCompensation, DpllPins []*dpll.PinInfo) //nolint:errcheck defer conn.Close() - for _, pin := range DpllPins { - for _, dc := range *comp { - var desiredClockID uint64 - desiredClockID, err = strconv.ParseUint(dc.clockID, 10, 64) - if err != nil { - return fmt.Errorf("failed to parse clock id %s: %v", dc.clockID, err) - } - if desiredClockID == pin.ClockID && strings.EqualFold(pin.BoardLabel, dc.pinLabel) { - err = conn.PinPhaseAdjust(dpll.PinPhaseAdjustRequest{ID: pin.ID, PhaseAdjust: dc.DelayPs}) - if err != nil { - return fmt.Errorf("failed to send phase adjustment to %s clock id %d: %v", - pin.BoardLabel, desiredClockID, err) - } - glog.Infof("set phaseAdjust of pin %s at clock ID %x to %d ps", pin.BoardLabel, pin.ClockID, dc.DelayPs) - } + for _, dc := range *comp { + pin := pins.GetByLabel(dc.pinLabel, dc.clockID) + if pin == nil { + glog.Warningf("pin %s not found for clock ID %d; skipping phase adjustment", dc.pinLabel, dc.clockID) + continue + } + err = conn.PinPhaseAdjust(dpll.PinPhaseAdjustRequest{ID: pin.ID, PhaseAdjust: dc.DelayPs}) + if err != nil { + return fmt.Errorf("failed to send phase adjustment to %s clock id %d: %v", + pin.BoardLabel, dc.clockID, err) } + glog.Infof("set phaseAdjust of pin %s at clock ID %x to %d ps", pin.BoardLabel, pin.ClockID, dc.DelayPs) } return nil } -func addClockID(iface string, nodeProfile *ptpv1.PtpProfile) (*string, error) { - dpllClockIdStr := fmt.Sprintf("%s[%s]", "clockId", iface) - clockID, found := (*nodeProfile).PtpSettings[dpllClockIdStr] +func addClockID(iface string, nodeProfile *ptpv1.PtpProfile) (uint64, error) { + dpllClockIDStr := fmt.Sprintf("clockId[%s]", iface) + clockIDStr, found := (*nodeProfile).PtpSettings[dpllClockIDStr] if !found { - return nil, fmt.Errorf("plugin E810 error: can't find clock ID for interface %s - are all pins configured?", iface) + return 0, fmt.Errorf("plugin E810 error: can't find clock ID for interface %s - are all pins configured?", iface) } - return &clockID, nil + return strconv.ParseUint(clockIDStr, 10, 64) } func findInternalLink(links []InternalLink, connector string) *InternalLink { diff --git a/addons/intel/pinConfig.go b/addons/intel/pinConfig.go index 4b2788c2..d46240a7 100644 --- a/addons/intel/pinConfig.go +++ b/addons/intel/pinConfig.go @@ -42,6 +42,17 @@ func (r realPinConfig) applyPinSet(device string, pins pinSet) error { return errors.Join(errList...) } +func hasSysfsSMAPins(device string) bool { + deviceDir := fmt.Sprintf("/sys/class/net/%s/device/ptp/", device) + phcs, err := filesystem.ReadDir(deviceDir) + if err != nil || len(phcs) == 0 { + return false + } + sma1Path := fmt.Sprintf("/sys/class/net/%s/device/ptp/%s/pins/SMA1", device, phcs[0].Name()) + _, err = filesystem.ReadFile(sma1Path) + return err == nil +} + func (r realPinConfig) applyPinFrq(device string, values frqSet) error { deviceDir := fmt.Sprintf("/sys/class/net/%s/device/ptp/", device) phcs, err := filesystem.ReadDir(deviceDir) diff --git a/addons/intel/pinConfig_test.go b/addons/intel/pinConfig_test.go index d16826ab..a157d99b 100644 --- a/addons/intel/pinConfig_test.go +++ b/addons/intel/pinConfig_test.go @@ -36,6 +36,33 @@ func Test_applyPinSet(t *testing.T) { assert.Equal(t, 1, mockFS.currentWriteFile) } +func Test_hasSysfsSMAPins(t *testing.T) { + t.Run("SMA1 exists", func(t *testing.T) { + mockFS, restoreFS := setupMockFS() + defer restoreFS() + phcEntries := []os.DirEntry{MockDirEntry{name: "ptp0", isDir: true}} + mockFS.ExpectReadDir("/sys/class/net/ens4f0/device/ptp/", phcEntries, nil) + mockFS.ExpectReadFile("/sys/class/net/ens4f0/device/ptp/ptp0/pins/SMA1", []byte("0 1"), nil) + assert.True(t, hasSysfsSMAPins("ens4f0")) + }) + + t.Run("SMA1 missing", func(t *testing.T) { + mockFS, restoreFS := setupMockFS() + defer restoreFS() + phcEntries := []os.DirEntry{MockDirEntry{name: "ptp0", isDir: true}} + mockFS.ExpectReadDir("/sys/class/net/ens4f0/device/ptp/", phcEntries, nil) + mockFS.ExpectReadFile("/sys/class/net/ens4f0/device/ptp/ptp0/pins/SMA1", nil, os.ErrNotExist) + assert.False(t, hasSysfsSMAPins("ens4f0")) + }) + + t.Run("no PHC directory", func(t *testing.T) { + mockFS, restoreFS := setupMockFS() + defer restoreFS() + mockFS.ExpectReadDir("/sys/class/net/ens4f0/device/ptp/", nil, os.ErrNotExist) + assert.False(t, hasSysfsSMAPins("ens4f0")) + }) +} + func Test_applyPinFrq(t *testing.T) { mockFS, restoreFS := setupMockFS() defer restoreFS() diff --git a/addons/intel/testdata/profile-t-tsc.yaml b/addons/intel/testdata/profile-t-tsc.yaml index f7c0e9b1..5f571c3c 100644 --- a/addons/intel/testdata/profile-t-tsc.yaml +++ b/addons/intel/testdata/profile-t-tsc.yaml @@ -1,6 +1,5 @@ name: tsc ptpSettings: - unitTest: /tmp/test clockId[ens4f0]: 5799633565432596414 phc2sysOpts: -a -r -n 24 -N 8 -R 16 -u 0 plugins: diff --git a/addons/intel/testdata/profile-tbc-no-input-delays.yaml b/addons/intel/testdata/profile-tbc-no-input-delays.yaml index 1352ad7d..3d0a3845 100644 --- a/addons/intel/testdata/profile-tbc-no-input-delays.yaml +++ b/addons/intel/testdata/profile-tbc-no-input-delays.yaml @@ -1,6 +1,5 @@ name: tbc ptpSettings: - unitTest: /tmp/test clockId[ens4f0]: 5799633565432596414 clockId[ens5f0]: 5799633565433967128 clockId[ens8f0]: 5799633565432596448 diff --git a/addons/intel/testdata/profile-tbc.yaml b/addons/intel/testdata/profile-tbc.yaml index 901063a4..93bb96d5 100644 --- a/addons/intel/testdata/profile-tbc.yaml +++ b/addons/intel/testdata/profile-tbc.yaml @@ -1,6 +1,5 @@ name: tbc ptpSettings: - unitTest: /tmp/test clockId[ens4f0]: 5799633565432596414 clockId[ens5f0]: 5799633565433967128 clockId[ens8f0]: 5799633565432596448 diff --git a/addons/intel/testdata/profile-tgm-old.yaml b/addons/intel/testdata/profile-tgm-old.yaml index 7714d361..457e9bb9 100644 --- a/addons/intel/testdata/profile-tgm-old.yaml +++ b/addons/intel/testdata/profile-tgm-old.yaml @@ -1,6 +1,5 @@ name: grandmaster ptpSettings: - unitTest: /tmp/test phc2sysOpts: -a -r -n 24 -N 8 -R 16 -u 0 plugins: e810: diff --git a/addons/intel/testdata/profile-tgm.yaml b/addons/intel/testdata/profile-tgm.yaml index d7e4b14b..f16b1acf 100644 --- a/addons/intel/testdata/profile-tgm.yaml +++ b/addons/intel/testdata/profile-tgm.yaml @@ -1,6 +1,5 @@ name: grandmaster ptpSettings: - unitTest: /tmp/test clockId[ens4f0]: 5799633565432596414 clockId[ens5f0]: 5799633565433967128 clockId[ens8f0]: 5799633565432596448 diff --git a/pkg/daemon/daemon.go b/pkg/daemon/daemon.go index 75bcc8ef..724bd97e 100644 --- a/pkg/daemon/daemon.go +++ b/pkg/daemon/daemon.go @@ -879,7 +879,7 @@ func (dn *Daemon) applyNodePtpProfile(runID int, nodeProfile *ptpv1.PtpProfile) dpllDaemon := dpll.NewDpll(clockId, localMaxHoldoverOffSet, localHoldoverTimeout, maxInSpecOffset, iface.Name, eventSource, dpll.NONE, dn.GetPhaseOffsetPinFilter(nodeProfile), // Used only in T-BC in-sync condition: - inSyncConditionTh, inSyncConditionTimes) + inSyncConditionTh, inSyncConditionTimes, 0) glog.Infof("depending on %s", dpllDaemon.DependsOn()) dpllDaemon.CmdInit() dprocess.depProcess = append(dprocess.depProcess, dpllDaemon) diff --git a/pkg/dpll-netlink/dpll-uapi.go b/pkg/dpll-netlink/dpll-uapi.go index 5c770a8d..e568400f 100644 --- a/pkg/dpll-netlink/dpll-uapi.go +++ b/pkg/dpll-netlink/dpll-uapi.go @@ -246,7 +246,7 @@ type PinInfoHR struct { type PinParentDeviceHR struct { ParentID uint32 `json:"parentId"` Direction string `json:"direction"` - Prio uint32 `json:"prio"` + Prio *uint32 `json:"prio,omitempty"` State string `json:"state"` PhaseOffsetPs float64 `json:"phaseOffsetPs"` } diff --git a/pkg/dpll-netlink/dpll.go b/pkg/dpll-netlink/dpll.go index 75e440d4..51d56cfa 100644 --- a/pkg/dpll-netlink/dpll.go +++ b/pkg/dpll-netlink/dpll.go @@ -307,7 +307,8 @@ func ParsePinReplies(msgs []genetlink.Message) ([]*PinInfo, error) { case DpllPinDirection: temp.Direction = ad.Uint32() case DpllPinPrio: - temp.Prio = ad.Uint32() + v := ad.Uint32() + temp.Prio = &v case DpllPinState: temp.State = ad.Uint32() case DpllPinPhaseOffset: @@ -471,7 +472,7 @@ type FrequencyRange struct { type PinParentDevice struct { ParentID uint32 Direction uint32 - Prio uint32 + Prio *uint32 State uint32 PhaseOffset int64 } diff --git a/pkg/dpll/dpll.go b/pkg/dpll/dpll.go index a8b42da0..108a8609 100644 --- a/pkg/dpll/dpll.go +++ b/pkg/dpll/dpll.go @@ -29,9 +29,9 @@ const ( DPLL_LOCKED_HO_ACQ = 3 DPLL_HOLDOVER = 4 - LocalMaxHoldoverOffSet = 1500 //ns - LocalHoldoverTimeout = 14400 //secs - MaxInSpecOffset = 1500 //ns + LocalMaxHoldoverOffSet = 1500 // ns + LocalHoldoverTimeout = 14400 // secs + MaxInSpecOffset = 1500 // ns monitoringInterval = 1 * time.Second LocalMaxHoldoverOffSetStr = "LocalMaxHoldoverOffSet" @@ -44,6 +44,38 @@ const ( PPS_PIN_INDEX = 1 ) +// Flag is a bitmask which changes the default DPLL monitoeing behavior +type Flag uint64 + +const ( + // FlagNoPhaseOffset allows skipping phase offset monitoring + FlagNoPhaseOffset Flag = (1 << 0) + // FlagNoPhaseStatus allows skipping phase status (pps lock/unlock) monitoring + FlagNoPhaseStatus Flag = (1 << 1) + // FlagNoFreqencyStatus allows skipping frequency status (eec lock/unlock) monitoring + FlagNoFreqencyStatus Flag = (1 << 2) + + // FlagOnlyPhaseStatus represents a DPll with only phase status (pps lock/unlock) + FlagOnlyPhaseStatus Flag = FlagNoFreqencyStatus | FlagNoPhaseOffset +) + +func stateName(state int64) string { + switch state { + case DPLL_INVALID: + return "INVALID" + case DPLL_FREERUN: + return "FREERUN" + case DPLL_LOCKED: + return "LOCKED" + case DPLL_LOCKED_HO_ACQ: + return "LOCKED_HO_ACQ" + case DPLL_HOLDOVER: + return "HOLDOVER" + default: + return "UNKNOWN" + } +} + type dpllApiType string var MockDpllReplies chan *nl.DoDeviceGetReply @@ -96,7 +128,7 @@ type DpllConfig struct { iface string name string slope float64 - timer int64 //secs + timer int64 // secs inSpec bool frequencyTraceable bool state event.PTPState @@ -117,6 +149,7 @@ type DpllConfig struct { phaseStatus int64 frequencyStatus int64 phaseOffset int64 + // clockId is needed to distinguish between DPLL associated with the particular // iface from other DPLL units that might be present on the system. Clock ID implementation // is driver-specific and vendor-specific. @@ -127,6 +160,12 @@ type DpllConfig struct { phaseOffsetPinFilter map[string]map[string]string inSyncConditionThreshold uint64 inSyncConditionTimes uint64 + // hardwareConfigHandler is called when device notifications are received + // All logic for processing device notifications is handled by the hardwareconfig layer + hardwareConfigHandler func(devices []*nl.DoDeviceGetReply) error + + // Some DPLLs (Carter Flats, for one) do not have both pps (phase) and eec (frequency) states. + flags Flag // devices holds the cache of DPLL device replies devices []*nl.DoDeviceGetReply @@ -170,11 +209,42 @@ func (d *DpllConfig) SetSourceLost(sourceLost bool) { d.sourceLost = sourceLost } +// SetHardwareConfigHandler sets the callback function to be invoked when device notifications are received. +// The handler receives all device notifications and is responsible for all matching logic. +func (d *DpllConfig) SetHardwareConfigHandler(handler func(devices []*nl.DoDeviceGetReply) error) { + d.hardwareConfigHandler = handler +} + // PhaseOffset ... get phase offset func (d *DpllConfig) PhaseOffset() int64 { return d.phaseOffset } +func (d *DpllConfig) hasFlag(flag Flag) bool { + return (d.flags & flag) == flag +} + +func (d *DpllConfig) flagsToStrings() []string { + result := make([]string, 0) + if d.hasFlag(FlagNoFreqencyStatus) { + result = append(result, "NoFrequencyStatus") + } + if d.hasFlag(FlagNoPhaseStatus) { + result = append(result, "NoPhaseStatus") + } + if d.hasFlag(FlagNoPhaseOffset) { + result = append(result, "NoPhaseOffset") + } + return result +} + +func (d *DpllConfig) phaseOffsetStr() string { + if d.hasFlag(FlagNoPhaseOffset) { + return "UNKNOWN" + } + return fmt.Sprintf("%d", d.phaseOffset) +} + // FrequencyStatus ... get frequency status func (d *DpllConfig) FrequencyStatus() int64 { return d.frequencyStatus @@ -233,7 +303,7 @@ func (d *DpllConfig) Name() string { // Stopped ... stopped func (d *DpllConfig) Stopped() bool { - //TODO implement me + // TODO implement me panic("implement me") } @@ -302,7 +372,8 @@ func (d *DpllConfig) unRegisterAll() { // NewDpll ... create new DPLL process func NewDpll(clockId uint64, localMaxHoldoverOffSet, localHoldoverTimeout, maxInSpecOffset uint64, iface string, dependsOn []event.EventSource, apiType dpllApiType, phaseOffsetPinFilter map[string]map[string]string, - inSyncConditionTh uint64, inSyncConditionTimes uint64) *DpllConfig { + inSyncConditionTh uint64, inSyncConditionTimes uint64, dpllFlags Flag, +) *DpllConfig { glog.Infof("Calling NewDpll with clockId %x, localMaxHoldoverOffSet=%d, localHoldoverTimeout=%d, maxInSpecOffset=%d, iface=%s, phase offset pin filter=%v", clockId, localMaxHoldoverOffSet, localHoldoverTimeout, maxInSpecOffset, iface, phaseOffsetPinFilter) d := &DpllConfig{ clockId: clockId, @@ -328,6 +399,12 @@ func NewDpll(clockId uint64, localMaxHoldoverOffSet, localHoldoverTimeout, maxIn phaseOffset: FaultyPhaseOffset, inSyncConditionThreshold: inSyncConditionTh, inSyncConditionTimes: inSyncConditionTimes, + flags: dpllFlags, + } + + if d.flags != 0 { + flagStrings := d.flagsToStrings() + glog.Warningf("Partial monitoring detected for %s clockId %#x: %v", iface, clockId, flagStrings) } // time to reach maxnInSpecOffset @@ -336,6 +413,7 @@ func NewDpll(clockId uint64, localMaxHoldoverOffSet, localHoldoverTimeout, maxIn d.slope, float64(d.MaxInSpecOffset), d.timer, int64(d.LocalHoldoverTimeout)) return d } + func (d *DpllConfig) Slope() float64 { return d.slope } @@ -378,9 +456,11 @@ func (d *DpllConfig) nlUpdateState(devices []*nl.DoDeviceGetReply, pins []*nl.Pi switch nl.GetDpllType(reply.Type) { case "eec": d.frequencyStatus = int64(reply.LockStatus) + glog.Infof("%s (%#x) updating eec to %s (%d)", d.iface, d.clockId, stateName(d.frequencyStatus), d.frequencyStatus) valid = true case "pps": d.phaseStatus = int64(reply.LockStatus) + glog.Infof("%s (%#x) updating pps to %s (%d)", d.iface, d.clockId, stateName(d.phaseStatus), d.phaseStatus) valid = true } } @@ -429,6 +509,13 @@ func (d *DpllConfig) monitorNtf(c *genetlink.Conn) { } } + // Pass device notifications to hardwareconfig handler if present + // All logic (clock ID matching, lock status checking) happens in hardwareconfig layer + if len(devices) > 0 && d.hardwareConfigHandler != nil { + if err = d.hardwareConfigHandler(devices); err != nil { + glog.Errorf("hardwareconfig handler error: %v", err) + } + } if d.nlUpdateState(devices, pins) { d.stateDecision() } @@ -602,7 +689,7 @@ func (d *DpllConfig) MonitorProcess(processCfg config.ProcessConfig) { d.processConfig = processCfg // register to event notification from other processes for _, dep := range d.dependsOn { - if dep == event.GNSS { //TODO: fow now no subscription for pps + if dep == event.GNSS { // TODO: fow now no subscription for pps dependingProcessStateMap.states[dep] = event.PTP_UNKNOWN // register to event notification from other processes d.subscriber = append(d.subscriber, &DpllSubscriber{source: dep, dpll: d, id: fmt.Sprintf("%s-%x", event.DPLL, d.clockId)}) @@ -649,14 +736,7 @@ func (d *DpllConfig) MonitorDpll() { // stateDecision func (d *DpllConfig) stateDecision() { - var dpllStatus int64 - switch { - case d.hasPTPAsSource(): - // For T-BC EEC DPLL state is not taken into account - dpllStatus = d.phaseStatus - default: - dpllStatus = d.getWorseState(d.phaseStatus, d.frequencyStatus) - } + dpllStatus := d.getDpllState() switch dpllStatus { case DPLL_FREERUN, DPLL_INVALID, DPLL_UNKNOWN: @@ -717,14 +797,7 @@ func (d *DpllConfig) stateDecision() { } } - case DPLL_LOCKED_HO_ACQ: - if d.isOffsetInRange() { - d.state = event.PTP_LOCKED - d.inSpec = true - } else { - d.state = event.PTP_FREERUN - d.sourceLost = false // phase offset will be the one that was read - } + case DPLL_LOCKED_HO_ACQ, DPLL_LOCKED: if d.isOffsetInRange() { glog.Infof("%s dpll is locked, source is not lost, offset is in range, state is DPLL_LOCKED_HO_ACQ", d.iface) if d.hasLeadingSource() && d.onHoldover { @@ -764,9 +837,6 @@ func (d *DpllConfig) sendDpllEvent() { IFace: d.iface, CfgName: d.processConfig.ConfigName, Values: map[event.ValueType]interface{}{ - event.FREQUENCY_STATUS: d.frequencyStatus, - event.OFFSET: d.phaseOffset, - event.PHASE_STATUS: d.phaseStatus, event.PPS_STATUS: func() int { if d.sourceLost { return 0 @@ -787,10 +857,19 @@ func (d *DpllConfig) sendDpllEvent() { WriteToLog: true, Reset: false, } + if !d.hasFlag(FlagNoFreqencyStatus) { + eventData.Values[event.FREQUENCY_STATUS] = d.frequencyStatus + } + if !d.hasFlag(FlagNoPhaseStatus) { + eventData.Values[event.PHASE_STATUS] = d.phaseStatus + } + if !d.hasFlag(FlagNoPhaseOffset) { + eventData.Values[event.OFFSET] = d.phaseOffset + } select { case d.processConfig.EventChannel <- eventData: - glog.Infof("dpll event sent for (%s): state %v, Offset %d, In spec %v, Source %v lost %v, On holdover %v", - d.iface, d.state, d.phaseOffset, d.inSpec, d.dependsOn[0], d.sourceLost, d.onHoldover) + glog.Infof("dpll event sent for (%s): state %v, Offset %s, In spec %v, Source %v lost %v, On holdover %v", + d.iface, d.state, d.phaseOffsetStr(), d.inSpec, d.dependsOn[0], d.sourceLost, d.onHoldover) default: glog.Infof("failed to send dpll event, retying.(%s)", d.iface) } @@ -847,6 +926,23 @@ func (d *DpllConfig) sendDpllTerminationEvent() { d.unRegisterAllSubscriber() } +func (d *DpllConfig) getDpllState() int64 { + switch { + case d.hasPTPAsSource(): + // For T-BC EEC DPLL state is not taken into account + return d.phaseStatus + case d.hasFlag(FlagNoPhaseStatus): + // Special case if there is no Phase Status (pps) for this DPLL + return d.frequencyStatus + case d.hasFlag(FlagNoFreqencyStatus): + // Special case if there is no Frequency Status (eec) for this DPLL + return d.phaseStatus + default: + // Normal case: Worst state of phase or frequency status + return d.getWorseState(d.phaseStatus, d.frequencyStatus) + } +} + // getStateQuality maps the state with relatively worse signal quality with // a lower number for easy comparison // Ref: ITU-T G.781 section 6.3.1 Auto selection operation @@ -888,7 +984,7 @@ func (d *DpllConfig) holdover() { if d.hasGNSSAsSource() { //nolint:all if d.frequencyTraceable { - //TODO: not implemented : add when syncE is handled here + // TODO: not implemented : add when syncE is handled here // use !d.isInSpecOffsetInRange() to declare HOLDOVER with clockClass 140 // !d.isMaxHoldoverOffsetInRange() for clock class to move from 140 to 248 and event to FREERUN } else if !d.isInSpecOffsetInRange() { // when holdover verify with local max holdover not with regular threshold @@ -921,6 +1017,10 @@ func (d *DpllConfig) holdover() { } func (d *DpllConfig) isMaxHoldoverOffsetInRange() bool { + if d.hasFlag(FlagNoPhaseOffset) { + // Special case when the DPLL has no reported phase offset + return true + } if d.phaseOffset <= int64(d.LocalMaxHoldoverOffSet) { return true } @@ -928,7 +1028,12 @@ func (d *DpllConfig) isMaxHoldoverOffsetInRange() bool { d.LocalMaxHoldoverOffSet, d.phaseOffset) return false } + func (d *DpllConfig) isInSpecOffsetInRange() bool { + if d.hasFlag(FlagNoPhaseOffset) { + // Special case when the DPLL has no reported phase offset + return true + } if d.phaseOffset <= int64(d.MaxInSpecOffset) { return true } @@ -938,6 +1043,10 @@ func (d *DpllConfig) isInSpecOffsetInRange() bool { } func (d *DpllConfig) isOffsetInRange() bool { + if d.hasFlag(FlagNoPhaseOffset) { + // Special case when the DPLL has no reported phase offset + return true + } if d.phaseOffset <= d.processConfig.GMThreshold.Max && d.phaseOffset >= d.processConfig.GMThreshold.Min { return true } @@ -1023,3 +1132,8 @@ func CalculateTimer(nodeProfile *ptpv1.PtpProfile) (int64, int64, int64, int64, func PtpSettingsDpllIgnoreKey(iface string) string { return fmt.Sprintf("dpll.%s.ignore", iface) } + +// PtpSettingsDpllFlagsKey returns the PtpSettings key to set DPLL behavioral flags for the given interface name: +func PtpSettingsDpllFlagsKey(iface string) string { + return fmt.Sprintf("dpll.%s.flags", iface) +} diff --git a/pkg/dpll/dpll_test.go b/pkg/dpll/dpll_test.go index 9aae036f..072c1a02 100644 --- a/pkg/dpll/dpll_test.go +++ b/pkg/dpll/dpll_test.go @@ -185,7 +185,7 @@ func TestDpllConfig_MonitorProcessGNSS(t *testing.T) { // event has to be running before dpll is started eventProcessor := event.Init("node", false, "/tmp/go.sock", eChannel, closeChn, nil, nil, nil) d := dpll.NewDpll(clockid, 10, 2, 5, "ens01", - []event.EventSource{event.GNSS}, dpll.MOCK, map[string]map[string]string{}, 0, 0) + []event.EventSource{event.GNSS}, dpll.MOCK, map[string]map[string]string{}, 0, 0, 0) d.CmdInit() eventChannel := make(chan event.EventChannel, 10) go eventProcessor.ProcessEvents() @@ -228,7 +228,7 @@ func TestDpllConfig_MonitorProcessPPS(t *testing.T) { // event has to be running before dpll is started eventProcessor := event.Init("node", false, "/tmp/go.sock", eChannel, closeChn, nil, nil, nil) d := dpll.NewDpll(clockid, 10, 2, 5, "ens01", - []event.EventSource{event.GNSS}, dpll.MOCK, map[string]map[string]string{}, 0, 0) + []event.EventSource{event.GNSS}, dpll.MOCK, map[string]map[string]string{}, 0, 0, 0) d.CmdInit() eventChannel := make(chan event.EventChannel, 10) go eventProcessor.ProcessEvents() @@ -292,7 +292,7 @@ func TestSlopeAndTimer(t *testing.T) { } for _, tt := range testCase { d := dpll.NewDpll(100, tt.localMaxHoldoverOffSet, tt.localHoldoverTimeout, tt.maxInSpecOffset, - "test", []event.EventSource{}, dpll.MOCK, map[string]map[string]string{}, 0, 0) + "test", []event.EventSource{}, dpll.MOCK, map[string]map[string]string{}, 0, 0, 0) assert.Equal(t, tt.localMaxHoldoverOffSet, d.LocalMaxHoldoverOffSet, "localMaxHoldover offset") assert.Equal(t, tt.localHoldoverTimeout, d.LocalHoldoverTimeout, "Local holdover timeout") assert.Equal(t, tt.maxInSpecOffset, d.MaxInSpecOffset, "Max In Spec Offset") From 7ef49fedcb60d4148fba4d378bfb00cedae6cc90 Mon Sep 17 00:00:00 2001 From: nocturnalastro Date: Thu, 5 Mar 2026 13:10:57 +0000 Subject: [PATCH 4/9] Remove pointer to pin commands Assisted-by: Cursor --- addons/intel/clock-chain.go | 15 ++++++++------- addons/intel/clock-chain_test.go | 14 +++++++------- addons/intel/dpllPins.go | 2 +- addons/intel/dpllPins_test.go | 2 +- addons/intel/e825.go | 2 +- addons/intel/e825_test.go | 6 +++--- addons/intel/mock_test.go | 14 +++++--------- 7 files changed, 26 insertions(+), 29 deletions(-) diff --git a/addons/intel/clock-chain.go b/addons/intel/clock-chain.go index 7c9b4ba4..e94b262a 100644 --- a/addons/intel/clock-chain.go +++ b/addons/intel/clock-chain.go @@ -232,7 +232,8 @@ func writeSysFs(path string, val string) error { return nil } -func (c *ClockChain) SetPinsControl(pins []PinControl) (*[]dpll.PinParentDeviceCtl, error) { +// SetPinsControl builds DPLL netlink commands for the given pins on the leading NIC. +func (c *ClockChain) SetPinsControl(pins []PinControl) ([]dpll.PinParentDeviceCtl, error) { pinCommands := []dpll.PinParentDeviceCtl{} for _, pinCtl := range pins { dpllPin := c.DpllPins.GetByLabel(pinCtl.Label, c.LeadingNIC.DpllClockID) @@ -242,12 +243,12 @@ func (c *ClockChain) SetPinsControl(pins []PinControl) (*[]dpll.PinParentDeviceC } pinCommands = append(pinCommands, SetPinControlData(*dpllPin, pinCtl.ParentControl)...) } - return &pinCommands, nil + return pinCommands, nil } // SetPinsControlForAllNICs sets pins across all NICs (leading + other NICs) // This is used specifically for initialization functions like SetPinDefaults -func (c *ClockChain) SetPinsControlForAllNICs(pins []PinControl) (*[]dpll.PinParentDeviceCtl, error) { +func (c *ClockChain) SetPinsControlForAllNICs(pins []PinControl) ([]dpll.PinParentDeviceCtl, error) { pinCommands := []dpll.PinParentDeviceCtl{} errs := make([]error, 0) @@ -266,7 +267,7 @@ func (c *ClockChain) SetPinsControlForAllNICs(pins []PinControl) (*[]dpll.PinPar } } - return &pinCommands, errors.Join(errs...) + return pinCommands, errors.Join(errs...) } // buildDirectionCmd checks if any parent device direction is changing. @@ -437,7 +438,7 @@ func (c *ClockChain) InitPinsTBC() error { if err != nil { glog.Error("failed to set pins control: ", err) } - *commands = append(*commands, *commandsGnss...) + commands = append(commands, commandsGnss...) err = BatchPinSet(commands) // even if there was an error we still need to refresh the pin state. @@ -603,14 +604,14 @@ func (c *ClockChain) SetPinDefaults() error { // BatchPinSet function pointer allows mocking of BatchPinSet var BatchPinSet = batchPinSet -func batchPinSet(commands *[]dpll.PinParentDeviceCtl) error { +func batchPinSet(commands []dpll.PinParentDeviceCtl) error { conn, err := dpll.Dial(nil) if err != nil { return fmt.Errorf("failed to dial DPLL: %v", err) } //nolint:errcheck defer conn.Close() - for _, command := range *commands { + for _, command := range commands { glog.Infof("DPLL pin command %#v", command) b, err := dpll.EncodePinControl(command) if err != nil { diff --git a/addons/intel/clock-chain_test.go b/addons/intel/clock-chain_test.go index f37b361e..446adb99 100644 --- a/addons/intel/clock-chain_test.go +++ b/addons/intel/clock-chain_test.go @@ -44,19 +44,19 @@ func Test_ProcessProfileTbcClockChain(t *testing.T) { assert.Equal(t, 0, mockPinConfig.actualPinSetCount) assert.Equal(t, 0, mockPinConfig.actualPinFrqCount) assert.NotNil(t, mockPinSet.commands, "DPLL commands should have been issued") - assert.Greater(t, len(*mockPinSet.commands), 0, "should have DPLL pin commands") + assert.Greater(t, len(mockPinSet.commands), 0, "should have DPLL pin commands") // Test holdover entry mockPinSet.reset() err = clockChain.EnterHoldoverTBC() assert.NoError(t, err) - assert.Equal(t, 2, len(*mockPinSet.commands)) + assert.Equal(t, 2, len(mockPinSet.commands)) // Test holdover exit mockPinSet.reset() err = clockChain.EnterNormalTBC() assert.NoError(t, err) - assert.Equal(t, 2, len(*mockPinSet.commands)) + assert.Equal(t, 2, len(mockPinSet.commands)) // Ensure switching back to TGM resets any pins mockPinSet.reset() @@ -109,13 +109,13 @@ func Test_ProcessProfileTtscClockChain(t *testing.T) { mockPinSet.reset() err = clockChain.EnterHoldoverTBC() assert.NoError(t, err) - assert.Equal(t, 2, len(*mockPinSet.commands)) + assert.Equal(t, 2, len(mockPinSet.commands)) // Test holdover exit mockPinSet.reset() err = clockChain.EnterNormalTBC() assert.NoError(t, err) - assert.Equal(t, 2, len(*mockPinSet.commands)) + assert.Equal(t, 2, len(mockPinSet.commands)) } func Test_SetPinDefaults_AllNICs(t *testing.T) { @@ -163,14 +163,14 @@ func Test_SetPinDefaults_AllNICs(t *testing.T) { // SetPinDefaults configures 9 different pin types, and we have 3 NICs total // Each pin type should have a command for each NIC that has that pin - assert.Equal(t, len(*mockPinSet.commands), 27, "should have exactly 27 pin commands") + assert.Equal(t, len(mockPinSet.commands), 27, "should have exactly 27 pin commands") // Verify that commands include pins from multiple clock IDs clockIDsSeen := make(map[uint64]bool) pinLabelsSeen := make(map[string]bool) mockPins := ccData.DpllPins.(*mockedDPLLPins) - for _, cmd := range *mockPinSet.commands { + for _, cmd := range mockPinSet.commands { // Find which pin this command refers to by searching all pins for _, pin := range mockPins.pins { if pin.ID == cmd.ID { diff --git a/addons/intel/dpllPins.go b/addons/intel/dpllPins.go index 1220e7d8..8a5a3058 100644 --- a/addons/intel/dpllPins.go +++ b/addons/intel/dpllPins.go @@ -136,7 +136,7 @@ func (d *dpllPins) GetCommandsForPluginPinSet(clockID uint64, pinset pinSet) []d } func (d *dpllPins) ApplyPinCommands(commands []dpll.PinParentDeviceCtl) error { - err := BatchPinSet(&commands) + err := BatchPinSet(commands) // event if there was an error we still need to refresh the pin state. fetchErr := d.FetchPins() return errors.Join(err, fetchErr) diff --git a/addons/intel/dpllPins_test.go b/addons/intel/dpllPins_test.go index 880b7cd4..e4d99cac 100644 --- a/addons/intel/dpllPins_test.go +++ b/addons/intel/dpllPins_test.go @@ -459,5 +459,5 @@ func TestApplyPinCommands(t *testing.T) { err := DpllPins.ApplyPinCommands(cmds) assert.NoError(t, err) assert.NotNil(t, mockPinSet.commands) - assert.Len(t, *mockPinSet.commands, 1) + assert.Len(t, mockPinSet.commands, 1) } diff --git a/addons/intel/e825.go b/addons/intel/e825.go index 09ffa69e..9c1c3ba3 100644 --- a/addons/intel/e825.go +++ b/addons/intel/e825.go @@ -219,7 +219,7 @@ func (d *E825PluginData) setupGnss(gnss GnssOptions) error { return errors.New("no GNSS pins found") } glog.Infof("Will %s %d GNSS pins: %v", action, len(commands), affectedPins) - return BatchPinSet(&commands) + return BatchPinSet(commands) } // AfterRunPTPCommandE825 performs actions after certain PTP commands for e825 plugin diff --git a/addons/intel/e825_test.go b/addons/intel/e825_test.go index d6cc180a..858d2929 100644 --- a/addons/intel/e825_test.go +++ b/addons/intel/e825_test.go @@ -264,12 +264,12 @@ func Test_setupGnss(t *testing.T) { assert.Error(tt, err) } else { assert.NoError(tt, err) - assert.Equal(tt, tc.expectedCmdCount, len(*mockPinSet.commands)) + assert.Equal(tt, tc.expectedCmdCount, len(mockPinSet.commands)) expectedState := uint32(dpll.PinStateSelectable) if tc.gnss.Disabled { expectedState = uint32(dpll.PinStateDisconnected) } - for _, cmd := range *mockPinSet.commands { + for _, cmd := range mockPinSet.commands { for _, ctrl := range cmd.PinParentCtl { assert.Equal(tt, expectedState, *ctrl.State) } @@ -334,7 +334,7 @@ func Test_OnPTPConfigChangeE825(t *testing.T) { assert.NoError(tt, err) assert.Equal(tt, tc.expectedPinSets, mockPins.actualPinSetCount) assert.Equal(tt, tc.expectedPinFrqs, mockPins.actualPinFrqCount) - assert.Equal(tt, 1, len(*mockDpllPinset.commands)) + assert.Equal(tt, 1, len(mockDpllPinset.commands)) } }) } diff --git a/addons/intel/mock_test.go b/addons/intel/mock_test.go index aba203c4..0f13dd12 100644 --- a/addons/intel/mock_test.go +++ b/addons/intel/mock_test.go @@ -16,20 +16,16 @@ import ( // mockBatchPinSet is a simple mock to unit-test pin set operations type mockBatchPinSet struct { - commands *[]dpll.PinParentDeviceCtl + commands []dpll.PinParentDeviceCtl } -func (m *mockBatchPinSet) mock(commands *[]dpll.PinParentDeviceCtl) error { - if m.commands == nil { - cmds := make([]dpll.PinParentDeviceCtl, 0) - m.commands = &cmds - } - *m.commands = append(*m.commands, *commands...) +func (m *mockBatchPinSet) mock(commands []dpll.PinParentDeviceCtl) error { + m.commands = append(m.commands, commands...) return nil } func (m *mockBatchPinSet) reset() { - m.commands = nil + m.commands = m.commands[:0] } func setupBatchPinSetMock() (*mockBatchPinSet, func()) { @@ -356,7 +352,7 @@ func (m *mockedDPLLPins) GetCommandsForPluginPinSet(clockID uint64, pinset pinSe } func (m *mockedDPLLPins) ApplyPinCommands(commands []dpll.PinParentDeviceCtl) error { - return BatchPinSet(&commands) + return BatchPinSet(commands) } func setupMockDPLLPins(pins ...*dpll.PinInfo) (*mockedDPLLPins, func()) { From 57dbd7639566efd5beae28f11e75bd0cdc4f07df Mon Sep 17 00:00:00 2001 From: nocturnalastro Date: Mon, 9 Mar 2026 10:01:45 +0000 Subject: [PATCH 5/9] Add ts2phc.pin_index 1 when sysfs SMA pins are missing --- addons/intel/e810.go | 43 +++++++++++++++ addons/intel/e810_test.go | 110 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 153 insertions(+) diff --git a/addons/intel/e810.go b/addons/intel/e810.go index 18e64a78..717e8ba1 100644 --- a/addons/intel/e810.go +++ b/addons/intel/e810.go @@ -44,6 +44,7 @@ func OnPTPConfigChangeE810(data *interface{}, nodeProfile *ptpv1.PtpProfile) err glog.Info("calling onPTPConfigChange for e810 plugin") autoDetectGNSSSerialPort(nodeProfile) + checkPinIndex(nodeProfile) var e810Opts E810Opts var err error @@ -243,3 +244,45 @@ func pinSetHasSMAInput(pins pinSet) bool { } return false } + +func checkPinIndex(nodeProfile *ptpv1.PtpProfile) { + if nodeProfile.Ts2PhcConf == nil { + return + } + + profileName := "" + if nodeProfile.Name != nil { + profileName = *nodeProfile.Name + } + + lines := strings.Split(*nodeProfile.Ts2PhcConf, "\n") + result := make([]string, 0, len(lines)+1) + shouldAddPinIndex := false + for _, line := range lines { + trimedLine := strings.TrimSpace(line) + if strings.HasPrefix(trimedLine, "[") && strings.HasSuffix(trimedLine, "]") { + // We went through the previous entry and didn't find a pin index + if shouldAddPinIndex { + glog.Infof("Adding 'ts2phc.pin_index 1' to ts2phc for profile name %s", profileName) + result = append(result, "ts2phc.pin_index 1") + shouldAddPinIndex = false + } + + ifName := strings.TrimSpace(strings.TrimRight(strings.TrimLeft(trimedLine, "["), "]")) + if ifName != "global" && ifName != "nmea" && !hasSysfsSMAPins(ifName) { + shouldAddPinIndex = true + } + } + if strings.HasPrefix(trimedLine, "ts2phc.pin_index") || strings.HasPrefix(trimedLine, "ts2phc.pin_name") { + shouldAddPinIndex = false + } + result = append(result, line) + } + if shouldAddPinIndex { + glog.Infof("Adding 'ts2phc.pin_index 1' to ts2phc for profile name %s", profileName) + result = append(result, "ts2phc.pin_index 1") + } + + updatedTs2phcConfig := strings.Join(result, "\n") + nodeProfile.Ts2PhcConf = &updatedTs2phcConfig +} diff --git a/addons/intel/e810_test.go b/addons/intel/e810_test.go index 0903bfe4..445789b1 100644 --- a/addons/intel/e810_test.go +++ b/addons/intel/e810_test.go @@ -470,6 +470,116 @@ func TestDevicePins_DPLL_NoSMAInput_NoGNSSCommand(t *testing.T) { } } +func Test_checkPinIndex(t *testing.T) { + strPtr := func(s string) *string { return &s } + + tests := []struct { + name string + ts2phcConf *string + setupMock func(*MockFileSystem) + expectedConf *string + }{ + { + name: "nil Ts2PhcConf is a no-op", + ts2phcConf: nil, + expectedConf: nil, + }, + { + name: "interface section without pin_index and no SMA pins gets pin_index added", + ts2phcConf: strPtr("[global]\nts2phc.nmea_serialport /dev/gnss0\n[ens4f0]\nts2phc.extts_polarity rising"), + setupMock: func(m *MockFileSystem) { + m.AllowReadDir("/sys/class/net/ens4f0/device/ptp/", []os.DirEntry{MockDirEntry{name: "ptp0", isDir: true}}, nil) + m.AllowReadFile("/sys/class/net/ens4f0/device/ptp/ptp0/pins/SMA1", nil, os.ErrNotExist) + }, + expectedConf: strPtr("[global]\nts2phc.nmea_serialport /dev/gnss0\n[ens4f0]\nts2phc.extts_polarity rising\nts2phc.pin_index 1"), + }, + { + name: "interface section with existing pin_index is not duplicated", + ts2phcConf: strPtr("[global]\n[ens4f0]\nts2phc.pin_index 0\nts2phc.extts_polarity rising"), + setupMock: func(m *MockFileSystem) { + m.AllowReadDir("/sys/class/net/ens4f0/device/ptp/", []os.DirEntry{MockDirEntry{name: "ptp0", isDir: true}}, nil) + m.AllowReadFile("/sys/class/net/ens4f0/device/ptp/ptp0/pins/SMA1", nil, os.ErrNotExist) + }, + expectedConf: strPtr("[global]\n[ens4f0]\nts2phc.pin_index 0\nts2phc.extts_polarity rising"), + }, + { + name: "global section does not get pin_index", + ts2phcConf: strPtr("[global]\nts2phc.nmea_serialport /dev/gnss0"), + expectedConf: strPtr("[global]\nts2phc.nmea_serialport /dev/gnss0"), + }, + { + name: "nmea section does not get pin_index", + ts2phcConf: strPtr("[nmea]\nts2phc.master 1"), + expectedConf: strPtr("[nmea]\nts2phc.master 1"), + }, + { + name: "interface with SMA pins does not get pin_index", + ts2phcConf: strPtr("[ens4f0]\nts2phc.extts_polarity rising"), + setupMock: func(m *MockFileSystem) { + m.AllowReadDir("/sys/class/net/ens4f0/device/ptp/", []os.DirEntry{MockDirEntry{name: "ptp0", isDir: true}}, nil) + m.AllowReadFile("/sys/class/net/ens4f0/device/ptp/ptp0/pins/SMA1", []byte("0 1"), nil) + }, + expectedConf: strPtr("[ens4f0]\nts2phc.extts_polarity rising"), + }, + { + name: "multiple interfaces - pin_index added only where needed", + ts2phcConf: strPtr("[global]\nts2phc.nmea_serialport /dev/gnss0\n[ens4f0]\nts2phc.extts_polarity rising\n[ens4f1]\nts2phc.pin_index 0\nts2phc.extts_polarity rising"), + setupMock: func(m *MockFileSystem) { + m.AllowReadDir("/sys/class/net/ens4f0/device/ptp/", []os.DirEntry{MockDirEntry{name: "ptp0", isDir: true}}, nil) + m.AllowReadFile("/sys/class/net/ens4f0/device/ptp/ptp0/pins/SMA1", nil, os.ErrNotExist) + m.AllowReadDir("/sys/class/net/ens4f1/device/ptp/", []os.DirEntry{MockDirEntry{name: "ptp0", isDir: true}}, nil) + m.AllowReadFile("/sys/class/net/ens4f1/device/ptp/ptp0/pins/SMA1", nil, os.ErrNotExist) + }, + expectedConf: strPtr("[global]\nts2phc.nmea_serialport /dev/gnss0\n[ens4f0]\nts2phc.extts_polarity rising\nts2phc.pin_index 1\n[ens4f1]\nts2phc.pin_index 0\nts2phc.extts_polarity rising"), + }, + { + name: "empty config string is unchanged", + ts2phcConf: strPtr(""), + expectedConf: strPtr(""), + }, + { + name: "pin_index added to last section when at end of file", + ts2phcConf: strPtr("[global]\n[ens4f0]\nts2phc.extts_polarity rising"), + setupMock: func(m *MockFileSystem) { + m.AllowReadDir("/sys/class/net/ens4f0/device/ptp/", []os.DirEntry{MockDirEntry{name: "ptp0", isDir: true}}, nil) + m.AllowReadFile("/sys/class/net/ens4f0/device/ptp/ptp0/pins/SMA1", nil, os.ErrNotExist) + }, + expectedConf: strPtr("[global]\n[ens4f0]\nts2phc.extts_polarity rising\nts2phc.pin_index 1"), + }, + { + name: "hasSysfsSMAPins returns false when ReadDir fails", + ts2phcConf: strPtr("[ens4f0]\nts2phc.extts_polarity rising"), + setupMock: func(m *MockFileSystem) { + m.AllowReadDir("/sys/class/net/ens4f0/device/ptp/", nil, fmt.Errorf("no such directory")) + }, + expectedConf: strPtr("[ens4f0]\nts2phc.extts_polarity rising\nts2phc.pin_index 1"), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mockFS, restoreFs := setupMockFS() + defer restoreFs() + if tt.setupMock != nil { + tt.setupMock(mockFS) + } + + profile := &ptpv1.PtpProfile{ + Ts2PhcConf: tt.ts2phcConf, + } + + checkPinIndex(profile) + + if tt.expectedConf == nil { + assert.Nil(t, profile.Ts2PhcConf) + } else { + assert.NotNil(t, profile.Ts2PhcConf) + assert.Equal(t, *tt.expectedConf, *profile.Ts2PhcConf) + } + }) + } +} + func Test_PopulateHwConfdigE810(t *testing.T) { p, d := E810("e810") data := (*d).(*E810PluginData) From 6facf71ea430a66c53a91ab0633a2859b9d8ba5b Mon Sep 17 00:00:00 2001 From: Micky Costa Date: Mon, 16 Mar 2026 14:31:14 +0000 Subject: [PATCH 6/9] Merge pull request #167 from nocturnalastro/set_pin_index Set pin index --- addons/intel/clockID.go | 3 +-- addons/intel/clockID_test.go | 11 +++++++++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/addons/intel/clockID.go b/addons/intel/clockID.go index e96cc774..4ca9366a 100644 --- a/addons/intel/clockID.go +++ b/addons/intel/clockID.go @@ -38,8 +38,7 @@ func getPCIClockID(device string) uint64 { var offset uint16 = pciConfigSpaceSize var id uint16 for { - // TODO: Add test for == case - if len(b) <= int(offset) { + if len(b) < int(offset)+pciExtendedCapabilityDataOffset { glog.Errorf("PCI config space too short (%d bytes) for device %s", len(b), device) return 0 } diff --git a/addons/intel/clockID_test.go b/addons/intel/clockID_test.go index a3e21ed4..678ec680 100644 --- a/addons/intel/clockID_test.go +++ b/addons/intel/clockID_test.go @@ -77,6 +77,17 @@ func Test_getPCIClockID(t *testing.T) { clockID = getPCIClockID("truncated") assert.Equal(t, notFound, clockID) mfs.VerifyAllCalls(t) + + // Config space barely holds capability ID but not the next-offset field; + // without a proper bounds check this would panic in Uint16(b[offset+2:]). + shortCap := make([]byte, pciConfigSpaceSize+2) + binary.LittleEndian.PutUint16(shortCap[pciConfigSpaceSize:], uint16(pciExtendedCapabilityDsnID+1)) + mfs.ExpectReadFile("/sys/class/net/short_cap/device/config", shortCap, nil) + assert.NotPanics(t, func() { + clockID = getPCIClockID("short_cap") + }) + assert.Equal(t, notFound, clockID) + mfs.VerifyAllCalls(t) } func Test_getClockIDByModule(t *testing.T) { From 7c5ac7a845b510cf0f686475a99401f9e092d941 Mon Sep 17 00:00:00 2001 From: Vitaly Grinberg Date: Tue, 24 Mar 2026 15:02:08 +0200 Subject: [PATCH 7/9] Improve DPLL pin commands printing --- addons/intel/clock-chain.go | 2 +- pkg/dpll-netlink/dpll-uapi.go | 53 +++++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 1 deletion(-) diff --git a/addons/intel/clock-chain.go b/addons/intel/clock-chain.go index e94b262a..9fb0ec01 100644 --- a/addons/intel/clock-chain.go +++ b/addons/intel/clock-chain.go @@ -612,7 +612,7 @@ func batchPinSet(commands []dpll.PinParentDeviceCtl) error { //nolint:errcheck defer conn.Close() for _, command := range commands { - glog.Infof("DPLL pin command %#v", command) + glog.Infof("DPLL pin command %s", command.String()) b, err := dpll.EncodePinControl(command) if err != nil { return err diff --git a/pkg/dpll-netlink/dpll-uapi.go b/pkg/dpll-netlink/dpll-uapi.go index e568400f..b88c6c90 100644 --- a/pkg/dpll-netlink/dpll-uapi.go +++ b/pkg/dpll-netlink/dpll-uapi.go @@ -322,6 +322,59 @@ func GetPinDirection(d uint32) string { return "" } +// String returns a concise debug representation aligned with PinParentDeviceHR +// field semantics (direction/state as names when known). +func (p PinControl) String() string { + var b strings.Builder + fmt.Fprintf(&b, "parentID=%d", p.PinParentID) + if p.Direction != nil { + if d := GetPinDirection(*p.Direction); d != "" { + fmt.Fprintf(&b, " direction=%s", d) + } else { + fmt.Fprintf(&b, " direction=%d", *p.Direction) + } + } + if p.Prio != nil { + fmt.Fprintf(&b, " prio=%d", *p.Prio) + } + if p.State != nil { + if s := GetPinState(*p.State); s != "" { + fmt.Fprintf(&b, " state=%s", s) + } else { + fmt.Fprintf(&b, " state=%d", *p.State) + } + } + return b.String() +} + +// String returns a concise debug representation of the pin parent device control request. +func (p PinParentDeviceCtl) String() string { + var b strings.Builder + b.WriteString("PinParentDeviceCtl{") + fmt.Fprintf(&b, "id=%d", p.ID) + if p.Frequency != nil { + fmt.Fprintf(&b, " frequency=%d", *p.Frequency) + } + if p.PhaseAdjust != nil { + fmt.Fprintf(&b, " phaseAdjust=%d", *p.PhaseAdjust) + } + if p.EsyncFrequency != nil { + fmt.Fprintf(&b, " esyncFrequency=%d", *p.EsyncFrequency) + } + if len(p.PinParentCtl) > 0 { + b.WriteString(" pinParent=[") + for i := range p.PinParentCtl { + if i > 0 { + b.WriteString(", ") + } + b.WriteString(p.PinParentCtl[i].String()) + } + b.WriteString("]") + } + b.WriteString("}") + return b.String() +} + // Defines pin capabilities const ( PinCapNone = 0 From 3c1e52b3713e4dde1ba2583e560f116b40e4d45c Mon Sep 17 00:00:00 2001 From: Vitaly Grinberg Date: Tue, 24 Mar 2026 16:31:59 +0200 Subject: [PATCH 8/9] add SDP22 channel assignment --- addons/intel/clock-chain.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/addons/intel/clock-chain.go b/addons/intel/clock-chain.go index 9fb0ec01..d4045e4b 100644 --- a/addons/intel/clock-chain.go +++ b/addons/intel/clock-chain.go @@ -367,6 +367,11 @@ func (c *ClockChain) EnableE810Outputs() error { glog.Errorf("failed to set SMA2 pin via sysfs: %s", err) } } else { + // This is to assign channel 2 to SDP22. The period command below will fail without it + err = pinConfig.applyPinSet(c.LeadingNIC.Name, pinSet{"SDP22": "2 2"}) + if err != nil { + glog.Errorf("failed to set SDP22 pin via sysfs: %s", err) + } sma2Cmds := c.DpllPins.GetCommandsForPluginPinSet(c.LeadingNIC.DpllClockID, map[string]string{"SMA2": "2 2"}) err = c.DpllPins.ApplyPinCommands(sma2Cmds) if err != nil { From fb35b8212aec2b0071cf59c22a68be3b6c2c0c4e Mon Sep 17 00:00:00 2001 From: Vitaly Grinberg Date: Wed, 25 Mar 2026 10:51:38 +0200 Subject: [PATCH 9/9] fix unit tests for pin compatibility --- addons/intel/clock-chain_test.go | 7 ++++--- addons/intel/e810_test.go | 2 +- addons/intel/mock_test.go | 25 +++++++++++++++++++++++++ 3 files changed, 30 insertions(+), 4 deletions(-) diff --git a/addons/intel/clock-chain_test.go b/addons/intel/clock-chain_test.go index 446adb99..0aba0669 100644 --- a/addons/intel/clock-chain_test.go +++ b/addons/intel/clock-chain_test.go @@ -22,6 +22,7 @@ func Test_ProcessProfileTbcClockChain(t *testing.T) { // EnableE810Outputs is called for the leading NIC (ens4f0) - needs specific paths mockFS.AllowReadDir("/sys/class/net/ens4f0/device/ptp/", phcEntries, nil) mockFS.AllowReadFile("/sys/class/net/ens4f0/device/ptp/ptp0/pins/SMA1", nil, os.ErrNotExist) + // mock applyPinSet does not hit the filesystem; only the period write from EnableE810Outputs is real. mockFS.ExpectWriteFile("/sys/class/net/ens4f0/device/ptp/ptp0/period", []byte("2 0 0 1 0"), os.FileMode(0o666), nil) mockPinConfig, restorePins := setupMockPinConfig() @@ -41,7 +42,7 @@ func Test_ProcessProfileTbcClockChain(t *testing.T) { assert.Equal(t, ClockTypeTBC, ccData.Type, "identified a wrong clock type") assert.Equal(t, uint64(5799633565432596414), ccData.LeadingNIC.DpllClockID, "identified a wrong clock ID ") assert.Equal(t, "ens4f1", ccData.LeadingNIC.UpstreamPort, "wrong upstream port") - assert.Equal(t, 0, mockPinConfig.actualPinSetCount) + assert.Equal(t, 1, mockPinConfig.actualPinSetCount, "SDP22 sysfs channel assignment for 1PPS") assert.Equal(t, 0, mockPinConfig.actualPinFrqCount) assert.NotNil(t, mockPinSet.commands, "DPLL commands should have been issued") assert.Greater(t, len(mockPinSet.commands), 0, "should have DPLL pin commands") @@ -102,7 +103,7 @@ func Test_ProcessProfileTtscClockChain(t *testing.T) { assert.Equal(t, uint64(5799633565432596414), ccData.LeadingNIC.DpllClockID, "identified a wrong clock ID ") assert.Equal(t, "ens4f1", ccData.LeadingNIC.UpstreamPort, "wrong upstream port") assert.NotNil(t, mockPinSet.commands, "Ensure some pins were set") - assert.Equal(t, 0, mockPinConfig.actualPinSetCount) + assert.Equal(t, 1, mockPinConfig.actualPinSetCount, "SDP22 sysfs channel assignment for 1PPS") assert.Equal(t, 0, mockPinConfig.actualPinFrqCount) // Test holdover entry @@ -146,7 +147,7 @@ func Test_SetPinDefaults_AllNICs(t *testing.T) { // Initialize the clock chain with multiple NICs err = OnPTPConfigChangeE810(nil, profile) assert.NoError(t, err) - assert.Equal(t, 0, mockPinConfig.actualPinSetCount) + assert.Equal(t, 1, mockPinConfig.actualPinSetCount, "SDP22 sysfs channel assignment for 1PPS") assert.Equal(t, 0, mockPinConfig.actualPinFrqCount) // Verify we have the expected clock chain structure diff --git a/addons/intel/e810_test.go b/addons/intel/e810_test.go index 445789b1..7271f9ae 100644 --- a/addons/intel/e810_test.go +++ b/addons/intel/e810_test.go @@ -126,7 +126,7 @@ func Test_ProcessProfileTBCNoPhaseInputs(t *testing.T) { err = p.OnPTPConfigChange(d, profile) assert.NoError(t, err) - assert.Equal(t, 0, mockPinConfig.actualPinSetCount) + assert.Equal(t, 1, mockPinConfig.actualPinSetCount, "SDP22 sysfs channel assignment for 1PPS") assert.Equal(t, 0, mockPinConfig.actualPinFrqCount) // Verify that clockChain was initialized (SetPinDefaults is called as part of InitClockChain) diff --git a/addons/intel/mock_test.go b/addons/intel/mock_test.go index 0f13dd12..ddc3a142 100644 --- a/addons/intel/mock_test.go +++ b/addons/intel/mock_test.go @@ -362,6 +362,30 @@ func setupMockDPLLPins(pins ...*dpll.PinInfo) (*mockedDPLLPins, func()) { return mock, func() { DpllPins = orig } } +// expandPinsForPluginYAMLCompatibility adds in-memory PinInfo clones so tests can keep legacy +// plugin YAML keys (SMA2, U.FL1, U.FL2) while dpll-pins.json reports boardLabel "SMA2/U.FL2" +// and no separate U.FL1 DPLL pin (tests only; production JSON and YAML unchanged). +func expandPinsForPluginYAMLCompatibility(pins []*dpll.PinInfo) []*dpll.PinInfo { + out := make([]*dpll.PinInfo, 0, len(pins)+32) + out = append(out, pins...) + for _, p := range pins { + if p.BoardLabel == "SMA2/U.FL2" { + s2 := *p + s2.BoardLabel = "SMA2" + out = append(out, &s2) + u2 := *p + u2.BoardLabel = "U.FL2" + out = append(out, &u2) + } + if p.BoardLabel == "SMA1" && p.ModuleName == "ice" { + u1 := *p + u1.BoardLabel = "U.FL1" + out = append(out, &u1) + } + } + return out +} + func setupMockDPLLPinsFromJSON(path string) (*mockedDPLLPins, func()) { //nolint: unparam // it may be used for other pin files in the future it doesn't make the code overly complex pins := []dpll.PinInfo{} data, err := os.ReadFile(path) @@ -375,6 +399,7 @@ func setupMockDPLLPinsFromJSON(path string) (*mockedDPLLPins, func()) { //nolint for i := range pins { ptrs[i] = &pins[i] } + ptrs = expandPinsForPluginYAMLCompatibility(ptrs) return setupMockDPLLPins(ptrs...) }