Add option for SONiC interface naming schema

README.md

@@ -13,3 +13,40 @@ Ensure you have `libpcap-dev` installed.
 ```bash
 make
 ```
+
+## Interface Naming on SONiC Switches
+
+On SONiC switches, there are different naming schemas for interfaces.
+For example, the first port could be named `Ethernet0`, or it could be `Eth1/1` or something similar.
+Additionally, a port can have an alias, which may be the same as its name or it may follow a different naming schema.
+If the port is named `Ethernet0` the alias could be `Eth1/1(Port1)`.
+But it could also be the other way round.
+The defaults for the naming schemas differ across distributions.
+
+These differences wouldn't cause any problems if it weren't for LLDP.
+An LLDP message carries two fields to identify the port, `portidsubtype` and `portdescription`.
+Depending on the distribution `portidsubtype` will be either the port's name or its alias and `portdescription`, usually, will be the other of the two.
+So the metal-core registers its ports at the metal-api which stores the names as `V1SwitchNic.Name` and aliases as `V1SwitchNic.Identifier`.
+At the same time, when a machine registers at the metal-api it reports its LLDP neighbors and identifies the neighbors' ports by `portidsubtype` and `portdescription`.
+When the metal-api attempts to match the machine's neighbors with the neighboring switches' ports it compares each neighboring switch's `portidsubtype` with all of its Nics' `.Identifier` fields.
+But this only works if the port's alias is identical to its `portidsubtype` which, as stated above, is not always the case.
+
+While it is possible to configure `portidsubtype` and `portdescription` via `lldpcli`, this configuration is not persisted.
+A simple change of the MTU will restore the defaults.
+So this is not a viable solution.
+
+To accommodate all of the possible combinations, there is an `InterfaceNamingSchema` option.
+This option has nothing to do with SONiC's `interface-naming`.
+It simply tells metal-core how it should report its ports to the metal-api.
+It allows the following values.
+
+- `default`: `V1SwitchNic.Name` is the interface name; `V1SwitchNic.Identifier` is the interface alias
+- `swap`: name and alias get swapped
+- `name`: both `V1SwitchNic.Name` and `V1SwitchNic.Identifier` will be the interface name
+- `alias`: both `V1SwitchNic.Name` and `V1SwitchNic.Identifier` will be the interface alias
+
+To determine which of these suits your setup compare the port aliases with the LLDP configuration for `portidsubtype`.
+If they match the correct value for `InterfaceNamingSchema` is `default`.
+If not, check if `portdescription` matches the alias and `portidsubtype` matches the name.
+In that case you can use `swap`.
+If you need to have both of the fields to have the same value, use either `name` or `alias`.

cmd/config.go

@@ -36,4 +36,5 @@ type Config struct {
 	GrpcClientKeyFile         string        `required:"false" desc:"the gRPC client key file" envconfig:"grpc_client_key_file"`
 	PXEVlanID                 uint16        `required:"false" default:"4000" desc:"the id of the pxe vlan" envconfig:"pxe_vlan_id"`
 	BGPNeighborStateFile      string        `required:"false" default:"/var/run/bgp-neighbors/bgp-neighbors.json" desc:"the file to read the BGP neighbor state from" envconfig:"bgp_neighbor_state_file"`
+	InterfaceNamingSchema     string        `required:"false" default:"default" desc:"schema for interface and alias naming of switch nics" envconfig:"interface_naming_schema"`
 }

cmd/internal/switcher/nos.go

@@ -9,6 +9,7 @@ import (
 
 	"github.com/metal-stack/metal-core/cmd/internal/switcher/cumulus"
 	"github.com/metal-stack/metal-core/cmd/internal/switcher/sonic"
+
 	"github.com/metal-stack/metal-core/cmd/internal/switcher/types"
 	"github.com/metal-stack/metal-go/api/models"
 )
@@ -23,10 +24,10 @@ type NOS interface {
 	GetManagement() (ip, user string, err error)
 }
 
