[ovs-dev] [PATCH ovn] ovn-controller: Monitor chassis_private by chassis name.

Han Zhou hzhou at ovn.org
Tue Oct 20 19:39:55 UTC 2020


On Tue, Oct 20, 2020 at 3:58 AM Mark Gray <mark.d.gray at redhat.com> wrote:
>
> On 20/10/2020 10:18, Dumitru Ceara wrote:
> > Remove the use of sbrec_chassis_is_new() for uncommitted records.  This
> > is not the way IDL *_is_new() functions are supposed to be used.
> >
> > Note: With this change if the system-id changes there will be a
> > transient error in ovn-controller due to ovn-controller trying to insert
> > a new chassis_private record.  This is due to the fact that the view of
> > the chassis_private table changes and only chassis_private records
> > matching the new chassis name are sent to ovn-controller.  This gets
> > corrected though in the next iteration of the ovn-controller processing
> > loop.
> >
> > Suggested-by: Han Zhou <hzhou at ovn.org>
> > Reported-at:
https://mail.openvswitch.org/pipermail/ovs-dev/2020-October/376339.html
> > Fixes: dce1af31b550 ("chassis: Fix chassis_private record updates when
the system-id changes.")
> > Signed-off-by: Dumitru Ceara <dceara at redhat.com>
> > ---
> >  controller/ovn-controller.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> > index 066e31f..a06cae3 100644
> > --- a/controller/ovn-controller.c
> > +++ b/controller/ovn-controller.c
> > @@ -181,7 +181,7 @@ update_sb_monitors(struct ovsdb_idl *ovnsb_idl,
> >       * chassis */
> >      sbrec_port_binding_add_clause_type(&pb, OVSDB_F_EQ,
"chassisredirect");
> >      sbrec_port_binding_add_clause_type(&pb, OVSDB_F_EQ, "external");
> > -    if (chassis && !sbrec_chassis_is_new(chassis)) {
> > +    if (chassis) {
> >          /* This should be mostly redundant with the other clauses for
port
> >           * bindings, but it allows us to catch any ports that are
assigned to
> >           * us but should not be.  That way, we can clear their chassis
> > @@ -205,8 +205,8 @@ update_sb_monitors(struct ovsdb_idl *ovnsb_idl,
> >                                              &chassis->header_.uuid);
> >
> >          /* Monitors Chassis_Private record for current chassis only. */
> > -        sbrec_chassis_private_add_clause_chassis(&chprv, OVSDB_F_EQ,
> > -
&chassis->header_.uuid);
> > +        sbrec_chassis_private_add_clause_name(&chprv, OVSDB_F_EQ,
> > +                                              chassis->name);
> >      } else {
> >          /* During initialization, we monitor all records in
Chassis_Private so
> >           * that we don't try to recreate existing ones. */
> >
>
> I tested this against my v1 patch [1] and the OVN tests passed as
expected.
>
> Acked-by: Mark Gray <mark.d.gray at redhat.com>
>
> [1] v1:
>
https://patchwork.ozlabs.org/project/openvswitch/patch/20200929183433.925570-1-mark.d.gray@redhat.com/
>

Thanks Dumitru and Mark. I applied this to master.

Han


More information about the dev mailing list