ietf
[Top] [All Lists]

RE: [Pce] Last Call: <draft-ietf-pce-pcep-service-aware-12.txt> (Extensions to the Path Computation Element Communication Protocol (PCEP) to compute service aware Label Switched Path (LSP).) to Proposed Standard

2016-08-31 04:30:37
Hi Adrian, 

Thanks for your review and especially for providing suggested text.
I have attached the working copy and diff. 


-----Original Message-----
From: Pce [mailto:pce-bounces(_at_)ietf(_dot_)org] On Behalf Of Adrian Farrel
Sent: 30 August 2016 19:44
To: ietf(_at_)ietf(_dot_)org
Cc: pce(_at_)ietf(_dot_)org; 
draft-ietf-pce-pcep-service-aware(_at_)ietf(_dot_)org;
pce-chairs(_at_)ietf(_dot_)org
Subject: Re: [Pce] Last Call: <draft-ietf-pce-pcep-service-aware-12.txt>
(Extensions to the Path Computation Element Communication Protocol (PCEP)
to compute service aware Label Switched Path (LSP).) to Proposed Standard

I just read this document for IETF last call.

I feel a little bad that I didn't review the document in WG last call since
I normally participate in the PCE WG, but the timing wasn't good.

The document is both well written and clear.  I support advancing this
document, but I have a few small comments.

Thanks,
Adrian

---

Requirement 4 has

   4.  A PCE is not required to support service aware path computation.
       Therefore, it MUST be possible for a PCE to reject a PCReq
       message with a reason code that indicates service-aware path
       computation is not supported.

It may be better to phrase this in terms of legacy systems since (obviously),
a PCE that does not support this spec must still be able to reject requests.
Perhaps...

   4.  A PCE that supports this specification is not required to provide
       service aware path computation to any PCC at any time.
       Therefore, it MUST be possible for a PCE to reject a PCReq
       message with a reason code that indicates service-aware path
       computation is not supported.  Furthermore, a PCE that does not
       support this specification will either ignore or reject such
       requests using pre-existing mechanisms, therefore the requests
       MUST be identifiable to legacy PCEs and rejections by legacy
       PCEs MUST be acceptable within this specification.

This is not a change to the requirement and doesn't change your protocol
solution.

[Dhruv] Updated!  
---

4.1.1 has...

   - A path delay metric for the P2P path P = Sum {D(Lpi), (i=1...K)}.

Is it worth noting that the path delay metric is not the effective delay
along the path? That is, delay on the path will also be introduced by delays
in the nodes along the path. However, information about the delays introduced
by nodes along the path is not currently made available through the routing
protocols, and is unlikely to be consistent as network load varies.

Note that your definition of path delay is good. I am not proposing any change
to the definition, just and explanation of what it is not.

[Dhruv] Agreed, I can add a note with reference to [RFC7823] for this. 
---

In several places in Section 4 you refer to specific bits (P bit, B bit...).
I think it would be helpful to understand that these are bits in the flags
field of the Metric object. Maybe an easy way to deal with this is...

OLD
   The METRIC object is defined in section 7.8 of [RFC5440], comprising
   metric-value, metric-type (T field) and flags.  This document defines
   the following types for the METRIC object.
NEW
   The METRIC object is defined in section 7.8 of [RFC5440], comprising
   metric-value, metric-type (T field) and a flags field comprising a
   number of bit-flags (B bit, P bit).  This document defines the
   following types for the METRIC object.
END

[Dhruv] Done!
---

I think you need to add (a very small amount of) text to describe how this
all works for initiated LSPs. Just like Section 5.

[Dhruv] Added! 
---

4.1.1.1

   [RFC7471] and [RFC7810] define the "Unidirectional Link Delay Sub-
   TLV" in a 24-bit field.

I think this should be...

   [RFC7471] and [RFC7810] define the "Unidirectional Link Delay Sub-
   TLV" to report link delay in microseconds in a 24-bit field.

Similarly in 4.1.2.1

[Dhruv] Done! 
---

4.1.6 has

   This section defines the following optional types for the METRIC
   object for P2MP TE LSPs.

4.1 does not say whether the P2P metrics are optional or mandatory.
I suspect you mean...
- Implementations of this spec must implement TBD1, TBD2, and TBD3
- Implementations of this spec that support P2MP (which is an optional
  feature) must support TBD8, TBD9, and TBD10.

[Dhruv] I will remove the word optional from 4.1.6 to keep it consistent with 
P2P, and avoid the confusion. 
---

In 4.1.6.1 when you wrote

   - The P2P path delay metric of the path to destination Dest_j is
   denoted by LM(Dest_j).

I wonder whether you were thinking "LM means Link delay Metric"?
A bit odd that you have actually written "LM means P2P path delay metric"

Ditto LVM in 4.1.6.2

[Dhruv] Yes that is odd, Updated to PDM and PDVM for path delay metric and path 
delay variation metric. 
---

