Message ID | 20210709131043.51831-1-odivlad@gmail.com |
---|---|
State | Accepted |
Headers | show |
Series | [ovs-dev,branch-20.09] ovn-controller: Monitor chassis_private by chassis name. | expand |
Context | Check | Description |
---|---|---|
ovsrobot/apply-robot | fail | apply and check: fail |
ovsrobot/github-robot | success | github build: passed |
Bleep bloop. Greetings Vladislav Odintsov, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. checkpatch: WARNING: Unexpected sign-offs from developers who are not authors or co-authors or committers: Han Zhou <hzhou@ovn.org> Lines checked: 54, Warnings: 1, Errors: 0 build: gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./ovn -I ./include -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/OVSDIR/include -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/OVSDIR/include -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/OVSDIR/lib -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/OVSDIR/lib -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/OVSDIR -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/OVSDIR -I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror -g -O2 -MT controller/pinctrl.o -MD -MP -MF $depbase.Tpo -c -o controller/pinctrl.o controller/pinctrl.c &&\ mv -f $depbase.Tpo $depbase.Po depbase=`echo controller/patch.o | sed 's|[^/]*$|.deps/&|;s|\.o$||'`;\ gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./ovn -I ./include -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/OVSDIR/include -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/OVSDIR/include -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/OVSDIR/lib -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/OVSDIR/lib -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/OVSDIR -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/OVSDIR -I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror -g -O2 -MT controller/patch.o -MD -MP -MF $depbase.Tpo -c -o controller/patch.o controller/patch.c &&\ mv -f $depbase.Tpo $depbase.Po depbase=`echo controller/ovn-controller.o | sed 's|[^/]*$|.deps/&|;s|\.o$||'`;\ gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./ovn -I ./include -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/OVSDIR/include -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/OVSDIR/include -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/OVSDIR/lib -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/OVSDIR/lib -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/OVSDIR -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/OVSDIR -I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror -g -O2 -MT controller/ovn-controller.o -MD -MP -MF $depbase.Tpo -c -o controller/ovn-controller.o controller/ovn-controller.c &&\ mv -f $depbase.Tpo $depbase.Po depbase=`echo controller/physical.o | sed 's|[^/]*$|.deps/&|;s|\.o$||'`;\ gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./ovn -I ./include -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/OVSDIR/include -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/OVSDIR/include -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/OVSDIR/lib -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/OVSDIR/lib -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/OVSDIR -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/OVSDIR -I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror -g -O2 -MT controller/physical.o -MD -MP -MF $depbase.Tpo -c -o controller/physical.o controller/physical.c &&\ mv -f $depbase.Tpo $depbase.Po controller/physical.c: In function ‘put_remote_port_redirect_overlay’: controller/physical.c:387:23: error: ‘struct ofpact_bundle’ has no member named ‘n_slaves’ if (bundle->n_slaves >= BUNDLE_MAX_SLAVES) { ^ controller/physical.c:387:37: error: ‘BUNDLE_MAX_SLAVES’ undeclared (first use in this function) if (bundle->n_slaves >= BUNDLE_MAX_SLAVES) { ^ controller/physical.c:387:37: note: each undeclared identifier is reported only once for each function it appears in controller/physical.c:395:19: error: ‘struct ofpact_bundle’ has no member named ‘n_slaves’ bundle->n_slaves++; ^ make[1]: *** [controller/physical.o] Error 1 make[1]: Leaving directory `/var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace' make: *** [all] Error 2 Please check this out. If you feel there has been an error, please email aconole@redhat.com Thanks, 0-day Robot
Acked-by: Vladislav Odintsov <odivlad@gmail.com> Regards, Vladislav Odintsov > On 9 Jul 2021, at 16:10, Vladislav Odintsov <odivlad@gmail.com> wrote: > > From: Dumitru Ceara <dceara@redhat.com> > > 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@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@redhat.com> > Acked-by: Mark Gray <mark.d.gray@redhat.com> > Signed-off-by: Han Zhou <hzhou@ovn.org> > (cherry picked from commit 1f915da95dc725131b7df094d494af9fda88ea92) > --- > 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 3665c7b4e..b154a8486 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. */ > -- > 2.30.0 >
Signed-off-by: Vladislav Odintsov <odivlad@gmail.com> Regards, Vladislav Odintsov > On 9 Jul 2021, at 16:55, Vladislav Odintsov <odivlad@gmail.com> wrote: > > Acked-by: Vladislav Odintsov <odivlad@gmail.com> > > Regards, > Vladislav Odintsov > >> On 9 Jul 2021, at 16:10, Vladislav Odintsov <odivlad@gmail.com> wrote: >> >> From: Dumitru Ceara <dceara@redhat.com> >> >> 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@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@redhat.com> >> Acked-by: Mark Gray <mark.d.gray@redhat.com> >> Signed-off-by: Han Zhou <hzhou@ovn.org> >> (cherry picked from commit 1f915da95dc725131b7df094d494af9fda88ea92) >> --- >> 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 3665c7b4e..b154a8486 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. */ >> -- >> 2.30.0 >> > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
The backport looks good to me, thanks! On 7/9/21 4:00 PM, Vladislav Odintsov wrote: > Signed-off-by: Vladislav Odintsov <odivlad@gmail.com> > > Regards, > Vladislav Odintsov > >> On 9 Jul 2021, at 16:55, Vladislav Odintsov <odivlad@gmail.com> wrote: >> >> Acked-by: Vladislav Odintsov <odivlad@gmail.com> >> >> Regards, >> Vladislav Odintsov >> >>> On 9 Jul 2021, at 16:10, Vladislav Odintsov <odivlad@gmail.com> wrote: >>> >>> From: Dumitru Ceara <dceara@redhat.com> >>> >>> 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@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@redhat.com> >>> Acked-by: Mark Gray <mark.d.gray@redhat.com> >>> Signed-off-by: Han Zhou <hzhou@ovn.org> >>> (cherry picked from commit 1f915da95dc725131b7df094d494af9fda88ea92) >>> --- >>> 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 3665c7b4e..b154a8486 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. */ >>> -- >>> 2.30.0 >>> >> >> _______________________________________________ >> dev mailing list >> dev@openvswitch.org >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev > >
On Fri, Jul 9, 2021 at 11:54 AM Dumitru Ceara <dceara@redhat.com> wrote: > > The backport looks good to me, thanks! Thanks. I applied this patch to branch-20.09. Numan > > On 7/9/21 4:00 PM, Vladislav Odintsov wrote: > > Signed-off-by: Vladislav Odintsov <odivlad@gmail.com> > > > > Regards, > > Vladislav Odintsov > > > >> On 9 Jul 2021, at 16:55, Vladislav Odintsov <odivlad@gmail.com> wrote: > >> > >> Acked-by: Vladislav Odintsov <odivlad@gmail.com> > >> > >> Regards, > >> Vladislav Odintsov > >> > >>> On 9 Jul 2021, at 16:10, Vladislav Odintsov <odivlad@gmail.com> wrote: > >>> > >>> From: Dumitru Ceara <dceara@redhat.com> > >>> > >>> 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@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@redhat.com> > >>> Acked-by: Mark Gray <mark.d.gray@redhat.com> > >>> Signed-off-by: Han Zhou <hzhou@ovn.org> > >>> (cherry picked from commit 1f915da95dc725131b7df094d494af9fda88ea92) > >>> --- > >>> 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 3665c7b4e..b154a8486 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. */ > >>> -- > >>> 2.30.0 > >>> > >> > >> _______________________________________________ > >> dev mailing list > >> dev@openvswitch.org > >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > > > > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index 3665c7b4e..b154a8486 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. */