-func NewNOS(log *slog.Logger, frrTplFile, interfacesTplFile string) (NOS, error) {
+func NewNOS(log *slog.Logger, frrTplFile, interfacesTplFile string, interfaceNamingSchema sonic.InterfaceNamingSchema) (NOS, error) {
 	if _, err := os.Stat(sonic.SonicVersionFile); err == nil {
 		log.Info("create sonic NOS")
-		nos, err := sonic.New(log.With("os", "sonic"), frrTplFile)
+		nos, err := sonic.New(log.With("os", "sonic"), frrTplFile, interfaceNamingSchema)
 		if err != nil {
 			return nil, fmt.Errorf("failed to initialize SONiC NOS %w", err)
 		}

cmd/internal/switcher/sonic/sonic.go

@@ -22,23 +22,37 @@ import (
 
 type (
 	Sonic struct {
-		db           *db.DB
-		frrApplier   *templates.Applier
-		log          *slog.Logger
-		redisApplier *redis.Applier
+		db                    *db.DB
+		frrApplier            *templates.Applier
+		log                   *slog.Logger
+		redisApplier          *redis.Applier
+		interfaceNamingSchema InterfaceNamingSchema
 	}
 
 	PortInfo struct {
 		Alias string `json:"alias"`
 	}
+
+	sonic_version struct {
+		BuildVersion string `yaml:"build_version"`
+	}
+
+	InterfaceNamingSchema string
+)
+
+const (
+	InterfaceNamingSchemaDefault = InterfaceNamingSchema("default")
+	InterfaceNamingSchemaSwap    = InterfaceNamingSchema("swap")
+	InterfaceNamingSchemaName    = InterfaceNamingSchema("name")
+	InterfaceNamingSchemaAlias   = InterfaceNamingSchema("alias")
 )
 
 const (
 	SonicVersionFile = "/etc/sonic/sonic_version.yml"
 	redisConfigFile  = "/var/run/redis/sonic-db/database_config.json"
 )
 
-func New(log *slog.Logger, frrTplFile string) (*Sonic, error) {
+func New(log *slog.Logger, frrTplFile string, interfaceNamingSchema InterfaceNamingSchema) (*Sonic, error) {
 	cfg, err := loadRedisConfig(redisConfigFile)
 	if err != nil {
 		return nil, fmt.Errorf("failed to load database config for SONiC: %w", err)
@@ -49,10 +63,11 @@ func New(log *slog.Logger, frrTplFile string) (*Sonic, error) {
 	}
 
 	return &Sonic{
-		db:           sonicDb,
-		frrApplier:   NewFrrApplier(log, frrTplFile),
-		log:          log,
-		redisApplier: redis.NewApplier(log, sonicDb),
+		db:                    sonicDb,
+		frrApplier:            NewFrrApplier(log, frrTplFile),
+		log:                   log,
+		redisApplier:          redis.NewApplier(log, sonicDb),
+		interfaceNamingSchema: interfaceNamingSchema,
 	}, nil
 }
 
@@ -94,10 +109,7 @@ func (s *Sonic) GetNics(ctx context.Context, log *slog.Logger, blacklist []strin
 			continue
 		}
 
-		nic := &models.V1SwitchNic{
-			Identifier: &portConfig.Alias,
-			Name:       &name,
-		}
+		nic := getSwitchNicByNamingSchema(name, portConfig.Alias, s.interfaceNamingSchema)
 		nics = append(nics, nic)
 	}
 
@@ -138,6 +150,10 @@ func (s *Sonic) getPortsConfig(ctx context.Context) (map[string]PortInfo, error)
 		return nil, err
 	}
 
+	// keep the real interface names as keys; the naming schema is only applied
+	// when reporting nics to the metal-api. The keys are used to match the
+	// interface blacklist and to open the LLDP pcap handles, both of which
+	// need the actual netdev names.
 	portConfig := map[string]PortInfo{}
 	for _, port := range ports {
 		portConfig[port.Name] = PortInfo{
@@ -148,10 +164,6 @@ func (s *Sonic) getPortsConfig(ctx context.Context) (map[string]PortInfo, error)
 	return portConfig, err
 }
 
-type sonic_version struct {
-	BuildVersion string `yaml:"build_version"`
-}
-
 func (s *Sonic) GetOS() (*models.V1SwitchOS, error) {
 	versionBytes, err := os.ReadFile(SonicVersionFile)
 	if err != nil {
@@ -168,10 +180,33 @@ func (s *Sonic) GetOS() (*models.V1SwitchOS, error) {
 		Version: sonicVersion.BuildVersion,
 	}, nil
 }
+
 func (s *Sonic) GetManagement() (ip, user string, err error) {
 	ip, err = internal.GetManagementIP("eth0")
 	if err != nil {
 		return "", "", err
 	}
 	return ip, "admin", nil
 }
+
+func getSwitchNicByNamingSchema(name, alias string, naming InterfaceNamingSchema) *models.V1SwitchNic {
+	var nic = &models.V1SwitchNic{}
+	switch naming {
+	case InterfaceNamingSchemaDefault:
+		nic.Name = new(name)
+		nic.Identifier = new(alias)
+	case InterfaceNamingSchemaSwap:
+		nic.Name = new(alias)
+		nic.Identifier = new(name)
+	case InterfaceNamingSchemaAlias:
+		nic.Name = new(alias)
+		nic.Identifier = new(alias)
+	case InterfaceNamingSchemaName:
+		nic.Name = new(name)
+		nic.Identifier = new(name)
+	default:
+		nic.Name = new(name)
+		nic.Identifier = new(alias)
+	}
+	return nic
+}

cmd/internal/switcher/sonic/sonic_test.go

@@ -5,6 +5,7 @@ import (
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
+	"github.com/metal-stack/metal-go/api/models"
 )
 
 func Test_portsToInterfaces(t *testing.T) {
@@ -63,3 +64,72 @@ func Test_portsToInterfaces(t *testing.T) {
 		})
 	}
 }
+
+func Test_getSwitchNicByNamingSchema(t *testing.T) {
+	tests := []struct {
+		name   string
+		ifname string
+		alias  string
+		naming InterfaceNamingSchema
+		want   *models.V1SwitchNic
+	}{
+		{
+			name:   "naming schema empty",
+			ifname: "Ethernet0",
+			alias:  "Eth1/1",
+			naming: "",
+			want: &models.V1SwitchNic{
+				Name:       new("Ethernet0"),
+				Identifier: new("Eth1/1"),
+			},
+		},
+		{
+			name:   "default",
+			ifname: "Ethernet0",
+			alias:  "Eth1/1",
+			naming: InterfaceNamingSchemaDefault,
+			want: &models.V1SwitchNic{
+				Name:       new("Ethernet0"),
+				Identifier: new("Eth1/1"),
+			},
+		},
+		{
+			name:   "swap",
+			ifname: "Ethernet0",
+			alias:  "Eth1/1",
+			naming: InterfaceNamingSchemaSwap,
+			want: &models.V1SwitchNic{
+				Name:       new("Eth1/1"),
+				Identifier: new("Ethernet0"),
+			},
+		},
+		{
+			name:   "name",
+			ifname: "Ethernet0",
+			alias:  "Eth1/1",
+			naming: InterfaceNamingSchemaName,
+			want: &models.V1SwitchNic{
+				Name:       new("Ethernet0"),
+				Identifier: new("Ethernet0"),
+			},
+		},
+		{
+			name:   "alias",
+			ifname: "Ethernet0",
+			alias:  "Eth1/1",
+			naming: InterfaceNamingSchemaAlias,
+			want: &models.V1SwitchNic{
+				Name:       new("Eth1/1"),
+				Identifier: new("Eth1/1"),
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := getSwitchNicByNamingSchema(tt.ifname, tt.alias, tt.naming)
+			if diff := cmp.Diff(tt.want, got); diff != "" {
+				t.Errorf("getNicByNamingSchema() diff = %s", diff)
+			}
+		})
+	}
+}

cmd/server.go

@@ -23,6 +23,7 @@ import (
 	"github.com/metal-stack/metal-core/cmd/internal/core"
 	"github.com/metal-stack/metal-core/cmd/internal/metrics"
 	"github.com/metal-stack/metal-core/cmd/internal/switcher"
+	"github.com/metal-stack/metal-core/cmd/internal/switcher/sonic"
 	metalgo "github.com/metal-stack/metal-go"
 	"github.com/metal-stack/v"
 )
@@ -89,7 +90,7 @@ func Run() {
 		os.Exit(1)
 	}
 
-	nos, err := switcher.NewNOS(log, cfg.FrrTplFile, cfg.InterfacesTplFile)
+	nos, err := switcher.NewNOS(log, cfg.FrrTplFile, cfg.InterfacesTplFile, sonic.InterfaceNamingSchema(cfg.InterfaceNamingSchema))
 	if err != nil {
 		log.Error("failed to create NOS instance", "error", err)
 		os.Exit(1)