diff mbox

[ovs-dev,v11,4/5] ovn: ovn-nbctl commands for distributed NAT

Message ID 1485046342-24044-5-git-send-email-mickeys.dev@gmail.com
State Superseded
Headers show

Commit Message

Mickey Spiegel Jan. 22, 2017, 12:52 a.m. UTC
This patch adds the new optional arguments "logical_port" and
"external_mac" to lr-nat-add, and displays that information in
lr-nat-list.

Signed-off-by: Mickey Spiegel <mickeys.dev@gmail.com>
---
 ovn/utilities/ovn-nbctl.8.xml | 27 +++++++++++++++++++---
 ovn/utilities/ovn-nbctl.c     | 54 +++++++++++++++++++++++++++++++++++++------
 tests/ovn-nbctl.at            | 47 +++++++++++++++++++++++++++++++++----
 tests/system-ovn.at           | 30 +++++-------------------
 4 files changed, 119 insertions(+), 39 deletions(-)

Comments

Gurucharan Shetty Jan. 26, 2017, 5:20 p.m. UTC | #1
On 21 January 2017 at 16:52, Mickey Spiegel <mickeys.dev@gmail.com> wrote:

> This patch adds the new optional arguments "logical_port" and
> "external_mac" to lr-nat-add, and displays that information in
> lr-nat-list.
>
> Signed-off-by: Mickey Spiegel <mickeys.dev@gmail.com>
>


Acked-by: Gurucharan Shetty <guru@ovn.org>

On a different note, can the external_mac be the same as the logical_port's
mac? I guess, it does not matter. It may makes sense to add that
information in ovn-nb.



