From e6a52543078d2d618b72151608b9c61326debe05 Mon Sep 17 00:00:00 2001
From: Numan Siddique <numans@ovn.org>
Date: Tue, 9 Feb 2021 00:44:41 +0530
Subject: [PATCH] northd: Provide the Gateway router option 'lb_force_snat_ip'
to take router port ips.
When a Gateway router is configured with a load balancer
and it is also configured with options:lb_force_snat_ip=<IP>,
OVN after load balancing the destination IP to one of the
backend also does a NAT on the source ip with the
lb_force_snat_ip if the packet is destined to a load balancer
VIP.
There is a problem with the snat of source ip to 'lb_force_snat_ip'
in one particular usecase. When the packet enters the Gateway router
from a provider logical switch destined to the load balancer VIP,
then it is first load balanced to one of the backend and then
the source ip is snatted to 'lb_force_snat_ip'. If the chosen
backend is reachable via the provider logical switch, then the
packet is hairpinned back and it may hit the wire with
the source ip 'lb_force_snat_ip'. If 'lb_force_snat_ip' happens
to be an OVN internal IP then the packet may be dropped.
This patch addresses this issue by providing the option to
set the option - 'lb_force_snat_ip=router_ip'. If 'router_ip'
is set, then OVN will snat the load balanced packet to the
router ip of the logical router port which is chosen as 'outport'
in lr_in_ip_routing stage.
Example.
If the gateway router is
ovn-nbctl show lr0
router 68f20092-5563-44b8-9ccb-b11de3e3a66c (lr0)
port lr0-sw0
mac: "00:00:00:00:ff:01"
networks: ["10.0.0.1/24"]
port lr0-public
mac: "00:00:20:20:12:13"
networks: ["172.168.0.100/24"]
Then the below logical flows are added if 'lb_force_snat_ip'
is configured to 'router_ip'.
table=1 (lr_out_snat), priority=110
match=(flags.force_snat_for_lb == 1 && ip4 && outport == "lr0-public"),
action=(ct_snat(172.168.0.100);)
table=1 (lr_out_snat), priority=110
match=(flags.force_snat_for_lb == 1 && ip4 && outport == "lr0-sw0")
action=(ct_snat(10.0.0.1);)
For the above described scenario, the packet will have source ip as
172.168.0.100 which belongs to the provider logical switch CIDR.
Reported-by: Tim Rozet <trozet@redhat.com>
Acked-by: Mark Gray <mark.d.gray@redhat.com>
Signed-off-by: Numan Siddique <numans@ovn.org>
(cherry-picked from upstream master commit c6e21a23bd8cfcf8dd8b6eb70c8b09e6f4582b2f)
Change-Id: Ie211ff4dd988b50dd48e7b09fd6d46715772a3b0
---
northd/ovn-northd.8.xml | 35 ++++++++++++++++
northd/ovn-northd.c | 91 +++++++++++++++++++++++++++++++++++++----
ovn-nb.xml | 33 ++++++++++-----
tests/ovn-northd.at | 79 +++++++++++++++++++++++++++++++++++
4 files changed, 220 insertions(+), 18 deletions(-)
diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
index 085e7801a..023fe093a 100644
--- a/northd/ovn-northd.8.xml
+++ b/northd/ovn-northd.8.xml
@@ -3653,6 +3653,32 @@ nd_ns {
<code>flags.force_snat_for_dnat == 1 && ip</code> with an
action <code>ct_snat(<var>B</var>);</code>.
</p>
+ </li>
+
+ <li>
+ <p>
+ If the Gateway router in the OVN Northbound database has been
+ configured to force SNAT a packet (that has been previously
+ load-balanced) using router IP (i.e <ref column="options"
+ table="Logical_Router"/>:lb_force_snat_ip=router_ip), then for
+ each logical router port <var>P</var> attached to the Gateway
+ router, a priority-110 flow matches
+ <code>flags.force_snat_for_lb == 1 && outport == <var>P</var>
+ </code> with an action <code>ct_snat(<var>R</var>);</code>
+ where <var>R</var> is the IP configured on the router port.
+ If <code>R</code> is an IPv4 address then the match will also
+ include <code>ip4</code> and if it is an IPv6 address, then the
+ match will also include <code>ip6</code>.
+ </p>
+
+ <p>
+ If the logical router port <var>P</var> is configured with multiple
+ IPv4 and multiple IPv6 addresses, only the first IPv4 and first IPv6
+ address is considered.
+ </p>
+ </li>
+
+ <li>
<p>
If the Gateway router in the OVN Northbound database has been
configured to force SNAT a packet (that has been previously
@@ -3660,6 +3686,9 @@ nd_ns {
<code>flags.force_snat_for_lb == 1 && ip</code> with an
action <code>ct_snat(<var>B</var>);</code>.
</p>
+ </li>
+
+ <li>
<p>
For each configuration in the OVN Northbound database, that asks
to change the source IP address of a packet from an IP address of
@@ -3673,14 +3702,18 @@ nd_ns {
options, then the action would be <code>ip4/6.src=
(<var>B</var>)</code>.
</p>
+ </li>
+ <li>
<p>
If the NAT rule has <code>allowed_ext_ips</code> configured, then
there is an additional match <code>ip4.dst == <var>allowed_ext_ips
</var></code>. Similarly, for IPV6, match would be <code>ip6.dst ==
<var>allowed_ext_ips</var></code>.
</p>
+ </li>
+ <li>
<p>
If the NAT rule has <code>exempted_ext_ips</code> set, then
there is an additional flow configured at the priority + 1 of
@@ -3689,7 +3722,9 @@ nd_ns {
</code>. This flow is used to bypass the ct_snat action for a packet
which is destinted to <code>exempted_ext_ips</code>.
</p>
+ </li>
+ <li>
<p>
A priority-0 logical flow with match <code>1</code> has actions
<code>next;</code>.
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 92e1f5b1f..3d81187d8 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -637,6 +637,7 @@ struct ovn_datapath {
struct lport_addresses dnat_force_snat_addrs;
struct lport_addresses lb_force_snat_addrs;
+ bool lb_force_snat_router_ip;
struct ovn_port **localnet_ports;
size_t n_localnet_ports;
@@ -730,14 +731,28 @@ init_nat_entries(struct ovn_datapath *od)
}
}
- if (get_force_snat_ip(od, "lb", &od->lb_force_snat_addrs)) {
- if (od->lb_force_snat_addrs.n_ipv4_addrs) {
- snat_ip_add(od, od->lb_force_snat_addrs.ipv4_addrs[0].addr_s,
- NULL);
- }
- if (od->lb_force_snat_addrs.n_ipv6_addrs) {
- snat_ip_add(od, od->lb_force_snat_addrs.ipv6_addrs[0].addr_s,
- NULL);
+ /* Check if 'lb_force_snat_ip' is configured with 'router_ip'. */
+ const char *lb_force_snat =
+ smap_get(&od->nbr->options, "lb_force_snat_ip");
+ if (lb_force_snat && !strcmp(lb_force_snat, "router_ip")
+ && smap_get(&od->nbr->options, "chassis")) {
+ /* Set it to true only if its gateway router and
+ * options:lb_force_snat_ip=router_ip. */
+ od->lb_force_snat_router_ip = true;
+ } else {
+ od->lb_force_snat_router_ip = false;
+
+ /* Check if 'lb_force_snat_ip' is configured with a set of
+ * IP address(es). */
+ if (get_force_snat_ip(od, "lb", &od->lb_force_snat_addrs)) {
+ if (od->lb_force_snat_addrs.n_ipv4_addrs) {
+ snat_ip_add(od, od->lb_force_snat_addrs.ipv4_addrs[0].addr_s,
+ NULL);
+ }
+ if (od->lb_force_snat_addrs.n_ipv6_addrs) {
+ snat_ip_add(od, od->lb_force_snat_addrs.ipv6_addrs[0].addr_s,
+ NULL);
+ }
}
}
@@ -9183,6 +9198,64 @@ build_lrouter_force_snat_flows(struct hmap *lflows, struct ovn_datapath *od,
ds_destroy(&actions);
}
+static void
+build_lrouter_force_snat_flows_op(struct ovn_port *op,
+ struct hmap *lflows,
+ struct ds *match, struct ds *actions)
+{
+ if (!op->nbrp || !op->peer || !op->od->lb_force_snat_router_ip) {
+ return;
+ }
+
+ if (op->lrp_networks.n_ipv4_addrs) {
+ ds_clear(match);
+ ds_clear(actions);
+
+ /* Higher priority rules to force SNAT with the router port ip.
+ * This only takes effect when the packet has already been
+ * load balanced once. */
+ ds_put_format(match, "flags.force_snat_for_lb == 1 && ip4 && "
+ "outport == %s", op->json_key);
+ ds_put_format(actions, "ct_snat(%s);",
+ op->lrp_networks.ipv4_addrs[0].addr_s);
+ ovn_lflow_add(lflows, op->od, S_ROUTER_OUT_SNAT, 110,
+ ds_cstr(match), ds_cstr(actions));
+ if (op->lrp_networks.n_ipv4_addrs > 2) {
+ static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
+ VLOG_WARN_RL(&rl, "Logical router port %s is configured with "
+ "multiple IPv4 addresses. Only the first "
+ "IP [%s] is considered as SNAT for load "
+ "balancer", op->json_key,
+ op->lrp_networks.ipv4_addrs[0].addr_s);
+ }
+ }
+
+ /* op->lrp_networks.ipv6_addrs will always have LLA and that will be
+ * last in the list. So add the flows only if n_ipv6_addrs > 1. */
+ if (op->lrp_networks.n_ipv6_addrs > 1) {
+ ds_clear(match);
+ ds_clear(actions);
+
+ /* Higher priority rules to force SNAT with the router port ip.
+ * This only takes effect when the packet has already been
+ * load balanced once. */
+ ds_put_format(match, "flags.force_snat_for_lb == 1 && ip6 && "
+ "outport == %s", op->json_key);
+ ds_put_format(actions, "ct_snat(%s);",
+ op->lrp_networks.ipv6_addrs[0].addr_s);
+ ovn_lflow_add(lflows, op->od, S_ROUTER_OUT_SNAT, 110,
+ ds_cstr(match), ds_cstr(actions));
+ if (op->lrp_networks.n_ipv6_addrs > 2) {
+ static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
+ VLOG_WARN_RL(&rl, "Logical router port %s is configured with "
+ "multiple IPv6 addresses. Only the first "
+ "IP [%s] is considered as SNAT for load "
+ "balancer", op->json_key,
+ op->lrp_networks.ipv6_addrs[0].addr_s);
+ }
+ }
+}
+
static void
build_lrouter_bfd_flows(struct hmap *lflows, struct ovn_port *op)
{
@@ -11706,6 +11779,8 @@ build_lswitch_and_lrouter_iterate_by_op(struct ovn_port *op,
&lsi->match, &lsi->actions);
build_lrouter_ipv4_ip_input(op, lsi->lflows,
&lsi->match, &lsi->actions);
+ build_lrouter_force_snat_flows_op(op, lsi->lflows, &lsi->match,
+ &lsi->actions);
}
static void
diff --git a/ovn-nb.xml b/ovn-nb.xml
index 86aa438bd..09b755f1a 100644
--- a/ovn-nb.xml
+++ b/ovn-nb.xml
@@ -1935,16 +1935,29 @@
</column>
<column name="options" key="lb_force_snat_ip">
<p>
- If set, indicates a set of IP addresses to use to force SNAT a packet
- that has already been load-balanced in the gateway router. When
- multiple gateway routers are configured, a packet can potentially
- enter any of the gateway routers, get DNATted as part of the load-
- balancing and eventually reach the logical switch port.
- For the return traffic to go back to the same gateway router (for
- unDNATing), the packet needs a SNAT in the first place. This can be
- achieved by setting the above option with a gateway specific set of
- IP addresses. This option may have exactly one IPv4 and/or one IPv6
- address on it, separated by a space character.
+ If set, this option can take two possible type of values. Either
+ a set of IP addresses or the string value - <code>router_ip</code>.
+ </p>
+
+ <p>
+ If a set of IP addresses are configured, it indicates to use to
+ force SNAT a packet that has already been load-balanced in the
+ gateway router. When multiple gateway routers are configured, a
+ packet can potentially enter any of the gateway routers, get
+ DNATted as part of the load-balancing and eventually reach the
+ logical switch port. For the return traffic to go back to the
+ same gateway router (for unDNATing), the packet needs a SNAT in the
+ first place. This can be achieved by setting the above option with
+ a gateway specific set of IP addresses. This option may have exactly
+ one IPv4 and/or one IPv6 address on it, separated by a space
+ character.
+ </p>
+
+ <p>
+ If it is configured with the value <code>router_ip</code>, then
+ the load balanced packet is SNATed with the IP of router port
+ (attached to the gateway router) selected as the destination after
+ taking the routing decision.
</p>
</column>
<column name="options" key="mcast_relay" type='{"type": "boolean"}'>
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 7a5c28ba4..dc87ec9d7 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -2557,3 +2557,82 @@ check ovn-sbctl set chassis hv1 other_config:port-up-notif=true
wait_row_count nb:Logical_Switch_Port 1 up=false name=lsp1
AT_CLEANUP
+
+AT_SETUP([ovn -- lb_force_snat_ip for Gateway Routers])
+ovn_start
+
+check ovn-nbctl ls-add sw0
+check ovn-nbctl ls-add sw1
+
+# Create a logical router and attach both logical switches
+check ovn-nbctl lr-add lr0
+check ovn-nbctl lrp-add lr0 lr0-sw0 00:00:00:00:ff:01 10.0.0.1/24
+check ovn-nbctl lsp-add sw0 sw0-lr0
+check ovn-nbctl lsp-set-type sw0-lr0 router
+check ovn-nbctl lsp-set-addresses sw0-lr0 00:00:00:00:ff:01
+check ovn-nbctl lsp-set-options sw0-lr0 router-port=lr0-sw0
+
+check ovn-nbctl lrp-add lr0 lr0-sw1 00:00:00:00:ff:02 20.0.0.1/24
+check ovn-nbctl lsp-add sw1 sw1-lr0
+check ovn-nbctl lsp-set-type sw1-lr0 router
+check ovn-nbctl lsp-set-addresses sw1-lr0 00:00:00:00:ff:02
+check ovn-nbctl lsp-set-options sw1-lr0 router-port=lr0-sw1
+
+check ovn-nbctl ls-add public
+check ovn-nbctl lrp-add lr0 lr0-public 00:00:20:20:12:13 172.168.0.100/24
+check ovn-nbctl lsp-add public public-lr0
+check ovn-nbctl lsp-set-type public-lr0 router
+check ovn-nbctl lsp-set-addresses public-lr0 router
+check ovn-nbctl lsp-set-options public-lr0 router-port=lr0-public
+
+check ovn-nbctl set logical_router lr0 options:chassis=ch1
+
+ovn-sbctl dump-flows lr0 > lr0flows
+AT_CAPTURE_FILE([lr0flows])
+
+AT_CHECK([grep "lr_out_snat" lr0flows | grep force_snat_for_lb | sort], [0], [dnl
+])
+
+check ovn-nbctl --wait=sb set logical_router lr0 options:lb_force_snat_ip="20.0.0.4 aef0::4"
+
+ovn-sbctl dump-flows lr0 > lr0flows
+AT_CAPTURE_FILE([lr0flows])
+
+AT_CHECK([grep "lr_out_snat" lr0flows | grep force_snat_for_lb | sort], [0], [dnl
+ table=1 (lr_out_snat ), priority=100 , match=(flags.force_snat_for_lb == 1 && ip4), action=(ct_snat(20.0.0.4);)
+ table=1 (lr_out_snat ), priority=100 , match=(flags.force_snat_for_lb == 1 && ip6), action=(ct_snat(aef0::4);)
+])
+
+check ovn-nbctl --wait=sb set logical_router lr0 options:lb_force_snat_ip="router_ip"
+
+ovn-sbctl dump-flows lr0 > lr0flows
+AT_CAPTURE_FILE([lr0flows])
+
+AT_CHECK([grep "lr_out_snat" lr0flows | grep force_snat_for_lb | sort], [0], [dnl
+ table=1 (lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && ip4 && outport == "lr0-public"), action=(ct_snat(172.168.0.100);)
+ table=1 (lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && ip4 && outport == "lr0-sw0"), action=(ct_snat(10.0.0.1);)
+ table=1 (lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && ip4 && outport == "lr0-sw1"), action=(ct_snat(20.0.0.1);)
+])
+
+check ovn-nbctl --wait=sb remove logical_router lr0 options chassis
+
+ovn-sbctl dump-flows lr0 > lr0flows
+AT_CAPTURE_FILE([lr0flows])
+
+AT_CHECK([grep "lr_out_snat" lr0flows | grep force_snat_for_lb | sort], [0], [dnl
+])
+
+check ovn-nbctl set logical_router lr0 options:chassis=ch1
+check ovn-nbctl --wait=sb add logical_router_port lr0-sw1 networks "bef0\:\:1/64"
+
+ovn-sbctl dump-flows lr0 > lr0flows
+AT_CAPTURE_FILE([lr0flows])
+
+AT_CHECK([grep "lr_out_snat" lr0flows | grep force_snat_for_lb | sort], [0], [dnl
+ table=1 (lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && ip4 && outport == "lr0-public"), action=(ct_snat(172.168.0.100);)
+ table=1 (lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && ip4 && outport == "lr0-sw0"), action=(ct_snat(10.0.0.1);)
+ table=1 (lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && ip4 && outport == "lr0-sw1"), action=(ct_snat(20.0.0.1);)
+ table=1 (lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && ip6 && outport == "lr0-sw1"), action=(ct_snat(bef0::1);)
+])
+
+AT_CLEANUP
--
2.29.2