Hi Renato, We've decided to go ahead and incorporate your comments despite the fact that the documents are approved. The changes are minor.
> On May 3, 2025, at 12:23 PM, Renato Westphal <[email protected]> wrote: > > Hi Yingzhen, > > Apologies for my late reply. Please find my comments inline below. > > Em ter., 22 de abr. de 2025 às 19:25, Yingzhen Qu > <[email protected]> escreveu: >> >>> While updating the code, I noticed a few minor inconsistencies that I >>> thought were worth sharing: >>> * In the OSPF module, SIDs are always represented using a >>> "label-value"/"index-value" choice. However, in some cases there's an >>> additional "length" leaf, and in others, there isn't. >>> * In the IS-IS module, SIDs are represented either as a simple "sid" >>> leaf or as a "label-value"/"index-value" choice combined with a >>> "length" field. >> >> [Yingzhen]: Thanks for catching this. I looked at RFC8667 and fixed the >> is-is model to be consistent with OSPF. > > Thanks for the update! I see that the OSPF and IS-IS modules are now > consistent. > > For both modules, I'm only unsure about the need of the "length" leaf > under the "sid-sub-tlv" container of the "sid-sub-tlv" grouping: > > leaf length { > type uint8; > description > "Length of the SID value. YANG model specification > is necessary since it dictates the semantics of the > SID."; > } > > Since we already have a choice between label-value and index-value, > this field might be redundant. If we decide to keep it, it might make > sense to also include it for Prefix-SIDs and Adj-SIDs, so that all SID > types are represented consistently. We agree and have removed the length as well as the extra level of grouping/container for sid-sub-tlv. > >>> * In the IS-IS module, the "adjacency" list is augmented with a list >>> of local Adjacency-SIDs ("adjacency-sid"). In the OSPF module, the >>> "neighbor" list doesn't have a similar augmentation. Adjacency-SID >>> information is only available in the LSDB. >> >> >> [Yingzhen]: >> do you mean the following in IS-IS? >> >> augment >> /rt:routing/rt:control-plane-protocols/rt:control-plane-protocol/isis:isis/isis:interfaces/isis:interface/isis:adjacencies/isis:adjacency: >> >> +--ro adjacency-sid* [] >> >> +--ro value? uint32 >> >> +--ro address-family? iana-rt-types:address-family >> >> +--ro weight? uint8 >> >> +--ro protection-requested? boolean >> >> >> This is an augmentation of operational state. You're right, we don't have >> this in OSPF. I looked at RFC9129, this can be added to >> ospf:interface/ospf:neighbors/ospf:neighbor. Unfortunately we have different >> names in RFC9129 and 9130, neighbors vs. adjacencies. > > Yes, that's what I mean. I think adding Adj-SID information to > ospf:interface/ospf:neighbors/ospf:neighbor would be useful. This is has been added. augment /rt:routing/rt:control-plane-protocols /rt:control-plane-protocol/ospf:ospf/ospf:areas /ospf:area/ospf:interfaces/ospf:interface /ospf:neighbors/ospf:neighbor: +--ro adjacency-sid* [] +--ro value? uint32 +--ro weight? uint8 +--ro protection-requested? boolean Thanks, Acee > > Best Regards, > -- > Renato Westphal _______________________________________________ Lsr mailing list -- [email protected] To unsubscribe send an email to [email protected]
