Subject: [Gen-art] Gen-ART review of draft-ietf-mpls-tp-gach-gal-04.txt

From: "Miguel A. Garcia" <[email-masked]>
Date: Wed, 13 May 2009 10:51:18 +0200

I have been selected as the General Area Review Team (Gen-ART)
reviewer for this draft (for background on Gen-ART, please see
http://www.alvestrand.no/ietf/gen/art/gen-art-FAQ.html).

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

Document: draft-ietf-mpls-tp-gach-gal-04.txt
Reviewer: Miguel Garcia <[email-masked]>
Review Date: 12-May-2005
IETF LC End Date: 14-May-2005
IESG Telechat date: (if known)

Summary: The document is technically ready for publication as a standards 
track RFC, but there is such a number of minor issues that a new revision 
is recommended.

Major issues:

Minor issues:

- Rule of 5. The document lists 6 authors, but the RFC Editor has a
guideline for not exceeding the number of 5 authors. Please read Section
2.12 of http://www.rfc-editor.org/rfc-editor/instructions2authors.txt

- You have an unnumbered section called Requirements Language (in between
the Abstract and the Table of Contents). I think this does not follow the
guidelines of the RFC Editor. See the previously mentioned document, now
in Section 4, where you can see that the Abstract is followed by the
Table of Contents, not by "Requirements Language". Usually the paragraph
that refers to RFC 2119 is placed within the "Terminology" section.

- Same comment for Section 1.1, Contributing authors, which according to
the structure, it should instead go to the end of the document, before
the Security Considerations.

- And same comment for Acknowledgments, which should go in between
Contributors and Security Considerations.

- On Sections 1 and 4.1 the IP address 127/8 should be written as 
127.0.0.0/8.

- On Section 3.1, first paragraph, the text reads:

     The structure of ACH TLVs that
     MAY follow an ACH TLV Header is defined and described in the
     following sections.

I am not sure what the text wants to say, because it mean want to say either:

a) The structure of ACH TLVs MAY follow an ACH TLV Header ...
b) The structure of ACH TLVs that can follow an ACH TLV Header ..

If the intention is a), then this text should be written in Section 3.3,
which is the section that describes the ACH TLV object. If the intention
is b), then it is correctly placed, but please do use "can" instead of 
"MAY".

- On Section 4, 1st paragraph, last sentence: "this document suggests the 
value 13", I think this should only be a note in the IANA Section, so it 
should be deleted from here.

- On Section 4.2.1.1, last paragraph starts:

"The G-ACh message, the ACH or the GAL SHOULD NOT be modified towards the 
targeted destination".

My recommendation is to make the sentence in active voice, there, it is 
easier to see which node has the requirement. So, I suggest something 
around these lines:

"LSRs (or whatever node) SHOULD NOT modify...."

And this brings me to another comment... I don't have a clear explanation 
for exceptions to the "SHOULD NOT". Therefore, I believe it should be a 
"MUST NOT".

This comment also applies to the last paragraph of Section 4.2.1.2.

- On Section 4.2.1.2, the last paragraph on page 13 starts with:

"To send a G-ACh message on a control channel..."

That is a horrible long and complicated sentence, which has normative 
text. So, please, split the long one into smaller parts, isolating the 
normative aspects from the introduction and motivation, so that it can be 
clearly understood which entity MAY do what.

- On the last paragraph of Section 5, replace the "it" in "it MAY 
increment" with the real actor, presumably "An LER, LSR, or PE".

- Section 8. The suggestion to IANA should be a note that with time, gets 
deleted by the RFC Editor before publication of the RFC.

- I noticed that IANA has reviewed the document and has some questions 
and comments, so I guess the authors will address them as well.

/Miguel

-- 
Miguel A. Garcia
+34-91-339-3608
Ericsson Spain

---

From: "BOCCI Matthew" <[email-masked]>
Date: Wed, 13 May 2009 17:27:50 +0200

