Re: [Pce] WG LC of draft-ietf-pce-association-group

[Dhruv Dhody <[e-mail]>]

Hi Adrian, 

Thanks for your review and suggestions. I could not get to this earlier, apologies for that! Please see inline...

Diff: https://tools.ietf.org/rfcdiff?url1=https://tools.ietf.org/id/draft-ietf-pce-association-group-04.txt&url2=https://raw.githubusercontent.com/dhruvdhody-huawei/ietf/master/draft-ietf-pce-association-group-05.txt
Txt: https://github.com/dhruvdhody-huawei/ietf/blob/master/draft-ietf-pce-association-group-05.txt

> -----Original Message-----
> From: Pce [mailto:[e-mail]] On Behalf Of Adrian Farrel
> Sent: 05 February 2018 01:22
> To: [e-mail]
> Subject: Re: [Pce] WG LC of draft-ietf-pce-association-group
> 
> This is an important piece of work and there are a number of drafts
> dependent on it that will provide useful function. So we should definitely
> push forward to publication, but we also have to get this foundational
> document right otherwise we will struggle later.
> 
> I have some moderate-sized issues set out below. Most just need editorial
> work although a couple are more substantive technical points that may need
> changes or better explanation.
> 
> Thanks for the work.
> 
> Adrian
> 
> ---
> 
> I spent a lot of time trying to understand the relationship between the
> svec-list and the association group. This understanding was not enhanced
> by this document since the mention of svec is actually only in the RBNF in
> section 5.2.2. I think the only thing to do to make this clear is to add a
> specific section "Relationship with the SVEC List" and use that to explain
> how the two constructs relate.
> 
> Now, I know that this document doesn't define any Association Types, and
> that might mean that you think you don't have to describe the
> relationship: "It is up to the documents that define individual
> Association Types," you may say. But I think you have to give some
> guidance in this document since you are defining a mechanism that is at
> last superficially similar. And certainly some (but not all) of the
> motivations in 3.1 might be achieved using SVEC.
> 
[[Dhruv Dhody]] I don't see a harm in tackling with SVEC in this document. I have added - 

3.2.  Relationship with the SVEC List

   Note that, [RFC5440] defines a mechanism for the synchronization of a
   set of path computation requests by using the SVEC (Synchronization
   VECtor) object, that specifies the list of synchronized requests that
   can either be dependent or independent.  The SVEC object identify the
   relationship between the set of path computation requests, identified
   by 'Request-ID-number' in RP (Request Parameters) object.  [RFC6007]
   further clarifies the use of the SVEC list for synchronized path
   computations when computing dependent requests as well as describes a
   number of usage scenarios for SVEC lists within single-domain and
   multi-domain environments.

   The motivation behind the association group defined in this document
   and the SVEC object are quite different.  The PCEP extensions that
   defines new association type, should clarify the relationship between
   SVEC object and association type, if any.

Let me know if you wish to add anything else? 

