Skip to content

[release-4.17] OCPBUGS-60193: Allow duplicate options in ptpconfig #478

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 28, 2025
Merged

[release-4.17] OCPBUGS-60193: Allow duplicate options in ptpconfig #478

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dcd4a43
Allow duplicate keys in ptpconfig
josephdrichard May 26, 2025
84a96cd
masterOnly 0 under [global] section shouldn't affect clockType
josephdrichard Jun 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
195 changes: 139 additions & 56 deletions pkg/daemon/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package daemon

import (
"bytes"
"encoding/json"
"errors"
"fmt"
Expand All @@ -19,6 +20,13 @@ import (
ptpv1 "github.com/openshift/ptp-operator/api/v1"
)

// predefined config section names
const (
GlobalSectionName = "[global]"
NmeaSectionName = "[nmea]"
UnicastSectionName = "[unicast_master_table]"
)

// LinuxPTPUpdate controls whether to update linuxPTP conf
// and contains linuxPTP conf to be updated. It's rendered
// and passed to linuxptp instance by daemon.
Expand All @@ -29,19 +37,77 @@ type LinuxPTPConfUpdate struct {
defaultPTP4lConfig []byte
}

type ptp4lConfOption struct {
key string
value string
}

type ptp4lConfSection struct {
sectionName string
options map[string]string
options []ptp4lConfOption
}

type ptp4lConf struct {
// Ptp4lConf is a structure to represent a parsed ptpconfig,
// which can then be rendered to a string again.
type Ptp4lConf struct {
sections []ptp4lConfSection
mapping []string
profile_name string
clock_type event.ClockType
gnss_serial_port string // gnss serial port
}

func (conf *Ptp4lConf) getPtp4lConfOptionOrEmptyString(sectionName string, key string) (string, bool) {
for _, section := range conf.sections {
if section.sectionName == sectionName {
for _, option := range section.options {
if option.key == key {
return option.value, true
}
}
}
}

return "", false
}

func (conf *Ptp4lConf) setPtp4lConfOption(sectionName string, key string, value string, overwrite bool) {
var updatedSection ptp4lConfSection
index := -1
for i, section := range conf.sections {
if section.sectionName == sectionName {
updatedSection = section
index = i
}
}
if index < 0 {
newSectionOptions := make([]ptp4lConfOption, 0)
updatedSection = ptp4lConfSection{options: newSectionOptions, sectionName: sectionName}
index = len(conf.sections)
conf.sections = append(conf.sections, updatedSection)
}

//Stop now if initializing section without option
if key == "" {
return
}
found := false
if overwrite {
for i := range updatedSection.options {
if updatedSection.options[i].key == key {
updatedSection.options[i] = ptp4lConfOption{key: key, value: value}
found = true
}
}
}
// Append unless already overwrote it.
if !found {
updatedSection.options = append(updatedSection.options, ptp4lConfOption{key: key, value: value})
}

//Update section in conf
conf.sections[index] = updatedSection
}

func NewLinuxPTPConfUpdate() (*LinuxPTPConfUpdate, error) {
if _, err := os.Stat(PTP4L_CONF_FILE_PATH); err != nil {
if os.IsNotExist(err) {
Expand All @@ -60,7 +126,7 @@ func NewLinuxPTPConfUpdate() (*LinuxPTPConfUpdate, error) {
}

func (l *LinuxPTPConfUpdate) UpdateConfig(nodeProfilesJson []byte) error {
if string(l.appliedNodeProfileJson) == string(nodeProfilesJson) {
if bytes.Equal(l.appliedNodeProfileJson, nodeProfilesJson) {
return nil
}
if nodeProfiles, ok := tryToLoadConfig(nodeProfilesJson); ok {
Expand Down Expand Up @@ -113,71 +179,82 @@ func tryToLoadOldConfig(nodeProfilesJson []byte) ([]ptpv1.PtpProfile, bool) {
return []ptpv1.PtpProfile{*ptpConfig}, true
}

// Takes as input a PtpProfile.Ptp4lConf and outputs as ptp4lConf struct
func (output *ptp4lConf) populatePtp4lConf(config *string) error {
// PopulatePtp4lConf takes as input a PtpProfile.Ptp4lConf string and outputs as ptp4lConf struct
func (conf *Ptp4lConf) PopulatePtp4lConf(config *string) error {
var currentSectionName string
var currentSection ptp4lConfSection
output.sections = make([]ptp4lConfSection, 0)
globalIsDefined := false
conf.sections = make([]ptp4lConfSection, 0)
hasSlaveConfigDefined := false

ifaceCount := 0
if config != nil {
for _, line := range strings.Split(*config, "\n") {
line = strings.TrimSpace(line)
if strings.HasPrefix(line, "#") {
continue
} else if strings.HasPrefix(line, "[") {
if currentSectionName != "" {
output.sections = append(output.sections, currentSection)
}
currentLine := strings.Split(line, "]")

if len(currentLine) < 2 {
return errors.New("Section missing closing ']': " + line)
}

currentSectionName = fmt.Sprintf("%s]", currentLine[0])
if currentSectionName == "[global]" {
globalIsDefined = true
if currentSectionName != GlobalSectionName && currentSectionName != NmeaSectionName && currentSectionName != UnicastSectionName {
ifaceCount++
}
currentSection = ptp4lConfSection{options: map[string]string{}, sectionName: currentSectionName}
conf.setPtp4lConfOption(currentSectionName, "", "", false)
} else if currentSectionName != "" {
split := strings.IndexByte(line, ' ')
if split > 0 {
currentSection.options[line[:split]] = line[split:]
if (line[:split] == "masterOnly" && line[split:] == "0") ||
(line[:split] == "serverOnly" && line[split:] == "0") ||
(line[:split] == "slaveOnly" && line[split:] == "1") ||
(line[:split] == "clientOnly" && line[split:] == "1") {
key := line[:split]
value := strings.TrimSpace(line[split:])
conf.setPtp4lConfOption(currentSectionName, key, value, false)
if (key == "masterOnly" && value == "0" && currentSectionName != GlobalSectionName) ||
(key == "serverOnly" && value == "0") ||
(key == "slaveOnly" && value == "1") ||
(key == "clientOnly" && value == "1") {
hasSlaveConfigDefined = true
}
}
} else {
return errors.New("Config option not in section: " + line)
}
}
if currentSectionName != "" {
output.sections = append(output.sections, currentSection)
}
}

if !globalIsDefined {
output.sections = append(output.sections, ptp4lConfSection{options: map[string]string{}, sectionName: "[global]"})
}

if !hasSlaveConfigDefined {
// No Slave Interfaces defined
output.clock_type = event.GM
} else if len(output.sections) > 2 {
conf.clock_type = event.GM
} else if ifaceCount > 1 {
// Multiple interfaces with at least one slave Interface defined
output.clock_type = event.BC
conf.clock_type = event.BC
} else {
// Single slave Interface defined
output.clock_type = event.OC
conf.clock_type = event.OC
}

return nil
}

// ExtendGlobalSection extends Ptp4lConf struct with fields not from ptp4lConf
func (conf *Ptp4lConf) ExtendGlobalSection(profileName string, messageTag string, socketPath string, pProcess string) {
conf.profile_name = profileName
conf.setPtp4lConfOption(GlobalSectionName, "message_tag", messageTag, true)
if socketPath != "" {
conf.setPtp4lConfOption(GlobalSectionName, "uds_address", socketPath, true)
}
if gnssSerialPort, ok := conf.getPtp4lConfOptionOrEmptyString(GlobalSectionName, "ts2phc.nmea_serialport"); ok {
conf.gnss_serial_port = strings.TrimSpace(gnssSerialPort)
conf.setPtp4lConfOption(GlobalSectionName, "ts2phc.nmea_serialport", GPSPIPE_SERIALPORT, true)
}
if _, ok := conf.getPtp4lConfOptionOrEmptyString(GlobalSectionName, "leapfile"); ok || pProcess == ts2phcProcessName { // not required to check process if leapfile is always included
conf.setPtp4lConfOption(GlobalSectionName, "leapfile", fmt.Sprintf("%s/%s", config.DefaultLeapConfigPath, os.Getenv("NODE_NAME")), true)
}
}

// AddInterfaceSection adds interface to Ptp4lConf
func (conf *Ptp4lConf) AddInterfaceSection(iface string) {
ifaceSectionName := fmt.Sprintf("[%s]", iface)
conf.setPtp4lConfOption(ifaceSectionName, "", "", false)
}

func getSource(isTs2phcMaster string) event.EventSource {
if ts2phcMaster, err := strconv.ParseBool(strings.TrimSpace(isTs2phcMaster)); err == nil {
if ts2phcMaster {
Expand All @@ -195,7 +272,7 @@ func getSource(isTs2phcMaster string) event.EventSource {
// (UNTIL next device section).
// 2. Port section - any other section not starting with < (e.g. [eth0]) is the port section.
// Multiple port sections are allowed. Each port participates in SyncE communication.
func (conf *ptp4lConf) extractSynceRelations() *synce.Relations {
func (conf *Ptp4lConf) extractSynceRelations() *synce.Relations {
var err error
r := &synce.Relations{
Devices: []*synce.Config{},
Expand All @@ -207,7 +284,8 @@ func (conf *ptp4lConf) extractSynceRelations() *synce.Relations {

var extendedTlv, networkOption int = synce.ExtendedTLV_DISABLED, synce.SYNCE_NETWORK_OPT_1
for _, section := range conf.sections {
if strings.HasPrefix(section.sectionName, "[<") {
sectionName := section.sectionName
if strings.HasPrefix(sectionName, "[<") {
if synceRelationInfo.Name != "" {
if len(ifaces) > 0 {
synceRelationInfo.Ifaces = ifaces
Expand All @@ -226,23 +304,23 @@ func (conf *ptp4lConf) extractSynceRelations() *synce.Relations {
}
extendedTlv, networkOption = synce.ExtendedTLV_DISABLED, synce.SYNCE_NETWORK_OPT_1

synceRelationInfo.Name = re.ReplaceAllString(section.sectionName, "")
if networkOptionStr, ok := section.options["network_option"]; ok {
synceRelationInfo.Name = re.ReplaceAllString(sectionName, "")
if networkOptionStr, ok := conf.getPtp4lConfOptionOrEmptyString(sectionName, "network_option"); ok {
if networkOption, err = strconv.Atoi(strings.TrimSpace(networkOptionStr)); err != nil {
glog.Errorf("error parsing `network_option`, setting network_option to default 1 : %s", err)
}
}
if extendedTlvStr, ok := section.options["extended_tlv"]; ok {
if extendedTlvStr, ok := conf.getPtp4lConfOptionOrEmptyString(sectionName, "extended_tlv"); ok {
if extendedTlv, err = strconv.Atoi(strings.TrimSpace(extendedTlvStr)); err != nil {
glog.Errorf("error parsing `extended_tlv`, setting extended_tlv to default 1 : %s", err)
}
}
synceRelationInfo.NetworkOption = networkOption
synceRelationInfo.ExtendedTlv = extendedTlv
} else if strings.HasPrefix(section.sectionName, "[{") {
synceRelationInfo.ExternalSource = re.ReplaceAllString(section.sectionName, "")
} else if strings.HasPrefix(section.sectionName, "[") && section.sectionName != "[global]" {
iface := re.ReplaceAllString(section.sectionName, "")
} else if strings.HasPrefix(sectionName, "[{") {
synceRelationInfo.ExternalSource = re.ReplaceAllString(sectionName, "")
} else if strings.HasPrefix(sectionName, "[") && sectionName != GlobalSectionName {
iface := re.ReplaceAllString(sectionName, "")
ifaces = append(ifaces, iface)
}
}
Expand All @@ -255,52 +333,55 @@ func (conf *ptp4lConf) extractSynceRelations() *synce.Relations {
return r
}

func (conf *ptp4lConf) renderSyncE4lConf(ptpSettings map[string]string) (configOut string, relations *synce.Relations) {
// RenderSyncE4lConf outputs synce4l config as string
func (conf *Ptp4lConf) RenderSyncE4lConf(ptpSettings map[string]string) (configOut string, relations *synce.Relations) {
configOut = fmt.Sprintf("#profile: %s\n", conf.profile_name)
relations = conf.extractSynceRelations()
relations.AddClockIds(ptpSettings)
deviceIdx := 0

for _, section := range conf.sections {
configOut = fmt.Sprintf("%s\n%s", configOut, section.sectionName)
if strings.HasPrefix(section.sectionName, "[<") {
if _, found := section.options["clock_id"]; !found {
section.options["clock_id"] = relations.Devices[deviceIdx].ClockId
if _, found := conf.getPtp4lConfOptionOrEmptyString(section.sectionName, "clock_id"); !found {
conf.setPtp4lConfOption(section.sectionName, "clock_id", relations.Devices[deviceIdx].ClockId, true)
deviceIdx++
}
}
for k, v := range section.options {
for _, option := range section.options {
k := option.key
v := option.value
configOut = fmt.Sprintf("%s\n%s %s", configOut, k, v)
}
}
return
}

func (conf *ptp4lConf) renderPtp4lConf() (configOut string, ifaces config.IFaces) {
// RenderPtp4lConf outputs ptp4l config as string
func (conf *Ptp4lConf) RenderPtp4lConf() (configOut string, ifaces config.IFaces) {
configOut = fmt.Sprintf("#profile: %s\n", conf.profile_name)
conf.mapping = nil
var nmea_source event.EventSource

for _, section := range conf.sections {
configOut = fmt.Sprintf("%s\n%s", configOut, section.sectionName)

if section.sectionName == "[nmea]" {
if source, ok := section.options["ts2phc.master"]; ok {
if section.sectionName == NmeaSectionName {
if source, ok := conf.getPtp4lConfOptionOrEmptyString(section.sectionName, "ts2phc.master"); ok {
nmea_source = getSource(source)
}
}
if section.sectionName != "[global]" && section.sectionName != "[nmea]" {
if section.sectionName != GlobalSectionName && section.sectionName != NmeaSectionName && section.sectionName != UnicastSectionName {
i := section.sectionName
i = strings.ReplaceAll(i, "[", "")
i = strings.ReplaceAll(i, "]", "")
conf.mapping = append(conf.mapping, i)
iface := config.Iface{Name: i}
if source, ok := section.options["ts2phc.master"]; ok {
if source, ok := conf.getPtp4lConfOptionOrEmptyString(section.sectionName, "ts2phc.master"); ok {
iface.Source = getSource(source)
} else {
// if not defined here, use source defined at nmea section
iface.Source = nmea_source
}
if masterOnly, ok := section.options["masterOnly"]; ok {
if masterOnly, ok := conf.getPtp4lConfOptionOrEmptyString(section.sectionName, "masterOnly"); ok {
// TODO add error handling
iface.IsMaster, _ = strconv.ParseBool(strings.TrimSpace(masterOnly))
}
Expand All @@ -310,7 +391,9 @@ func (conf *ptp4lConf) renderPtp4lConf() (configOut string, ifaces config.IFaces
PhcId: iface.PhcId,
})
}
for k, v := range section.options {
for _, option := range section.options {
k := option.key
v := option.value
configOut = fmt.Sprintf("%s\n%s %s", configOut, k, v)
}
}
Expand Down
Loading