Re: [Softwires] Last Call: <draft-ietf-softwire-yang-06.txt> (YANG Modules for IPv4-in-IPv6 Address plus Port Softwires) to Proposed Standard
<mohamed.boucadair@orange.com> Mon, 22 October 2018 14:49 UTC
Return-Path: <mohamed.boucadair@orange.com>
X-Original-To: softwires@ietfa.amsl.com
Delivered-To: softwires@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 67E11130DC9; Mon, 22 Oct 2018 07:49:17 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -0.7
X-Spam-Level:
X-Spam-Status: No, score=-0.7 tagged_above=-999 required=5 tests=[RCVD_IN_DNSWL_LOW=-0.7, SPF_PASS=-0.001, UNPARSEABLE_RELAY=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 vFGtq0vk6XcD; Mon, 22 Oct 2018 07:49:14 -0700 (PDT)
Received: from orange.com (mta136.mail.business.static.orange.com [80.12.70.36]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id A21D01276D0; Mon, 22 Oct 2018 07:49:13 -0700 (PDT)
Received: from opfednr07.francetelecom.fr (unknown [xx.xx.xx.71]) by opfednr25.francetelecom.fr (ESMTP service) with ESMTP id 42dztc2PGXzCrD0; Mon, 22 Oct 2018 16:49:12 +0200 (CEST)
Received: from Exchangemail-eme2.itn.ftgroup (unknown [xx.xx.31.63]) by opfednr07.francetelecom.fr (ESMTP service) with ESMTP id 42dztc0kwFzFpWM; Mon, 22 Oct 2018 16:49:12 +0200 (CEST)
Received: from OPEXCLILMA3.corporate.adroot.infra.ftgroup ([fe80::60a9:abc3:86e6:2541]) by OPEXCLILM6E.corporate.adroot.infra.ftgroup ([fe80::f5a7:eab1:c095:d9ec%18]) with mapi id 14.03.0415.000; Mon, 22 Oct 2018 16:49:11 +0200
From: mohamed.boucadair@orange.com
To: tom petch <daedulus@btconnect.com>, ietf <ietf@ietf.org>
CC: "softwires@ietf.org" <softwires@ietf.org>, "softwire-chairs@ietf.org" <softwire-chairs@ietf.org>, "jiangsheng@huawei.com" <jiangsheng@huawei.com>, "draft-ietf-softwire-yang@ietf.org" <draft-ietf-softwire-yang@ietf.org>
Thread-Topic: Last Call: <draft-ietf-softwire-yang-06.txt> (YANG Modules for IPv4-in-IPv6 Address plus Port Softwires) to Proposed Standard
Thread-Index: AQHUVlqQnNxPR9IrokuKmtHmCXfJNKUrcFSQ
Date: Mon, 22 Oct 2018 14:49:11 +0000
Message-ID: <787AE7BB302AE849A7480A190F8B93302E018830@OPEXCLILMA3.corporate.adroot.infra.ftgroup>
References: <153805000273.26427.17737657568994190653.idtracker@ietfa.amsl.com> <005501d45746$079c5480$4001a8c0@gateway.2wire.net> <022801d45979$9499a880$4001a8c0@gateway.2wire.net> <052a01d45b10$0abc3d60$4001a8c0@gateway.2wire.net>
In-Reply-To: <052a01d45b10$0abc3d60$4001a8c0@gateway.2wire.net>
Accept-Language: fr-FR, en-US
Content-Language: fr-FR
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
x-originating-ip: [10.168.234.4]
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: base64
MIME-Version: 1.0
Archived-At: <https://mailarchive.ietf.org/arch/msg/softwires/qmpbTra0nYdM1eaSwDX_1WsZCDs>
Subject: Re: [Softwires] Last Call: <draft-ietf-softwire-yang-06.txt> (YANG Modules for IPv4-in-IPv6 Address plus Port Softwires) to Proposed Standard
X-BeenThere: softwires@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: softwires wg discussion list <softwires.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/softwires>, <mailto:softwires-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/softwires/>
List-Post: <mailto:softwires@ietf.org>
List-Help: <mailto:softwires-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/softwires>, <mailto:softwires-request@ietf.org?subject=subscribe>
X-List-Received-Date: Mon, 22 Oct 2018 14:49:18 -0000
Re-, Thank you for the review. Please see inline. Cheers, Med > -----Message d'origine----- > De : tom petch [mailto:daedulus@btconnect.com] > Envoyé : mercredi 3 octobre 2018 13:57 > À : tom petch; ietf > Cc : softwires@ietf.org; softwire-chairs@ietf.org; jiangsheng@huawei.com; > draft-ietf-softwire-yang@ietf.org > Objet : Re: Last Call: <draft-ietf-softwire-yang-06.txt> (YANG Modules for > IPv4-in-IPv6 Address plus Port Softwires) to Proposed Standard > > And some technical comments on this I-D as a result of which, I do not > think this is ready to progress; perhaps no show stoppers, just a lot of > changes are needed IMHO. The more I look into this I-D, the less I > understand (which may be my ignorance of YANG). > > This I-D is concerned with three tunnel types (Lw4o6, MAP-E, MAP-T). In > several places, you have > augment "/if:interfaces/if:interface" { > when "if:type = 'ianaift:tunnel'"; > This will augment for all tunnel types, not just those of this I-D. I > think you should have your own three (?) derived identities for the > three specific tunnels described here, all in the common module. [Med] Good point. A new parameter called tunnel-type is now added to the interface YANG module (augments 8343). > > The three YANG modules are for Customer Premises Equipment, Border Relay > and a common module. Both the first two define two features, binding and > algorithm, binding for Lw4o6 and algorithm for MAP-E/MAP-T - I do not > know if/how this duplication of features will work (and as I said > before, I am confused about support for 'MAP-E and MAP-T' versus 'MAP-E > or > MAP-T' both of which phrases appear in the I-D). As with tunnel types, > should > there be three features, should they be in the common module? (yes, and > yes) [Med] The feature indicates the support of MAP-E and/or MAP-T. the data plane clause is supposed to hint whether this is about MAP-E (encapsulation) or MAP-T (transaltion). I do agree that defining features for each of these two may be more simple. > > 2.2 > 'they should be imported here, as needed. ' > import is a YANG technical term as used in this I-D so I think the use > of it here wrong [Med] Fixed. > > ' The CE must already have minimal IPv6 configuration' > Could that also be IPv4 which, by definition, is going to be widespread > in the deployment? [Med] A+P mechanisms assumes that no IPv4 configuration is provided because A+P is meant to provide IPv4 service continuity over IPv6. > > 3.2 > softwire-path-mru: > what is the point of this field? I looked at RFC7596, RFC7597 and > RFC7599 > and could find no reference thereto. I went back to their references, > such as RFC4821, RFC2473, RFC1981, RFC6346 ... - no joy. I had thought > to suggest a reference was called for but seeing the complete absence of > any connection, I think that a substantial explanation is called for. > [Med] I completely lost the context why the document ended to have this in. I'm sure Ian has a record about this. > Likewise, I note that references to MTU are qualified with IPv4 but > references to MRU are not; rather, this object > 'set the maximum IPv6 softwire packet size that can be received ...' > when > 'the implementation is unable to calculate the correct IPv4 size ' > Really? > > 'IPv6 prefix type' or ''IPv6 address type'. > Well, no It can be > type ipv6-prefix or ipv6-address > [Med] Yeah! > This description of the ietf-softwire-ce module describes objects that > are not in the ietf-softwire-ce module, which confuses me. Rather they > are in the ietf-softwire-common module. I think you need a description > of the objects in the ietf-softwire-common module first and then moving > on to the two specific modules. The sense I get is that while splitting > into three makes sense, the consequences have not been thought through. [Med] Not sure which part you are referring to. Can you please explicit your comment? Thank you. > > The descriptions of objects here are (mostly) good, but do not appear in > the YANG module. Those in the YANG module are shallow by comparison and > should be at least as comprehensive as those in the body of the I-D; the > YANG module has to be able to survive on its own. > > 'The version number is incremented ' > Any constraints on the increment? one, a hundred, a million...? [Med] This is implementation-specific. As indicated in this example, a timestamp would be sufficient. > > 4.2 > As with 3.2, the descriptions here of the objects look fine, those in > the description clauses of the YANG module do not; they are thin by > comparison. > > 5 > leaf br-ipv6-addr { > mandatory true; > "The IPv6 address for of the binding BR."; > This is not quite English. [Med] Fixed. > And since this object is MAP-E only, should it be, can it be, mandatory? [Med] this is of lw4over6. Hence, the mandatory clause. > > leaf id { > type uint32; > mandatory true; > description "Algorithm Instance ID"; > Any constraints on how this 32-bit integer will be used? [Med] No, there is no constraint defined in existing RFCs. > > 6 > > leaf softwire-num-max { type uint32; > description > "The maximum number of softwires that can be created on > the binding BR."; > Any restriction on the range of this. Can it be zero? [Med] No restriction is set because we don't have such recommendation in softwire RFCs. Setting it to zero is equivalent to disabling the instance. > > / "Enables the generation of ICMPv6 errors messages/ > "Enables the generation of ICMPv6 error messages/ > [Med] Fixed. Thank you. > leaf icmpv4-rate { > leaf icmpv6-rate { > Can these validly be zero? Also, I think that there should be a > recommended value (the Transport Area are keen on such things) [Med] We'd love to add a reco here, but absent any authoritative RFC, we are avoiding to add it. > > leaf id { > type uint32; > mandatory true; > description "id"; > Not the most helpful of descriptions [Med] Yes, indeed. > > 'This must be initialized when the BR instance is configured' > Perhaps > "This must be reset to the current date-and-time when the BR instance is > configured" > [Med] This is better. > 'Notify the client that a specific binding entry has been > expired/invalid. > Perhaps > 'Notify the client that a specific binding entry has expired or is > invalid.' > [Med] Fixed. > leaf date { > type yang:date-and-time; > description "Timestamp to the algorithm"; > Not a helpful description IMHO > [Med] Updated the text. > leaf name { > type string; > description "The name for the instance."; > ditto; what is the namespace, how is the name used? > [Med] This is used to uniquely distinguish instances by their alias/name. > "Embedded Address (EA) bits are the IPv4 EA-bits in the IPv6 > address identify an IPv4 prefix/address (or part thereof) or > a shared IPv4 address (or part thereof) and a port-set > identifier. > This is not quite English [Med] s/identify/identifying. > > 8 > The IESG like to see TLS1.3 RFC8446 here [Med] Done. > > I have yet to review the examples, but if my suggestion above result in > changes, then the examples will change significantly. > > Tom Petch [Med] Many thanks for the detailed review .
- [Softwires] Last Call: <draft-ietf-softwire-yang-… The IESG
- Re: [Softwires] Last Call: <draft-ietf-softwire-y… tom petch
- Re: [Softwires] Last Call: <draft-ietf-softwire-y… tom petch
- Re: [Softwires] Last Call: <draft-ietf-softwire-y… tom petch
- Re: [Softwires] Last Call: <draft-ietf-softwire-y… mohamed.boucadair
- Re: [Softwires] Last Call: <draft-ietf-softwire-y… mohamed.boucadair
- Re: [Softwires] Last Call: <draft-ietf-softwire-y… mohamed.boucadair
- Re: [Softwires] Last Call: <draft-ietf-softwire-y… tom petch