Hi Tom,
> On Dec 11, 2023, at 7:45 AM, tom petch <[email protected]> wrote:
>
> A convenient addressee list so top posting my first thoughts on ospf-sr-yang,
> I hope to find time to have a more detailed look, at least at ospfv2.
>
> I have looked at ospf-sr-yang and have some queries.
>
> Is this all flavours of SR or just some? Most discussion I see these days
> relates to SRv6 I guess because SR-MPLS is mature in many respects but think
> that this I-D needs to spell out the scope (like its lsr twin)
This specifies OSPF SR for the MPLS data plane. I’m considering renaming the
data module to ietf-ospf-sr-mpls.yang as well.
>
> I note the import from sr-mpls and think it a mistake. The routing RFC says
> that new protocols should have a presence container to switch the protocol on
> and off which sr-mpls does not do but I think that ospf-sr-yang should follow
> the guidelines.
We need to follow the sr-mpls model. We can’t change it in the OSPF SR model.
>
> There are mentions of vendor augmentations but no indications of what they
> might be and, importantly, where they would go. Other I-D, anticipating
> augments, include containers explicitly for augments so that different
> vendors put the same information in the same place.
>
> I am used to ospfv2 and ospfv3 being derived identities from ospf which makes
> reference to one of the other or both simple, as ospf-yang does. Why not
> here?
I’ve updated these to use derived-from() and the current path.
>
> I-D references seems to lack
> RFC8102
> "draft-ietf-rtgwg-segment-routing-ti-lfa -
> Latter needs to be Normative since a feature
I hate making the latter normative but I guess it needs to be hopefully the
authors of this draft will finally bring it to completion.
>
> s.1.1 is ood
This has been removed.
>
> router-id is provided by RFC8294 so it should be imported and not be
> reinvented here
Okay - I have used this definition.
>
> import ietf-ospfv3-extended-lsa {
> lacks a reference clause
>
> leaf preference {
> type uint8;
> description
> "SRMS preference TLV, value from 0 to 255.";
>
> so what? what difference soes it make to be 0 or 255 or 42?
The description has been updated to indicate that an SR Mapping Server with a
higher preference is preferred.
Thanks,
Acee
>
> Tom Petch
>
> ________________________________________
> From: Lsr <[email protected]> on behalf of [email protected]
> <[email protected]>
> Sent: 05 December 2023 08:15
>
> Hi Acee,
>
> I've looked at the diff: the new version looks good to me. Thanks to the
> update.
>
> Regards,
>
> Julien
>
>
> On 01/12/2023 18:05, Acee Lindem wrote:
>> Hi Julien,
>>
>> Thanks much for your review. I’ve incorporated almost all of your comments
>> in the -23 version.
>>
>> See inline.
>>
>>> On Nov 29, 2023, at 11:03 AM, [email protected] wrote:
>>>
>>> Hello,
>>>
>>> I have been selected as the Routing Directorate reviewer for this draft.
>>> The Routing Directorate seeks to review all routing or routing-related
>>> drafts as they pass through IETF last call and IESG review, and sometimes
>>> on special request. The purpose of the review is to provide assistance to
>>> the Routing ADs. For more information about the Routing Directorate, please
>>> see https://wiki.ietf.org/en/group/rtg/RtgDir
>>> <https://wiki.ietf.org/en/group/rtg/RtgDir>
>>>
>>> Although these comments are primarily for the use of the Routing ADs, it
>>> would be helpful if you could consider them along with any other IETF Last
>>> Call comments that you receive, and strive to resolve them through
>>> discussion or by updating the draft.
>>>
>>> Document: draft-ietf-ospf-sr-yang-22
>>> Reviewer: Julien Meuric
>>> Review Date: 2023-11-29
>>> Intended Status: Standard Tracks
>>>
>>>
>>> *Summary:*
>>>
>>> This document is basically ready for publication but has nits that should
>>> be considered prior to publication.
>>>
>>>
>>> *Comments:*
>>>
>>> - The very first paragraph of the introduction/overview section summarizes
>>> the basis of YANG, XML, JSON, data models... I believe we are now far
>>> beyond those general considerations and we could skip that paragraph.
>> Removed - thanks.
>>
>>
>>> - In the grouping "ospfv3-lan-adj-sid-sub-tlvs" (p23), the leaf
>>> "neighbor-router-id" uses type "dotted-quad". This is consistent with RFC
>>> 8666 which specifies the associated OSPFv3 TLV, but we had a discussion
>>> about the type for router-id in the TE YANG models. The current resolution
>>> on TEAS side will be to consider a union of dotted-quad and ipv6-address. I
>>> wonder how much RTGWG would be ready to consider a superset of the existing
>>> OSPFv3 TLVs.
>> This is the OSPF Router-ID which is different from the OSPF TE Router-ID.
>> The two should not be confused as the OSPF Router ID is simply a 32 bit
>> unsigned integer that is typically represented in dotted quad format. It
>> only need be unique within the OSPF Routing Domain. Conversely, the OSPF TE
>> Router ID is a routable IPv4 or IPv6 address.
>>
>>> From RFC 2328 (which was inherited by RFC 5340):
>>
>> Router ID
>> A 32-bit number assigned to each router running the OSPF
>> protocol. This number uniquely identifies the router within
>> an Autonomous System.
>>
>>>
>>> *Nits:*
>>>
>>> - Multiple times in description: s/SR specific/SR-specific/
>> Fixed.
>>
>>
>>> - Multiple times in description: s/flag bits list/flag list/
>>> - Multiple times in description: s/flags list/flag list/
>> I changed these to either just “bits” or “flags” - the fact that it is a
>> YANG list need not be included in the description.
>>
>>
>>> - The description fields use a mix of "Adj sid", "adj sid", "Adj SID"...
>>> sometimes with hyphens (not to mention the full expansions). A single
>>> phrase should be chosen and used all along the module.
>> Changed them all to “Adj-SID” consistent with RFC8665.
>>
>>> - A few description starts with "The..." (e.g., in
>>> "ospfv2-extended-prefix-range-tlvs" on p 19, or v3 on p 22) while most of
>>> them don't. For consistency, it should be dropped from every brief
>>> description.
>> I removed “The “ from all the brief descriptions. I left it in two of the
>> TLV description that were written as complete sentences.
>>
>>> - In the grouping "ospfv3-prefix-sid-sub-tlvs" (p 21 and all resulting
>>> pieces of tree): s/perfix-sid-sub-tlvs/prefix-sid-sub-tlvs/
>>> - In the same grouping, the description of the container should be "Prefix
>>> SID sub-TLV *list*." (and "Prefix SID sub-TLV." reserved for the following
>>> list element).
>> Fixed both in the module and tree (which was regenerated from tree).
>>
>>
>>> - In the container "ti-lfa" (p 25): s/Enables TI-LFA/Enable TI-LFA/ [Not
>>> wrong, but should be consistent with others.]
>> Fixed.
>>
>>> - In the same container (p 26): "s/Topology Independent Loop Free
>>> Alternate/Topology-Independent Loop-Free Alternate/
>> Fixed in this place and in another.
>>
>>> - In section 3 (p 37): s/The YANG modules [...] define/The YANG module
>>> [...] defines/
>> Fixed.
>>
>>> - In the same section: s/in the modules/in the module/
>> Fixed.
>>
>>> - In the same section: s/Module ietf-ospf-sr/The module ietf-ospf-sr/
>> Fixed.
>>
>> Thanks,
>> Acee
>>
>>
>>>
>>> Thanks,
>>>
>>> Julien
>>>
>
> _________________________________
_______________________________________________
Lsr mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/lsr
