Skip to content

Commit 480f238

Browse files
LorenzoBianconidceara
authored andcommitted
northd: Fix action parsing in build_lb_vip_actions().
Fix the following error reported in build_lb_vip_actions() if the CMS configure a load balancer with no backends: 2025-01-21T18:18:26.668Z|04152|lflow|WARN|error parsing actions "flags.force_snat_for_lb = 1; drop;": Syntax error at `drop' expecting action. The issue can be triggered with the following reproducer in ovn-sandbox: $ovn-nbctl create load_balancer name=lb-empty vips:\"172.168.10.30\"=\"\" protocol=tcp options:skip_snat=true $ovn-nbctl lr-lb-add lr0 lb-empty $grep Syntax ./sandbox/ovn-controller.log 2025-01-22T17:13:49.709Z|00047|lflow|WARN|error parsing actions "flags.skip_snat_for_lb = 1; drop;": Syntax error at `drop' expecting action. Reported-at: https://issues.redhat.com/browse/FDP-1095 Signed-off-by: Lorenzo Bianconi <[email protected]> Acked-by: Ales Musil <[email protected]> Signed-off-by: Dumitru Ceara <[email protected]> (cherry picked from commit c0af418)
1 parent a3a45a1 commit 480f238

File tree

2 files changed

+8
-4
lines changed

2 files changed

+8
-4
lines changed

northd/northd.c

+6-2
Original file line numberDiff line numberDiff line change
@@ -3498,10 +3498,14 @@ build_lb_vip_actions(const struct ovn_northd_lb *lb,
34983498
const char *enclose = is_lb_action ? ");" : "";
34993499

35003500
if (!ls_dp) {
3501-
ds_put_format(skip_snat_action, "flags.skip_snat_for_lb = 1; %s%s",
3501+
if (!drop) {
3502+
ds_put_cstr(skip_snat_action, "flags.skip_snat_for_lb = 1; ");
3503+
ds_put_cstr(force_snat_action, "flags.force_snat_for_lb = 1; ");
3504+
}
3505+
ds_put_format(skip_snat_action, "%s%s",
35023506
ds_cstr(action),
35033507
is_lb_action ? "; skip_snat);" : enclose);
3504-
ds_put_format(force_snat_action, "flags.force_snat_for_lb = 1; %s%s",
3508+
ds_put_format(force_snat_action, "%s%s",
35053509
ds_cstr(action),
35063510
is_lb_action ? "; force_snat);" : enclose);
35073511
}

tests/ovn-northd.at

+2-2
Original file line numberDiff line numberDiff line change
@@ -6266,7 +6266,7 @@ check ovn-nbctl --wait=sb set load_balancer lb6 options:skip_snat=true
62666266

62676267
AT_CHECK([ovn-sbctl dump-flows lr0 | grep "lr_in_dnat" | sort], [0], [dnl
62686268
table=7 (lr_in_dnat ), priority=0 , match=(1), action=(next;)
6269-
table=7 (lr_in_dnat ), priority=110 , match=(ct.new && !ct.rel && ip4 && ip4.dst == 172.168.10.30), action=(flags.skip_snat_for_lb = 1; drop;)
6269+
table=7 (lr_in_dnat ), priority=110 , match=(ct.new && !ct.rel && ip4 && ip4.dst == 172.168.10.30), action=(drop;)
62706270
table=7 (lr_in_dnat ), priority=50 , match=(ct.est && !ct.rel && !ct.new && !ct.rpl && ct_mark.natted), action=(next;)
62716271
table=7 (lr_in_dnat ), priority=50 , match=(ct.rel && !ct.est && !ct.new && !ct.rpl), action=(ct_commit_nat;)
62726272
table=7 (lr_in_dnat ), priority=70 , match=(ct.est && !ct.rel && !ct.new && !ct.rpl && ct_mark.natted && ct_mark.force_snat == 1), action=(flags.force_snat_for_lb = 1; next;)
@@ -6282,7 +6282,7 @@ check ovn-nbctl --wait=sb set logical_router lr0 options:lb_force_snat_ip="route
62826282

62836283
AT_CHECK([ovn-sbctl dump-flows lr0 | grep "lr_in_dnat" | sort], [0], [dnl
62846284
table=7 (lr_in_dnat ), priority=0 , match=(1), action=(next;)
6285-
table=7 (lr_in_dnat ), priority=110 , match=(ct.new && !ct.rel && ip4 && ip4.dst == 172.168.10.30), action=(flags.force_snat_for_lb = 1; drop;)
6285+
table=7 (lr_in_dnat ), priority=110 , match=(ct.new && !ct.rel && ip4 && ip4.dst == 172.168.10.30), action=(drop;)
62866286
table=7 (lr_in_dnat ), priority=50 , match=(ct.est && !ct.rel && !ct.new && !ct.rpl && ct_mark.natted), action=(next;)
62876287
table=7 (lr_in_dnat ), priority=50 , match=(ct.rel && !ct.est && !ct.new && !ct.rpl), action=(ct_commit_nat;)
62886288
table=7 (lr_in_dnat ), priority=70 , match=(ct.est && !ct.rel && !ct.new && !ct.rpl && ct_mark.natted && ct_mark.force_snat == 1), action=(flags.force_snat_for_lb = 1; next;)

0 commit comments

Comments
 (0)