[Isis-wg] AD review of draft-ietf-isis-l2bundles-03

Alia Atlas <akatlas@gmail.com> Thu, 06 April 2017 20:57 UTC

Return-Path: <akatlas@gmail.com>
X-Original-To: isis-wg@ietfa.amsl.com
Delivered-To: isis-wg@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 8BE56129634; Thu, 6 Apr 2017 13:57:14 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.999
X-Spam-Level:
X-Spam-Status: No, score=-1.999 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, FREEMAIL_FROM=0.001, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_NONE=-0.0001, SPF_PASS=-0.001] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (2048-bit key) header.d=gmail.com
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id Y-E_Spc_ACrC; Thu, 6 Apr 2017 13:57:11 -0700 (PDT)
Received: from mail-wr0-x230.google.com (mail-wr0-x230.google.com [IPv6:2a00:1450:400c:c0c::230]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 374421294FB; Thu, 6 Apr 2017 13:57:11 -0700 (PDT)
Received: by mail-wr0-x230.google.com with SMTP id t20so80642367wra.1; Thu, 06 Apr 2017 13:57:11 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:from:date:message-id:subject:to; bh=k5pkL696afN2xHIFA7g0h4b2gynD6hSXwNmIWDTnHg0=; b=Hyw2BrUeIx3bRE+qMe8AlqJluqGDgaVMTZ4//vhMFUp3MxXqxtP0fhDqqLk7oiUaj8 6ghNOtHoS7Dt5lVcI6Tz4GfNqTruiH6McczGd0FJJtOyBRGiXaGFdV3f4+WO2MAkEqfs YYvsexalyQ1bfKo7cm9hgqdAX3zobO0WXe77JQ9JcTc7RoXP+JrAtpMfvy3Lox161/IP hl1dg8AaGncrXCDGt5n9k5K4rVrYUObs+yXoo4JXiWkfZUj0jOesr9fZGGaPEwNHYhr2 iCbuCaRbj3XbZ+Ex9bLswpHywhugp5tjutEiWxL3pKAJaxcICpeQglHWfI/uWxB+ecXU EBYw==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:from:date:message-id:subject:to; bh=k5pkL696afN2xHIFA7g0h4b2gynD6hSXwNmIWDTnHg0=; b=JPklyxNqPHxLKiAd+iU3WBkV35MbWn1SPn8cf7Q6RIKZzhWPgtC2U/zkv5qnDnIWWI Upb/jnra22DEn0LTja7s96nYJiSSqDa/hGmS/znYIon5hGCyVPCv7GVI8P/BQOioXUaU 07rrMQpZjWMpXHptXEmlalhhsTUVVprwtFly383fpmNWFh+KDWpua+v8+onDBWAA6WWL BHqsxrjZXtST9YcUlVHEHOHNp8s8485sjE/rS6dtzmcwPKLaBRr4dnObiSQxDCUKKJdK Yf1jUZmORlJDkISTEUPqcwe0BtBZ5gDV2roXuMHvORj2z+AVlK1mX33lRea40Xx6Z8dz gaAQ==
X-Gm-Message-State: AFeK/H1r9P/D9+KEkGRStGFHOmSW3sDfZjQu0wSlEm8NW8PYQ5Kq5Dg1+ieprXNk11+G7wloGIcedA1BuFCkWw==
X-Received: by 10.223.166.170 with SMTP id t39mr28484326wrc.44.1491512229255; Thu, 06 Apr 2017 13:57:09 -0700 (PDT)
MIME-Version: 1.0
Received: by 10.223.135.17 with HTTP; Thu, 6 Apr 2017 13:57:08 -0700 (PDT)
From: Alia Atlas <akatlas@gmail.com>
Date: Thu, 6 Apr 2017 16:57:08 -0400
Message-ID: <CAG4d1rcv9mwE78VjFTcnWK_XcM-U-JujNXrkCxhaRNV+j9YOLA@mail.gmail.com>
To: "isis-wg@ietf.org" <isis-wg@ietf.org>, draft-ietf-isis-l2bundles@ietf.org
Content-Type: multipart/alternative; boundary=001a114008989a60af054c85c083
Archived-At: <https://mailarchive.ietf.org/arch/msg/isis-wg/HLUHKuOEI9okN1f8SkH-a0-lc4E>
Subject: [Isis-wg] AD review of draft-ietf-isis-l2bundles-03
X-BeenThere: isis-wg@ietf.org
X-Mailman-Version: 2.1.22
Precedence: list
List-Id: IETF IS-IS working group <isis-wg.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/isis-wg>, <mailto:isis-wg-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/isis-wg/>
List-Post: <mailto:isis-wg@ietf.org>
List-Help: <mailto:isis-wg-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/isis-wg>, <mailto:isis-wg-request@ietf.org?subject=subscribe>
X-List-Received-Date: Thu, 06 Apr 2017 20:57:15 -0000

As is customary, I have done my AD review of draft-ietf-isis-l2bundles-03.
  First, I would like to thank the authors - Les, Ahmed, Clarence, Stefano,
Mohan and Ebben - for their work on this document.

Based upon the urgency expressed at IETF 98 for this document, of course, I
would very much like to get it moving through the IETF process as rapidly
as possible.  Regrettably, the document has 6 authors which is above the
limit.  I would welcome either having a subset selected as the editor(s) or
reducing the authorship to no more than 5.   It is most irksome to have to
serve as the enforcement arm in this matter with authors and WG Chairs who
have long experience with the IETF publication process and are surely aware
and able to handle this issue without waiting for AD enforcement.

Assuming that the author limit issue is rapidly resolved, I do also have a
number of technical concerns and minor improvements.  None of these are
expected to be blocking - but they do need to be resolved.    I am
targeting this for the IESG telechat on April 27 which means that it needs
to ideally enter into IETF Last Call tomorrow to allow adequate time for
the IESG and directorate review.   Knowing the speed with which some of the
authors respond, I am confident that this is possible.

As discussed at IETF 98, I will call attention to this draft and that it
will be entering IETF Last Call to WGs (rtgwg, ospf, idr) that may be
interested.

Major:

a) The assumption that bandwidth is only a shared attribute (and similarly
weight in the new Adj-SID sub-TLV) is very concerning.  While I believe
that standardized LACP only supports the same link speeds, I know that
there are some routers and switches from different vendors that do allow
different speeds.  This functionality is something that could meet some of
the needs for RFC 7226 where the composite-links are assumed to have
different speeds.   A solution to this for the bandwidth, for instance,
could be to have a section describing how to take a previously defined
sub-TLV and then modify it into a general sub-TLV that can apply to the L2
bundle members.  The draft even has an existing example of doing this for
the Adj-SID sub-TLV.

