[ovs-dev,1/2] ovn-nbctl: Add NAT commands.

This patch provides the command line to create NAT rules
on logical router.

Signed-off-by: nickcooper-zhangtonghao <nickcooper-zhangtonghao@opencloud.tech>
---
 ovn/utilities/ovn-nbctl.8.xml |  66 +++++++++++++++
 ovn/utilities/ovn-nbctl.c     | 185 ++++++++++++++++++++++++++++++++++++++++++
 tests/ovn-nbctl.at            | 114 ++++++++++++++++++++++++++
 3 files changed, 365 insertions(+)

On 11 October 2016 at 07:36, nickcooper-zhangtonghao <
nickcooper-zhangtonghao@opencloud.tech> wrote:

> This patch provides the command line to create NAT rules
> on logical router.
>
> Signed-off-by: nickcooper-zhangtonghao <nickcooper-zhangtonghao@
> opencloud.tech>
> ---
>  ovn/utilities/ovn-nbctl.8.xml |  66 +++++++++++++++
>  ovn/utilities/ovn-nbctl.c     | 185 ++++++++++++++++++++++++++++++
> ++++++++++++
>  tests/ovn-nbctl.at            | 114 ++++++++++++++++++++++++++
>  3 files changed, 365 insertions(+)
>
> diff --git a/ovn/utilities/ovn-nbctl.8.xml b/ovn/utilities/ovn-nbctl.8.xml
> index 2cbd6e0..be802da 100644
> --- a/ovn/utilities/ovn-nbctl.8.xml
> +++ b/ovn/utilities/ovn-nbctl.8.xml
> @@ -424,6 +424,72 @@
>        </dd>
>      </dl>
>
> +    <h1>NAT Commands</h1>
> +
> +    <dl>
> +      <dt>[<code>--may-exist</code>] <code>lr-nat-add</code>
> <var>router</var> <var>type</var> <var>external_ip</var>
> <var>logical_ip</var></dt>
> +      <dd>
> +        <p>
> +          Adds the specified NAT to <var>router</var>.
> +          The <var>type</var> must be one of <code>snat</code>,
> +          <code>dnat</code>, or <code>dnat_and_snat</code>.
> +          The <var>external_ip</var> is an IPv4 address.
> +          The <var>logical_ip</var> is an IPv4 network (e.g
> 192.168.1.0/24)
> +          or an IPv4 address.
> +        </p>
> +        <p>
> +          When <var>type</var> is <code>dnat</code>, the externally
> +          visible IP address <var>external_ip</var> is DNATted to the
> +          IP address <var>logical_ip</var> in the logical space.
> +        </p>
> +        <p>
> +          When <var>type</var> is <code>snat</code>, IP packets with their
> +          source IP address that either matches the IP address in
> +          <var>logical_ip</var> or is in the network provided by
> +          <var>logical_ip</var> is SNATed into the IP address in
> +          <var>external_ip</var>.
> +        </p>
> +        <p>
> +          When <var>type</var> is <code>dnat_and_snat</code>,
> +          the externally visible IP address <var>external_ip</var>
> +          is DNATted to the IP address <var>logical_ip</var> in
> +          the logical space.  In addition, IP packets with the source
> +          IP address that matches <var>logical_ip</var> is SNATed into
> +          the IP address in <var>external_ip</var>.
> +        </p>
> +        <p>
> +          It is an error if a NAT already exists,
> +          unless <code>--may-exist</code> is specified.
> +        </p>
> +      </dd>
> +
> +      <dt>[<code>--if-exists</code>] <code>lr-nat-del</code>
> <var>router</var> [<var>type</var> [<var>ip</var>]]</dt>
> +      <dd>
> +        <p>
> +          Deletes NATs from <var>router</var>.  If only <var>router</var>
> +          is supplied, all the NATs from the logical router are
> +          deleted.  If <var>type</var> is also specified, then all the
> +          NATs that match the <var>type</var> will be deleted from the
> logical
> +          router.  If all the fields are given, then a single NAT rule
> +          that matches all the fields will be deleted.  When
> <var>type</var>
> +          is <code>snat</code>, the <var>ip</var> should be logical_ip.
> +          When <var>type</var> is <code>dnat</code> or
> +          <code>dnat_and_snat</code>, the <var>ip</var> shoud be
> external_ip.
> +        </p>
> +
> +        <p>
> +          It is an error if <var>ip</var> is specified and there
> +          is no matching NAT entry, unless <code>--if-exists</code> is
> +          specified.
> +        </p>
> +      </dd>
> +
> +      <dt><code>lr-nat-list</code> <var>router</var></dt>
> +      <dd>
> +        Lists the NATs on <var>router</var>.
> +      </dd>
> +    </dl>
> +
>      <h1>Load Balancer Commands</h1>
>      <dl>
>        <dt>[<code>--may-exist</code> | <code>--add-duplicate</code>]
> <code>lb-add</code> <var>lb</var> <var>vip</var> <var>ips</var>
> [<var>protocol</var>]</dt>
> diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
> index ad2d2f8..916dedb 100644
> --- a/ovn/utilities/ovn-nbctl.c
> +++ b/ovn/utilities/ovn-nbctl.c
> @@ -384,6 +384,13 @@ Route commands:\n\
>                              remove routes from ROUTER\n\
>    lr-route-list ROUTER      print routes for ROUTER\n\
>  \n\
> +NAT commands:\n\
> +  lr-nat-add ROUTER TYPE EXTERNAL_IP LOGICAL_IP\n\
> +                            add a NAT to ROUTER\n\
> +  lr-nat-del ROUTER [TYPE [IP]]\n\
> +                            remove NATs from ROUTER\n\
> +  lr-nat-list ROUTER        print NATs for ROUTER\n\
> +\n\
>  LB commands:\n\
>    lb-add LB VIP[:PORT] IP[:PORT]... [PROTOCOL]\n\
>                              create a load-balancer or add a VIP to an\n\
> @@ -2165,6 +2172,177 @@ nbctl_lr_route_del(struct ctl_context *ctx)
>      }
>      free(prefix);
>  }
> +
> +static void
> +nbctl_lr_nat_add(struct ctl_context *ctx)
> +{
> +    const struct nbrec_logical_router *lr;
> +    const char *nat_type = ctx->argv[2];
> +    const char *external_ip = ctx->argv[3];
> +    const char *logical_ip = ctx->argv[4];
> +    char *new_logical_ip = NULL;
> +
> +    lr = lr_by_name_or_uuid(ctx, ctx->argv[1], true);
> +
> +    if (strcmp(nat_type, "dnat") && strcmp(nat_type, "snat")
> +            && strcmp(nat_type, "dnat_and_snat")) {
> +        ctl_fatal("%s: type must be one of \"dnat\", \"snat\" and "
> +                "\"dnat_and_snat\".", nat_type);
> +    }
> +
> +    ovs_be32 ipv4 = 0;
> +    unsigned int plen;
> +    if (!ip_parse(external_ip, &ipv4)) {
> +        ctl_fatal("%s: should be an IPv4 address.", external_ip);
> +    }
> +
> +    if (strcmp("snat", nat_type)) {
> +        if (!ip_parse(logical_ip, &ipv4)) {
> +            ctl_fatal("%s: should be an IPv4 address.", logical_ip);
> +        }
> +        new_logical_ip = xstrdup(logical_ip);
> +    } else {
> +        char *error = ip_parse_cidr(logical_ip, &ipv4, &plen);
> +        if (error) {
> +            free(error);
> +            ctl_fatal("%s: should be an IPv4 address or network.",
> +                    logical_ip);
> +        }
> +        new_logical_ip = normalize_ipv4_prefix(ipv4, plen);
> +    }
> +
> +    bool may_exist = shash_find(&ctx->options, "--may-exist") != NULL;
> +    int is_snat = !strcmp("snat", nat_type);
> +    for (size_t i = 0; i < lr->n_nat; i++) {
> +        const struct nbrec_nat *nat = lr->nat[i];
> +        if (!strcmp(nat_type, nat->type)) {
> +            if (!strcmp(is_snat ? new_logical_ip : external_ip,
> +                        is_snat ? nat->logical_ip : nat->external_ip)) {
> +                if (!strcmp(is_snat ? external_ip : new_logical_ip,
> +                            is_snat ? nat->external_ip :
> nat->logical_ip)) {
>

Is there any reason why we need the is_snat check in the above 2
conditions? It looks to me that we check the logical_ip and external_ip for
all cases. Why is it important what the outer condition or inner condition
is?

Otherwise, this looks good to me.



> +                        if (may_exist) {
> +                            free(new_logical_ip);
> +                            return;
> +                        }
> +                        ctl_fatal("%s, %s: a NAT with this external_ip
> and "
> +                                "logical_ip already exists",
> +                                external_ip, new_logical_ip);
> +                } else {
> +                        ctl_fatal("a NAT with this type (%s) and %s (%s) "
> +                                "already exists",
> +                                nat_type,
> +                                is_snat ? "logical_ip" : "external_ip",
> +                                is_snat ? new_logical_ip : external_ip);
> +                }
> +            }
> +        }
> +    }
> +
> +    /* Create the NAT. */
> +    struct nbrec_nat *nat = nbrec_nat_insert(ctx->txn);
> +    nbrec_nat_set_type(nat, nat_type);
> +    nbrec_nat_set_external_ip(nat, external_ip);
> +    nbrec_nat_set_logical_ip(nat, new_logical_ip);
> +    free(new_logical_ip);
> +
> +    /* Insert the NAT into the logical router. */
> +    nbrec_logical_router_verify_nat(lr);
> +    struct nbrec_nat **new_nats = xmalloc(sizeof *new_nats * (lr->n_nat +
> 1));
> +    memcpy(new_nats, lr->nat, sizeof *new_nats * lr->n_nat);
> +    new_nats[lr->n_nat] = nat;
> +    nbrec_logical_router_set_nat(lr, new_nats, lr->n_nat + 1);
> +    free(new_nats);
> +}
> +
> +static void
> +nbctl_lr_nat_del(struct ctl_context *ctx)
> +{
> +    const struct nbrec_logical_router *lr;
> +    bool must_exist = !shash_find(&ctx->options, "--if-exists");
> +    lr = lr_by_name_or_uuid(ctx, ctx->argv[1], true);
> +
> +    if (ctx->argc == 2) {
> +        /* If type, external_ip and logical_ip are not specified, delete
> +         * all NATs. */
> +        nbrec_logical_router_verify_nat(lr);
> +        nbrec_logical_router_set_nat(lr, NULL, 0);
> +        return;
> +    }
> +
> +    const char *nat_type = ctx->argv[2];
> +    if (strcmp(nat_type, "dnat") && strcmp(nat_type, "snat")
> +            && strcmp(nat_type, "dnat_and_snat")) {
> +        ctl_fatal("%s: type must be one of \"dnat\", \"snat\" and "
> +                "\"dnat_and_snat\".", nat_type);
> +    }
> +
> +    if (ctx->argc == 3) {
> +        /*Deletes all NATs with the specified type. */
> +        struct nbrec_nat **new_nats = xmalloc(sizeof *new_nats *
> lr->n_nat);
> +        int n_nat = 0;
> +        for (size_t i = 0; i < lr->n_nat; i++) {
> +            if (strcmp(nat_type, lr->nat[i]->type)) {
> +                new_nats[n_nat++] = lr->nat[i];
> +            }
> +        }
> +
> +        nbrec_logical_router_verify_nat(lr);
> +        nbrec_logical_router_set_nat(lr, new_nats, n_nat);
> +        free(new_nats);
> +        return;
> +    }
> +
> +    const char *nat_ip = ctx->argv[3];
> +    int is_snat = !strcmp("snat", nat_type);
> +    /* Remove the matching NAT. */
> +    for (size_t i = 0; i < lr->n_nat; i++) {
> +        struct nbrec_nat *nat = lr->nat[i];
> +        if (!strcmp(nat_type, nat->type) &&
> +             !strcmp(nat_ip, is_snat ? nat->logical_ip :
> nat->external_ip)) {
> +            struct nbrec_nat **new_nats
> +                = xmemdup(lr->nat, sizeof *new_nats * lr->n_nat);
> +            new_nats[i] = lr->nat[lr->n_nat - 1];
> +            nbrec_logical_router_verify_nat(lr);
> +            nbrec_logical_router_set_nat(lr, new_nats,
> +                                          lr->n_nat - 1);
> +            free(new_nats);
> +            return;
> +        }
> +    }
> +
> +    if (must_exist) {
> +        ctl_fatal("no matching NAT with the type (%s) and %s (%s)",
> +                nat_type, is_snat ? "logical_ip" : "external_ip", nat_ip);
> +    }
> +}
> +
> +static void
> +nbctl_lr_nat_list(struct ctl_context *ctx)
> +{
> +    const struct nbrec_logical_router *lr;
> +    lr = lr_by_name_or_uuid(ctx, ctx->argv[1], true);
> +
> +    struct smap lr_nats = SMAP_INITIALIZER(&lr_nats);
> +    for (size_t i = 0; i < lr->n_nat; i++) {
> +        const struct nbrec_nat *nat = lr->nat[i];
> +        smap_add_format(&lr_nats, nat->type, "%-19.15s%s",
> +                        nat->external_ip, nat->logical_ip);
> +    }
> +
> +    const struct smap_node **nodes = smap_sort(&lr_nats);
> +    if (nodes) {
> +        ds_put_format(&ctx->output, "%-17.13s%-19.15s%s\n",
> +                "TYPE", "EXTERNAL_IP", "LOGICAL_IP");
> +        for (size_t i = 0; i < smap_count(&lr_nats); i++) {
> +            const struct smap_node *node = nodes[i];
> +            ds_put_format(&ctx->output, "%-17.13s%s\n",
> +                    node->key, node->value);
> +        }
> +        free(nodes);
> +    }
> +    smap_destroy(&lr_nats);
> +}
> +
>
>  static const struct nbrec_logical_router_port *
>  lrp_by_name_or_uuid(struct ctl_context *ctx, const char *id, bool
> must_exist)
> @@ -2954,6 +3132,13 @@ static const struct ctl_command_syntax
> nbctl_commands[] = {
>      { "lr-route-list", 1, 1, "ROUTER", NULL, nbctl_lr_route_list, NULL,
>        "", RO },
>
> +    /* NAT commands. */
> +    { "lr-nat-add", 4, 4, "ROUTER TYPE EXTERNAL_IP LOGICAL_IP", NULL,
> +      nbctl_lr_nat_add, NULL, "--may-exist", RW },
> +    { "lr-nat-del", 1, 3, "ROUTER [TYPE [IP]]", NULL,
> +        nbctl_lr_nat_del, NULL, "--if-exists", RW },
> +    { "lr-nat-list", 1, 1, "ROUTER", NULL, nbctl_lr_nat_list, NULL, "",
> RO },
> +
>      /* load balancer commands. */
>      { "lb-add", 3, 4, "LB VIP[:PORT] IP[:PORT]... [PROTOCOL]", NULL,
>        nbctl_lb_add, NULL, "--may-exist,--add-duplicate", RW },
> diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at
> index af00dad..96269f8 100644
> --- a/tests/ovn-nbctl.at
> +++ b/tests/ovn-nbctl.at
> @@ -238,6 +238,120 @@ OVN_NBCTL_TEST_STOP
>  AT_CLEANUP
>
>  dnl ---------------------------------------------------------------------
> +AT_SETUP([ovn-nbctl - NATs])
> +OVN_NBCTL_TEST_START
> +AT_CHECK([ovn-nbctl lr-add lr0])
> +AT_CHECK([ovn-nbctl lr-nat-add lr0 snatt 30.0.0.2 192.168.1.2], [1], [],
> +[ovn-nbctl: snatt: type must be one of "dnat", "snat" and "dnat_and_snat".
> +])
> +AT_CHECK([ovn-nbctl lr-nat-add lr0 snat 30.0.0.2a 192.168.1.2], [1], [],
> +[ovn-nbctl: 30.0.0.2a: should be an IPv4 address.
> +])
> +AT_CHECK([ovn-nbctl lr-nat-add lr0 snat 30.0.0 192.168.1.2], [1], [],
> +[ovn-nbctl: 30.0.0: should be an IPv4 address.
> +])
> +AT_CHECK([ovn-nbctl lr-nat-add lr0 snat 30.0.0.2/24 192.168.1.2], [1],
> [],
> +[ovn-nbctl: 30.0.0.2/24: should be an IPv4 address.
> +])
> +AT_CHECK([ovn-nbctl lr-nat-add lr0 snat 30.0.0.2:80 192.168.1.2], [1],
> [],
> +[ovn-nbctl: 30.0.0.2:80: should be an IPv4 address.
> +])
> +AT_CHECK([ovn-nbctl lr-nat-add lr0 snat 30.0.0.2 192.168.1.2a], [1], [],
> +[ovn-nbctl: 192.168.1.2a: should be an IPv4 address or network.
> +])
> +AT_CHECK([ovn-nbctl lr-nat-add lr0 snat 30.0.0.2 192.168.1], [1], [],
> +[ovn-nbctl: 192.168.1: should be an IPv4 address or network.
> +])
> +AT_CHECK([ovn-nbctl lr-nat-add lr0 snat 30.0.0.2 192.168.1.2:80], [1],
> [],
> +[ovn-nbctl: 192.168.1.2:80: should be an IPv4 address or network.
> +])
> +AT_CHECK([ovn-nbctl lr-nat-add lr0 snat 30.0.0.2 192.168.1.2/a], [1], [],
> +[ovn-nbctl: 192.168.1.2/a: should be an IPv4 address or network.
> +])
> +AT_CHECK([ovn-nbctl lr-nat-add lr0 dnat 30.0.0.2 192.168.1.2a], [1], [],
> +[ovn-nbctl: 192.168.1.2a: should be an IPv4 address.
> +])
> +AT_CHECK([ovn-nbctl lr-nat-add lr0 dnat 30.0.0.2 192.168.1], [1], [],
> +[ovn-nbctl: 192.168.1: should be an IPv4 address.
> +])
> +AT_CHECK([ovn-nbctl lr-nat-add lr0 dnat 30.0.0.2 192.168.1.2:80], [1],
> [],
> +[ovn-nbctl: 192.168.1.2:80: should be an IPv4 address.
> +])
> +AT_CHECK([ovn-nbctl lr-nat-add lr0 dnat 30.0.0.2 192.168.1.2/24], [1],
> [],
> +[ovn-nbctl: 192.168.1.2/24: should be an IPv4 address.
> +])
> +AT_CHECK([ovn-nbctl lr-nat-add lr0 dnat_and_snat 30.0.0.2 192.168.1.2/24],
> [1], [],
> +[ovn-nbctl: 192.168.1.2/24: should be an IPv4 address.
> +])
> +
> +dnl Add snat and dnat
> +AT_CHECK([ovn-nbctl lr-nat-add lr0 snat 30.0.0.1 192.168.1.0/24])
> +AT_CHECK([ovn-nbctl lr-nat-add lr0 dnat 30.0.0.1 192.168.1.2])
> +AT_CHECK([ovn-nbctl lr-nat-add lr0 dnat_and_snat 30.0.0.1 192.168.1.2])
> +AT_CHECK([ovn-nbctl lr-nat-list lr0], [0], [dnl
> +TYPE             EXTERNAL_IP        LOGICAL_IP
> +dnat             30.0.0.1           192.168.1.2
> +dnat_and_snat    30.0.0.1           192.168.1.2
> +snat             30.0.0.1           192.168.1.0/24
> +])
> +AT_CHECK([ovn-nbctl lr-nat-add lr0 snat 30.0.0.1 192.168.1.0/24], [1],
> [],
> +[ovn-nbctl: 30.0.0.1, 192.168.1.0/24: a NAT with this external_ip and
> logical_ip already exists
> +])
> +AT_CHECK([ovn-nbctl lr-nat-add lr0 snat 30.0.0.1 192.168.1.10/24], [1],
> [],
> +[ovn-nbctl: 30.0.0.1, 192.168.1.0/24: a NAT with this external_ip and
> logical_ip already exists
> +])
> +AT_CHECK([ovn-nbctl --may-exist lr-nat-add lr0 snat 30.0.0.1
> 192.168.1.0/24])
> +AT_CHECK([ovn-nbctl lr-nat-add lr0 snat 30.0.0.2 192.168.1.0/24], [1],
> [],
> +[ovn-nbctl: a NAT with this type (snat) and logical_ip (192.168.1.0/24)
> already exists
> +])
> +AT_CHECK([ovn-nbctl lr-nat-add lr0 dnat 30.0.0.1 192.168.1.2], [1], [],
> +[ovn-nbctl: 30.0.0.1, 192.168.1.2: a NAT with this external_ip and
> logical_ip already exists
> +])
> +AT_CHECK([ovn-nbctl --may-exist lr-nat-add lr0 dnat 30.0.0.1 192.168.1.2])
> +AT_CHECK([ovn-nbctl lr-nat-add lr0 dnat 30.0.0.1 192.168.1.3], [1], [],
> +[ovn-nbctl: a NAT with this type (dnat) and external_ip (30.0.0.1)
> already exists
> +])
> +AT_CHECK([ovn-nbctl lr-nat-add lr0 dnat_and_snat 30.0.0.1 192.168.1.2],
> [1], [],
> +[ovn-nbctl: 30.0.0.1, 192.168.1.2: a NAT with this external_ip and
> logical_ip already exists
> +])
> +AT_CHECK([ovn-nbctl --may-exist lr-nat-add lr0 dnat_and_snat 30.0.0.1
> 192.168.1.2])
> +AT_CHECK([ovn-nbctl lr-nat-add lr0 dnat_and_snat 30.0.0.1 192.168.1.3],
> [1], [],
> +[ovn-nbctl: a NAT with this type (dnat_and_snat) and external_ip
> (30.0.0.1) already exists
> +])
> +
> +dnl Deletes the NATs
> +AT_CHECK([ovn-nbctl lr-nat-del lr0 dnat_and_snat 30.0.0.2], [1], [],
> +[ovn-nbctl: no matching NAT with the type (dnat_and_snat) and external_ip
> (30.0.0.2)
> +])
> +AT_CHECK([ovn-nbctl lr-nat-del lr0 dnat 30.0.0.2], [1], [],
> +[ovn-nbctl: no matching NAT with the type (dnat) and external_ip
> (30.0.0.2)
> +])
> +AT_CHECK([ovn-nbctl lr-nat-del lr0 snat 192.168.10.0/24], [1], [],
> +[ovn-nbctl: no matching NAT with the type (snat) and logical_ip (
> 192.168.10.0/24)
> +])
> +AT_CHECK([ovn-nbctl --if-exists lr-nat-del lr0 snat 192.168.10.0/24])
> +
> +AT_CHECK([ovn-nbctl lr-nat-del lr0 dnat_and_snat 30.0.0.1])
> +AT_CHECK([ovn-nbctl lr-nat-list lr0], [0], [dnl
> +TYPE             EXTERNAL_IP        LOGICAL_IP
> +dnat             30.0.0.1           192.168.1.2
> +snat             30.0.0.1           192.168.1.0/24
> +])
> +
> +AT_CHECK([ovn-nbctl lr-nat-del lr0 dnat])
> +AT_CHECK([ovn-nbctl lr-nat-list lr0], [0], [dnl
> +TYPE             EXTERNAL_IP        LOGICAL_IP
> +snat             30.0.0.1           192.168.1.0/24
> +])
> +
> +AT_CHECK([ovn-nbctl lr-nat-del lr0])
> +AT_CHECK([ovn-nbctl lr-nat-list lr0], [0], [])
> +AT_CHECK([ovn-nbctl lr-nat-del lr0])
> +AT_CHECK([ovn-nbctl lr-nat-del lr0 dnat])
> +OVN_NBCTL_TEST_STOP
> +AT_CLEANUP
> +
> +dnl ---------------------------------------------------------------------
>
>  AT_SETUP([ovn-nbctl - LBs])
>  OVN_NBCTL_TEST_START
> --
> 1.8.3.1
>
>
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>

Hi Guru Shetty
The logical_ip address or network should be SNATed to an external_ip in a gw router, then it's unnecessary that the logical_ip is mapped to 
multiple external IPs. The external_ip should also be DNATed to a logical_ip address, then it's unnecessary that external_ip is mapped to 
multiple logical IPs.

The first condition check the logical ip or external ip according to nat type. If first condition check return true,
we check external ip or logical ip for giving more details about the reason for the error.


Thanks.
Nick

> On Nov 19, 2016, at 1:42 AM, Guru Shetty <guru@ovn.org> wrote:
> 
> Is there any reason why we need the is_snat check in the above 2 conditions? It looks to me that we check the logical_ip and external_ip for all cases. Why is it important what the outer condition or inner condition is?
> 
> Otherwise, this looks good to me.

On 21 November 2016 at 07:56, nickcooper-zhangtonghao <nic@opencloud.tech>
wrote:

> Hi Guru Shetty
> The logical_ip address or network should be SNATed to an external_ip in a
> gw router, then it's unnecessary that the logical_ip is mapped to
> multiple external IPs. The external_ip should also be DNATed to a
> logical_ip address, then it's unnecessary that external_ip is mapped to
> multiple logical IPs.
>
> The first condition check the logical ip or external ip according to nat
> type. If first condition check return true,
> we check external ip or logical ip for giving more details about the
> reason for the error.
>

Thanks for the explanation. I understand it now. I applied this.


>
>
> Thanks.
> Nick
>
> > On Nov 19, 2016, at 1:42 AM, Guru Shetty <guru@ovn.org> wrote:
> >
> > Is there any reason why we need the is_snat check in the above 2
> conditions? It looks to me that we check the logical_ip and external_ip for
> all cases. Why is it important what the outer condition or inner condition
> is?
> >
> > Otherwise, this looks good to me.
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>