openconfig
Remove ISIS instance leaf
- (M) release/models/isis/openconfig-isis-lsp.yang
- (M) release/models/isis/openconfig-isis-routing.yang
- (M) release/models/isis/openconfig-isis.yang
- Remove ISIS instance leaf in favor of network-instance protocol instance name
Change Scope
It appears that when /isis was relocated under /network-instances, while we
inherit the concept of multiple protocol instances by way of the protocol
list, that the ISIS domain retained the concept of multi-instance within making
the concept redundant and ambiguous.
This change removes the leaf instance within the ISIS domain in favor of the
protocol instance name to signal multi-instance support of a protocol.
Per YANG standards, this change is backwards incompatible but also highly unlikely implementations have supported such due to the ambiguity. The major version has thus been updated as a result.
Platform Implementations
N/A: This change is to resolve ambiguity in the modeling and multi-instance support is still intact per the protocol list name.
I think the original purpose of this leaf was to support multi-instance ISIS (RFC 8202). In MI-ISIS each of the potentially N different instances on the router needs a unique instance ID in the range 0-65535. I believe the purpose of this leaf should have been to provide that number; the default of "0" suggests this, although the modeling of the leaf type as a string rather than uint16 was probably wrong.
There is no doubt that the instance name should come from /network-instances/network-instance/protocols/protocol/config/name, as your PR suggests, but we still need a number and this can't be derived from the string name.
To be clear, what I am suggesting is that maybe this leaf should be replaced with a new leaf called instance-id or even multi-instance-id of type uint16. Rather than merely deleting it.
@nokia1adam agreed we have a type mismatch for the intention here. The protocol instance name was a loose design pattern for extensibility that in most circumstances would only ever see a single instance relying on out of band documentation and error handling per implementation/protocol (Some of the protocols would likely never have a concept of multi-instance within a single NI such as STATIC, DIRECTLY_CONNECTED, LOCAL_AGGREGATE so the construct is mostly meaningless).
The fact that each protocol instance needs a name is a construct that doesn't map to every implementation either so its an abstract key in those cases. If we reused the name key in this case and just loosely put a protocol behind ensure this is a uint16 value in the instance name string field in the case of IS-IS it would mimic the behavior of the instance leaf suggested for removal in this PR - not precise as you'd hope but a middle ground (these tradeoffs exist throughout the model set already)
If we keep the instance leaf, make it a uint16 then we have 2 identifiers for a protocol instance here - one at an abstract level that then carries no real meaning in most cases and one to convey the underlying protocol semantics.
This concept likely also holds true for some BGP implementations where each "instance" can be categorized by uint ASN vs. the current string protocol name
@earies sorry for the slow reply. Agree with your comment that "if we keep the instance leaf, make it a uint16 then we have 2 identifiers for a protocol instance here - one at an abstract level that then carries no real meaning in most cases and one to convey the underlying protocol semantics."
However, is this so bad? If the OC client wants to create an IS-IS multi-instance with ID 10 (10 being the value actually signaled in the TLV) should we really force that the abstract name for that protocol instance must be "10" rather than something more descriptively meaningful?
I kinda agree with @nokia1adam, making it a explicit field for the protocol specific meaning (e.g. instance-id) seems ok. This avoid binding some protocol specific semantics to an abstract filed (name) that used across all protocols,
@robshakir @dplore some comments?
@nokia1adam @earies - it looks like Juniper doesn't allow specifying a specific instance ID integer; Cisco doesn't seem to have such a field; Nokia does support specifically specifying the instance-id in SROS; Arista EOS does not appear to support it.
For the implementations that don't support this - how is the integer instance ID determined? Since it's a string that can be anything in the CLI, I don't see that parsing it from the name is a requirement.
So, to me, if we want to specify an instance ID then we need such a leaf; but unless it's supported across vendors, we probably should omit it. However, we would need guidance as to how the instance ID should be calculated in such multi-instance cases.
I think we need to reset what we're talking about here.
Instance-id refers to multi-instance ISIS. This means running ISIS on the same interface but with an identifier that allows for disjoint routing. This does not simply mean running multiple instances of isis on the same device, which is also a thing.
In IOS XR you can have multiple isis instances with different string names running simultaneously on different interfaces. You may also have multiple isis instances with different string names on the same interface if you specify an instance-id.
Using vendor CLI for illustration. Let's say I try to commit this configuration:
router isis green
net 40.0002.0000.0000.0000.0000.0000.0000.0000.0000.00
address-family ipv4 unicast
!
address-family ipv6 unicast
!
interface Bundle-Ether50
point-to-point
!
!
router isis purple
net 40.0003.0000.0000.0000.0000.0000.0000.0000.0000.00
address-family ipv4 unicast
!
address-family ipv6 unicast
!
interface Bundle-Ether50
point-to-point
!
!
I will get an error because Bundle-Ether50 is being configured in two different 'plain' isis domains:
RP/0/RP0/CPU0:r555(config-isis-if)#show configuration failed
Thu Jan 26 01:30:08.248 UTC
!! SEMANTIC ERRORS: This configuration was rejected by
!! the system due to semantic errors. The individual
!! errors with each failed configuration command can be
!! found below.
router isis purple
interface Bundle-Ether50
!!% 'clns-isis' detected the 'warning' condition 'Entity already exists'
point-to-point
!!% Configuration invalid with current submode running config: Configuration will not be accepted unless the interface-running flag exists
!
!
end
If I want to use multi-instance is-is, I can specify an instance-id.
router isis red
net 40.0000.0000.0000.0000.0000.0000.0000.0000.0000.00
instance-id 10
address-family ipv4 unicast
!
address-family ipv6 unicast
!
interface Bundle-Ether50
point-to-point
!
!
router isis blue
net 40.0001.0000.0000.0000.0000.0000.0000.0000.0000.00
instance-id 50
address-family ipv4 unicast
!
address-family ipv6 unicast
!
interface Bundle-Ether50
point-to-point
!
!
This works.
Thanks @aaronmillisor!
The instance-id statement is what I was missing here. This matches the configuration of the uint ID that Nokia SROS has.
So is this the conclusion?
Implementations that support multi-instance ISIS (RFC 8202) need an integer instance-id. The OC model can either introduce a new leaf and deprecate the existing one that is the subject of this PR, or change the type (with backwards compatibility consequences).
Implementations that do not support MI-ISIS may need the ability to name different IS-IS instances, however this is already provided at a higher level in the OC model, so neither the existing leaf that is the subject of this PR, nor the new integer leaf (if created) should ever be mandatory.
That's my proposed solution, yes. We'd remove this leaf, and add a new instance-id leaf that matches the encoding in the protocol.
For backwards compatibility, instead of removing the leaf consider setting status deprecated for this leaf and add a new instance-id leaf with reference to rfc8202 to provide clarity of the intent.
@earies -- would you like to make the proposed changes above? If not, I'm happy to open a PR for this.