[Sidrops] Re: [yang-doctors] draft-ietf-sidrops-rtr-yang-05 early Yangdoctors review

["Roy, Jishnu" <[e-mail]>]

Hi Per,


As requested from your end, below lies the updated draft links with comments implemented.

https://datatracker.ietf.org/doc/draft-ietf-sidrops-rtr-yang/08/
https://author-tools.ietf.org/iddiff?url1=draft-ietf-sidrops-rtr-yang-07&url2=draft-ietf-sidrops-rtr-yang-08&difftype=--html

Let us know if you need any additional inputs from our end.

Best Regards,
Jishnu R

[image.png]

From: Per Andersson <[e-mail]>
Date: Tuesday, 1 September 2026 at 3:34 PM
To: Per Andersson <[e-mail]>
Cc: [e-mail] <[e-mail]>; [e-mail] <[e-mail]>; [e-mail] <[e-mail]>
Subject: [Sidrops] Re: [yang-doctors] draft-ietf-sidrops-rtr-yang-05 early Yangdoctors review

Hi,

See my comment inline.

Thanks for accomodating for most of the issues I raised.
I still have some comments for minor things, see inline
and new issues last.


On Wed, May 27, 2026 at 6:54 PM Per Andersson via Datatracker
<[e-mail]> wrote:
>
> Document: draft-ietf-sidrops-rtr-yang
> Title: YANG Data Model for RPKI to Router Protocol
> Reviewer: Per Andersson
> Review result: Ready with Issues
>
> Hi!
>
> This is my Early Review of draft-ietf-sidrops-rtr-yang-05.
> My conclusion is that the YANG modules are in good shape but still need
> some work.
>
> Note that I am no expert in RTR, so take my comments as such.
> I am mainly reviewing the YANG modules in this document.
>
> Result: Ready with issues
>
>
> Review of the ietf-rpki-rtr YANG module
> ---------------------------------------
>
> Major issues:
>
> Section 3.2 need to say that it includes normative references to the
> following RFCs 9911, 8349, 7317, 9644, 8343, 8177, 6810, 8210, and
> probably 8340 as well.

Ok.

Nit: order the RFCs numerically (ascending) for an easier read.


> The leaf last-session-up-down is of type yang:timestamp, which is
> measured in hundreths of a second. However, the description states that
> the value is a timestamp in microseconds relative to the Unix Epoch.
>
> Suggest to change the type to uinte32 and either add 'unit "usec"'
> statement or present the time unit in the description, or both.

Ok.


> The "unit" and description statements need to include the time unit for
> all of the timestamp and time leafs.

The descriptions for all the timestamps state "time of", but since the
yang:timestamp type is timeticks, it is "time since". Hundredths of
seconds since observed occurance.


> Nit: Suggest to remove the -timestamp suffix used, it is possible infer
> that it is a timestamp from the rest of the name.

Ok.


>
> Medium issues:
>
> The tree diagram in Sections 3.1 is not very helpful as it is are
> presented without context. I suggest that you take parts of the YANG
> trees and present them with some context, at a minimum references to
> relevant sections in e.g. RFC 8340.

Ok.


> Minor issues:
>
> I prefer to present "config" statements early, especially before
> enumeration types. See e.g. the session-state leaf, I suggest to start
> with the "config false" statement. (This is the statement ordering used
> in the connection-data container.)

No change, but I'll leave it to you. I'm ok with it as is.


> The threshold-percentage leaf can use the rt-types:percentage type, I
> leave this decision to you.

Ok.


>
> Review of the ietf-rpki-table YANG module
> -----------------------------------------
>
> Major issues:
>
> Section 4.2 needs to state that it inludes normative references to the
> following RFCs 9911, 8349, 6487, 8608, and probably 8340 as well.

Ok.


> Note that it is not explained anywhere what RFC YYYY is, guessing that
> it is draft-ietf-sidrops-rpki-ccr. Include this in the list of normative
> references as well, and either add a note that RFC YYYY is this draft,
> or update mentions of RFC YYYY to actually be this draft.

Ok.


> Medium issues:
>
> The tree diagram could be broken up per component and explained with
> some context as suggested for ietf-rpki-rtr.

Ok.


> Rename the ipv4-pfx-len and ipv6-pfx-len derived types to
> ipv4-prefix-length and ipv6-prefix-length. All other types have long
> names, no need to trade these bytes for lesser readability. If the RTR
> protocol doesn't require it to say "max-len" type it out in full
> "max-length" as well.

Ok.

(Is it an established term "max-len" or could that also be renamed
to "max-length"?)


> Is it possible to tighten up the type for the router-key list leaf
> "key", which is a subjectPublicKeyInfo? This is outside my area of
> expertise, but it seems there are requirements on format and content?

Ok.


> Nit: Suggest to remove the -time suffix used, it is possible infer that
> it is time from the rest of the name.
>
>
> General Nits:
>
> Section 2:
>
> OLD:
> Two YANG data models are defined in this document.
>
> NEW:
> Two YANG modules are defined in this document.
>
>
> OLD:
> The ietf-rpki-rtr.yang data model
>
> NEW:
> The ietf-rpki-rtr YANG module
>
>
> OLD:
> The ietf-rpki-table.yang data model
>
> NEW:
> The ietf-rpki-table YANG module

Leaving this to your discretion.


> $ pyang -f yang --yang-line-length=72 --ietf \
>         [e-mail] > new.yang
> $ diff [e-mail] new.yang
>
> 2c2
> <   yang-version "1.1";
> ---
> >   yang-version 1.1;
> 4c4,5
> <   prefix "rpki-rtr";
> ---
> >   prefix rpki-rtr;
> >
> 6c7
> <     prefix "yang";
> ---
> >     prefix yang;
> 11c12
> <     prefix "inet";
> ---
> >     prefix inet;
> 22c23
> <     prefix "ianach";
> ---
> >     prefix ianach;
> 27c28
> <     prefix "ssh";
> ---
> >     prefix ssh;
> 32c33
> <     prefix "if";
> ---
> >     prefix if;
> 45d45
> <
> 64d63
> <
> 97a97
> >
> 104a105
> >
> 111a113
> >
> 114c116
> <       length 20;
> ---
> >       length "20";
> 118a121
> >
> 123a127
> >
> 177a182
> >

Ok.


> $ pyang -f yang --yang-line-length=72 --ietf \
>         [e-mail] > new.yang
> $ diff [e-mail] new.yang
>
> 2c2
> <   yang-version "1.1";
> ---
> >   yang-version 1.1;
> 4c4,5
> <   prefix "rpki-table";
> ---
> >   prefix rpki-table;
> >
> 6c7
> <     prefix "yang";
> ---
> >     prefix yang;
> 11c12
> <     prefix "inet";
> ---
> >     prefix inet;
> 25d25
> <
> 44d43
> <
> 78a78
> >
> 85a86
> >
> 92a94
> >
> 95c97
> <       length 20;
> ---
> >       length "20";
> 99a102
> >
> 102c105
> <       length 32;
> ---
> >       length "32";
> 117a121
> >

Ok.


> enum reconnect
> enum idle-forever
> s/Diconncets/Disconnects/
>
> s/secruring/securing/

Ok.


Additional issues:

* Update the revision-dates for the YANG modules (match the draft
  published date).

* The prefix for ietf-ssh-client is defined as "sshc" in the YANG module.
  I suggest you use that when importing it.



--
Per

_______________________________________________
Sidrops mailing list -- [e-mail]
To unsubscribe send an email to [e-mail]