Partially revert the following commit since it introduces a regression
when we want to directly connect to a backend ip from a client outside
the cluster for gw-router-port scenario.
commit e3bc68c3be6967916674119b14fe2bef081ac6ad
Author: Lorenzo Bianconi <lorenzo.bianc...@redhat.com>
Date: Mon Mar 20 19:30:13 2023 +0100
northd: drop ct.inv packets in post snat and lb_aff_learn stages
Drop ip packets with ct status set to invalid in post snat and
lb_aff_learn router stages.
Skip ICMPv{4,6} error messages packet in ct.inv rules in order to
avoid to introduce too complicated code.
Even if the issue is not directly introduced by the commit above, it is
not easy to fix it without committing all IP traffic to connection
tracking or adding a flow per load-balancer backend.
Signed-off-by: Lorenzo Bianconi <lorenzo.bianc...@redhat.com>
---
northd/northd.c | 25 -------------------------
northd/ovn-northd.8.xml | 22 ----------------------
tests/ovn-northd.at | 4 ----
tests/system-ovn.at | 16 ++++++++--------
4 files changed, 8 insertions(+), 59 deletions(-)
diff --git a/northd/northd.c b/northd/northd.c
index 7a10e4dcf..25b27607a 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -13901,30 +13901,6 @@ build_lrouter_out_is_dnat_local(struct hmap *lflows,
struct ovn_datapath *od,
&nat->header_);
}
-static void
-build_lrouter_drop_ct_inv_flow(struct ovn_datapath *od, struct hmap *lflows)
-{
- ovs_assert(od->nbr);
- if (!use_ct_inv_match) {
- return;
- }
- /* Advance ICMP Destination Unreachable - Fragmentation Needed
- * packets and drop other packets which are ct tracked and invalid. */
- ovn_lflow_add(lflows, od, S_ROUTER_IN_LB_AFF_LEARN, 250,
- "((ip4 && icmp4.type == 3 && icmp4.code == 4) ||"
- " (ip6 && icmp6.type == 2 && icmp6.code == 0))",
- "next;");
- ovn_lflow_add(lflows, od, S_ROUTER_IN_LB_AFF_LEARN, 200,
- "ip && ct.trk && ct.inv", debug_drop_action());
-
- ovn_lflow_add(lflows, od, S_ROUTER_OUT_POST_SNAT, 150,
- "((ip4 && icmp4.type == 3 && icmp4.code == 4) ||"
- " (ip6 && icmp6.type == 2 && icmp6.code == 0))",
- "next;");
- ovn_lflow_add(lflows, od, S_ROUTER_OUT_POST_SNAT, 100,
- "ip && ct.trk && ct.inv", debug_drop_action());
-}
-
static void
build_lrouter_out_snat_flow(struct hmap *lflows, struct ovn_datapath *od,
const struct nbrec_nat *nat, struct ds *match,
@@ -14725,7 +14701,6 @@ build_lswitch_and_lrouter_iterate_by_lr(struct
ovn_datapath *od,
lsi->lr_ports, &lsi->match,
&lsi->actions, lsi->meter_groups,
lsi->features);
- build_lrouter_drop_ct_inv_flow(od, lsi->lflows);
build_lrouter_lb_affinity_default_flows(od, lsi->lflows);
}
diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
index 4100cae0d..98aa060b9 100644
--- a/northd/ovn-northd.8.xml
+++ b/northd/ovn-northd.8.xml
@@ -3602,17 +3602,6 @@ icmp6 {
</p>
<ul>
- <li>
- For ICMPv4/ICMPv6 packet too big traffic, a priority-250 flow with
- action <code>next;</code>.
- </li>
-
- <li>
- If <code>use_ct_inv_match</code> is set, a priority-200 flow
- matches <code>ip && ct.trk && ct.inv</code> with
- action <code>drop;</code>.
- </li>
-
<li>
For all the configured load balancing rules for a logical router where
a positive affinity timeout <var>T</var> is specified in <code>options
@@ -4959,17 +4948,6 @@ nd_ns {
<p>
Packets reaching this table are processed according to the flows below:
<ul>
- <li>
- For ICMPv4/ICMPv6 packet too big traffic, a priority-150 flow with
- action <code>next;</code>.
- </li>
-
- <li>
- If <code>use_ct_inv_match</code> is set, a priority-100 flow
- matches <code>ip && ct.trk && ct.inv</code> with
- action <code>drop;</code>.
- </li>
-
<li>
A priority-0 logical flow that matches all packets not already
handled (match <code>1</code>) and action <code>next;</code>.
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index a3db189ac..9fee00094 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -8329,8 +8329,6 @@ AT_CHECK([grep "lr_in_lb_aff_check" R1flows | sort], [0],
[dnl
])
AT_CHECK([grep "lr_in_lb_aff_learn" R1flows | sort], [0], [dnl
table=8 (lr_in_lb_aff_learn ), priority=0 , match=(1), action=(next;)
- table=8 (lr_in_lb_aff_learn ), priority=200 , match=(ip && ct.trk &&
ct.inv), action=(drop;)
- table=8 (lr_in_lb_aff_learn ), priority=250 , match=(((ip4 && icmp4.type ==
3 && icmp4.code == 4) || (ip6 && icmp6.type == 2 && icmp6.code == 0))),
action=(next;)
])
ovn-nbctl --wait=sb set load_balancer lb0 options:affinity_timeout=60
@@ -8379,8 +8377,6 @@ AT_CHECK([grep "lr_in_lb_aff_learn" R1flows | sort], [0],
[dnl
table=8 (lr_in_lb_aff_learn ), priority=0 , match=(1), action=(next;)
table=8 (lr_in_lb_aff_learn ), priority=100 , match=(reg9[[6]] == 0 &&
ct.new && ip4 && reg0 == 172.16.0.10 && reg9[[16..31]] == 80 && ip4.dst ==
10.0.0.2 && tcp.dst == 80), action=(commit_lb_aff(vip = "172.16.0.10:80",
backend = "10.0.0.2:80", proto = tcp, timeout = 60); /* drop */)
table=8 (lr_in_lb_aff_learn ), priority=100 , match=(reg9[[6]] == 0 &&
ct.new && ip4 && reg0 == 172.16.0.10 && reg9[[16..31]] == 80 && ip4.dst ==
20.0.0.2 && tcp.dst == 80), action=(commit_lb_aff(vip = "172.16.0.10:80",
backend = "20.0.0.2:80", proto = tcp, timeout = 60); /* drop */)
- table=8 (lr_in_lb_aff_learn ), priority=200 , match=(ip && ct.trk &&
ct.inv), action=(drop;)
- table=8 (lr_in_lb_aff_learn ), priority=250 , match=(((ip4 && icmp4.type ==
3 && icmp4.code == 4) || (ip6 && icmp6.type == 2 && icmp6.code == 0))),
action=(next;)
])
AS_BOX([Test LR flows - skip_snat=true])
diff --git a/tests/system-ovn.at b/tests/system-ovn.at
index 939c2c3dc..96d1c295e 100644
--- a/tests/system-ovn.at
+++ b/tests/system-ovn.at
@@ -6131,10 +6131,10 @@
tcp,orig=(src=172.16.0.1,dst=10.0.0.2,sport=<cleared>,dport=<cleared>),reply=(sr
# Ensure datapaths show conntrack states as expected
# Like with conntrack entries, we shouldn't try to predict
# port binding tunnel keys. So omit them from expected labels.
-AT_CHECK([ovs-appctl dpctl/dump-flows | grep
'ct_state(+new-est-rpl-inv+trk).*ct(.*label=0x401020400000000/.*)' -c], [0],
[dnl
+AT_CHECK([ovs-appctl dpctl/dump-flows | grep
'ct_state(+new-est-rpl+trk).*ct(.*label=0x401020400000000/.*)' -c], [0], [dnl
1
])
-AT_CHECK([ovs-appctl dpctl/dump-flows | grep
'ct_state(-new+est+rpl-inv+trk).*ct_label(0x401020400000000)' -c], [0], [dnl
+AT_CHECK([ovs-appctl dpctl/dump-flows | grep
'ct_state(-new+est+rpl+trk).*ct_label(0x401020400000000)' -c], [0], [dnl
1
])
@@ -6149,10 +6149,10 @@ ovn-nbctl set Logical_Switch_Port r2-ext \
ovn-nbctl --wait=hv sync
NS_CHECK_EXEC([bob1], [nc -z 10.0.0.2 80], [0])
-AT_CHECK([ovs-appctl dpctl/dump-flows | grep
'ct_state(+new-est-rpl-inv+trk).*ct(.*label=0x1001020400000000/.*)' -c], [0],
[dnl
+AT_CHECK([ovs-appctl dpctl/dump-flows | grep
'ct_state(+new-est-rpl+trk).*ct(.*label=0x1001020400000000/.*)' -c], [0], [dnl
1
])
-AT_CHECK([ovs-appctl dpctl/dump-flows | grep
'ct_state(-new+est+rpl-inv+trk).*ct_label(0x1001020400000000)' -c], [0], [dnl
+AT_CHECK([ovs-appctl dpctl/dump-flows | grep
'ct_state(-new+est+rpl+trk).*ct_label(0x1001020400000000)' -c], [0], [dnl
1
])
@@ -6316,11 +6316,11 @@ NS_CHECK_EXEC([bob1], [nc -6 -z fd01::2 80], [0])
# Ensure datapaths show conntrack states as expected
# Like with conntrack entries, we shouldn't try to predict
# port binding tunnel keys. So omit them from expected labels.
-AT_CHECK([ovs-appctl dpctl/dump-flows | grep
'ct_state(+new-est-rpl-inv+trk).*ct(.*label=0x401020400000000/.*)' -c], [0],
[dnl
+AT_CHECK([ovs-appctl dpctl/dump-flows | grep
'ct_state(+new-est-rpl+trk).*ct(.*label=0x401020400000000/.*)' -c], [0], [dnl
1
])
-AT_CHECK([ovs-appctl dpctl/dump-flows | grep
'ct_state(-new+est+rpl-inv+trk).*ct_label(0x401020400000000)' -c], [0], [dnl
+AT_CHECK([ovs-appctl dpctl/dump-flows | grep
'ct_state(-new+est+rpl+trk).*ct_label(0x401020400000000)' -c], [0], [dnl
1
])
@@ -6343,10 +6343,10 @@ ovn-nbctl set Logical_Switch_Port r2-ext \
NS_CHECK_EXEC([bob1], [nc -6 -z fd01::2 80], [0])
-AT_CHECK([ovs-appctl dpctl/dump-flows | grep
'ct_state(+new-est-rpl-inv+trk).*ct(.*label=0x1001020400000000/.*)' -c], [0],
[dnl
+AT_CHECK([ovs-appctl dpctl/dump-flows | grep
'ct_state(+new-est-rpl+trk).*ct(.*label=0x1001020400000000/.*)' -c], [0], [dnl
1
])
-AT_CHECK([ovs-appctl dpctl/dump-flows | grep
'ct_state(-new+est+rpl-inv+trk).*ct_label(0x1001020400000000)' -c], [0], [dnl
+AT_CHECK([ovs-appctl dpctl/dump-flows | grep
'ct_state(-new+est+rpl+trk).*ct_label(0x1001020400000000)' -c], [0], [dnl
1
])
--
2.39.2
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
