Skip to content

Commit

Permalink
fix: allow to use string aliases for raw nv config values
Browse files Browse the repository at this point in the history 
Now, we parse out the string aliases for nv config parameters' values,
which results in a case where a user can provide a valid config
that can get applied but will never be seen as successfully applied,
resulting in an endless return loop

e.g.

Configurations:            Default       Current       Next Boot
KEEP_IB_LINK_UP_P2         False(0)      True(1)       True(1)
KEEP_LINK_UP_ON_BOOT_P2    False(0)      False(0)      False(0)
MEMIC_SIZE_LIMIT           _256KB(1)     _256KB(1)     DISABLED(0)

The fix is to keep both numeric values and string aliases and look
for a match among them.

Signed-off-by: Alexander Maslennikov <[email protected]>
  • Loading branch information
@almaslennikov
almaslennikov committed Oct 30, 2024
1 parent 7a4cdf8 commit 36b73a6
Show file tree
Hide file tree
Showing 10 changed files with 353 additions and 214 deletions.
40 changes: 21 additions & 19 deletions pkg/host/configvalidation.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package host
import (
"fmt"
"reflect"
"slices"
"strconv"
"strings"

Expand All @@ -32,14 +33,14 @@ type configValidation interface {
// ConstructNvParamMapFromTemplate translates a configuration template into a set of nvconfig parameters
// operates under the assumption that spec validation was already carried out
ConstructNvParamMapFromTemplate(
device *v1alpha1.NicDevice, defaultValues map[string]string) (map[string]string, error)
device *v1alpha1.NicDevice, nvConfigQuery types.NvConfigQuery) (map[string]string, error)
// ValidateResetToDefault checks if device's nv config has been reset to default in current and next boots
// returns bool - need to perform reset
// returns bool - reboot required
// returns error - if an error occurred during validation
ValidateResetToDefault(nvConfig types.NvConfigQuery) (bool, bool, error)
// AdvancedPCISettingsEnabled returns true if ADVANCED_PCI_SETTINGS param is enabled for current config
AdvancedPCISettingsEnabled(currentConfig map[string]string) bool
AdvancedPCISettingsEnabled(nvConfig types.NvConfigQuery) bool
// RuntimeConfigApplied checks if desired runtime config is applied
RuntimeConfigApplied(device *v1alpha1.NicDevice) (bool, error)
// CalculateDesiredRuntimeConfig returns desired values for runtime config
Expand All @@ -64,18 +65,19 @@ func nvParamLinkTypeFromName(linkType string) string {
}

func applyDefaultNvConfigValueIfExists(
paramName string, desiredParameters map[string]string, defaultValues map[string]string) {
defaultValue, found := defaultValues[paramName]
paramName string, desiredParameters map[string]string, query types.NvConfigQuery) {
defaultValues, found := query.DefaultConfig[paramName]
// Default values might not yet be available if ENABLE_PCI_OPTIMIZATIONS is disabled
if found {
desiredParameters[paramName] = defaultValue
// Take the default numeric value
desiredParameters[paramName] = defaultValues[len(defaultValues)-1]
}
}

// ConstructNvParamMapFromTemplate translates a configuration template into a set of nvconfig parameters
// operates under the assumption that spec validation was already carried out
func (v *configValidationImpl) ConstructNvParamMapFromTemplate(
device *v1alpha1.NicDevice, defaultValues map[string]string) (map[string]string, error) {
device *v1alpha1.NicDevice, query types.NvConfigQuery) (map[string]string, error) {
desiredParameters := map[string]string{}

template := device.Spec.Configuration.Template
Expand All @@ -90,7 +92,7 @@ func (v *configValidationImpl) ConstructNvParamMapFromTemplate(
}

// Link type change is not allowed on some devices
_, canChangeLinkType := defaultValues[consts.LinkTypeP1Param]
_, canChangeLinkType := query.DefaultConfig[consts.LinkTypeP1Param]
if canChangeLinkType {
linkType := nvParamLinkTypeFromName(string(template.LinkType))
desiredParameters[consts.LinkTypeP1Param] = linkType
Expand Down Expand Up @@ -131,7 +133,7 @@ func (v *configValidationImpl) ConstructNvParamMapFromTemplate(

// maxReadRequest is applied as runtime configuration
} else {
applyDefaultNvConfigValueIfExists(consts.MaxAccOutReadParam, desiredParameters, defaultValues)
applyDefaultNvConfigValueIfExists(consts.MaxAccOutReadParam, desiredParameters, query)
}

if template.RoceOptimized != nil && template.RoceOptimized.Enabled {
Expand All @@ -154,13 +156,13 @@ func (v *configValidationImpl) ConstructNvParamMapFromTemplate(

// qos settings are applied as runtime configuration
} else {
applyDefaultNvConfigValueIfExists(consts.RoceCcPrioMaskP1Param, desiredParameters, defaultValues)
applyDefaultNvConfigValueIfExists(consts.CnpDscpP1Param, desiredParameters, defaultValues)
applyDefaultNvConfigValueIfExists(consts.Cnp802pPrioP1Param, desiredParameters, defaultValues)
applyDefaultNvConfigValueIfExists(consts.RoceCcPrioMaskP1Param, desiredParameters, query)
applyDefaultNvConfigValueIfExists(consts.CnpDscpP1Param, desiredParameters, query)
applyDefaultNvConfigValueIfExists(consts.Cnp802pPrioP1Param, desiredParameters, query)
if secondPortPresent {
applyDefaultNvConfigValueIfExists(consts.RoceCcPrioMaskP2Param, desiredParameters, defaultValues)
applyDefaultNvConfigValueIfExists(consts.CnpDscpP2Param, desiredParameters, defaultValues)
applyDefaultNvConfigValueIfExists(consts.Cnp802pPrioP2Param, desiredParameters, defaultValues)
applyDefaultNvConfigValueIfExists(consts.RoceCcPrioMaskP2Param, desiredParameters, query)
applyDefaultNvConfigValueIfExists(consts.CnpDscpP2Param, desiredParameters, query)
applyDefaultNvConfigValueIfExists(consts.Cnp802pPrioP2Param, desiredParameters, query)
}
}

Expand All @@ -179,7 +181,7 @@ func (v *configValidationImpl) ConstructNvParamMapFromTemplate(
return desiredParameters, err
}
} else {
applyDefaultNvConfigValueIfExists(consts.AtsEnabledParam, desiredParameters, defaultValues)
applyDefaultNvConfigValueIfExists(consts.AtsEnabledParam, desiredParameters, query)
}

for _, rawParam := range template.RawNvConfig {
Expand Down Expand Up @@ -207,7 +209,7 @@ func (v *configValidationImpl) ValidateResetToDefault(nvConfig types.NvConfigQue
willResetInNextBoot := false

advancedPciSettingsEnabledInCurrentConfig := false
if value, found := nvConfig.CurrentConfig[consts.AdvancedPCISettingsParam]; found && value == consts.NvParamTrue {
if values, found := nvConfig.CurrentConfig[consts.AdvancedPCISettingsParam]; found && slices.Contains(values, consts.NvParamTrue) {
advancedPciSettingsEnabledInCurrentConfig = true
}
if advancedPciSettingsEnabledInCurrentConfig {
Expand All @@ -218,7 +220,7 @@ func (v *configValidationImpl) ValidateResetToDefault(nvConfig types.NvConfigQue
}

advancedPciSettingsEnabledInNextBootConfig := false
if value, found := nvConfig.NextBootConfig[consts.AdvancedPCISettingsParam]; found && value == consts.NvParamTrue {
if values, found := nvConfig.NextBootConfig[consts.AdvancedPCISettingsParam]; found && slices.Contains(values, consts.NvParamTrue) {
advancedPciSettingsEnabledInNextBootConfig = true
}
if advancedPciSettingsEnabledInNextBootConfig {
Expand All @@ -241,8 +243,8 @@ func (v *configValidationImpl) ValidateResetToDefault(nvConfig types.NvConfigQue
}

// AdvancedPCISettingsEnabled returns true if ADVANCED_PCI_SETTINGS param is enabled for current config
func (v *configValidationImpl) AdvancedPCISettingsEnabled(currentConfig map[string]string) bool {
if value, found := currentConfig[consts.AdvancedPCISettingsParam]; found && value == consts.NvParamTrue {
func (v *configValidationImpl) AdvancedPCISettingsEnabled(nvConfig types.NvConfigQuery) bool {
if values, found := nvConfig.CurrentConfig[consts.AdvancedPCISettingsParam]; found && slices.Contains(values, consts.NvParamTrue) {
return true
}
return false
Expand Down
Loading

0 comments on commit 36b73a6

Please sign in to comment.