The affinity code was differentiating between force_snat
and skip snat. However, if both parameters were set at the
same time the force_snat would be preferred, which shouldn't
be the case.
Make sure the skip_snat is preferred if both parameters are
present.
Fixes: d3926b4 ("northd: rely on new actions for lb affinity")
Reported-at: https://bugzilla.redhat.com/2224260
Signed-off-by: Ales Musil <amu...@redhat.com>
Signed-off-by: Dumitru Ceara <dce...@redhat.com>
(cherry picked from commit 0f4df76)
---
northd/northd.c | 20 +++++++++++++++-----
tests/ovn-northd.at | 19 +++++++++++++++++++
2 files changed, 34 insertions(+), 5 deletions(-)
diff --git a/northd/northd.c b/northd/northd.c
index 00a9013dd..fd1f9c629 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -10669,6 +10669,10 @@ build_lrouter_nat_flows_for_lb(struct ovn_lb_vip
*lb_vip,
xcalloc(lb->n_nb_lr, sizeof *distributed_router);
int n_distributed_router = 0;
+ struct ovn_datapath **lb_aff_skip_snat_router =
+ xcalloc(lb->n_nb_lr, sizeof *lb_aff_skip_snat_router);
+ int n_lb_aff_skip_snat_router = 0;
+
struct ovn_datapath **lb_aff_force_snat_router =
xcalloc(lb->n_nb_lr, sizeof *lb_aff_force_snat_router);
int n_lb_aff_force_snat_router = 0;
@@ -10695,7 +10699,9 @@ build_lrouter_nat_flows_for_lb(struct ovn_lb_vip
*lb_vip,
distributed_router[n_distributed_router++] = od;
}
- if (!lport_addresses_is_empty(&od->lb_force_snat_addrs) ||
+ if (lb->skip_snat) {
+ lb_aff_skip_snat_router[n_lb_aff_skip_snat_router++] = od;
+ } else if (!lport_addresses_is_empty(&od->lb_force_snat_addrs) ||
od->lb_force_snat_router_ip) {
lb_aff_force_snat_router[n_lb_aff_force_snat_router++] = od;
} else {
@@ -10752,11 +10758,14 @@ build_lrouter_nat_flows_for_lb(struct ovn_lb_vip
*lb_vip,
"next;", lflows, prio, meter_groups);
/* LB affinity flows for datapaths where CMS has specified
- * skip_snat_for_lb floag option or regular datapaths.
+ * skip_snat_for_lb flag option.
*/
- char *lb_aff_action =
- lb->skip_snat ? "flags.skip_snat_for_lb = 1; " : NULL;
- build_lb_affinity_lr_flows(lflows, lb, lb_vip, new_match, lb_aff_action,
+ build_lb_affinity_lr_flows(lflows, lb, lb_vip, new_match,
+ "flags.skip_snat_for_lb = 1; ",
+ lb_aff_skip_snat_router,
+ n_lb_aff_skip_snat_router);
+
+ build_lb_affinity_lr_flows(lflows, lb, lb_vip, new_match, NULL,
lb_aff_router, n_lb_aff_router);
/* Distributed router logic */
@@ -10852,6 +10861,7 @@ build_lrouter_nat_flows_for_lb(struct ovn_lb_vip
*lb_vip,
free(gw_router_force_snat);
free(gw_router_skip_snat);
free(distributed_router);
+ free(lb_aff_skip_snat_router);
free(lb_aff_force_snat_router);
free(lb_aff_router);
free(gw_router);
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index b50f23e19..4e8eb38b8 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -8334,6 +8334,25 @@ AT_CHECK([grep "lr_in_dnat " R1flows_force_snat | sort],
[0], [dnl
table=7 (lr_in_dnat ), priority=70 , match=(ct.rel && !ct.est &&
!ct.new && ct_mark.skip_snat == 1), action=(flags.skip_snat_for_lb = 1;
ct_commit_nat;)
])
+AS_BOX([Test LR flows - lb_force_snat_ip="172.16.0.1" + skip_snat=true])
+check ovn-nbctl --wait=sb set logical_router R1
options:lb_force_snat_ip="172.16.0.1"
+check ovn-nbctl --wait=sb set load_balancer lb0 options:skip_snat=true
+
+ovn-sbctl dump-flows R1 > R1flows_force_skip_snat
+AT_CAPTURE_FILE([R1flows_force_skip_snat])
+
+grep "lr_in_dnat " R1flows_force_skip_snat | sed 's/table=../table=??/' | sort
+AT_CHECK([grep "lr_in_dnat " R1flows_force_skip_snat | sed
's/table=../table=??/' | sort], [0], [dnl
+ table=??(lr_in_dnat ), priority=0 , match=(1), action=(next;)
+ table=??(lr_in_dnat ), priority=120 , match=(ct.est && !ct.rel &&
ip4 && reg0 == 172.16.0.10 && tcp && reg9[[16..31]] == 80 && ct_mark.natted ==
1), action=(flags.skip_snat_for_lb = 1; next;)
+ table=??(lr_in_dnat ), priority=120 , match=(ct.new && !ct.rel &&
ip4 && reg0 == 172.16.0.10 && tcp && reg9[[16..31]] == 80),
action=(flags.skip_snat_for_lb = 1;
ct_lb_mark(backends=10.0.0.2:80,20.0.0.2:80; skip_snat);)
+ table=??(lr_in_dnat ), priority=150 , match=(reg9[[6]] == 1 &&
ct.new && ip4 && reg4 == 10.0.0.2 && reg8[[0..15]] == 80), action=(reg0 =
172.16.0.10; flags.skip_snat_for_lb = 1; ct_lb_mark(backends=10.0.0.2:80;
skip_snat);)
+ table=??(lr_in_dnat ), priority=150 , match=(reg9[[6]] == 1 &&
ct.new && ip4 && reg4 == 20.0.0.2 && reg8[[0..15]] == 80), action=(reg0 =
172.16.0.10; flags.skip_snat_for_lb = 1; ct_lb_mark(backends=20.0.0.2:80;
skip_snat);)
+ table=??(lr_in_dnat ), priority=50 , match=(ct.rel && !ct.est &&
!ct.new), action=(ct_commit_nat;)
+ table=??(lr_in_dnat ), priority=70 , match=(ct.rel && !ct.est &&
!ct.new && ct_mark.force_snat == 1), action=(flags.force_snat_for_lb = 1;
ct_commit_nat;)
+ table=??(lr_in_dnat ), priority=70 , match=(ct.rel && !ct.est &&
!ct.new && ct_mark.skip_snat == 1), action=(flags.skip_snat_for_lb = 1;
ct_commit_nat;)
+])
+
AT_CLEANUP
])
--
2.41.0
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