b)  draft-ietf-isis-segment-routing-extensions needs to be a Normative
reference.


Minor:

1) In Sec 2, the definition of the TLV would be greatly improved with a
figure that shows the actual layout.   Take a look at RFC5308 for a decent
example.  It takes careful reading to figure out what layouts are
possible.  I think that it is something like:

 0                   1                   2                   3
 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
| Type 25?      |  Length       |   Neighbor System ID          |  Parent L3
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
|       Neighbor System ID continued                            |  Neighbor
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
|  Pseudonode   |P|R|R|R|R|R|R|R| if P, sub-TLV 4, 6, or 12     |
Descriptor
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
~  if P, sub-TLV 4, 6, or 12                                     ~
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

etc.   For instance, I can't tell whether the "L2 Attribute Parent
Descriptor"
is more than just a length octet plus the nested contents.  I assume that
is all,
but it would be much better to see it written out.  If doing a layout
figure is too
painful - then a more detailed (think c-structure with bit-fields)
description would
help.  It shouldn't take cross-referencing different sections to figure out
the bits.

2) In Sec 2, it states " NOTE: An L2 Bundle Member Descriptor is a Link
Local Identifier as defined in [RFC5307]."
but RFC5307 merely uses the Link Local Identifier and specifies that it is
defined in rfc4202 Section-2.1.   What about instead
"NOTE: An L2 Bundle Descriptor is a Link Local Identifier as defined in
Section 2.1 of [RFC4202] and used as a value in
sub-TLV type=4 defined in [RFC5307]."   That makes it clearer.  I feel
there is some lack of clarity because what is defined in
RFC5307 and - without a clear bit layout - it takes some assumptions
(looking at the length computation) to be clear that what
is included for the L2 Bundle Member Descriptor isn't a sub-TLV type=4, but
rather just the 4 octet local link identifier."  Frankly,
I'm not positive why it wasn't deemed useful to also include the Link
Remote Identifier for each L2 member; clearly correlating
will be non-trivial, but understanding that the draft is implemented, I am
merely requested a bit of explanatory text also in the draft.

3) In Sec 2.2: "If multiple copies of a given  sub-TLV are present both
MUST be ignored."   Substitute "all copies" instead of "both".

4) In Sec 2.2: "All existing sub-TLVs defined in the IANA Sub-TLVs for TLVs
22, 23,   141, 222, and 223 registry are in the category of shared attribute
   sub-TLVs unless otherwise specified in this document."   This is not a
useful statement for going forward as an RFC.   Do you mean
"At the time this document is published, all sub-TVLs are shared attribute
sub-TLVs unless otherwise specified in this document.  For future sub-TLVs
to not be considered shared attribute sub-TLVs, this MUST be explicitly
defined when those sub-TLVs are defined?"  Or do you intend to add a field
to the IANA registry indicating whether it is not a shared attribute - so
that the question is asked every time a new sub-TLV is added?  Or do you
mean
that there will be no need for sub-TLVs that aren't shared attribute except
as in this document (reasoning please)?    This document needs to provide
sufficient information to be sustainable.

5) In Sec 3.1 and Sec 3.2: It looks like the flags definitions are intended
to be the same as in [draft-ietf-isis-segment-routing-extensions, Sec
2.1.1].  If so, then please
say so and simply refer to  [draft-ietf-isis-segment-routing-extensions,
Sec 2.1.1] instead of redefining them!

6) In Sec 3.1 and Sec 3.2:  For " L2 Bundle Member Adj-SID Descriptors." -
first it looks like the details are duplicating definition in [SR].  Again
a direct reference with - at most - quoting is better.   Second, please add
text on how to associate each of the Adj-SID Descriptors with the L2 Bundle
members.  YES - I realize that it is probably assuming a consistently
ordered list and 1-to-1 mapping, but there is no text that says so.


Nits:

1) In Sec 1: "In order to do so link attribute information about individual
bundle members is required - but currently IS-IS only supports advertising
link
   attributes for the Layer 3 interfaces on which it operates."
   Rephrase "currently" to "without the TLV defined in this document" or
something equivalent.   Time to rephrase for it being an RFC.