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:[email protected]>
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 <[email protected]> on behalf of Reshad Rahman
<[email protected]>
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
<[email protected]> wrote:
Thanks Reshad - are there any other notable discrepancies?
Thanks,
Acee
> On Nov 14, 2023, at 9:55 AM, Reshad Rahman
> <[email protected]> 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
> <[email protected]<mailto:[email protected]>> 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
[email protected]
https://www.ietf.org/mailman/listinfo/lsr
