Subject: Gen-ART LC review of draft-ietf-pce-path-key-03

From: Ben Campbell <[email-masked]>
Date: Mon, 13 Oct 2008 16:42:21 -0500

Please resolve these comments along with any other Last Call comments
you may receive.

Document: draft-ietf-pce-path-key-03
Reviewer: Ben Campbell
Review Date:  2008-10-13
IETF LC End Date: 2008-10-22

Summary:

This draft is almost ready for publication as a proposed standard. I  
have a few nits and editorial comments that should be considered first.

Comments:

Section 2.1, paragraph 3:

The last sentence is confusing. "...until the LSR that can process  
it." does not seem to describe an event that one can wait "until".  
Should it say "...until it reaches the LSR..."?

Section 2.2, paragraph 1:

  s/ingress/"ingress LSR"

Sections 3.1.1 and 3.1.2

No explicit definitions for "Path Key" in either section. If the  
intent is for the language in section 3.1 to serve as the definition  
in each of these subsections, it would be good to have something like  
"Path Key: See section 3.1".  (Although just reprinting it here would  
allow each of the formal subobject definitions to stand alone a little  
better.)

Section 4:

The section covers actions in failure cases, i.e. if the PCE does not  
recognize itself, or if the requesting LSR receives a negative reply.  
The actions taken in the success case may seem to obvious to state,  
but it would be good to state them explicitly anyway :-)

Section 5, third bullet point in first list:

Do you mean "DoS attacks" rather than "DNS attacks"?

Section 6.1, paragraphs 2 and 3:

Can you either restate the suggested default, or reference the  
section? Otherwise, this requires a bit of an easter egg hunt on the  
part of the reader.

6.2, paragraph 1:

If I read this right, you state a normative requirement that another  
draft be updated. That seems an odd use of normative language. In any  
case, am I correct in assuming that someone is ensuring that this  
update happens?

References:

Outdated reference: draft-ietf-ccamp-inter-domain-recovery-analysis  
has been published as RFC 5298.

_______________________________________________
Ietf mailing list
[email-masked]
https://www.ietf.org/mailman/listinfo/ietf

---

From: "Adrian Farrel" <[email-masked]>
Date: Tue, 14 Oct 2008 00:20:28 +0100

Hi Ben,

Thanks for the time and effort.

Responding as an author not as a WG chair...

> The last sentence is confusing. "...until the LSR that can process  it." 
> does not seem to describe an event that one can wait "until".  Should it 
> say "...until it reaches the LSR..."?

The grammer and syntax are fine (you just have to know the "normal" ERO 
processing rules set out in RFC 3209 :-)

We can tidy as follows...

OLD
   The PKS MUST be preceded by an ERO subobject that
   identifies the LSR that must expand the PKS, so the PKS will not
   be encountered in ERO processing until the LSR that can process
   it.
NEW
   The PKS MUST be preceded by an ERO subobject that
   identifies the LSR that must expand the PKS. This means that
   (following the rules for ERO processing set out in [RFC3209])
   the PKS will not be encountered in ERO processing until the ERO
   is being processed by the LSR that is capable of correctly handling
   the PKS.

> Section 2.2, paragraph 1:

Not sure this is necessary. "Ingress" is generally used in related work with 
"Ingress LSR" being used the first time to help set the scene. Ditto 
"egress".

> Sections 3.1.1 and 3.1.2
>
> No explicit definitions for "Path Key" in either section. If the  intent 
> is for the language in section 3.1 to serve as the definition  in each of 
> these subsections, it would be good to have something like  "Path Key: See 
> section 3.1".  (Although just reprinting it here would  allow each of the 
> formal subobject definitions to stand alone a little  better.)

Hmmm.
This is a more fundamental concept and so is already defined in Section 1

   This document defines the notion of a Path Key that is a token
   that replaces a path segment in an explicit route. The Path Key is
   encoded as a Path Key Subobject (PKS) returned in the PCEP Path
   Computation Reply message (PCRep) ([PCEP]). Upon receiving the
   computed path, the PKS will be carried in an RSVP-TE Path message
   (RSVP-TE [RFC3209] and [RSVP-PKS]) during signaling.

I don't think we need to reproduce the definition or refer back to it from 
later in the document. It is, after all, the only thing that the whole I-D 
is about.

> recognize itself, or if the requesting LSR receives a negative reply.  The 
> actions taken in the success case may seem to obvious to state,  but it 
> would be good to state them explicitly anyway :-)

