Skip to content
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.

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

Implementing Line Card Port function #33

Open
nleiva opened this issue Jun 27, 2024 · 6 comments
Open

Implementing Line Card Port function #33

nleiva opened this issue Jun 27, 2024 · 6 comments
Assignees

Comments

@nleiva
Copy link

nleiva commented Jun 27, 2024

Hi,

How do you envision implementing other Line Cards for a given vendor? Do you expand the Vendor Port function matching on HardwareModel?

Let's say I want to add support for other Juniper interfaces:

  • et—Ethernet interfaces (10-, 25-, 40-, 50-, 100-, 200-, and 400-Gigabit Ethernet interface).
  • ge—Gigabit Ethernet interface
  • xe—10-Gigabit Ethernet interface. Some older 10-Gigabit Ethernet interfaces use the ge media type (rather than xe) to identify the physical part of the network device (XENPAK 10-Gigabit Ethernet interface PIC, which is supported only on M series routers).

Then, I'd have to check HardwareModel before nameBuilder.WriteString("et-") and also change nameBuilder.WriteString("/0") to nameBuilder.WriteString(fmt.Sprintf("%d", pp.PICIndex)). Does that sound correct to you?

unc (n *Namer) Port(pp *namer.PortParams) (string, error) {
	if !pp.Channelizable {
		return "", fmt.Errorf("Juniper does not support unchannelizable ports")
	}

	var nameBuilder strings.Builder
	nameBuilder.WriteString("et-")
	if pp.SlotIndex == nil {
		nameBuilder.WriteString("0/")
		nameBuilder.WriteString(fmt.Sprintf("%d", pp.PICIndex))
	} else {
		nameBuilder.WriteString(fmt.Sprintf("%d", *pp.SlotIndex))
		nameBuilder.WriteString("/0")
	}
	nameBuilder.WriteString(fmt.Sprintf("/%d", pp.PortIndex))
	if pp.ChannelIndex != nil {
		nameBuilder.WriteString(fmt.Sprintf(":%d", *pp.ChannelIndex))
	}
	return nameBuilder.String(), nil
}

@nleiva
Copy link
Author

nleiva commented Jul 9, 2024

Before I submit a PR for Juniper devices, how would something like this look to you:

  1. Add a file to store a LineCard to interface name mapping.
const (
	GE = "ge"
	ET = "et"
	TE = "te"
	XE = "xe"
)

var speedStrings = map[string]string{
	// Gigabit Ethernet MIC with SFP.
	"MIC-3D-20GE-SFP":"GE",
	// Gigabit Ethernet MIC with SFP (E).
	"MIC-3D-20GE-SFP-E":"GE",
	// 10-Gigabit Ethernet MICs with XFP.
	"MIC-3D-2XGE-XFP":"XE",
	"MIC-3D-4XGE-XFP":"XE",
	// 10-Gigabit Ethernet MIC with SFP+.
	"MIC3-3D-10XGE-SFPP":"XE",
	// 40-Gigabit Ethernet MIC with QSFP+.
	"MIC3-3D-2X40GE-QSFPP":"ET",
	// 16-Port Gigabit Ethernet XPIM (with PoE)
	"SRX-GP-16GE-POE":"GE",
	// 100-Gigabit Ethernet MIC with CFP
	"MIC3-3D-1X100GE-CFP": "ET",
}
  1. Modify Juniper's Port function to check if we have a match for HardwareModel, otherwise use a default value (as is today). I also removed the current hardcoded PIC value (0).
// Port is an implementation of namer.Port.
func (n *Namer) Port(pp *namer.PortParams) (string, error) {
	if !pp.Channelizable {
		return "", fmt.Errorf("Juniper does not support unchannelizable ports")
	}
	speed := "et"
	ifsymb, ok := speedStrings[n.HardwareModel]
	if ok {
		speed = ifsymb
	}

	var nameBuilder strings.Builder
	nameBuilder.WriteString(speed + "-")
	if pp.SlotIndex == nil {
		nameBuilder.WriteString("0/")
	} else {
		nameBuilder.WriteString(fmt.Sprintf("%d/", *pp.SlotIndex))
	}
	nameBuilder.WriteString(fmt.Sprintf("%d", pp.PICIndex))
	nameBuilder.WriteString(fmt.Sprintf("/%d", pp.PortIndex))
	if pp.ChannelIndex != nil {
		nameBuilder.WriteString(fmt.Sprintf(":%d", *pp.ChannelIndex))
	}
	return nameBuilder.String(), nil
}