> ---
> 
> You don't make a lot of mention of RFC 6780. In fact, you only use it to
> define the association source. I think you need to describe how the PCEP
> Association maps to the RSVP-TE Association at least at a high level.
> (Again, I hear you saying that is an issue for the subsequent documents,
> but I think you have to give a framework.
> 
[[Dhruv Dhody]] Sure, I have added - 

   [RFC6780] defines the RSVP ASSOCIATION object, which was defined in
   the context of GMPLS- controlled Label Switched Paths (LSPs) to be
   used to associate recovery LSPs with the LSP they are protecting.
   This object also has broader applicability as a mechanism to
   associate RSVP state and [RFC6780] described how the ASSOCIATION
   object can be more generally applied.

...

5.2.  Relationship with the RSVP ASSOCIATION

   The format of PCEP ASSOCIATION Object defined in this document, is
   aligned with the RSVP ASSOCIATION object ([RFC6780]).  Various
   Association-Types related to RSVP association are defined in
   [RFC4872], [RFC4873], and [RFC7551].  The PCEP extensions that
   defines new association type, should clarify how the PCEP association
   would work with RSVP association and vice-versa.

> ---
> 
> Why is there a different timeout for association state and LSP state?
> 
> You have
> 
>    Association Timeout Interval: when a PCEP session is terminated, a
>    PCC waits for this time period before deleting associations created
>    by the PCEP peer.
> 
> and
> 
>    The association are properties of the LSP and thus could be stored in
>    the LSP state database.  The dynamic association exist as long as the
>    LSP state.  In case of PCEP session termination, the LSP state clean
>    up SHOULD also take care of associations.
> 
> Section 5.3 is not altogether clear and I suspect some of the procedures
> it describes are not defined in this document but are inherited from
> another document.
> 
[[Dhruv Dhody]] "Association Timeout Interval" has been removed but not cleaned up from the terminology section; thanks for pointing that out. 
Regarding section 5.3 (now 5.4), I have made some modifications, please have a look. 

> ---
> 
> 3.3 has
> 
>    A range of association identifier for each association-type are kept
>    for the operator-configured associations.  Dynamic associations MUST
>    NOT use the association identifier from this range.
> 
> What error code is used if a dynamic association breaks this rule? I can't
> see the behavior described.
> 
[[Dhruv Dhody]] I have added - 

   If a PCEP speaker receives ASSOCIATION object for an operator
   configured association and the association identifier is not in the
   operator-configured association range for the association-type and
   association-source, it MUST return a PCErr message with Error-Type 26
   (Early allocation by IANA) "Association Error" and Error-Value 8
   "Association identifier not in range".

> ---
> 
> As far as I can see, the only reason you need the Operator-configured
> Association Range TLV is because you allow the Association Source to be
> set arbitrarily. But there is completely no value in doing that! An
> association is created by some element in the system and that element
> needs to be clearly identified. Thus the association ID is not globally
> unique, but is interpreted in the context of the association source.
> 
> Now, when a new association is advertised by a PCEP speaker, it only has
> to fill in itself as the source and then it can choose an association ID
> freely according to its own configuration an without negotiation with its
> peers.
> 

[[Dhruv Dhody]] It is the combination of Association ID, Type and Source that uniquely identifies an association group, I have clarified this as association parameters in the draft. Because of the operator configured manual associations and for the PCE redundancy cases, the mandate of using a PCEP speaker address only was found to be limiting. 
We received inputs from other association drafts authors, and in consultation with the WG we decided to allow other values for the source. 

Now we needed to solve a conflict problem for an association type that allows both dynamic and operator-configured associations (and uses the same association source). You are right that one way to solve this would be to mandate that operator configured association to not use PCEP speaker address as source and avoid a clash, but that would lead us back to the original text which mandated the usage of association source; we thought using range as good way out (as it would be needed only for a few of those association types that are both dynamic and operator configured). 

I have added an example in appendix as well for the range.  


> ---
> 
> Notwithstanding the previous point, I think there is a slew of error cases
> with the Operator-configured Association Range TLV that haven't described.
> The start+range for given type could be larger than 0xffffffff. Two ranges
> may overlap. The range could be zero.
> 

[[Dhruv Dhody]] Thanks for pointing that out. I have added text for this cases. 

      Range (2 bytes): The number of associations marked for the
      Operator-configured Associations.  The Range MUST be greater than
      0, and it MUST be such that (Start-Assoc-ID + Range) do not cross
      the association identifier range of 0xffff.

...

   If the
   Start-Assoc-ID or Range are set incorrectly, the PCEP session MUST be
   rejected with error type 1 and error value 1 (PCEP session
   establishment failure / Reception of an invalid Open message).

> ---
> 
> 5.1
> 
>    R (Removal - 1 bit):  when set, the requesting PCE peer requires the
>       removal of an LSP from the association group.  This flag is used
>       for ASSOCIATION object in PCRpt and PCUpd message, the flag is
>       ignored in other PCEP messages.
> 
> What does R==0 signify?
> 
[[Dhruv Dhody]] Not removal! :) 
Do you see a need for some text here? (This would be similar to R flag in LSP object for PCRpt?) 

> ---
> 
> 5.1
> 
>    Association type (2-byte): the association type (for example
>    protection).  The association type are defined in separate documents.
> 
> You should delete this example as you have no firm evidence that there
> will be a protection association type. On the other hand, you should
> reference section 6.4.
> 
[[Dhruv Dhody]] Ok
> ---
> 
> 5.1
> 
>    Association Source: 4 or 16 bytes - An IPv4 or IPv6 address.  This
>    could be the IP address of the PCEP speaker that created a dynamic
>    association, an operator configured IP address, or an IP address
>    selected as per the local policy.  The value such as 0.0.0.0 or
>    ::/128 are acceptable.
> 
> As per my previous comments, I'm not comfortable with a lot of this.
> If you use anything other than a local identifier (for creating an
> association) or the identifier used when the association was created (for
> modifying an existing association) then you have a recipe for complete
> chaos!
> 
> It may be that what you want to do here is completely rational, but the
> explanation is foggy at best. Perhaps an approach is to describe this
> field as:
> 
>    Association Source: 4 or 16 bytes - An IPv4 or IPv6 address that
>    provides scoping for the Association ID.
> 
> Then, in the procedures, you can describe for the different cases
> (e.g., configured, dynamic, created, modified) how the value is assigned
> and what values are legal. You need to be pretty careful with what
> addresses are valid, but especially careful with v6 address.
> 
[[Dhruv Dhody]] Thanks for the suggestion, please check the updated version if you are happy with the updated text. There is a separate section on association source now as - 

5.1.3.  Association Source

   The Association Source field in the ASSOCIATION object is set to a
   valid IP address to identify the node that originate the association.
   In case of dynamic associations, the association source is usually
   set as the local PCEP speaker address, unless local policy dictates
   otherwise, in which case association source is set based on the local
   policy.  In case of PCE redundancy, local policy could set the source
   as virtual IP which identify all instances of PCE.  In case of
   operator configured association, the association source is manually
   configured and it could be set as one of the PCEP speakers, Network
   Management System (NMS), or any other valid IP address that scopes
   the association identifier for the association type. 


