Hi Tianran, Thank you for the comments.
Please see inline. Cheers, Med > -----Message d'origine----- > De : Tianran Zhou [mailto:[email protected]] > Envoyé : jeudi 18 mai 2017 10:11 > À : BOUCADAIR Mohamed IMT/OLN; Senthil Sivakumar (ssenthil); > [email protected] > Cc : Ian Farrer > Objet : RE: Comments on draft-sivakumar-yang-nat-05 > > Hi Med and Senthil, > > I had a look at this draft, and found some issues that I'd like to > discuss. > > Regards, > Tianran > > > 1. You use the uint32 type id as key for many list objects. For example: > | +--rw nat-instances > | +--rw nat-instance* [id] > | +--rw id uint32 > | +--rw enable? boolean > > Is this design from your implementation? I think a string type "name" > is more easy to use. > For example, when we create a nat instance, we may type a command line > like: nat-instance nat-name > This ACL module might be another example. > https://datatracker.ietf.org/doc/draft-ietf-netmod-acl- > model/?include_text=1 > [Med] OK with the use of name. > 2. It seems your IP address pool only support prefix description. Some > more IP pool descriptions may be useful. For example, describe a IP > address section using the start IP and the length or end IP. > | +--rw external-ip-address-pool* [pool-id] > | | +--rw pool-id uint32 > | | +--rw external-ip-pool? inet:ipv4-prefix > [Med] The external IP address pool can be a contiguous or a set of non-contiguous IP prefixes. As such, It covers the scheme you are proposing. > 3. Did you consider the NAT server or dest_address NAT Scenario? [Med] I'm not sure to understand the question. If you meant, whether the model supports rewriting the destination address, then the answer is Yes. This is for example useful for NAT64. > > 4. Is this "port-quota" stands for the number of ports that can be used by > a nat instance? [Med] It corresponds to the maximum number of ports that can be used by a given subscriber. See this text excerpt from the draft: "Configures a port quota to be assigned per subscriber."; But I did not find something to indicate the port range. > Or could you please show me? > > 5. Your port configuration is associated with the whole IP address pool > list. Maybe it's more flexible if we can assign each ip pool with a port > range. [Med] This may be flexible, but I'm afraid there is no point in customizing the port configuration per address pool. A global port configuration is used by NATs/CGNs. > > 6. Here you can set connection-limit by subscriber, vrf,... How about also > by the session type, e.g. UDP, TCP, ... > | +--rw connection-limit > | | +--rw limit-per-subscriber? uint32 > | | +--rw limit-per-vrf? uint32 > | | +--rw limit-per-subnet? inet:ipv4-prefix > | | +--rw limit-per-instance uint32 > [Med] I will defer to Senthil on this one. > 7. I am not clear on how to use this mapping table. My understanding is > this is used for static mapping only. > | +--rw mapping-table > | +--rw mapping-entry* [index] > | +--rw index uint32 > | +--rw type? enumeration > | +--rw internal-src-address inet:ip-address [Med] The mapping table is used for dynamic mapping. The draft says the following: The NAT data model is designed to cover both configuration and state retrieval, nevertheless this document covers dynamic (implicit) mapping while PCP-related functionality to instruct dynamic explicit mapping is defined in [I-D.boucadair-pcp-yang]. > > 8. In all, I think it's better to describe how this module is structured > and also give some examples on how to use this module. [Med] Point taken. > > > > -----Original Message----- > > From: [email protected] [mailto:[email protected]] > > Sent: Wednesday, April 26, 2017 4:18 PM > > To: Tianran Zhou; Senthil Sivakumar (ssenthil); [email protected] > > Cc: Ian Farrer > > Subject: RE: [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
