Subject: [Gen-art] Gen-ART review of draft-ietf-trill-rbridge-protocol-14

From: "McCann Peter-A001034" <[email-masked]>
Date: Thu, 7 Jan 2010 15:13:24 -0500

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
<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-trill-rbridge-protocol-14
Reviewer: Pete McCann
Review Date: 2010-01-08
IETF LC End Date: 2010-01-11
IESG Telechat date: unknown 

Summary: Basically ready, a few minor issues & nits.

Major issues: None.

Minor issues:

Section 4.1.4:
This text seems to imply that the FCS of the inner, native frame is
removed and only a single FCS covering the encapsulated frame is
generated.  However, it is not quite clear to me which behavior 
is intended.  Could you add text to clarify?

Section 4.5:
   The k trees are ordered from 1 to k, with up to k of the s trees
   advertised by RB1 given tree numbers 1 through s, respectively, and
   any remaining trees given numbers in order of priority.
Is this worded correctly?  Should it say:
   The k trees are ordered from 1 to k, with up to s of the k trees
   advertised by RB1 given tree numbers 1 through s, respectively, and
   any remaining trees given numbers in order of priority.

Section 4.5.5:
The bullets in this section say, "forwarded onto
adjacencies in the nickname1 tree" but don't explicitly
say that you shouldn't forward onto the adjacency on which
the packet arrived.

Section 7.2:
Have you made arrangements with IEEE to get these values?  They
normally charge a fee for new Ethertypes.

Nits/editorial comments:

Section 3.5:

   To ensure backward compatible safe operation, when Op-Length is non-
   zero indicating that options are present, the top two bits of the
   first octet of the options area are specified as follows:

               +------+------+----+----+----+----+----+----+
               | CHbH | CItE |          Reserved           |
               +------+------+----+----+----+----+----+----+

               Figure 3.2: Options Area Initial Flags Octet

The diagram shows a 6-bit Reserved field, but the text says "the top
two bits ... are specified".  The text doesn't mention the Reserved
field after this.  Should you specify that Reserved is set to zero
at Ingress, copied on Transit, and ignored on Egress, like you did 
in Section 3.3?  And in that case say "the first byte"?

Section 3.7.2:
   This simplifies end node learning
Missing a period here?  There is also a blank line after this one,
is something else missing?

Section 4.1.2:
   TRILL data frame with the associated VLAN ID and priority placed in
   the Inner.VLAN information.
Should it also copy the C bit?

Section 4.2.4.3:
   in the VLAN for which it is appointed
Missing period?

   30 second
SHOULD BE:
   30 seconds

Section 4.5:
   as describe below
SHOULD BE:
   as described below

Section 4.6.2.4:
   know unicast
SHOULD BE:
   known unicast

Section 4.8.1:
   to not learn
SHOULD BE:
   not to learn

Section 4.9.1:
   ports an
   enabled.
SHOULD BE:
   ports are
   enabled.

Section 4.9.2:
There is a reference to Figure 4.7 but the figure in this section is
labeled 4.5.

Section 6:
   Layer 2 bridging in not inherently secure.
SHOULD BE:
   Layer 2 bridging is not inherently secure.

Section 6.2:
   although this processing
SHOULD BE:
   and this processing

Appendix B:
   on such as link as it
SHOULD BE:
   on such a link as it

---

From: Donald Eastlake <[email-masked]>
Date: Sun, 10 Jan 2010 15:49:47 -0500

Hi Peter,

Thanks for the detailed review. See below,

> Summary: Basically ready, a few minor issues & nits.

Thanks!

> is intended.  Could you add text to clarify?

You are correct in your interpretation. I suggest adding, after the first
sentence of the first paragarph in 4.1.4, the following: "Thus, when a frame
is encapsulated, the original FCS is not included but is discarded."

>   any remaining trees given numbers in order of priority.

