Nvidia hardware profile by mrbojangles3 · Pull Request #1382 · githedgehog/fabric

mrbojangles3 · 2026-03-26T17:40:18Z

In conversation the goal of the validation is to make sure that we don't commit typos in the profile.

Signed-off-by: Logan Blyth <logan@githedgehog.com>

Copilot

Pull request overview

Adds NVIDIA (Spectrum-based) Cumulus switch profiles and related schema/validation adjustments so profiles can be validated and port breakout naming can be translated correctly (helping prevent typos in profile definitions).

Changes:

Added NVIDIA Spectrum-2/3 silicon identifiers and two new NVIDIA switch profiles (SN3700C, SN4600) to the default Cumulus catalog.
Updated SwitchProfile validation and API↔NOS port mapping to better handle Cumulus (MLX/VX) breakout naming.
Extended CRDs/docs (and tests) with a new nosFamily field and added a Cumulus-focused port-mapping test case.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
pkg/ctrl/switchprofile/silicons.go	Adds Spectrum 2/3 silicon constants for NVIDIA profiles.
pkg/ctrl/switchprofile/p_nv_msn4600.go	Introduces NVIDIA SN4600 SwitchProfile definition (ports, breakouts, pipelines).
pkg/ctrl/switchprofile/p_nv_msn3700c.go	Introduces NVIDIA SN3700C SwitchProfile definition (ports, breakouts).
pkg/ctrl/switchprofile/catalog.go	Registers the new NVIDIA profiles in the default Cumulus profile list.
pkg/ctrl/switchprofile/README.md	Adds guidance for gathering platform info and validating breakouts on Cumulus.
docs/api.md	Documents the new `nosFamily` field on port profiles.
config/crd/bases/wiring.githedgehog.com_switchprofiles.yaml	Adds `nosFamily` to SwitchProfile CRD schema.
config/crd/bases/agent.githedgehog.com_agents.yaml	Adds `nosFamily` to Agent CRD schema (embedded SwitchProfile schema).
api/wiring/v1beta1/switchprofile_types_test.go	Adds/updates tests for Cumulus port breakout translation behavior.
api/wiring/v1beta1/switchprofile_types.go	Adds `nosFamily` to API type, adjusts validation rules, and updates breakout mapping for Cumulus.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-26T17:46:08Z

pkg/ctrl/switchprofile/p_nv_msn4600.go

