Message ID | 1564095792-32282-1-git-send-email-hzhou8@ebay.com |
---|---|
State | Accepted |
Headers | show |
Series | [ovs-dev] ovn-northd: Fix ARP respond flows flapping. | expand |
On Thu, Jul 25, 2019 at 4:03 PM Han Zhou <zhouhan@gmail.com> wrote: > > From: Han Zhou <hzhou8@ebay.com> > > From ovn-controller debug log it is seen that when creating a lsp > in NB, a lflow for ARP respond is added and then deleted in SB > by northd (the flow will be added again when the port is bound > to a chassis). This causes unnecessary handling from ovn-controller. > > The root cause is lsp_is_up() returns true when the column is not > set, when the lsp is just created. So northd adds the ARP respond > flow in SB lflow table. At the same time it will create port-binding > in SB without chassis binding. Then in the next iteration northd > will process that port-binding change and notice that there is no > chassis binding for this lsp, so it will set the "up" to false, > which causes northd to delete the ARP respond flow. > > The fix is to make sure when "up" is not set, it is regarded as > false by default. > > Signed-off-by: Han Zhou <hzhou8@ebay.com> > --- > ovn/northd/ovn-northd.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c > index eb6c47c..212aa88 100644 > --- a/ovn/northd/ovn-northd.c > +++ b/ovn/northd/ovn-northd.c > @@ -3495,7 +3495,7 @@ lsp_is_enabled(const struct nbrec_logical_switch_port *lsp) > static bool > lsp_is_up(const struct nbrec_logical_switch_port *lsp) > { > - return !lsp->up || *lsp->up; > + return lsp->up && *lsp->up; > } > > static bool > -- > 2.1.0 > Ben/Justin, this has been merged to OVN repo. Could you please backport to 2.12? Thanks.
> On Aug 1, 2019, at 11:08 AM, Han Zhou <zhouhan@gmail.com> wrote: > > > diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c > > index eb6c47c..212aa88 100644 > > --- a/ovn/northd/ovn-northd.c > > +++ b/ovn/northd/ovn-northd.c > > @@ -3495,7 +3495,7 @@ lsp_is_enabled(const struct nbrec_logical_switch_port *lsp) > > static bool > > lsp_is_up(const struct nbrec_logical_switch_port *lsp) > > { > > - return !lsp->up || *lsp->up; > > + return lsp->up && *lsp->up; > > } > > > > static bool > > -- > > 2.1.0 > > > > Ben/Justin, this has been merged to OVN repo. Could you please backport to 2.12? Thanks. Thanks, Han. I pushed this to branch-2.12. Han and Numan, I sent a follow up patch, which I think clarifies the code a bit (by using the "n_" member to check whether the column is set) and improves the documentation: https://patchwork.ozlabs.org/patch/1144879/ Can you take a look? Thanks, --Justin
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c index eb6c47c..212aa88 100644 --- a/ovn/northd/ovn-northd.c +++ b/ovn/northd/ovn-northd.c @@ -3495,7 +3495,7 @@ lsp_is_enabled(const struct nbrec_logical_switch_port *lsp) static bool lsp_is_up(const struct nbrec_logical_switch_port *lsp) { - return !lsp->up || *lsp->up; + return lsp->up && *lsp->up; } static bool