| Message ID | 20220505174315.232636-1-sangana.abhiram@nutanix.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [ovs-dev] northd: Determine gateway port for NAT when not specified | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | fail | github build: failed |
| ovsrobot/github-robot-_ovn-kubernetes | fail | github build: failed |
Hi Abhiram, This is a great idea. I only have a couple of minor comments below. On 5/5/22 13:43, Abhiram Sangana wrote: > If multiple distributed gateway ports (DGP) are configured on a > logical router, "gateway_port" column needs to be set for NAT entries > of the logical router for the rules to be applied, as part of commit > 2d942be (northd: Add support for NAT with multiple DGP). > > This patch updates the behavior by inferring the DGP where the NAT > rule needs to be applied based on the "external_ip" column of the > NAT rule when "gateway_port" column is not set. > > Signed-off-by: Abhiram Sangana <sangana.abhiram@nutanix.com> > --- > northd/northd.c | 49 ++++++++++++------ > northd/ovn-northd.8.xml | 19 ++++--- > ovn-nb.xml | 19 ++++--- > tests/ovn-nbctl.at | 47 ++++++++++-------- > tests/ovn-northd.at | 102 ++++++++++++++++++++++++-------------- > tests/ovn.at | 2 +- > utilities/ovn-nbctl.8.xml | 5 +- > utilities/ovn-nbctl.c | 94 ++++++++++++++++++++++++++++------- > 8 files changed, 228 insertions(+), 109 deletions(-) > > diff --git a/northd/northd.c b/northd/northd.c > index a56666297..aa1c2ae05 100644 > --- a/northd/northd.c > +++ b/northd/northd.c > @@ -2729,14 +2729,17 @@ join_logical_ports(struct northd_input *input_data, > } > } > > +static const char *find_lrp_member_ip(const struct ovn_port *op, > + const char *ip_s); > + > /* Returns an array of strings, each consisting of a MAC address followed > * 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 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. > + * external IP addresses of NAT rules defined on that router whose > + * gateway_port is 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(), > * and must free the returned array when it is no longer needed. */ > @@ -2779,7 +2782,9 @@ get_nat_addresses(const struct ovn_port *op, size_t *n, bool routable_only, > > /* Not including external IP of NAT rules whose gateway_port is > * not 'op'. */ > - if (nat->gateway_port && nat->gateway_port != op->nbrp) { > + if (op->od->n_l3dgw_ports > 1 && > + ((!nat->gateway_port && !find_lrp_member_ip(op, nat->external_ip)) > + || (nat->gateway_port && nat->gateway_port != op->nbrp))) { This same if statement is repeated later in this patch. It would probably be good to factor this into a function. > continue; > } > > @@ -3454,8 +3459,12 @@ ovn_port_update_sbrec(struct northd_input *input_data, > struct ds nat_addr = DS_EMPTY_INITIALIZER; > ds_put_format(&nat_addr, "%s", nat_addresses); > if (l3dgw_ports) { > + const struct ovn_port *l3dgw_port = ( > + is_l3dgw_port(op->peer) > + ? op->peer > + : op->peer->od->l3dgw_ports[0]); > ds_put_format(&nat_addr, " is_chassis_resident(%s)", > - op->peer->od->l3dgw_ports[0]->cr_port->json_key); > + l3dgw_port->cr_port->json_key); > } > nats[0] = xstrdup(ds_cstr(&nat_addr)); > ds_destroy(&nat_addr); > @@ -10502,9 +10511,11 @@ 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 > + /* ARP/ND should be sent from distributed gateway port relevant to > * the NAT rule. */ > - if (nat->gateway_port && nat->gateway_port != op->nbrp) { > + if (op->od->n_l3dgw_ports > 1 && > + ((!nat->gateway_port && !find_lrp_member_ip(op, nat->external_ip)) || > + (nat->gateway_port && nat->gateway_port != op->nbrp))) { > return; > } > > @@ -13287,14 +13298,24 @@ lrouter_check_nat_entry(struct ovn_datapath *od, const struct nbrec_nat *nat, > /* 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) { > + if (od->n_l3dgw_ports == 1) { > *nat_l3dgw_port = od->l3dgw_ports[0]; > + } else if (od->n_l3dgw_ports > 1) { > + /* Find the DGP reachable for the NAT external IP. */ > + for (size_t i = 0; i < od->n_l3dgw_ports; i++) { > + if (find_lrp_member_ip(od->l3dgw_ports[i], nat->external_ip)) { > + *nat_l3dgw_port = od->l3dgw_ports[i]; > + break; > + } > + } > + if (*nat_l3dgw_port == NULL) { > + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); > + VLOG_WARN_RL(&rl, "Unable to determine gateway_port for NAT " > + "with external_ip: %s configured on logical " > + "router: %s with multiple distributed gateway " > + "ports", nat->external_ip, od->nbr->name); > + return -EINVAL; > + } > } > } else { > *nat_l3dgw_port = ovn_port_find(ports, nat->gateway_port->name); > diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml > index 0fe350d0e..948c929e7 100644 > --- a/northd/ovn-northd.8.xml > +++ b/northd/ovn-northd.8.xml > @@ -2137,7 +2137,8 @@ output; > router that specifies an external Ethernet address <var>E</var>, > a priority-50 flow that matches <code>inport == <var>GW</var> > && eth.dst == <var>E</var></code>, where <var>GW</var> > - is the logical router gateway port, with action > + is the logical router distributed gateway port corresponding to the > + NAT rule (specified or inferred), with action > <code>xreg0[0..47]=E; next;</code>. > </p> > > @@ -2453,11 +2454,12 @@ icmp6_error { > <li> > <p> > For each NAT entry of a distributed logical router (with > - distributed gateway router port) of type <code>snat</code>, > + distributed gateway router port(s)) of type <code>snat</code>, > a priority-120 flow with the match <code>inport == <var>P</var> > && ip4.src == <var>A</var></code> advances the packet to > the next pipeline, where <var>P</var> is the distributed logical > - router port and <var>A</var> is the <code>external_ip</code> set > + router port corresponding to the NAT entry (specified or inferred) > + and <var>A</var> is the <code>external_ip</code> set > in the NAT entry. If <var>A</var> is an IPv6 address, then > <code>ip6.src</code> is used for the match. > </p> > @@ -3057,7 +3059,7 @@ icmp6 { > ip6.dst == <var>B</var> && inport == <var>GW</var> > && flags.loopback == 0</code> > where <var>GW</var> is the distributed gateway port > - specified in the NAT rule, with an > + corresponding to the NAT rule (specified or inferred), 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 > @@ -3083,7 +3085,7 @@ icmp6 { > && flags.loopback == 0 && > flags.use_snat_zone == 1</code> > where <var>GW</var> is the distributed gateway port > - specified in the NAT rule, with an > + corresponding to the NAT rule (specified or inferred), 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 > @@ -3364,8 +3366,8 @@ 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 configured > - for the NAT rule, with an action > + where <var>GW</var> is the logical router gateway port corresponding > + to the NAT rule (specified or inferred), 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 > @@ -4683,7 +4685,8 @@ 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 distributed gateway port specified in the NAT rule. > + is the distributed gateway port corresponding to the NAT rule > + (specified or inferred). > For dnat_and_snat NAT rule, <var>P</var> is the logical port > specified in the NAT rule. > If <ref column="logical_port" > diff --git a/ovn-nb.xml b/ovn-nb.xml > index 9010240a8..288d00f00 100644 > --- a/ovn-nb.xml > +++ b/ovn-nb.xml > @@ -3366,14 +3366,6 @@ > 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 > @@ -3391,6 +3383,17 @@ > <ref column="networks" table="Logical_Router_Port"/> > <code>50.0.0.10/24</code>. > </p> > + > + <p> > + When a logical router has multiple distributed gateway ports and this > + column is not set for a NAT rule, then the rule will be applied at the > + distributed gateway port which is in the same network as the > + <ref column="external_ip"/> of the NAT rule, if such a router port > + exists. If logical router has a single distributed gateway port and > + this column is not set for a NAT rule, the rule will be applied at the > + distributed gateway port even if the router port is not in the same > + network as the <ref column="external_ip"/> of the NAT rule. > + </p> > </column> > > <column name="options" key="stateless"> > diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at > index f9b9defd0..8b8801af0 100644 > --- a/tests/ovn-nbctl.at > +++ b/tests/ovn-nbctl.at > @@ -521,28 +521,28 @@ snat 30.0.0.1 192.1 > 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, logical_ip and gateway_port already exists > +[ovn-nbctl: 30.0.0.1, 192.168.1.0/24: a NAT with this external_ip and logical_ip already exists > ]) > AT_CHECK([ovn-nbctl lr-nat-add lr0 snat 30.0.0.1 192.168.1.10/24], [1], [], > -[ovn-nbctl: 30.0.0.1, 192.168.1.0/24, : a NAT with this external_ip, logical_ip and gateway_port already exists > +[ovn-nbctl: 30.0.0.1, 192.168.1.0/24: a NAT with this external_ip and logical_ip already exists > ]) > AT_CHECK([ovn-nbctl --may-exist lr-nat-add lr0 snat 30.0.0.1 192.168.1.0/24]) > AT_CHECK([ovn-nbctl lr-nat-add lr0 snat 30.0.0.2 192.168.1.0/24], [1], [], > -[ovn-nbctl: a NAT with this type (snat), logical_ip (192.168.1.0/24) and gateway_port () already exists > +[ovn-nbctl: a NAT with this type (snat), logical_ip (192.168.1.0/24) already exists > ]) > AT_CHECK([ovn-nbctl lr-nat-add lr0 dnat 30.0.0.1 192.168.1.2], [1], [], > -[ovn-nbctl: 30.0.0.1, 192.168.1.2, : a NAT with this external_ip, logical_ip and gateway_port already exists > +[ovn-nbctl: 30.0.0.1, 192.168.1.2: a NAT with this external_ip and logical_ip already exists > ]) > AT_CHECK([ovn-nbctl --may-exist lr-nat-add lr0 dnat 30.0.0.1 192.168.1.2]) > AT_CHECK([ovn-nbctl lr-nat-add lr0 dnat 30.0.0.1 192.168.1.3], [1], [], > -[ovn-nbctl: a NAT with this type (dnat), external_ip (30.0.0.1) and gateway_port () already exists > +[ovn-nbctl: a NAT with this type (dnat), external_ip (30.0.0.1) already exists > ]) > AT_CHECK([ovn-nbctl lr-nat-add lr0 dnat_and_snat 30.0.0.1 192.168.1.2], [1], [], > -[ovn-nbctl: 30.0.0.1, 192.168.1.2, : a NAT with this external_ip, logical_ip and gateway_port already exists > +[ovn-nbctl: 30.0.0.1, 192.168.1.2: a NAT with this external_ip and logical_ip already exists > ]) > AT_CHECK([ovn-nbctl --may-exist lr-nat-add lr0 dnat_and_snat 30.0.0.1 192.168.1.2]) > AT_CHECK([ovn-nbctl lr-nat-add lr0 dnat_and_snat 30.0.0.1 192.168.1.3], [1], [], > -[ovn-nbctl: a NAT with this type (dnat_and_snat), external_ip (30.0.0.1) and gateway_port () already exists > +[ovn-nbctl: a NAT with this type (dnat_and_snat), external_ip (30.0.0.1) already exists > ]) > AT_CHECK([ovn-nbctl --may-exist lr-nat-add lr0 dnat_and_snat 30.0.0.2 192.168.1.3 lp0 00:00:00:04:05:06]) > AT_CHECK([ovn-nbctl lr-nat-list lr0], [0], [dnl > @@ -754,7 +754,6 @@ 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. > ]) > @@ -764,50 +763,54 @@ AT_CHECK([ovn-nbctl --gateway-port=lrp10 lr-nat-add lr0 dnat_and_snat 172.64.2.1 > > 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 snat 172.64.0.10 20.0.0.10]) > +AT_CHECK([ovn-nbctl lr-nat-add lr0 snat 172.64.1.10 20.0.0.10]) > +AT_CHECK([ovn-nbctl lr-nat-add lr0 dnat 30.0.0.10 20.0.0.10], [1], [], > +[ovn-nbctl: logical router: lr0 has multiple distributed gateway ports and gateway_port can not be determined from external IP of NAT rule. > ]) > 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 --gateway-port=lrp01 lr-nat-add lr0 snat 172.64.1.10 20.0.0.10], [1], [], > +[ovn-nbctl: 172.64.1.10, 20.0.0.10: a NAT with this external_ip and logical_ip already exists > ]) > +AT_CHECK([ovn-nbctl --gateway-port=lrp00 lr-nat-add lr0 dnat 30.0.0.10 20.0.0.11], [1], [], > +[ovn-nbctl: a NAT with this type (dnat), external_ip (30.0.0.10) already exists > +]) > +AT_CHECK([ovn-nbctl --gateway-port=lrp01 lr-nat-add lr0 dnat 30.0.0.10 20.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 > dnat lrp00 30.0.0.10 20.0.0.10 > dnat lrp01 30.0.0.10 20.0.0.10 > +snat 172.64.1.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 172.64.1.10 20.0.0.10 > snat lrp00 172.64.0.10 20.0.0.10 > ]) > > AT_CHECK([ovn-nbctl --gateway-port=lrp00 lr-nat-add lr0 snat 30.0.0.10 20.0.0.20]) > -AT_CHECK([ovn-nbctl --gateway-port=lrp01 lr-nat-add lr0 snat 30.0.0.10 20.0.0.20]) > AT_CHECK([ovn-nbctl lr-nat-list lr0], [0], [dnl > TYPE GATEWAY_PORT EXTERNAL_IP EXTERNAL_PORT LOGICAL_IP EXTERNAL_MAC LOGICAL_PORT > +snat 172.64.1.10 20.0.0.10 > snat lrp00 172.64.0.10 20.0.0.10 > snat lrp00 30.0.0.10 20.0.0.20 > -snat lrp01 30.0.0.10 20.0.0.20 > ]) > AT_CHECK([ovn-nbctl lr-nat-del lr0 dnat lrp11], [1], [], > [ovn-nbctl: lrp11: port name not found > ]) > -AT_CHECK([ovn-nbctl lr-nat-del lr0 dnat lrp00]) > +AT_CHECK([ovn-nbctl lr-nat-del lr0 snat lrp00]) > AT_CHECK([ovn-nbctl lr-nat-list lr0], [0], [dnl > TYPE GATEWAY_PORT EXTERNAL_IP EXTERNAL_PORT LOGICAL_IP EXTERNAL_MAC LOGICAL_PORT > -snat lrp00 172.64.0.10 20.0.0.10 > -snat lrp00 30.0.0.10 20.0.0.20 > -snat lrp01 30.0.0.10 20.0.0.20 > +snat 172.64.1.10 20.0.0.10 > ]) > -AT_CHECK([ovn-nbctl lr-nat-del lr0 snat lrp00]) > +AT_CHECK([ovn-nbctl lr-nat-del lr0 snat lrp01]) > AT_CHECK([ovn-nbctl lr-nat-list lr0], [0], [dnl > TYPE GATEWAY_PORT EXTERNAL_IP EXTERNAL_PORT LOGICAL_IP EXTERNAL_MAC LOGICAL_PORT > -snat lrp01 30.0.0.10 20.0.0.20 > +snat 172.64.1.10 20.0.0.10 > ]) > > AT_CHECK([ovn-nbctl lr-nat-del lr0 snat 20.0.0 lrp01], [1], [], > @@ -817,7 +820,7 @@ AT_CHECK([ovn-nbctl lr-nat-del lr0 snat 20.0.0.10 lrp01], [1], [], > [ovn-nbctl: no matching NAT with the type (snat), logical_ip (20.0.0.10) and gateway_port (lrp01) > ]) > AT_CHECK([ovn-nbctl --if-exists lr-nat-del lr0 snat 20.0.0.10 lrp01]) > -AT_CHECK([ovn-nbctl lr-nat-del lr0 snat 20.0.0.20 lrp01]) > +AT_CHECK([ovn-nbctl lr-nat-del lr0 snat]) > ]) > > dnl --------------------------------------------------------------------- > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > index 69ad85533..4b49540a9 100644 > --- a/tests/ovn-northd.at > +++ b/tests/ovn-northd.at > @@ -6536,18 +6536,17 @@ 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 > > -# Configure SNAT > -check ovn-nbctl --gateway-port=DR-S1 lr-nat-add DR snat 172.16.1.10 20.0.0.10 > +# Configure SNAT with and without setting "gateway_port" column > +check ovn-nbctl 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 > +check ovn-nbctl lr-nat-add DR snat 192.168.0.10 20.0.0.10 > > check ovn-nbctl --wait=sb sync > > 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 > +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;) > @@ -6558,10 +6557,8 @@ check_lr_in_arp_nat_flows() { > 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 > +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;) > @@ -6569,10 +6566,8 @@ check_lr_in_unsnat_flows() { > 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 > +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);) > @@ -6580,45 +6575,44 @@ check_lr_out_snat_flows() { > 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 --wait=sb 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 > +# Configure DNAT - 2 gateway_ports configured for same external IP > +check ovn-nbctl 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 > +check ovn-nbctl --gateway-port=DR-S3 lr-nat-add DR dnat 172.16.1.10 20.0.0.10 > > check ovn-nbctl --wait=sb sync > > 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 > +AT_CHECK([grep lr_in_ip_input lrflows | grep arp | grep -e 172.16.1.10 -e 10.0.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 == 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=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 == 172.16.1.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 == 172.16.1.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;) > +]) > + > +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);) > + table=??(lr_in_dnat ), priority=100 , match=(ip && ip4.dst == 172.16.1.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 > +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 --wait=sb lr-nat-del DR dnat > > @@ -6627,7 +6621,7 @@ AT_CHECK([ovn-sbctl dump-flows DR | grep -e lr_in_dnat -e lr_out_undnat | grep c > > # 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 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 > > check ovn-nbctl --wait=sb sync > @@ -6635,11 +6629,47 @@ check ovn-nbctl --wait=sb sync > 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 > +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;) > +]) > + > +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;) > +]) > + > +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);) > +]) > + > +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);) > +]) > + > +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 ovn-nbctl --wait=sb lr-nat-del DR dnat_and_snat > > diff --git a/tests/ovn.at b/tests/ovn.at > index 799a6aabd..fefad934b 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -29869,7 +29869,7 @@ ovn-nbctl --wait=hv sync > AT_CHECK([as hv2 ovs-ofctl dump-flows br-int table=26 | grep 10.0.1.2], [1]) > > # Enable dnat_and_snat on lr, and now hv2 should see flows for lsp1. > -AT_CHECK([ovn-nbctl --wait=hv --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([ovn-nbctl --wait=hv --gateway-port=lrp_lr_ls1 lr-nat-add lr dnat_and_snat 192.168.0.1 10.0.1.3 lsp1 f0:00:00:00:00:03]) > AT_CHECK([as hv2 ovs-ofctl dump-flows br-int table=26 | grep 10.0.1.2], [0], [ignore]) > > OVN_CLEANUP([hv1],[hv2]) > diff --git a/utilities/ovn-nbctl.8.xml b/utilities/ovn-nbctl.8.xml > index 3e9176fa0..040d05227 100644 > --- a/utilities/ovn-nbctl.8.xml > +++ b/utilities/ovn-nbctl.8.xml > @@ -1174,8 +1174,9 @@ > 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>. > + distributed gateway ports and the gateway port for this NAT can't > + be inferred from the <var>external_ip</var>, it is an error to not > + specify the <var>GATEWAY_PORT</var>. > </p> > > <p> > diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c > index e747f6933..68b158e9c 100644 > --- a/utilities/ovn-nbctl.c > +++ b/utilities/ovn-nbctl.c > @@ -4569,6 +4569,54 @@ done: > return ret; > } > > +static bool > +ip_in_lrp_networks(const struct nbrec_logical_router_port *lrp, The vast majority of this function is a copy of find_lrp_member_ip() from northd.c . It's probably best to move the common functionality into a function in lib/ovn-util.c and keep the northd and ovn-nbctl-specific parts locally. So if the common function in ovn-util is const char * find_lport_address(struct lport_addresses *addrs, const char *ip_s); (there's probably a better name than that) then northd.c would have: static const char * find_lrp_member_ip(const struct ovn_port *op, const char *ip_s) { return find_lport_address(&op->lrp_networks, ip_s); } and ovn-nbctl would have: static bool ip_in_lrp_networks(const struct nbrec_logical_rouer_port *lrp, const char *ip_s) { struct lport_addresses lrp_networks; extract_lrp_networks(lrp, &lrp_networks); bool retval = find_lport_address(&lrp_networks, ip_s) ? true : false; destroy_lport_addresses(&lrp_networks); return retval; } > + const char *ip_s) { > + struct lport_addresses lrp_networks; > + extract_lrp_networks(lrp, &lrp_networks); > + > + bool is_ipv4 = strchr(ip_s, '.') ? true : false; > + > + if (is_ipv4) { > + ovs_be32 ip; > + > + if (!ip_parse(ip_s, &ip)) { > + destroy_lport_addresses(&lrp_networks); > + return false; > + } > + > + for (int i = 0; i < lrp_networks.n_ipv4_addrs; i++) { > + const struct ipv4_netaddr *na = &lrp_networks.ipv4_addrs[i]; > + > + if (!((na->network ^ ip) & na->mask)) { > + destroy_lport_addresses(&lrp_networks); > + return true; > + } > + } > + } else { > + struct in6_addr ip6; > + > + if (!ipv6_parse(ip_s, &ip6)) { > + destroy_lport_addresses(&lrp_networks); > + return false; > + } > + > + for (int i = 0; i < lrp_networks.n_ipv6_addrs; i++) { > + const struct ipv6_netaddr *na = &lrp_networks.ipv6_addrs[i]; > + struct in6_addr xor_addr = ipv6_addr_bitxor(&na->network, &ip6); > + struct in6_addr and_addr = ipv6_addr_bitand(&xor_addr, &na->mask); > + > + if (ipv6_is_zero(&and_addr)) { > + destroy_lport_addresses(&lrp_networks); > + return true; > + } > + } > + } > + > + destroy_lport_addresses(&lrp_networks); > + return false; > +} > + > static void > nbctl_pre_lr_nat_add(struct ctl_context *ctx) > { > @@ -4587,6 +4635,8 @@ nbctl_pre_lr_nat_add(struct ctl_context *ctx) > 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_mac); > + ovsdb_idl_add_column(ctx->idl, &nbrec_logical_router_port_col_networks); > ovsdb_idl_add_column(ctx->idl, > &nbrec_logical_router_port_col_gateway_chassis); > ovsdb_idl_add_column(ctx->idl, > @@ -4729,6 +4779,8 @@ nbctl_lr_nat_add(struct ctl_context *ctx) > const char *dgw_port_name = shash_find_data(&ctx->options, > "--gateway-port"); > const struct nbrec_logical_router_port *dgw_port = NULL; > + size_t num_l3dgw_ports = 0; > + > if (dgw_port_name) { > error = lrp_by_name_or_uuid(ctx, dgw_port_name, > true, &dgw_port); > @@ -4737,14 +4789,17 @@ nbctl_lr_nat_add(struct ctl_context *ctx) > goto cleanup; > } > > - bool nat_lr_port = false; > + bool is_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->ha_chassis_group || lrp->n_gateway_chassis) { > + num_l3dgw_ports++; > + } > if (lrp == dgw_port) { > - nat_lr_port = true; > + is_lr_port = true; > } > } > - if (!nat_lr_port) { > + if (!is_lr_port) { > ctl_error(ctx, "%s is not a router port of logical router: %s.", > dgw_port_name, ctx->argv[1]); > goto cleanup; > @@ -4756,17 +4811,19 @@ nbctl_lr_nat_add(struct ctl_context *ctx) > 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 (ip_in_lrp_networks(lrp, new_external_ip)) { > + dgw_port = lrp; > + } > } > } > - if (num_l3dgw_ports > 1) { > + if (num_l3dgw_ports > 1 && !dgw_port) { > ctl_error(ctx, "logical router: %s has multiple distributed " > - "gateway ports. NAT rule needs to specify " > - "gateway_port.", ctx->argv[1]); > + "gateway ports and gateway_port can not be determined " > + "from external IP of NAT rule.", ctx->argv[1]); > goto cleanup; > } > } > @@ -4787,8 +4844,11 @@ nbctl_lr_nat_add(struct ctl_context *ctx) > continue; > } > > - if (!strcmp(nat_type, nat->type) && > - dgw_port == nat->gateway_port) { > + if (!strcmp(nat_type, nat->type) > + && (num_l3dgw_ports <= 1 > + || (nat->gateway_port && nat->gateway_port == dgw_port) > + || (!nat->gateway_port > + && ip_in_lrp_networks(dgw_port, old_external_ip)))) { > 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, > @@ -4800,20 +4860,18 @@ 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, %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 : ""); > + ctl_error(ctx, "%s, %s: a NAT with this " > + "external_ip and logical_ip already " > + "exists", new_external_ip, > + new_logical_ip); > should_return = true; > } > } else { > ctl_error(ctx, "a NAT with this type (%s), %s (%s) " > - "and gateway_port (%s) already exists", > + "already exists", > nat_type, > is_snat ? "logical_ip" : "external_ip", > - is_snat ? new_logical_ip : new_external_ip, > - dgw_port ? dgw_port->name : ""); > + is_snat ? new_logical_ip : new_external_ip); > should_return = true; > } > } > @@ -4858,7 +4916,7 @@ nbctl_lr_nat_add(struct ctl_context *ctx) > smap_add(&nat_options, "add_route", "true"); > } > > - if (dgw_port) { > + if (dgw_port_name) { > nbrec_nat_update_gateway_port_addvalue(nat, dgw_port); > } > nbrec_nat_set_options(nat, &nat_options); >
Hi Mark, Thanks for reviewing the patch. I have made the suggested changes and sent out a v2 patch. Thanks, Abhiram Sangana On 19 May 2022, at 21:06, Mark Michelson <mmichels@redhat.com<mailto:mmichels@redhat.com>> wrote: Hi Abhiram, This is a great idea. I only have a couple of minor comments below.
diff --git a/northd/northd.c b/northd/northd.c index a56666297..aa1c2ae05 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -2729,14 +2729,17 @@ join_logical_ports(struct northd_input *input_data, } } +static const char *find_lrp_member_ip(const struct ovn_port *op, + const char *ip_s); + /* Returns an array of strings, each consisting of a MAC address followed * 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 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. + * external IP addresses of NAT rules defined on that router whose + * gateway_port is 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(), * and must free the returned array when it is no longer needed. */ @@ -2779,7 +2782,9 @@ get_nat_addresses(const struct ovn_port *op, size_t *n, bool routable_only, /* Not including external IP of NAT rules whose gateway_port is * not 'op'. */ - if (nat->gateway_port && nat->gateway_port != op->nbrp) { + if (op->od->n_l3dgw_ports > 1 && + ((!nat->gateway_port && !find_lrp_member_ip(op, nat->external_ip)) + || (nat->gateway_port && nat->gateway_port != op->nbrp))) { continue; } @@ -3454,8 +3459,12 @@ ovn_port_update_sbrec(struct northd_input *input_data, struct ds nat_addr = DS_EMPTY_INITIALIZER; ds_put_format(&nat_addr, "%s", nat_addresses); if (l3dgw_ports) { + const struct ovn_port *l3dgw_port = ( + is_l3dgw_port(op->peer) + ? op->peer + : op->peer->od->l3dgw_ports[0]); ds_put_format(&nat_addr, " is_chassis_resident(%s)", - op->peer->od->l3dgw_ports[0]->cr_port->json_key); + l3dgw_port->cr_port->json_key); } nats[0] = xstrdup(ds_cstr(&nat_addr)); ds_destroy(&nat_addr); @@ -10502,9 +10511,11 @@ 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 + /* ARP/ND should be sent from distributed gateway port relevant to * the NAT rule. */ - if (nat->gateway_port && nat->gateway_port != op->nbrp) { + if (op->od->n_l3dgw_ports > 1 && + ((!nat->gateway_port && !find_lrp_member_ip(op, nat->external_ip)) || + (nat->gateway_port && nat->gateway_port != op->nbrp))) { return; } @@ -13287,14 +13298,24 @@ lrouter_check_nat_entry(struct ovn_datapath *od, const struct nbrec_nat *nat, /* 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) { + if (od->n_l3dgw_ports == 1) { *nat_l3dgw_port = od->l3dgw_ports[0]; + } else if (od->n_l3dgw_ports > 1) { + /* Find the DGP reachable for the NAT external IP. */ + for (size_t i = 0; i < od->n_l3dgw_ports; i++) { + if (find_lrp_member_ip(od->l3dgw_ports[i], nat->external_ip)) { + *nat_l3dgw_port = od->l3dgw_ports[i]; + break; + } + } + if (*nat_l3dgw_port == NULL) { + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); + VLOG_WARN_RL(&rl, "Unable to determine gateway_port for NAT " + "with external_ip: %s configured on logical " + "router: %s with multiple distributed gateway " + "ports", nat->external_ip, od->nbr->name); + return -EINVAL; + } } } else { *nat_l3dgw_port = ovn_port_find(ports, nat->gateway_port->name); diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml index 0fe350d0e..948c929e7 100644 --- a/northd/ovn-northd.8.xml +++ b/northd/ovn-northd.8.xml @@ -2137,7 +2137,8 @@ output; router that specifies an external Ethernet address <var>E</var>, a priority-50 flow that matches <code>inport == <var>GW</var> && eth.dst == <var>E</var></code>, where <var>GW</var> - is the logical router gateway port, with action + is the logical router distributed gateway port corresponding to the + NAT rule (specified or inferred), with action <code>xreg0[0..47]=E; next;</code>. </p> @@ -2453,11 +2454,12 @@ icmp6_error { <li> <p> For each NAT entry of a distributed logical router (with - distributed gateway router port) of type <code>snat</code>, + distributed gateway router port(s)) of type <code>snat</code>, a priority-120 flow with the match <code>inport == <var>P</var> && ip4.src == <var>A</var></code> advances the packet to the next pipeline, where <var>P</var> is the distributed logical - router port and <var>A</var> is the <code>external_ip</code> set + router port corresponding to the NAT entry (specified or inferred) + and <var>A</var> is the <code>external_ip</code> set in the NAT entry. If <var>A</var> is an IPv6 address, then <code>ip6.src</code> is used for the match. </p> @@ -3057,7 +3059,7 @@ icmp6 { ip6.dst == <var>B</var> && inport == <var>GW</var> && flags.loopback == 0</code> where <var>GW</var> is the distributed gateway port - specified in the NAT rule, with an + corresponding to the NAT rule (specified or inferred), 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 @@ -3083,7 +3085,7 @@ icmp6 { && flags.loopback == 0 && flags.use_snat_zone == 1</code> where <var>GW</var> is the distributed gateway port - specified in the NAT rule, with an + corresponding to the NAT rule (specified or inferred), 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 @@ -3364,8 +3366,8 @@ 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 configured - for the NAT rule, with an action + where <var>GW</var> is the logical router gateway port corresponding + to the NAT rule (specified or inferred), 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 @@ -4683,7 +4685,8 @@ 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 distributed gateway port specified in the NAT rule. + is the distributed gateway port corresponding to the NAT rule + (specified or inferred). For dnat_and_snat NAT rule, <var>P</var> is the logical port specified in the NAT rule. If <ref column="logical_port" diff --git a/ovn-nb.xml b/ovn-nb.xml index 9010240a8..288d00f00 100644 --- a/ovn-nb.xml +++ b/ovn-nb.xml @@ -3366,14 +3366,6 @@ 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 @@ -3391,6 +3383,17 @@ <ref column="networks" table="Logical_Router_Port"/> <code>50.0.0.10/24</code>. </p> + + <p> + When a logical router has multiple distributed gateway ports and this + column is not set for a NAT rule, then the rule will be applied at the + distributed gateway port which is in the same network as the + <ref column="external_ip"/> of the NAT rule, if such a router port + exists. If logical router has a single distributed gateway port and + this column is not set for a NAT rule, the rule will be applied at the + distributed gateway port even if the router port is not in the same + network as the <ref column="external_ip"/> of the NAT rule. + </p> </column> <column name="options" key="stateless"> diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at index f9b9defd0..8b8801af0 100644 --- a/tests/ovn-nbctl.at +++ b/tests/ovn-nbctl.at @@ -521,28 +521,28 @@ snat 30.0.0.1 192.1 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, logical_ip and gateway_port already exists +[ovn-nbctl: 30.0.0.1, 192.168.1.0/24: a NAT with this external_ip and logical_ip already exists ]) AT_CHECK([ovn-nbctl lr-nat-add lr0 snat 30.0.0.1 192.168.1.10/24], [1], [], -[ovn-nbctl: 30.0.0.1, 192.168.1.0/24, : a NAT with this external_ip, logical_ip and gateway_port already exists +[ovn-nbctl: 30.0.0.1, 192.168.1.0/24: a NAT with this external_ip and logical_ip already exists ]) AT_CHECK([ovn-nbctl --may-exist lr-nat-add lr0 snat 30.0.0.1 192.168.1.0/24]) AT_CHECK([ovn-nbctl lr-nat-add lr0 snat 30.0.0.2 192.168.1.0/24], [1], [], -[ovn-nbctl: a NAT with this type (snat), logical_ip (192.168.1.0/24) and gateway_port () already exists +[ovn-nbctl: a NAT with this type (snat), logical_ip (192.168.1.0/24) already exists ]) AT_CHECK([ovn-nbctl lr-nat-add lr0 dnat 30.0.0.1 192.168.1.2], [1], [], -[ovn-nbctl: 30.0.0.1, 192.168.1.2, : a NAT with this external_ip, logical_ip and gateway_port already exists +[ovn-nbctl: 30.0.0.1, 192.168.1.2: a NAT with this external_ip and logical_ip already exists ]) AT_CHECK([ovn-nbctl --may-exist lr-nat-add lr0 dnat 30.0.0.1 192.168.1.2]) AT_CHECK([ovn-nbctl lr-nat-add lr0 dnat 30.0.0.1 192.168.1.3], [1], [], -[ovn-nbctl: a NAT with this type (dnat), external_ip (30.0.0.1) and gateway_port () already exists +[ovn-nbctl: a NAT with this type (dnat), external_ip (30.0.0.1) already exists ]) AT_CHECK([ovn-nbctl lr-nat-add lr0 dnat_and_snat 30.0.0.1 192.168.1.2], [1], [], -[ovn-nbctl: 30.0.0.1, 192.168.1.2, : a NAT with this external_ip, logical_ip and gateway_port already exists +[ovn-nbctl: 30.0.0.1, 192.168.1.2: a NAT with this external_ip and logical_ip already exists ]) AT_CHECK([ovn-nbctl --may-exist lr-nat-add lr0 dnat_and_snat 30.0.0.1 192.168.1.2]) AT_CHECK([ovn-nbctl lr-nat-add lr0 dnat_and_snat 30.0.0.1 192.168.1.3], [1], [], -[ovn-nbctl: a NAT with this type (dnat_and_snat), external_ip (30.0.0.1) and gateway_port () already exists +[ovn-nbctl: a NAT with this type (dnat_and_snat), external_ip (30.0.0.1) already exists ]) AT_CHECK([ovn-nbctl --may-exist lr-nat-add lr0 dnat_and_snat 30.0.0.2 192.168.1.3 lp0 00:00:00:04:05:06]) AT_CHECK([ovn-nbctl lr-nat-list lr0], [0], [dnl @@ -754,7 +754,6 @@ 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. ]) @@ -764,50 +763,54 @@ AT_CHECK([ovn-nbctl --gateway-port=lrp10 lr-nat-add lr0 dnat_and_snat 172.64.2.1 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 snat 172.64.0.10 20.0.0.10]) +AT_CHECK([ovn-nbctl lr-nat-add lr0 snat 172.64.1.10 20.0.0.10]) +AT_CHECK([ovn-nbctl lr-nat-add lr0 dnat 30.0.0.10 20.0.0.10], [1], [], +[ovn-nbctl: logical router: lr0 has multiple distributed gateway ports and gateway_port can not be determined from external IP of NAT rule. ]) 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 --gateway-port=lrp01 lr-nat-add lr0 snat 172.64.1.10 20.0.0.10], [1], [], +[ovn-nbctl: 172.64.1.10, 20.0.0.10: a NAT with this external_ip and logical_ip already exists ]) +AT_CHECK([ovn-nbctl --gateway-port=lrp00 lr-nat-add lr0 dnat 30.0.0.10 20.0.0.11], [1], [], +[ovn-nbctl: a NAT with this type (dnat), external_ip (30.0.0.10) already exists +]) +AT_CHECK([ovn-nbctl --gateway-port=lrp01 lr-nat-add lr0 dnat 30.0.0.10 20.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 dnat lrp00 30.0.0.10 20.0.0.10 dnat lrp01 30.0.0.10 20.0.0.10 +snat 172.64.1.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 172.64.1.10 20.0.0.10 snat lrp00 172.64.0.10 20.0.0.10 ]) AT_CHECK([ovn-nbctl --gateway-port=lrp00 lr-nat-add lr0 snat 30.0.0.10 20.0.0.20]) -AT_CHECK([ovn-nbctl --gateway-port=lrp01 lr-nat-add lr0 snat 30.0.0.10 20.0.0.20]) AT_CHECK([ovn-nbctl lr-nat-list lr0], [0], [dnl TYPE GATEWAY_PORT EXTERNAL_IP EXTERNAL_PORT LOGICAL_IP EXTERNAL_MAC LOGICAL_PORT +snat 172.64.1.10 20.0.0.10 snat lrp00 172.64.0.10 20.0.0.10 snat lrp00 30.0.0.10 20.0.0.20 -snat lrp01 30.0.0.10 20.0.0.20 ]) AT_CHECK([ovn-nbctl lr-nat-del lr0 dnat lrp11], [1], [], [ovn-nbctl: lrp11: port name not found ]) -AT_CHECK([ovn-nbctl lr-nat-del lr0 dnat lrp00]) +AT_CHECK([ovn-nbctl lr-nat-del lr0 snat lrp00]) AT_CHECK([ovn-nbctl lr-nat-list lr0], [0], [dnl TYPE GATEWAY_PORT EXTERNAL_IP EXTERNAL_PORT LOGICAL_IP EXTERNAL_MAC LOGICAL_PORT -snat lrp00 172.64.0.10 20.0.0.10 -snat lrp00 30.0.0.10 20.0.0.20 -snat lrp01 30.0.0.10 20.0.0.20 +snat 172.64.1.10 20.0.0.10 ]) -AT_CHECK([ovn-nbctl lr-nat-del lr0 snat lrp00]) +AT_CHECK([ovn-nbctl lr-nat-del lr0 snat lrp01]) AT_CHECK([ovn-nbctl lr-nat-list lr0], [0], [dnl TYPE GATEWAY_PORT EXTERNAL_IP EXTERNAL_PORT LOGICAL_IP EXTERNAL_MAC LOGICAL_PORT -snat lrp01 30.0.0.10 20.0.0.20 +snat 172.64.1.10 20.0.0.10 ]) AT_CHECK([ovn-nbctl lr-nat-del lr0 snat 20.0.0 lrp01], [1], [], @@ -817,7 +820,7 @@ AT_CHECK([ovn-nbctl lr-nat-del lr0 snat 20.0.0.10 lrp01], [1], [], [ovn-nbctl: no matching NAT with the type (snat), logical_ip (20.0.0.10) and gateway_port (lrp01) ]) AT_CHECK([ovn-nbctl --if-exists lr-nat-del lr0 snat 20.0.0.10 lrp01]) -AT_CHECK([ovn-nbctl lr-nat-del lr0 snat 20.0.0.20 lrp01]) +AT_CHECK([ovn-nbctl lr-nat-del lr0 snat]) ]) dnl --------------------------------------------------------------------- diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index 69ad85533..4b49540a9 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -6536,18 +6536,17 @@ 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 -# Configure SNAT -check ovn-nbctl --gateway-port=DR-S1 lr-nat-add DR snat 172.16.1.10 20.0.0.10 +# Configure SNAT with and without setting "gateway_port" column +check ovn-nbctl 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 +check ovn-nbctl lr-nat-add DR snat 192.168.0.10 20.0.0.10 check ovn-nbctl --wait=sb sync 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 +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;) @@ -6558,10 +6557,8 @@ check_lr_in_arp_nat_flows() { 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 +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;) @@ -6569,10 +6566,8 @@ check_lr_in_unsnat_flows() { 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 +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);) @@ -6580,45 +6575,44 @@ check_lr_out_snat_flows() { 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 --wait=sb 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 +# Configure DNAT - 2 gateway_ports configured for same external IP +check ovn-nbctl 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 +check ovn-nbctl --gateway-port=DR-S3 lr-nat-add DR dnat 172.16.1.10 20.0.0.10 check ovn-nbctl --wait=sb sync 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 +AT_CHECK([grep lr_in_ip_input lrflows | grep arp | grep -e 172.16.1.10 -e 10.0.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 == 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=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 == 172.16.1.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 == 172.16.1.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;) +]) + +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);) + table=??(lr_in_dnat ), priority=100 , match=(ip && ip4.dst == 172.16.1.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 +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 --wait=sb lr-nat-del DR dnat @@ -6627,7 +6621,7 @@ AT_CHECK([ovn-sbctl dump-flows DR | grep -e lr_in_dnat -e lr_out_undnat | grep c # 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 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 check ovn-nbctl --wait=sb sync @@ -6635,11 +6629,47 @@ check ovn-nbctl --wait=sb sync 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 +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;) +]) + +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;) +]) + +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);) +]) + +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);) +]) + +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 ovn-nbctl --wait=sb lr-nat-del DR dnat_and_snat diff --git a/tests/ovn.at b/tests/ovn.at index 799a6aabd..fefad934b 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -29869,7 +29869,7 @@ ovn-nbctl --wait=hv sync AT_CHECK([as hv2 ovs-ofctl dump-flows br-int table=26 | grep 10.0.1.2], [1]) # Enable dnat_and_snat on lr, and now hv2 should see flows for lsp1. -AT_CHECK([ovn-nbctl --wait=hv --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([ovn-nbctl --wait=hv --gateway-port=lrp_lr_ls1 lr-nat-add lr dnat_and_snat 192.168.0.1 10.0.1.3 lsp1 f0:00:00:00:00:03]) AT_CHECK([as hv2 ovs-ofctl dump-flows br-int table=26 | grep 10.0.1.2], [0], [ignore]) OVN_CLEANUP([hv1],[hv2]) diff --git a/utilities/ovn-nbctl.8.xml b/utilities/ovn-nbctl.8.xml index 3e9176fa0..040d05227 100644 --- a/utilities/ovn-nbctl.8.xml +++ b/utilities/ovn-nbctl.8.xml @@ -1174,8 +1174,9 @@ 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>. + distributed gateway ports and the gateway port for this NAT can't + be inferred from the <var>external_ip</var>, it is an error to not + specify the <var>GATEWAY_PORT</var>. </p> <p> diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c index e747f6933..68b158e9c 100644 --- a/utilities/ovn-nbctl.c +++ b/utilities/ovn-nbctl.c @@ -4569,6 +4569,54 @@ done: return ret; } +static bool +ip_in_lrp_networks(const struct nbrec_logical_router_port *lrp, + const char *ip_s) { + struct lport_addresses lrp_networks; + extract_lrp_networks(lrp, &lrp_networks); + + bool is_ipv4 = strchr(ip_s, '.') ? true : false; + + if (is_ipv4) { + ovs_be32 ip; + + if (!ip_parse(ip_s, &ip)) { + destroy_lport_addresses(&lrp_networks); + return false; + } + + for (int i = 0; i < lrp_networks.n_ipv4_addrs; i++) { + const struct ipv4_netaddr *na = &lrp_networks.ipv4_addrs[i]; + + if (!((na->network ^ ip) & na->mask)) { + destroy_lport_addresses(&lrp_networks); + return true; + } + } + } else { + struct in6_addr ip6; + + if (!ipv6_parse(ip_s, &ip6)) { + destroy_lport_addresses(&lrp_networks); + return false; + } + + for (int i = 0; i < lrp_networks.n_ipv6_addrs; i++) { + const struct ipv6_netaddr *na = &lrp_networks.ipv6_addrs[i]; + struct in6_addr xor_addr = ipv6_addr_bitxor(&na->network, &ip6); + struct in6_addr and_addr = ipv6_addr_bitand(&xor_addr, &na->mask); + + if (ipv6_is_zero(&and_addr)) { + destroy_lport_addresses(&lrp_networks); + return true; + } + } + } + + destroy_lport_addresses(&lrp_networks); + return false; +} + static void nbctl_pre_lr_nat_add(struct ctl_context *ctx) { @@ -4587,6 +4635,8 @@ nbctl_pre_lr_nat_add(struct ctl_context *ctx) 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_mac); + ovsdb_idl_add_column(ctx->idl, &nbrec_logical_router_port_col_networks); ovsdb_idl_add_column(ctx->idl, &nbrec_logical_router_port_col_gateway_chassis); ovsdb_idl_add_column(ctx->idl, @@ -4729,6 +4779,8 @@ nbctl_lr_nat_add(struct ctl_context *ctx) const char *dgw_port_name = shash_find_data(&ctx->options, "--gateway-port"); const struct nbrec_logical_router_port *dgw_port = NULL; + size_t num_l3dgw_ports = 0; + if (dgw_port_name) { error = lrp_by_name_or_uuid(ctx, dgw_port_name, true, &dgw_port); @@ -4737,14 +4789,17 @@ nbctl_lr_nat_add(struct ctl_context *ctx) goto cleanup; } - bool nat_lr_port = false; + bool is_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->ha_chassis_group || lrp->n_gateway_chassis) { + num_l3dgw_ports++; + } if (lrp == dgw_port) { - nat_lr_port = true; + is_lr_port = true; } } - if (!nat_lr_port) { + if (!is_lr_port) { ctl_error(ctx, "%s is not a router port of logical router: %s.", dgw_port_name, ctx->argv[1]); goto cleanup; @@ -4756,17 +4811,19 @@ nbctl_lr_nat_add(struct ctl_context *ctx) 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 (ip_in_lrp_networks(lrp, new_external_ip)) { + dgw_port = lrp; + } } } - if (num_l3dgw_ports > 1) { + if (num_l3dgw_ports > 1 && !dgw_port) { ctl_error(ctx, "logical router: %s has multiple distributed " - "gateway ports. NAT rule needs to specify " - "gateway_port.", ctx->argv[1]); + "gateway ports and gateway_port can not be determined " + "from external IP of NAT rule.", ctx->argv[1]); goto cleanup; } } @@ -4787,8 +4844,11 @@ nbctl_lr_nat_add(struct ctl_context *ctx) continue; } - if (!strcmp(nat_type, nat->type) && - dgw_port == nat->gateway_port) { + if (!strcmp(nat_type, nat->type) + && (num_l3dgw_ports <= 1 + || (nat->gateway_port && nat->gateway_port == dgw_port) + || (!nat->gateway_port + && ip_in_lrp_networks(dgw_port, old_external_ip)))) { 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, @@ -4800,20 +4860,18 @@ 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, %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 : ""); + ctl_error(ctx, "%s, %s: a NAT with this " + "external_ip and logical_ip already " + "exists", new_external_ip, + new_logical_ip); should_return = true; } } else { ctl_error(ctx, "a NAT with this type (%s), %s (%s) " - "and gateway_port (%s) already exists", + "already exists", nat_type, is_snat ? "logical_ip" : "external_ip", - is_snat ? new_logical_ip : new_external_ip, - dgw_port ? dgw_port->name : ""); + is_snat ? new_logical_ip : new_external_ip); should_return = true; } } @@ -4858,7 +4916,7 @@ nbctl_lr_nat_add(struct ctl_context *ctx) smap_add(&nat_options, "add_route", "true"); } - if (dgw_port) { + if (dgw_port_name) { nbrec_nat_update_gateway_port_addvalue(nat, dgw_port); } nbrec_nat_set_options(nat, &nat_options);
If multiple distributed gateway ports (DGP) are configured on a logical router, "gateway_port" column needs to be set for NAT entries of the logical router for the rules to be applied, as part of commit 2d942be (northd: Add support for NAT with multiple DGP). This patch updates the behavior by inferring the DGP where the NAT rule needs to be applied based on the "external_ip" column of the NAT rule when "gateway_port" column is not set. Signed-off-by: Abhiram Sangana <sangana.abhiram@nutanix.com> --- northd/northd.c | 49 ++++++++++++------ northd/ovn-northd.8.xml | 19 ++++--- ovn-nb.xml | 19 ++++--- tests/ovn-nbctl.at | 47 ++++++++++-------- tests/ovn-northd.at | 102 ++++++++++++++++++++++++-------------- tests/ovn.at | 2 +- utilities/ovn-nbctl.8.xml | 5 +- utilities/ovn-nbctl.c | 94 ++++++++++++++++++++++++++++------- 8 files changed, 228 insertions(+), 109 deletions(-)