An independant review draft-ississ-sr-yang so top posting. Not Ready is my take for two major issues (was three but I m confused).
The I-D defines a raft of identity for the SR flags; it uses different names to those used in the base documents, it gives no explanation why it has done this and it gives no references. Even without the name changes, I think the references should be to RFC and section for each flag. Many YANG modules do this and this one should do too. The I-D imports from sr-mpls and uses this in XPath to control augmentations. The title of the I-D is not 'MPLS' but the abstract is. My first take was that this has nothing to do with MPLS. I do see a lot of noise about IPv6 dataplane and was expecting both to be included but reading the module it does seem to be for an MPLS dataplane only [I note that SRv6-yang is an expired I-D) I first thought that the augmentations were for all SR not just MPLS even if the import is from SR-MPLS. Then the SPRING WG did put what seems to be a common grouping in an MPLS module. Perhaps it would be better for this module not to use this import; I notice too that while most routing protocol augments define a presence container. as RECOMMENDED in RFC9349, this does not, again a reason to DIY. So I got confused and would change at least the title in case there is another I-D for IPv6 and also say something about other dataplanes e.g for future study. Thirdly, this I-D uses the term 'perfix' which I have not come across before as in ================================= container perfix-sid-flags { leaf-list bits { type identityref { base prefix-sid-bit; ================================= Perhaps one for Terminology Some minor issues. Common practice is to make a YANG module a section on its own for ease of reference. Here it is preceded by other information which I think should be a separate section. Author: Yingzhen Qu <mailto:yingzhen.i...@gmail.com.com> is not the mail address I see on the list "IS-IS will not advertise nor receive any mapping server..." 'not receive' implies a filter for such traffic. Perhaps not act upon. /allows to advertise/advertises/ /allows to enable/controls/ " /* Notifications */ " probably redundant or else something missing On to OSPF (so much simpler) Tom Petch ________________________________________ From: Lsr <lsr-boun...@ietf.org> on behalf of Reshad Rahman <reshad=40yahoo....@dmarc.ietf.org> Sent: 14 November 2023 17:17 Hi Acee, Couple of other differences (I didn't dig to see whether they are justified): - Naming discrepancies e.g. TLV suffix is used more in OSPF (local-blocks v/s local-blocks-tlv) - No global blocks in ISIS - No capabilities in OSPF Regards, Reshad. On Tuesday, November 14, 2023, 10:11:02 AM EST, Acee Lindem <acee.i...@gmail.com> wrote: Thanks Reshad - are there any other notable discrepancies? Thanks, Acee > On Nov 14, 2023, at 9:55 AM, Reshad Rahman > <reshad=40yahoo....@dmarc.ietf.org> wrote: > > My suggestion is that authors of these 2 documents spend some time together > to try to align the 2 documents. After that we can do YD review. > > Regards, > Reshad. > > On Wednesday, November 1, 2023, 10:58:56 AM EDT, Reshad Rahman > <res...@yahoo.com<mailto:res...@yahoo.com>> wrote: > > > Hi, > > Background: those 2 documents have just been assigned YD review, I am > reviewing OSPF and Jan is reviewing ISIS. > > Was an effort made to keep those 2 documents aligned/in-sync where > possible/desirable? My expectation is that the SR specifics would be > near-identical in the 2 documents. e.g. shouldn't the capabilities for the 2 > protocols be very similar. > Here are some differences which don't seem justified: > - sr-algorithm in ISIS is a uint8 and in OSPF is an identityref > - range-size is a uint32 in ISIS and is a uint24 in OSPF > > > augment /rt:routing/rt:control-plane-protocols > /rt:control-plane-protocol/isis:isis/isis:database > /isis:levels/isis:lsp/isis:router-capabilities: > +--ro sr-capability > | +--ro sr-capability > | | +--ro sr-capability-bits* identityref > | +--ro global-blocks > | +--ro global-block* [] > | +--ro range-size? uint32 > | +--ro sid-sub-tlv > | +--ro sid? uint32 > +--ro sr-algorithms > | +--ro sr-algorithm* uint8 > +--ro local-blocks > | +--ro local-block* [] > | +--ro range-size? uint32 > | +--ro sid-sub-tlv > | +--ro sid? uint32 > +--ro srms-preference > +--ro preference? uint8 > > augment /rt:routing/rt:control-plane-protocols > /rt:control-plane-protocol/ospf:ospf/ospf:areas/ospf:area > /ospf:interfaces/ospf:interface/ospf:database > /ospf:link-scope-lsa-type/ospf:link-scope-lsas > /ospf:link-scope-lsa/ospf:version/ospf:ospfv2/ospf:ospfv2 > /ospf:body/ospf:opaque/ospf:ri-opaque: > +--ro sr-algorithm-tlv > | +--ro sr-algorithm* identityref > +--ro sid-range-tlvs > | +--ro sid-range-tlv* [] > | +--ro range-size? rt-types:uint24 > | +--ro sid-sub-tlv > | +--ro sid? uint32 > +--ro local-block-tlvs > | +--ro local-block-tlv* [] > | +--ro range-size? rt-types:uint24 > | +--ro sid-sub-tlv > | +--ro sid? uint32 > +--ro srms-preference-tlv > +--ro preference? uint8 > > Disclaimer: I don't follow LSR... > > Regards, > Reshad. _______________________________________________ Lsr mailing list Lsr@ietf.org https://www.ietf.org/mailman/listinfo/lsr