Re: [OPSAWG] AD Review of " A YANG Module for Network Address Translation (NAT) and Network Prefix Translation (NPT)" draft-ietf-opsawg-nat-yang-13
<mohamed.boucadair@orange.com> Fri, 23 March 2018 11:19 UTC
Return-Path: <mohamed.boucadair@orange.com>
X-Original-To: opsawg@ietfa.amsl.com
Delivered-To: opsawg@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id E07FB12D7E5; Fri, 23 Mar 2018 04:19:10 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -2.61
X-Spam-Level:
X-Spam-Status: No, score=-2.61 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H2=-0.001, SPF_PASS=-0.001, T_RP_MATCHES_RCVD=-0.01, UNPARSEABLE_RELAY=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 LmCyFLbco1pb; Fri, 23 Mar 2018 04:19:08 -0700 (PDT)
Received: from orange.com (mta134.mail.business.static.orange.com [80.12.70.34]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 8768312D86B; Fri, 23 Mar 2018 04:19:08 -0700 (PDT)
Received: from opfednr00.francetelecom.fr (unknown [xx.xx.xx.64]) by opfednr27.francetelecom.fr (ESMTP service) with ESMTP id 29FA5A00DF; Fri, 23 Mar 2018 12:19:07 +0100 (CET)
Received: from Exchangemail-eme3.itn.ftgroup (unknown [xx.xx.50.66]) by opfednr00.francetelecom.fr (ESMTP service) with ESMTP id 14A861A0067; Fri, 23 Mar 2018 12:19:07 +0100 (CET)
Received: from OPEXCNORMAD.corporate.adroot.infra.ftgroup ([fe80::f1a0:3c6b:bc7b:3aaf]) by OPEXCNORM4E.corporate.adroot.infra.ftgroup ([fe80::d5d9:c91a:994b:fc0b%21]) with mapi id 14.03.0382.000; Fri, 23 Mar 2018 12:19:04 +0100
From: mohamed.boucadair@orange.com
To: Warren Kumari <warren@kumari.net>
CC: "opsawg@ietf.org" <opsawg@ietf.org>, "draft-ietf-opsawg-nat-yang.all@ietf.org" <draft-ietf-opsawg-nat-yang.all@ietf.org>
Thread-Topic: AD Review of " A YANG Module for Network Address Translation (NAT) and Network Prefix Translation (NPT)" draft-ietf-opsawg-nat-yang-13
Thread-Index: AQHTwo3fwcVJ1O/VBkamvGfcreFLJKPdmRFwgAABtwCAABFS0A==
Date: Fri, 23 Mar 2018 11:19:04 +0000
Message-ID: <787AE7BB302AE849A7480A190F8B93302DEE6B53@OPEXCNORMAD.corporate.adroot.infra.ftgroup>
References: <CAHw9_iLf+od4LyeUOO2E8Nn2O2FB4R+=LXArZ6K74aWb5RHkJA@mail.gmail.com> <787AE7BB302AE849A7480A190F8B93302DEE6A43@OPEXCNORMAD.corporate.adroot.infra.ftgroup> <CAHw9_iLPXArKGBLzQSLJezESb6KQMwpLcMKUDDwebL+a0zy7zg@mail.gmail.com>
In-Reply-To: <CAHw9_iLPXArKGBLzQSLJezESb6KQMwpLcMKUDDwebL+a0zy7zg@mail.gmail.com>
Accept-Language: fr-FR, en-US
Content-Language: fr-FR
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
x-originating-ip: [10.168.234.2]
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: base64
MIME-Version: 1.0
Archived-At: <https://mailarchive.ietf.org/arch/msg/opsawg/w9LSU_EEAaTVTxPVwRaU1GQ55mc>
Subject: Re: [OPSAWG] AD Review of " A YANG Module for Network Address Translation (NAT) and Network Prefix Translation (NPT)" draft-ietf-opsawg-nat-yang-13
X-BeenThere: opsawg@ietf.org
X-Mailman-Version: 2.1.22
Precedence: list
List-Id: OPSA Working Group Mail List <opsawg.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/opsawg>, <mailto:opsawg-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/opsawg/>
List-Post: <mailto:opsawg@ietf.org>
List-Help: <mailto:opsawg-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/opsawg>, <mailto:opsawg-request@ietf.org?subject=subscribe>
X-List-Received-Date: Fri, 23 Mar 2018 11:19:11 -0000
Re-, Cool! Thanks. An updated version with these changes is available online: https://datatracker.ietf.org/doc/draft-ietf-opsawg-nat-yang/ Cheers, Med > -----Message d'origine----- > De : Warren Kumari [mailto:warren@kumari.net] > Envoyé : vendredi 23 mars 2018 11:16 > À : BOUCADAIR Mohamed IMT/OLN > Cc : opsawg@ietf.org; draft-ietf-opsawg-nat-yang.all@ietf.org > Objet : Re: AD Review of " A YANG Module for Network Address Translation > (NAT) and Network Prefix Translation (NPT)" draft-ietf-opsawg-nat-yang-13 > > On Fri, Mar 23, 2018 at 10:35 AM, <mohamed.boucadair@orange.com> wrote: > > Hi Warren, > > > > Thank you for the review. > > > > Please see inline. > > > > Cheers, > > Med > > > >> -----Message d'origine----- > >> De : Warren Kumari [mailto:warren@kumari.net] > >> Envoyé : vendredi 23 mars 2018 10:00 > >> À : opsawg@ietf.org; draft-ietf-opsawg-nat-yang.all@ietf.org > >> Objet : AD Review of " A YANG Module for Network Address Translation (NAT) > >> and Network Prefix Translation (NPT)" draft-ietf-opsawg-nat-yang-13 > >> > >> AD Review of " A YANG Module for Network Address Translation (NAT) and > >> Network Prefix Translation (NPT)" draft-ietf-opsawg-nat-yang-13 > >> > >> Note: I started while I was the Responsible AD for OpsAWG; Igans has > >> taken this over, but these may still be helpful. > >> > >> ------- > >> Hi there, > >> > >> Apologies for how long this AD review took -- various travels got in > >> the way and this got delayed. > >> > >> Thank you to the editors and WG for your efforts on this document, > >> it's a well written and easy to understand > >> draft. I do have a few comments that I’d like addressed before I > >> start IETF LC — addressing these now will avoid > >> issues later in the process. > >> > >> 1: Section Abstract, Terminology > >> "NAT44, Network Address and Protocol Translation from IPv6 Clients to > >> IPv4 Servers (NAT64), ..." > >> NAT44 is not defined, nor is it in the RFC Editor Well Known Acronyms > >> list - I think RFC7857 might work or just add something like "Network > >> Address Translation from IPv4 > >> to IPv4 (NAT44)" to terminology. > >> > > > > [Med] I added "Network Address Translation from IPv4 to IPv4 (NAT44)" > > > >> 2: Section 2.1. Overview > >> "This YANG module allows to instruct a NAT function to enable the > >> logging feature" > >> This is missing some words -- perhaps "This YANG module provides a > >> method to instruct a NAT function to enable the logging feature" (or > >> similar) > >> > > > > [Med] Works for me. Fixed. > > > > > >> 3: Section 2.2. Various Translation Flavors > >> "The NAT YANG module allows to retrieve the capabilities of a NAT > >> instance " -- same as above > >> > > > > [Med] Fixed. > > > >> 4: Section 2.4. Other Transport Protocols > >> "The module is structured to support other protocols than UDP, TCP, and > ICMP. > >> " > >> s/other protocols than/protocols other than/ (readability / flow) > >> > > > > [Med] OK > > > >> 5: "The mapping table is designed so that it can indicate any > >> transport protocol. For example, this module may be used to manage a > >> DCCP-capable NAT that adheres to [RFC5597]. > >> Future extensions can be defined to cover NAT-related considerations > >> that are specific to other transport protocols such as SCTP > >> [I-D.ietf-tsvwg-natsupp]." > >> The above sounds confusing (to me at least) - the mapping table is > >> designed so it can indicate any transport protocol. Future extensions > >> can be defined to make it do so? (not sure how to word it better, but > >> the above sounds unclear as to if the mapping table can actually > >> indicate any transport protocol or if it itself needed to be extended) > >> > > > > [Med] The mapping table can indicate any transport protocols. Nevertheless, > if some transport needs to manipulate some specific information, then the > mapping entry needs to be extended. > > > > I changed the text to "Future extensions may be needed to cover ..." > > > >> 6: "Also, the module allows to enable translation for these protocols > >> when required" > >> Similar to #2, #3 -- perhaps "the module allows the operator to > >> enable translation" or "the module enables translation for" (I think > >> the former, or reword). > >> > > > > [Med] Fixed. > > > >> > >> 7: Section 2.6. Port Set Assignment > >> "Port numbers can be assigned by a NAT individually (that is, a single > >> port is assigned on a per session basis). Nevertheless, this port > >> allocation scheme may not be optimal for logging purposes" > >> I would suggest combining these into a single sentence -- "... on a > >> per session basis), but this port..." - purely a readability nit > >> > > > > [Med] Deal. > > > > > >> 8: "Therefore, a NAT function should be able to assign > >> port sets (e.g., [RFC7753]) to optimize the volume of the logging > >> data (REQ-14 of [RFC6888])." > >> "Therefore" sounds like it is a new requirement on NATS - can you > >> reword to make it clear it isn't. > >> > > > > [Med] I deleted "Therefore" to avoid that misinterpretation. > > > >> 9: Section 2.7. Port-Restricted IP Addresses > >> "Some NATs require to restrict the source port numbers" > >> I'd suggest s/require to// > >> > > > > [Med] Fixed. > > > >> 10: Section 2.8. NAT Mapping Entries > >> "A TCP/UDP mapping entry maintains an association between the > >> following information:" > >> It this true for all types of NATs? For example, a 1:1 NAT / > >> rewriting doesn't need to track ports, because 192.0.2.1:xxx -> > >> 10.10.10.10:xxx (internal port == external port, so no need to track > >> port state) > >> > > > > [Med] TCP/UDP mapping does make sense only when ports are rewritten. For > the case you cited, mappings are not tracked at the transport level. > > > >> 11: "In order to cover both NAT64 and NAT44 flavors in particular, > >> the NAT mapping structure allows to include an IPv4" > >> I think you can drop the "in particular" > >> > > > > [Med] Works for me. > > > >> 12: "In order to cover both NAT64 and NAT44 flavors in particular, the > >> NAT mapping structure allows to include an IPv4" > >> "allows to include an" parses oddly - perhaps "allows for the > >> inclusion of an..." (or similar) > >> > > > > [Med] Fixed. > > > >> 13: Table 5 is formatted oddly / weird justification - presumably the > >> RFC Editor would fix this, but if you can, it would make review > >> easier. > >> > > [Med] Will check and fix as appropriate. > > > >> 14: "In order to prevent from generating frequent notifications" > >> This is missing a word or words. > >> > > > > [Med] I added "...prevent a NAT implementation ... " > > > >> 15: "The NAT YANG module allows to set parameters to prevent a user from" > >> Similar to #2, #3. > >> > > > > [Med] Fixed. > > > >> 16: "Nevertheless, an attacker who is able to access to the NAT can > >> undertake" > >> s/to// > >> > > > > [Med] Fixed. > > Win! > > Thank you, > W > > > > >> > >> Thank you. > >> W > >> > >> -- > >> I don't think the execution is relevant when it was obviously a bad > >> idea in the first place. > >> This is like putting rabid weasels in your pants, and later expressing > >> regret at having chosen those particular rabid weasels and that pair > >> of pants. > >> ---maf > > > > -- > I don't think the execution is relevant when it was obviously a bad > idea in the first place. > This is like putting rabid weasels in your pants, and later expressing > regret at having chosen those particular rabid weasels and that pair > of pants. > ---maf
- [OPSAWG] AD Review of " A YANG Module for Network… Warren Kumari
- Re: [OPSAWG] AD Review of " A YANG Module for Net… mohamed.boucadair
- Re: [OPSAWG] AD Review of " A YANG Module for Net… Warren Kumari
- Re: [OPSAWG] AD Review of " A YANG Module for Net… mohamed.boucadair