OLD
   The retrieval of the explicit path (the CPS) associated with a PKS
   by a PCC is no different than any other path computation request
   with the exception that the PCReq message MUST contain a PATH_KEY
   object and the Path Key bit of the RP object MUST the set.
NEW
   The retrieval of the explicit path (the CPS) associated with a PKS
   by a PCC is no different than any other path computation request
   with the exception that the PCReq message MUST contain a PATH_KEY
   object and the Path Key bit of the RP object MUST the set. On
   receipt of a PCRep containing a CPS, the requesting PCC SHOULD
   use insert the CPS into the ERO that it will signal, in accordance
   with local policy.

> Do you mean "DoS attacks" rather than "DNS attacks"?

Thanks!

> Can you either restate the suggested default, or reference the  section? 
> Otherwise, this requires a bit of an easter egg hunt on the  part of the 
> reader.

Para 2: yes. Should ref Section 2.1
Para 3: I think you mean para 4. Should also ref Section 2.1

> case, am I correct in assuming that someone is ensuring that this  update 
> happens?

Eeek!

I think we need to use 2119 language for what happens if the MIB document is 
updated to cover this function, but I don't think we can require that the 
document is updated.

Ops ADs may disagree!

> References:
>
> Outdated reference: draft-ietf-ccamp-inter-domain-recovery-analysis  has 
> been published as RFC 5298.

Thanks.
Time flies.

Cheers,
Adrian

---

From: Ben Campbell <[email-masked]>
Date: Mon, 13 Oct 2008 20:54:56 -0500

On Oct 13, 2008, at 6:20 PM, Adrian Farrel wrote:

> The grammer and syntax are fine (you just have to know the "normal"  
> ERO processing rules set out in RFC 3209 :-)
>

That helps, thanks.

> Not sure this is necessary. "Ingress" is generally used in related  
> work with "Ingress LSR" being used the first time to help set the  
> scene. Ditto "egress".
>

Okay. If that is the normal usage among MPLS documents, then it's fine  
with me.

>> intent is for the language in section 3.1 to serve as the  
>> definition  in each of these subsections, it would be good to have  
>> something like  "Path Key: See section 3.1".  (Although just  
>> reprinting it here would  allow each of the formal subobject  
>> definitions to stand alone a little  better.)
>
> Hmmm.
> I don't think we need to reproduce the definition or refer back to  
> it from later in the document. It is, after all, the only thing that  
> the whole I-D is about.

Understood, but I'm not sure I agree. The two sections I mentioned are  
the formal definition of the subobject syntax. People will jump to  
that section for reference purposes, or even try to cut-and-paste from  
it.  Even having a placeholder for Path Key, with the description of  
"defined by this document" would help. But even then, the rest of the  
document doesn't have a single  place that fully describes it. For  
example, Section 1 says it is a "token", but as far as I can tell (and  
I could be mistaken--I don't have the draft in front of me as I right  
this), the only place to learn it's length is in the diagrams in  
sections 3.1.1 and 3.1.2,

>
>
>> Section 4:
>>
>> The section covers actions in failure cases, i.e. if the PCE does  
>> not recognize itself, or if the requesting LSR receives a negative  
>> reply.  The actions taken in the success case may seem to obvious  
>> to state,  but it would be good to state them explicitly anyway :-)
>
> OLD

That helps, thanks.

>> section? Otherwise, this requires a bit of an easter egg hunt on  
>> the  part of the reader.
>

Yes, thanks.

>
>
>> 6.2, paragraph 1:
>>
>> If I read this right, you state a normative requirement that  
>> another draft be updated. That seems an odd use of normative  
>> language. In any case, am I correct in assuming that someone is  
>> ensuring that this  update happens?
>
> Eeek!
>
> I think we need to use 2119 language for what happens if the MIB  
> document is updated to cover this function, but I don't think we can  
> require that the document is updated.

I'm not sure if you mean the language is fine as is, or should be  
revised. I admit to not being an expert in what conventions we follow  
for keeping MIBs up to date with protocol extensions.

> Ops ADs may disagree!

Always!

>
>
>> References:
>>
>> Outdated reference: draft-ietf-ccamp-inter-domain-recovery- 
>> analysis  has been published as RFC 5298.
>
> Thanks.
> Time flies.
>
> Cheers,
> Adrian