From ec4f7228c1b828974ec2909bd108af6a94c3ebc9 Mon Sep 17 00:00:00 2001
From: Numan Siddique <numans@ovn.org>
Date: Wed, 3 Mar 2021 08:41:08 +0530
Subject: [PATCH 1/2] northd: Fix the missing force_snat_for_lb flows when
router_ip is configured.
The commit c6e21a23bd8 which supported the option 'lb_force_snat_ip=router_ip'
on a gateway router, missed out on
- updating the flows in 'lr_in_dnat' to set 'flags.force_snat_for_lb = 1'.
- removing the flow to drop if ip.dst == router port IP in 'lr_in_ip_input'
stage.
This patch fixes these issue and adds a system test to cover the
hairpin load balancer traffic for the gateway routers.
Fixes: c6e21a23bd8("northd: Provide the Gateway router option 'lb_force_snat_ip' to take router port ips.")
Acked-by: Ben Pfaff <blp@ovn.org>
Signed-off-by: Numan Siddique <numans@ovn.org>
(cherry-picked from master commit a8362ff85deea66741661a0df458c3305f46af8b)
---
northd/ovn-northd.8.xml | 10 +++
northd/ovn-northd.c | 36 ++++++++---
tests/ovn-northd.at | 54 ++++++++++++++++
tests/system-ovn.at | 139 ++++++++++++++++++++++++++++++++++++++++
4 files changed, 231 insertions(+), 8 deletions(-)
diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
index 0a2202334..55b1c9655 100644
--- a/northd/ovn-northd.8.xml
+++ b/northd/ovn-northd.8.xml
@@ -2611,6 +2611,16 @@ icmp6 {
with an action <code>ct_snat; </code>.
</p>
+ <p>
+ If the Gateway router is configured with
+ <code>lb_force_snat_ip=router_ip</code> then for every logical router
+ port <var>P</var> attached to the Gateway router with the router ip
+ <var>B</var>, a priority-110 flow is added with the match
+ <code>inport == <var>P</var> && ip4.dst == <var>B</var></code> or
+ <code>inport == <var>P</var> && ip6.dst == <var>B</var></code>
+ with an action <code>ct_snat; </code>.
+ </p>
+
<p>
If the Gateway router has been configured to force SNAT any
previously load-balanced packets to <var>B</var>, a priority-100 flow
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index c1614d8fd..05541506d 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -8800,7 +8800,7 @@ get_force_snat_ip(struct ovn_datapath *od, const char *key_type,
static void
add_router_lb_flow(struct hmap *lflows, struct ovn_datapath *od,
struct ds *match, struct ds *actions, int priority,
- bool lb_force_snat_ip, struct ovn_lb_vip *lb_vip,
+ bool force_snat_for_lb, struct ovn_lb_vip *lb_vip,
const char *proto, struct nbrec_load_balancer *lb,
struct shash *meter_groups, struct sset *nat_entries)
{
@@ -8809,7 +8809,7 @@ add_router_lb_flow(struct hmap *lflows, struct ovn_datapath *od,
/* A match and actions for new connections. */
char *new_match = xasprintf("ct.new && %s", ds_cstr(match));
- if (lb_force_snat_ip) {
+ if (force_snat_for_lb) {
char *new_actions = xasprintf("flags.force_snat_for_lb = 1; %s",
ds_cstr(actions));
ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_DNAT, priority,
@@ -8822,7 +8822,7 @@ add_router_lb_flow(struct hmap *lflows, struct ovn_datapath *od,
/* A match and actions for established connections. */
char *est_match = xasprintf("ct.est && %s", ds_cstr(match));
- if (lb_force_snat_ip) {
+ if (force_snat_for_lb) {
ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_DNAT, priority,
est_match,
"flags.force_snat_for_lb = 1; ct_dnat;",
@@ -8899,7 +8899,7 @@ add_router_lb_flow(struct hmap *lflows, struct ovn_datapath *od,
ds_put_format(&undnat_match, ") && outport == %s && "
"is_chassis_resident(%s)", od->l3dgw_port->json_key,
od->l3redirect_port->json_key);
- if (lb_force_snat_ip) {
+ if (force_snat_for_lb) {
ovn_lflow_add_with_hint(lflows, od, S_ROUTER_OUT_UNDNAT, 120,
ds_cstr(&undnat_match),
"flags.force_snat_for_lb = 1; ct_dnat;",
@@ -9379,6 +9379,13 @@ build_lrouter_force_snat_flows_op(struct ovn_port *op,
ds_clear(match);
ds_clear(actions);
+ ds_put_format(match, "inport == %s && ip4.dst == %s",
+ op->json_key, op->lrp_networks.ipv4_addrs[0].addr_s);
+ ovn_lflow_add(lflows, op->od, S_ROUTER_IN_UNSNAT, 110,
+ ds_cstr(match), "ct_snat;");
+
+ ds_clear(match);
+
/* Higher priority rules to force SNAT with the router port ip.
* This only takes effect when the packet has already been
* load balanced once. */
@@ -9404,6 +9411,13 @@ build_lrouter_force_snat_flows_op(struct ovn_port *op,
ds_clear(match);
ds_clear(actions);
+ ds_put_format(match, "inport == %s && ip6.dst == %s",
+ op->json_key, op->lrp_networks.ipv6_addrs[0].addr_s);
+ ovn_lflow_add(lflows, op->od, S_ROUTER_IN_UNSNAT, 110,
+ ds_cstr(match), "ct_snat;");
+
+ ds_clear(match);
+
/* Higher priority rules to force SNAT with the router port ip.
* This only takes effect when the packet has already been
* load balanced once. */
@@ -11156,11 +11170,15 @@ build_lrouter_ipv4_ip_input(struct ovn_port *op,
* also a SNAT IP. Those are dropped later, in stage
* "lr_in_arp_resolve", if unSNAT was unsuccessful.
*
+ * If op->pd->lb_force_snat_router_ip is true, it means the IP of the
+ * router port is also SNAT IP.
+ *
* Priority 60.
*/
- build_lrouter_drop_own_dest(op, S_ROUTER_IN_IP_INPUT, 60, false,
- lflows);
-
+ if (!op->od->lb_force_snat_router_ip) {
+ build_lrouter_drop_own_dest(op, S_ROUTER_IN_IP_INPUT, 60, false,
+ lflows);
+ }
/* ARP / ND handling for external IP addresses.
*
* DNAT and SNAT IP addresses are external IP addresses that need ARP
@@ -11860,8 +11878,10 @@ build_lrouter_nat_defrag_and_lb(struct ovn_datapath *od,
ds_put_format(match, " && is_chassis_resident(%s)",
od->l3redirect_port->json_key);
}
+ bool force_snat_for_lb =
+ lb_force_snat_ip || od->lb_force_snat_router_ip;
add_router_lb_flow(lflows, od, match, actions, prio,
- lb_force_snat_ip, lb_vip, proto,
+ force_snat_for_lb, lb_vip, proto,
nb_lb, meter_groups, &nat_entries);
}
}
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 3592c046d..11ca6e456 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -2577,11 +2577,21 @@ 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 lb-add lb1 10.0.0.10:80 10.0.0.4:8080
+check ovn-nbctl lr-lb-add lr0 lb1
check ovn-nbctl set logical_router lr0 options:chassis=ch1
ovn-sbctl dump-flows lr0 > lr0flows
AT_CAPTURE_FILE([lr0flows])
+AT_CHECK([grep "lr_in_unsnat" lr0flows | sort], [0], [dnl
+ table=5 (lr_in_unsnat ), priority=0 , match=(1), action=(next;)
+])
+
+AT_CHECK([grep "lr_in_dnat" lr0flows | grep force_snat_for_lb | sort], [0], [dnl
+])
+
+
AT_CHECK([grep "lr_out_snat" lr0flows | grep force_snat_for_lb | sort], [0], [dnl
])
@@ -2590,6 +2600,18 @@ check ovn-nbctl --wait=sb set logical_router lr0 options:lb_force_snat_ip="20.0.
ovn-sbctl dump-flows lr0 > lr0flows
AT_CAPTURE_FILE([lr0flows])
+
+AT_CHECK([grep "lr_in_unsnat" lr0flows | sort], [0], [dnl
+ table=5 (lr_in_unsnat ), priority=0 , match=(1), action=(next;)
+ table=5 (lr_in_unsnat ), priority=110 , match=(ip4 && ip4.dst == 20.0.0.4), action=(ct_snat;)
+ table=5 (lr_in_unsnat ), priority=110 , match=(ip6 && ip6.dst == aef0::4), action=(ct_snat;)
+])
+
+AT_CHECK([grep "lr_in_dnat" lr0flows | grep force_snat_for_lb | sort], [0], [dnl
+ table=6 (lr_in_dnat ), priority=120 , match=(ct.est && ip && ip4.dst == 10.0.0.10 && tcp && tcp.dst == 80), action=(flags.force_snat_for_lb = 1; ct_dnat;)
+ table=6 (lr_in_dnat ), priority=120 , match=(ct.new && ip && ip4.dst == 10.0.0.10 && tcp && tcp.dst == 80), action=(flags.force_snat_for_lb = 1; ct_lb(backends=10.0.0.4:8080);)
+])
+
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);)
@@ -2600,6 +2622,21 @@ check ovn-nbctl --wait=sb set logical_router lr0 options:lb_force_snat_ip="route
ovn-sbctl dump-flows lr0 > lr0flows
AT_CAPTURE_FILE([lr0flows])
+AT_CHECK([grep "lr_in_ip_input" lr0flows | grep "priority=60" | sort], [0], [dnl
+])
+
+AT_CHECK([grep "lr_in_unsnat" lr0flows | sort], [0], [dnl
+ table=5 (lr_in_unsnat ), priority=0 , match=(1), action=(next;)
+ table=5 (lr_in_unsnat ), priority=110 , match=(inport == "lr0-public" && ip4.dst == 172.168.0.100), action=(ct_snat;)
+ table=5 (lr_in_unsnat ), priority=110 , match=(inport == "lr0-sw0" && ip4.dst == 10.0.0.1), action=(ct_snat;)
+ table=5 (lr_in_unsnat ), priority=110 , match=(inport == "lr0-sw1" && ip4.dst == 20.0.0.1), action=(ct_snat;)
+])
+
+AT_CHECK([grep "lr_in_dnat" lr0flows | grep force_snat_for_lb | sort], [0], [dnl
+ table=6 (lr_in_dnat ), priority=120 , match=(ct.est && ip && ip4.dst == 10.0.0.10 && tcp && tcp.dst == 80), action=(flags.force_snat_for_lb = 1; ct_dnat;)
+ table=6 (lr_in_dnat ), priority=120 , match=(ct.new && ip && ip4.dst == 10.0.0.10 && tcp && tcp.dst == 80), action=(flags.force_snat_for_lb = 1; ct_lb(backends=10.0.0.4:8080);)
+])
+
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);)
@@ -2611,6 +2648,10 @@ 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_in_unsnat" lr0flows | sort], [0], [dnl
+ table=5 (lr_in_unsnat ), priority=0 , match=(1), action=(next;)
+])
+
AT_CHECK([grep "lr_out_snat" lr0flows | grep force_snat_for_lb | sort], [0], [dnl
])
@@ -2620,6 +2661,19 @@ 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_in_unsnat" lr0flows | sort], [0], [dnl
+ table=5 (lr_in_unsnat ), priority=0 , match=(1), action=(next;)
+ table=5 (lr_in_unsnat ), priority=110 , match=(inport == "lr0-public" && ip4.dst == 172.168.0.100), action=(ct_snat;)
+ table=5 (lr_in_unsnat ), priority=110 , match=(inport == "lr0-sw0" && ip4.dst == 10.0.0.1), action=(ct_snat;)
+ table=5 (lr_in_unsnat ), priority=110 , match=(inport == "lr0-sw1" && ip4.dst == 20.0.0.1), action=(ct_snat;)
+ table=5 (lr_in_unsnat ), priority=110 , match=(inport == "lr0-sw1" && ip6.dst == bef0::1), action=(ct_snat;)
+])
+
+AT_CHECK([grep "lr_in_dnat" lr0flows | grep force_snat_for_lb | sort], [0], [dnl
+ table=6 (lr_in_dnat ), priority=120 , match=(ct.est && ip && ip4.dst == 10.0.0.10 && tcp && tcp.dst == 80), action=(flags.force_snat_for_lb = 1; ct_dnat;)
+ table=6 (lr_in_dnat ), priority=120 , match=(ct.new && ip && ip4.dst == 10.0.0.10 && tcp && tcp.dst == 80), action=(flags.force_snat_for_lb = 1; ct_lb(backends=10.0.0.4:8080);)
+])
+
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);)
diff --git a/tests/system-ovn.at b/tests/system-ovn.at
index f29234432..9819573bb 100644
--- a/tests/system-ovn.at
+++ b/tests/system-ovn.at
@@ -2224,6 +2224,144 @@ tcp,orig=(src=172.16.1.2,dst=192.168.2.2,sport=<cleared>,dport=<cleared>),reply=
OVS_WAIT_UNTIL([check_est_flows], [check established flows])
+ovn-nbctl set logical_router R2 options:lb_force_snat_ip=router_ip
+
+# Destroy the load balancer and create again. ovn-controller will
+# clear the OF flows and re add again and clears the n_packets
+# for these flows.
+ovn-nbctl destroy load_balancer $uuid
+uuid=`ovn-nbctl create load_balancer vips:30.0.0.1="192.168.1.2,192.168.2.2"`
+ovn-nbctl set logical_router R2 load_balancer=$uuid
+
+# Config OVN load-balancer with another VIP (this time with ports).
+ovn-nbctl set load_balancer $uuid vips:'"30.0.0.2:8000"'='"192.168.1.2:80,192.168.2.2:80"'
+
+ovn-nbctl list load_balancer
+ovn-sbctl dump-flows R2
+OVS_WAIT_UNTIL([ovs-ofctl -O OpenFlow13 dump-flows br-int table=41 | \
+grep 'nat(src=20.0.0.2)'])
+
+rm -f wget*.log
+
+dnl Test load-balancing that includes L4 ports in NAT.
+for i in `seq 1 20`; do
+ echo Request $i
+ NS_CHECK_EXEC([alice1], [wget 30.0.0.2:8000 -t 5 -T 1 --retry-connrefused -v -o wget$i.log])
+done
+
+dnl Each server should have at least one connection.
+AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(30.0.0.2) |
+sed -e 's/zone=[[0-9]]*/zone=<cleared>/'], [0], [dnl
+tcp,orig=(src=172.16.1.2,dst=30.0.0.2,sport=<cleared>,dport=<cleared>),reply=(src=192.168.1.2,dst=172.16.1.2,sport=<cleared>,dport=<cleared>),zone=<cleared>,labels=0x2,protoinfo=(state=<cleared>)
+tcp,orig=(src=172.16.1.2,dst=30.0.0.2,sport=<cleared>,dport=<cleared>),reply=(src=192.168.2.2,dst=172.16.1.2,sport=<cleared>,dport=<cleared>),zone=<cleared>,labels=0x2,protoinfo=(state=<cleared>)
+])
+
+AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(20.0.0.2) |
+sed -e 's/zone=[[0-9]]*/zone=<cleared>/'], [0], [dnl
+tcp,orig=(src=172.16.1.2,dst=192.168.1.2,sport=<cleared>,dport=<cleared>),reply=(src=192.168.1.2,dst=20.0.0.2,sport=<cleared>,dport=<cleared>),zone=<cleared>,protoinfo=(state=<cleared>)
+tcp,orig=(src=172.16.1.2,dst=192.168.2.2,sport=<cleared>,dport=<cleared>),reply=(src=192.168.2.2,dst=20.0.0.2,sport=<cleared>,dport=<cleared>),zone=<cleared>,protoinfo=(state=<cleared>)
+])
+
+OVS_WAIT_UNTIL([check_est_flows], [check established flows])
+
+OVS_APP_EXIT_AND_WAIT([ovn-controller])
+
+as ovn-sb
+OVS_APP_EXIT_AND_WAIT([ovsdb-server])
+
+as ovn-nb
+OVS_APP_EXIT_AND_WAIT([ovsdb-server])
+
+as northd
+OVS_APP_EXIT_AND_WAIT([ovn-northd])
+
+as
+OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d
+/connection dropped.*/d"])
+AT_CLEANUP
+
+AT_SETUP([ovn -- load balancing in gateway router hairpin scenario])
+AT_KEYWORDS([ovnlb])
+
+CHECK_CONNTRACK()
+CHECK_CONNTRACK_NAT()
+ovn_start
+OVS_TRAFFIC_VSWITCHD_START()
+ADD_BR([br-int])
+check ovs-vsctl add-br br-ext
+
+
+# Set external-ids in br-int needed for ovn-controller
+ovs-vsctl \
+ -- set Open_vSwitch . external-ids:system-id=hv1 \
+ -- set Open_vSwitch . external-ids:ovn-remote=unix:$ovs_base/ovn-sb/ovn-sb.sock \
+ -- set Open_vSwitch . external-ids:ovn-encap-type=geneve \
+ -- set Open_vSwitch . external-ids:ovn-encap-ip=169.0.0.1 \
+ -- set bridge br-int fail-mode=secure other-config:disable-in-band=true
+
+# Start ovn-controller
+start_daemon ovn-controller
+
+check ovn-nbctl lr-add R1
+
+check ovn-nbctl ls-add sw0
+check ovn-nbctl ls-add public
+
+check ovn-nbctl lrp-add R1 rp-sw0 00:00:01:01:02:03 192.168.1.1/24
+check ovn-nbctl lrp-add R1 rp-public 00:00:02:01:02:03 172.16.1.1/24
+
+check ovn-nbctl set logical_router R1 options:chassis=hv1
+
+check ovn-nbctl lsp-add sw0 sw0-rp -- set Logical_Switch_Port sw0-rp \
+ type=router options:router-port=rp-sw0 \
+ -- lsp-set-addresses sw0-rp router
+
+check ovn-nbctl lsp-add public public-rp -- set Logical_Switch_Port public-rp \
+ type=router options:router-port=rp-public \
+ -- lsp-set-addresses public-rp router
+
+check ovs-vsctl set Open_vSwitch . external-ids:ovn-bridge-mappings=phynet:br-ext
+
+check ovn-nbctl lsp-add public public1 \
+ -- lsp-set-addresses public1 unknown \
+ -- lsp-set-type public1 localnet \
+ -- lsp-set-options public1 network_name=phynet
+
+ADD_NAMESPACES(server)
+ADD_VETH(s1, server, br-ext, "172.16.1.100/24", "1a:00:00:00:00:01", \
+ "172.16.1.1")
+
+OVS_WAIT_UNTIL([test "$(ip netns exec server ip a | grep fe80 | grep tentative)" = ""])
+
+ADD_NAMESPACES(client)
+ADD_VETH(c1, client, br-ext, "172.16.1.110/24", "1a:00:00:00:00:02", \
+ "172.16.1.1")
+
+OVS_WAIT_UNTIL([test "$(ip netns exec client ip a | grep fe80 | grep tentative)" = ""])
+
+# Start webservers in 'server'.
+OVS_START_L7([server], [http])
+
+# Create a load balancer and associate to R1
+check ovn-nbctl lb-add lb1 172.16.1.150:80 172.16.1.100:80
+check ovn-nbctl lr-lb-add R1 lb1
+
+check ovn-nbctl --wait=hv sync
+
+for i in $(seq 1 5); do
+ echo Request $i
+ NS_CHECK_EXEC([client], [wget 172.16.1.100 -t 5 -T 1 --retry-connrefused -v -o wget$i.log])
+done
+
+# Now send the traffic from client to the VIP - 172.16.1.150
+check ovn-nbctl set logical_router R1 options:lb_force_snat_ip=router_ip
+check ovn-nbctl --wait=hv sync
+
+for i in $(seq 1 5); do
+ echo Request $i
+ NS_CHECK_EXEC([client], [wget 172.16.1.150 -t 5 -T 1 --retry-connrefused -v -o wget$i.log])
+done
+
OVS_APP_EXIT_AND_WAIT([ovn-controller])
as ovn-sb
@@ -2237,6 +2375,7 @@ OVS_APP_EXIT_AND_WAIT([ovn-northd])
as
OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d
+/Failed to acquire.*/d
/connection dropped.*/d"])
AT_CLEANUP
--
2.29.2