Hi,
A few nitty comments on this draft.
Adrian
===
Abstract. Second sentence.
"In PCE-based environments, it is thus critical..."
I don't think this follows as a conclusion from the previous sentence. In
particular, there is no definition of a "path computation chain" and no
explanation of the process involved in PCE that leads to the conclusion that
certain things need to be monitored or troubleshooted.
I suggest inserting a new second sentence to give some background. Something
like...
Path Computation Clients (PCCs) send computation requests to PCEs,
and these may forward the requests to and cooperate with other PCEs
forming a "path computation chain".
You can then put in a paragraph break and continue with your text.
===
Section 1
You need to expand "PCC" and "TE LSP" on first usage as the Terminology
section comes later.
===
Section 3
Please include a reference to draft-farrel-rtg-common-bnf-07.txt.
At this stage you should (and can safely) make this a normative reference.
The RBNF draft will go to IETF last call in early January and will be ahead
of this draft in the processing queue.
===
Section 3
s/Computation Monitoring request/Computation Monitoring Request/
===
Section 3.1
<request> is shown to optionally contain the RRO.
The text that follows refers to the ERO.
I think the text is wrong.
===
Section 3.1
s/metric(s)/metrics/
===
Section 3.1
Examples 1 and 3 should also include a note that the requested information
is returned in a PCMonRep.
===
Section 3.2
Are you sure that you want to send an Error message in response to a bad
PCMonRep?
Sending an Error in response to a PCMonReq is useful because it cancels the
request and means the sender does not continue to wait or a PCMonRep. But
sending an Error in response to a PCMonRep does not seem to have any use
except to load the network. For example, what is the state of the
outstanding PCMonReq in this situation? What is the sender of the PCMonRep
supposed to do when it receives the Error message?
More appropriate would be for the receiver of the bad PCMonRep to raise an
alert to the operator and to put the sender of the bad message on a
blacklist so that it never send it another PCMonReq.
===
Section 3.2
It would be useful to include references for the objects listed in the BNF
as you do in Section 3.1.
===
Section 3.2
I'm slightly confused by the BNF.
<metric-pce>::=[<PCE-ID>]
[<PROC-TIME>]
[<CONGESTION>]
This means that <metric-pce> can be completely empty.
Surely <PCE-ID> is mandatory within <metric-pce> ?
===
Section 4
You state that the P and I flags in the new objects are cleared.
I think you mean "SHOULD be set to zero and MUST be ignored"
===
Section 4
Second paragraph.
The case of "in band" monitoring seems to be described twice.
===
Section 4.2
The object has previously been referred to as the PCE-ID object. That seems
like a better name. Can you check the whole I-D to make sure this is
consistent.
===
Section 4.3
If allowed by policy, the PCE includes a PROC-TIME object within a
PCMonRep or a PCRep message if the P bit of the MONITORING object
This conflicts with Section 4.1
P (Processing Time) - 1 bit: the P bit of the MONITORING object
carried in a PCMonReq or a PCReq message is set to indicate that the
processing times is a metric of interest, in which case a PROC-TIME
object MUST be inserted in the corresponding PCMonRep or PCRep
message.
I prefer the "if allowed by policy" which means you need to change the text
Section 4.1.
===
Section 4.3
s/algorithm(s)/algorithms/ (twice)
===
Section 4.3
Flags: 18 bits - No Flags are currently defined:
Looks like 16 bits, and one flag is defined!
===
Section 4.3
s/computation(s)/computations/
===
Section 4.3
Unassigned bits are considered as reserved and MUST be set to zero on
transmission.
Move this text to immediately after the definition of the E bit
===
Section 4.3
More granularity may be introduced in further revision of this
document to get a monitoring metric for a general request of a
particular class (e.g. all PCReq of priority X).
I guess you decided not to do this. Delete the text.
===
Section 4.4
- Rather than one bit and a reserved field, shouldn't you have a
flags field (with one bit assigned) and a reserved field.
- You need to explain that setting the C bit and a congestion
duration of zero has meaning. If it has no meaning, you don't
need the C bit.
- Although it verges on telling the implementer how to write code
I think some advice is needed on what to do when a
CONGESTION object is received with the C bit set. There
are two aspects...
- How long has the CONGESTION object taken to propagate
from the reporting PCE to the PCC (might not be one hop)?
- What should the receiver do with the Congestion Duration
value?
- It may also be worth noting whether a PCE in the middle of a
chain is allowed to look at the CONGESTION information
that it receives from a downstream PCE and plans to pass on
to an upstream PCE or PCC.
===
Section 6
Reception of a PCMonReq message: upon receiving a PCMonReq message:
Can you sort out the over-use of colons?
===
Section 6, case 1)
I think you should refer explicitly to Section 6.9 of [PCEP] because that
includes not only the error message, but some important back-off and
shutdown procedures.
===
Section 6, case 2)
You should refer back to Section 5.
But note also that you have an explicit case (section 4.3) where policy
causes the message to be processed but an object to be left out of the
reply.
===
Section 6
You need a similar set of text for the in band case.
===
Section 7.2
s/MAY/may/
===
Section 7.6
An implementation SHOULD allow handling PCReq messages with
a higher priority than PCMonReq messages.
And when the monitoring is in band?
===
Section 8
It would be helpful to IANA if you split this into subsections for each
registry/subregistry.
Also nice if you could try to cut and paste from the registry naming used in
[PCEP].
===
Section 8
The IANA section doesn't seem to be complete.
For example:
- The flags field in the Monitoring Object
(be careful to get the bit numbering correct :-)
- The flags field in the PROC-TIME object
- The flags field in the CONGESTION object
===
Section 9
Does the information gathered by monitoring represent any additional
vulnerability? Could an attacker gain interesting information by snooping
this?
I think the Congestion object is a good and lightweight way to attack a PCE
deployment. You should note this and suggest that this makes the use of
security mechanisms important. You may also need to node that there is a
chain of trust model here such that even if one PCE ensures that it uses
security, the information it supplies can be changed on a hop further
upstream. Therefore, a consistent security model across all cooperating PCEs
is desirable.
===
Section 11.2
[I-D.ietf-pce-disco-proto-isis] is not referenced (did you run I-D nits?)
[I-D.ietf-pce-of] and [I-D.ietf-pce-pcep-xro] are normative since they
define objects that you cite explicitly.
_______________________________________________
Pce mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/pce
