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

[Donald Eastlake <[e-mail]>]

Hi John,

Thanks for your thorough review. See below:

On Wed, Jun 6, 2012 at 4:29 PM, John G. Scudder <[e-mail]> 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.
>
> 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.

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.

> 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.

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."

> 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.

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

> 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.)

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.
>
> (This would probably be a "major issue" if I were the Security Area
> reviewer, but I'm not.)

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.

> 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."

OK.

> 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..."

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

> 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).

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)
>
> Should be "following conditions".

OK.

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

I think that is a good suggestion.

> 9. "although work is being done in the Area [MultiBFD]."
>
> 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.

> 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.

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."
>
> Should be: "... and such authentication SHOULD ..."

OK.

> 13. "IANA is request"
>
> Should be: "IANA is requested"

OK.

Thanks,
Donald
=============================
 Donald E. Eastlake 3rd   [phone] (cell)
 155 Beaver Street, Milford, MA 01757 USA
 [e-mail]