The wording is certainly confusing. There are a total of k trees campus
wide. RB1 can advertise a list of trees to calculate which could be a list
of less than k, exactly k, or more than k. s is the number of trees to
calculate advertised by RB1. The numbering of a tree is significant for some
multipathing computations. There is also a priority ordering of all possible
trees, but the list of trees to calculate advertised by RB1 overrides that
priority ordering. So, if is k less than or equal to s, then the first k
trees advertised by RB1 are calculated and numbered 1 through k. If k > s,
then all trees advertised by RB1 are calculated and numbered 1 through s.
And an additional (s - k) tree are calculated and numbered s+1 ... through
k. Which trees are computed is actually explained above the text you
mention. The text you mention is just about tree numbering / ordering.

I suggest replacing the paragraph just before the 4.5.1 section heading with
the following:

"The k trees calculated for a campus are ordered and numbered from 1 to k.
In addition to advertising the number k, RB1 might explicitly advertise a
set of s trees by providing a list of s nicknames that are the root of the
trees.

  -   If s == k, then the trees are numbered in the order of RB1 advertises
them.

  -   If s == 0, then the trees are numbered in order of decreasing
priority. For example, if RB1 advertises only that k=2, then the highest
priority tree is number 1 and the 2nd highest priority tree is number 2.

   -  If s < k, then those advertised by RB1 are numbered from 1 in the
order advertised. Then the remainder are chosen by priority order from among
the remaining possible trees with the numbering continuing. For example, if
RB1 advertises k=4, advertises {Tx, Ty} as the nicknames of the root of the
trees, and the campus wide priority ordering of trees in decreasing order is
Ty > Ta > Tc > Tb > Tx, the numbering will be as follows: Tx is 1 and Ty is
2 since that is the order they are advertised in by RB1. Then Ta is 3  and
Tc is 4 because they are the highest priority trees that have not already
been numbered."

> say that you shouldn't forward onto the adjacency on which

Yes. Since it seems like it would be a bit verbose to add this qualification
in the many cases where there are references to adjacencies in this bulleted
list, I suggest the following. Replace the ":" at the end of the first
sentence in 4.5.5 with a period and add the following sentence after it:
"References to adjacencies below do not include the adjacency on which a
frame was received."

> normally charge a fee for new Ethertypes.

We are presently working on the applications, but they have not been
submitted. Our understanding is that it is the policy of the IEEE RAC
(Registration Authority Committee) to, on a case-by-case basis, grant code
points for standards use to standards development organizations free of
charge. For one type of allocation, a standards use multicast address, they
even have a standard form to use for this purpose:
http://standards.ieee.org/regauth/registry_stdgroupmac.html

> Nits/editorial comments:

On the items below, unless I comment otherwise, I agree.

>   first octet of the options area are specified as follows:

The idea is to push as much as possible off to a different draft and only
put in the minimum hooks in the main protocol document. That different draft
is now a working group document at
http://tools.ietf.org/html/draft-ietf-trill-rbridge-options-00

I suggest that, at the beginning of the third paragraph below the above
figure, the first sentence be changed so that instead of starting "Options
will be further specified in other documents...", it would start "Options,
including the meaning of the bits labeled as Reserved in Figure 3.2, will be
further specified in other documents...".

> is something else missing?

Yes, there is a missing period. But I've checked and nothing else is
missing.

> Should it also copy the C bit?

No, as specified above in the draft, the C bit is always zero in TRILL.

>   in the VLAN for which it is appointed

Yes.

>   not to learn

I don't actually see anything wrong with the current wording but I'm willing
to change it.

> There is a reference to Figure 4.7 but the figure in this section is

Good catch, thanks.

>   Layer 2 bridging is not inherently secure.

I think the current wording is better.

>   on such a link as it

Thanks!
Donald

=============================
Donald E. Eastlake 3rd   +1-508-634-2066 (home)
155 Beaver Street
Milford, MA 01757 USA
[email-masked]

---

From: "McCann Peter-A001034" <[email-masked]>
Date: Mon, 11 Jan 2010 13:37:13 -0500

Hi, Donald,

The changes you propose will address my concerns.  Thanks.

