Hi Mahesh,
> On Apr 1, 2025, at 5:14 PM, Mahesh Jethanandani via Datatracker
> <nore...@ietf.org> wrote:
>
> Mahesh Jethanandani has entered the following ballot position for
> draft-ietf-ospf-sr-yang-37: No Objection
>
> When responding, please keep the subject line intact and reply to all
> email addresses included in the To and CC lines. (Feel free to cut this
> introductory paragraph, however.)
>
>
> Please refer to
> https://www.ietf.org/about/groups/iesg/statements/handling-ballot-positions/
> for more information about how to handle DISCUSS and COMMENT positions.
>
>
> The document, along with other ballot positions, can be found here:
> https://datatracker.ietf.org/doc/draft-ietf-ospf-sr-yang/
>
>
>
> ----------------------------------------------------------------------
> COMMENT:
> ----------------------------------------------------------------------
>
> Section 1, paragraph 0
>> This document defines a YANG data model [RFC7950] that can be used to
>> manage OSPFv2 extensions for Segment Routing [RFC8665] and OSPFv3
>> extensions for Segment Routing [RFC8666] for the MPLS data plane. It
>> is an augmentation to the OSPF YANG data model [RFC9129].
>
> This is a similar comment to the YANG module for SR on ISIS. There seems to be
> some confusion on the use of the terms "YANG module" and "YANG data model" in
> this document. A "YANG data model" refers to a collection of YANG modules and
> submodules that together define a structured representation configuration,
> operational data, notifications, and RPCs for a given system or protocol,
> while
> a "YANG module" refers to a specific YANG file (.yang) defining a set of nodes
> (container, list, leaf, etc.) that represent configuration or state data.
> Moreover, a YANG module can be independent and augment other modules.
>
> Based on that definition, what you seem to be defining is a YANG module more
> than a YANG data model. Can that be reflected consistently in this document?
I'll fix this.
>
> Section 3, paragraph 9
>> This version of this YANG module is part of RFC XXXX; see
>> the RFC itself for full legal notices.
>
> BTW, is there an instruction for the RFC Editor on what to do with RFC XXXX?
This is present is other YANG modules. For example, ietf-ospf.yang in RFC 9129.
>
> Section 3, paragraph 28
>> grouping sid-tlv-encoding {
>> description
>> "SID TLV Encoding - 20-bit label or 32-bit SID whose
>> interpretation is dependent on the TLV length (3 for an
>> MPLS label or 4 for a 32-bit value) or the TLV V-Flag and
>> L-Flag settings:
>>
>> If the V-Flag is set to 0 and L-Flag is set to 0:
>> The SID/Index/Label field is a 4-octet index defining
>> the offset in the SID/Label space advertised by this
>> router.
>>
>> If V-Flag is set to 1 and L-Flag is set to 1: The
>> ID/Index/Label field is a 3-octet local label where the
>> 20 rightmost bits are used for encoding the label value.";
>> reference
>> "RFC 8665: OSPF Extensions for Segment Routing, Section 5
>> RFC 8666: OSPFv3 Extensions for Segment Routing, Section 3";
>> leaf sid-raw {
>> type uint32;
>> description
>> "Raw SID value - labels are in the rightmost 20 bits of
>> the value";
>> }
>> choice sid-value {
>> case label-sid {
>> leaf label-value {
>> type uint32 {
>> range "0 .. 1048575";
>> }
>> description
>> "A 20-bit MPLS Label";
>> }
>> }
>> case offset-sid {
>> leaf offset-value {
>> type uint32;
>> description
>> "Offset into a label space advertised by this router.";
>> }
>> }
>> description
>> "Choice of either a 20-bit MPLS lable or 32-bit offset into
>> an advertised label space.";
>> }
>> }
>
> I might be completely off, but what is the relationship between 'sid-raw' and
> the choice statement of 'sid-value'? Is the choice statement being used to
> determine what value 'sid-raw' will carry? If that is the case, YANG is being
> used to model a particular wire format. Tell me that is not what is happening
> here. I will note that Med has pointed out something similar as part of his
> DISCUSS in other parts of the model.
This the same value although the choice is the interpreted value and the
raw-sid is the value from the TLV without any interpretation. I was thinking
both could be useful but adamantly opposed to removing it.
>
> Section 4, paragraph 9
>> Some of the readable data nodes in this YANG module may be considered
>> sensitive or vulnerable in some network environments. It is thus
>> important to control read access (e.g., via get, get-config, or
>> notification) to these data nodes. Specifically, the following
>> subtrees and data nodes have particular sensitivities/
>> vulnerabilities:
>
> The paragraph seems to imply that specific subtrees and data nodes will be
> identified for vulnerability. Instead, what I see is a paragraph with some
> generic description. Did the authors forget to identify particular nodes or
> subtrees?
>
> No reference entries found for these items, which were mentioned in the text:
> [draft-ietf-rtgwg-segment-routing-ti-lfa].
What do you mean? It references the read-only Link State Database
augmentations. Are you asking me to resist every LSA?
The module ietf-ospf-sr-mpls augments base OSPF module Link State
Database (LSDB) with various TLVs. Knowledge of these data nodes
can be used to attack other routers in the OSPF domain.
>
> -------------------------------------------------------------------------------
> NIT
> -------------------------------------------------------------------------------
>
> All comments below are about very minor potential issues that you may choose
> to
> address in some way - or ignore - as you see fit. Some were flagged by
> automated tools (via https://github.com/larseggert/ietf-reviewtool), so there
> will likely be some false positives. There is no need to let me know what you
> did with these suggestions.
>
> Section 3, paragraph 10
>> reference
>> "RFC XXXX";
>
> s/RFC XXXX/RFC XXXX: A YANG Data Model for OSPF Segment Routing for the MPLS
> Data Plane/
Will fix.
Thanks,
Acee
>
>
>
_______________________________________________
Lsr mailing list -- lsr@ietf.org
To unsubscribe send an email to lsr-le...@ietf.org