> ---
> 
> Nowhere is it clear whether an {Association ID, Association Source} must
> be unique or can be re-used with a different Association Type.
> If unique then what happens if a PCEP speaker uses a valid ID/Source pair
> but gets the Type wrong? Perhaps that would be Error-Value 6 "Association
> information mismatch".
> 

[[Dhruv Dhody]] I have added this - 

5.1.4.  Unique Identification for an Association Group

   The combination of the mandatory fields Association type, Association
   ID and Association Source in the ASSOCIATION object uniquely identify
   the association group.  If the optional TLVs - Global Association
   Source or Extended Association ID are included, then they MUST be
   included in combination with mandatory fields to uniquely identifying
   the association group.  In this document, all these fields are called
   'association parameters'.  Note that the ASSOCIATION object MAY
   include other optional TLVs (not defined in this document) based on
   the association types, that provides 'information' related to the
   association type, this document uses the term 'association
   information' for it.
 
BTW, you are right about the error! 


> ---
> 
> 5.2
> 
> Presumably a PCErr can also include the <association list>.
> 
[[Dhruv Dhody]] We don't carry the 'offending' object in PCErr, thus do not see a need for extending PCErr.    

> ---
> 
> 5.2.2
> 
> I should have thought that if an association list can be present on a
> PCReq it would also be returned on the PCRep (with interesting variants
> like "you can have it in these associations, but not in those", and
> possible need for an OF or two).
> 
[[Dhruv Dhody]] With NO-PATH this could be useful. I have added it in the update (section 5.3.3)

> ---
> 
> 5.3
> 
> Presumably if the R bit is set and the LSP was not in the association, we
> act as though it had been and we had removed it?
> 
[[Dhruv Dhody]] We have this text in the draft - 

   If a PCEP speaker receives ASSOCIATION object with R bit set for
   removal, and the association group (identified by association
   parameters) is not known, it MUST return a PCErr message with Error-
   Type 26 (Early allocation by IANA) "Association Error" and Error-
   Value 4 "Association unknown".

Since this LSP is not in the association, we considered this as an association not known in the context of the LSP and re-used the error. If this is not clear we could define a new error-value instead? 

> ---
> 
> 6.
> 
> It would be hugely wise to go through the document and replace each "TBD"
> with "TBDn" (for n=1....) so that IANA and the RFC editor can clearly tell
> which case is which.
> 
[[Dhruv Dhody]] Ack, we already have early IANA allocation we have reflected that in this update of the draft. 
> ---
> 
> 7.
> 
> Can I deduce things about customers, the services they have bought, and
> their relationships from the association information? - that would be a
> privacy concern.
> 
> Can I deduce anything about the network from the association source ID?
> 
> Note that privacy is not just about encrypting info in PCEP messages to
> stop outsiders snooping it; it is also about only revealing to your peers
> information that should be shared, and protecting access (e.g., through
> management tools) to private information.
> 
[[Dhruv Dhody]] I have added this - 

   Much of the information carried in the ASSOCIATION object, as per
   this document is not extra sensitive.  It often reflects information
   that can also be derived from the LSP Database, but association
   provides a much easier grouping of related LSPs and messages.
   Implementations and operator can and should use indirect values in
   ASSOCIATION as a way to hide any sensitive business relationships.

Do you have any additional text on this? 

> ---
> 
> 8.
> 
> Should I be able to see which associations are known about at a PCEP peer
> and which LSPs belong to them?
> 
[[Dhruv Dhody]] We have this in 8.2 - 

8.2.  Information and Data Models

   An implementation SHOULD allow the operator to view the associations
   configured or created dynamically.  Further implementation SHOULD
   allow to view associations reported by each peer, and the current set
   of LSPs in the association.  To serve this purpose, the PCEP YANG
   module [I-D.ietf-pce-pcep-yang] includes association groups.

   It might also be useful to find out how many associations for each
   association type currently exist and to know how many free
   association identifiers are available for a particular association
   type and source.

> Should I be able to find out which ranges are (nearly) depleted?
> 
[[Dhruv Dhody]] Added (see above). 
> ---
> 
> You have used [I-D.ietf-pce-pceps] in a normative way.
> 
[[Dhruv Dhody]] Ack. 
> ====
> 
> Nits...
> 
> ---
> 
> A few references seem to be out of date.
> 
> ---
> 
> There are quite a few editorial nits which would be good to clean up
> before sending to the RFC editor.
> 
> ---
> 
> Section 5.2 needs a reference to RFC 5511 to give context to the encoding.
> 
[[Dhruv Dhody]] Ack (for all nits!) 

Thanks again for your review and great set of comments! 

Regards,
Dhruv

> > -----Original Message-----
> > From: Pce [mailto:[e-mail]] On Behalf Of Julien Meuric
> > Sent: 01 February 2018 14:10
> > To: [e-mail]
> > Subject: [Pce] WG LC of draft-ietf-pce-association-group
> >
> > Hi all,
> >
> > This message initiates a 2-week WG last call for
> > draft-ietf-pce-association-group-04. Please review and share your
> > feedback on the PCE mailing list. This LC will end on Thursday February,
> 15.
> 
> _______________________________________________
> Pce mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/pce