Subject: [RTG-DIR] RtgDir review: draft-ietf-trill-rbridge-bfd-05.txt

From: "John G. Scudder" <[email-masked]>
Date: Wed, 6 Jun 2012 16:29:09 -0400

Hello,

I have been selected as the Routing Directorate reviewer for this draft. The Routing Directorate seeks to review all routing or routing-related drafts as they pass through IETF last call and IESG review, and sometimes on special request. The purpose of the review is to provide assistance to the Routing ADs. For more information about the Routing Directorate, please see http://www.ietf.org/iesg/directorate/routing.html

Although these comments are primarily for the use of the Routing ADs, it would be helpful if you could consider them along with any other IETF Last Call comments that you receive, and strive to resolve them through discussion or by updating the draft.

Document: draft-ietf-trill-rbridge-bfd-05.txt with attention given to -06 as well.
Reviewer: John Scudder 
Review Date: June 6, 2012 
IETF LC End Date: June 6, 2012 
Intended Status: Proposed Standard

Summary:

I have some minor concerns about this document that I think should be resolved before publication.

Comments:

None.

Major Issues:

No major issues found (although see minor issue 5 below).

Minor Issues:

1. It is not clear why demand mode was explicitly excluded. TRILL would actually seem to be a reasonable fit for demand mode.

2. "there will be only a single TRILL BFD
   Control session between two RBridges over a given interface visible
   to TRILL."

I was not able to unambiguously parse this clause. I suggest rewriting it, possibly with reference to a pair of RBridges RB1 and RB2 and their interfaces RB1_Int1 and RB2_Int1. Or something like that.

3. "the entire TRILL BFD Echo protocol specific
   data area is considered opaque and left to the discretion of the
   originating RBridge.  Nevertheless, it is RECOMMENDED that this data
   include information by which the originating RBridge can authenticate
   the returned BFD Echo frame and confirm the neighbor that echoed the
   frame back."

The "RECOMMENDED" above seems like an abuse of RFC 2119 terminology. As a reminder, RECOMMENDED is a synonym for SHOULD which in turn basically means "MUST unless you have a good reason". So the RECOMMENDED contradicts "left to the discretion", and furthermore it's kind of pointless to normatively-in-all-caps issue what amounts to very general advice. One heuristic I sometimes use: can I write a conformance test for the text in question? If not, it is very likely not really normative. 

Changing to a lower-case "recommended" would fix this.

4. In two places text like the following appears:

   Is the M-bit in the TRILL Header non-zero?  If so, discard the frame.
   TRILL support of multi-destination BFD Echo is beyond the scope of
   this document.

If multi-destination doesn't make sense in the context of TRILL, it is fine to exclude it by enforcing that the M-bit be zero. However, I normally parse "beyond the scope of this document" to mean something like "we may do it in the future but haven't worked it out yet". By forcing the M-bit to be zero, you're placing multi-destination *in* scope, inasmuch as you are actively forbidding it from being supported. 

Presumably the sentence about "beyond the scope" was meant to justify this decision. Perhaps the comment about "scope" should be dropped and a different justifying sentence used. (Or the sentence omitted altogether.)

5. Presumably the Security Area reviewer may raise this, but the Security Considerations section seems to be misused. My understanding (confirmed by referring to RFC 3552 Section 5) is that the Security Considerations section is intended to be an analysis of issues that arise from the operation of the protocol defined in the rest of the document, including but not limited to the security features of that protocol. 

This document's Security Considerations section instead specifies how authentication is to be done, though it doesn't provide an analysis of it or of any broader issues. I presume the section should be retitled (e.g., to "Authentication") and a proper Security Considerations section added.