Miguel,

Many thanks for your detailed review. Please see below for responses.

Regards

Matthew

> -----Original Message-----
> From: Miguel A. Garcia [mailto:[email-masked]]
> Sent: 13 May 2009 09:51
> To: BOCCI Matthew; VIGOUREUX MARTIN; [email-masked];
[email-masked];
> [email-masked]; [email-masked]; [email-masked]
> Cc: [email-masked]; General Area Review Team
> Subject: Gen-ART review of draft-ietf-mpls-tp-gach-gal-04.txt
> 

The draft is the result of a merger between two former drafts specifying
the GACH and GAL concepts, respectively. The author's list represents
the editors of those original drafts, some of whom continued to edit the
combine draft.

> that refers to RFC 2119 is placed within the "Terminology" section.

OK. That is the result of the default XML2RFC template, which puts the
requirements language section in the header, for some reason. I'll try
to fix that in the updated draft.

> the Security Considerations.

OK

> Contributors and Security Considerations.

OK
 
> - On Sections 1 and 4.1 the IP address 127/8 should be written as

OK

> is b), then it is correctly placed, but please do use "can" instead of

The meaning is (a), so I will move the statement to 3.3.

> should be deleted from here.

OK

> "LSRs (or whatever node) SHOULD NOT modify...."

OK

> for exceptions to the "SHOULD NOT". Therefore, I believe it should be
a

I agree. In my view, once a G-ACh packet has been sent, no node has
visibility of it unless TTL expires or the GAL is exposed when a label
is popped. If this is at the targeted destination, for example indicated
by an address in an ACH TLV, then we are done. If this is not the
targeted destination, but the node has agreed to process the packet on
that ACH channel, then that processing is out of scope here except to
the extent that clearly the ACH type must be maintained if the packet is
forwarded to another node.
 
> clearly understood which entity MAY do what.
I assume this comment also applies to a similar paragraph in Section
4.2.1.1.  How about the following revisions?:

NEW paragraph in 4.2.1.2:
To send a G-ACh message on a control channel associated to the
Section, the head-end LSR (A) of the Section generates a G-Ach message,
to which it MAY prepend an ACH TLV Header and appropriate ACH TLVs. Next
the LSR adds an ACH. Finally it pushes a GAL LSE.

NEW paragraph in 4.2.1.1:
To send a G-ACh message on the LSP associated control channel,
the LER (A) generates a G-ACh message, to which it MAY prepend an ACH
TLV Header and appropriate ACH TLVs. It then adds an ACH, onto which it
pushes a GAL LSE.  Finally, the LSP Label LSE is pushed onto the
resulting packet.

> increment" with the real actor, presumably "An LER, LSR, or PE".

OK

> deleted by the RFC Editor before publication of the RFC.

OK

> and comments, so I guess the authors will address them as well.

Yes, those will be addressed.

---

From: "Miguel A. Garcia" <[email-masked]>
Date: Thu, 14 May 2009 11:24:41 +0200

Hi Matthew:

Some further discussion in those points that require it. See below.

> the editors of those original drafts, some of whom continued to edit the

Usually these drafts with a long list of authors come from the merger of 
several drafts... Well, I am just saying that the RFC Editor is going to 
complain. You can select a number of editors, and push some authors to 
the list of contributors, but from my point of view this will be solve 
either by you, your AD, or the RFC Editor.

> forwarded to another node.

Perhaps you should add this explanation, for example, as a note 
(indented). Well, it is up to you, but when I read the document I didn't 
understand the reason for SHOULD NOT rather than MUST NOT, so, any 
additional information that puts the reader into perspective is welcome.

> pushes a GAL LSE.  Finally, the LSP Label LSE is pushed onto the

Very good. And thanks for detecting that this affects to 4.2.1.1 as well.

Thanks,

         Miguel

-- 
Miguel A. Garcia
+34-91-339-3608
Ericsson Spain