Last Call Review of draft-ietf-opsawg-capwap-alt-tunnel-10
review-ietf-opsawg-capwap-alt-tunnel-10-genart-lc-kyzivat-2017-12-11-00

Request Review of draft-ietf-opsawg-capwap-alt-tunnel
Requested rev. no specific revision (document currently at 11)
Type Last Call Review
Team General Area Review Team (Gen-ART) (genart)
Deadline 2017-12-13
Requested 2017-11-29
Other Reviews Genart Last Call review of -08 by Paul Kyzivat (diff)
Genart Telechat review of -08 by Paul Kyzivat (diff)
Secdir Last Call review of -08 by Catherine Meadows (diff)
Opsdir Last Call review of -08 by Will LIU (diff)
Secdir Early review of -09 by Catherine Meadows (diff)
Genart Telechat review of -11 by Paul Kyzivat
Review State Completed
Reviewer Paul Kyzivat
Review review-ietf-opsawg-capwap-alt-tunnel-10-genart-lc-kyzivat-2017-12-11
Posted at https://mailarchive.ietf.org/arch/msg/gen-art/7ylS0ZIAm9jEbQ27WJqlJvSQaGw
Reviewed rev. 10 (document currently at 11)
Review result Ready with Issues
Draft last updated 2017-12-11
Review completed: 2017-12-11

Review
review-ietf-opsawg-capwap-alt-tunnel-10-genart-lc-kyzivat-2017-12-11

I am the assigned Gen-ART reviewer for this draft. The General Area 
Review Team (Gen-ART) reviews all IETF documents being processed by the 
IESG for the IETF Chair. Please treat these comments just like any other 
last call comments. For more information, please see the FAQ at 
<​http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>.

Document: draft-ietf-opsawg-capwap-alt-tunnel-10
Reviewer: Paul Kyzivat
Review Date: 2017-12-11
IETF LC End Date: 2017-12-13
IESG Telechat date: TBD

Summary:

This draft is on the right track but has open issues, described in the 
review.

(Thanks for fixing most of the issues I raised in the previous round.)

Issues:

Major: 0
Minor: 7
Nits:  1

(1) MINOR:

In Section 1.3, if this document is intended to serve as a *historical* 
reference, then why isn't then intended status "Historic" rather than 
"Experimental"?

(2) MINOR:

Section 3 contains:

    Since AC can configure a WTP with more than one AR available for the
    WTP to establish the data tunnel(s) for user traffic, it may be
    useful for the WTP to communicate the selected AR.  To enable this,
    the IEEE 802.11 WLAN Configuration Response may contain the AR list
    element containing the selected AR.

But "IEEE 802.11 WLAN Configuration Response" is not defined in this 
version of the document. Seems like this may be a dangling reference 
from a prior version.

(3) MINOR:

In Section 3.1, Figure 6 shows Tunnel-Type1 occupying the first 16 bits 
of a 32 bit value, and Tunnel-Type (2..N) all occupying the 2nd 16 bits 
of that value. That doesn't work for N>2. I *presume* that the intent is 
that this be an array of 16 bit values in network order starting with 
Tunnel-Type1, but that is not what it says. Needs some work.

(4) MINOR:

In Section 3.3 I find the wording of the usage unclear in the following:

    The Alternate Tunnel Failure Indication message element is sent by
    the WTP to inform the AC about the status of the Alternate Tunnel.
    It MAY be included in the CAPWAP Station Configuration Request
    message.  ...

It is the way "MAY" is used here that causes me confusion, as if there 
is some other way to achieve this goal. Perhaps the following would be 
clearer:

    The WTP MAY include the Alternate Tunnel Failure Indication message
    in a CAPWAP Station Configuration Request message to inform the AC
    about the status of the Alternate Tunnel.

(5) MINOR:

In Section 4.2 I find the usage of the term "Access Router (LMA) 
Information Element" confusing. I find no definition of this as a 
distinct thing, so I gather the intent is that this is a particular 
usage of "Access Router Information Element". I think this would be 
clearer to remove "(LMA)" from Figure 10.

(6) MINOR:

Section 5.2 uses "ARs" and "ARs information" in ways that are unclear 
and improper grammar. IIUC "AR" in this document means "Access Router", 
and so "ARs" should mean "Access Routers". It is used that way once in 
section 3.3, and once in 5.2. But several other usages in 5.2 are 
different, and seem to be intended to refer to "Access Router 
Information Elements". I suggest the following change:

OLD

    ... If there are more than one ARs
    information provided by the AC for reliability reasons, the same
    Tunnel DTLS Policy (see Figure 14) is generally applied for all
    tunnels associated with the ARs.  Otherwise, Tunnel DTLS Policy MUST
    be bonding together with each of the ARs, then WTP will enforce the
    independent tunnel DTLS policy for each tunnel with a specific AR.

NEW

    ... If, for reliability reasons, the AC has provided more than one
    AR address in the Access Router Information Element, the same
    Tunnel DTLS Policy (see Figure 14) is generally applied for all
    tunnels associated with those ARs.  Otherwise, Tunnel DTLS Policy
    MUST be bonded together with each of the Access Router Information
    Elements, and the WTP will enforce the independent tunnel DTLS policy
    for each tunnel with a specific AR.

In addition the mechanics of this "bonding" aren't entirely clear. This 
seems to be covered by:

    A: If A bit is set, there is an AR information associated with the
    DTLS policy.  There may be an array of pairs binding DTLS policy
    information and AR information contained in the Tunnel DTLS Policy
    Element.  Otherwise, the same Tunnel DTLS Policy (see Figure 14) is
    generally applied for all tunnels associated with the ARs configured
    by the AC.

The above says "There may be an array of pairs". How is the array 
encoded and how is its length specified? I'm guessing you intend:

When A=0:

       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
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      |Tunnel DTLS Policy Element Type|        Length                 |
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      |                        Reserved                       |A|D|C|R|
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

When A=1:

       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
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      |Tunnel DTLS Policy Element Type|        Length                 |
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      |                        Reserved                       |A|D|C|R|
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      .                       AR Information                          .
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      |                        Reserved                       |A|D|C|R|
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      .                       AR Information                          .
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      |                        Reserved                       |A|D|C|R|
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      .                       AR Information                          .
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+


where the number of repeats is equal to the number of AR addresses 
previously specified.

This needs to be made much clearer. ISTM it would be cleaner to forget 
the flag, and simply say this is always a list, where the last element 
has no AR Information and provides options for any address not 
previously mentioned. (But this isn't an option if it is documenting 
existing usage.)

In Figure 9, I gather that "AR Information" means "Access Router 
Information Element", and in this context it must be restricted to a 
single address, and must be the address of one of previously specified 
AR addresses. If so, please say this explicitly.

(7) MINOR:

Section 5.3 has a similar construction to that in 5.2, with the same 
issues and should get a comparable fix.

(8) NIT:

IdNits reports that the reference to RFC2460 in section 5.6 is obsolete.