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

Guru Shetty guru at ovn.org
Fri Nov 18 17:42:13 UTC 2016


On 11 October 2016 at 07:36, nickcooper-zhangtonghao <
nickcooper-zhangtonghao at 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 at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>


More information about the dev mailing list