[ovs-dev] [PATCH ovn v2 2/3] northd.c: Lock to protect against possible od->group corruption.
Han Zhou
hzhou at ovn.org
Tue Oct 5 07:36:00 UTC 2021
On Mon, Oct 4, 2021 at 8:08 PM Numan Siddique <numans at ovn.org> wrote:
>
> On Mon, Oct 4, 2021 at 5:29 AM Anton Ivanov
> <anton.ivanov at cambridgegreys.com> wrote:
> >
> >
> > On 03/10/2021 23:45, Han Zhou wrote:
> > > When parallel build is used, od->group can be updated by threads
outside
> > > of the function do_ovn_lflow_add_pd (for lb related flow building). So
> > > use the function ovn_dp_group_add_with_reference() to update it in
> > > function do_ovn_lflow_add() when it is not a newly created flow.
> > >
> > > Signed-off-by: Han Zhou <hzhou at ovn.org>
> > > ---
> > > northd/northd.c | 42 +++++++++++++++++++++---------------------
> > > 1 file changed, 21 insertions(+), 21 deletions(-)
> > >
> > > diff --git a/northd/northd.c b/northd/northd.c
> > > index 027c5b170..afd812700 100644
> > > --- a/northd/northd.c
> > > +++ b/northd/northd.c
> > > @@ -4299,6 +4299,26 @@ ovn_lflow_init(struct ovn_lflow *lflow, struct
ovn_datapath *od,
> > > }
> > > }
> > >
> > > +static bool
> > > +ovn_dp_group_add_with_reference(struct ovn_lflow *lflow_ref,
> > > + struct ovn_datapath *od)
> > > + OVS_NO_THREAD_SAFETY_ANALYSIS
> > > +{
> > > + if (!use_logical_dp_groups || !lflow_ref) {
> > > + return false;
> > > + }
> > > +
> > > + if (use_parallel_build) {
> > > + ovs_mutex_lock(&lflow_ref->odg_lock);
> > > + hmapx_add(&lflow_ref->od_group, od);
> > > + ovs_mutex_unlock(&lflow_ref->odg_lock);
> > > + } else {
> > > + hmapx_add(&lflow_ref->od_group, od);
> > > + }
> > > +
> > > + return true;
> > > +}
> > > +
> > > /* Adds a row with the specified contents to the Logical_Flow table.
> > > * Version to use with dp_groups + parallel - when locking is
required.
> > > *
> > > @@ -4351,7 +4371,7 @@ do_ovn_lflow_add(struct hmap *lflow_map, struct
ovn_datapath *od,
> > > old_lflow = ovn_lflow_find(lflow_map, NULL, stage,
priority, match,
> > > actions, ctrl_meter, hash);
> > > if (old_lflow) {
> > > - hmapx_add(&old_lflow->od_group, od);
> > > + ovn_dp_group_add_with_reference(old_lflow, od);
> > > return old_lflow;
> > > }
> > > }
> > > @@ -4466,26 +4486,6 @@ ovn_lflow_add_at(struct hmap *lflow_map,
struct ovn_datapath *od,
> > > io_port, ctrl_meter, stage_hint,
where, hash);
> > > }
> > >
> > > -static bool
> > > -ovn_dp_group_add_with_reference(struct ovn_lflow *lflow_ref,
> > > - struct ovn_datapath *od)
> > > - OVS_NO_THREAD_SAFETY_ANALYSIS
> > > -{
> > > - if (!use_logical_dp_groups || !lflow_ref) {
> > > - return false;
> > > - }
> > > -
> > > - if (use_parallel_build) {
> > > - ovs_mutex_lock(&lflow_ref->odg_lock);
> > > - hmapx_add(&lflow_ref->od_group, od);
> > > - ovs_mutex_unlock(&lflow_ref->odg_lock);
> > > - } else {
> > > - hmapx_add(&lflow_ref->od_group, od);
> > > - }
> > > -
> > > - return true;
> > > -}
> > > -
> > > /* Adds a row with the specified contents to the Logical_Flow
table. */
> > > #define ovn_lflow_add_with_hint__(LFLOW_MAP, OD, STAGE, PRIORITY,
MATCH, \
> > > ACTIONS, IN_OUT_PORT, CTRL_METER,
\
> >
> >
> > +1
> >
>
> Acked-by: Numan Siddique <numans at ovn.org>
Thanks Anton and Numan for the review. I applied the patch 1 and 2 in this
series to main branch and branch-21.09.
>
> Numan
>
> > --
> > Anton R. Ivanov
> > Cambridgegreys Limited. Registered in England. Company Number 10273661
> > https://www.cambridgegreys.com/
> >
> > _______________________________________________
> > dev mailing list
> > dev at openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >
More information about the dev
mailing list