> ---
>  ovn/utilities/ovn-nbctl.8.xml | 27 +++++++++++++++++++---
>  ovn/utilities/ovn-nbctl.c     | 54 ++++++++++++++++++++++++++++++
> +++++++------
>  tests/ovn-nbctl.at            | 47 +++++++++++++++++++++++++++++++++----
>  tests/system-ovn.at           | 30 +++++-------------------
>  4 files changed, 119 insertions(+), 39 deletions(-)
>
> diff --git a/ovn/utilities/ovn-nbctl.8.xml b/ovn/utilities/ovn-nbctl.8.xml
> index f95b88d..d81e99f 100644
> --- a/ovn/utilities/ovn-nbctl.8.xml
> +++ b/ovn/utilities/ovn-nbctl.8.xml
> @@ -444,7 +444,7 @@
>      <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>
> +      <dt>[<code>--may-exist</code>] <code>lr-nat-add</code>
> <var>router</var> <var>type</var> <var>external_ip</var>
> <var>logical_ip</var> [<var>logical_port</var> <var>external_mac</var>]</dt>
>        <dd>
>          <p>
>            Adds the specified NAT to <var>router</var>.
> @@ -453,6 +453,13 @@
>            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.
> +          The <var>logical_port</var> and <var>external_mac</var> are only
> +          accepted when <var>router</var> is a distributed router (rather
> +          than a gateway router) and <var>type</var> is
> +          <code>dnat_and_snat</code>.
> +          The <var>logical_port</var> is the name of an existing logical
> +          switch port where the <var>logical_ip</var> resides.
> +          The <var>external_mac</var> is an Ethernet address.
>          </p>
>          <p>
>            When <var>type</var> is <code>dnat</code>, the externally
> @@ -475,8 +482,22 @@
>            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.
> +          When the <var>logical_port</var> and <var>external_mac</var>
> +          are specified, the NAT rule will be programmed on the chassis
> +          where the <var>logical_port</var> resides.  This includes
> +          ARP replies for the <var>external_ip</var>, which return the
> +          value of <var>external_mac</var>.  All packets transmitted
> +          with source IP address equal to <var>external_ip</var> will
> +          be sent using the <var>external_mac</var>.
> +        </p>
> +        <p>
> +          It is an error if a NAT already exists with the same values
> +          of <var>router</var>, <var>type</var>, <var>external_ip</var>,
> +          and <var>logical_ip</var>, unless <code>--may-exist</code> is
> +          specified.  When <code>--may-exist</code>,
> +          <var>logical_port</var>, and <var>external_mac</var> are all
> +          specified, the existing values of <var>logical_port</var> and
> +          <var>external_mac</var> are overwritten.
>          </p>
>        </dd>
>
> diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
> index f0ff27a..3dac434 100644
> --- a/ovn/utilities/ovn-nbctl.c
> +++ b/ovn/utilities/ovn-nbctl.c
> @@ -390,7 +390,7 @@ Route commands:\n\
>    lr-route-list ROUTER      print routes for ROUTER\n\
>  \n\
>  NAT commands:\n\
> -  lr-nat-add ROUTER TYPE EXTERNAL_IP LOGICAL_IP\n\
> +  lr-nat-add ROUTER TYPE EXTERNAL_IP LOGICAL_IP [LOGICAL_PORT
> EXTERNAL_MAC]\n\
>                              add a NAT to ROUTER\n\
>    lr-nat-del ROUTER [TYPE [IP]]\n\
>                              remove NATs from ROUTER\n\
> @@ -2239,6 +2239,30 @@ nbctl_lr_nat_add(struct ctl_context *ctx)
>          new_logical_ip = normalize_ipv4_prefix(ipv4, plen);
>      }
>
> +    const char *logical_port;
> +    const char *external_mac;
> +    if (ctx->argc == 6) {
> +        ctl_fatal("lr-nat-add with logical_port "
> +                  "must also specify external_mac.");
> +    } else if (ctx->argc == 7) {
> +        if (strcmp(nat_type, "dnat_and_snat")) {
> +            ctl_fatal("logical_port and external_mac are only valid when "
> +                      "type is \"dnat_and_snat\".");
> +        }
> +
> +        logical_port = ctx->argv[5];
> +        lsp_by_name_or_uuid(ctx, logical_port, true);
> +
> +        external_mac = ctx->argv[6];
> +        struct eth_addr ea;
> +        if (!eth_addr_from_string(external_mac, &ea)) {
> +            ctl_fatal("invalid mac address %s.", external_mac);
> +        }
> +    } else {
> +        logical_port = NULL;
> +        external_mac = NULL;
> +    }
> +
>      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++) {
> @@ -2249,6 +2273,10 @@ nbctl_lr_nat_add(struct ctl_context *ctx)
>                  if (!strcmp(is_snat ? external_ip : new_logical_ip,
>                              is_snat ? nat->external_ip :
> nat->logical_ip)) {
>                          if (may_exist) {
> +                            nbrec_nat_verify_logical_port(nat);
> +                            nbrec_nat_verify_external_mac(nat);
> +                            nbrec_nat_set_logical_port(nat,
> logical_port);
> +                            nbrec_nat_set_external_mac(nat,
> external_mac);
>                              free(new_logical_ip);
>                              return;
>                          }
> @@ -2271,6 +2299,10 @@ nbctl_lr_nat_add(struct ctl_context *ctx)
>      nbrec_nat_set_type(nat, nat_type);
>      nbrec_nat_set_external_ip(nat, external_ip);
>      nbrec_nat_set_logical_ip(nat, new_logical_ip);
> +    if (logical_port && external_mac) {
> +        nbrec_nat_set_logical_port(nat, logical_port);
> +        nbrec_nat_set_external_mac(nat, external_mac);
> +    }
>      free(new_logical_ip);
>
>      /* Insert the NAT into the logical router. */
> @@ -2353,17 +2385,24 @@ nbctl_lr_nat_list(struct ctl_context *ctx)
>      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 char *key = xasprintf("%-17.13s%s", nat->type,
> nat->external_ip);
> +        if (nat->external_mac && nat->logical_port) {
> +            smap_add_format(&lr_nats, key, "%-22.18s%-21.17s%s",
> +                            nat->logical_ip, nat->external_mac,
> +                            nat->logical_port);
> +        } else {
> +            smap_add_format(&lr_nats, key, "%s", 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");
> +        ds_put_format(&ctx->output, "%-17.13s%-19.15s%-22.18s%-21.
> 17s%s\n",
> +                "TYPE", "EXTERNAL_IP", "LOGICAL_IP", "EXTERNAL_MAC",
> +                "LOGICAL_PORT");
>          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",
> +            ds_put_format(&ctx->output, "%-36.32s%s\n",
>                      node->key, node->value);
>          }
>          free(nodes);
> @@ -3314,7 +3353,8 @@ static const struct ctl_command_syntax
> nbctl_commands[] = {
>        "", RO },
>
>      /* NAT commands. */
> -    { "lr-nat-add", 4, 4, "ROUTER TYPE EXTERNAL_IP LOGICAL_IP", NULL,
> +    { "lr-nat-add", 4, 6,
> +      "ROUTER TYPE EXTERNAL_IP LOGICAL_IP [LOGICAL_PORT EXTERNAL_MAC]",
> 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 },
> diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at
> index 164c81a..cec516f 100644
> --- a/tests/ovn-nbctl.at
> +++ b/tests/ovn-nbctl.at
> @@ -283,15 +283,34 @@ AT_CHECK([ovn-nbctl lr-nat-add lr0 dnat 30.0.0.2
> 192.168.1.2/24], [1], [],
>  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.
>  ])
> +AT_CHECK([ovn-nbctl lr-nat-add lr0 dnat_and_snat 30.0.0.2 192.168.1.2
> lp0], [1], [],
> +[ovn-nbctl: lr-nat-add with logical_port must also specify external_mac.
> +])
> +AT_CHECK([ovn-nbctl lr-nat-add lr0 dnat 30.0.0.2 192.168.1.2 lp0
> 00:00:00:01:02:03], [1], [],
> +[ovn-nbctl: logical_port and external_mac are only valid when type is
> "dnat_and_snat".
> +])
> +AT_CHECK([ovn-nbctl lr-nat-add lr0 snat 30.0.0.2 192.168.1.2 lp0
> 00:00:00:01:02:03], [1], [],
> +[ovn-nbctl: logical_port and external_mac are only valid when type is
> "dnat_and_snat".
> +])
> +AT_CHECK([ovn-nbctl lr-nat-add lr0 dnat_and_snat 30.0.0.2 192.168.1.2 lp0
> 00:00:00:01:02:03], [1], [],
> +[ovn-nbctl: lp0: port name not found
> +])
> +AT_CHECK([ovn-nbctl ls-add ls0])
> +AT_CHECK([ovn-nbctl lsp-add ls0 lp0])
> +AT_CHECK([ovn-nbctl lr-nat-add lr0 dnat_and_snat 30.0.0.2 192.168.1.2 lp0
> 00:00:00:01:02], [1], [],
> +[ovn-nbctl: invalid mac address 00:00:00:01:02.
> +])
>
>  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-add lr0 dnat_and_snat 30.0.0.2 192.168.1.3 lp0
> 00:00:00:01:02:03])
>  AT_CHECK([ovn-nbctl lr-nat-list lr0], [0], [dnl
> -TYPE             EXTERNAL_IP        LOGICAL_IP
> +TYPE             EXTERNAL_IP        LOGICAL_IP            EXTERNAL_MAC
>      LOGICAL_PORT
>  dnat             30.0.0.1           192.168.1.2
>  dnat_and_snat    30.0.0.1           192.168.1.2
> +dnat_and_snat    30.0.0.2           192.168.1.3
>  00:00:00:01:02:03    lp0
>  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],
> [],
> @@ -318,10 +337,26 @@ AT_CHECK([ovn-nbctl --may-exist lr-nat-add lr0
> dnat_and_snat 30.0.0.1 192.168.1.
>  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
>  ])
> +AT_CHECK([ovn-nbctl --may-exist lr-nat-add lr0 dnat_and_snat 30.0.0.2
> 192.168.1.3 lp0 00:00:00:04:05:06])
> +AT_CHECK([ovn-nbctl lr-nat-list lr0], [0], [dnl
> +TYPE             EXTERNAL_IP        LOGICAL_IP            EXTERNAL_MAC
>      LOGICAL_PORT
> +dnat             30.0.0.1           192.168.1.2
> +dnat_and_snat    30.0.0.1           192.168.1.2
> +dnat_and_snat    30.0.0.2           192.168.1.3
>  00:00:00:04:05:06    lp0
> +snat             30.0.0.1           192.168.1.0/24
> +])
> +AT_CHECK([ovn-nbctl --may-exist lr-nat-add lr0 dnat_and_snat 30.0.0.2
> 192.168.1.3])
> +AT_CHECK([ovn-nbctl lr-nat-list lr0], [0], [dnl
> +TYPE             EXTERNAL_IP        LOGICAL_IP            EXTERNAL_MAC
>      LOGICAL_PORT
> +dnat             30.0.0.1           192.168.1.2
> +dnat_and_snat    30.0.0.1           192.168.1.2
> +dnat_and_snat    30.0.0.2           192.168.1.3
> +snat             30.0.0.1           192.168.1.0/24
> +])
>
>  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_and_snat 30.0.0.3], [1], [],
> +[ovn-nbctl: no matching NAT with the type (dnat_and_snat) and external_ip
> (30.0.0.3)
>  ])
>  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)
> @@ -333,14 +368,16 @@ 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
> +TYPE             EXTERNAL_IP        LOGICAL_IP            EXTERNAL_MAC
>      LOGICAL_PORT
>  dnat             30.0.0.1           192.168.1.2
> +dnat_and_snat    30.0.0.2           192.168.1.3
>  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
> +TYPE             EXTERNAL_IP        LOGICAL_IP            EXTERNAL_MAC
>      LOGICAL_PORT
> +dnat_and_snat    30.0.0.2           192.168.1.3
>  snat             30.0.0.1           192.168.1.0/24
>  ])
>
> diff --git a/tests/system-ovn.at b/tests/system-ovn.at
> index 307cbb3..638ac56 100644
> --- a/tests/system-ovn.at
> +++ b/tests/system-ovn.at
> @@ -1145,20 +1145,11 @@ ovn-nbctl lsp-add alice alice1 \
>  -- lsp-set-addresses alice1 "f0:00:00:01:02:05 172.16.1.2"
>
>  # Add DNAT rules
> -ovn-nbctl -- --id=@nat create nat type="dnat_and_snat" \
> -    logical_ip=192.168.1.2 external_ip=172.16.1.3 \
> -    external_mac=\"00:00:02:02:03:04\" logical_port=foo1 \
> -    -- add logical_router R1 nat @nat
> -
> -ovn-nbctl -- --id=@nat create nat type="dnat_and_snat" \
> -    logical_ip=192.168.1.3 external_ip=172.16.1.4 \
> -    external_mac=\"00:00:02:02:03:05\" logical_port=foo2 \
> -    -- add logical_router R1 nat @nat
> +AT_CHECK([ovn-nbctl lr-nat-add R1 dnat_and_snat 172.16.1.3 192.168.1.2
> foo1 00:00:02:02:03:04])
> +AT_CHECK([ovn-nbctl lr-nat-add R1 dnat_and_snat 172.16.1.4 192.168.1.3
> foo2 00:00:02:02:03:05])
>
>  # Add a SNAT rule
> -ovn-nbctl -- --id=@nat create nat type="snat" \
> -    logical_ip=192.168.0.0/16 external_ip=172.16.1.1 \
> -    -- add logical_router R1 nat @nat
> +AT_CHECK([ovn-nbctl lr-nat-add R1 snat 172.16.1.1 192.168.0.0/16])
>
>  OVS_WAIT_UNTIL([ovs-ofctl dump-flows br-int | grep ct\( | grep nat])
>
> @@ -1300,20 +1291,11 @@ ovn-nbctl lsp-add alice alice1 \
>  -- lsp-set-addresses alice1 "f0:00:00:01:02:05 172.16.1.2"
>
>  # Add DNAT rules
> -ovn-nbctl -- --id=@nat create nat type="dnat_and_snat" \
> -    logical_ip=192.168.1.2 external_ip=172.16.1.3 \
> -    external_mac=\"00:00:02:02:03:04\" logical_port=foo1 \
> -    -- add logical_router R1 nat @nat
> -
> -ovn-nbctl -- --id=@nat create nat type="dnat_and_snat" \
> -    logical_ip=192.168.2.2 external_ip=172.16.1.4 \
> -    external_mac=\"00:00:02:02:03:05\" logical_port=bar1 \
> -    -- add logical_router R1 nat @nat
> +AT_CHECK([ovn-nbctl lr-nat-add R1 dnat_and_snat 172.16.1.3 192.168.1.2
> foo1 00:00:02:02:03:04])
> +AT_CHECK([ovn-nbctl lr-nat-add R1 dnat_and_snat 172.16.1.4 192.168.2.2
> bar1 00:00:02:02:03:05])
>
>  # Add a SNAT rule
> -ovn-nbctl -- --id=@nat create nat type="snat" \
> -    logical_ip=192.168.0.0/16 external_ip=172.16.1.1 \
> -    -- add logical_router R1 nat @nat
> +AT_CHECK([ovn-nbctl lr-nat-add R1 snat 172.16.1.1 192.168.0.0/16])
>
>  OVS_WAIT_UNTIL([ovs-ofctl dump-flows br-int | grep ct\( | grep nat])
>
> --
> 1.9.1
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
Mickey Spiegel Jan. 26, 2017, 5:50 p.m. UTC | #2
On Thu, Jan 26, 2017 at 9:20 AM, Guru Shetty <guru@ovn.org> wrote:

>
>
> On 21 January 2017 at 16:52, Mickey Spiegel <mickeys.dev@gmail.com> wrote:
>
>> This patch adds the new optional arguments "logical_port" and
>> "external_mac" to lr-nat-add, and displays that information in
>> lr-nat-list.
>>
>> Signed-off-by: Mickey Spiegel <mickeys.dev@gmail.com>
>>
>
>
> Acked-by: Gurucharan Shetty <guru@ovn.org>
>
> On a different note, can the external_mac be the same as the
> logical_port's mac? I guess, it does not matter. It may makes sense to add
> that information in ovn-nb.
>

It just needs to be unique on the logical switch with the localnet port. If
the logical_port's mac is globally unique, then sure that can be used. I
can add that to ovn-nb.

Mickey
diff mbox

Patch

diff --git a/ovn/utilities/ovn-nbctl.8.xml b/ovn/utilities/ovn-nbctl.8.xml
index f95b88d..d81e99f 100644
--- a/ovn/utilities/ovn-nbctl.8.xml
+++ b/ovn/utilities/ovn-nbctl.8.xml
@@ -444,7 +444,7 @@ 
     <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>
+      <dt>[<code>--may-exist</code>] <code>lr-nat-add</code> <var>router</var> <var>type</var> <var>external_ip</var> <var>logical_ip</var> [<var>logical_port</var> <var>external_mac</var>]</dt>
       <dd>
         <p>
           Adds the specified NAT to <var>router</var>.
@@ -453,6 +453,13 @@ 
           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.
+          The <var>logical_port</var> and <var>external_mac</var> are only
+          accepted when <var>router</var> is a distributed router (rather
+          than a gateway router) and <var>type</var> is
+          <code>dnat_and_snat</code>.
+          The <var>logical_port</var> is the name of an existing logical
+          switch port where the <var>logical_ip</var> resides.
+          The <var>external_mac</var> is an Ethernet address.
         </p>
         <p>
           When <var>type</var> is <code>dnat</code>, the externally
@@ -475,8 +482,22 @@ 
           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.
+          When the <var>logical_port</var> and <var>external_mac</var>
+          are specified, the NAT rule will be programmed on the chassis
+          where the <var>logical_port</var> resides.  This includes
+          ARP replies for the <var>external_ip</var>, which return the
+          value of <var>external_mac</var>.  All packets transmitted
+          with source IP address equal to <var>external_ip</var> will
+          be sent using the <var>external_mac</var>.
+        </p>
+        <p>
+          It is an error if a NAT already exists with the same values
+          of <var>router</var>, <var>type</var>, <var>external_ip</var>,
+          and <var>logical_ip</var>, unless <code>--may-exist</code> is
+          specified.  When <code>--may-exist</code>,
+          <var>logical_port</var>, and <var>external_mac</var> are all
+          specified, the existing values of <var>logical_port</var> and
+          <var>external_mac</var> are overwritten.
         </p>
       </dd>
 
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index f0ff27a..3dac434 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -390,7 +390,7 @@  Route commands:\n\
   lr-route-list ROUTER      print routes for ROUTER\n\
 \n\
 NAT commands:\n\
-  lr-nat-add ROUTER TYPE EXTERNAL_IP LOGICAL_IP\n\
+  lr-nat-add ROUTER TYPE EXTERNAL_IP LOGICAL_IP [LOGICAL_PORT EXTERNAL_MAC]\n\
                             add a NAT to ROUTER\n\
   lr-nat-del ROUTER [TYPE [IP]]\n\
                             remove NATs from ROUTER\n\
@@ -2239,6 +2239,30 @@  nbctl_lr_nat_add(struct ctl_context *ctx)
         new_logical_ip = normalize_ipv4_prefix(ipv4, plen);
     }
 
+    const char *logical_port;
+    const char *external_mac;
+    if (ctx->argc == 6) {
+        ctl_fatal("lr-nat-add with logical_port "
+                  "must also specify external_mac.");
+    } else if (ctx->argc == 7) {
+        if (strcmp(nat_type, "dnat_and_snat")) {
+            ctl_fatal("logical_port and external_mac are only valid when "
+                      "type is \"dnat_and_snat\".");
+        }
+
+        logical_port = ctx->argv[5];
+        lsp_by_name_or_uuid(ctx, logical_port, true);
+
+        external_mac = ctx->argv[6];
+        struct eth_addr ea;
+        if (!eth_addr_from_string(external_mac, &ea)) {
+            ctl_fatal("invalid mac address %s.", external_mac);
+        }
+    } else {
+        logical_port = NULL;
+        external_mac = NULL;
+    }
+
     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++) {
@@ -2249,6 +2273,10 @@  nbctl_lr_nat_add(struct ctl_context *ctx)
                 if (!strcmp(is_snat ? external_ip : new_logical_ip,
                             is_snat ? nat->external_ip : nat->logical_ip)) {
                         if (may_exist) {
+                            nbrec_nat_verify_logical_port(nat);
+                            nbrec_nat_verify_external_mac(nat);
+                            nbrec_nat_set_logical_port(nat, logical_port);
+                            nbrec_nat_set_external_mac(nat, external_mac);
                             free(new_logical_ip);
                             return;
                         }
@@ -2271,6 +2299,10 @@  nbctl_lr_nat_add(struct ctl_context *ctx)
     nbrec_nat_set_type(nat, nat_type);
     nbrec_nat_set_external_ip(nat, external_ip);
     nbrec_nat_set_logical_ip(nat, new_logical_ip);
+    if (logical_port && external_mac) {
+        nbrec_nat_set_logical_port(nat, logical_port);
+        nbrec_nat_set_external_mac(nat, external_mac);
+    }
     free(new_logical_ip);
 
     /* Insert the NAT into the logical router. */
@@ -2353,17 +2385,24 @@  nbctl_lr_nat_list(struct ctl_context *ctx)
     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 char *key = xasprintf("%-17.13s%s", nat->type, nat->external_ip);
+        if (nat->external_mac && nat->logical_port) {
+            smap_add_format(&lr_nats, key, "%-22.18s%-21.17s%s",
+                            nat->logical_ip, nat->external_mac,
+                            nat->logical_port);
+        } else {
+            smap_add_format(&lr_nats, key, "%s", 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");
+        ds_put_format(&ctx->output, "%-17.13s%-19.15s%-22.18s%-21.17s%s\n",
+                "TYPE", "EXTERNAL_IP", "LOGICAL_IP", "EXTERNAL_MAC",
+                "LOGICAL_PORT");
         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",
+            ds_put_format(&ctx->output, "%-36.32s%s\n",
                     node->key, node->value);
         }
         free(nodes);
@@ -3314,7 +3353,8 @@  static const struct ctl_command_syntax nbctl_commands[] = {
       "", RO },
 
     /* NAT commands. */
-    { "lr-nat-add", 4, 4, "ROUTER TYPE EXTERNAL_IP LOGICAL_IP", NULL,
+    { "lr-nat-add", 4, 6,
+      "ROUTER TYPE EXTERNAL_IP LOGICAL_IP [LOGICAL_PORT EXTERNAL_MAC]", 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 },
diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at
index 164c81a..cec516f 100644
--- a/tests/ovn-nbctl.at
+++ b/tests/ovn-nbctl.at
@@ -283,15 +283,34 @@  AT_CHECK([ovn-nbctl lr-nat-add lr0 dnat 30.0.0.2 192.168.1.2/24], [1], [],
 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.
 ])
+AT_CHECK([ovn-nbctl lr-nat-add lr0 dnat_and_snat 30.0.0.2 192.168.1.2 lp0], [1], [],
+[ovn-nbctl: lr-nat-add with logical_port must also specify external_mac.
+])
+AT_CHECK([ovn-nbctl lr-nat-add lr0 dnat 30.0.0.2 192.168.1.2 lp0 00:00:00:01:02:03], [1], [],
+[ovn-nbctl: logical_port and external_mac are only valid when type is "dnat_and_snat".
+])
+AT_CHECK([ovn-nbctl lr-nat-add lr0 snat 30.0.0.2 192.168.1.2 lp0 00:00:00:01:02:03], [1], [],
+[ovn-nbctl: logical_port and external_mac are only valid when type is "dnat_and_snat".
+])
+AT_CHECK([ovn-nbctl lr-nat-add lr0 dnat_and_snat 30.0.0.2 192.168.1.2 lp0 00:00:00:01:02:03], [1], [],
+[ovn-nbctl: lp0: port name not found
+])
+AT_CHECK([ovn-nbctl ls-add ls0])
+AT_CHECK([ovn-nbctl lsp-add ls0 lp0])
+AT_CHECK([ovn-nbctl lr-nat-add lr0 dnat_and_snat 30.0.0.2 192.168.1.2 lp0 00:00:00:01:02], [1], [],
+[ovn-nbctl: invalid mac address 00:00:00:01:02.
+])
 
 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-add lr0 dnat_and_snat 30.0.0.2 192.168.1.3 lp0 00:00:00:01:02:03])
 AT_CHECK([ovn-nbctl lr-nat-list lr0], [0], [dnl
-TYPE             EXTERNAL_IP        LOGICAL_IP
+TYPE             EXTERNAL_IP        LOGICAL_IP            EXTERNAL_MAC         LOGICAL_PORT
 dnat             30.0.0.1           192.168.1.2
 dnat_and_snat    30.0.0.1           192.168.1.2
+dnat_and_snat    30.0.0.2           192.168.1.3           00:00:00:01:02:03    lp0
 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], [],
@@ -318,10 +337,26 @@  AT_CHECK([ovn-nbctl --may-exist lr-nat-add lr0 dnat_and_snat 30.0.0.1 192.168.1.
 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
 ])
+AT_CHECK([ovn-nbctl --may-exist lr-nat-add lr0 dnat_and_snat 30.0.0.2 192.168.1.3 lp0 00:00:00:04:05:06])
+AT_CHECK([ovn-nbctl lr-nat-list lr0], [0], [dnl
+TYPE             EXTERNAL_IP        LOGICAL_IP            EXTERNAL_MAC         LOGICAL_PORT
+dnat             30.0.0.1           192.168.1.2
+dnat_and_snat    30.0.0.1           192.168.1.2
+dnat_and_snat    30.0.0.2           192.168.1.3           00:00:00:04:05:06    lp0
+snat             30.0.0.1           192.168.1.0/24
+])
+AT_CHECK([ovn-nbctl --may-exist lr-nat-add lr0 dnat_and_snat 30.0.0.2 192.168.1.3])
+AT_CHECK([ovn-nbctl lr-nat-list lr0], [0], [dnl
+TYPE             EXTERNAL_IP        LOGICAL_IP            EXTERNAL_MAC         LOGICAL_PORT
+dnat             30.0.0.1           192.168.1.2
+dnat_and_snat    30.0.0.1           192.168.1.2
+dnat_and_snat    30.0.0.2           192.168.1.3
+snat             30.0.0.1           192.168.1.0/24
+])
 
 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_and_snat 30.0.0.3], [1], [],
+[ovn-nbctl: no matching NAT with the type (dnat_and_snat) and external_ip (30.0.0.3)
 ])
 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)
