Re: [bess] MIBDoc review of draft-ietf-bess-l2l3-vpn-mcast-mib-07

Glenn Mansfield Keeni <glenn@cysols.com> Sun, 28 May 2017 22:35 UTC

Return-Path: <glenn@cysols.com>
X-Original-To: bess@ietfa.amsl.com
Delivered-To: bess@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 72D42126C23; Sun, 28 May 2017 15:35:03 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -0.001
X-Spam-Level:
X-Spam-Status: No, score=-0.001 tagged_above=-999 required=5 tests=[BAYES_40=-0.001, SPF_PASS=-0.001, URIBL_BLOCKED=0.001] autolearn=ham autolearn_force=no
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 NydItzdTKg5i; Sun, 28 May 2017 15:35:01 -0700 (PDT)
Received: from niseko.cysol.co.jp (niseko.cysol.co.jp [210.233.3.236]) (using TLSv1 with cipher DHE-RSA-CAMELLIA256-SHA (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id AE905126BFD; Sun, 28 May 2017 15:35:00 -0700 (PDT)
Received: from [192.168.0.92] (cysvpn05.priv.cysol.co.jp [192.168.0.92]) (authenticated bits=0) by aso.priv.cysol.co.jp (8.14.9/8.14.9) with ESMTP id v4SMYpSC081041 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO); Mon, 29 May 2017 07:34:52 +0900 (JST) (envelope-from glenn@cysols.com)
To: Hiroshi Tsunoda <tsuno@m.ieice.org>
Cc: Mach Chen <mach.chen@huawei.com>, "mib-doctors@ietf.org" <mib-doctors@ietf.org>, "Jeffrey (Zhaohui) Zhang" <zzhang@juniper.net>, "EXT - thomas.morin@orange.com" <thomas.morin@orange.com>, Martin Vigoureux <martin.vigoureux@nokia.com>, "bess@ietf.org" <bess@ietf.org>
References: <56E7D219.7000902@orange.com> <BLUPR0501MB17151A695785D4D8DD485633D4690@BLUPR0501MB1715.namprd05.prod.outlook.com> <b4249e61-0a11-2ce1-c846-67096858fa2c@cysols.com> <BLUPR0501MB1715A3B288A27A39E99203B8D4490@BLUPR0501MB1715.namprd05.prod.outlook.com> <c757a323-24a7-2696-657e-88f8e15e8a36@cysols.com> <CAPbjwkyFeX-S=sJwNMX-fgWThnMMiu_nF8xvcMow_BgJSfwsSQ@mail.gmail.com> <5e663cf0-1418-c410-bcf8-b235ee73fc29@cysols.com> <CAPbjwkyN0yLkpOXWt8D2-Niw7BCoujF+8JLjrPwgWobF03hZ7g@mail.gmail.com> <6f89f1f2-31e9-bf4a-05e9-1bb6e02f339e@cysols.com> <CAPbjwkyEnCGZEsGKjHozWmg-X-P3483=205BBGV9+DxbfJsDmQ@mail.gmail.com> <03f83a27-e397-818d-65e7-27f95cd6e6e0@cysols.com> <CAPbjwkzkKOULh98QBmbArFWXv=o_pyd7J82u7_GvwkWELdY0Pg@mail.gmail.com> <8f9aaac8-f44d-4844-d376-9c37bd7a81e0@cysols.com> <1848f053-9a5e-2f52-09b4-ce0e1688b557@cysols.com> <CAPbjwkzgORotsvYPWF2fqC7icJFXi5z10D1UHDTtYp1wojxw0Q@mail.gmail.com> <CAPbjwkw=zTUB+RL2MN7g2CeW07Xc6o39g6sf6ZpKap1+49ENiw@mail.gmail.com>
From: Glenn Mansfield Keeni <glenn@cysols.com>
Message-ID: <3c7ef211-7c41-ecee-7465-3987a890ce55@cysols.com>
Date: Mon, 29 May 2017 07:34:46 +0900
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.1.1
MIME-Version: 1.0
In-Reply-To: <CAPbjwkw=zTUB+RL2MN7g2CeW07Xc6o39g6sf6ZpKap1+49ENiw@mail.gmail.com>
Content-Type: text/plain; charset="iso-2022-jp"; format="flowed"; delsp="yes"
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Archived-At: <https://mailarchive.ietf.org/arch/msg/bess/chNXw33JdSRbO3Rinj_Garq5xhI>
Subject: Re: [bess] MIBDoc review of draft-ietf-bess-l2l3-vpn-mcast-mib-07
X-BeenThere: bess@ietf.org
X-Mailman-Version: 2.1.22
Precedence: list
List-Id: BGP-Enabled ServiceS working group discussion list <bess.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/bess>, <mailto:bess-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/bess/>
List-Post: <mailto:bess@ietf.org>
List-Help: <mailto:bess-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/bess>, <mailto:bess-request@ietf.org?subject=subscribe>
X-List-Received-Date: Sun, 28 May 2017 22:35:03 -0000

Dear Tsuno,
      Thanks for the good work.
I will review and get back to you.

Glenn
On 2017/05/26 21:25, Hiroshi Tsunoda wrote:
> Dear Glenn,
> 
> Thank you for your thorough review and waiting for the updated.
> 
> I posted a new revision taking into account your latest comments.
> In the new revision, all of your comments are addressed.
> I also revised the draft to improve the readability.
> 
> Please see some notes below.
> 
> URL:
> https://www.ietf.org/internet-drafts/draft-ietf-bess-l2l3-vpn-mcast-mib-08.txt
> Htmlized(1):
> https://tools.ietf.org/html/draft-ietf-bess-l2l3-vpn-mcast-mib-08
> Htmlized(2):
> https://datatracker.ietf.org/doc/html/draft-ietf-bess-l2l3-vpn-mcast-mib-08
> Diff:
> https://www.ietf.org/rfcdiff?url2=draft-ietf-bess-l2l3-vpn-mcast-mib-08
> 
> 2017-05-02 14:44 GMT+02:00 Hiroshi Tsunoda <tsuno@m.ieice.org>:
>> 2017-05-01 12:32 GMT+02:00 Glenn Mansfield Keeni <glenn@cysols.com>:
>>> Dear Tsuno/Zhang
>>>       Thanks for waiting. The review of
>>>   draft-ietf-bess-l2l3-vpn-mcast-mib-07 follows.
>>>
>>> Glenn
>>>
>>> C1. Abstract:
>>>      The draft now defines 2 MIB modules.  Please revise the abstract
>>>      and probably the title of the document too.
> 
> Fixed.
> 
>>> C2. The MIBs (L2L3-VPN-MCAST-TC-MIB, L2L3-VPN-MCAST-MIB) compile OK.
>>>      (Three {type-unref} warnings are there, may be ignored.)
> 
> I have confirmed that the latest version of MIB modules can also be
> compiled successfully.
> 
>>> C3. Page 4:
>>>        s/3.  Summary of MIB Module/
>>>          3.  Summary of MIB Modules/
> 
> Fixed.
> 
>>> C4. Page 6: L2L3VpnMcastProviderTunnelPointer DESCRIPTION
>>>        s/"Denotes a pointer to the row pertaining to a table entry/
>>>         /"This textual convention represents a pointer to a row in
>>>          the table represented by the following object of type
>>>          L2L3VpnMcastProviderTunnelPointerType./
> 
> Fixed.
> 
>>> C5. Page 7: L2L3VpnMcastProviderTunnelPointer DESCRIPTION
>>>          The explanation in the last paragraph seems out of place.
>>>          It may be removed.
> 
> Fixed.
> 
>>> C6  Page 7: L2L3VpnMcastProviderTunnelPointerType DESCRIPTION
>>>          it is unclear when the value 'null(0)' will be used.
>>>          Is this allowed only when the corresponding object of type
>>>          L2L3VpnMcastProviderTunnelPointer has a value that is a
>>>          zero-length string ? If yes, please make that clear.
> 
> 'null(0)' is the default value and indicates that the corresponding
> L2L3VpnMcastProviderTunnelPointer object is not assigned.
> In the new revision, this point is described clearly.
> 
>>> C7. Page 9: l2L3VpnMcastPmsiTunnelAttributeTable DESCRIPTION
>>>          s/created by a PE router/maintained by a PE router/
> 
> Fixed.
> 
>>> C8. Page 12: l2L3VpnMcastPmsiTunnelAttributeId
>>>           Do you really want to keep this object in L2L3-VPN-MCAST-MIB.
>>>           It will change every time a new "tunnel type" is added to
>>>           L2L3VpnMcastProviderTunnelType. That will defeat the purpose
>>>           of separating L2L3-VPN-MCAST-TC-MIB from L2L3-VPN-MCAST-MIB
>>>           It may be a good idea to define a textual convention like
>>>               L2L3VpnMcastPmsiTunnelAttributeId
>>>           in the L2L3-VPN-MCAST-TC-MIB and use that textual convention
>>>           in the L2L3-VPN-MCAST-MIB
> 
> Thank you for your suggestion. I revised the definition according to
> your suggestion.
> 
>>> C9. Page 13: l2L3VpnMcastPmsiTunnelAttributeId DESCRIPTION
>>> A.       s/Thus, the size of this object is 16 octets in IPv4/
>>>             Thus, the size of this object is 8 octets in IPv4/
> 
> Fixed.
> 
>>> B.       The last 2 paragraphs do not tie up well with the previous
>>>           paragraphs in the DESCRIPTION.
> 
> Those 2 paragraphs are removed in the new revision.
> 
>>> C.       In the last paragraph
>>>           "this object is a pair of source and group IP addresses"
>>>           is unlcear. Please clarify.
> 
> Fixed. In the new revision, this point is described as follows.
> 
> "the corresponding Tunnel Identifier is composed of
> the source IP address and the group IP address."
> 
>>> C10. Page 15: Security Considerations
>>>           I would think that any field that reveals information about
>>>           a Multicast VPN and/or its members is sensitive.
>>>           Does the l2L3VpnMcastPmsiTunnelAttributeId field reveal
>>>           information about the participating members? If yes, it must
>>>           be marked as sensitive.
> 
> I revised this point according to your suggestion.
> 
>>> C11. Editorial:
>>>
>>> A. This does not pertain to the MIBs as such,
>>>     but I am uncomfortable with the  several variations
>>>     of the phrase
>>>     "Layer 2 and Layer 3 Virtual Private Networks (VPN)
>>>      that support multicast"
>>>     that appears in the text. I do not have a solution
>>>     on hand but it would be perhaps be more readable to
>>>     define and use a simple name/notation the beast for
>>>     which the MIB is being designed (e.g. "L2L3VPNMCast").
>>>
>>> B. Same with the phrase
>>>      "Layer 2 (L2) and Layer 3 (L3) VPN (Virtual Private
>>>       Network)
>>>      it would be probably be easier on the reader if an
>>>      abbreviation like L2L3VPNs was used where ever
>>>      applicable.
> 
> Thank you for your comments.
> In the new reivsion the abbreviation "L2L3VPNMCast"
> is defined and used throughout the draft.
> 
>>> C12. P2:Para4: s/within MIB module specifications//;
> 
> Fixed.
> 
>>> C13. General:
>>> A.      The DESCRIPTION clauses could do would some more
>>>          packing. (Too much space at the end of lines)
>>> B.      Please check the articles a/an/the once again. Some
>>>          usages do not seem right to me.
> 
> Fixed.
> 
> Sincerely yours,
> 
> -- tsuno
>