Hi Tom,
On Thu, Jan 6, 2022 at 5:10 PM tom petch <[email protected]> wrote:
> Top posting a separate issue
>
> The PCC sends an Open message with a proposed value for Keepalive (can be
> zero)
>
> If receiver is ok with the proposed value, Open continues
>
> If receiver is not ok, then it sends a PCErr message Type 1 Value 4 with a
> suggested value
>
> If sender is willing to negotiate (negotiation enabled, min and max values
> configured for Keepalive), and the proposed value falls within range of min
> and max, then Open continues using the proposed value for Keepalive. If
> not, open fails.
>
> This implies that the sender is configured with boolean 'negotiation
> enabled', min and max acceptable values for Keepalive (values can be equal).
>
>
This seems correct.
> To me, this means:
> - if negotiation is disabled, then min and max MUST NOT be configured.
> - if negotiation is enabled, min and max MUST be configured with min GTE
> max,
>
> Have I understood?
>
> This is not what I see in the YANG module so if I am right, then I will
> propose some changes such as making 'enabled' a presence container with min
> and max in it and with a constraint max GTE min.. I have yet to look at
> the other negotiated values but expect that something similar will apply.
>
> If not ....?
>
>
What we currently have is -
+--rw allow-negotiation? boolean
+--rw max-keepalive-timer? uint8
+--rw max-dead-timer? uint8
+--rw min-keepalive-timer? uint8
+--rw min-dead-timer? uint8
Note that there is a difference between "unacceptable" and
"non-negotiable", the max value is used to also check if the received
parameter is acceptable, thus the max values are needed even when it is
non-negotiable.
I don't see a need to make max values mandatory to configure -- most
implementation would have some logic to initialize these values when they
bring the system up, as well as using the locally configured value as a
guide to generate min/max acceptable from the peer. Since this is local
behavior and not standardized, perhaps no need to update here. Thoughts?
BTW this is the same as our MIB - RFC7420.
Thanks!
Dhruv
Tom Petch
> ________________________________________
> From: Pce <[email protected]> on behalf of tom petch <
> [email protected]>
> Sent: 05 January 2022 12:09
> Subject: Re: [Pce] I-D Action: draft-ietf-pce-pcep-yang-16.txt
>
> From: Dhruv Dhody <[email protected]>
> Sent: 23 October 2021 12:37
>
> 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...
>
> <tp>
> Some more thoughts
>
> /refrenc/referenc/ in lots of places
>
> 'Further incase when the entity is PCE '
> perhaps just
> 'When the entity is PCE '
>
> 'Each peer in the list is identified by its IP address'
> (addr-type, addr).
> I do not see addr-type in the YANG
>
> 5.4. The Session Lists
> The session list contains PCEP session
> Perhaps 'PCEP sessions'
>
>
> 'If either of these sessions reaches active state first,'
> perhaps
> 'When one of these sessions reaches the active state, '
>
> s.5.5
> PCEP MIB
> could do with a reference
>
> 7.1. Stateful PCE's LSP-DB
> ' In the operational state '
> perhaps 'operational datastore'
>
> 8.1
> What is the procedure when a PCE is both client and server?
>
> RFC8253 is TLS1.2 only and TLS1.3 bears little resemblance to anything
> that has gone before so probably worth mentioning that this is 1.2 and
> referencing RFC5246
>
> 9.1
> ' https://tools'
> datatracker I think
>
> TLP out of date
>
> ' leaf capability'
> I think that this should reference the IANA registry and not the RFC that
> created it since several of the bits do not appear in the RFC
>
> I echo the comment about the addresses being 'no-zone'
>
> NMDA I do not understand. I thought it meant that different states of the
> same entity appear in different datastores so we did not need
> leaf admin-status {
> leaf oper-status {
> but perhaps we do
>
> leaf open-wait-timer {
> type uint16;
> units "seconds";
> default "60";
> config false;
> likewise what does a default do for 'config false'? The RFC says that the
> value is fixed.
>
> /retreived/retrieved/
> in several places
>
> case auth-tls
> what if the role is both client and server?
>
> RPC
> might benefit from a NACM default deny all
>
> 9.2
> /tools/datatracker/
>
> TLP is out of date
>
>
>
> The modelling of Keepalive confuses me - I need to check the RFC and the
> MIB and come back to you
>
> Tom Petch
>
> On Thu, Mar 11, 2021 at 6:26 PM tom petch <[email protected]<mailto:
> [email protected]>> wrote:
> From: Pce <[email protected]<mailto:[email protected]>> on behalf
> of [email protected]<mailto:[email protected]> <
> [email protected]<mailto:[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<
> http://tools.ietf.org>.
>
> Internet-Drafts are also available by anonymous FTP at:
> ftp://ftp.ietf.org/internet-drafts/
>
>
> _______________________________________________
> Pce mailing list
> [email protected]<mailto:[email protected]>
> https://www.ietf.org/mailman/listinfo/pce
>
> _______________________________________________
> Pce mailing list
> [email protected]<mailto:[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