4.2.3.1

   A PCC SHOULD request the PCE to factor in the bandwidth utilization
   during path computation by including a BU object in the PCReq
   message.

I don't understand the use of "SHOULD" here. I suspect you mean...

   A PCC that wants the PCE to factor in the bandwidth utilization
   during path computation includes a BU object in the PCReq message.

What you have is a statement that unless there is a good reason why not,
a PCC is supposed to request the PCE to factor in the bandwidth utilization
during path computation.

[Dhruv] Good catch! 
---

4.2.3.1

I found the following clumsy and suggest a change...

OLD
   Multiple BU objects MAY be inserted in a PCReq or a PCRep message for
   a given request but there MUST be at most one instance of the BU
   object for each type.  If, for a given request, two or more instances
   of a BU object with the same type are present, only the first
   instance MUST be considered and other instances MUST be ignored.
NEW
   A PCReq or PCRep message MAY contain multiple BU objects so long as
   each is for a different bandwidth utilization type.  If a message
   contains more than one BU object with the same bandwidth utilization
   type, the first MUST be processed by the receiver and subsequent
   instances MUST be ignored.
END

[Dhruv] Updated!
---

4.2.3.1 should not define legacy behavior. A legacy node will not see this
spec and cannot be influenced by what you write. So...

OLD
   If a PCE receives a PCReq message containing a BU object, and the PCE
   does not understand or support the BU object, and the P bit is clear
   in the BU object header then the PCE SHOULD simply ignore the BU
   object.

   If the PCE does not understand the BU object, and the P bit is set in
   the BU object header, then the PCE MUST send a PCErr message
   containing a PCEP-ERROR Object with Error-Type = 3 (Unknown object)
   and Error-value = 1 (Unrecognized object class) as per [RFC5440].
NEW
   The behavior of a PCE that does not understand an object that it
   receives on PCReq message is defined in [RFC5440] and depends on the
   setting of the P bit in the object header (P bit clear means ignore
   the object, P but set means return an "Unknown object" error).
END

[Dhruv] I have updated it as - 
     If the BU object is unknown/unsupported, the PCE MUST follow
     procedures defined in [RFC5440]. That is, if the P bit is set, the
     PCE sends a PCErr message with error type 3 or 4 (Unknown / Not
     supported object) and error value 1 or 2 (unknown / unsupported
     object class / object type), and the related path computation
     request MUST be discarded.  If the P bit is cleared, the PCE is
     free to ignore the object.
---

4.2.3.1

Your use of passive voice may cause confusion. You write...

   The path computation
   request MUST then be cancelled.

Does this mean:
1. The PCE must do not more processing on the request and the PCC must
   not expect any further response
2. The PCC is required to issue a message to cancel the request


There are two cases of this text in this section.

[Dhruv] You are right, I have updated it to - 
     ... and the related path
     computation request MUST be discarded.

---

Section 5 does a nice job for stateful PCE. I think you also need to cover
PCE initiated LSPs.

[Dhruv] I have tried to handle it. 

---

Section 6 needs a reference to RFC 5511 so that the message formats can be
understood.

[Dhruv] added.

Thanks again for your comments!
 
Dhruv


-----Original Message-----
From: Pce [mailto:pce-bounces(_at_)ietf(_dot_)org] On Behalf Of The IESG
Sent: 23 August 2016 18:59
To: IETF-Announce
Cc: pce-chairs(_at_)ietf(_dot_)org; pce(_at_)ietf(_dot_)org; 
draft-ietf-pce-pcep-service-
aware(_at_)ietf(_dot_)org
Subject: [Pce] Last Call: <draft-ietf-pce-pcep-service-aware-12.txt>
(Extensions
to the Path Computation Element Communication Protocol (PCEP) to
compute service aware Label Switched Path (LSP).) to Proposed Standard


The IESG has received a request from the Path Computation Element WG
(pce) to consider the following document:
- 'Extensions to the Path Computation Element Communication Protocol
   (PCEP) to compute service aware Label Switched Path (LSP).'
  <draft-ietf-pce-pcep-service-aware-12.txt> as Proposed Standard

The IESG plans to make a decision in the next few weeks, and solicits
final comments on this action. Please send substantive comments to the
ietf(_at_)ietf(_dot_)org mailing lists by 2016-09-06. Exceptionally, 
comments may
be sent to iesg(_at_)ietf(_dot_)org instead. In either case, please retain 
the
beginning of the Subject line to allow automated sorting.

_______________________________________________
Pce mailing list
Pce(_at_)ietf(_dot_)org
https://www.ietf.org/mailman/listinfo/pce

Attachment: draft-ietf-pce-pcep-service-aware-13.txt
Description: draft-ietf-pce-pcep-service-aware-13.txt


<<< text/html; name="Diff_ draft-ietf-pce-pcep-service-aware-12.txt - draft-ietf-pce-pcep-service-aware-13.txt.html": Unrecognized >>>