"
On Thu, Mar 24, 2022 at 12:14 PM Abhiram Sangana
<sangana.abhi...@nutanix.com> wrote:
>
> Currently, if multiple distributed gateway ports (DGP) are configured
> on a logical router, NAT is disabled as part of commit 15348b7
> (northd: Multiple distributed gateway port support.)
>
> This patch adds a new column called "gateway_port" in NAT table that
> references a distributed gateway port in the Logical_Router_Port
> table. A NAT rule is only applied on matching packets entering or
> leaving the DGP configured for the rule, when a router has
> multiple DGPs.
>
> If a router has a single DGP, NAT rules are applied at the DGP even if
> the "gateway_port" column is not set. It is an error to not set this
> column for a NAT rule when the router has multiple DGPs.
>
> This patch also updates the NAT commands in ovn-nbctl to support the
> new column.
>
> Signed-off-by: Abhiram Sangana <sangana.abhi...@nutanix.com>
> ---
> NEWS | 1 +
> northd/northd.c | 184 +++++++++++++++++++++--------------
> northd/ovn-northd.8.xml | 27 +++---
> ovn-architecture.7.xml | 6 +-
> ovn-nb.ovsschema | 10 +-
> ovn-nb.xml | 37 ++++++-
> tests/ovn-nbctl.at | 197 +++++++++++++++++++++++++-------------
> tests/ovn-northd.at | 172 +++++++++++++++++++++++++++++++--
> tests/ovn.at | 2 +-
> utilities/ovn-nbctl.8.xml | 48 ++++++----
> utilities/ovn-nbctl.c | 170 +++++++++++++++++++++++++++-----
> 11 files changed, 647 insertions(+), 207 deletions(-)
Hi Abhiram,
Thanks for the patch. I've a few comments. Please see below.
>
> diff --git a/NEWS b/NEWS
> index 5ba9c3cf4..458db45a0 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -2,6 +2,7 @@ Post v22.03.0
> -------------
> - Support IGMP and MLD snooping on transit logical switches that connect
> different OVN Interconnection availability zones.
> + - Support NAT for logical routers with multiple distributed gateway ports.
>
> OVN v22.03.0 - XX XXX XXXX
> --------------------------
> diff --git a/northd/northd.c b/northd/northd.c
> index a2cf8d6fc..df890d97d 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -606,11 +606,11 @@ struct ovn_datapath {
>
> /* Applies to only logical router datapath.
> * True if logical router is a gateway router. i.e options:chassis is
> set.
> - * If this is true, then 'l3dgw_port' will be ignored. */
> + * If this is true, then 'l3dgw_ports' will be ignored. */
> bool is_gw_router;
>
> - /* OVN northd only needs to know about the logical router gateway port
> for
> - * NAT on a distributed router. The "distributed gateway ports" are
> + /* OVN northd only needs to know about logical router gateway ports for
> + * NAT/LB on a distributed router. The "distributed gateway ports" are
> * populated only when there is a gateway chassis or ha chassis group
> * specified for some of the ports on the logical router. Otherwise this
> * will be NULL. */
> @@ -2702,8 +2702,9 @@ join_logical_ports(struct northd_input *input_data,
> * by one or more IP addresses, and if the port is a distributed gateway
> * port, followed by 'is_chassis_resident("LPORT_NAME")', where the
> * LPORT_NAME is the name of the L3 redirect port or the name of the
> - * logical_port specified in a NAT rule. These strings include the
> - * external IP addresses of all NAT rules defined on that router, and all
> + * logical_port specified in a NAT rule. These strings include the
> + * external IP addresses of NAT rules defined on that router which have
> + * gateway_port not set or have gateway_port as the router port 'op', and all
> * of the IP addresses used in load balancer VIPs defined on that router.
> *
> * The caller must free each of the n returned strings with free(),
> @@ -2716,8 +2717,7 @@ get_nat_addresses(const struct ovn_port *op, size_t *n,
> bool routable_only,
> struct eth_addr mac;
> if (!op || !op->nbrp || !op->od || !op->od->nbr
> || (!op->od->nbr->n_nat && !op->od->has_lb_vip)
> - || !eth_addr_from_string(op->nbrp->mac, &mac)
> - || op->od->n_l3dgw_ports > 1) {
> + || !eth_addr_from_string(op->nbrp->mac, &mac)) {
> *n = n_nats;
> return NULL;
> }
> @@ -2746,6 +2746,12 @@ get_nat_addresses(const struct ovn_port *op, size_t
> *n, bool routable_only,
> continue;
> }
>
> + /* Not including external IP of NAT rules whose gateway_port is
> + * not 'op'. */
> + if (nat->gateway_port && nat->gateway_port != op->nbrp) {
> + continue;
> + }
> +
> /* Determine whether this NAT rule satisfies the conditions for
> * distributed NAT processing. */
> if (op->od->n_l3dgw_ports && !strcmp(nat->type, "dnat_and_snat")
> @@ -2816,9 +2822,9 @@ get_nat_addresses(const struct ovn_port *op, size_t *n,
> bool routable_only,
> if (central_ip_address) {
> /* Gratuitous ARP for centralized NAT rules on distributed gateway
> * ports should be restricted to the gateway chassis. */
> - if (op->od->n_l3dgw_ports) {
> + if (is_l3dgw_port(op)) {
> ds_put_format(&c_addresses, " is_chassis_resident(%s)",
> - op->od->l3dgw_ports[0]->cr_port->json_key);
> + op->cr_port->json_key);
> }
>
> addresses[n_nats++] = ds_steal_cstr(&c_addresses);
> @@ -3458,9 +3464,12 @@ ovn_port_update_sbrec(struct northd_input *input_data,
> }
>
> if (op->peer->od->n_l3dgw_ports) {
> + const struct ovn_port *l3dgw_port = (
> + is_l3dgw_port(op->peer)
> + ? op->peer
> + : op->peer->od->l3dgw_ports[0]);
> ds_put_format(&garp_info, " is_chassis_resident(%s)",
> - op->peer->od->l3dgw_ports[0]
> - ->cr_port->json_key);
> + l3dgw_port->cr_port->json_key);
> }
>
> n_nats++;
> @@ -10338,6 +10347,12 @@ build_lrouter_port_nat_arp_nd_flow(struct ovn_port
> *op,
> const struct nbrec_nat *nat = nat_entry->nb;
> struct ds match = DS_EMPTY_INITIALIZER;
>
> + /* ARP/ND should be sent from distributed gateway port specified in
> + * the NAT rule. */
> + if (nat->gateway_port && nat->gateway_port != op->nbrp) {
> + return;
> + }
> +
> /* Mac address to use when replying to ARP/NS. */
> const char *mac_s = REG_INPORT_ETH_ADDR;
> struct eth_addr mac;
> @@ -10361,10 +10376,9 @@ build_lrouter_port_nat_arp_nd_flow(struct ovn_port
> *op,
> * upstream MAC learning points to the gateway chassis.
> * Also need to avoid generation of multiple ARP responses
> * from different chassis. */
> - if (op->od->n_l3dgw_ports) {
> - ds_put_format(&match, "is_chassis_resident(%s)",
> - op->od->l3dgw_ports[0]->cr_port->json_key);
> - }
> + ovs_assert(is_l3dgw_port(op));
> + ds_put_format(&match, "is_chassis_resident(%s)",
> + op->cr_port->json_key);
> }
>
> /* Respond to ARP/NS requests on the chassis that binds the gw
> @@ -12110,7 +12124,7 @@ build_ipv6_input_flows_for_lrouter_port(
> struct ds *match, struct ds *actions,
> const struct shash *meter_groups)
> {
> - if (op->nbrp && (!op->l3dgw_port)) {
> + if (op->nbrp && !is_cr_port(op)) {
> /* No ingress packets are accepted on a chassisredirect
> * port, so no need to program flows for that port. */
> if (op->lrp_networks.n_ipv6_addrs) {
> @@ -12239,7 +12253,7 @@ build_ipv6_input_flows_for_lrouter_port(
> ds_clear(match);
> ds_clear(actions);
> ds_clear(&ip_ds);
> - if (op->od->n_l3dgw_ports && op->od->l3dgw_ports[0] == op) {
> + if (is_l3dgw_port(op)) {
> ds_put_cstr(&ip_ds, "ip6.dst <-> ip6.src");
> } else {
> ds_put_format(&ip_ds, "ip6.dst = ip6.src; ip6.src = %s",
> @@ -12329,7 +12343,7 @@ build_lrouter_ipv4_ip_input(struct ovn_port *op,
> {
> /* No ingress packets are accepted on a chassisredirect
> * port, so no need to program flows for that port. */
> - if (op->nbrp && (!op->l3dgw_port)) {
> + if (op->nbrp && !is_cr_port(op)) {
> if (op->lrp_networks.n_ipv4_addrs) {
> /* L3 admission control: drop packets that originate from an
> * IPv4 address owned by the router or a broadcast address
> @@ -12371,7 +12385,7 @@ build_lrouter_ipv4_ip_input(struct ovn_port *op,
> ds_clear(match);
> ds_clear(actions);
> ds_clear(&ip_ds);
> - if (op->od->n_l3dgw_ports && op->od->l3dgw_ports[0] == op) {
> + if (is_l3dgw_port(op)) {
> ds_put_cstr(&ip_ds, "ip4.dst <-> ip4.src");
> } else {
> ds_put_format(&ip_ds, "ip4.dst = ip4.src; ip4.src = %s",
> @@ -12616,7 +12630,8 @@ build_lrouter_ipv4_ip_input(struct ovn_port *op,
> static void
> build_lrouter_in_unsnat_flow(struct hmap *lflows, struct ovn_datapath *od,
> const struct nbrec_nat *nat, struct ds *match,
> - struct ds *actions, bool distributed, bool
> is_v6)
> + struct ds *actions, bool distributed, bool
> is_v6,
> + struct ovn_port *l3dgw_port)
> {
> /* Ingress UNSNAT table: It is for already established connections'
> * reverse traffic. i.e., SNAT has already been done in egress
> @@ -12655,12 +12670,12 @@ build_lrouter_in_unsnat_flow(struct hmap *lflows,
> struct ovn_datapath *od,
> ds_clear(actions);
> ds_put_format(match, "ip && ip%s.dst == %s && inport == %s && "
> "flags.loopback == 0", is_v6 ? "6" : "4",
> - nat->external_ip, od->l3dgw_ports[0]->json_key);
> + nat->external_ip, l3dgw_port->json_key);
> if (!distributed && od->n_l3dgw_ports) {
> /* Flows for NAT rules that are centralized are only
> * programmed on the gateway chassis. */
> ds_put_format(match, " && is_chassis_resident(%s)",
> - od->l3dgw_ports[0]->cr_port->json_key);
> + l3dgw_port->cr_port->json_key);
> }
>
> if (!strcmp(nat->type, "dnat_and_snat") && stateless) {
> @@ -12680,12 +12695,12 @@ build_lrouter_in_unsnat_flow(struct hmap *lflows,
> struct ovn_datapath *od,
> ds_put_format(match, "ip && ip%s.dst == %s && inport == %s && "
> "flags.loopback == 1 && flags.use_snat_zone == 1",
> is_v6 ? "6" : "4", nat->external_ip,
> - od->l3dgw_ports[0]->json_key);
> + l3dgw_port->json_key);
> if (!distributed && od->n_l3dgw_ports) {
> /* Flows for NAT rules that are centralized are only
> * programmed on the gateway chassis. */
> ds_put_format(match, " && is_chassis_resident(%s)",
> - od->l3dgw_ports[0]->cr_port->json_key);
> + l3dgw_port->cr_port->json_key);
> }
> ds_put_cstr(actions, "ct_snat;");
> ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_UNSNAT,
> @@ -12699,7 +12714,8 @@ static void
> build_lrouter_in_dnat_flow(struct hmap *lflows, struct ovn_datapath *od,
> const struct nbrec_nat *nat, struct ds *match,
> struct ds *actions, bool distributed,
> - ovs_be32 mask, bool is_v6)
> + ovs_be32 mask, bool is_v6,
> + struct ovn_port *l3dgw_port)
> {
> /* Ingress DNAT table: Packets enter the pipeline with destination
> * IP address that needs to be DNATted from a external IP address
> @@ -12751,12 +12767,12 @@ build_lrouter_in_dnat_flow(struct hmap *lflows,
> struct ovn_datapath *od,
> ds_clear(match);
> ds_put_format(match, "ip && ip%s.dst == %s && inport == %s",
> is_v6 ? "6" : "4", nat->external_ip,
> - od->l3dgw_ports[0]->json_key);
> + l3dgw_port->json_key);
> if (!distributed && od->n_l3dgw_ports) {
> /* Flows for NAT rules that are centralized are only
> * programmed on the gateway chassis. */
> ds_put_format(match, " && is_chassis_resident(%s)",
> - od->l3dgw_ports[0]->cr_port->json_key);
> + l3dgw_port->cr_port->json_key);
> }
> ds_clear(actions);
> if (nat->allowed_ext_ips || nat->exempted_ext_ips) {
> @@ -12786,7 +12802,8 @@ static void
> build_lrouter_out_undnat_flow(struct hmap *lflows, struct ovn_datapath *od,
> const struct nbrec_nat *nat, struct ds *match,
> struct ds *actions, bool distributed,
> - struct eth_addr mac, bool is_v6)
> + struct eth_addr mac, bool is_v6,
> + struct ovn_port *l3dgw_port)
> {
> /* Egress UNDNAT table: It is for already established connections'
> * reverse traffic. i.e., DNAT has already been done in ingress
> @@ -12803,12 +12820,12 @@ build_lrouter_out_undnat_flow(struct hmap *lflows,
> struct ovn_datapath *od,
> ds_clear(match);
> ds_put_format(match, "ip && ip%s.src == %s && outport == %s",
> is_v6 ? "6" : "4", nat->logical_ip,
> - od->l3dgw_ports[0]->json_key);
> + l3dgw_port->json_key);
> if (!distributed && od->n_l3dgw_ports) {
> /* Flows for NAT rules that are centralized are only
> * programmed on the gateway chassis. */
> ds_put_format(match, " && is_chassis_resident(%s)",
> - od->l3dgw_ports[0]->cr_port->json_key);
> + l3dgw_port->cr_port->json_key);
> }
> ds_clear(actions);
> if (distributed) {
> @@ -12834,7 +12851,7 @@ static void
> build_lrouter_out_is_dnat_local(struct hmap *lflows, struct ovn_datapath *od,
> const struct nbrec_nat *nat, struct ds
> *match,
> struct ds *actions, bool distributed,
> - bool is_v6)
> + bool is_v6, struct ovn_port *l3dgw_port)
> {
> /* Note that this only applies for NAT on a distributed router.
> */
> @@ -12849,7 +12866,7 @@ build_lrouter_out_is_dnat_local(struct hmap *lflows,
> struct ovn_datapath *od,
> ds_put_format(match, "is_chassis_resident(\"%s\")",
> nat->logical_port);
> } else {
> ds_put_format(match, "is_chassis_resident(%s)",
> - od->l3dgw_ports[0]->cr_port->json_key);
> + l3dgw_port->cr_port->json_key);
> }
>
> ds_clear(actions);
> @@ -12865,7 +12882,8 @@ build_lrouter_out_snat_flow(struct hmap *lflows,
> struct ovn_datapath *od,
> const struct nbrec_nat *nat, struct ds *match,
> struct ds *actions, bool distributed,
> struct eth_addr mac, ovs_be32 mask,
> - int cidr_bits, bool is_v6)
> + int cidr_bits, bool is_v6,
> + struct ovn_port *l3dgw_port)
> {
> /* Egress SNAT table: Packets enter the egress pipeline with
> * source ip address that needs to be SNATted to a external ip
> @@ -12912,7 +12930,7 @@ build_lrouter_out_snat_flow(struct hmap *lflows,
> struct ovn_datapath *od,
> ds_clear(match);
> ds_put_format(match, "ip && ip%s.src == %s && outport == %s",
> is_v6 ? "6" : "4", nat->logical_ip,
> - od->l3dgw_ports[0]->json_key);
> + l3dgw_port->json_key);
> if (od->n_l3dgw_ports) {
> if (distributed) {
> ovs_assert(nat->logical_port);
> @@ -12924,7 +12942,7 @@ build_lrouter_out_snat_flow(struct hmap *lflows,
> struct ovn_datapath *od,
> * programmed on the gateway chassis. */
> priority += 128;
> ds_put_format(match, " && is_chassis_resident(%s)",
> - od->l3dgw_ports[0]->cr_port->json_key);
> + l3dgw_port->cr_port->json_key);
> }
> }
> ds_clear(actions);
> @@ -12983,13 +13001,13 @@ build_lrouter_ingress_nat_check_pkt_len(struct hmap
> *lflows,
> const struct nbrec_nat *nat,
> struct ovn_datapath *od, bool is_v6,
> struct ds *match, struct ds *actions,
> - int mtu,
> + int mtu, struct ovn_port *l3dgw_port,
> const struct shash *meter_groups)
> {
> ds_clear(match);
> ds_put_format(match, "inport == %s && "REGBIT_PKT_LARGER
> " && "REGBIT_EGRESS_LOOPBACK" == 0",
> - od->l3dgw_ports[0]->json_key);
> + l3dgw_port->json_key);
>
> ds_clear(actions);
> if (!is_v6) {
> @@ -13010,7 +13028,7 @@ build_lrouter_ingress_nat_check_pkt_len(struct hmap
> *lflows,
> "outport = %s; flags.loopback = 1; output; };",
> nat->external_mac,
> nat->external_ip,
> - mtu, od->l3dgw_ports[0]->json_key);
> + mtu, l3dgw_port->json_key);
> ovn_lflow_add_with_hint__(lflows, od, S_ROUTER_IN_IP_INPUT, 160,
> ds_cstr(match), ds_cstr(actions),
> NULL,
> @@ -13037,7 +13055,7 @@ build_lrouter_ingress_nat_check_pkt_len(struct hmap
> *lflows,
> "outport = %s; flags.loopback = 1; output; };",
> nat->external_mac,
> nat->external_ip,
> - mtu, od->l3dgw_ports[0]->json_key);
> + mtu, l3dgw_port->json_key);
> ovn_lflow_add_with_hint__(lflows, od, S_ROUTER_IN_IP_INPUT, 160,
> ds_cstr(match), ds_cstr(actions),
> NULL,
> @@ -13054,13 +13072,14 @@ build_lrouter_ingress_flow(struct hmap *lflows,
> struct ovn_datapath *od,
> const struct nbrec_nat *nat, struct ds *match,
> struct ds *actions, struct eth_addr mac,
> bool distributed, bool is_v6,
> + struct ovn_port *l3dgw_port,
> const struct shash *meter_groups)
> {
> if (od->n_l3dgw_ports && !strcmp(nat->type, "snat")) {
> ds_clear(match);
> ds_put_format(
> match, "inport == %s && %s == %s",
> - od->l3dgw_ports[0]->json_key,
> + l3dgw_port->json_key,
> is_v6 ? "ip6.src" : "ip4.src", nat->external_ip);
> ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_IP_INPUT,
> 120, ds_cstr(match), "next;",
> @@ -13076,32 +13095,33 @@ build_lrouter_ingress_flow(struct hmap *lflows,
> struct ovn_datapath *od,
> * This will save us from having to match on inport further
> * down in the pipeline.
> */
> - int gw_mtu = smap_get_int(&od->l3dgw_ports[0]->nbrp->options,
> + int gw_mtu = smap_get_int(&l3dgw_port->nbrp->options,
> "gateway_mtu", 0);
> ds_clear(match);
> ds_put_format(match,
> "eth.dst == "ETH_ADDR_FMT" && inport == %s"
> " && is_chassis_resident(\"%s\")",
> ETH_ADDR_ARGS(mac),
> - od->l3dgw_ports[0]->json_key,
> + l3dgw_port->json_key,
> nat->logical_port);
> - build_gateway_mtu_flow(lflows, od->l3dgw_ports[0],
> + build_gateway_mtu_flow(lflows, l3dgw_port,
> S_ROUTER_IN_ADMISSION, 50, 55,
> match, actions, &nat->header_,
> REG_INPORT_ETH_ADDR " = %s; next;",
> - od->l3dgw_ports[0]->lrp_networks.ea_s);
> + l3dgw_port->lrp_networks.ea_s);
> if (gw_mtu) {
> build_lrouter_ingress_nat_check_pkt_len(lflows, nat, od, is_v6,
> match, actions, gw_mtu,
> - meter_groups);
> + l3dgw_port,
> meter_groups);
> }
> }
> }
>
> static int
> lrouter_check_nat_entry(struct ovn_datapath *od, const struct nbrec_nat *nat,
> - ovs_be32 *mask, bool *is_v6, int *cidr_bits,
> - struct eth_addr *mac, bool *distributed)
> + const struct hmap *ports, ovs_be32 *mask,
> + bool *is_v6, int *cidr_bits, struct eth_addr *mac,
> + bool *distributed, struct ovn_port **nat_l3dgw_port)
> {
> struct in6_addr ipv6, mask_v6, v6_exact = IN6ADDR_EXACT_INIT;
> ovs_be32 ip;
> @@ -13135,6 +13155,32 @@ lrouter_check_nat_entry(struct ovn_datapath *od,
> const struct nbrec_nat *nat,
> *is_v6 = true;
> }
>
> + /* Validate gateway_port of NAT rule. */
> + *nat_l3dgw_port = NULL;
> + if (nat->gateway_port == NULL) {
> + if (od->n_l3dgw_ports > 1) {
> + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
> + VLOG_WARN_RL(&rl, "NAT configured on logical router: %s with"
> + "multiple distributed gateway ports needs to
> specify"
> + "valid gateway_port.", od->nbr->name);
> + return -EINVAL;
> + } else if (od->n_l3dgw_ports) {
> + *nat_l3dgw_port = od->l3dgw_ports[0];
> + }
> + } else {
> + *nat_l3dgw_port = ovn_port_find(ports, nat->gateway_port->name);
> +
> + if (!(*nat_l3dgw_port) || (*nat_l3dgw_port)->od != od ||
> + !is_l3dgw_port(*nat_l3dgw_port)) {
> + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
> + VLOG_WARN_RL(&rl, "gateway_port: %s of NAT configured on "
> + "logical router: %s is not a valid distributed "
> + "gateway port on that router",
> + nat->gateway_port->name, od->nbr->name);
> + return -EINVAL;
> + }
> + }
> +
> /* Check the validity of nat->logical_ip. 'logical_ip' can
> * be a subnet when the type is "snat". */
> if (*is_v6) {
> @@ -13234,24 +13280,12 @@ build_lrouter_nat_defrag_and_lb(struct ovn_datapath
> *od, struct hmap *lflows,
> ovn_lflow_add(lflows, od, S_ROUTER_OUT_SNAT, 120, "nd_ns", "next;");
>
> /* NAT rules are only valid on Gateway routers and routers with
> - * l3dgw_port (router has a port with gateway chassis
> + * l3dgw_ports (router has port(s) with gateway chassis
> * specified). */
> if (!od->is_gw_router && !od->n_l3dgw_ports) {
> return;
> }
>
> - /* NAT rules are not currently supported on logical routers with multiple
> - * distributed gateway ports. */
> - if (od->n_l3dgw_ports > 1) {
> - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
> - VLOG_WARN_RL(&rl, "NAT is configured on logical router %s, which has
> %"
> - PRIuSIZE" distributed gateway ports. NAT is not
> supported"
> - " yet when there is more than one distributed gateway "
> - "port on the router.",
> - od->nbr->name, od->n_l3dgw_ports);
> - return;
> - }
> -
> struct sset nat_entries = SSET_INITIALIZER(&nat_entries);
>
> bool dnat_force_snat_ip =
> @@ -13265,18 +13299,19 @@ build_lrouter_nat_defrag_and_lb(struct ovn_datapath
> *od, struct hmap *lflows,
> bool is_v6, distributed;
> ovs_be32 mask;
> int cidr_bits;
> + struct ovn_port *l3dgw_port;
>
> - if (lrouter_check_nat_entry(od, nat, &mask, &is_v6, &cidr_bits,
> - &mac, &distributed) < 0) {
> + if (lrouter_check_nat_entry(od, nat, ports, &mask, &is_v6,
> &cidr_bits,
> + &mac, &distributed, &l3dgw_port) < 0) {
> continue;
> }
>
> /* S_ROUTER_IN_UNSNAT */
> build_lrouter_in_unsnat_flow(lflows, od, nat, match, actions,
> distributed,
> - is_v6);
> + is_v6, l3dgw_port);
> /* S_ROUTER_IN_DNAT */
> build_lrouter_in_dnat_flow(lflows, od, nat, match, actions,
> distributed,
> - mask, is_v6);
> + mask, is_v6, l3dgw_port);
>
> /* ARP resolve for NAT IPs. */
> if (od->is_gw_router) {
> @@ -13289,14 +13324,14 @@ build_lrouter_nat_defrag_and_lb(struct ovn_datapath
> *od, struct hmap *lflows,
> ds_clear(match);
> ds_put_format(
> match, "outport == %s && %s == %s",
> - od->l3dgw_ports[0]->json_key,
> + l3dgw_port->json_key,
> is_v6 ? REG_NEXT_HOP_IPV6 : REG_NEXT_HOP_IPV4,
> nat->external_ip);
> ds_clear(actions);
> ds_put_format(
> actions, "eth.dst = %s; next;",
> distributed ? nat->external_mac :
> - od->l3dgw_ports[0]->lrp_networks.ea_s);
> + l3dgw_port->lrp_networks.ea_s);
> ovn_lflow_add_with_hint(lflows, od,
> S_ROUTER_IN_ARP_RESOLVE,
> 100, ds_cstr(match),
> @@ -13308,18 +13343,19 @@ build_lrouter_nat_defrag_and_lb(struct ovn_datapath
> *od, struct hmap *lflows,
>
> /* S_ROUTER_OUT_DNAT_LOCAL */
> build_lrouter_out_is_dnat_local(lflows, od, nat, match, actions,
> - distributed, is_v6);
> + distributed, is_v6, l3dgw_port);
>
> /* S_ROUTER_OUT_UNDNAT */
> build_lrouter_out_undnat_flow(lflows, od, nat, match, actions,
> distributed,
> - mac, is_v6);
> + mac, is_v6, l3dgw_port);
> /* S_ROUTER_OUT_SNAT */
> build_lrouter_out_snat_flow(lflows, od, nat, match, actions,
> distributed,
> - mac, mask, cidr_bits, is_v6);
> + mac, mask, cidr_bits, is_v6, l3dgw_port);
>
> /* S_ROUTER_IN_ADMISSION - S_ROUTER_IN_IP_INPUT */
> - build_lrouter_ingress_flow(lflows, od, nat, match, actions,
> - mac, distributed, is_v6, meter_groups);
> + build_lrouter_ingress_flow(lflows, od, nat, match, actions, mac,
> + distributed, is_v6, l3dgw_port,
> + meter_groups);
>
> /* Ingress Gateway Redirect Table: For NAT on a distributed
> * router, add flows that are specific to a NAT rule. These
> @@ -13336,7 +13372,7 @@ build_lrouter_nat_defrag_and_lb(struct ovn_datapath
> *od, struct hmap *lflows,
> ds_put_format(match,
> "ip%s.src == %s && outport == %s",
> is_v6 ? "6" : "4", nat->logical_ip,
> - od->l3dgw_ports[0]->json_key);
> + l3dgw_port->json_key);
> /* Add a rule to drop traffic from a distributed NAT if
> * the virtual port has not claimed yet becaused otherwise
> * the traffic will be centralized misconfiguring the TOR switch.
> @@ -13369,10 +13405,10 @@ build_lrouter_nat_defrag_and_lb(struct ovn_datapath
> *od, struct hmap *lflows,
> ds_put_format(match, "ip%s.dst == %s && outport == %s",
> is_v6 ? "6" : "4",
> nat->external_ip,
> - od->l3dgw_ports[0]->json_key);
> + l3dgw_port->json_key);
> if (!distributed) {
> ds_put_format(match, " && is_chassis_resident(%s)",
> - od->l3dgw_ports[0]->cr_port->json_key);
> + l3dgw_port->cr_port->json_key);
> } else {
> ds_put_format(match, " && is_chassis_resident(\"%s\")",
> nat->logical_port);
> diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
> index 4784bff04..5f7891b65 100644
> --- a/northd/ovn-northd.8.xml
> +++ b/northd/ovn-northd.8.xml
> @@ -3026,7 +3026,8 @@ icmp6 {
> <code>ip &&
> ip6.dst == <var>B</var> && inport == <var>GW</var>
> && flags.loopback == 0</code>
> - where <var>GW</var> is the logical router gateway port, with an
> + where <var>GW</var> is the distributed gateway port
> + specified in the NAT rule, with an
> action <code>ct_snat_in_czone;</code> to unSNAT in the common
> zone. If the NAT rule is of type dnat_and_snat and has
> <code>stateless=true</code> in the options, then the action
> @@ -3051,7 +3052,8 @@ icmp6 {
> ip6.dst == <var>B</var> && inport == <var>GW</var>
> && flags.loopback == 0 &&
> flags.use_snat_zone == 1</code>
> - where <var>GW</var> is the logical router gateway port, with an
> + where <var>GW</var> is the distributed gateway port
> + specified in the NAT rule, with an
> action <code>ct_snat;</code> to unSNAT in the snat zone. If the
> NAT rule is of type dnat_and_snat and has
> <code>stateless=true</code> in the options, then the action
> @@ -3332,9 +3334,10 @@ icmp6 {
> to change the destination IP address of a packet from <var>A</var>
> to
> <var>B</var>, a priority-100 flow matches <code>ip &&
> ip4.dst == <var>B</var> && inport == <var>GW</var></code>,
> - where <var>GW</var> is the logical router gateway port, with an
> - action <code>ct_dnat(<var>B</var>);</code>. The match will
> - include <code>ip6.dst == <var>B</var></code> in the IPv6 case.
> + where <var>GW</var> is the logical router gateway port configured
> + for the NAT rule, with an action
> + <code>ct_dnat(<var>B</var>);</code>. The match will include
> + <code>ip6.dst == <var>B</var></code> in the IPv6 case.
> If the NAT rule is of type dnat_and_snat and has
> <code>stateless=true</code> in the options, then the action
> would be <code>ip4/6.dst=(<var>B</var>)</code>.
> @@ -4165,10 +4168,11 @@ icmp6 {
> flow with match <code>ip4.src == <var>B</var> &&
> outport == <var>GW</var></code> &&
> is_chassis_resident(<var>P</var>), where <var>GW</var> is
> - the logical router distributed gateway port and <var>P</var>
> - is the NAT logical port. IP traffic matching the above rule
> - will be managed locally setting <code>reg1</code> to <var>C</var>
> - and <code>eth.src</code> to <var>D</var>, where <var>C</var> is NAT
> + the distributed gateway port specified in the
> + NAT rule and <var>P</var> is the NAT logical port. IP traffic
> + matching the above rule will be managed locally setting
> + <code>reg1</code> to <var>C</var> and
> + <code>eth.src</code> to <var>D</var>, where <var>C</var> is NAT
> external ip and <var>D</var> is NAT external mac.
> </li>
>
> @@ -4635,8 +4639,9 @@ nd_ns {
> outport == <var>GW</var> &&
> is_chassis_resident(<var>P</var>)</code>, where <var>E</var> is the
> external IP address specified in the NAT rule, <var>GW</var>
> - is the logical router distributed gateway port. For dnat_and_snat
> - NAT rule, <var>P</var> is the logical port specified in the NAT
> rule.
> + is the distributed gateway port specified in the NAT rule.
> + For dnat_and_snat NAT rule, <var>P</var> is the logical port
> + specified in the NAT rule.
> If <ref column="logical_port"
> table="NAT" db="OVN_Northbound"/> column of
> <ref table="NAT" db="OVN_Northbound"/> table is NOT set, then
> diff --git a/ovn-architecture.7.xml b/ovn-architecture.7.xml
> index ef8d669a2..a48757761 100644
> --- a/ovn-architecture.7.xml
> +++ b/ovn-architecture.7.xml
> @@ -742,9 +742,9 @@
>
> <p>
> A logical router can have multiple distributed gateway ports, each
> - connecting different external networks. However, some features, such as
> NAT
> - and load balancers, are not supported yet for logical routers with more
> - than one distributed gateway port configured.
> + connecting different external networks. Load balancing is not yet
> + supported for logical routers with more than one distributed gateway
> + port configured.
> </p>
>
> <h4>Physical VLAN MTU Issues</h4>
> diff --git a/ovn-nb.ovsschema b/ovn-nb.ovsschema
> index 80b830629..1a342c473 100644
> --- a/ovn-nb.ovsschema
> +++ b/ovn-nb.ovsschema
> @@ -1,7 +1,7 @@
> {
> "name": "OVN_Northbound",
> - "version": "6.1.0",
> - "cksum": "4010776751 31237",
> + "version": "6.2.0",
> + "cksum": "3707848838 31535",
> "tables": {
> "NB_Global": {
> "columns": {
> @@ -479,6 +479,12 @@
> "refType": "strong"},
> "min": 0,
> "max": 1}},
> + "gateway_port": {
> + "type": {"key": {"type": "uuid",
> + "refTable": "Logical_Router_Port",
> + "refType": "strong"},
I think the "gateway_port" column can be a weak reference to
"Logical_Router_Port".
Otherwise CMS cannot delete the logical router port unless it clears
this column or deletes the NAT row.
IMO, this column should be made optional. If CMS doesn''t specify the
gateway port to use for a NAT entry,
ovn-northd can figure out which router port is reachable for the
"external_ip" of the NAT.
ovn-northd already does this for the Logical_Router_Static_Route's
output_port column.
Please check this out -
https://github.com/ovn-org/ovn/blob/main/northd/northd.c#L9327
I'd suggest making this column optional. What do you think ?
> + "min": 0,
> + "max": 1}},
> "options": {"type": {"key": "string", "value": "string",
> "min": 0, "max": "unlimited"}},
> "external_ids": {
> diff --git a/ovn-nb.xml b/ovn-nb.xml
> index 4d7a23c52..0e3ad2b9b 100644
> --- a/ovn-nb.xml
> +++ b/ovn-nb.xml
> @@ -2619,8 +2619,8 @@
> <p>
> There can be more than one distributed gateway ports configured
> on each logical router, each connecting to different L2 segments.
> - However, features such as NAT and load-balancer are not supported
> - on logical routers with more than one distributed gateway ports.
> + Load-balancing is not yet supported on logical routers with more
> + than one distributed gateway ports.
> </p>
>
> <p>
> @@ -3352,6 +3352,39 @@
> </p>
> </column>
>
> + <column name="gateway_port">
> + <p>
> + A distributed gateway port in the <ref table="Logical_Router_Port"/>
> + table where the NAT rule needs to be applied.
> + </p>
> +
> + <p>
> + This column needs to be set when multiple distributed gateway ports
> + are configured on a <ref table="Logical_Router"/> for the NAT rule to
> + be applied. If logical router has a single distributed gateway port,
> + NAT rule is applied at the distributed gateway port even if this
> + column is not set.
> + </p>
> +
> + <p>
> + When multiple distributed gateway ports are configured on a
> + <ref table="Logical_Router"/>, applying a NAT rule at each of the
> + distributed gateway ports might not be desired. Consider the case
> + where a logical router has 2 distributed gateway port, one with
> + <ref column="networks" table="Logical_Router_Port"/>
> + <code>50.0.0.10/24</code> and the other with
> + <ref column="networks" table="Logical_Router_Port"/>
> + <code>60.0.0.10/24</code>. If the logical router has a
> + NAT rule of <ref column="type"/> <code>snat</code>,
> + <ref column="logical_ip"/> <code>10.1.1.0/24</code> and
> + <ref column="external_ip"/> <code>50.1.1.20/24</code>, the rule needs
> + to be selectively applied on matching packets entering/leaving
> + through the distributed gateway port with
> + <ref column="networks" table="Logical_Router_Port"/>
> + <code>50.0.0.10/24</code>.
> + </p>
> + </column>
> +
> <column name="options" key="stateless">
> Indicates if a dnat_and_snat rule should lead to connection
> tracking state or not.
> diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at
> index 539a121c0..db0d23099 100644
> --- a/tests/ovn-nbctl.at
> +++ b/tests/ovn-nbctl.at
> @@ -510,64 +510,64 @@ AT_CHECK([ovn-nbctl lr-nat-add lr0 dnat_and_snat
> fd01::1 fd11::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-add lr0 dnat_and_snat fd01::2 fd11::3 lp0
> 00:00:00:01:02:03])
> AT_CHECK([ovn-nbctl lr-nat-list lr0], [0], [dnl
> -TYPE EXTERNAL_IP EXTERNAL_PORT LOGICAL_IP
> EXTERNAL_MAC LOGICAL_PORT
> -dnat 30.0.0.1 192.168.1.2
> -dnat fd01::1 fd11::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
> -dnat_and_snat fd01::1 fd11::2
> -dnat_and_snat fd01::2 fd11::3
> 00:00:00:01:02:03 lp0
> -snat 30.0.0.1 192.168.1.0/24
> -snat fd01::1 fd11::/64
> +TYPE GATEWAY_PORT EXTERNAL_IP EXTERNAL_PORT
> LOGICAL_IP EXTERNAL_MAC LOGICAL_PORT
> +dnat 30.0.0.1
> 192.168.1.2
> +dnat fd01::1
> fd11::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
> +dnat_and_snat fd01::1
> fd11::2
> +dnat_and_snat fd01::2
> fd11::3 00:00:00:01:02:03 lp0
> +snat 30.0.0.1
> 192.168.1.0/24
> +snat fd01::1
> fd11::/64
> ])
> 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
> +[ovn-nbctl: 30.0.0.1, 192.168.1.0/24, : a NAT with this external_ip,
> logical_ip and gateway_port 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
> +[ovn-nbctl: 30.0.0.1, 192.168.1.0/24, : a NAT with this external_ip,
> logical_ip and gateway_port 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
> +[ovn-nbctl: a NAT with this type (snat), logical_ip (192.168.1.0/24) and
> gateway_port () 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
> +[ovn-nbctl: 30.0.0.1, 192.168.1.2, : a NAT with this external_ip, logical_ip
> and gateway_port 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
> +[ovn-nbctl: a NAT with this type (dnat), external_ip (30.0.0.1) and
> gateway_port () 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
> +[ovn-nbctl: 30.0.0.1, 192.168.1.2, : a NAT with this external_ip, logical_ip
> and gateway_port 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
> +[ovn-nbctl: a NAT with this type (dnat_and_snat), external_ip (30.0.0.1) and
> gateway_port () 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 EXTERNAL_PORT LOGICAL_IP
> EXTERNAL_MAC LOGICAL_PORT
> -dnat 30.0.0.1 192.168.1.2
> -dnat fd01::1 fd11::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
> -dnat_and_snat fd01::1 fd11::2
> -dnat_and_snat fd01::2 fd11::3
> 00:00:00:01:02:03 lp0
> -snat 30.0.0.1 192.168.1.0/24
> -snat fd01::1 fd11::/64
> +TYPE GATEWAY_PORT EXTERNAL_IP EXTERNAL_PORT
> LOGICAL_IP EXTERNAL_MAC LOGICAL_PORT
> +dnat 30.0.0.1
> 192.168.1.2
> +dnat fd01::1
> fd11::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
> +dnat_and_snat fd01::1
> fd11::2
> +dnat_and_snat fd01::2
> fd11::3 00:00:00:01:02:03 lp0
> +snat 30.0.0.1
> 192.168.1.0/24
> +snat fd01::1
> fd11::/64
> ])
> 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 --may-exist lr-nat-add lr0 dnat_and_snat fd01::2
> fd11::3])
> AT_CHECK([ovn-nbctl lr-nat-list lr0], [0], [dnl
> -TYPE EXTERNAL_IP EXTERNAL_PORT LOGICAL_IP
> EXTERNAL_MAC LOGICAL_PORT
> -dnat 30.0.0.1 192.168.1.2
> -dnat fd01::1 fd11::2
> -dnat_and_snat 30.0.0.1 192.168.1.2
> -dnat_and_snat 30.0.0.2 192.168.1.3
> -dnat_and_snat fd01::1 fd11::2
> -dnat_and_snat fd01::2 fd11::3
> -snat 30.0.0.1 192.168.1.0/24
> -snat fd01::1 fd11::/64
> +TYPE GATEWAY_PORT EXTERNAL_IP EXTERNAL_PORT
> LOGICAL_IP EXTERNAL_MAC LOGICAL_PORT
> +dnat 30.0.0.1
> 192.168.1.2
> +dnat fd01::1
> fd11::2
> +dnat_and_snat 30.0.0.1
> 192.168.1.2
> +dnat_and_snat 30.0.0.2
> 192.168.1.3
> +dnat_and_snat fd01::1
> fd11::2
> +dnat_and_snat fd01::2
> fd11::3
> +snat 30.0.0.1
> 192.168.1.0/24
> +snat fd01::1
> fd11::/64
> ])
>
> check_row_count nb:NAT 0 options:stateless=true
> @@ -598,40 +598,31 @@ AT_CHECK([ovn-nbctl --stateless lr-nat-add lr0
> dnat_and_snat 40.0.0.3 192.168.1.
> ])
>
> dnl Deletes the NATs
> -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)
> -])
> -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.3])
>
> AT_CHECK([ovn-nbctl lr-nat-del lr0 dnat_and_snat 30.0.0.1])
> AT_CHECK([ovn-nbctl lr-nat-del lr0 dnat_and_snat fd01::1])
> AT_CHECK([ovn-nbctl lr-nat-list lr0], [0], [dnl
> -TYPE EXTERNAL_IP EXTERNAL_PORT LOGICAL_IP
> EXTERNAL_MAC LOGICAL_PORT
> -dnat 30.0.0.1 192.168.1.2
> -dnat fd01::1 fd11::2
> -dnat_and_snat 30.0.0.2 192.168.1.3
> -dnat_and_snat 40.0.0.2 192.168.1.4
> -dnat_and_snat fd01::2 fd11::3
> -snat 30.0.0.1 192.168.1.0/24
> -snat 40.0.0.3 192.168.1.6
> -snat fd01::1 fd11::/64
> +TYPE GATEWAY_PORT EXTERNAL_IP EXTERNAL_PORT
> LOGICAL_IP EXTERNAL_MAC LOGICAL_PORT
> +dnat 30.0.0.1
> 192.168.1.2
> +dnat fd01::1
> fd11::2
> +dnat_and_snat 30.0.0.2
> 192.168.1.3
> +dnat_and_snat 40.0.0.2
> 192.168.1.4
> +dnat_and_snat fd01::2
> fd11::3
> +snat 30.0.0.1
> 192.168.1.0/24
> +snat 40.0.0.3
> 192.168.1.6
> +snat fd01::1
> fd11::/64
> ])
>
> AT_CHECK([ovn-nbctl lr-nat-del lr0 dnat])
> AT_CHECK([ovn-nbctl lr-nat-list lr0], [0], [dnl
> -TYPE EXTERNAL_IP EXTERNAL_PORT LOGICAL_IP
> EXTERNAL_MAC LOGICAL_PORT
> -dnat_and_snat 30.0.0.2 192.168.1.3
> -dnat_and_snat 40.0.0.2 192.168.1.4
> -dnat_and_snat fd01::2 fd11::3
> -snat 30.0.0.1 192.168.1.0/24
> -snat 40.0.0.3 192.168.1.6
> -snat fd01::1 fd11::/64
> +TYPE GATEWAY_PORT EXTERNAL_IP EXTERNAL_PORT
> LOGICAL_IP EXTERNAL_MAC LOGICAL_PORT
> +dnat_and_snat 30.0.0.2
> 192.168.1.3
> +dnat_and_snat 40.0.0.2
> 192.168.1.4
> +dnat_and_snat fd01::2
> fd11::3
> +snat 30.0.0.1
> 192.168.1.0/24
> +snat 40.0.0.3
> 192.168.1.6
> +snat fd01::1
> fd11::/64
> ])
>
> AT_CHECK([ovn-nbctl lr-nat-del lr0])
> @@ -702,12 +693,12 @@ AT_CHECK([ovn-nbctl show lr0 | grep -C2 'external
> port(s): "1"' | uuidfilt], [0]
> ])
>
> AT_CHECK([ovn-nbctl lr-nat-list lr0], [0], [dnl
> -TYPE EXTERNAL_IP EXTERNAL_PORT LOGICAL_IP
> EXTERNAL_MAC LOGICAL_PORT
> -dnat 40.0.0.4 1-3000 192.168.1.7
> -dnat 40.0.0.5 1 192.168.1.10
> -dnat_and_snat 40.0.0.5 1-3000 192.168.1.8
> -dnat_and_snat 40.0.0.6 1-3000 192.168.1.9
> 00:00:00:04:05:06 lp0
> -snat 40.0.0.3 21-65535 192.168.1.6
> +TYPE GATEWAY_PORT EXTERNAL_IP EXTERNAL_PORT
> LOGICAL_IP EXTERNAL_MAC LOGICAL_PORT
> +dnat 40.0.0.4 1-3000
> 192.168.1.7
> +dnat 40.0.0.5 1
> 192.168.1.10
> +dnat_and_snat 40.0.0.5 1-3000
> 192.168.1.8
> +dnat_and_snat 40.0.0.6 1-3000
> 192.168.1.9 00:00:00:04:05:06 lp0
> +snat 40.0.0.3 21-65535
> 192.168.1.6
> ])
>
> AT_CHECK([ovn-nbctl lr-nat-del lr0])
> @@ -755,7 +746,79 @@ AT_CHECK([ovn-nbctl lr-nat-update-ext-ip lr0 snat
> 192.168.16 allowed_range], [1]
> [ovn-nbctl: 192.168.16: Invalid IP address or CIDR
> ])
>
> -AT_CHECK([ovn-nbctl lr-nat-del lr0])])
> +AT_CHECK([ovn-nbctl lr-nat-del lr0])
> +
> +AT_CHECK([ovn-nbctl lrp-add lr0 lrp00 00:00:00:01:02:03 172.64.0.10/24])
> +AT_CHECK([ovn-nbctl lrp-add lr0 lrp01 00:00:00:01:02:04 172.64.1.10/24])
> +AT_CHECK([ovn-nbctl lrp-set-gateway-chassis lrp00 chassis1])
> +AT_CHECK([ovn-nbctl lr-add lr1])
> +AT_CHECK([ovn-nbctl lrp-add lr1 lrp10 00:00:00:01:02:05 172.64.2.10/24])
> +
> +AT_CHECK([ovn-nbctl --gateway-port=lrp00 lr-nat-add lr0 snat 172.64.0.10
> 20.0.0.10])
> +AT_CHECK([ovn-nbctl --gateway-port=lrp01 lr-nat-add lr0 dnat 172.64.1.10
> 20.0.0.10], [1], [],
> +[ovn-nbctl: lrp01 is not a distributed gateway router port.
> +])
> +AT_CHECK([ovn-nbctl --gateway-port=lrp10 lr-nat-add lr0 dnat_and_snat
> 172.64.2.10 20.0.0.10], [1], [],
> +[ovn-nbctl: lrp10 is not a router port of logical router: lr0.
> +])
> +
> +AT_CHECK([ovn-nbctl lrp-set-gateway-chassis lrp01 chassis2])
> +
> +AT_CHECK([ovn-nbctl lr-nat-add lr0 dnat 172.64.1.10 20.0.0.10], [1], [],
> +[ovn-nbctl: logical router: lr0 has multiple distributed gateway ports. NAT
> rule needs to specify gateway_port.
> +])
> +AT_CHECK([ovn-nbctl --gateway-port=lrp00 lr-nat-add lr0 dnat 30.0.0.10
> 20.0.0.10])
> +AT_CHECK([ovn-nbctl --gateway-port=lrp01 lr-nat-add lr0 dnat 30.0.0.10
> 20.0.0.10])
> +AT_CHECK([ovn-nbctl --gateway-port=lrp01 lr-nat-add lr0 dnat 30.0.0.10
> 20.0.0.20], [1], [],
> +[ovn-nbctl: a NAT with this type (dnat), external_ip (30.0.0.10) and
> gateway_port (lrp01) already exists
> +])
> +
> +AT_CHECK([ovn-nbctl lr-nat-list lr0], [0], [dnl
> +TYPE GATEWAY_PORT EXTERNAL_IP EXTERNAL_PORT
> LOGICAL_IP EXTERNAL_MAC LOGICAL_PORT
> +dnat lrp00 30.0.0.10
> 20.0.0.10
> +dnat lrp01 30.0.0.10
> 20.0.0.10
> +snat lrp00 172.64.0.10
> 20.0.0.10
> +])
> +
> +AT_CHECK([ovn-nbctl lr-nat-del lr0 dnat 30.0.0.10])
> +AT_CHECK([ovn-nbctl lr-nat-list lr0], [0], [dnl
> +TYPE GATEWAY_PORT EXTERNAL_IP EXTERNAL_PORT
> LOGICAL_IP EXTERNAL_MAC LOGICAL_PORT
> +snat lrp00 172.64.0.10
> 20.0.0.10
> +])
> +
> +AT_CHECK([ovn-nbctl --gateway-port=lrp00 lr-nat-add lr0 snat 30.0.0.10
> 20.0.0.20])
> +AT_CHECK([ovn-nbctl --gateway-port=lrp01 lr-nat-add lr0 snat 30.0.0.10
> 20.0.0.20])
> +AT_CHECK([ovn-nbctl lr-nat-list lr0], [0], [dnl
> +TYPE GATEWAY_PORT EXTERNAL_IP EXTERNAL_PORT
> LOGICAL_IP EXTERNAL_MAC LOGICAL_PORT
> +snat lrp00 172.64.0.10
> 20.0.0.10
> +snat lrp00 30.0.0.10
> 20.0.0.20
> +snat lrp01 30.0.0.10
> 20.0.0.20
> +])
> +AT_CHECK([ovn-nbctl lr-nat-del lr0 dnat lrp11], [1], [],
> +[ovn-nbctl: lrp11: port name not found
> +])
> +AT_CHECK([ovn-nbctl lr-nat-del lr0 dnat lrp00])
> +AT_CHECK([ovn-nbctl lr-nat-list lr0], [0], [dnl
> +TYPE GATEWAY_PORT EXTERNAL_IP EXTERNAL_PORT
> LOGICAL_IP EXTERNAL_MAC LOGICAL_PORT
> +snat lrp00 172.64.0.10
> 20.0.0.10
> +snat lrp00 30.0.0.10
> 20.0.0.20
> +snat lrp01 30.0.0.10
> 20.0.0.20
> +])
> +AT_CHECK([ovn-nbctl lr-nat-del lr0 snat lrp00])
> +AT_CHECK([ovn-nbctl lr-nat-list lr0], [0], [dnl
> +TYPE GATEWAY_PORT EXTERNAL_IP EXTERNAL_PORT
> LOGICAL_IP EXTERNAL_MAC LOGICAL_PORT
> +snat lrp01 30.0.0.10
> 20.0.0.20
> +])
> +
> +AT_CHECK([ovn-nbctl lr-nat-del lr0 snat 20.0.0 lrp01], [1], [],
> +[ovn-nbctl: 20.0.0: Invalid IP address or CIDR
> +])
> +AT_CHECK([ovn-nbctl lr-nat-del lr0 snat 20.0.0.10 lrp01], [1], [],
> +[ovn-nbctl: no matching NAT with the type (snat), logical_ip (20.0.0.10) and
> gateway_port (lrp01)
> +])
> +AT_CHECK([ovn-nbctl --if-exists lr-nat-del lr0 snat 20.0.0.10 lrp01])
> +AT_CHECK([ovn-nbctl lr-nat-del lr0 snat 20.0.0.20 lrp01])
> +])
>
> dnl ---------------------------------------------------------------------
>
> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> index 17d4f31b3..5a5909a3f 100644
> --- a/tests/ovn-northd.at
> +++ b/tests/ovn-northd.at
> @@ -4306,8 +4306,6 @@ check ovn-nbctl lsp-set-options lrp1-attachment
> router-port=lrp1
> check ovn-nbctl lr-nat-add lr0 dnat 42.42.42.42 192.168.0.2
> check ovn-nbctl --wait=sb sync
>
> -ovn-trace --minimal 'inport == "sw1-port1" && eth.src == 50:54:00:00:00:03
> && eth.dst == 00:00:00:00:ff:02 && ip4.dst == 42.42.42.42 && ip4.src ==
> 11.0.0.2 && ip.ttl == 64'
> -
> AT_CHECK([ovn-trace --minimal 'inport == "sw1-port1" && eth.src ==
> 50:54:00:00:00:03 && eth.dst == 00:00:00:00:ff:02 && ip4.dst == 42.42.42.42
> && ip4.src == 11.0.0.2 && ip.ttl == 64' | grep "output(\"sw0-port1\")"], [0],
> [ignore])
>
> dnl If we remove the DNAT entry we will be unable to trace to the DNAT
> address
> @@ -5804,12 +5802,6 @@ AT_CHECK([grep lr_in_gw_redirect lrflows | grep cr-DR
> | sed 's/table=../table=??
> table=??(lr_in_gw_redirect ), priority=50 , match=(outport == "DR-S3"),
> action=(outport = "cr-DR-S3"; next;)
> ])
>
> -# Check that ovn-northd logs a warning when trying to configure NAT
> -# on the router with multiple distributed gw ports. Such configurations are
> -# not supported yet.
> -check ovn-nbctl lr-nat-add DR dnat_and_snat 42.42.42.1 20.0.0.2
> -AT_CHECK([grep -q 'NAT is configured on logical router DR, which has 2
> distributed gateway ports. NAT is not supported yet when there is more than
> one distributed gateway port on the router.' northd/ovn-northd.log], [0])
> -
> AT_CLEANUP
> ])
>
> @@ -6475,3 +6467,167 @@ AT_CHECK([grep -e "ls_in_stateful" lsflows | sed
> 's/table=../table=??/' | sort],
>
> AT_CLEANUP
> ])
> +
> +AT_SETUP([ovn-northd -- lr multiple gw ports NAT])
> +AT_KEYWORDS([multiple-l3dgw-ports])
> +ovn_start
> +
> +# Logical network:
> +# 1 Logical Router, 3 bridged Logical Switches,
> +# 1 gateway chassis attached to each corresponding LRP.
> +#
> +# | S1 (gw1)
> +# |
> +# ls ---- DR -- S3 (gw3)
> +# (20.0.0.0/24) |
> +# | S2 (gw2)
> +#
> +# Validate SNAT, DNAT and DNAT_AND_SNAT behavior with multiple
> +# distributed gateway LRPs.
> +
> +check ovn-sbctl chassis-add gw1 geneve 127.0.0.1
> +check ovn-sbctl chassis-add gw2 geneve 128.0.0.1
> +check ovn-sbctl chassis-add gw3 geneve 129.0.0.1
> +
> +check ovn-nbctl lr-add DR
> +check ovn-nbctl lrp-add DR DR-S1 02:ac:10:01:00:01 172.16.1.1/24
> +check ovn-nbctl lrp-add DR DR-S2 03:ac:10:01:00:01 10.0.0.1/24
> +check ovn-nbctl lrp-add DR DR-S3 04:ac:10:01:00:01 192.168.0.1/24
> +check ovn-nbctl lrp-add DR DR-ls 05:ac:10:01:00:01 20.0.0.1/24
> +
> +check ovn-nbctl ls-add S1
> +check ovn-nbctl lsp-add S1 S1-DR
> +check ovn-nbctl lsp-set-type S1-DR router
> +check ovn-nbctl lsp-set-addresses S1-DR router
> +check ovn-nbctl --wait=sb lsp-set-options S1-DR router-port=DR-S1
> +
> +check ovn-nbctl ls-add S2
> +check ovn-nbctl lsp-add S2 S2-DR
> +check ovn-nbctl lsp-set-type S2-DR router
> +check ovn-nbctl lsp-set-addresses S2-DR router
> +check ovn-nbctl --wait=sb lsp-set-options S2-DR router-port=DR-S2
> +
> +check ovn-nbctl ls-add S3
> +check ovn-nbctl lsp-add S3 S3-DR
> +check ovn-nbctl lsp-set-type S3-DR router
> +check ovn-nbctl lsp-set-addresses S3-DR router
> +check ovn-nbctl --wait=sb lsp-set-options S3-DR router-port=DR-S3
> +
> +check ovn-nbctl ls-add ls
> +check ovn-nbctl lsp-add ls ls-DR
> +check ovn-nbctl lsp-set-type ls-DR router
> +check ovn-nbctl lsp-set-addresses ls-DR router
> +check ovn-nbctl --wait=sb lsp-set-options ls-DR router-port=DR-ls
> +
> +check ovn-nbctl lrp-set-gateway-chassis DR-S1 gw1
> +check ovn-nbctl lrp-set-gateway-chassis DR-S2 gw2
> +check ovn-nbctl lrp-set-gateway-chassis DR-S3 gw3
> +
> +check ovn-nbctl --wait=sb sync
> +
> +# Configure SNAT
> +check ovn-nbctl --gateway-port=DR-S1 lr-nat-add DR snat 172.16.1.10
> 20.0.0.10
> +check ovn-nbctl --gateway-port=DR-S2 lr-nat-add DR snat 10.0.0.10
> 20.0.0.10
> +check ovn-nbctl --gateway-port=DR-S3 lr-nat-add DR snat 192.168.0.10
> 20.0.0.10
> +
> +ovn-sbctl dump-flows DR > lrflows
> +AT_CAPTURE_FILE([lrflows])
> +
> +check_lr_in_arp_nat_flows() {
> + AT_CHECK([grep lr_in_ip_input lrflows | grep arp | grep -e 172.16.1.10
> -e 10.0.0.10 -e 192.168.0.10 | sed 's/table=../table=??/' | sort], [0], [dnl
> + table=??(lr_in_ip_input ), priority=90 , match=(arp.op == 1 &&
> arp.tpa == 10.0.0.10), action=(eth.dst = eth.src; eth.src = xreg0[[0..47]];
> arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]];
> arp.tpa <-> arp.spa; outport = inport; flags.loopback = 1; output;)
> + table=??(lr_in_ip_input ), priority=90 , match=(arp.op == 1 &&
> arp.tpa == 172.16.1.10), action=(eth.dst = eth.src; eth.src = xreg0[[0..47]];
> arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]];
> arp.tpa <-> arp.spa; outport = inport; flags.loopback = 1; output;)
> + table=??(lr_in_ip_input ), priority=90 , match=(arp.op == 1 &&
> arp.tpa == 192.168.0.10), action=(eth.dst = eth.src; eth.src =
> xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha =
> xreg0[[0..47]]; arp.tpa <-> arp.spa; outport = inport; flags.loopback = 1;
> output;)
> + table=??(lr_in_ip_input ), priority=91 , match=(inport == "DR-S1" &&
> arp.op == 1 && arp.tpa == 172.16.1.10), action=(drop;)
> + table=??(lr_in_ip_input ), priority=91 , match=(inport == "DR-S2" &&
> arp.op == 1 && arp.tpa == 10.0.0.10), action=(drop;)
> + table=??(lr_in_ip_input ), priority=91 , match=(inport == "DR-S3" &&
> arp.op == 1 && arp.tpa == 192.168.0.10), action=(drop;)
> + table=??(lr_in_ip_input ), priority=92 , match=(inport == "DR-S1" &&
> arp.op == 1 && arp.tpa == 172.16.1.10 && is_chassis_resident("cr-DR-S1")),
> action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply
> */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa <-> arp.spa; outport
> = inport; flags.loopback = 1; output;)
> + table=??(lr_in_ip_input ), priority=92 , match=(inport == "DR-S2" &&
> arp.op == 1 && arp.tpa == 10.0.0.10 && is_chassis_resident("cr-DR-S2")),
> action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply
> */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa <-> arp.spa; outport
> = inport; flags.loopback = 1; output;)
> + table=??(lr_in_ip_input ), priority=92 , match=(inport == "DR-S3" &&
> arp.op == 1 && arp.tpa == 192.168.0.10 && is_chassis_resident("cr-DR-S3")),
> action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply
> */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa <-> arp.spa; outport
> = inport; flags.loopback = 1; output;)
> +])
> +}
> +
> +check_lr_in_unsnat_flows() {
> + AT_CHECK([grep lr_in_unsnat lrflows | grep ct_snat | sed
> 's/table=../table=??/' | sort], [0], [dnl
> + table=??(lr_in_unsnat ), priority=100 , match=(ip && ip4.dst ==
> 10.0.0.10 && inport == "DR-S2" && flags.loopback == 0 &&
> is_chassis_resident("cr-DR-S2")), action=(ct_snat_in_czone;)
> + table=??(lr_in_unsnat ), priority=100 , match=(ip && ip4.dst ==
> 10.0.0.10 && inport == "DR-S2" && flags.loopback == 1 && flags.use_snat_zone
> == 1 && is_chassis_resident("cr-DR-S2")), action=(ct_snat;)
> + table=??(lr_in_unsnat ), priority=100 , match=(ip && ip4.dst ==
> 172.16.1.10 && inport == "DR-S1" && flags.loopback == 0 &&
> is_chassis_resident("cr-DR-S1")), action=(ct_snat_in_czone;)
> + table=??(lr_in_unsnat ), priority=100 , match=(ip && ip4.dst ==
> 172.16.1.10 && inport == "DR-S1" && flags.loopback == 1 &&
> flags.use_snat_zone == 1 && is_chassis_resident("cr-DR-S1")),
> action=(ct_snat;)
> + table=??(lr_in_unsnat ), priority=100 , match=(ip && ip4.dst ==
> 192.168.0.10 && inport == "DR-S3" && flags.loopback == 0 &&
> is_chassis_resident("cr-DR-S3")), action=(ct_snat_in_czone;)
> + table=??(lr_in_unsnat ), priority=100 , match=(ip && ip4.dst ==
> 192.168.0.10 && inport == "DR-S3" && flags.loopback == 1 &&
> flags.use_snat_zone == 1 && is_chassis_resident("cr-DR-S3")),
> action=(ct_snat;)
> +])
> +}
> +
> +check_lr_out_snat_flows() {
> + AT_CHECK([grep lr_out_snat lrflows | grep ct_snat | sed
> 's/table=../table=??/' | sort], [0], [dnl
> + table=??(lr_out_snat ), priority=161 , match=(ip && ip4.src ==
> 20.0.0.10 && outport == "DR-S1" && is_chassis_resident("cr-DR-S1")),
> action=(ct_snat_in_czone(172.16.1.10);)
> + table=??(lr_out_snat ), priority=161 , match=(ip && ip4.src ==
> 20.0.0.10 && outport == "DR-S2" && is_chassis_resident("cr-DR-S2")),
> action=(ct_snat_in_czone(10.0.0.10);)
> + table=??(lr_out_snat ), priority=161 , match=(ip && ip4.src ==
> 20.0.0.10 && outport == "DR-S3" && is_chassis_resident("cr-DR-S3")),
> action=(ct_snat_in_czone(192.168.0.10);)
> + table=??(lr_out_snat ), priority=162 , match=(ip && ip4.src ==
> 20.0.0.10 && outport == "DR-S1" && is_chassis_resident("cr-DR-S1") &&
> reg9[[4]] == 1), action=(reg9[[4]] = 0; ct_snat(172.16.1.10);)
> + table=??(lr_out_snat ), priority=162 , match=(ip && ip4.src ==
> 20.0.0.10 && outport == "DR-S2" && is_chassis_resident("cr-DR-S2") &&
> reg9[[4]] == 1), action=(reg9[[4]] = 0; ct_snat(10.0.0.10);)
> + table=??(lr_out_snat ), priority=162 , match=(ip && ip4.src ==
> 20.0.0.10 && outport == "DR-S3" && is_chassis_resident("cr-DR-S3") &&
> reg9[[4]] == 1), action=(reg9[[4]] = 0; ct_snat(192.168.0.10);)
> +])
> +}
> +
> +check_lr_in_unsnat_flows
> +check_lr_out_snat_flows
> +check_lr_in_arp_nat_flows
> +
> +check ovn-nbctl lr-nat-del DR snat 20.0.0.10
> +AT_CHECK([ovn-sbctl dump-flows DR | grep -e lr_in_unsnat -e lr_out_snat |
> grep ct_snat | wc -l], [0], [0
> +])
> +
> +# Configure DNAT
> +check ovn-nbctl --gateway-port=DR-S1 lr-nat-add DR dnat 172.16.1.10
> 20.0.0.10
> +check ovn-nbctl --gateway-port=DR-S2 lr-nat-add DR dnat 10.0.0.10
> 20.0.0.10
> +check ovn-nbctl --gateway-port=DR-S3 lr-nat-add DR dnat 192.168.0.10
> 20.0.0.10
> +
> +ovn-sbctl dump-flows DR > lrflows
> +AT_CAPTURE_FILE([lrflows])
> +
> +check_lr_in_dnat_flows() {
> + AT_CHECK([grep lr_in_dnat lrflows | grep ct_dnat | sed
> 's/table=../table=??/' | sort], [0], [dnl
> + table=??(lr_in_dnat ), priority=100 , match=(ip && ip4.dst ==
> 10.0.0.10 && inport == "DR-S2" && is_chassis_resident("cr-DR-S2")),
> action=(ct_dnat_in_czone(20.0.0.10);)
> + table=??(lr_in_dnat ), priority=100 , match=(ip && ip4.dst ==
> 172.16.1.10 && inport == "DR-S1" && is_chassis_resident("cr-DR-S1")),
> action=(ct_dnat_in_czone(20.0.0.10);)
> + table=??(lr_in_dnat ), priority=100 , match=(ip && ip4.dst ==
> 192.168.0.10 && inport == "DR-S3" && is_chassis_resident("cr-DR-S3")),
> action=(ct_dnat_in_czone(20.0.0.10);)
> +])
> +}
> +
> +check_lr_out_undnat_flows() {
> + AT_CHECK([grep lr_out_undnat lrflows | grep ct_dnat | sed
> 's/table=../table=??/' | sort], [0], [dnl
> + table=??(lr_out_undnat ), priority=100 , match=(ip && ip4.src ==
> 20.0.0.10 && outport == "DR-S1" && is_chassis_resident("cr-DR-S1")),
> action=(ct_dnat_in_czone;)
> + table=??(lr_out_undnat ), priority=100 , match=(ip && ip4.src ==
> 20.0.0.10 && outport == "DR-S2" && is_chassis_resident("cr-DR-S2")),
> action=(ct_dnat_in_czone;)
> + table=??(lr_out_undnat ), priority=100 , match=(ip && ip4.src ==
> 20.0.0.10 && outport == "DR-S3" && is_chassis_resident("cr-DR-S3")),
> action=(ct_dnat_in_czone;)
> +])
> +}
> +
> +check_lr_in_dnat_flows
> +check_lr_out_undnat_flows
> +check_lr_in_arp_nat_flows
> +
> +check ovn-nbctl lr-nat-del DR dnat
> +
> +AT_CHECK([ovn-sbctl dump-flows DR | grep -e lr_in_dnat -e lr_out_undnat |
> grep ct_dnat | wc -l], [0], [0
> +])
> +
> +# Configure DNAT_AND_SNAT
> +check ovn-nbctl --gateway-port=DR-S1 lr-nat-add DR dnat_and_snat
> 172.16.1.10 20.0.0.10
> +check ovn-nbctl --gateway-port=DR-S2 lr-nat-add DR dnat_and_snat 10.0.0.10
> 20.0.0.10
> +check ovn-nbctl --gateway-port=DR-S3 lr-nat-add DR dnat_and_snat
> 192.168.0.10 20.0.0.10
> +
> +ovn-sbctl dump-flows DR > lrflows
> +AT_CAPTURE_FILE([lrflows])
> +
> +check_lr_in_unsnat_flows
> +check_lr_out_snat_flows
> +check_lr_in_dnat_flows
> +check_lr_out_undnat_flows
> +check_lr_in_arp_nat_flows
> +
> +check ovn-nbctl lr-nat-del DR dnat_and_snat
> +
> +AT_CHECK([ovn-sbctl dump-flows DR | grep -e lr_in_unsnat -e lr_out_snat -e
> lr_in_dnat -e lr_out_undnat | grep ct_snat| wc -l], [0], [0
> +])
> +
There can be odd timing issues in CI. So I'd suggest adding --wait=sb
or --wait=hv in the ovn-nbctl command just before AT_CHECK is
called.
There are many places in the test case you can do the same i..e use
--wait=sb/hv.
Can you please check and add this in the relevant places ?
Thanks
Numan
> +AT_CLEANUP
> +])
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 166b5f72e..4a507ec86 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -29536,7 +29536,7 @@ as hv2 check ovn-appctl -t ovn-controller recompute
> AT_CHECK([as hv2 ovs-ofctl dump-flows br-int table=26 | grep 10.0.1.2], [1])
>
> # Enable dnat_and_snat on lr, and now hv2 should see flows for lsp1.
> -AT_CHECK([ovn-nbctl --wait=hv lr-nat-add lr dnat_and_snat 192.168.0.1
> 10.0.1.3 lsp1 f0:00:00:00:00:03])
> +AT_CHECK([ovn-nbctl --wait=hv --gateway-port=lrp_lr_ls1 lr-nat-add lr
> dnat_and_snat 192.168.0.1 10.0.1.3 lsp1 f0:00:00:00:00:03])
> AT_CHECK([as hv2 ovs-ofctl dump-flows br-int table=26 | grep 10.0.1.2], [0],
> [ignore])
>
> OVN_CLEANUP([hv1],[hv2])
> diff --git a/utilities/ovn-nbctl.8.xml b/utilities/ovn-nbctl.8.xml
> index 74651c762..3e9176fa0 100644
> --- a/utilities/ovn-nbctl.8.xml
> +++ b/utilities/ovn-nbctl.8.xml
> @@ -1142,7 +1142,7 @@
> <h2>NAT Commands</h2>
>
> <dl>
> - <dt>[<code>--may-exist</code>]
> [<code>--stateless</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>
> + <dt>[<code>--may-exist</code>] [<code>--stateless</code>]
> [<code>--gateway_port</code>=<var>GATEWAY_PORT</var>] <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>.
> @@ -1158,7 +1158,6 @@
> 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.
> - The <var>--stateless</var>
> </p>
> <p>
> When <code>--stateless</code> is specified then it implies that
> @@ -1169,6 +1168,16 @@
> with any other NAT rule.
> </p>
>
> + <p>
> + <code>--gateway-port</code> option allows specifying the
> distributed
> + gateway port of <var>router</var> where the NAT rule needs to be
> + applied. <var>GATEWAY_PORT</var> should reference a
> + <ref table="Logical_Router_Port"/> row that is a distributed
> gateway
> + port of <var>router</var>. When <var>router</var> has multiple
> + distributed gateway ports, it is an error to not specify the
> + <var>GATEWAY_PORT</var>.
> + </p>
> +
> <p>
> When <var>type</var> is <code>dnat</code>, the externally
> visible IP address <var>external_ip</var> is DNATted to the
> @@ -1201,32 +1210,39 @@
> <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_ip</var> and <var>GATEWAY_PORT</var> (in case of
> + multiple distributed gateway ports), 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>
>
> - <dt>[<code>--if-exists</code>] <code>lr-nat-del</code>
> <var>router</var> [<var>type</var> [<var>ip</var>]]</dt>
> + <dt>[<code>--if-exists</code>] <code>lr-nat-del</code>
> <var>router</var> [<var>type</var> [<var>ip</var>]
> [<var>gateway_port</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.
> + router. If <var>ip</var> is also specified without specifying
> + <var>gateway_port</var>, then all the NATs that match the
> + <var>type</var> and <var>ip</var> will be deleted from the logical
> + router. If <var>gateway_port</var> is specified without specifying
> + <var>ip</var>, then all the NATs that match the <var>type</var> and
> + <var>gateway_port</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 both <var>ip</var> and <var>gateway_port</var>
> are
> + specified and there is no matching NAT entry, unless
> + <code>--if-exists</code> is specified.
> </p>
> </dd>
>
> diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
> index 7bcc2c66a..f8b957792 100644
> --- a/utilities/ovn-nbctl.c
> +++ b/utilities/ovn-nbctl.c
> @@ -378,10 +378,11 @@ NAT commands:\n\
> [--stateless]\n\
> [--portrange]\n\
> [--add-route]\n\
> + [--gateway-port=GATEWAY_PORT]\n\
> lr-nat-add ROUTER TYPE EXTERNAL_IP LOGICAL_IP [LOGICAL_PORT
> EXTERNAL_MAC]\n\
> [EXTERNAL_PORT_RANGE]\n\
> add a NAT to ROUTER\n\
> - lr-nat-del ROUTER [TYPE [IP]]\n\
> + lr-nat-del ROUTER [TYPE [IP] [GATEWAY_PORT]]\n\
> remove NATs from ROUTER\n\
> lr-nat-list ROUTER print NATs for ROUTER\n\
> \n\
> @@ -4566,6 +4567,7 @@ nbctl_pre_lr_nat_add(struct ctl_context *ctx)
> {
> ovsdb_idl_add_column(ctx->idl, &nbrec_logical_router_col_name);
> ovsdb_idl_add_column(ctx->idl, &nbrec_logical_router_col_nat);
> + ovsdb_idl_add_column(ctx->idl, &nbrec_logical_router_col_ports);
>
> ovsdb_idl_add_column(ctx->idl, &nbrec_logical_switch_port_col_name);
>
> @@ -4574,7 +4576,14 @@ nbctl_pre_lr_nat_add(struct ctl_context *ctx)
> ovsdb_idl_add_column(ctx->idl, &nbrec_nat_col_type);
> ovsdb_idl_add_column(ctx->idl, &nbrec_nat_col_logical_port);
> ovsdb_idl_add_column(ctx->idl, &nbrec_nat_col_external_mac);
> + ovsdb_idl_add_column(ctx->idl, &nbrec_nat_col_gateway_port);
> ovsdb_idl_add_column(ctx->idl, &nbrec_nat_col_options);
> +
> + ovsdb_idl_add_column(ctx->idl, &nbrec_logical_router_port_col_name);
> + ovsdb_idl_add_column(ctx->idl,
> + &nbrec_logical_router_port_col_gateway_chassis);
> + ovsdb_idl_add_column(ctx->idl,
> + &nbrec_logical_router_port_col_ha_chassis_group);
> }
>
> static void
> @@ -4709,6 +4718,52 @@ nbctl_lr_nat_add(struct ctl_context *ctx)
> ctl_error(ctx, "routes cannot be added for snat types.");
> goto cleanup;
> }
> +
> + const char *dgw_port_name = shash_find_data(&ctx->options,
> + "--gateway-port");
> + const struct nbrec_logical_router_port *dgw_port = NULL;
> + if (dgw_port_name) {
> + error = lrp_by_name_or_uuid(ctx, dgw_port_name,
> + true, &dgw_port);
> + if (error) {
> + ctx->error = error;
> + goto cleanup;
> + }
> +
> + bool nat_lr_port = false;
> + for (size_t i = 0; i < lr->n_ports; i++) {
> + const struct nbrec_logical_router_port *lrp = lr->ports[i];
> + if (lrp == dgw_port) {
> + nat_lr_port = true;
> + }
> + }
> + if (!nat_lr_port) {
> + ctl_error(ctx, "%s is not a router port of logical router: %s.",
> + dgw_port_name, ctx->argv[1]);
> + goto cleanup;
> + }
> +
> + if (!dgw_port->ha_chassis_group && !dgw_port->n_gateway_chassis) {
> + ctl_error(ctx, "%s is not a distributed gateway router port.",
> + dgw_port_name);
> + goto cleanup;
> + }
> + } else {
> + size_t num_l3dgw_ports = 0;
> + for (size_t i = 0; i < lr->n_ports; i++) {
> + const struct nbrec_logical_router_port *lrp = lr->ports[i];
> + if (lrp->ha_chassis_group || lrp->n_gateway_chassis) {
> + num_l3dgw_ports++;
> + }
> + }
> + if (num_l3dgw_ports > 1) {
> + ctl_error(ctx, "logical router: %s has multiple distributed "
> + "gateway ports. NAT rule needs to specify "
> + "gateway_port.", ctx->argv[1]);
> + goto cleanup;
> + }
> + }
> +
> for (size_t i = 0; i < lr->n_nat; i++) {
> const struct nbrec_nat *nat = lr->nat[i];
>
> @@ -4725,7 +4780,8 @@ nbctl_lr_nat_add(struct ctl_context *ctx)
> continue;
> }
>
> - if (!strcmp(nat_type, nat->type)) {
> + if (!strcmp(nat_type, nat->type) &&
> + dgw_port == nat->gateway_port) {
> if (!strcmp(is_snat ? new_logical_ip : new_external_ip,
> is_snat ? old_logical_ip : old_external_ip)) {
> if (!strcmp(is_snat ? new_external_ip : new_logical_ip,
> @@ -4737,18 +4793,20 @@ nbctl_lr_nat_add(struct ctl_context *ctx)
> nbrec_nat_set_external_mac(nat, external_mac);
> should_return = true;
> } else {
> - ctl_error(ctx, "%s, %s: a NAT with this "
> - "external_ip and logical_ip already "
> - "exists", new_external_ip,
> - new_logical_ip);
> + ctl_error(ctx, "%s, %s, %s: a NAT with this "
> + "external_ip, logical_ip and "
> + "gateway_port already exists",
> + new_external_ip, new_logical_ip,
> + dgw_port ? dgw_port->name : "");
> should_return = true;
> }
> } else {
> - ctl_error(ctx, "a NAT with this type (%s) and %s (%s) "
> - "already exists",
> + ctl_error(ctx, "a NAT with this type (%s), %s (%s) "
> + "and gateway_port (%s) already exists",
> nat_type,
> is_snat ? "logical_ip" : "external_ip",
> - is_snat ? new_logical_ip : new_external_ip);
> + is_snat ? new_logical_ip : new_external_ip,
> + dgw_port ? dgw_port->name : "");
> should_return = true;
> }
> }
> @@ -4792,6 +4850,10 @@ nbctl_lr_nat_add(struct ctl_context *ctx)
> if (add_route) {
> smap_add(&nat_options, "add_route", "true");
> }
> +
> + if (dgw_port) {
> + nbrec_nat_update_gateway_port_addvalue(nat, dgw_port);
> + }
> nbrec_nat_set_options(nat, &nat_options);
>
> smap_destroy(&nat_options);
> @@ -4813,6 +4875,9 @@ nbctl_pre_lr_nat_del(struct ctl_context *ctx)
> ovsdb_idl_add_column(ctx->idl, &nbrec_nat_col_type);
> ovsdb_idl_add_column(ctx->idl, &nbrec_nat_col_logical_ip);
> ovsdb_idl_add_column(ctx->idl, &nbrec_nat_col_external_ip);
> + ovsdb_idl_add_column(ctx->idl, &nbrec_nat_col_gateway_port);
> +
> + ovsdb_idl_add_column(ctx->idl, &nbrec_logical_router_port_col_name);
> }
>
> static void
> @@ -4853,13 +4918,58 @@ nbctl_lr_nat_del(struct ctl_context *ctx)
> }
>
> char *nat_ip = normalize_prefix_str(ctx->argv[3]);
> + int is_snat = !strcmp("snat", nat_type);
> + const struct nbrec_logical_router_port *dgw_port = NULL;
> +
> + if (ctx->argc == 4) {
> + if (!nat_ip) {
> + /* GATEWAY_PORT is assumed to be passed. */
> + error = lrp_by_name_or_uuid(ctx, ctx->argv[3], true, &dgw_port);
> + if (error) {
> + ctx->error = error;
> + return;
> + }
> +
> + /* Deletes all NATs matching the type and gateway_port
> + * specified. */
> + for (size_t i = 0; i < lr->n_nat; i++) {
> + if (!strcmp(nat_type, lr->nat[i]->type) &&
> + lr->nat[i]->gateway_port == dgw_port) {
> + nbrec_logical_router_update_nat_delvalue(lr, lr->nat[i]);
> + }
> + }
> + return;
> + }
> +
> + /* Remove NAT rules matching the type and IP (based on type). */
> + for (size_t i = 0; i < lr->n_nat; i++) {
> + struct nbrec_nat *nat = lr->nat[i];
> + char *old_ip = normalize_prefix_str(is_snat
> + ? nat->logical_ip
> + : nat->external_ip);
> + if (!old_ip) {
> + continue;
> + }
> + if (!strcmp(nat_type, nat->type) && !strcmp(nat_ip, old_ip)) {
> + nbrec_logical_router_update_nat_delvalue(lr, nat);
> + }
> + free(old_ip);
> + }
> + goto cleanup;
> + }
> +
> if (!nat_ip) {
> ctl_error(ctx, "%s: Invalid IP address or CIDR", ctx->argv[3]);
> return;
> }
>
> - int is_snat = !strcmp("snat", nat_type);
> - /* Remove the matching NAT. */
> + error = lrp_by_name_or_uuid(ctx, ctx->argv[4], true, &dgw_port);
> + if (error) {
> + ctx->error = error;
> + goto cleanup;
> + }
> +
> + /* Remove matching NAT. */
> for (size_t i = 0; i < lr->n_nat; i++) {
> struct nbrec_nat *nat = lr->nat[i];
> bool should_return = false;
> @@ -4869,7 +4979,8 @@ nbctl_lr_nat_del(struct ctl_context *ctx)
> if (!old_ip) {
> continue;
> }
> - if (!strcmp(nat_type, nat->type) && !strcmp(nat_ip, old_ip)) {
> + if (!strcmp(nat_type, nat->type) && !strcmp(nat_ip, old_ip) &&
> + nat->gateway_port == dgw_port) {
> nbrec_logical_router_update_nat_delvalue(lr, nat);
> should_return = true;
> }
> @@ -4880,8 +4991,10 @@ nbctl_lr_nat_del(struct ctl_context *ctx)
> }
>
> if (must_exist) {
> - ctl_error(ctx, "no matching NAT with the type (%s) and %s (%s)",
> - nat_type, is_snat ? "logical_ip" : "external_ip", nat_ip);
> + ctl_error(ctx, "no matching NAT with the type (%s), %s (%s) and "
> + "gateway_port (%s)", nat_type,
> + is_snat ? "logical_ip" : "external_ip", nat_ip,
> + ctx->argv[4]);
> }
>
> cleanup:
> @@ -4900,6 +5013,9 @@ nbctl_pre_lr_nat_list(struct ctl_context *ctx)
> ovsdb_idl_add_column(ctx->idl, &nbrec_nat_col_logical_ip);
> ovsdb_idl_add_column(ctx->idl, &nbrec_nat_col_external_mac);
> ovsdb_idl_add_column(ctx->idl, &nbrec_nat_col_logical_port);
> + ovsdb_idl_add_column(ctx->idl, &nbrec_nat_col_gateway_port);
> +
> + ovsdb_idl_add_column(ctx->idl, &nbrec_logical_router_port_col_name);
> }
>
> static void
> @@ -4915,11 +5031,18 @@ 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];
> - char *key = xasprintf("%-17.13s%s", nat->type, nat->external_ip);
> + char *key = xasprintf("%-17.13s%-22.18s%s",
> + nat->type,
> + nat->gateway_port
> + ? nat->gateway_port->name
> + : "",
> + nat->external_ip);
> if (nat->external_mac && nat->logical_port) {
> - smap_add_format(&lr_nats, key, "%-17.13s%-22.18s%-21.17s%s",
> + smap_add_format(&lr_nats, key,
> + "%-17.13s%-20.16s%-21.17s%s",
> nat->external_port_range,
> - nat->logical_ip, nat->external_mac,
> + nat->logical_ip,
> + nat->external_mac,
> nat->logical_port);
> } else {
> smap_add_format(&lr_nats, key, "%-17.13s%s",
> @@ -4932,12 +5055,12 @@ nbctl_lr_nat_list(struct ctl_context *ctx)
> const struct smap_node **nodes = smap_sort(&lr_nats);
> if (nodes) {
> ds_put_format(&ctx->output,
> - "%-17.13s%-19.15s%-17.13s%-22.18s%-21.17s%s\n",
> - "TYPE", "EXTERNAL_IP", "EXTERNAL_PORT", "LOGICAL_IP",
> - "EXTERNAL_MAC", "LOGICAL_PORT");
> + "%-17.13s%-22.18s%-19.15s%-17.13s%-20.16s%-21.17s%s\n",
> + "TYPE", "GATEWAY_PORT", "EXTERNAL_IP", "EXTERNAL_PORT",
> + "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, "%-36.32s%s\n",
> + ds_put_format(&ctx->output, "%-58.54s%s\n",
> node->key, node->value);
> }
> free(nodes);
> @@ -7109,8 +7232,9 @@ static const struct ctl_command_syntax nbctl_commands[]
> = {
> "ROUTER TYPE EXTERNAL_IP LOGICAL_IP"
> "[LOGICAL_PORT EXTERNAL_MAC] [EXTERNAL_PORT_RANGE]",
> nbctl_pre_lr_nat_add, nbctl_lr_nat_add,
> - NULL, "--may-exist,--stateless,--portrange,--add-route", RW },
> - { "lr-nat-del", 1, 3, "ROUTER [TYPE [IP]]",
> + NULL, "--may-exist,--stateless,--portrange,--add-route,"
> + "--gateway-port=", RW },
> + { "lr-nat-del", 1, 4, "ROUTER [TYPE [IP] [GATEWAY_PORT]]",
> nbctl_pre_lr_nat_del, nbctl_lr_nat_del, NULL, "--if-exists", RW },
> { "lr-nat-list", 1, 1, "ROUTER", nbctl_pre_lr_nat_list,
> nbctl_lr_nat_list, NULL, "", RO },
> --
> 2.22.3
>
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
