[ovs-dev] [PATCH ovn] ovn-controller: Fix use of dangling pointers in I-P runtime_data.

Han Zhou hzhou at ovn.org
Thu Oct 31 05:35:00 UTC 2019


On Wed, Oct 30, 2019 at 10:15 PM Han Zhou <hzhou at ovn.org> wrote:
>
> >
> >
> > +                    /* We need to make sure that at least the runtime
data
> > +                     * (e.g., local datapaths, ct zones) are fresh
before
> > +                     * calling ofctrl_put and pinctrl_run to avoid
using
> > +                     * potentially freed references to database
records.
> > +                     */
> > +                    if (engine_node_data_valid(&en_runtime_data,
> > +                                               engine_run_id)) {
> > +                        ofctrl_put(&ed_flow_output.flow_table,
> > +                                   &ed_runtime_data.pending_ct_zones,
> > +
sbrec_meter_table_get(ovnsb_idl_loop.idl),
> > +
get_nb_cfg(sbrec_sb_global_table_get(
> > +
ovnsb_idl_loop.idl)),
> > +
engine_node_data_changed(&en_flow_output,
> > +
 engine_run_id));
>
>
> I think ofctrl_put() needs to be invoked every iteration, because it
takes care of reinstalling flows to OVS when ovs-vswitchd is restarted. See
"need_reinstall_flows" in ofctrl.c.
> For now, I think it is ok to leave this function out of the if
(engine_node_data_valid()) check, because pending_ct_zones is always valid.
For improvement, we should move pending_ct_zones to a separate engine node
so that it always passes the check.
>

I thought about this again and it seems ok. If ovs-vswitchd happened to be
restarted before this iteration and now we skipped ofctrl_put() because
runtime_data is not valid, then main loop should be waked up immediately
later and then ofctrl_put() will be executed finally and flows will be
reinstalled.


More information about the dev mailing list