Skip to content

Commit 47f5fa1

Browse files
almusilalmusil
committed
northd: Make sure we handle multicast group update.
The multicast groups were updated as part of the lflow handler which is wrong. There is multicast group node which takes care of the group updates. Also, the updates in the lflow handler were partially wrong and not in line with the multicast group node. At the same time remove the lsp_can_receive_multicast() check that would prevent the lflow creation for new LSPs. Reported-at: https://redhat.atlassian.net/browse/FDP-4022 Assisted-by: Claude Opus 4.6, Claude Code Fixes: b741cb7 ("northd: Incremental processing of VIF updates and deletions in 'lflow' node.") Fixes: 8e2d6fa ("northd, tests: Network Function insertion logical flow programming.") Acked-by: Dumitru Ceara <dceara@redhat.com> Signed-off-by: Ales Musil <amusil@redhat.com> (cherry picked from commit 5a72b76)
1 parent 9054c40 commit 47f5fa1

3 files changed

Lines changed: 101 additions & 43 deletions

File tree

‎northd/en-multicast.c‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,10 @@ multicast_igmp_northd_handler(struct engine_node *node, void *data OVS_UNUSED)
153153
return EN_UNHANDLED;
154154
}
155155

156-
if (hmapx_count(&northd_data->trk_data.trk_switches.deleted)) {
156+
struct tracked_ovn_ports *trk_lsps = &northd_data->trk_data.trk_lsps;
157+
if (hmapx_count(&trk_lsps->created) ||
158+
hmapx_count(&trk_lsps->updated) ||
159+
hmapx_count(&trk_lsps->deleted)) {
157160
return EN_UNHANDLED;
158161
}
159162

@@ -171,7 +174,7 @@ multicast_igmp_northd_handler(struct engine_node *node, void *data OVS_UNUSED)
171174
* This node also accesses the router ports of the logical router
172175
* (od->ports). When these logical router ports gets updated,
173176
* en_northd engine recomputes and so does this node.
174-
* Note: When we add I-P to handle switch/router port changes, we
177+
* Note: When we add I-P to handle router port changes, we
175178
* need to revisit this handler.
176179
*
177180
* */

‎northd/northd.c‎

Lines changed: 0 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -20119,30 +20119,13 @@ lflow_handle_northd_port_changes(struct ovsdb_idl_txn *ovnsb_txn,
2011920119
if (!handled) {
2012020120
return false;
2012120121
}
20122-
20123-
/* SB port_binding is not deleted, so don't update SB multicast
20124-
* groups. */
2012520122
}
2012620123

2012720124
HMAPX_FOR_EACH (hmapx_node, &trk_lsps->created) {
2012820125
op = hmapx_node->data;
2012920126
/* Make sure 'op' is an lsp and not lrp. */
2013020127
ovs_assert(op->nbsp);
2013120128

20132-
if (!lsp_can_receive_multicast(op->nbsp)) {
20133-
continue;
20134-
}
20135-
20136-
const struct sbrec_multicast_group *sbmc_flood =
20137-
mcast_group_lookup(lflow_input->sbrec_mcast_group_by_name_dp,
20138-
MC_FLOOD, op->od->sdp->sb_dp);
20139-
const struct sbrec_multicast_group *sbmc_flood_l2 =
20140-
mcast_group_lookup(lflow_input->sbrec_mcast_group_by_name_dp,
20141-
MC_FLOOD_L2, op->od->sdp->sb_dp);
20142-
const struct sbrec_multicast_group *sbmc_unknown =
20143-
mcast_group_lookup(lflow_input->sbrec_mcast_group_by_name_dp,
20144-
MC_UNKNOWN, op->od->sdp->sb_dp);
20145-
2014620129
struct ds match = DS_EMPTY_INITIALIZER;
2014720130
struct ds actions = DS_EMPTY_INITIALIZER;
2014820131
build_lswitch_and_lrouter_iterate_by_lsp(op, lflow_input->ls_ports,
@@ -20177,30 +20160,6 @@ lflow_handle_northd_port_changes(struct ovsdb_idl_txn *ovnsb_txn,
2017720160
if (!handled) {
2017820161
return false;
2017920162
}
20180-
20181-
/* Update SB multicast groups for the new port. */
20182-
if (!sbmc_flood) {
20183-
sbmc_flood = create_sb_multicast_group(ovnsb_txn,
20184-
op->od->sdp->sb_dp, MC_FLOOD, OVN_MCAST_FLOOD_TUNNEL_KEY);
20185-
}
20186-
sbrec_multicast_group_update_ports_addvalue(sbmc_flood, op->sb);
20187-
20188-
if (!sbmc_flood_l2) {
20189-
sbmc_flood_l2 = create_sb_multicast_group(ovnsb_txn,
20190-
op->od->sdp->sb_dp, MC_FLOOD_L2,
20191-
OVN_MCAST_FLOOD_L2_TUNNEL_KEY);
20192-
}
20193-
sbrec_multicast_group_update_ports_addvalue(sbmc_flood_l2, op->sb);
20194-
20195-
if (op->has_unknown) {
20196-
if (!sbmc_unknown) {
20197-
sbmc_unknown = create_sb_multicast_group(ovnsb_txn,
20198-
op->od->sdp->sb_dp, MC_UNKNOWN,
20199-
OVN_MCAST_UNKNOWN_TUNNEL_KEY);
20200-
}
20201-
sbrec_multicast_group_update_ports_addvalue(sbmc_unknown,
20202-
op->sb);
20203-
}
2020420163
}
2020520164

2020620165
return true;

‎tests/ovn-northd.at‎

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ m4_define([_DUMP_DB_TABLES], [
1212
ovn-sbctl list meter >> $1
1313
ovn-sbctl list meter_band >> $1
1414
ovn-sbctl list port_group >> $1
15+
ovn-sbctl list multicast_group >> $1
1516
ovn-sbctl list bfd >> $1
1617
ovn-sbctl dump-flows > lflows_$1
1718
])
@@ -15710,6 +15711,101 @@ OVN_CLEANUP([hv1], [hv2])
1571015711
AT_CLEANUP
1571115712
])
1571215713

15714+
OVN_FOR_EACH_NORTHD_NO_HV([
15715+
AT_SETUP([Multicast group incremental processing])
15716+
ovn_start
15717+
15718+
dnl Helper: verify that Multicast_Group 'mc_name' on datapath 'dp_name'
15719+
dnl contains exactly the listed ports.
15720+
dnl Usage: check_mc_group_ports MC_NAME DP_NAME PORT1 [PORT2 ...]
15721+
check_mc_group_ports() {
15722+
mc_group_name=$1; shift
15723+
mc_group_dp=$(fetch_column Datapath_Binding _uuid external_ids:name=$1); shift
15724+
mc_group_expected=""
15725+
for port_name; do
15726+
mc_group_expected="$mc_group_expected $(fetch_column Port_Binding _uuid logical_port=$port_name)"
15727+
done
15728+
mc_group_expected=$(echo $mc_group_expected | tr ' ' '\n' | sort | xargs)
15729+
15730+
OVS_WAIT_UNTIL([
15731+
mc_group_found=$(fetch_column Multicast_Group ports name=$mc_group_name datapath=$mc_group_dp)
15732+
test "$mc_group_found" = "$mc_group_expected"
15733+
])
15734+
}
15735+
15736+
check ovn-nbctl --wait=sb ls-add ls0
15737+
check ovn-nbctl --wait=sb lsp-add-localnet-port ls0 ln0 physnet0
15738+
15739+
dnl Baseline: localnet port only.
15740+
check_mc_group_ports _MC_flood ls0 ln0
15741+
check_mc_group_ports _MC_flood_l2 ls0 ln0
15742+
CHECK_NO_CHANGE_AFTER_RECOMPUTE
15743+
15744+
dnl Add an enabled VIF.
15745+
check ovn-nbctl --wait=sb lsp-add ls0 lsp0
15746+
check_mc_group_ports _MC_flood ls0 ln0 lsp0
15747+
check_mc_group_ports _MC_flood_l2 ls0 ln0 lsp0
15748+
CHECK_NO_CHANGE_AFTER_RECOMPUTE
15749+
15750+
dnl Add a disabled VIF; it must NOT appear in multicast groups.
15751+
check ovn-nbctl --wait=sb \
15752+
lsp-add ls0 lsp1 \
15753+
-- set logical_switch_port lsp1 enabled=false
15754+
check_mc_group_ports _MC_flood ls0 ln0 lsp0
15755+
check_mc_group_ports _MC_flood_l2 ls0 ln0 lsp0
15756+
CHECK_NO_CHANGE_AFTER_RECOMPUTE
15757+
15758+
dnl Enable the disabled VIF.
15759+
check ovn-nbctl --wait=sb set logical_switch_port lsp1 enabled=true
15760+
check_mc_group_ports _MC_flood ls0 ln0 lsp0 lsp1
15761+
check_mc_group_ports _MC_flood_l2 ls0 ln0 lsp0 lsp1
15762+
CHECK_NO_CHANGE_AFTER_RECOMPUTE
15763+
15764+
dnl Disable an enabled VIF.
15765+
check ovn-nbctl --wait=sb set logical_switch_port lsp0 enabled=false
15766+
check_mc_group_ports _MC_flood ls0 ln0 lsp1
15767+
check_mc_group_ports _MC_flood_l2 ls0 ln0 lsp1
15768+
CHECK_NO_CHANGE_AFTER_RECOMPUTE
15769+
15770+
dnl Re-enable it.
15771+
check ovn-nbctl --wait=sb set logical_switch_port lsp0 enabled=true
15772+
check_mc_group_ports _MC_flood ls0 ln0 lsp0 lsp1
15773+
check_mc_group_ports _MC_flood_l2 ls0 ln0 lsp0 lsp1
15774+
CHECK_NO_CHANGE_AFTER_RECOMPUTE
15775+
15776+
dnl Delete an enabled VIF.
15777+
check ovn-nbctl --wait=sb lsp-del lsp0
15778+
check_mc_group_ports _MC_flood ls0 ln0 lsp1
15779+
check_mc_group_ports _MC_flood_l2 ls0 ln0 lsp1
15780+
CHECK_NO_CHANGE_AFTER_RECOMPUTE
15781+
15782+
dnl Add a router and connect it to ls0. The router LSP should appear
15783+
dnl in _MC_flood but NOT in _MC_flood_l2 (router ports are excluded
15784+
dnl from _MC_flood_l2).
15785+
check ovn-nbctl --wait=sb \
15786+
lr-add lr0 \
15787+
-- lrp-add lr0 lr0-ls0 00:00:00:00:01:00 10.0.0.254/24 \
15788+
-- lsp-add-router-port ls0 ls0-lr0 lr0-ls0
15789+
check_mc_group_ports _MC_flood ls0 ln0 lsp1 ls0-lr0
15790+
check_mc_group_ports _MC_flood_l2 ls0 ln0 lsp1
15791+
CHECK_NO_CHANGE_AFTER_RECOMPUTE
15792+
15793+
dnl Add a second switch connected to the router, with a localnet port
15794+
dnl and a VIF.
15795+
check ovn-nbctl --wait=sb \
15796+
ls-add ls1 \
15797+
-- lsp-add-localnet-port ls1 ln1 physnet1 \
15798+
-- lrp-add lr0 lr0-ls1 00:00:00:00:02:00 10.0.1.254/24 \
15799+
-- lsp-add-router-port ls1 ls1-lr0 lr0-ls1 \
15800+
-- lsp-add ls1 lsp2
15801+
check_mc_group_ports _MC_flood ls1 ln1 ls1-lr0 lsp2
15802+
check_mc_group_ports _MC_flood_l2 ls1 ln1 lsp2
15803+
CHECK_NO_CHANGE_AFTER_RECOMPUTE
15804+
15805+
OVN_CLEANUP_NORTHD
15806+
AT_CLEANUP
15807+
])
15808+
1571315809
OVN_FOR_EACH_NORTHD_NO_HV([
1571415810
AT_SETUP([Transit router - remote ports])
1571515811
ovn_start

0 commit comments

Comments
 (0)