@@ -333,14 +368,16 @@  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
+TYPE             EXTERNAL_IP        LOGICAL_IP            EXTERNAL_MAC         LOGICAL_PORT
 dnat             30.0.0.1           192.168.1.2
+dnat_and_snat    30.0.0.2           192.168.1.3
 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
+TYPE             EXTERNAL_IP        LOGICAL_IP            EXTERNAL_MAC         LOGICAL_PORT
+dnat_and_snat    30.0.0.2           192.168.1.3
 snat             30.0.0.1           192.168.1.0/24
 ])
 
diff --git a/tests/system-ovn.at b/tests/system-ovn.at
index 307cbb3..638ac56 100644
--- a/tests/system-ovn.at
+++ b/tests/system-ovn.at
@@ -1145,20 +1145,11 @@  ovn-nbctl lsp-add alice alice1 \
 -- lsp-set-addresses alice1 "f0:00:00:01:02:05 172.16.1.2"
 
 # Add DNAT rules
-ovn-nbctl -- --id=@nat create nat type="dnat_and_snat" \
-    logical_ip=192.168.1.2 external_ip=172.16.1.3 \
-    external_mac=\"00:00:02:02:03:04\" logical_port=foo1 \
-    -- add logical_router R1 nat @nat
-
-ovn-nbctl -- --id=@nat create nat type="dnat_and_snat" \
-    logical_ip=192.168.1.3 external_ip=172.16.1.4 \
-    external_mac=\"00:00:02:02:03:05\" logical_port=foo2 \
-    -- add logical_router R1 nat @nat
+AT_CHECK([ovn-nbctl lr-nat-add R1 dnat_and_snat 172.16.1.3 192.168.1.2 foo1 00:00:02:02:03:04])
+AT_CHECK([ovn-nbctl lr-nat-add R1 dnat_and_snat 172.16.1.4 192.168.1.3 foo2 00:00:02:02:03:05])
 
 # Add a SNAT rule