-Pete

Donald Eastlake wrote:
> On Thu, Jan 7, 2010 at 3:13 PM, McCann Peter-A001034
> <[email-masked]> wrote: 
> 
> 
> 	I have been selected as the General Area Review Team (Gen-ART)
> 	reviewer for this draft (for background on Gen-ART, please see
> You are correct in your interpretation. I suggest adding, after the
> first sentence of the first paragarph in 4.1.4, the following: "Thus,
> when a frame is encapsulated, the original FCS is not included but is
> discarded."   
> 
> 
> 	Section 4.5:
> The wording is certainly confusing. There are a total of k trees
> campus wide. RB1 can advertise a list of trees to calculate which
> could be a list of less than k, exactly k, or more than k. s is the
> number of trees to calculate advertised by RB1. The numbering of a
> tree is significant for some multipathing computations. There is also
> a priority ordering of all possible trees, but the list of trees to
> calculate advertised by RB1 overrides that priority ordering. So, if
> is k less than or equal to s, then the first k trees advertised by
> RB1 are calculated and numbered 1 through k. If k > s, then all trees
> advertised by RB1 are calculated and numbered 1 through s. And an
> additional (s - k) tree are calculated and numbered s+1 ... through
> I suggest replacing the paragraph just before the 4.5.1 section
> heading with the following: 
> 
> "The k trees calculated for a campus are ordered and numbered from 1
> to k. In addition to advertising the number k, RB1 might explicitly
> advertise a set of s trees by providing a list of s nicknames that
> are the root of the trees.   
> 
>   -   If s == k, then the trees are numbered in the order of RB1
> advertises them. 
> 
> priority. For example, if RB1 advertises only that k=2, then the
> highest priority tree is number 1 and the 2nd highest priority tree
> is number 2.   
> 
> 
>    -  If s < k, then those advertised by RB1 are numbered from 1 in
> the order advertised. Then the remainder are chosen by priority order
> from among the remaining possible trees with the numbering
> continuing. For example, if RB1 advertises k=4, advertises {Tx, Ty}
> as the nicknames of the root of the trees, and the campus wide
> priority ordering of trees in decreasing order is Ty > Ta > Tc > Tb >
> Tx, the numbering will be as follows: Tx is 1 and Ty is 2 since that
> is the order they are advertised in by RB1. Then Ta is 3  and Tc is 4
> because they are the highest priority trees that have not already
> been numbered."         
> 
> 
> 	Section 4.5.5:
> Yes. Since it seems like it would be a bit verbose to add this
> qualification in the many cases where there are references to
> adjacencies in this bulleted list, I suggest the following. Replace
> the ":" at the end of the first sentence in 4.5.5 with a period and
> add the following sentence after it: "References to adjacencies below
> do not include the adjacency on which a frame was received."     
> 
> 
> 	Section 7.2:
> (Registration Authority Committee) to, on a case-by-case basis, grant
> code points for standards use to standards development organizations
> free of charge. For one type of allocation, a standards use multicast
> address, they even have a standard form to use for this purpose:
> 	  To ensure backward compatible safe operation, when Op-Length
is
> 	  non- zero indicating that options are present, the top two
bits of
> 	  the first octet of the options area are specified as follows:
> 
> The idea is to push as much as possible off to a different draft and
> only put in the minimum hooks in the main protocol document. That
> different draft is now a working group document at
> I suggest that, at the beginning of the third paragraph below the
> above figure, the first sentence be changed so that instead of
> starting "Options will be further specified in other documents...",
> it would start "Options, including the meaning of the bits labeled as
> Reserved in Figure 3.2, will be further specified in other
> documents...".     
> 
> 
> 	Section 3.7.2:
> I don't actually see anything wrong with the current wording but I'm
> willing to change it. 
> 
> 
> 	Section 4.9.1:
> 	  ports an
> 	  enabled.
> 	SHOULD BE:
> 	  ports are
> 	  enabled.
> 
> 	Section 4.9.2: