Re: [OPSAWG] Comments on draft-sivakumar-yang-nat-05

[<[e-mail]>]

Hi Tianran, 

Thank you for the comments. 

Please see inline. 

Cheers,
Med

> -----Message d'origine-----
> De : Tianran Zhou [mailto:[e-mail]]
> Envoyé : jeudi 18 mai 2017 10:11
> À : BOUCADAIR Mohamed IMT/OLN; Senthil Sivakumar (ssenthil);
> [e-mail]
> 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: [e-mail] [mailto:[e-mail]]
> > Sent: Wednesday, April 26, 2017 4:18 PM
> > To: Tianran Zhou; Senthil Sivakumar (ssenthil); [e-mail]
> > 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:[e-mail]] De la part de Tianran
> > > Zhou Envoyé : mercredi 26 avril 2017 09:46 À : Senthil Sivakumar
> > > (ssenthil); [e-mail] 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:[e-mail]] On Behalf Of Senthil
> > > > Sivakumar (ssenthil)
> > > > Sent: Monday, April 24, 2017 9:44 PM
> > > > To: [e-mail]
> > > > 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)"
> > > <[e-mail]>
> > > > 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)"
> > > > <[e-mail] on behalf of [e-mail]> 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 [e-mail] 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
> > > >         [e-mail]
> > > >         https://www.ietf.org/mailman/listinfo/softwires
> > > >
> > > >
> > > >
> > > >
> > > > _______________________________________________
> > > > OPSAWG mailing list
> > > > [e-mail]
> > > > https://www.ietf.org/mailman/listinfo/opsawg
> > > _______________________________________________
> > > OPSAWG mailing list
> > > [e-mail]
> > > https://www.ietf.org/mailman/listinfo/opsawg