diff mbox series

[ovs-dev,1/2] binding: Don't reset expected seqno for interfaces already being installed.

Message ID 20210423141738.15080.66344.stgit@dceara.remote.csb
State Accepted
Headers show
Series Make Port.UP and ovn-installed notifications more reliable. | expand

Commit Message

Dumitru Ceara April 23, 2021, 2:17 p.m. UTC
If an interface is already in 'binding_iface_seqno_map' then that means
that the openflow modifications needed for its functionality have
already been sent to ovs-vswitchd and we're waiting for an openflow
barrier reply.  In such cases, don't reset the expected seqno because
this would delay the notification unnecessarily.

This scenario can be hit at scale, if Southbound DB sends a port
related update before processing the transaction in which ovn-controller
claimed the logical port.

Reported-at: https://bugzilla.redhat.com/1946420
Fixes: 4d3cb42b076b ("binding: Set Logical_Switch_Port.up when all OVS flows are installed.")
Signed-off-by: Dumitru Ceara <dceara@redhat.com>
---
 controller/binding.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Numan Siddique April 28, 2021, 10:39 a.m. UTC | #1
On Fri, Apr 23, 2021 at 10:18 AM Dumitru Ceara <dceara@redhat.com> wrote:
>
> If an interface is already in 'binding_iface_seqno_map' then that means
> that the openflow modifications needed for its functionality have
> already been sent to ovs-vswitchd and we're waiting for an openflow
> barrier reply.  In such cases, don't reset the expected seqno because
> this would delay the notification unnecessarily.
>
> This scenario can be hit at scale, if Southbound DB sends a port
> related update before processing the transaction in which ovn-controller
> claimed the logical port.
>
> Reported-at: https://bugzilla.redhat.com/1946420
> Fixes: 4d3cb42b076b ("binding: Set Logical_Switch_Port.up when all OVS flows are installed.")
> Signed-off-by: Dumitru Ceara <dceara@redhat.com>

Thanks for fixing this issue.

I applied this patch to master and branch-21.03

Numan

> ---
>  controller/binding.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/controller/binding.c b/controller/binding.c
> index 514f5f33f..451f00e34 100644
> --- a/controller/binding.c
> +++ b/controller/binding.c
> @@ -2602,7 +2602,8 @@ binding_seqno_run(struct local_binding_data *lbinding_data)
>           * Port_Binding 'up' field and the OVS Interface 'external-id'.
>           */
>          struct binding_lport *b_lport = local_binding_get_primary_lport(lb);
> -        if (lb && b_lport && lb->iface) {
> +        if (lb && b_lport && lb->iface
> +                && !simap_contains(&binding_iface_seqno_map, lb->name)) {
>              new_ifaces = true;
>
>              if (smap_get(&lb->iface->external_ids, OVN_INSTALLED_EXT_ID)) {
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
diff mbox series

Patch

diff --git a/controller/binding.c b/controller/binding.c
index 514f5f33f..451f00e34 100644
--- a/controller/binding.c
+++ b/controller/binding.c
@@ -2602,7 +2602,8 @@  binding_seqno_run(struct local_binding_data *lbinding_data)
          * Port_Binding 'up' field and the OVS Interface 'external-id'.
          */
         struct binding_lport *b_lport = local_binding_get_primary_lport(lb);
-        if (lb && b_lport && lb->iface) {
+        if (lb && b_lport && lb->iface
+                && !simap_contains(&binding_iface_seqno_map, lb->name)) {
             new_ifaces = true;
 
             if (smap_get(&lb->iface->external_ids, OVN_INSTALLED_EXT_ID)) {