Hi Acee,
All good with me.
Regards,Reshad.
On Friday, April 14, 2023, 01:13:28 PM EDT, Acee Lindem
<[email protected]> wrote:
Hi Reshad,
Thanks for the review. You raised some very good points and especially the
point about the OSPFv3 MPLS encoding being missing. We may delay the document
until these are added. However, we will address your comments in the next
revision of the document.
On Apr 3, 2023, at 23:18, Reshad Rahman <[email protected]>
wrote:
Line-breaks (hopefully) fixed below.
On Monday, April 3, 2023, 11:08:26 PM EDT, Reshad Rahman via Datatracker
<[email protected]> wrote:
Reviewer: Reshad Rahman
Review result: Ready with Issues
Main comments
=============
- Please add lots of references in the YANG. There are many (most/all?) nodes
e.g. m-bit, protocol-srgb etc etc, and probably types, which need references to
the relevant sections in corresponding RFCs.
We’ll add these.
- The document needs a least 1,probably more, examples.
Yes.
- The abstract mentions OSPF, the overview mentionsOSPFv2 only and the YANG
module has OSPFv2 and OSPFv3.
The authors will need to discuss this - it is only complete for OSPFv2 due to
the OSPFv3 dependence on the extended LSA YANG model
(https://datatracker.ietf.org/doc/draft-ietf-lsr-ospfv3-extended-lsa-yang/)
which didn’t exist when this document was first written. We may want to add and
delay publication.
- sr-algorithm should be a leafref to an algorithm somewhere, right now it's a
uint8.
I changed this to sr-cmn:prefix-sr-algorithm.
- No need to redefine uint24, it's already define in RFC8294.
Fixed.
- Leaf preference, no need for range 0..255 since it's a uint8
Fixed.
- Looking at RFC9020, I see that containersegment-routing, in grouping
sr-control-plane, is non-presence and leaf receive
has default value true, meaning receive is true by default even if the
container hasn’t been created. Is that the intention? And is it the desired
behaviour in OSPF? If no, you can add a refine statement. My guess though is
that is a mistake in RFC9020.
This leaf would be ignored if “enabled" is false (which is the default) in the
same grouping. If “enabled” is “true”, then it would be applicable.
Questions
=========
- extended-prefix-range-tlvs is used only for ospfv2. Is that on purpose. Since
there's an "af" node in the grouping, I assumed it'd be used for ospfv3 also.
BTW 'af' is defined as uint8, there's an address-family type in RFC6991 and
that should be used instead.
See discussion above on OSPFv3.
- Is uint16 sufficient for range-size?
Yes. See section 3.2 or RFC 8665.
- I see augment ospf:ospfv2 when rt:type = ospfv2, is the when-stmt needed? The
ospfv2container should only exist for ospfv2?
Yes - the encoding is completely different for OSPFv3.
- Leaf sid is a uint32. In SR models, is the Sid always represented as a uint32
or is there a type defined. Andshould it be a choice for 20-bit label v/s
32-bit SID.
It is not always a label - see section 5 of RFC 8665.
- List lan-adj-sid-sub-tlv in container lan-adj-sid-sub-tlvs, no need for a key?
It is not read-only and there can be one for each neighbor on the LAN. I
believe we need a key.
Thanks,Acee
Regards,
Reshad.
_______________________________________________
yang-doctors mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/yang-doctors
_______________________________________________
Lsr mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/lsr
_______________________________________________
Lsr mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/lsr
