On Fri, Jan 26, 2024 at 4:30 AM Dumitru Ceara <dce...@redhat.com> wrote: > > On 1/25/24 22:01, Numan Siddique wrote: > > On Thu, Jan 25, 2024, 3:29 PM Dumitru Ceara <dce...@redhat.com> wrote: > > > >> On 1/25/24 17:46, Numan Siddique wrote: > >>> On Tue, Jan 16, 2024 at 6:15 AM Dumitru Ceara <dce...@redhat.com> wrote: > >>>> > >>>> On 1/11/24 16:28, num...@ovn.org wrote: > >>>>> From: Numan Siddique <num...@ovn.org> > >>>>> > >>>>> It also moves the logical router port IPv6 prefix delegation > >>>>> updates to "sync-from-sb" engine node. > >>>>> > >>>>> With these changes, northd engine node doesn't need to do much > >>>>> for SB Port binding changes other than updating 'op->sb'. And > >>>>> there is no need to fall back to recompute. Prior to this patch > >>>>> northd_handle_sb_port_binding_changes() was returning false for > >>>>> all SB port binding updates except for the VIF PBs. This patch > >>>>> now handles updates to all the SB port binding by setting 'op->sb'. > >>>>> > >>>>> Signed-off-by: Numan Siddique <num...@ovn.org> > >>>>> --- > >>>> > >>>> Hi Numan, > >>>> > >>>> With this patch applied the "633: ovn-northd.at:10298 SB Port binding > >>>> incremental processing -- parallelization=yes" test occasionally fails, > >>>> e.g.: > >>>> > >>>> https://github.com/dceara/ovn/actions/runs/7539663594/job/20524238508 > >>>> > >>>> I saw it locally too when running the test in a loop: > >>>> > >>>> It fails with: > >>>> ovn-nbctl lrp-set-gateway-chassis lrp hv1 > >>>> ./ovn-macros.at:449: "$@" > >>>> northd recompute count - 2 > >>>> ./ovn-northd.at:10238: test x$northd_recomp = x$1 > >>>> ./ovn-northd.at:10238: exit code was 1, expected 0 > >>>> > >>>> Here: > >>>> > >> https://github.com/dceara/ovn/blob/e1c76b6d347570618591fe6587f6af83445223ec/tests/ovn-northd.at#L10327 > >>>> > >>>> For some reason in some cases only 2 recomputes get triggered. > >>> > >>> Thanks Dumitru and Han for the reviews. > >>> > >>> @Dumitru Ceara I've addressed all the review comments below. > >>> > >>> I think the test case is failing because of the missing "--wait=sb" in > >>> the ovn-nbctl commands. > >>> > >> > >> I don't think it's only that. It still fails even with your updated > >> test changes, in the same way: > >> > >> as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > >> ./ovn-macros.at:449: "$@" > >> ovn-nbctl lrp-set-gateway-chassis lrp hv1 > >> ./ovn-macros.at:449: "$@" > >> northd recompute count - 2 > >> ./ovn-northd.at:10238: test x$northd_recomp = x$1 > >> ./ovn-northd.at:10238: exit code was 1, expected 0 > >> > >>> Will the below changes in ovn-northd.at look good to you ? If so, > >>> and if you can provide your ack I can apply this patch. > >>> Otherwise I'll include it in the v6. > >>> > >> > >> I would prefer if we don't apply it until we figure out this test, the > >> CI has been relatively stable the last few weeks, I'd rather not > >> destabilize it if possible. > >> > > > > Sure. Definitely. > > > > Its passing for me locally when run in a loop. > > > > Let me dig into it further. > > > > > > Sorry, Numan, I had made a mistake when applying your incremental patch. > With the right changes applied it passes now for me too. > > Acked-by: Dumitru Ceara <dce...@redhat.com>
Thanks. I applied this patch to the main. Patches 1,2 and 3 are merged. I'll submit the rest of the patches in v6 and will apply those as one series after all the reviews and acks. Thanks Numan > > Thanks, > Dumitru > > _______________________________________________ > dev mailing list > d...@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