Thanks

@lgomez9
Copy link

lgomez9 commented Jul 22, 2024

Hi Nicolas,

That looks like a good solution to me.

As a small nit, that I realize is not coming from you - we can also put together the namebuilder lines after the if statement:

nameBuilder.WriteString(fmt.Sprintf("%d/%d", pp.PICIndex, pp.PortIndex)

@dplore
Copy link
Member

dplore commented Jul 23, 2024

This is a bit of a tangent,but I think we should define what hardware_model in the entity-naming maps to in OpenConfig data models. My thought is it should map to /components/component/state/model-name

Further, the PORT component probably does not have a useful model-name. Rather one could use the component tree to resolve the parent of the port, recursing up to the first component of type LINECARD or CHASSIS to determine the model-name. I think we need some specification like this to make using the entity-naming library deterministic across multiple vendors. This could start out just as markdown documentation. It's not clear if the code for this should be in entity-naming or elsewhere, but I do think we should at least document this recommendation.

@nleiva
Copy link
Author

nleiva commented Jul 24, 2024

Hi Nicolas,

That looks like a good solution to me.

As a small nit, that I realize is not coming from you - we can also put together the namebuilder lines after the if statement:

nameBuilder.WriteString(fmt.Sprintf("%d/%d", pp.PICIndex, pp.PortIndex)

Thanks!

Yeah, I didn't modify the name-building pattern to keep the example as close as the current implementation.

@nleiva
Copy link
Author

nleiva commented Jul 24, 2024

This is a bit of a tangent,but I think we should define what hardware_model in the entity-naming maps to in OpenConfig data models. My thought is it should map to /components/component/state/model-name

Further, the PORT component probably does not have a useful model-name. Rather one could use the component tree to resolve the parent of the port, recursing up to the first component of type LINECARD or CHASSIS to determine the model-name. I think we need some specification like this to make using the entity-naming library deterministic across multiple vendors. This could start out just as markdown documentation. It's not clear if the code for this should be in entity-naming or elsewhere, but I do think we should at least document this recommendation.

This is a great point. I see the model also has an install-position they want to use instead of slot-id, which we could return based on this project's SlotIndex, PICIndex, and PortIndex.

// / openconfig-platform/components/component 
type Component struct {
	Chassis	*Component_Chassis	`path:"chassis" module:"openconfig-platform"`
	InstallPosition	*string	`path:"state/install-position" module:"openconfig-platform/openconfig-platform"`
	Linecard	*Component_Linecard	`path:"linecard" module:"openconfig-platform-linecard"`
	ModelName	*string	`path:"state/model-name" module:"openconfig-platform/openconfig-platform"`
	Port	*Component_Port	`path:"port" module:"openconfig-platform"`
}

type Component_Chassis struct {
	Utilization	*Component_Chassis_Utilization	`path:"utilization" module:"openconfig-platform"`
}

type Component_Linecard struct {
	PowerAdminState	E_PlatformTypes_ComponentPowerType	`path:"config/power-admin-state" module:"openconfig-platform-linecard/openconfig-platform-linecard"`
	SlotId	*string	`path:"state/slot-id" module:"openconfig-platform-linecard/openconfig-platform-linecard"`
	Utilization	*Component_Linecard_Utilization	`path:"utilization" module:"openconfig-platform-linecard"`
}

type Component_Port struct {
}

@dplore
Copy link
Member

dplore commented Jul 24, 2024

This is a bit of a tangent,but I think we should define what hardware_model in the entity-naming maps to in OpenConfig data models. My thought is it should map to /components/component/state/model-name
Further, the PORT component probably does not have a useful model-name. Rather one could use the component tree to resolve the parent of the port, recursing up to the first component of type LINECARD or CHASSIS to determine the model-name. I think we need some specification like this to make using the entity-naming library deterministic across multiple vendors. This could start out just as markdown documentation. It's not clear if the code for this should be in entity-naming or elsewhere, but I do think we should at least document this recommendation.

This is a great point. I see the model also has an install-position they want to use instead of slot-id we might want to return based on this project's SlotIndex, PICIndex, and PortIndex.

Yes, install-position is a very recent addition which deprecates slot-id and generalizes the use case for resolving the physical location where a component is installed in the hardware / component tree. I expect to see support for install-position appear in implementations over the next 6-18 months. Since slot-id still exists (deprecated paths are expected to be supported until they are actually removed from the model) we should be flexible and use it when install-position is not available.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants