On Mon, Dec 1, 2025 at 5:40 PM Ales Musil <[email protected]> wrote:
> > > On Mon, Dec 1, 2025 at 5:37 PM Mark Michelson <[email protected]> wrote: > >> On Mon, Dec 1, 2025 at 10:50 AM Ales Musil via dev >> <[email protected]> wrote: >> > >> > The hmap could be NULL under one circumstance, node depending >> > on the northd data would run before the northd node right >> > after the start. Make sure we won't crash in that case. >> >> Can you explain this in a bit more detail? How does a node that >> depends on northd data run before the northd node? >> > > It also depends on SB nodes. So it will get triggered from the SB update > first. > Looking through the issue once more you are right, that doesn't make sense. It's actually slightly different. We only pass ls_datapaths, so if the datapath is lr our lr_datapaths are NULL so we would crash there. I can change the commit message during merge to reflect that better: northd: Add back check for datapath hmap. In some cases northd is passing only ls_datapaths/lr_datapaths into the ovn_datapath_from_sbrec(), the other hmap being NULL. This is problematic when the function runs into the second datapath type "unexpectedly" during processing, which would lead to NULL ptr deref and crash: openvswitch/hmap.h:360:40: runtime error: member access within null pointer of type 'const struct hmap' 0x0000004f5b74 in hmap_first_with_hash openvswitch/hmap.h:360:40 0x0000004f1d3a in ovn_datapath_find_ northd/northd.c:601:5 0x0000004f21ad in ovn_datapath_from_sbrec northd/northd.c:654:31 0x00000060aa50 in build_mcast_groups northd/en-multicast.c:280:14 0x000000608c4b in en_multicast_igmp_run northd/en-multicast.c:135:5 0x0000006f11ed in engine_recompute lib/inc-proc-eng.c:445:33 0x0000006ef315 in engine_run_node lib/inc-proc-eng.c:519:9 0x0000006ef315 in engine_run lib/inc-proc-eng.c:576:9 0x0000006635fc in inc_proc_northd_run northd/inc-proc-northd.c:608:5 0x0000005dc196 in main northd/ovn-northd.c:1109:36 Make sure we check for the hmap being NULL before attempting to find the ovn_datapath struct. WDYT? > > >> >> > >> > Fixes: 6919992d8781 ("Datapath_Binding: Separate type column and sync >> NB.UUID to SB.") >> > Reported-at: https://issues.redhat.com/browse/FDP-2757 >> > Signed-off-by: Ales Musil <[email protected]> >> > --- >> > northd/northd.c | 4 ++++ >> > tests/ovn-northd.at | 29 +++++++++++++++++++++++++++++ >> > 2 files changed, 33 insertions(+) >> > >> > diff --git a/northd/northd.c b/northd/northd.c >> > index ec219a0c7..3bd1f6849 100644 >> > --- a/northd/northd.c >> > +++ b/northd/northd.c >> > @@ -647,6 +647,10 @@ ovn_datapath_from_sbrec(const struct hmap >> *ls_datapaths, >> > return NULL; >> > } >> > >> > + if (!dps) { >> > + return NULL; >> > + } >> > + >> > struct ovn_datapath *od = ovn_datapath_find_(dps, &key); >> > if (od && (od->sdp->sb_dp == sb)) { >> > return od; >> > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at >> > index 380c9ff80..0bc2808ac 100644 >> > --- a/tests/ovn-northd.at >> > +++ b/tests/ovn-northd.at >> > @@ -18885,3 +18885,32 @@ AT_CHECK([ovn-sbctl lflow-list ls1 | grep >> ls_in_apply_port_sec | ovn_strip_lflow >> > >> > AT_CLEANUP >> > ]) >> > + >> > +OVN_FOR_EACH_NORTHD_NO_HV([ >> > +AT_SETUP([IGMP northd crash]) >> > +ovn_start >> > + >> > +check ovn-nbctl lr-add lr >> > +check ovn-sbctl chassis-add hv1 geneve 127.0.0.1 >> > + >> > +# Kill northd and start it paused this time. >> > +as northd >> > +OVS_APP_EXIT_AND_WAIT([ovn-northd]) >> > +ovn_start_northd --paused >> > + >> > +# Create IGMP Group for lr datapath. >> > +check_uuid ovn-sbctl create IGMP_Group address=mrouters \ >> > + datapath=$(fetch_column Datapath_Binding _uuid >> external_ids:name=lr) \ >> > + chassis=$(fetch_column Chassis _uuid name=hv1) \ >> > + chassis_name=hv1 >> > +wait_row_count Igmp_Group 1 address=mrouters >> > + >> > +# Unpause northd and check if we didn't crash. >> > +as northd ovn-appctl -t ovn-northd resume >> > +check ovn-nbctl --wait=sb sync >> > + >> > +wait_row_count Igmp_Group 0 address=mrouters >> > + >> > +OVN_CLEANUP_NORTHD >> > +AT_CLEANUP >> > +]) >> > -- >> > 2.51.1 >> > >> > _______________________________________________ >> > dev mailing list >> > [email protected] >> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> > >> >> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