(This would probably be a "major issue" if I were the Security Area reviewer, but I'm not.)

Nits:

1. "The BFD (Bi-directional Forwarding Detection [RFC5880]) protocol provides a low-overhead, short- duration detection of failures"

The use of the term "short-duration" seems broken, to the extent that I don't know what you really mean. Maybe you mean that failures are expected to be detected quickly? In any case, I suggest you rewrite this, maybe with more words, to say what you mean.

2. "   This document describes a TRILL encapsulation for BFD packets for
   networks that do not use IP addressing or for ones where it is not
   desireable."

You misspelled "desirable". But also, what is the referent of "it"? Use of IP addressing? As written, I parse that clause as "networks that use IP addressing but in an undesirable way" which is probably not what you meant. I'd actually suggest just dropping the "or" clause.

3. "When running BFD
   over TRILL both Single Hop as well as in Multi Hop sessions are
   supported."

This should either be "... both single-hop and multihop ..." or "When running BFD over TRILL single-hop as well as multihop sessions ...".

Also, are the capitalizations really needed? As far as I know neither term is proper. I have flagged other capitalization errors where noticed, below.

4. "BFD over TRILL supports the Echo function, however this
   can be used for only Single hop sessions."

Should be "this can only be used for single-hop sessions."

5. "For Multi Hop
   sessions the Hop count check can be disabled if the MH flag is one."

Should be "For multihop sessions the hop count check..."

6. "Note that TRILL BFD provides OAM facilities for the TRILL Data plane"

Should be "TRILL data plane" (or less likely, "TRILL Data Plane" if it's really proper).

7. "TRILL BFD Echo frames SHOULD be sent on a link only if the following	
 	   points are met." (Draft -06)

Should be "following conditions".

8. "TRILL BFD frames over one hop for such
   purposes SHOULD be sent with priority 7."

Suggest "frame priority 7" for clarity.

9. "although work is being done in the Area [MultiBFD]."

Should be "in this area" (or "in the area").

10.    "Consistent with TRILL's goal of being able to operate with minimum
   configuration, the default for BFD authentication between neighbor
   RBridges is based on that state of IS-IS shared secret authentication
   for Hello between those RBridges. However, if such BFD authentication
   is configured then its configuration is independent of that for IS-IS
   security."

a. Should be "... based on the state of IS-IS ..."
b. Suggest adding "as detailed below" to make it be: "based on the state of IS-IS shared secret authentication for Hello between those RBridges, as detailed below."
c. It seems what you are really trying to say is that the default authentication for BFD is inherited from the IS-IS authentication configuration as given below, but that the default can be overridden. The way you have said it is pretty hard to understand. Suggest rewording it more as stated here.

11. "       HMAC-SHA256 ( ( "TRILL BFD Control" | originatorMAC ),
                     IS-IS-shared-key )"

Is it common to use the pipe character for concatenation? I'm not familiar with it for such, and found it confusing.

12. "   Echo frames to be returned by that neighbor SHOULD be authenticated
   and such authenticate SHOULD use different keying material from other
   types of authentication."

Should be: "... and such authentication SHOULD ..."

13. "IANA is request"

Should be: "IANA is requested"

---

From: Donald Eastlake <[email-masked]>
Date: Mon, 11 Jun 2012 16:48:14 -0400

Hi John,

Thanks for your thorough review. See below:

On Wed, Jun 6, 2012 at 4:29 PM, John G. Scudder <[email-masked]> wrote:
> Hello,
>
> I have been selected as the Routing Directorate reviewer for this
> draft. The Routing Directorate seeks to review all routing or
> routing-related drafts as they pass through IETF last call and IESG
> review, and sometimes on special request. The purpose of the review
> is to provide assistance to the Routing ADs. For more information
> about the Routing Directorate, please see
> http://www.ietf.org/iesg/directorate/routing.html
>
> Although these comments are primarily for the use of the Routing
> ADs, it would be helpful if you could consider them along with any
> other IETF Last Call comments that you receive, and strive to
> resolve them through discussion or by updating the draft.
>
> 1. It is not clear why demand mode was explicitly excluded. TRILL
> would actually seem to be a reasonable fit for demand mode.

Demand modes generally assumes some other way of testing
connectivity. While you can do that with IS-IS Hellos, they are
(modulo deliberate jitter to avoid synchonrization) limited to no more
often than once a second. However, if BFD Echo is supported, it would
be reasonable to test with BFD Echo and then use Demand mode BFD with
that. [RF5880] also suggests that Demand mode may be useful if there
are a very large number of stations attached to a multi-access link to
avoid an N**2 effect getting too large. So I'm thinking that you are
correct and the use of Demand should be allowed.

> I was not able to unambiguously parse this clause. I suggest
>  rewriting it, possibly with reference to a pair of RBridges RB1 and
>  RB2 and their interfaces RB1_Int1 and RB2_Int1. Or something like
>  that.

Peraps it should say something closer to "There will be no more than
one TRILL BFD Control session from any RBridge RB1 to RBridge RB2 for
each RB1 TRILL port."

>    originating RBridge. Nevertheless, it is RECOMMENDED that this
>    data include information by which the originating RBridge can
>    authenticate the returned BFD Echo frame and confirm the neighbor
>    that echoed the frame back."
>
> The "RECOMMENDED" above seems like an abuse of RFC 2119
> terminology. As a reminder, RECOMMENDED is a synonym for SHOULD
> which in turn basically means "MUST unless you have a good
> reason". So the RECOMMENDED contradicts "left to the discretion",
> and furthermore it's kind of pointless to normatively-in-all-caps
> issue what amounts to very general advice. One heuristic I sometimes
> use: can I write a conformance test for the text in question? If
> not, it is very likely not really normative.
>

I agree that in this case the "RECOMMENDED"s should be lower cased.

>   Is the M-bit in the TRILL Header non-zero? If so, discard the
>   frame.  TRILL support of multi-destination BFD Echo is beyond the
>   scope of this document.
>
> If multi-destination doesn't make sense in the context of TRILL, it
> is fine to exclude it by enforcing that the M-bit be zero. However,
> I normally parse "beyond the scope of this document" to mean
> something like "we may do it in the future but haven't worked it out
> yet". By forcing the M-bit to be zero, you're placing
> multi-destination *in* scope, inasmuch as you are actively
> forbidding it from being supported.
>
> Presumably the sentence about "beyond the scope" was meant to
> justify this decision. Perhaps the comment about "scope" should be
> dropped and a different justifying sentence used. (Or the sentence
> omitted altogether.)

Multi-destination makes sense for TRILL. It is just not yet
standardized, as far as I know, for BFD. Perhaps the "beyond the
scope" verbage should be removed and a note added somewhere such as
"(Multi-destination BFD is a work in progress [...].)"

> 5. Presumably the Security Area reviewer may raise this, but the
> Security Considerations section seems to be misused. My
> understanding (confirmed by referring to RFC 3552 Section 5) is that
> the Security Considerations section is intended to be an analysis of
> issues that arise from the operation of the protocol defined in the
> rest of the document, including but not limited to the security
> features of that protocol.
>
> This document's Security Considerations section instead specifies
> how authentication is to be done, though it doesn't provide an
> analysis of it or of any broader issues. I presume the section
> should be retitled (e.g., to "Authentication") and a proper Security
> Considerations section added.
>

As far as I can tell (and I'm a member of the Security Directorate),
the Security Area considers this a non-issue. However, I've got no
problem with moving this material to a new "Default Authentication"
section or the like.

> Nits:
>
> 1. "The BFD (Bi-directional Forwarding Detection [RFC5880]) protocol
> provides a low-overhead, short- duration detection of failures"
>
> The use of the term "short-duration" seems broken, to the extent
> that I don't know what you really mean. Maybe you mean that failures
> are expected to be detected quickly? In any case, I suggest you
> rewrite this, maybe with more words, to say what you mean.

How about: "The BFD (Bi-directional Forwarding Detection [RFC5880])
protocol provides a low-overhead method for the rapid detection of
connectivity failures"

> 2. "  This document describes a TRILL encapsulation for BFD packets
>    for networks that do not use IP addressing or for ones where it
>    is not desireable."
>
> You misspelled "desirable". But also, what is the referent of "it"?
> Use of IP addressing? As written, I parse that clause as "networks
> that use IP addressing but in an undesirable way" which is probably
> not what you meant. I'd actually suggest just dropping the "or"
> clause.

I'm fine with just dropping the "or" clause. How about "This document
describes a TRILL encapsulation for BFD packets for networks that do
not use IP addressing and forward based on the TRILL Header."

> 3. "When running BFD over TRILL both Single Hop as well as in Multi
>    Hop sessions are supported."
>
> This should either be "... both single-hop and multihop ..." or
> "When running BFD over TRILL single-hop as well as multihop sessions
> ...".
>
> Also, are the capitalizations really needed? As far as I know
> neither term is proper. I have flagged other capitalization errors
> where noticed, below.

I think going with "both single-hop and multihop" would be best.

> Should be "this can only be used for single-hop sessions."

OK.

> Should be "For multihop sessions the hop count check..."

OK. Perhaps, "For multihop sessions the hop count check can be
disabled by setting the MH flag to one."

> Should be "TRILL data plane" (or less likely, "TRILL Data Plane" if

OK. I think lower case is better.

> 7. "TRILL BFD Echo frames SHOULD be sent on a link only if the
>    following points are met." (Draft -06)
>

OK.

> Suggest "frame priority 7" for clarity.

I think that is a good suggestion.

> Should be "in this area" (or "in the area").

OK. I think "this area" is best.

> 10. "Consistent with TRILL's goal of being able to operate with
>      minimum configuration, the default for BFD authentication
>      between neighbor RBridges is based on that state of IS-IS
>      shared secret authentication for Hello between those
>      RBridges. However, if such BFD authentication is configured
>      then its configuration is independent of that for IS-IS
>      security."

> a. Should be "... based on the state of IS-IS ..."

OK.

> b. Suggest adding "as detailed below" to make it be: "based on the
> state of IS-IS shared secret authentication for Hello between those
> RBridges, as detailed below."

OK.

> c. It seems what you are really trying to say is that the default
> authentication for BFD is inherited from the IS-IS authentication
> configuration as given below, but that the default can be
> overridden. The way you have said it is pretty hard to
> understand. Suggest rewording it more as stated here.

OK. How about:

   The BFD authentication keys between neighbor RBridges by default
   are derived from the IS-IS shared secret authentication keys for
   Hellos between those RBridges as detailed below. This is consistent
   with TRILL's goal of being able to operate with minimum
   configuration.  However, such BFD authentication keys MAY be
   configured to some other value.

> Is it common to use the pipe character for concatenation? I'm not
> familiar with it for such, and found it confusing.

Yes, vertical bar is a standard character to indicate byte string
concatenation. I have no problem, in the two instances, with adding
"where "|" indicates concatenation" or the like to the text.

> 12. "  Echo frames to be returned by that neighbor SHOULD be
>     authenticated and such authenticate SHOULD use different keying
>     material from other types of authentication."
>

OK.

> Should be: "IANA is requested"

OK.

Thanks,
Donald
=============================
 Donald E. Eastlake 3rd   +1-508-333-2270 (cell)
 155 Beaver Street, Milford, MA 01757 USA
 [email-masked]

---

From: "John G. Scudder" <[email-masked]>
Date: Mon, 11 Jun 2012 17:02:45 -0400

Hi Donald,

Comments below only where I had something to say other than "OK". You can take that as read otherwise.

On Jun 11, 2012, at 4:48 PM, Donald Eastlake wrote:
...
>> Minor Issues:
...
> "(Multi-destination BFD is a work in progress [...].)"

By requiring the M-bit to be zero you effectively preclude the later introduction of multi-destination, do you not?

OK.

(As an aside, if the Security Directorate considers this a non-issue, maybe it's time to revise RFC 3552? I say this as an occasional author of Security Considerations sections and not with regard to your draft.)

--John

---

From: Eric Gray <[email-masked]>
Date: Mon, 11 Jun 2012 21:15:59 -0400

Donald,

I may regret weighing in on this, but I strongly agree with
John's comment on the Security Considerations section below.

The Security Considerations section needs an introduction 
that - at the very least - explains why the section only 
addresses authentication.  In fact, the section only talks
about what authentication is to be used, without providing
and indication of why.

I personally doubt that the Security Directorate feels - 
as you say - that this is not significant.  The section
should describe the security concerns associated with the
protocol and/or encapsulations associated with this draft
- either directly, or by reference.

At the moment the section does not do this, even by giving
a pointer to someplace else where some analysis has already 
been done.

And this is not difficult to fix.  Minimally, the authors
can point to Security Considerations in RFC 5881 (which,
in turn, points to RFC 5880).

RFC 5880, provides a very good analysis, IMO, and it makes 
sense for this analysis to be re-used by reference in this
draft.

--
Eric

-----Original Message-----
From: [email-masked] [mailto:[email-masked]] On Behalf Of John G. Scudder
Sent: Monday, June 11, 2012 5:03 PM
To: Donald Eastlake
Cc: Vishwas Manral; [email-masked]; [email-masked]; [email-masked]
Subject: Re: [RTG-DIR] RtgDir review: draft-ietf-trill-rbridge-bfd-05.txt

Hi Donald,

...

OK.

--John

---

From: Donald Eastlake <[email-masked]>
Date: Mon, 11 Jun 2012 21:45:43 -0400

Hi Eri,

Thanks for your comments. I agree that at least a reference to RFC
5881 should be added and probably some additional material on security
considerations.

I think you misunderstood my response to John Scudder where perhaps I
did not express myself clearly enough. I was just trying to say that,
as far as I can tell, the Security Directorate would not be concerned
that the material currently in the Security Considerations is in that
Section. I did not mean my comment to apply to any possible omission
from the Security Considerations section.

Thanks,
Donald

---

From: "John G. Scudder" <[email-masked]>
Date: Tue, 12 Jun 2012 11:43:14 -0400

> from the Security Considerations section.

Thanks for the clarification. I had also not understood that (as might have been evident from my followup comment). 

--John

---

From: "John G. Scudder" <[email-masked]>
Date: Tue, 12 Jun 2012 11:57:19 -0400

After some offline discussion I will withdraw my gripe about M-bit processing. 

BTW one more nit:

In one place you refer to 'M-bit' and in another, 'M bit'. Please make this consistent. Thanks.

--John

---

From: Donald Eastlake <[email-masked]>
Date: Tue, 12 Jun 2012 12:02:30 -0400

Hi John,

OK.

Thanks,
Donald