Hi, Paul

        Thank you very much for your careful review. 
        We will updated the draft accordingly. 

Best Regards
Zongpeng Du

-----Original Message-----
From: Paul Kyzivat [mailto:pkyzi...@alum.mit.edu] 
Sent: Tuesday, December 12, 2017 3:48 AM
To: draft-ietf-opsawg-capwap-alt-tunnel....@ietf.org
Cc: General Area Review Team
Subject: Gen-ART Last Call review of draft-ietf-opsawg-capwap-alt-tunnel-10

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.
_______________________________________________
Gen-art mailing list
Gen-art@ietf.org
https://www.ietf.org/mailman/listinfo/gen-art

Reply via email to