Hi Tom, Apologies for the delay. I have made an update to the I-D.
https://www.ietf.org/rfcdiff?url2=draft-ietf-pce-pcep-yang-17 Further details inline... On Thu, Mar 11, 2021 at 6:26 PM tom petch <[email protected]> wrote: > From: Pce <[email protected]> on behalf of [email protected] < > [email protected]> > Sent: 22 February 2021 14:27 > > <tp> > I notice a number of glitches in this I-D, several about terminology, > others about arithmetic. I post the ones I have seen so far because my > cache is full, but I expect that there will be more! > > s.3 lacks > **PLSPID > ** Association > ** OF RFC5441 > **SRP RFC8231 > all of which I think are needed along with the relevant RFC as references > > [Dhruv]: Done! > RFC5540 is consistent in its terminology as it should be, but this I-D > does not follow that terminology and is not consistent. I refer to such as > Keepalive > OpenWait > KeepWait > DeadTimer > SyncTimer > back-off > Where these appear in text in the I-D then I think that they should appear > exactly as in the RFC. YANG leaf names is trickier. YANG does not do > camelcase so I think that hyphenated is best e,g, > open-wait, keep-wait; keep-alive I am unsure about and note that the I-D > has both with and without a hyphen which needs fixing > > [Dhruv]: Done! > grouping pce-scope lacks the bit name as in the RFC, L, R, S, Rd, etc > > leaf intra-area-pref { > does not say which is the higher pref and again lacks the names in the RFC > > [Dhruv]: Added > there are several 'domain' which is why the YANG convention is to make a > list name plural with the leaf therein as singular lest you get a path with > the same element repeated several times! > > [Dhruv]: Updated > container capability > the reference should be to the IANA registry not the RFCs; in two leaf > > [Dhruv]: Added > NAI needs expanding in the text, SID too > > [Dhruv]: Ack > container neigh > neigh is the sound that a horse makes but .... > > [Dhruv]: Updated > list domain perhaps better as list domains as above > > [Dhruv]: OK > admin-status oper-status > did NMDA make this approach redundant? > > [Dhruv]: Hmm, the current way is clearer with admin-status is boolean (and configurable) whereas oper-status has many more states (and config-false). > There is an awful lot of 'when pcc or pcc-and-pce. > It would be lovely if the pcc-and-pce case could be represented with two > values so that it becomes just when pcc; ditto pce > > [Dhruv]: I am not sure how to do that with enum. > leaf enabled { > type boolean; > description > "Enabled or Disabled"; > which is true? I know, obvious, but I like stating the obvious > > [Dhruv]: Updated. > leaf open-wait-timer { > the RFC says this is fixed not configurable. If there is a reason to > ignore the RFC then that needs to be spelt out IMO > leaf keep-wait-timer { > ditto > > [Dhruv]: It is marked config false already. It's kept mainly because RFC 7420 choose to keep it as well. > leaf dead-timer { > the RFC recommends a multiplier not a value which I think better lest an > operator increase the wait, forget to increase dead and so kills sessions > > [Dhruv]: Most implementation would set the dead timer value automatically based on the Keepalive timer. But it is useful for the dead timer value to be over-written via configuration when needed. You will also find DeadTimer included in the OPEN message as a value in time and thus consistent with RFC 5440 and RFC 7420. > leaf allow-negotiation { > default? > > [Dhruv]: Added > Zero means that the PCEP > entity will allow no Keepalive transmission at > all."; > This seems like a bad idea. If the peer is not allowed to send Keepalive > then I would expect them to be greeted with an error message > > [Dhruv]: The zero in max-keepalive-timer is to indicate that during the negotiation of the keepalive value in the Open message that the only acceptable value of peer's keepalive-timer is zero. If one receives an unacceptable value, that does lead to the PCErr message Error-Type=1 and Error-value=5! > Zero means that > the PCEP entity will allow not running a Dead > timer."; > Likewise, if the peer wants to run a Deadtimer how can you stop them? > > leaf min-dead-timer { > and again. I think that the underlying concepts need more consideration. > I see nothing like this in the RFC. > > [Dhruv]: PCEP session will not be established with session parameters are unacceptable during negotiation. Also, this aligns with RFC 7420. > leaf max-unknown-reqs { > I think that the RFC fixes this as five. > > [Dhruv]: it is recommended, not fixed. > /"The PCEP association type";/"The PCEP Association Type";/ > and the reference should be to IANA not RFC > > [Dhruv]: OK > /"PCEP Association Global Source.";/"PCEP Global Association Source.";/ > as per RFC; this also occurs lower down > > leaf srp-id { > RFC8231 says 0 and ffffffff reserved, YANG does not. > > /"The Path-key should be retreived";/"The Path-key should be retrieved";/ > > [Dhruv]: Ack for these! > case auth-tls { > if-feature "tls"; > choice role { > description > "The role of the local entity"; > case server { > > what happens when entity is pcc and pce? the PCC client must be the TLS > client but I do not know how this is handled. > > [Dhruv]: This is configured per peer. So a "pcc-and-pce" entity could act as a client towards one peer and server towards another. This is consistent with how RFC 5440 and RFC 8253 describe it. > .... appear transiently in this yang module. The > caps for YANG > > [Dhruv]: Ack > leaf dead-timer { > as above I like counts not times > > [Dhruv]: See above! > leaf overload-time { > in several places, does the time duration have any meaning when there is > no time of day to go with it to say when it started? > > [Dhruv]: Added > "The PST authorized"; > I am unclear why this is 'authorized; who has said so? > > [Dhruv]: Updated > notification pcep-session-local-overload { > as above does this need a data and time? > > [Dhruv]: Added > /"Trigger the resyncrinization at the PCE";/"Trigger the resynchronization > at the PCE"; > > leaf avg-rsp-time { > I would find > leaf rsp-time-avg > clearer for this and the other two times > > [Dhruv]: Updated > counters > do they need a discontinuity date and time? > > [Dhruv]: It was in the ietf-pcep, I moved to the ietf-pcep-stats module. > leaf num-keepalive-sent { > elsewhere keep-alive is hyphenated in YANG leaf names > > 5.2 > /capcabilities/capabilities > > [Dhruv]: Updated Thanks for your review! Dhruv > > There are parts of PCE I have not looked at previously and so plan to look > at the RFC which is likely to generate further comments. > > Tom Petch > > > > A New Internet-Draft is available from the on-line Internet-Drafts > directories. > This draft is a work item of the Path Computation Element WG of the IETF. > > Title : A YANG Data Model for Path Computation Element > Communications Protocol (PCEP) > Authors : Dhruv Dhody > Jonathan Hardwick > Vishnu Pavan Beeram > Jeff Tantsura > Filename : draft-ietf-pce-pcep-yang-16.txt > Pages : 112 > Date : 2021-02-22 > > Abstract: > This document defines a YANG data model for the management of Path > Computation Element communications Protocol (PCEP) for communications > between a Path Computation Client (PCC) and a Path Computation > Element (PCE), or between two PCEs. The data model includes > configuration and state data. > > > The IETF datatracker status page for this draft is: > https://datatracker.ietf.org/doc/draft-ietf-pce-pcep-yang/ > > There are also htmlized versions available at: > https://tools.ietf.org/html/draft-ietf-pce-pcep-yang-16 > https://datatracker.ietf.org/doc/html/draft-ietf-pce-pcep-yang-16 > > A diff from the previous version is available at: > https://www.ietf.org/rfcdiff?url2=draft-ietf-pce-pcep-yang-16 > > > Please note that it may take a couple of minutes from the time of > submission > until the htmlized version and diff are available at tools.ietf.org. > > Internet-Drafts are also available by anonymous FTP at: > ftp://ftp.ietf.org/internet-drafts/ > > > _______________________________________________ > Pce mailing list > [email protected] > https://www.ietf.org/mailman/listinfo/pce > > _______________________________________________ > Pce mailing list > [email protected] > https://www.ietf.org/mailman/listinfo/pce >
_______________________________________________ Pce mailing list [email protected] https://www.ietf.org/mailman/listinfo/pce
