Med This I-D references YANG 1.0 and not YANG 1.1.
Since there are incompatible changes between the two releases, I think that the I-D should address that. Tom Petch ----- Original Message ----- From: <[email protected]> To: "Tianran Zhou" <[email protected]>; "Senthil Sivakumar (ssenthil)" <[email protected]>; <[email protected]> Cc: "Ian Farrer" <[email protected]> Sent: Wednesday, April 26, 2017 9:18 AM Subject: Re: [OPSAWG] [Softwires] Comments on draft-sivakumar-yang-nat-05 > Hi Tianran, > > Yes, we would like to have a consensus on the NAT YANG data model and proceed with its publication in opsawg. > > Cheers, > Med > > > -----Message d'origine----- > > De : OPSAWG [mailto:[email protected]] De la part de Tianran Zhou > > Envoyé : mercredi 26 avril 2017 09:46 > > À : Senthil Sivakumar (ssenthil); [email protected] > > Objet : Re: [OPSAWG] [Softwires] Comments on draft-sivakumar-yang-nat-05 > > > > I am looking at the YANG tree. The overall structure of this module is > > clear and simple. > > In addition to the text improvement, what do you want to discuss and get > > consensus wrt the YANG data model? > > > > Regards, > > Tianran > > > > > -----Original Message----- > > > From: OPSAWG [mailto:[email protected]] On Behalf Of Senthil > > > Sivakumar (ssenthil) > > > Sent: Monday, April 24, 2017 9:44 PM > > > To: [email protected] > > > Subject: [OPSAWG] FW: [Softwires] Comments on draft-sivakumar-yang-nat- > > 05 > > > > > > Please find below the comments on the draft from Dan Wing, who chaired > > Behave > > > WG in the past and my response to his comments On the draft-sivakumar- > > yang-nat. > > > Opsawg chairs had asked us to seek reviews from NAT experts. > > > > > > Thanks > > > Senthil > > > > > > On 4/21/17, 10:42 AM, "Senthil Sivakumar (ssenthil)" > > <[email protected]> > > > wrote: > > > > > > > > > Sorry, it took a while to get back to this. Thanks for your > > comments. > > > Please see below for the responses. > > > > > > Thanks > > > Senthil > > > > > > On 2/8/17, 1:15 PM, "Softwires on behalf of Senthil Sivakumar > > (ssenthil)" > > > <[email protected] on behalf of [email protected]> wrote: > > > > > > I had reached out to Dan Wing and some others, as suggested by > > Ian > > > to get reviews on the > > > https://tools.ietf.org/html/draft-sivakumar-yang-nat-05. > > > > > > I am just posting the response from Dan here, I will address his > > > comments in a separate email and copy the alias. > > > > > > Thanks > > > Senthil > > > > > > Use [email protected] as my email address, to ease sorting > > the > > > email if someone happens to follow up. > > > > > > >> was going to ask you for a favor to review a I-D for me. The > > > recommendation from OPS WG was to get > > > >> Some expert reviews from Behave members, finding any > > behave > > > members willing to do a review is hard these days. > > > >> > > > >> https://tools.ietf.org/html/draft-sivakumar-yang-nat-05 > > > > > > > > My comments -- do you want them public somewhere? > > > > > > > > 1. Why not also cover NAT66 and NPTv6? It seems the > > design > > > allows those, but the text doesn't mention that they can also be > > expressed > > > in this model. > > > > > > NPTv6 is definitely a possibility, I am not sure if that is > > something > > > that needs to be in this yang model though. NAT66 is a can of worms and > > > I would like to stay away from that. > > > If NAT66 becomes a reality, we could define a yang model for it > > then. > > > > > > > > 2. " A NAT function can either assign individual port > > > numbers or port > > > > sets. Both features are supported in the YANG data > > model." > > > -> can you provide citation or definition of "port set"? > > > Ok, I will add the definition of port sets. > > > > > > > > > > > 3. " To accommodate deployments where [RFC6302] is > > not > > > enabled, the NAT > > > > function can be configured to log the destination > > port > > > number." -> that logging is not a "NAT function" (whereas the previous > > > paragraph does describe a NAT function). Logging is a function of YANG, > > > right? Perhaps instead how about text like "... this defined Yang model > > > can be configured to ..." > > > > > > Well, semantically, the NAT must provide the destination port to be > > able > > > to log the destination port. draft-ietf-ipfix-nat-logging provides the > > > templates to log the destination ports, the logging of the destination > > port > > > can be enabled by the Yang model. The intention of the text here is > > to > > > enable NAT logging of destination ports through the Yang model. > > > > > > > > > > > 4. "This data model assumes that pools of IPv4 > > addresses > > > can be > > > > provisioned to NAT function. These pools may be > > > contiguous or non- > > > > contiguous." > > > > > > > > First sentence does not read well, would be improved > > with > > > "... can be provisioned to *the* NAT function". Also, please provide > > > description or citation to what is meant by "pool". > > > I agree, I rephrase the first sentence as follows: > > > “This data model assumes that a block of IPv4 global addresses can > > be > > > provisioned to the NAT function.” > > > > > > > > > > > 5. "A NAT device can enabled multiple NAT instances;" > > > > > > > > does not read well. > > > > > > Yes, agreed. Will rephrase as: > > > “A single NAT device can have multiple NAT instances;" > > > > > > > > 6. " o Exclude/include ports (e.g.; system port) > > from the > > > port assignment > > > > pool." > > > > > > > > Nit: That should be "system portS" (plural). > > > > > > Ok. > > > > > > > > Serious: that is a significant deficiency. > > > The reason I think it was excluded was that, it comes with > > significant > > > complexity and most of the > > > NAT implementations don’t have explicit configuration to allow this > > > behavior. We will discuss this among the authors and > > > See what is the best way to address this. > > > > > > > > 7. "Deterministic NAT assignment scheme" -> provide > > > description or citation about what that means. > > > > > > > Ok. > > > > 8. "module: ietf-nat > > > > +--rw nat-config > > > > | +--rw nat-instances > > > > | +--rw nat-instance* [id] > > > > | +--rw id > > uint32 > > > > | +--rw enable? > > boolean > > > > | +--rw external-ip-address-pool* [pool-id] > > > > | | +--rw pool-id uint32 > > > > | | +--rw external-ip-pool? inet:ipv4- > > prefix" > > > > > > > > How is IPv6 expressed for NAT64? > > > > > > The address pool is always a IPv4 global address pool whether is > > NAT64 > > > or NAT44. > > > This is the address block from which the source is translated to. > > This > > > configuration applies > > > To both NAT64 and NAT44 and hence no distinction is required. > > > > > > > > > > > > 9. " | +--rw subscriber-mask-v6? > > > uint8 > > > > | +--rw subscriber-mask-v4* [sub-mask-id] > > > > | | +--rw sub-mask-id uint32 > > > > | | +--rw sub-mask inet:ipv4-prefix" > > > > > > > > Why are IPv6 masks expressed differently than IPv4 > > mask? > > > > > > The subscriber-mask-v6 is described in the later part of the > > document > > > and it was thought of having a single mask value that can be used > > > to apply to determine if the packet need to be translated or not. We > > > thought it would simplify the configuration. But thinking about it > > again, > > > I think some of the implementations of NAT64 don’t understand an > > integer > > > as a mask but they need the whole ipv6-prefix. I am going to discuss > > > This with the authors and change this to ipv6-prefix. At the least, > > we > > > will have either a prefix or an integer to represent the mask length. > > > > > > > > Is "subscriber" the same as "internal"? I mean, this > > whole > > > Yang model seems to use "subscriber" and "external", rather than > > "internal" > > > and "external". What if the "internal" side isn't """subscribers""", > > such > > > as a NAT in front of a datacenter, like a NAT64 in front of an IPv6-only > > > datacenter, or a NAT44 in front of an IPv4-only datacenter; in that case > > > the "subscriber" is now confusingly the server. > > > > > > Subscriber does imply internal. I will add some text around this to > > clarify > > > what subscribers mean here. If all the authors agree, I will change it > > to > > > internal and external, rather than subscriber and external. > > > > > > > > > > > > 10. | +--rw port-randomization-enable? > > > boolean > > > > | +--rw port-preservation-enable? > > boolean > > > > > > > > both of those could be set to TRUE, creating > > opportunity for > > > configuration and implementation conflict, creating interoperability > > > problems. Can you instead define a trinary value, or does Yang like to > > have > > > booleans so much, even when they can cause interop problems? > > > > > > I will discuss this with the authors and address this. I don’t know > > if > > > we can represent this more efficiently. > > > > > > > > 11. | +--rw udp-timeouts? > > uint32 > > > > > > > > Have you considered per-port timeouts? Some NATs are > > > configurable with short timeouts for certain ports, such as 10 seconds > > on > > > port 53 (DNS) and NTP (123) and longer timeouts on other ports. This > > Yang > > > model doesn't allow that. Seems a port list might be better to handle > > such > > > configurations. > > > > > > Good point. I will add the per-port timeouts with a list. > > > > > > > > 12. I noticed there isn't any reference in the text to > > RFC7857, > > > which updates and clarifies a lot of things. Is the Yang model > > compliant > > > with the changes caused by RFC7857? The text should certainly cite it > > where > > > it makes sense, but more importantly if additional settings are required > > > by RFC7857, they need to be part of the yang model. > > > > > > > > > > I believe it is compliant, I will talk to Med and make sure we are > > covered > > > here. > > > > > > > 13. | +--rw logging-info > > > > | | +--rw destination-address inet:ipv4- > > prefix > > > > | | +--rw destination-port inet:port- > > number > > > > > > > > this doesn't indicate UDP or TCP, and doesn't seem able > > to > > > log ICMP or other protocols that lack ports, which NATs might NAT (e.g., > > > IPsec ESP protocol 50). Should be highlighted in security > > considerations. > > > Ok. > > > > > > > > 14. | +--rw connection-limit > > > > | | +--rw limit-per-subscriber? uint32 > > > > | | +--rw limit-per-vrf? uint32 > > > > | | +--rw limit-per-subnet? > > > inet:ipv4-prefix > > > > > > > > Can only list one subnet? > > > > > > > Yes, it is a per-subnet basis and optional. > > > > > > > This doesn't allow different limits per VRF (e.g., VRF > > 1 is > > > limited to 100 mappings, VRF 2 is limited to 5555 mappings). Seems > > > restrictive. > > > > > > > Ok, we will discuss this again and decide if these should be an > > array > > > of limits. > > > > > > > 15. | +--rw ftp-alg-enable? > > > boolean > > > > | +--rw dns-alg-enable? > > boolean > > > > | +--rw tftp-alg-enable? > > boolean > > > > | +--rw msrpc-alg-enable? > > boolean > > > > | +--rw netbios-alg-enable? > > boolean > > > > | +--rw rcmd-alg-enable? > > boolean > > > > | +--rw ldap-alg-enable? > > boolean > > > > | +--rw sip-alg-enable? > > boolean > > > > | +--rw rtsp-alg-enable? > > boolean > > > > | +--rw h323-alg-enable? > > boolean > > > > | +--rw all-algs-enable? > > boolean > > > > > > > > OMG. ALGs, really? Do those need to be per-subscriber > > or > > > per-VRF or per-subnet? How are new ALGs added to Yang, like if I want > > an > > > ALG for, um, I dunno, WebRTC or ssh. > > > > > > > The yang models are extensible, you can augment more nodes or > > fields. > > > But you could make the above argument for anything that changes. > > Honestly, > > > I haven’t seen a need for a new ALG > > > In quite a few years. > > > > > > > 16. In mapping-table, I see: "rw lifetime > > > uint32". Would this track the lifetime while the TCP connection is > > becoming > > > fully-formed and hits the various Yang-defined 'timeout' values > > > (tcp-idle-timeout, tcp-trans-open-timeout, and so on)?". I suppose it > > does. > > > Text should say so. > > > > > > > > 17. | +--ro nat44-support? > > > boolean > > > > | +--ro nat64-support? > > > boolean > > > > > > > > > > > > NAT46? NAT66? NPTv6? XLAT64? CLAT? > > > Well, I will add NPTv6 to it, I don’t want to support non-ietf > > flavors > > > like 46 and 66. > > > > > > > > 18. stealth-mode-support needs better description than > > > "Indicates whether to respond for unsolicited traffic.", and needs to > > align > > > with IETF host requirements and IETF router requirements RFCs. > > > > > > > Ok, I will improve the description and add references. > > > > > > > > -d > > > > > > > > > > > > > > > > > > > > > > > > > > Thanks > > > Senthil > > > > > > _______________________________________________ > > > Softwires mailing list > > > [email protected] > > > https://www.ietf.org/mailman/listinfo/softwires > > > > > > > > > > > > > > > _______________________________________________ > > > OPSAWG mailing list > > > [email protected] > > > https://www.ietf.org/mailman/listinfo/opsawg > > _______________________________________________ > > OPSAWG mailing list > > [email protected] > > https://www.ietf.org/mailman/listinfo/opsawg > _______________________________________________ > OPSAWG mailing list > [email protected] > https://www.ietf.org/mailman/listinfo/opsawg > _______________________________________________ OPSAWG mailing list [email protected] https://www.ietf.org/mailman/listinfo/opsawg
