@@ -7437,14 +7437,12 @@ build_lb_affinity_lr_flows(struct lflow_table *lflows,
* table=ls_in_lb, priority=150
* match=(REGBIT_KNOWN_LB_SESSION == 1 && ct.new && ip4.dst == V
* && REG_LB_AFF_BACKEND_IP4 == B1 && REG_LB_AFF_MATCH_PORT == BP1)
- * action=(REGBIT_CONNTRACK_COMMIT = 0;
- * REG_ORIG_DIP_IPV4 = V; REG_ORIG_TP_DPORT = VP;
+ * action=(REG_ORIG_DIP_IPV4 = V; REG_ORIG_TP_DPORT = VP;
* ct_lb_mark(backends=B1:BP1);)
* table=ls_in_lb, priority=150
* match=(REGBIT_KNOWN_LB_SESSION == 1 && ct.new && ip4.dst == V
* && REG_LB_AFF_BACKEND_IP4 == B2 && REG_LB_AFF_MATCH_PORT == BP2)
- * action=(REGBIT_CONNTRACK_COMMIT = 0;
- * REG_ORIG_DIP_IPV4 = V;
+ * action=(REG_ORIG_DIP_IPV4 = V;
* REG_ORIG_TP_DPORT = VP;
* ct_lb_mark(backends=B1:BP2);)
*
@@ -7514,8 +7512,7 @@ build_lb_affinity_ls_flows(struct lflow_table *lflows,
ipv6 ? REG_LB_L2_AFF_BACKEND_IP6 : REG_LB_AFF_BACKEND_IP4;
/* Prepare common part of affinity LB and affinity learn action. */
- ds_put_format(&aff_action, REGBIT_CONNTRACK_COMMIT" = 0; %s = %s; ",
- reg_vip, lb_vip->vip_str);
+ ds_put_format(&aff_action, "%s = %s; ", reg_vip, lb_vip->vip_str);
ds_put_cstr(&aff_action_learn, "commit_lb_aff(vip = \"");
if (lb_vip->port_str) {
@@ -7655,11 +7652,6 @@ build_lb_rules(struct lflow_table *lflows, struct ovn_lb_datapaths *lb_dps,
ds_clear(action);
ds_clear(match);
- /* Make sure that we clear the REGBIT_CONNTRACK_COMMIT flag. Otherwise
- * the load balanced packet will be committed again in
- * S_SWITCH_IN_STATEFUL. */
- ds_put_format(action, REGBIT_CONNTRACK_COMMIT" = 0; ");
-
/* New connections in Ingress table. */
const char *meter = NULL;
bool reject = build_lb_vip_actions(lb, lb_vip, lb_vip_nb, action,
@@ -1413,7 +1413,7 @@ check ovn-nbctl --wait=sb ls-lb-add sw0 lb1
AT_CAPTURE_FILE([sbflows])
OVS_WAIT_FOR_OUTPUT(
[ovn-sbctl dump-flows sw0 | tee sbflows | grep 'priority=120.*backends' | ovn_strip_lflows], 0, [dnl
- table=??(ls_in_lb ), priority=120 , match=(ct.new && ip4.dst == 10.0.0.10 && tcp.dst == 80), action=(reg0[[1]] = 0; ct_lb_mark(backends=10.0.0.3:80,20.0.0.3:80);)
+ table=??(ls_in_lb ), priority=120 , match=(ct.new && ip4.dst == 10.0.0.10 && tcp.dst == 80), action=(ct_lb_mark(backends=10.0.0.3:80,20.0.0.3:80);)
])
# disabled LSPs should not be a backend of Load Balancer
@@ -1422,7 +1422,7 @@ check ovn-nbctl lsp-set-enabled sw0-p1 disabled
AT_CAPTURE_FILE([sbflows])
OVS_WAIT_FOR_OUTPUT(
[ovn-sbctl dump-flows sw0 | tee sbflows | grep 'priority=120.*backends' | ovn_strip_lflows], 0, [dnl
- table=??(ls_in_lb ), priority=120 , match=(ct.new && ip4.dst == 10.0.0.10 && tcp.dst == 80), action=(reg0[[1]] = 0; ct_lb_mark(backends=20.0.0.3:80);)
+ table=??(ls_in_lb ), priority=120 , match=(ct.new && ip4.dst == 10.0.0.10 && tcp.dst == 80), action=(ct_lb_mark(backends=20.0.0.3:80);)
])
wait_row_count Service_Monitor 1
@@ -1431,7 +1431,7 @@ check ovn-nbctl lsp-set-enabled sw0-p1 enabled
AT_CAPTURE_FILE([sbflows])
OVS_WAIT_FOR_OUTPUT(
[ovn-sbctl dump-flows sw0 | tee sbflows | grep 'priority=120.*backends' | ovn_strip_lflows], 0, [dnl
- table=??(ls_in_lb ), priority=120 , match=(ct.new && ip4.dst == 10.0.0.10 && tcp.dst == 80), action=(reg0[[1]] = 0; ct_lb_mark(backends=10.0.0.3:80,20.0.0.3:80);)
+ table=??(ls_in_lb ), priority=120 , match=(ct.new && ip4.dst == 10.0.0.10 && tcp.dst == 80), action=(ct_lb_mark(backends=10.0.0.3:80,20.0.0.3:80);)
])
wait_row_count Service_Monitor 2
@@ -1442,7 +1442,7 @@ wait_row_count Service_Monitor 0
AT_CAPTURE_FILE([sbflows2])
OVS_WAIT_FOR_OUTPUT(
[ovn-sbctl dump-flows sw0 | tee sbflows2 | grep 'priority=120.*backends' | ovn_strip_lflows], [0],
-[ table=??(ls_in_lb ), priority=120 , match=(ct.new && ip4.dst == 10.0.0.10 && tcp.dst == 80), action=(reg0[[1]] = 0; ct_lb_mark(backends=10.0.0.3:80,20.0.0.3:80);)
+[ table=??(ls_in_lb ), priority=120 , match=(ct.new && ip4.dst == 10.0.0.10 && tcp.dst == 80), action=(ct_lb_mark(backends=10.0.0.3:80,20.0.0.3:80);)
])
AS_BOX([Create the Load_Balancer_Health_Check again.])
@@ -1454,7 +1454,7 @@ check ovn-nbctl --wait=sb sync
ovn-sbctl dump-flows sw0 | grep backends | grep priority=120 > lflows.txt
AT_CHECK([cat lflows.txt | ovn_strip_lflows], [0], [dnl
- table=??(ls_in_lb ), priority=120 , match=(ct.new && ip4.dst == 10.0.0.10 && tcp.dst == 80), action=(reg0[[1]] = 0; ct_lb_mark(backends=10.0.0.3:80,20.0.0.3:80);)
+ table=??(ls_in_lb ), priority=120 , match=(ct.new && ip4.dst == 10.0.0.10 && tcp.dst == 80), action=(ct_lb_mark(backends=10.0.0.3:80,20.0.0.3:80);)
])
AS_BOX([Get the uuid of both the service_monitor])
@@ -1464,7 +1464,7 @@ sm_sw1_p1=$(fetch_column Service_Monitor _uuid logical_port=sw1-p1)
AT_CAPTURE_FILE([sbflows3])
OVS_WAIT_FOR_OUTPUT(
[ovn-sbctl dump-flows sw0 | tee sbflows 3 | grep 'priority=120.*backends' | ovn_strip_lflows], [0],
-[ table=??(ls_in_lb ), priority=120 , match=(ct.new && ip4.dst == 10.0.0.10 && tcp.dst == 80), action=(reg0[[1]] = 0; ct_lb_mark(backends=10.0.0.3:80,20.0.0.3:80);)
+[ table=??(ls_in_lb ), priority=120 , match=(ct.new && ip4.dst == 10.0.0.10 && tcp.dst == 80), action=(ct_lb_mark(backends=10.0.0.3:80,20.0.0.3:80);)
])
AS_BOX([Set the service monitor for sw1-p1 to offline])
@@ -1475,7 +1475,7 @@ check ovn-nbctl --wait=sb sync
AT_CAPTURE_FILE([sbflows4])
OVS_WAIT_FOR_OUTPUT(
[ovn-sbctl dump-flows sw0 | tee sbflows4 | grep 'priority=120.*backends' | ovn_strip_lflows], [0],
-[ table=??(ls_in_lb ), priority=120 , match=(ct.new && ip4.dst == 10.0.0.10 && tcp.dst == 80), action=(reg0[[1]] = 0; ct_lb_mark(backends=10.0.0.3:80);)
+[ table=??(ls_in_lb ), priority=120 , match=(ct.new && ip4.dst == 10.0.0.10 && tcp.dst == 80), action=(ct_lb_mark(backends=10.0.0.3:80);)
])
AS_BOX([Set the service monitor for sw0-p1 to offline])
@@ -1504,7 +1504,7 @@ check ovn-nbctl --wait=sb sync
AT_CAPTURE_FILE([sbflows7])
OVS_WAIT_FOR_OUTPUT(
[ovn-sbctl dump-flows sw0 | tee sbflows7 | grep backends | grep priority=120 | ovn_strip_lflows], 0,
-[ table=??(ls_in_lb ), priority=120 , match=(ct.new && ip4.dst == 10.0.0.10 && tcp.dst == 80), action=(reg0[[1]] = 0; ct_lb_mark(backends=10.0.0.3:80,20.0.0.3:80);)
+[ table=??(ls_in_lb ), priority=120 , match=(ct.new && ip4.dst == 10.0.0.10 && tcp.dst == 80), action=(ct_lb_mark(backends=10.0.0.3:80,20.0.0.3:80);)
])
AS_BOX([Set the service monitor for sw1-p1 to error])
@@ -1515,7 +1515,7 @@ check ovn-nbctl --wait=sb sync
ovn-sbctl dump-flows sw0 | grep "ip4.dst == 10.0.0.10 && tcp.dst == 80" \
| grep priority=120 > lflows.txt
AT_CHECK([cat lflows.txt | grep ls_in_lb | ovn_strip_lflows], [0], [dnl
- table=??(ls_in_lb ), priority=120 , match=(ct.new && ip4.dst == 10.0.0.10 && tcp.dst == 80), action=(reg0[[1]] = 0; ct_lb_mark(backends=10.0.0.3:80);)
+ table=??(ls_in_lb ), priority=120 , match=(ct.new && ip4.dst == 10.0.0.10 && tcp.dst == 80), action=(ct_lb_mark(backends=10.0.0.3:80);)
])
AS_BOX([Add one more vip to lb1])
@@ -1541,8 +1541,8 @@ AT_CAPTURE_FILE([sbflows9])
OVS_WAIT_FOR_OUTPUT(
[ovn-sbctl dump-flows sw0 | tee sbflows9 | grep backends | grep priority=120 | ovn_strip_lflows],
0,
-[ table=??(ls_in_lb ), priority=120 , match=(ct.new && ip4.dst == 10.0.0.10 && tcp.dst == 80), action=(reg0[[1]] = 0; ct_lb_mark(backends=10.0.0.3:80);)
- table=??(ls_in_lb ), priority=120 , match=(ct.new && ip4.dst == 10.0.0.40 && tcp.dst == 1000), action=(reg0[[1]] = 0; ct_lb_mark(backends=10.0.0.3:1000);)
+[ table=??(ls_in_lb ), priority=120 , match=(ct.new && ip4.dst == 10.0.0.10 && tcp.dst == 80), action=(ct_lb_mark(backends=10.0.0.3:80);)
+ table=??(ls_in_lb ), priority=120 , match=(ct.new && ip4.dst == 10.0.0.40 && tcp.dst == 1000), action=(ct_lb_mark(backends=10.0.0.3:1000);)
])
AS_BOX([Set the service monitor for sw1-p1 to online])
@@ -1555,8 +1555,8 @@ AT_CAPTURE_FILE([sbflows10])
OVS_WAIT_FOR_OUTPUT(
[ovn-sbctl dump-flows sw0 | tee sbflows10 | grep backends | grep priority=120 | ovn_strip_lflows],
0,
-[ table=??(ls_in_lb ), priority=120 , match=(ct.new && ip4.dst == 10.0.0.10 && tcp.dst == 80), action=(reg0[[1]] = 0; ct_lb_mark(backends=10.0.0.3:80,20.0.0.3:80);)
- table=??(ls_in_lb ), priority=120 , match=(ct.new && ip4.dst == 10.0.0.40 && tcp.dst == 1000), action=(reg0[[1]] = 0; ct_lb_mark(backends=10.0.0.3:1000,20.0.0.3:80);)
+[ table=??(ls_in_lb ), priority=120 , match=(ct.new && ip4.dst == 10.0.0.10 && tcp.dst == 80), action=(ct_lb_mark(backends=10.0.0.3:80,20.0.0.3:80);)
+ table=??(ls_in_lb ), priority=120 , match=(ct.new && ip4.dst == 10.0.0.40 && tcp.dst == 1000), action=(ct_lb_mark(backends=10.0.0.3:1000,20.0.0.3:80);)
])
AS_BOX([Associate lb1 to sw1])
@@ -1565,8 +1565,8 @@ AT_CAPTURE_FILE([sbflows11])
OVS_WAIT_FOR_OUTPUT(
[ovn-sbctl dump-flows sw1 | tee sbflows11 | grep backends | grep priority=120 | ovn_strip_lflows],
0, [dnl
- table=??(ls_in_lb ), priority=120 , match=(ct.new && ip4.dst == 10.0.0.10 && tcp.dst == 80), action=(reg0[[1]] = 0; ct_lb_mark(backends=10.0.0.3:80,20.0.0.3:80);)
- table=??(ls_in_lb ), priority=120 , match=(ct.new && ip4.dst == 10.0.0.40 && tcp.dst == 1000), action=(reg0[[1]] = 0; ct_lb_mark(backends=10.0.0.3:1000,20.0.0.3:80);)
+ table=??(ls_in_lb ), priority=120 , match=(ct.new && ip4.dst == 10.0.0.10 && tcp.dst == 80), action=(ct_lb_mark(backends=10.0.0.3:80,20.0.0.3:80);)
+ table=??(ls_in_lb ), priority=120 , match=(ct.new && ip4.dst == 10.0.0.40 && tcp.dst == 1000), action=(ct_lb_mark(backends=10.0.0.3:1000,20.0.0.3:80);)
])
AS_BOX([Now create lb2 same as lb1 but udp protocol.])
@@ -4602,8 +4602,8 @@ check_stateful_flows() {
AT_CHECK([grep "ls_in_lb " sw0flows | ovn_strip_lflows], [0], [dnl
table=??(ls_in_lb ), priority=0 , match=(1), action=(next;)
- table=??(ls_in_lb ), priority=120 , match=(ct.new && ip4.dst == 10.0.0.10 && tcp.dst == 80), action=(reg0[[1]] = 0; ct_lb_mark(backends=10.0.0.4:8080);)
- table=??(ls_in_lb ), priority=120 , match=(ct.new && ip4.dst == 10.0.0.20 && tcp.dst == 80), action=(reg0[[1]] = 0; ct_lb_mark(backends=10.0.0.40:8080);)
+ table=??(ls_in_lb ), priority=120 , match=(ct.new && ip4.dst == 10.0.0.10 && tcp.dst == 80), action=(ct_lb_mark(backends=10.0.0.4:8080);)
+ table=??(ls_in_lb ), priority=120 , match=(ct.new && ip4.dst == 10.0.0.20 && tcp.dst == 80), action=(ct_lb_mark(backends=10.0.0.40:8080);)
])
AT_CHECK([grep "ls_in_stateful" sw0flows | ovn_strip_lflows], [0], [dnl
@@ -4805,6 +4805,51 @@ AT_CHECK([grep "ls_out_stateful" sw0flows | ovn_strip_lflows], [0], [dnl
AT_CLEANUP
])
+OVN_FOR_EACH_NORTHD_NO_HV([
+AT_SETUP([ovn -- ACL label commit - load balancers])
+ovn_start
+
+dnl One logical switch, two ports, one load balancer and ACLs with label set.
+check ovn-nbctl \
+ -- ls-add ls \
+ -- lsp-add ls lsp1 \
+ -- lsp-set-addresses lsp1 "00:00:00:00:00:01" \
+ -- lsp-add ls lsp2 \
+ -- lsp-set-addresses lsp2 "00:00:00:00:00:02" \
+ -- lb-add lb-test 42.42.42.42:4242 43.43.43.43:4343 udp \
+ -- ls-lb-add ls lb-test
+
+check ovn-nbctl --wait=sb sync
+
+flow="inport == \"lsp1\" && eth.src == 00:00:00:00:00:01 && eth.dst == 00:00:00:00:00:02 && ip4.src == 42.42.42.1 && ip4.dst == 42.42.42.42 && ip.ttl==64 && udp.dst == 4242"
+
+AS_BOX([from-lport allow-related ACL])
+check ovn-nbctl --wait=sb -- acl-del ls -- --label=1234 acl-add ls from-lport 1 ip allow-related
+
+dnl Check that the label is committed to conntrack in the ingress pipeline
+AT_CHECK_UNQUOTED([ovn_trace --ct new --ct new --ct new ls "$flow" | grep -e ls_in_stateful -A 2 | grep commit], [0], [dnl
+ ct_commit { ct_mark.blocked = 0; ct_label.label = reg3; };
+])
+
+AS_BOX([from-lport --apply-after-lb allow-related ACL])
+check ovn-nbctl --wait=sb -- acl-del ls -- --apply-after-lb --label=1234 acl-add ls from-lport 1 ip allow-related
+
+dnl Check that the label is committed to conntrack in the ingress pipeline
+AT_CHECK_UNQUOTED([ovn_trace --ct new --ct new --ct new ls "$flow" | grep -e ls_in_stateful -A 2 | grep commit], [0], [dnl
+ ct_commit { ct_mark.blocked = 0; ct_label.label = reg3; };
+])
+
+AS_BOX([to-lport allow-related ACL])
+check ovn-nbctl --wait=sb -- acl-del ls -- --label=1234 acl-add ls to-lport 1 ip allow-related
+
+dnl Check that the label is committed to conntrack in the ingress pipeline
+AT_CHECK_UNQUOTED([ovn_trace --ct new --ct new --ct new ls "$flow" | grep -e ls_out_stateful -A 2 | grep commit], [0], [dnl
+ ct_commit { ct_mark.blocked = 0; ct_label.label = reg3; };
+])
+
+AT_CLEANUP
+])
+
OVN_FOR_EACH_NORTHD_NO_HV_PARALLELIZATION([
AT_SETUP([ovn -- ct.inv usage])
ovn_start
@@ -7629,7 +7674,7 @@ AT_CHECK([grep -e "ls_in_acl.*eval" -e "ls_in_acl_hint" lsflows | ovn_strip_lflo
AT_CHECK([grep -e "ls_in_lb " lsflows | ovn_strip_lflows], [0], [dnl
table=??(ls_in_lb ), priority=0 , match=(1), action=(next;)
- table=??(ls_in_lb ), priority=110 , match=(ct.new && ip4.dst == 10.0.0.2), action=(reg0[[1]] = 0; ct_lb_mark(backends=10.0.0.10);)
+ table=??(ls_in_lb ), priority=110 , match=(ct.new && ip4.dst == 10.0.0.2), action=(ct_lb_mark(backends=10.0.0.10);)
])
AT_CHECK([grep -e "ls_in_stateful" lsflows | ovn_strip_lflows], [0], [dnl
@@ -7684,7 +7729,7 @@ AT_CHECK([grep -e "ls_in_acl.*eval" -e "ls_in_acl_hint" lsflows | ovn_strip_lflo
AT_CHECK([grep -e "ls_in_lb " lsflows | ovn_strip_lflows], [0], [dnl
table=??(ls_in_lb ), priority=0 , match=(1), action=(next;)
- table=??(ls_in_lb ), priority=110 , match=(ct.new && ip4.dst == 10.0.0.2), action=(reg0[[1]] = 0; ct_lb_mark(backends=10.0.0.10);)
+ table=??(ls_in_lb ), priority=110 , match=(ct.new && ip4.dst == 10.0.0.2), action=(ct_lb_mark(backends=10.0.0.10);)
])
AT_CHECK([grep -e "ls_in_stateful" lsflows | ovn_strip_lflows], [0], [dnl
@@ -7739,7 +7784,7 @@ AT_CHECK([grep -e "ls_in_acl.*eval" -e "ls_in_acl_hint" lsflows | ovn_strip_lflo
AT_CHECK([grep -e "ls_in_lb " lsflows | ovn_strip_lflows], [0], [dnl
table=??(ls_in_lb ), priority=0 , match=(1), action=(next;)
- table=??(ls_in_lb ), priority=110 , match=(ct.new && ip4.dst == 10.0.0.2), action=(reg0[[1]] = 0; ct_lb_mark(backends=10.0.0.10);)
+ table=??(ls_in_lb ), priority=110 , match=(ct.new && ip4.dst == 10.0.0.2), action=(ct_lb_mark(backends=10.0.0.10);)
])
AT_CHECK([grep -e "ls_in_stateful" lsflows | ovn_strip_lflows], [0], [dnl
@@ -9063,13 +9108,13 @@ AT_CAPTURE_FILE([S1flows])
AT_CHECK([grep "ls_in_lb " S0flows | ovn_strip_lflows], [0], [dnl
table=??(ls_in_lb ), priority=0 , match=(1), action=(next;)
- table=??(ls_in_lb ), priority=120 , match=(ct.new && ip4.dst == 172.16.0.10 && tcp.dst == 80), action=(reg0[[1]] = 0; ct_lb_mark(backends=10.0.0.2:80);)
- table=??(ls_in_lb ), priority=120 , match=(ct.new && ip4.dst == 172.16.0.11 && tcp.dst == 8080), action=(reg0[[1]] = 0; ct_lb_mark(backends=10.0.0.2:8080);)
+ table=??(ls_in_lb ), priority=120 , match=(ct.new && ip4.dst == 172.16.0.10 && tcp.dst == 80), action=(ct_lb_mark(backends=10.0.0.2:80);)
+ table=??(ls_in_lb ), priority=120 , match=(ct.new && ip4.dst == 172.16.0.11 && tcp.dst == 8080), action=(ct_lb_mark(backends=10.0.0.2:8080);)
])
AT_CHECK([grep "ls_in_lb " S1flows | ovn_strip_lflows], [0], [dnl
table=??(ls_in_lb ), priority=0 , match=(1), action=(next;)
- table=??(ls_in_lb ), priority=120 , match=(ct.new && ip4.dst == 172.16.0.10 && tcp.dst == 80), action=(reg0[[1]] = 0; ct_lb_mark(backends=10.0.0.2:80);)
- table=??(ls_in_lb ), priority=120 , match=(ct.new && ip4.dst == 172.16.0.11 && tcp.dst == 8080), action=(reg0[[1]] = 0; ct_lb_mark(backends=10.0.0.2:8080);)
+ table=??(ls_in_lb ), priority=120 , match=(ct.new && ip4.dst == 172.16.0.10 && tcp.dst == 80), action=(ct_lb_mark(backends=10.0.0.2:80);)
+ table=??(ls_in_lb ), priority=120 , match=(ct.new && ip4.dst == 172.16.0.11 && tcp.dst == 8080), action=(ct_lb_mark(backends=10.0.0.2:8080);)
])
ovn-sbctl get datapath S0 _uuid > dp_uuids
@@ -9199,9 +9244,9 @@ AT_CHECK([grep "ls_in_lb_aff_check" S0flows | ovn_strip_lflows], [0], [dnl
])
AT_CHECK([grep "ls_in_lb " S0flows | ovn_strip_lflows], [0], [dnl
table=??(ls_in_lb ), priority=0 , match=(1), action=(next;)
- table=??(ls_in_lb ), priority=120 , match=(ct.new && ip4.dst == 172.16.0.10 && tcp.dst == 80), action=(reg0[[1]] = 0; ct_lb_mark(backends=10.0.0.2:80,20.0.0.2:80);)
- table=??(ls_in_lb ), priority=150 , match=(reg9[[6]] == 1 && ct.new && ip4.dst == 172.16.0.10 && reg4 == 10.0.0.2 && reg8[[0..15]] == 80), action=(reg0[[1]] = 0; reg1 = 172.16.0.10; reg2[[0..15]] = 80; ct_lb_mark(backends=10.0.0.2:80);)
- table=??(ls_in_lb ), priority=150 , match=(reg9[[6]] == 1 && ct.new && ip4.dst == 172.16.0.10 && reg4 == 20.0.0.2 && reg8[[0..15]] == 80), action=(reg0[[1]] = 0; reg1 = 172.16.0.10; reg2[[0..15]] = 80; ct_lb_mark(backends=20.0.0.2:80);)
+ table=??(ls_in_lb ), priority=120 , match=(ct.new && ip4.dst == 172.16.0.10 && tcp.dst == 80), action=(ct_lb_mark(backends=10.0.0.2:80,20.0.0.2:80);)
+ table=??(ls_in_lb ), priority=150 , match=(reg9[[6]] == 1 && ct.new && ip4.dst == 172.16.0.10 && reg4 == 10.0.0.2 && reg8[[0..15]] == 80), action=(reg1 = 172.16.0.10; reg2[[0..15]] = 80; ct_lb_mark(backends=10.0.0.2:80);)
+ table=??(ls_in_lb ), priority=150 , match=(reg9[[6]] == 1 && ct.new && ip4.dst == 172.16.0.10 && reg4 == 20.0.0.2 && reg8[[0..15]] == 80), action=(reg1 = 172.16.0.10; reg2[[0..15]] = 80; ct_lb_mark(backends=20.0.0.2:80);)
])
AT_CHECK([grep "ls_in_lb_aff_learn" S0flows | ovn_strip_lflows], [0], [dnl
table=??(ls_in_lb_aff_learn ), priority=0 , match=(1), action=(next;)
@@ -10286,14 +10331,17 @@ dnl It should not commit anything in the egress pipeline of S1 or in the
dnl ingress pipeline of S2.
flow="inport == \"vm1\" && eth.src == 00:de:ad:01:00:01 && eth.dst == 00:de:ad:fe:00:01 && ip4.src == 173.0.1.2 && ip4.dst == 30.0.0.1 && ip.ttl==64"
-dnl Check that we only commit once for ACLs, in the egress ACL pipeline
-dnl (in S2, towards vm2). The original problem this test is trying to
-dnl cover was that ct_state wasn't cleared when traversing from s1 -> r1
-dnl which caused two additional commits to happen:
+dnl Check that we only commit twice for ACLs:
+dnl - in the ingress ACL pipeline (in s1, from vm1)
+dnl - in the egress ACL pipeline (in S2, towards vm2)
+dnl The original problem this test is trying to cover was that ct_state
+dnl wasn't cleared when traversing from s1 -> r1 which caused two additional
+dnl commits to happen:
dnl - in the egress pipeline of S1, when sending the packet out on s1_r1
dnl - in the ingress pipeline of S2, when processing the packet on s2_r1
AT_CHECK_UNQUOTED([ovn_trace --ct new --ct new --ct new s1 "$flow" | grep -e ls_in_stateful -e ls_out_stateful -A 2 | grep commit], [0], [dnl
ct_commit { ct_mark.blocked = 0; };
+ ct_commit { ct_mark.blocked = 0; };
])
AT_CLEANUP
@@ -25509,7 +25509,7 @@ OVS_WAIT_FOR_OUTPUT(
ovn-sbctl dump-flows sw0 | grep ct_lb_mark | grep priority=120 | sed 's/table=..//'], 0,
[dnl
(ls_in_pre_stateful ), priority=120 , match=(reg0[[2]] == 1 && ip4.dst == 10.0.0.10 && tcp.dst == 80), action=(reg1 = 10.0.0.10; reg2[[0..15]] = 80; ct_lb_mark;)
- (ls_in_lb ), priority=120 , match=(ct.new && ip4.dst == 10.0.0.10 && tcp.dst == 80), action=(reg0[[1]] = 0; ct_lb_mark(backends=10.0.0.3:80,20.0.0.3:80; hash_fields="ip_dst,ip_src,tcp_dst,tcp_src");)
+ (ls_in_lb ), priority=120 , match=(ct.new && ip4.dst == 10.0.0.10 && tcp.dst == 80), action=(ct_lb_mark(backends=10.0.0.3:80,20.0.0.3:80; hash_fields="ip_dst,ip_src,tcp_dst,tcp_src");)
])
AT_CAPTURE_FILE([sbflows2])
@@ -25708,7 +25708,7 @@ OVS_WAIT_FOR_OUTPUT(
ovn-sbctl dump-flows sw0 | grep ct_lb_mark | grep priority=120 | sed 's/table=..//'], 0,
[dnl
(ls_in_pre_stateful ), priority=120 , match=(reg0[[2]] == 1 && ip6.dst == 2001::a && tcp.dst == 80), action=(xxreg1 = 2001::a; reg2[[0..15]] = 80; ct_lb_mark;)
- (ls_in_lb ), priority=120 , match=(ct.new && ip6.dst == 2001::a && tcp.dst == 80), action=(reg0[[1]] = 0; ct_lb_mark(backends=[[2001::3]]:80,[[2002::3]]:80; hash_fields="ip_dst,ip_src,tcp_dst,tcp_src");)
+ (ls_in_lb ), priority=120 , match=(ct.new && ip6.dst == 2001::a && tcp.dst == 80), action=(ct_lb_mark(backends=[[2001::3]]:80,[[2002::3]]:80; hash_fields="ip_dst,ip_src,tcp_dst,tcp_src");)
])
AT_CAPTURE_FILE([sbflows2])
Quoting the ACL label section in the ovn.nb.5 man page: Associates an identifier with the ACL. The same value will be written to corresponding connection tracker entry. The value should be a valid 32-bit unsigned integer. This value can help in debugging from connection tracker side. For example, through this "label" we can backtrack to the ACL rule which is causing a "leaked" connection. Connection tracker entries are created only for allowed connections so the label is valid only for allow and allow-related actions. The above states that the ACL label must always be stored in the connection tracker entry label for allow-related ACLs (regardless of the direction of the ACL). However, since 74d82e296f80 ("northd: Support the option to apply from-lport ACLs after load balancer."), the connection is not re-committed in the ls_in_stateful stage (because it already was committed as part of the load balancer DNAT). Moreover, by not re-committing the connection after LB we also risk not re-setting any potential ct_mark.blocked value the connection might have. This patch addresses the issue by always committing packets matched by allow-related (or stateful in general) ACLs even if they were also committed as part of the load balancing stage. There's potentially a slight overhead when doing this (an additional commit call into conntrack but _no_ recirculation). This is however acceptable as it is required for a correct packet processing pipeline implementation. Even without this fix, packets creating new connections that hit "--apply-after-lb" ACLs trigger a re-commit (for storing the label and ct_mark.blocked). A new test is added to ensure we don't break this functionality in the future. CC: Numan Siddique <numans@ovn.org> Fixes: 74d82e296f80 ("northd: Support the option to apply from-lport ACLs after load balancer.") Acked-by: Mark Michelson <mmichels@redhat.com> Acked-by: Numan Siddique <numans@ovn.org> Signed-off-by: Dumitru Ceara <dceara@redhat.com> --- V4: - Addressed Mark's comment: - fixed up build_lb_affinity_ls_flows() comment - Added Mark's and Numan's acks. --- northd/northd.c | 14 ++---- tests/ovn-northd.at | 110 +++++++++++++++++++++++++++++++------------- tests/ovn.at | 4 +- 3 files changed, 84 insertions(+), 44 deletions(-)