-ovn-nbctl -- --id=@nat create nat type="snat" \
-    logical_ip=192.168.0.0/16 external_ip=172.16.1.1 \
-    -- add logical_router R1 nat @nat
+AT_CHECK([ovn-nbctl lr-nat-add R1 snat 172.16.1.1 192.168.0.0/16])
 
 OVS_WAIT_UNTIL([ovs-ofctl dump-flows br-int | grep ct\( | grep nat])
 
@@ -1300,20 +1291,11 @@  ovn-nbctl lsp-add alice alice1 \
 -- lsp-set-addresses alice1 "f0:00:00:01:02:05 172.16.1.2"
 
 # Add DNAT rules
-ovn-nbctl -- --id=@nat create nat type="dnat_and_snat" \
-    logical_ip=192.168.1.2 external_ip=172.16.1.3 \
-    external_mac=\"00:00:02:02:03:04\" logical_port=foo1 \
-    -- add logical_router R1 nat @nat
-
-ovn-nbctl -- --id=@nat create nat type="dnat_and_snat" \
-    logical_ip=192.168.2.2 external_ip=172.16.1.4 \
-    external_mac=\"00:00:02:02:03:05\" logical_port=bar1 \
-    -- add logical_router R1 nat @nat
+AT_CHECK([ovn-nbctl lr-nat-add R1 dnat_and_snat 172.16.1.3 192.168.1.2 foo1 00:00:02:02:03:04])
+AT_CHECK([ovn-nbctl lr-nat-add R1 dnat_and_snat 172.16.1.4 192.168.2.2 bar1 00:00:02:02:03:05])
 
 # Add a SNAT rule
-ovn-nbctl -- --id=@nat create nat type="snat" \
-    logical_ip=192.168.0.0/16 external_ip=172.16.1.1 \
-    -- add logical_router R1 nat @nat
+AT_CHECK([ovn-nbctl lr-nat-add R1 snat 172.16.1.1 192.168.0.0/16])
 
 OVS_WAIT_UNTIL([ovs-ofctl dump-flows br-int | grep ct\( | grep nat])