| Message ID | 20260817022030.1345589-1-travis.wu@bigstack.co |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | [ovs-dev] controller: Guard NULL sb_global in pflow debug output. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/apply-robot | success | apply and check: success |
On 8/17/26 4:20 AM, Travis Wu via dev wrote: > pflow_output_get_debug() null-checks debug, ovs_cfg and br_int but reads > sb_global->options unconditionally via smap_get_uint(). When > ovn-controller connects to a Southbound DB that has no SB_Global row yet > -- e.g. an HA bootstrap or failover where a cluster manager starts the SB > and ovn-northd creates SB_Global moments later -- the engine recompute > dereferences a NULL sb_global and crashes: > > smap_find <- smap_get_uint <- pflow_output_get_debug > <- en_pflow_output_run <- engine_recompute <- engine_run > > The controller then crash-loops into its systemd start limit until northd > populates SB_Global. Guard the dereference, matching the existing NULL > checks in the same function. > > Signed-off-by: Travis Wu <travis.wu@bigstack.co> > --- Hi Travis, Thanks for the patch! I'm not sure I understand how this happen (at least on the most recent OVN main branch). We don't call engine_run() if there's no SB_Global first record: const struct sbrec_sb_global *sbg = sbrec_sb_global_first(ovnsb_idl_loop.idl); if (chassis && sbg && ovs_feature_set_discovered()) { [...] bool recompute_allowed = (ovnsb_idl_txn && !ofctrl_has_backlog()); engine_run(recompute_allowed); So we should never reach pflow_output_get_debug() without a sb_global record. Are you hitting this on a recent version of OVN? That is, something that includes: https://github.com/ovn-org/ovn/commit/fa72572 Regards, Dumitru > controller/ovn-controller.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c > index 6551a4e..9da2365 100644 > --- a/controller/ovn-controller.c > +++ b/controller/ovn-controller.c > @@ -4240,6 +4240,10 @@ pflow_output_get_debug(struct engine_node *node, struct physical_debug *debug) > const struct sbrec_sb_global *sb_global = > sbrec_sb_global_table_first(sb_global_table); > > + if (!sb_global) { > + return; > + } > + > if (!debug) { > return; > }
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index 6551a4e..9da2365 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -4240,6 +4240,10 @@ pflow_output_get_debug(struct engine_node *node, struct physical_debug *debug) const struct sbrec_sb_global *sb_global = sbrec_sb_global_table_first(sb_global_table); + if (!sb_global) { + return; + } + if (!debug) { return; }
pflow_output_get_debug() null-checks debug, ovs_cfg and br_int but reads sb_global->options unconditionally via smap_get_uint(). When ovn-controller connects to a Southbound DB that has no SB_Global row yet -- e.g. an HA bootstrap or failover where a cluster manager starts the SB and ovn-northd creates SB_Global moments later -- the engine recompute dereferences a NULL sb_global and crashes: smap_find <- smap_get_uint <- pflow_output_get_debug <- en_pflow_output_run <- engine_recompute <- engine_run The controller then crash-loops into its systemd start limit until northd populates SB_Global. Guard the dereference, matching the existing NULL checks in the same function. Signed-off-by: Travis Wu <travis.wu@bigstack.co> --- controller/ovn-controller.c | 4 ++++ 1 file changed, 4 insertions(+)