diff mbox series

[ovs-dev,ovn] ovn-northd: Fix ARP respond flows flapping.

Message ID 1564442655-93062-1-git-send-email-hzhou8@ebay.com
State Accepted
Headers show
Series [ovs-dev,ovn] ovn-northd: Fix ARP respond flows flapping. | expand

Commit Message

Han Zhou July 29, 2019, 11:24 p.m. UTC 
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>
---
 northd/ovn-northd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Numan Siddique Aug. 1, 2019, 6:04 p.m. UTC | #1 
On Tue, Jul 30, 2019 at 4:57 AM 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>
>

Thanks Han. I pushed this patch to master.

Numan


> ---
>  northd/ovn-northd.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index bed2993..020cde4 100644
> --- a/northd/ovn-northd.c
> +++ b/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
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
diff mbox series

Patch

diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index bed2993..020cde4 100644
--- a/northd/ovn-northd.c
+++ b/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