[ovs-dev] [PATCH ovn] northd: Restore flows that recirculate packets in the router DNAT zone.
Dumitru Ceara
dceara at redhat.com
Thu Apr 1 09:25:39 UTC 2021
Also improve the tests to make sure this doesn't break again in the
future.
Fixes: 225426081f85 ("northd: introduce build_lrouter_in_dnat_flow routine")
CC: Lorenzo Bianconi <lorenzo.bianconi at redhat.com>
Reported-by: Ilya Maximets <i.maximets at ovn.org>
Signed-off-by: Dumitru Ceara <dceara at redhat.com>
---
Note: the regression was only in the ovn-northd C implementation, there
are no ovn-northd-ddlog changes required.
---
northd/ovn-northd.c | 26 ++++++++++++--------------
tests/ovn-northd.at | 40 +++++++++++++++++++++++++++-------------
2 files changed, 39 insertions(+), 27 deletions(-)
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 57df62b92..9839b8c4f 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -11240,20 +11240,6 @@ build_lrouter_in_dnat_flow(struct hmap *lflows, struct ovn_datapath *od,
&nat->header_);
}
}
-
- if (!od->l3dgw_port) {
- /* For gateway router, re-circulate every packet through
- * the DNAT zone. This helps with the following.
- *
- * Any packet that needs to be unDNATed in the reverse
- * direction gets unDNATed. Ideally this could be done in
- * the egress pipeline. But since the gateway router
- * does not have any feature that depends on the source
- * ip address being external IP address for IP routing,
- * we can do it here, saving a future re-circulation. */
- ovn_lflow_add(lflows, od, S_ROUTER_IN_DNAT, 50,
- "ip", "flags.loopback = 1; ct_dnat;");
- }
}
static void
@@ -11716,6 +11702,18 @@ build_lrouter_nat_defrag_and_lb(struct ovn_datapath *od,
od->lb_force_snat_addrs.ipv6_addrs[0].addr_s, "lb");
}
}
+
+ /* For gateway router, re-circulate every packet through
+ * the DNAT zone. This helps with the following.
+ *
+ * Any packet that needs to be unDNATed in the reverse
+ * direction gets unDNATed. Ideally this could be done in
+ * the egress pipeline. But since the gateway router
+ * does not have any feature that depends on the source
+ * ip address being external IP address for IP routing,
+ * we can do it here, saving a future re-circulation. */
+ ovn_lflow_add(lflows, od, S_ROUTER_IN_DNAT, 50,
+ "ip", "flags.loopback = 1; ct_dnat;");
}
/* Load balancing and packet defrag are only valid on
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 47f2662e4..96476497d 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -2700,7 +2700,7 @@ wait_row_count nb:Logical_Switch_Port 1 up=false name=lsp1
AT_CLEANUP
OVN_FOR_EACH_NORTHD([
-AT_SETUP([ovn -- lb_force_snat_ip for Gateway Routers])
+AT_SETUP([ovn -- Load Balancers and lb_force_snat_ip for Gateway Routers])
ovn_start
check ovn-nbctl ls-add sw0
@@ -2740,11 +2740,11 @@ 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
+AT_CHECK([grep "lr_in_dnat" lr0flows | sort], [0], [dnl
+ table=6 (lr_in_dnat ), priority=0 , match=(1), action=(next;)
+ table=6 (lr_in_dnat ), priority=120 , match=(ct.est && ip && ip4.dst == 10.0.0.10 && tcp && tcp.dst == 80), action=(ct_dnat;)
+ table=6 (lr_in_dnat ), priority=120 , match=(ct.new && ip && ip4.dst == 10.0.0.10 && tcp && tcp.dst == 80), action=(ct_lb(backends=10.0.0.4:8080);)
+ table=6 (lr_in_dnat ), priority=50 , match=(ip), action=(flags.loopback = 1; ct_dnat;)
])
check ovn-nbctl --wait=sb set logical_router lr0 options:lb_force_snat_ip="20.0.0.4 aef0::4"
@@ -2759,14 +2759,18 @@ AT_CHECK([grep "lr_in_unsnat" lr0flows | sort], [0], [dnl
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
+AT_CHECK([grep "lr_in_dnat" lr0flows | sort], [0], [dnl
+ table=6 (lr_in_dnat ), priority=0 , match=(1), action=(next;)
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);)
+ table=6 (lr_in_dnat ), priority=50 , match=(ip), action=(flags.loopback = 1; ct_dnat;)
])
-AT_CHECK([grep "lr_out_snat" lr0flows | grep force_snat_for_lb | sort], [0], [dnl
+AT_CHECK([grep "lr_out_snat" lr0flows | sort], [0], [dnl
+ table=1 (lr_out_snat ), priority=0 , match=(1), action=(next;)
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);)
+ table=1 (lr_out_snat ), priority=120 , match=(nd_ns), action=(next;)
])
check ovn-nbctl --wait=sb set logical_router lr0 options:lb_force_snat_ip="router_ip"
@@ -2784,15 +2788,19 @@ AT_CHECK([grep "lr_in_unsnat" lr0flows | sort], [0], [dnl
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
+AT_CHECK([grep "lr_in_dnat" lr0flows | sort], [0], [dnl
+ table=6 (lr_in_dnat ), priority=0 , match=(1), action=(next;)
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);)
+ table=6 (lr_in_dnat ), priority=50 , match=(ip), action=(flags.loopback = 1; ct_dnat;)
])
-AT_CHECK([grep "lr_out_snat" lr0flows | grep force_snat_for_lb | sort], [0], [dnl
+AT_CHECK([grep "lr_out_snat" lr0flows | sort], [0], [dnl
+ table=1 (lr_out_snat ), priority=0 , match=(1), action=(next;)
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=120 , match=(nd_ns), action=(next;)
])
check ovn-nbctl --wait=sb remove logical_router lr0 options chassis
@@ -2804,7 +2812,9 @@ 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
+AT_CHECK([grep "lr_out_snat" lr0flows | sort], [0], [dnl
+ table=1 (lr_out_snat ), priority=0 , match=(1), action=(next;)
+ table=1 (lr_out_snat ), priority=120 , match=(nd_ns), action=(next;)
])
check ovn-nbctl set logical_router lr0 options:chassis=ch1
@@ -2821,16 +2831,20 @@ AT_CHECK([grep "lr_in_unsnat" lr0flows | sort], [0], [dnl
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
+AT_CHECK([grep "lr_in_dnat" lr0flows | sort], [0], [dnl
+ table=6 (lr_in_dnat ), priority=0 , match=(1), action=(next;)
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);)
+ table=6 (lr_in_dnat ), priority=50 , match=(ip), action=(flags.loopback = 1; ct_dnat;)
])
-AT_CHECK([grep "lr_out_snat" lr0flows | grep force_snat_for_lb | sort], [0], [dnl
+AT_CHECK([grep "lr_out_snat" lr0flows | sort], [0], [dnl
+ table=1 (lr_out_snat ), priority=0 , match=(1), action=(next;)
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);)
+ table=1 (lr_out_snat ), priority=120 , match=(nd_ns), action=(next;)
])
AT_CLEANUP
--
2.27.0
More information about the dev
mailing list