+		Config:   wiringapi.SwitchProfileConfig{},
+		MaxPorts: 128,
+		Ports: map[string]wiringapi.SwitchProfilePort{
+			"M1":    {NOSName: "eth0", Management: true, OniePortName: "eth0"},


SwitchProfile.Validate currently requires management ports' NOSName to start with "Management" (e.g., "Management0"). This profile sets the management port NOSName to "eth0", so it will fail validation and won’t register in default profiles. Align the management NOSName with the validation rules, or update validation to explicitly allow Cumulus management interface naming.

Suggested change

"M1": {NOSName: "eth0", Management: true, OniePortName: "eth0"},

"M1": {NOSName: "Management0", Management: true, OniePortName: "eth0"},

Copilot · 2026-03-26T17:46:12Z

pkg/ctrl/switchprofile/p_nv_msn3700c.go

+		Config:   wiringapi.SwitchProfileConfig{},
+		MaxPorts: 128,
+		Ports: map[string]wiringapi.SwitchProfilePort{
+			"M1":    {NOSName: "eth0", Management: true, OniePortName: "eth0"},


SwitchProfile.Validate currently requires management ports' NOSName to start with "Management" (e.g., "Management0"). This profile sets the management port NOSName to "eth0", so it will fail validation and won’t register in default profiles. Align the management NOSName with the validation rules, or update validation to explicitly allow Cumulus management interface naming.

Suggested change

"M1": {NOSName: "eth0", Management: true, OniePortName: "eth0"},

"M1": {NOSName: "Management0", Management: true, OniePortName: "eth0"},

Copilot · 2026-03-26T17:46:14Z

api/wiring/v1beta1/switchprofile_types.go

+					nosNameBaseStr, cut := strings.CutPrefix(port.BaseNOSName, DataPortNOSNamePrefix)
+					if !cut {
+						return nil, errors.Errorf("port %q base NOS name %q is invalid (no expected prefix)", portName, port.NOSName)
+					}
+					nosNameBase, err := strconv.Atoi(nosNameBaseStr)
 					if err != nil {
-						return nil, errors.Errorf("port %q NOS name %q breakout mode %q offset %q is invalid (not a number)", portName, port.NOSName, swBreakout, offsetStr)
+						return nil, errors.Errorf("port %q base NOS name %q is invalid (suffix isn't a number)", portName, port.NOSName)
 					}


These errors are reporting the wrong value: they say “base NOS name %q” but format port.NOSName. This makes debugging invalid BaseNOSName values hard. Use port.BaseNOSName in both messages so the user sees the actual base name that failed parsing.

Copilot · 2026-03-26T17:46:17Z

api/wiring/v1beta1/switchprofile_types.go

+	// NOS Family allows the code to differentiate from sonic or cumulus or any future NOS
+	NOSFamily string `json:"nosFamily,omitempty"`


nosFamily is added to the SwitchProfilePortProfile API/CRD schema, but it isn’t referenced anywhere in the codebase (validation, planning, or translation). Exposing an unused API field can confuse users and creates long-term compatibility baggage; either wire it into the logic that needs it or remove it until it has a concrete use case.

Suggested change

// NOS Family allows the code to differentiate from sonic or cumulus or any future NOS

NOSFamily string `json:"nosFamily,omitempty"`

Copilot · 2026-03-26T17:46:20Z

pkg/ctrl/switchprofile/README.md

+
+0. Start with adjacent ports for example swp5 and swp6. Attempt the largest breakouts first. If there is a limitation on adjacent ports that will cut down on the options that you need to test.
+
+  **note** In the past breakout limitations have been on physically adjacent ports, if there are 4 ports in a column on the fron panel of a switch include all of those ports in the breakout testing from the previous step


Typo in documentation: “fron panel” should be “front panel”.

Suggested change

**note** In the past breakout limitations have been on physically adjacent ports, if there are 4 ports in a column on the fron panel of a switch include all of those ports in the breakout testing from the previous step

**note** In the past breakout limitations have been on physically adjacent ports, if there are 4 ports in a column on the front panel of a switch include all of those ports in the breakout testing from the previous step

Frostman · 2026-03-26T17:43:04Z

api/wiring/v1beta1/switchprofile_types.go

 	// AutoNegDefault defines the default auto-negotiation state for the port
 	AutoNegDefault bool `json:"autoNegDefault,omitempty"`
+	// NOS Family allows the code to differentiate from sonic or cumulus or any future NOS
+	NOSFamily string `json:"nosFamily,omitempty"`


I don't see it used it anywhere and it feels like we can live w/o it "easily" by looking at the NOSType

I forgot to remove it

Frostman · 2026-03-26T17:44:12Z

api/wiring/v1beta1/switchprofile_types_test.go

+			},
+			want: map[string]string{
+				"E1/1":   "swp1",
+				"E1/1/1": "swp1",


Sounds like it should be swp1s1 (or s0?)

mrbojangles3 added 3 commits March 26, 2026 12:10

feat: add support for sn4600

b5fcf91

Signed-off-by: Logan Blyth <logan@githedgehog.com>

feat: add support for sn3700c

af60c6c

Signed-off-by: Logan Blyth <logan@githedgehog.com>

Add parsing for cumulus port profiles

effa29a

Signed-off-by: Logan Blyth <logan@githedgehog.com>

Frostman requested review from Frostman and Copilot March 26, 2026 17:40

Copilot started reviewing on behalf of Frostman March 26, 2026 17:41 View session

Copilot AI reviewed Mar 26, 2026

View reviewed changes

Frostman reviewed Mar 26, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Nvidia hardware profile#1382

Nvidia hardware profile#1382
mrbojangles3 wants to merge 3 commits intomasterfrom
nividia_hardware_profile

mrbojangles3 commented Mar 26, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 26, 2026

Uh oh!

Copilot AI Mar 26, 2026

Uh oh!

Copilot AI Mar 26, 2026

Uh oh!

Copilot AI Mar 26, 2026

Uh oh!

Copilot AI Mar 26, 2026

Uh oh!

Frostman Mar 26, 2026

Uh oh!

mrbojangles3 Mar 26, 2026

Uh oh!

Frostman Mar 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	"M1": {NOSName: "eth0", Management: true, OniePortName: "eth0"},
	"M1": {NOSName: "Management0", Management: true, OniePortName: "eth0"},

		// NOS Family allows the code to differentiate from sonic or cumulus or any future NOS
		NOSFamily string `json:"nosFamily,omitempty"`


		0. Start with adjacent ports for example swp5 and swp6. Attempt the largest breakouts first. If there is a limitation on adjacent ports that will cut down on the options that you need to test.

		note In the past breakout limitations have been on physically adjacent ports, if there are 4 ports in a column on the fron panel of a switch include all of those ports in the breakout testing from the previous step

Conversation

mrbojangles3 commented Mar 26, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

Frostman Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

mrbojangles3 Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

Frostman Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants