Skip to content

Commit

Permalink
northd: Fix action parsing in build_lb_vip_actions().
Browse files Browse the repository at this point in the history
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 <lorenzo.bianconi@redhat.com>
Acked-by: Ales Musil <amusil@redhat.com>
Signed-off-by: Dumitru Ceara <dceara@redhat.com>
(cherry picked from commit c0af418)
  • Loading branch information
LorenzoBianconi authored and dceara committed Feb 28, 2025
1 parent d8e3fb6 commit 319cc0e
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 4 deletions.
8 changes: 6 additions & 2 deletions northd/northd.c
Original file line number Diff line number Diff line change
Expand Up @@ -3802,10 +3802,14 @@ build_lb_vip_actions(const struct ovn_northd_lb *lb,
const char *enclose = is_lb_action ? ");" : "";

if (!ls_dp) {
ds_put_format(skip_snat_action, "flags.skip_snat_for_lb = 1; %s%s",
if (!drop) {
ds_put_cstr(skip_snat_action, "flags.skip_snat_for_lb = 1; ");
ds_put_cstr(force_snat_action, "flags.force_snat_for_lb = 1; ");
}
ds_put_format(skip_snat_action, "%s%s",
ds_cstr(action),
is_lb_action ? "; skip_snat);" : enclose);
ds_put_format(force_snat_action, "flags.force_snat_for_lb = 1; %s%s",
ds_put_format(force_snat_action, "%s%s",
ds_cstr(action),
is_lb_action ? "; force_snat);" : enclose);
}
Expand Down
4 changes: 2 additions & 2 deletions tests/ovn-northd.at
Original file line number Diff line number Diff line change
Expand Up @@ -6566,7 +6566,7 @@ check ovn-nbctl --wait=sb set load_balancer lb6 options:skip_snat=true

AT_CHECK([ovn-sbctl dump-flows lr0 | grep "lr_in_dnat" | ovn_strip_lflows], [0], [dnl
table=??(lr_in_dnat ), priority=0 , match=(1), action=(next;)
table=??(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;)
table=??(lr_in_dnat ), priority=110 , match=(ct.new && !ct.rel && ip4 && ip4.dst == 172.168.10.30), action=(drop;)
table=??(lr_in_dnat ), priority=50 , match=(ct.est && !ct.rel && !ct.new && !ct.rpl && ct_mark.natted), action=(next;)
table=??(lr_in_dnat ), priority=50 , match=(ct.rel && !ct.est && !ct.new && !ct.rpl), action=(ct_commit_nat;)
table=??(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;)
Expand All @@ -6582,7 +6582,7 @@ check ovn-nbctl --wait=sb set logical_router lr0 options:lb_force_snat_ip="route

AT_CHECK([ovn-sbctl dump-flows lr0 | grep "lr_in_dnat" | ovn_strip_lflows], [0], [dnl
table=??(lr_in_dnat ), priority=0 , match=(1), action=(next;)
table=??(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;)
table=??(lr_in_dnat ), priority=110 , match=(ct.new && !ct.rel && ip4 && ip4.dst == 172.168.10.30), action=(drop;)
table=??(lr_in_dnat ), priority=50 , match=(ct.est && !ct.rel && !ct.new && !ct.rpl && ct_mark.natted), action=(next;)
table=??(lr_in_dnat ), priority=50 , match=(ct.rel && !ct.est && !ct.new && !ct.rpl), action=(ct_commit_nat;)
table=??(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;)
Expand Down

0 comments on commit 319cc0e

Please # to comment.