| Message ID | 20220307145347.42028-1-sangana.abhiram@nutanix.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [ovs-dev,v3] northd: Add support for NAT with multiple DGP | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | warning | apply and check: warning |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/github-robot-_ovn-kubernetes | fail | github build: failed |
References: <20220307145347.42028-1-sangana.abhiram@nutanix.com>
Bleep bloop. Greetings Abhiram Sangana, I am a robot and I have tried out your patch.
Thanks for your contribution.
I encountered some error that I wasn't expecting. See the details below.
checkpatch:
WARNING: Line is 275 characters long (recommended limit is 79)
#1171 FILE: utilities/ovn-nbctl.8.xml:1138:
<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>
WARNING: Line is 143 characters long (recommended limit is 79)
#1216 FILE: utilities/ovn-nbctl.8.xml:1215:
<dt>[<code>--if-exists</code>] <code>lr-nat-del</code> <var>router</var> [<var>type</var> [<var>ip</var> [<var>gateway_port</var>]]]</dt>
WARNING: Line lacks whitespace around operator
WARNING: Line lacks whitespace around operator
WARNING: Line lacks whitespace around operator
#1250 FILE: utilities/ovn-nbctl.c:381:
[--gateway-port=GATEWAY_PORT]\n\
WARNING: Line lacks whitespace around operator
#1255 FILE: utilities/ovn-nbctl.c:385:
lr-nat-del ROUTER [TYPE [IP [GATEWAY_PORT]]]\n\
WARNING: Line has non-spaces leading whitespace
WARNING: Line has trailing whitespace
#1416 FILE: utilities/ovn-nbctl.c:4935:
ERROR: Inappropriate bracing around statement
#1440 FILE: utilities/ovn-nbctl.c:4959:
if (should_return)
Lines checked: 1518, Warnings: 8, Errors: 1
Please check this out. If you feel there has been an error, please email aconole@redhat.com
Thanks,
0-day Robot
Hi Abhiram, I had a look through the code and I'm happy with how it looks. I also did a quick check through the testsuite and it all seems good. All that being said, I nearly acked this, but I have one question down below in the test code. It may indicate some issue in `ovn-nbctl lr-nat-del` so I'd like to get that cleared up first. On 3/7/22 09:53, Abhiram Sangana 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.abhiram@nutanix.com> > --- > NEWS | 1 + > northd/northd.c | 182 +++++++++++++++++++++++--------------- > northd/ovn-northd.8.xml | 27 +++--- > ovn-architecture.7.xml | 6 +- > ovn-nb.ovsschema | 10 ++- > ovn-nb.xml | 37 +++++++- > tests/ovn-nbctl.at | 172 +++++++++++++++++++++-------------- > tests/ovn-northd.at | 166 +++++++++++++++++++++++++++++++++- > tests/ovn.at | 2 +- > utilities/ovn-nbctl.8.xml | 32 ++++--- > utilities/ovn-nbctl.c | 151 ++++++++++++++++++++++++++----- > 11 files changed, 592 insertions(+), 194 deletions(-) > > diff --git a/NEWS b/NEWS > index 9648f6cb2..764b2a03e 100644 > --- a/NEWS > +++ b/NEWS > @@ -1,5 +1,6 @@ > Post v22.03.0 > ------------- > + - Support NAT with multiple distributed gateway ports on a logical router. > > OVN v22.03.0 - XX XXX XXXX > -------------------------- > diff --git a/northd/northd.c b/northd/northd.c > index 294a59bd7..351f41134 100644 > --- a/northd/northd.c > +++ b/northd/northd.c > @@ -604,11 +604,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. */ > @@ -761,16 +761,6 @@ init_nat_entries(struct ovn_datapath *od) > return; > } > > - 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; > - } > - > od->nat_entries = xmalloc(od->nbr->n_nat * sizeof *od->nat_entries); > > for (size_t i = 0; i < od->nbr->n_nat; i++) { > @@ -2704,8 +2694,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(), > @@ -2718,8 +2709,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; > } > @@ -2748,6 +2738,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") > @@ -2818,9 +2814,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); > @@ -3453,9 +3449,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++; > @@ -10278,6 +10277,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; > @@ -10301,10 +10306,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 > @@ -12007,7 +12011,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) { > @@ -12136,7 +12140,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", > @@ -12226,7 +12230,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 > @@ -12268,7 +12272,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", > @@ -12513,7 +12517,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 > @@ -12552,12 +12557,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) { > @@ -12577,12 +12582,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, > @@ -12596,7 +12601,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 > @@ -12648,12 +12654,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) { > @@ -12683,7 +12689,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 > @@ -12700,12 +12707,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) { > @@ -12731,7 +12738,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. > */ > @@ -12746,7 +12753,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); > @@ -12762,7 +12769,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 > @@ -12809,7 +12817,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); > @@ -12821,7 +12829,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); > @@ -12880,13 +12888,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) { > @@ -12907,7 +12915,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, > @@ -12934,7 +12942,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, > @@ -12951,13 +12959,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;", > @@ -12973,32 +12982,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; > @@ -13032,6 +13042,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) { > @@ -13131,7 +13167,7 @@ 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; > @@ -13150,18 +13186,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) { > @@ -13174,14 +13211,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), > @@ -13193,18 +13230,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 > @@ -13221,7 +13259,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. > @@ -13254,10 +13292,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 e495db46a..6adec765b 100644 > --- a/northd/ovn-northd.8.xml > +++ b/northd/ovn-northd.8.xml > @@ -2943,7 +2943,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 > @@ -2968,7 +2969,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 > @@ -3249,9 +3251,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>. > @@ -4061,10 +4064,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> > > @@ -4531,8 +4535,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"}, > + "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 beb3ded79..fc2b917ac 100644 > --- a/ovn-nb.xml > +++ b/ovn-nb.xml > @@ -2590,8 +2590,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> > @@ -3323,6 +3323,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..b4acb2da6 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]) I don't understand the above change. First, I don't understand why many of the lr-nat-del checks were removed. Second, I don't understand why the lr-nat-del of 30.0.0.3 doesn't return an error. There is no NAT with that address, so shouldn't that return an error? > > 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,54 @@ 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 lr-nat-del lr0 dnat 30.0.0.10 lrp00], [1], [], > +[ovn-nbctl: no matching NAT with the type (dnat), external_ip (30.0.0.10) and gateway_port (lrp00) > +]) > +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.10 lrp00]) > +]) > > dnl --------------------------------------------------------------------- > > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > index fe2786973..166e1e0e9 100644 > --- a/tests/ovn-northd.at > +++ b/tests/ovn-northd.at > @@ -4296,8 +4296,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 > @@ -6249,3 +6247,167 @@ check_log_flows_count 0 in > > 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 > +]) > + > +AT_CLEANUP > +]) > diff --git a/tests/ovn.at b/tests/ovn.at > index 69270601a..62b6ed7db 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -29272,7 +29272,7 @@ as hv2 check ovn-appctl -t ovn-controller recompute > AT_CHECK([as hv2 ovs-ofctl dump-flows br-int table=24 | 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=24 | 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 545f3bf27..1f75c3ebc 100644 > --- a/utilities/ovn-nbctl.8.xml > +++ b/utilities/ovn-nbctl.8.xml > @@ -1135,7 +1135,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>. > @@ -1151,7 +1151,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 > @@ -1162,6 +1161,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 > @@ -1194,30 +1203,33 @@ > <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 > + router. If <var>ip</var> is also specified, then all the > + NATs that match the <var>type</var> and <var>ip</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 > + It is an error if <var>gateway_port</var> is specified and there > is no matching NAT entry, unless <code>--if-exists</code> is > specified. > </p> > diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c > index adb08c6c9..163e64b67 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\ > @@ -4557,6 +4558,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); > > @@ -4565,7 +4567,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 > @@ -4700,6 +4709,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]; > > @@ -4716,7 +4771,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, > @@ -4728,18 +4784,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; > } > } > @@ -4783,6 +4841,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); > @@ -4804,6 +4866,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 > @@ -4850,7 +4915,32 @@ nbctl_lr_nat_del(struct ctl_context *ctx) > } > > int is_snat = !strcmp("snat", nat_type); > - /* Remove the matching NAT. */ > + if (ctx->argc == 4) { > + /* 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; > + } > + > + const struct nbrec_logical_router_port *dgw_port = NULL; > + 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; > @@ -4860,19 +4950,21 @@ 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; > } > free(old_ip); > - if (should_return) { > + if (should_return) > goto cleanup; > - } > } > > 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: > @@ -4891,6 +4983,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 > @@ -4906,11 +5001,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", > @@ -4923,12 +5025,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); > @@ -7099,8 +7201,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 }, >
Hey Mark, Thanks for reviewing the patch. Regarding `ovn-nbctl lr-nat-del`, I have updated the command to have the following structure: `lr-nat-del ROUTER [TYPE [IP [GATEWAY_PORT]]]`. Most of the earlier checks are not enforced unless `GATEWAY_PORT` is also passed. With the new patch, a NAT rule is no longer uniquely identified by `TYPE` and `IP`. The user also needs to pass `GATEWAY_PORT` to uniquely identify a NAT rule. So, I have updated the behavior of `lr-nat-del` to not raise an error when `IP` is passed but no NAT rules match, similar to its behavior when `TYPE` is passed and no NAT rules match. Also, with the new patch, if `IP` is passed without passing `GATEWAY_PORT`, all NAT rules (possibly None) with the `TYPE` and `IP` values are deleted irrespective of their `GATEWAY_PORT` value. Should we retain the previous behavior of `lr-nat-del` when LR has a single DGP - raise an error if no NAT rules match when `IP` is passed? On 14 Mar 2022, at 20:55, Mark Michelson <mmichels@redhat.com<mailto:mmichels@redhat.com>> wrote: Hi Abhiram, I had a look through the code and I'm happy with how it looks. I also did a quick check through the testsuite and it all seems good. All that being said, I nearly acked this, but I have one question down below in the test code. It may indicate some issue in `ovn-nbctl lr-nat-del` so I'd like to get that cleared up first. -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]) I don't understand the above change. First, I don't understand why many of the lr-nat-del checks were removed. Second, I don't understand why the lr-nat-del of 30.0.0.3 doesn't return an error. There is no NAT with that address, so shouldn't that return an error? 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])
On 3/15/22 10:35, Abhiram Sangana wrote: > Hey Mark, > > Thanks for reviewing the patch. Regarding `ovn-nbctl lr-nat-del`, I > have updated the command to have the following structure: > `lr-nat-del ROUTER [TYPE [IP [GATEWAY_PORT]]]`. Most of the > earlier checks are not enforced unless `GATEWAY_PORT` is also > passed. > > With the new patch, a NAT rule is no longer uniquely identified by > `TYPE` and `IP`. The user also needs to pass `GATEWAY_PORT` to > uniquely identify a NAT rule. So, I have updated the behavior of > `lr-nat-del` to not raise an error when `IP` is passed but no NAT > rules match, similar to its behavior when `TYPE` is passed and no > NAT rules match. Ah, I didn't realize that this case didn't return an error. It seems odd to me that we wouldn't return an error here, but I won't try to rock the boat too much with regards to existing behavior. > Also, with the new patch, if `IP` is passed without > passing `GATEWAY_PORT`, all NAT rules (possibly None) with the > `TYPE` and `IP` values are deleted irrespective of their > `GATEWAY_PORT` value. I'm a bit surprised that with this change, the hierarchy for specificity is TYPE < IP < GATEWAY_PORT . This makes it sound as though the primary use case would be to use the same IP for multiple NAT rules across different gateway ports. Wouldn't it be just as likely that the same gateway port would be used for multiple NAT rules all with different IPs? In other words, I suspect that something like this: ovn-nbctl lr-nat-del my_router dnat_and_snat my_router_gateway_port_1 where the intent is to remove all NAT rules from my_router whose gateway port is my_router_gateway_port_1, would be just as useful as something like: ovn-nbctl lr-nat-del my_router dnat 10.0.0.1 where the intent is to remove all NAT rules from my_router whose external IP address is 10.0.0.1 . Would it make sense to change the syntax from ovn-nbctl lr-nat-del [TYPE [IP [GATEWAY_PORT]]] to ovn-nbctl lr-nat-del [TYPE [IP] [GATEWAY_PORT]] ? It would be a bit of work to determine if the two-parameter version of the command is giving an IP address or gateway port, but it wouldn't be too difficult I don't think. > > Should we retain the previous behavior of `lr-nat-del` when LR has a > single DGP - raise an error if no NAT rules match when `IP` is passed? Short version: no. Long version: If I had it my way, then lr-nat-del would raise an error any time it is run and no NAT rules can be found to delete (unless --if-exists is passed). This way it would be very easy to explain when an error is returned and when one isn't. However, as you pointed out there is a precedent where an attempted deletion of multiple rows does not raise an error if matches are not found. Since it is now possible for a router to have multiple NAT entries with the same IP address (as long as they are on different gateway ports), then I guess we should follow that same precedent. I think you should leave it the way you have it since it is easiest to explain (deleting multiple rules == never raise an error, deleting a specific rule == raise an error if it doesn't exist). Otherwise, the nuances are difficult to explain and difficult to maintain. > >> On 14 Mar 2022, at 20:55, Mark Michelson <mmichels@redhat.com >> <mailto:mmichels@redhat.com>> wrote: >> >> Hi Abhiram, >> >> I had a look through the code and I'm happy with how it looks. I also >> did a quick check through the testsuite and it all seems good. All >> that being said, >> >> I nearly acked this, but I have one question down below in the test >> code. It may indicate some issue in `ovn-nbctl lr-nat-del` so I'd like >> to get that cleared up first. >> >>> >>> -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]) >> >> I don't understand the above change. First, I don't understand why >> many of the lr-nat-del checks were removed. Second, I don't understand >> why the lr-nat-del of 30.0.0.3 doesn't return an error. There is no >> NAT with that address, so shouldn't that return an error? >> >>> 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]) >>> >
> On 21 Mar 2022, at 19:49, Mark Michelson <mmichels@redhat.com> wrote: > > I'm a bit surprised that with this change, the hierarchy for specificity is TYPE < IP < GATEWAY_PORT . This makes it sound as though the primary use case would be to use the same IP for multiple NAT rules across different gateway ports. Wouldn't it be just as likely that the same gateway port would be used for multiple NAT rules all with different IPs? Yes, that makes sense. > > ovn-nbctl lr-nat-del [TYPE [IP] [GATEWAY_PORT]] > > I think you should leave it the way you have it since it is easiest to explain (deleting multiple rules == never raise an error, deleting a specific rule == raise an error if it doesn't exist). Otherwise, the nuances are difficult to explain and difficult to maintain. This syntax looks good. So, with this syntax, we never expect to match a single NAT rule and hence, we would not need —if_exists, right? I will retain the arg but we might never hit that case. Thanks, Abhiram Sangana
On 3/22/22 09:46, Abhiram Sangana wrote: > > >> On 21 Mar 2022, at 19:49, Mark Michelson <mmichels@redhat.com> wrote: >> >> I'm a bit surprised that with this change, the hierarchy for specificity is TYPE < IP < GATEWAY_PORT . This makes it sound as though the primary use case would be to use the same IP for multiple NAT rules across different gateway ports. Wouldn't it be just as likely that the same gateway port would be used for multiple NAT rules all with different IPs? > Yes, that makes sense. > >> >> ovn-nbctl lr-nat-del [TYPE [IP] [GATEWAY_PORT]] >> >> I think you should leave it the way you have it since it is easiest to explain (deleting multiple rules == never raise an error, deleting a specific rule == raise an error if it doesn't exist). Otherwise, the nuances are difficult to explain and difficult to maintain. > > This syntax looks good. So, with this syntax, we never expect to match a single NAT rule and hence, we would not need —if_exists, right? I will retain the arg but we might never hit that case. No, that was not my intention. Basically, the following forms would never return an error: ovn-nbctl lr-nat-del my_router dnat (type only) ovn-nbctl lr-nat-del my_router dnat 172.16.0.1 (type and IP) ovn-nbctl lr-nat-del my_router dnat my_router_gateway_port1 (type and gateway port) But the following form could return an error ovn-nbctl lr-nat-del my_router dnat 172.16.0.1 my_router_gateway_port1 (type, IP, and gateway port specified) > > Thanks, > Abhiram Sangana >
> On 22 Mar 2022, at 14:31, Mark Michelson <mmichels@redhat.com> wrote: > > No, that was not my intention. Basically, the following forms would never return an error: > > ovn-nbctl lr-nat-del my_router dnat (type only) > ovn-nbctl lr-nat-del my_router dnat 172.16.0.1 (type and IP) > ovn-nbctl lr-nat-del my_router dnat my_router_gateway_port1 (type and gateway port) > > But the following form could return an error > > ovn-nbctl lr-nat-del my_router dnat 172.16.0.1 my_router_gateway_port1 (type, IP, and gateway port specified) My bad - Got it.
diff --git a/NEWS b/NEWS index 9648f6cb2..764b2a03e 100644 --- a/NEWS +++ b/NEWS @@ -1,5 +1,6 @@ Post v22.03.0 ------------- + - Support NAT with multiple distributed gateway ports on a logical router. OVN v22.03.0 - XX XXX XXXX -------------------------- diff --git a/northd/northd.c b/northd/northd.c index 294a59bd7..351f41134 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -604,11 +604,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. */ @@ -761,16 +761,6 @@ init_nat_entries(struct ovn_datapath *od) return; } - 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; - } - od->nat_entries = xmalloc(od->nbr->n_nat * sizeof *od->nat_entries); for (size_t i = 0; i < od->nbr->n_nat; i++) { @@ -2704,8 +2694,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(), @@ -2718,8 +2709,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; } @@ -2748,6 +2738,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") @@ -2818,9 +2814,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); @@ -3453,9 +3449,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++; @@ -10278,6 +10277,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; @@ -10301,10 +10306,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 @@ -12007,7 +12011,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) { @@ -12136,7 +12140,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", @@ -12226,7 +12230,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 @@ -12268,7 +12272,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", @@ -12513,7 +12517,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 @@ -12552,12 +12557,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) { @@ -12577,12 +12582,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, @@ -12596,7 +12601,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 @@ -12648,12 +12654,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) { @@ -12683,7 +12689,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 @@ -12700,12 +12707,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) { @@ -12731,7 +12738,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. */ @@ -12746,7 +12753,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); @@ -12762,7 +12769,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 @@ -12809,7 +12817,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); @@ -12821,7 +12829,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); @@ -12880,13 +12888,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) { @@ -12907,7 +12915,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, @@ -12934,7 +12942,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, @@ -12951,13 +12959,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;", @@ -12973,32 +12982,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; @@ -13032,6 +13042,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) { @@ -13131,7 +13167,7 @@ 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; @@ -13150,18 +13186,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) { @@ -13174,14 +13211,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), @@ -13193,18 +13230,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 @@ -13221,7 +13259,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. @@ -13254,10 +13292,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 e495db46a..6adec765b 100644 --- a/northd/ovn-northd.8.xml +++ b/northd/ovn-northd.8.xml @@ -2943,7 +2943,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 @@ -2968,7 +2969,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 @@ -3249,9 +3251,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>. @@ -4061,10 +4064,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> @@ -4531,8 +4535,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"}, + "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 beb3ded79..fc2b917ac 100644 --- a/ovn-nb.xml +++ b/ovn-nb.xml @@ -2590,8 +2590,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> @@ -3323,6 +3323,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..b4acb2da6 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,54 @@ 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 lr-nat-del lr0 dnat 30.0.0.10 lrp00], [1], [], +[ovn-nbctl: no matching NAT with the type (dnat), external_ip (30.0.0.10) and gateway_port (lrp00) +]) +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.10 lrp00]) +]) dnl --------------------------------------------------------------------- diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index fe2786973..166e1e0e9 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -4296,8 +4296,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 @@ -6249,3 +6247,167 @@ check_log_flows_count 0 in 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 +]) + +AT_CLEANUP +]) diff --git a/tests/ovn.at b/tests/ovn.at index 69270601a..62b6ed7db 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -29272,7 +29272,7 @@ as hv2 check ovn-appctl -t ovn-controller recompute AT_CHECK([as hv2 ovs-ofctl dump-flows br-int table=24 | 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=24 | 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 545f3bf27..1f75c3ebc 100644 --- a/utilities/ovn-nbctl.8.xml +++ b/utilities/ovn-nbctl.8.xml @@ -1135,7 +1135,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>. @@ -1151,7 +1151,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 @@ -1162,6 +1161,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 @@ -1194,30 +1203,33 @@ <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 + router. If <var>ip</var> is also specified, then all the + NATs that match the <var>type</var> and <var>ip</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 + It is an error if <var>gateway_port</var> is specified and there is no matching NAT entry, unless <code>--if-exists</code> is specified. </p> diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c index adb08c6c9..163e64b67 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\ @@ -4557,6 +4558,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); @@ -4565,7 +4567,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 @@ -4700,6 +4709,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]; @@ -4716,7 +4771,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, @@ -4728,18 +4784,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; } } @@ -4783,6 +4841,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); @@ -4804,6 +4866,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 @@ -4850,7 +4915,32 @@ nbctl_lr_nat_del(struct ctl_context *ctx) } int is_snat = !strcmp("snat", nat_type); - /* Remove the matching NAT. */ + if (ctx->argc == 4) { + /* 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; + } + + const struct nbrec_logical_router_port *dgw_port = NULL; + 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; @@ -4860,19 +4950,21 @@ 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; } free(old_ip); - if (should_return) { + if (should_return) goto cleanup; - } } 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: @@ -4891,6 +4983,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 @@ -4906,11 +5001,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", @@ -4923,12 +5025,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); @@ -7099,8 +7201,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 },
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.abhiram@nutanix.com> --- NEWS | 1 + northd/northd.c | 182 +++++++++++++++++++++++--------------- northd/ovn-northd.8.xml | 27 +++--- ovn-architecture.7.xml | 6 +- ovn-nb.ovsschema | 10 ++- ovn-nb.xml | 37 +++++++- tests/ovn-nbctl.at | 172 +++++++++++++++++++++-------------- tests/ovn-northd.at | 166 +++++++++++++++++++++++++++++++++- tests/ovn.at | 2 +- utilities/ovn-nbctl.8.xml | 32 ++++--- utilities/ovn-nbctl.c | 151 ++++++++++++++++++++++++++----- 11 files changed, 592 insertions(+), 194 deletions(-)