diff mbox series

[ovs-dev] controller: Fall back to full recompute of pflow_output for vtep lport changes.

Message ID 20210907171507.1617879-1-numans@ovn.org
State Accepted
Headers show
Series [ovs-dev] controller: Fall back to full recompute of pflow_output for vtep lport changes. | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed
ovsrobot/github-robot-_ovn-kubernetes fail github build: failed

Commit Message

Numan Siddique Sept. 7, 2021, 5:15 p.m. UTC 
From: Numan Siddique <numans@ovn.org>

When a vtep logical port changes, necessary flows are not added as
expected.  This is because the function physical_handle_flows_for_lport()
in physical.c does not add flows required for vtep logical ports.
These flows are added by physical_run(). So fall back to full recompute
of pflow_output engine node when vtep lports change.

Reported-by: Odintsov Vladislav <VlOdintsov@croc.ru>
Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2021-September/387435.html
Signed-off-by: Numan Siddique <numans@ovn.org>
---
 controller/ovn-controller.c | 11 ++++++++---
 controller/physical.c       |  9 ++++++++-
 controller/physical.h       |  2 +-
 3 files changed, 17 insertions(+), 5 deletions(-)

Comments

Mark Gray Sept. 8, 2021, 9:56 a.m. UTC | #1 
On 07/09/2021 18:15, numans@ovn.org wrote:
> From: Numan Siddique <numans@ovn.org>
> 
> When a vtep logical port changes, necessary flows are not added as
> expected.  This is because the function physical_handle_flows_for_lport()
> in physical.c does not add flows required for vtep logical ports.
> These flows are added by physical_run(). So fall back to full recompute
> of pflow_output engine node when vtep lports change.
> 
> Reported-by: Odintsov Vladislav <VlOdintsov@croc.ru>
> Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2021-September/387435.html
> Signed-off-by: Numan Siddique <numans@ovn.org>
> ---
>  controller/ovn-controller.c | 11 ++++++++---
>  controller/physical.c       |  9 ++++++++-
>  controller/physical.h       |  2 +-
>  3 files changed, 17 insertions(+), 5 deletions(-)
> 
> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> index 0031a1035..d98ebbbfa 100644
> --- a/controller/ovn-controller.c
> +++ b/controller/ovn-controller.c
> @@ -2867,7 +2867,10 @@ pflow_output_sb_port_binding_handler(struct engine_node *node,
>      const struct sbrec_port_binding *pb;
>      SBREC_PORT_BINDING_TABLE_FOR_EACH_TRACKED (pb, p_ctx.port_binding_table) {
>          bool removed = sbrec_port_binding_is_deleted(pb);
> -        physical_handle_flows_for_lport(pb, removed, &p_ctx, &pfo->flow_table);
> +        if (!physical_handle_flows_for_lport(pb, removed, &p_ctx,
> +                                             &pfo->flow_table)) {
> +            return false;
> +        }
>      }
>  
>      engine_set_node_state(node, EN_UPDATED);
> @@ -2930,8 +2933,10 @@ pflow_output_runtime_data_handler(struct engine_node *node, void *data)
>              struct tracked_lport *lport = shash_node->data;
>              bool removed =
>                  lport->tracked_type == TRACKED_RESOURCE_REMOVED ? true: false;
> -            physical_handle_flows_for_lport(lport->pb, removed, &p_ctx,
> -                                            &pfo->flow_table);
> +            if (!physical_handle_flows_for_lport(lport->pb, removed, &p_ctx,
> +                                                 &pfo->flow_table)) {
> +                return false;
> +            }
>          }
>      }
>  
> diff --git a/controller/physical.c b/controller/physical.c
> index 6f2c1cea0..ffb9f9952 100644
> --- a/controller/physical.c
> +++ b/controller/physical.c
> @@ -1509,11 +1509,16 @@ consider_mc_group(enum mf_field_id mff_ovn_geneve,
>      sset_destroy(&remote_chassis);
>  }
>  
> -void
> +bool
>  physical_handle_flows_for_lport(const struct sbrec_port_binding *pb,
>                                  bool removed, struct physical_ctx *p_ctx,
>                                  struct ovn_desired_flow_table *flow_table)
>  {
> +    if (!strcmp(pb->type, "vtep")) {
> +        /* Cannot handle changes to vtep lports (yet). */
> +        return false;
> +    }
> +
>      ofctrl_remove_flows(flow_table, &pb->header_.uuid);
>  
>      if (!strcmp(pb->type, "external")) {
> @@ -1553,6 +1558,8 @@ physical_handle_flows_for_lport(const struct sbrec_port_binding *pb,
>                                p_ctx->chassis, flow_table, &ofpacts);
>          ofpbuf_uninit(&ofpacts);
>      }
> +
> +    return true;
>  }
>  
>  void
> diff --git a/controller/physical.h b/controller/physical.h
> index c4540ad7f..ee4b1ae1f 100644
> --- a/controller/physical.h
> +++ b/controller/physical.h
> @@ -65,7 +65,7 @@ void physical_run(struct physical_ctx *,
>                    struct ovn_desired_flow_table *);
>  void physical_handle_mc_group_changes(struct physical_ctx *,
>                                        struct ovn_desired_flow_table *);
> -void physical_handle_flows_for_lport(const struct sbrec_port_binding *,
> +bool physical_handle_flows_for_lport(const struct sbrec_port_binding *,
>                                       bool removed,
>                                       struct physical_ctx *,
>                                       struct ovn_desired_flow_table *);
> 

The change looks ok to me. I am not really sure how to test it though?
Any suggestions?
Numan Siddique Sept. 8, 2021, 5:14 p.m. UTC | #2 
On Wed, Sep 8, 2021 at 5:59 AM Mark Gray <mark.d.gray@redhat.com> wrote:
>
> On 07/09/2021 18:15, numans@ovn.org wrote:
> > From: Numan Siddique <numans@ovn.org>
> >
> > When a vtep logical port changes, necessary flows are not added as
> > expected.  This is because the function physical_handle_flows_for_lport()
> > in physical.c does not add flows required for vtep logical ports.
> > These flows are added by physical_run(). So fall back to full recompute
> > of pflow_output engine node when vtep lports change.
> >
> > Reported-by: Odintsov Vladislav <VlOdintsov@croc.ru>
> > Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2021-September/387435.html
> > Signed-off-by: Numan Siddique <numans@ovn.org>
> > ---
> >  controller/ovn-controller.c | 11 ++++++++---
> >  controller/physical.c       |  9 ++++++++-
> >  controller/physical.h       |  2 +-
> >  3 files changed, 17 insertions(+), 5 deletions(-)
> >
> > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> > index 0031a1035..d98ebbbfa 100644
> > --- a/controller/ovn-controller.c
> > +++ b/controller/ovn-controller.c
> > @@ -2867,7 +2867,10 @@ pflow_output_sb_port_binding_handler(struct engine_node *node,
> >      const struct sbrec_port_binding *pb;
> >      SBREC_PORT_BINDING_TABLE_FOR_EACH_TRACKED (pb, p_ctx.port_binding_table) {
> >          bool removed = sbrec_port_binding_is_deleted(pb);
> > -        physical_handle_flows_for_lport(pb, removed, &p_ctx, &pfo->flow_table);
> > +        if (!physical_handle_flows_for_lport(pb, removed, &p_ctx,
> > +                                             &pfo->flow_table)) {
> > +            return false;
> > +        }
> >      }
> >
> >      engine_set_node_state(node, EN_UPDATED);
> > @@ -2930,8 +2933,10 @@ pflow_output_runtime_data_handler(struct engine_node *node, void *data)
> >              struct tracked_lport *lport = shash_node->data;
> >              bool removed =
> >                  lport->tracked_type == TRACKED_RESOURCE_REMOVED ? true: false;
> > -            physical_handle_flows_for_lport(lport->pb, removed, &p_ctx,
> > -                                            &pfo->flow_table);
> > +            if (!physical_handle_flows_for_lport(lport->pb, removed, &p_ctx,
> > +                                                 &pfo->flow_table)) {
> > +                return false;
> > +            }
> >          }
> >      }
> >
> > diff --git a/controller/physical.c b/controller/physical.c
> > index 6f2c1cea0..ffb9f9952 100644
> > --- a/controller/physical.c
> > +++ b/controller/physical.c
> > @@ -1509,11 +1509,16 @@ consider_mc_group(enum mf_field_id mff_ovn_geneve,
> >      sset_destroy(&remote_chassis);
> >  }
> >
> > -void
> > +bool
> >  physical_handle_flows_for_lport(const struct sbrec_port_binding *pb,
> >                                  bool removed, struct physical_ctx *p_ctx,
> >                                  struct ovn_desired_flow_table *flow_table)
> >  {
> > +    if (!strcmp(pb->type, "vtep")) {
> > +        /* Cannot handle changes to vtep lports (yet). */
> > +        return false;
> > +    }
> > +
> >      ofctrl_remove_flows(flow_table, &pb->header_.uuid);
> >
> >      if (!strcmp(pb->type, "external")) {
> > @@ -1553,6 +1558,8 @@ physical_handle_flows_for_lport(const struct sbrec_port_binding *pb,
> >                                p_ctx->chassis, flow_table, &ofpacts);
> >          ofpbuf_uninit(&ofpacts);
> >      }
> > +
> > +    return true;
> >  }
> >
> >  void
> > diff --git a/controller/physical.h b/controller/physical.h
> > index c4540ad7f..ee4b1ae1f 100644
> > --- a/controller/physical.h
> > +++ b/controller/physical.h
> > @@ -65,7 +65,7 @@ void physical_run(struct physical_ctx *,
> >                    struct ovn_desired_flow_table *);
> >  void physical_handle_mc_group_changes(struct physical_ctx *,
> >                                        struct ovn_desired_flow_table *);
> > -void physical_handle_flows_for_lport(const struct sbrec_port_binding *,
> > +bool physical_handle_flows_for_lport(const struct sbrec_port_binding *,
> >                                       bool removed,
> >                                       struct physical_ctx *,
> >                                       struct ovn_desired_flow_table *);
> >
>
> The change looks ok to me. I am not really sure how to test it though?
> Any suggestions?

Thanks for the review.  Vladislav will test this patch out.  He confirmed to me
in the thread where he reported the issue.

Numan

>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
Odintsov Vladislav Sept. 16, 2021, 12:12 a.m. UTC | #3 
Hi Numan,

for me this patch solves the issue.
Actually, it’s a good point about adding test for ovn-controller-vtep to eliminate this problem in future.
However there was another problem with HW VTEP support in OVN with using stateful services in the datapath.

I’ve sent a patch series with a test, which reproduces the mentioned problem. Bigfix is in a third patch. Please try those patches:
https://patchwork.ozlabs.org/project/ovn/cover/20210916000624.1609-1-odivlad@gmail.com/

Tested-by: Vladislav Odintsov <odivlad@gmail.com<mailto:odivlad@gmail.com>>

Regards,
Vladislav Odintsov

On 8 Sep 2021, at 20:14, Numan Siddique <numans@ovn.org<mailto:numans@ovn.org>> wrote:

On Wed, Sep 8, 2021 at 5:59 AM Mark Gray <mark.d.gray@redhat.com<mailto:mark.d.gray@redhat.com>> wrote:

On 07/09/2021 18:15, numans@ovn.org<mailto:numans@ovn.org> wrote:
From: Numan Siddique <numans@ovn.org<mailto:numans@ovn.org>>

When a vtep logical port changes, necessary flows are not added as
expected.  This is because the function physical_handle_flows_for_lport()
in physical.c does not add flows required for vtep logical ports.
These flows are added by physical_run(). So fall back to full recompute
of pflow_output engine node when vtep lports change.

Reported-by: Odintsov Vladislav <VlOdintsov@croc.ru<mailto:VlOdintsov@croc.ru>>
Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2021-September/387435.html
Signed-off-by: Numan Siddique <numans@ovn.org<mailto:numans@ovn.org>>
---
controller/ovn-controller.c | 11 ++++++++---
controller/physical.c       |  9 ++++++++-
controller/physical.h       |  2 +-
3 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index 0031a1035..d98ebbbfa 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -2867,7 +2867,10 @@ pflow_output_sb_port_binding_handler(struct engine_node *node,
    const struct sbrec_port_binding *pb;
    SBREC_PORT_BINDING_TABLE_FOR_EACH_TRACKED (pb, p_ctx.port_binding_table) {
        bool removed = sbrec_port_binding_is_deleted(pb);
-        physical_handle_flows_for_lport(pb, removed, &p_ctx, &pfo->flow_table);
+        if (!physical_handle_flows_for_lport(pb, removed, &p_ctx,
+                                             &pfo->flow_table)) {
+            return false;
+        }
    }

    engine_set_node_state(node, EN_UPDATED);
@@ -2930,8 +2933,10 @@ pflow_output_runtime_data_handler(struct engine_node *node, void *data)
            struct tracked_lport *lport = shash_node->data;
            bool removed =
                lport->tracked_type == TRACKED_RESOURCE_REMOVED ? true: false;
-            physical_handle_flows_for_lport(lport->pb, removed, &p_ctx,
-                                            &pfo->flow_table);
+            if (!physical_handle_flows_for_lport(lport->pb, removed, &p_ctx,
+                                                 &pfo->flow_table)) {
+                return false;
+            }
        }
    }

diff --git a/controller/physical.c b/controller/physical.c
index 6f2c1cea0..ffb9f9952 100644
--- a/controller/physical.c
+++ b/controller/physical.c
@@ -1509,11 +1509,16 @@ consider_mc_group(enum mf_field_id mff_ovn_geneve,
    sset_destroy(&remote_chassis);
}

-void
+bool
physical_handle_flows_for_lport(const struct sbrec_port_binding *pb,
                                bool removed, struct physical_ctx *p_ctx,
                                struct ovn_desired_flow_table *flow_table)
{
+    if (!strcmp(pb->type, "vtep")) {
+        /* Cannot handle changes to vtep lports (yet). */
+        return false;
+    }
+
    ofctrl_remove_flows(flow_table, &pb->header_.uuid);

    if (!strcmp(pb->type, "external")) {
@@ -1553,6 +1558,8 @@ physical_handle_flows_for_lport(const struct sbrec_port_binding *pb,
                              p_ctx->chassis, flow_table, &ofpacts);
        ofpbuf_uninit(&ofpacts);
    }
+
+    return true;
}

void
diff --git a/controller/physical.h b/controller/physical.h
index c4540ad7f..ee4b1ae1f 100644
--- a/controller/physical.h
+++ b/controller/physical.h
@@ -65,7 +65,7 @@ void physical_run(struct physical_ctx *,
                  struct ovn_desired_flow_table *);
void physical_handle_mc_group_changes(struct physical_ctx *,
                                      struct ovn_desired_flow_table *);
-void physical_handle_flows_for_lport(const struct sbrec_port_binding *,
+bool physical_handle_flows_for_lport(const struct sbrec_port_binding *,
                                     bool removed,
                                     struct physical_ctx *,
                                     struct ovn_desired_flow_table *);


The change looks ok to me. I am not really sure how to test it though?
Any suggestions?

Thanks for the review.  Vladislav will test this patch out.  He confirmed to me
in the thread where he reported the issue.

Numan
Numan Siddique Sept. 17, 2021, 2:10 p.m. UTC | #4 
On Wed, Sep 15, 2021 at 8:12 PM Odintsov Vladislav <VlOdintsov@croc.ru> wrote:
>
> Hi Numan,
>
> for me this patch solves the issue.
> Actually, it’s a good point about adding test for ovn-controller-vtep to eliminate this problem in future.
> However there was another problem with HW VTEP support in OVN with using stateful services in the datapath.
>
> I’ve sent a patch series with a test, which reproduces the mentioned problem. Bigfix is in a third patch. Please try those patches:
> https://patchwork.ozlabs.org/project/ovn/cover/20210916000624.1609-1-odivlad@gmail.com/
>
> Tested-by: Vladislav Odintsov <odivlad@gmail.com<mailto:odivlad@gmail.com>>

Thanks for testing it out and adding the test case in your other series.

I applied this patch to the main branch.

Numan

>
> Regards,
> Vladislav Odintsov
>
> On 8 Sep 2021, at 20:14, Numan Siddique <numans@ovn.org<mailto:numans@ovn.org>> wrote:
>
> On Wed, Sep 8, 2021 at 5:59 AM Mark Gray <mark.d.gray@redhat.com<mailto:mark.d.gray@redhat.com>> wrote:
>
> On 07/09/2021 18:15, numans@ovn.org<mailto:numans@ovn.org> wrote:
> From: Numan Siddique <numans@ovn.org<mailto:numans@ovn.org>>
>
> When a vtep logical port changes, necessary flows are not added as
> expected.  This is because the function physical_handle_flows_for_lport()
> in physical.c does not add flows required for vtep logical ports.
> These flows are added by physical_run(). So fall back to full recompute
> of pflow_output engine node when vtep lports change.
>
> Reported-by: Odintsov Vladislav <VlOdintsov@croc.ru<mailto:VlOdintsov@croc.ru>>
> Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2021-September/387435.html
> Signed-off-by: Numan Siddique <numans@ovn.org<mailto:numans@ovn.org>>
> ---
> controller/ovn-controller.c | 11 ++++++++---
> controller/physical.c       |  9 ++++++++-
> controller/physical.h       |  2 +-
> 3 files changed, 17 insertions(+), 5 deletions(-)
>
> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> index 0031a1035..d98ebbbfa 100644
> --- a/controller/ovn-controller.c
> +++ b/controller/ovn-controller.c
> @@ -2867,7 +2867,10 @@ pflow_output_sb_port_binding_handler(struct engine_node *node,
>     const struct sbrec_port_binding *pb;
>     SBREC_PORT_BINDING_TABLE_FOR_EACH_TRACKED (pb, p_ctx.port_binding_table) {
>         bool removed = sbrec_port_binding_is_deleted(pb);
> -        physical_handle_flows_for_lport(pb, removed, &p_ctx, &pfo->flow_table);
> +        if (!physical_handle_flows_for_lport(pb, removed, &p_ctx,
> +                                             &pfo->flow_table)) {
> +            return false;
> +        }
>     }
>
>     engine_set_node_state(node, EN_UPDATED);
> @@ -2930,8 +2933,10 @@ pflow_output_runtime_data_handler(struct engine_node *node, void *data)
>             struct tracked_lport *lport = shash_node->data;
>             bool removed =
>                 lport->tracked_type == TRACKED_RESOURCE_REMOVED ? true: false;
> -            physical_handle_flows_for_lport(lport->pb, removed, &p_ctx,
> -                                            &pfo->flow_table);
> +            if (!physical_handle_flows_for_lport(lport->pb, removed, &p_ctx,
> +                                                 &pfo->flow_table)) {
> +                return false;
> +            }
>         }
>     }
>
> diff --git a/controller/physical.c b/controller/physical.c
> index 6f2c1cea0..ffb9f9952 100644
> --- a/controller/physical.c
> +++ b/controller/physical.c
> @@ -1509,11 +1509,16 @@ consider_mc_group(enum mf_field_id mff_ovn_geneve,
>     sset_destroy(&remote_chassis);
> }
>
> -void
> +bool
> physical_handle_flows_for_lport(const struct sbrec_port_binding *pb,
>                                 bool removed, struct physical_ctx *p_ctx,
>                                 struct ovn_desired_flow_table *flow_table)
> {
> +    if (!strcmp(pb->type, "vtep")) {
> +        /* Cannot handle changes to vtep lports (yet). */
> +        return false;
> +    }
> +
>     ofctrl_remove_flows(flow_table, &pb->header_.uuid);
>
>     if (!strcmp(pb->type, "external")) {
> @@ -1553,6 +1558,8 @@ physical_handle_flows_for_lport(const struct sbrec_port_binding *pb,
>                               p_ctx->chassis, flow_table, &ofpacts);
>         ofpbuf_uninit(&ofpacts);
>     }
> +
> +    return true;
> }
>
> void
> diff --git a/controller/physical.h b/controller/physical.h
> index c4540ad7f..ee4b1ae1f 100644
> --- a/controller/physical.h
> +++ b/controller/physical.h
> @@ -65,7 +65,7 @@ void physical_run(struct physical_ctx *,
>                   struct ovn_desired_flow_table *);
> void physical_handle_mc_group_changes(struct physical_ctx *,
>                                       struct ovn_desired_flow_table *);
> -void physical_handle_flows_for_lport(const struct sbrec_port_binding *,
> +bool physical_handle_flows_for_lport(const struct sbrec_port_binding *,
>                                      bool removed,
>                                      struct physical_ctx *,
>                                      struct ovn_desired_flow_table *);
>
>
> The change looks ok to me. I am not really sure how to test it though?
> Any suggestions?
>
> Thanks for the review.  Vladislav will test this patch out.  He confirmed to me
> in the thread where he reported the issue.
>
> Numan
>
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org<mailto:dev@openvswitch.org>
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org<mailto: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 mbox series

Patch

diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index 0031a1035..d98ebbbfa 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -2867,7 +2867,10 @@  pflow_output_sb_port_binding_handler(struct engine_node *node,
     const struct sbrec_port_binding *pb;
     SBREC_PORT_BINDING_TABLE_FOR_EACH_TRACKED (pb, p_ctx.port_binding_table) {
         bool removed = sbrec_port_binding_is_deleted(pb);
-        physical_handle_flows_for_lport(pb, removed, &p_ctx, &pfo->flow_table);
+        if (!physical_handle_flows_for_lport(pb, removed, &p_ctx,
+                                             &pfo->flow_table)) {
+            return false;
+        }
     }
 
     engine_set_node_state(node, EN_UPDATED);
@@ -2930,8 +2933,10 @@  pflow_output_runtime_data_handler(struct engine_node *node, void *data)
             struct tracked_lport *lport = shash_node->data;
             bool removed =
                 lport->tracked_type == TRACKED_RESOURCE_REMOVED ? true: false;
-            physical_handle_flows_for_lport(lport->pb, removed, &p_ctx,
-                                            &pfo->flow_table);
+            if (!physical_handle_flows_for_lport(lport->pb, removed, &p_ctx,
+                                                 &pfo->flow_table)) {
+                return false;
+            }
         }
     }
 
diff --git a/controller/physical.c b/controller/physical.c
index 6f2c1cea0..ffb9f9952 100644
--- a/controller/physical.c
+++ b/controller/physical.c
@@ -1509,11 +1509,16 @@  consider_mc_group(enum mf_field_id mff_ovn_geneve,
     sset_destroy(&remote_chassis);
 }
 
-void
+bool
 physical_handle_flows_for_lport(const struct sbrec_port_binding *pb,
                                 bool removed, struct physical_ctx *p_ctx,
                                 struct ovn_desired_flow_table *flow_table)
 {
+    if (!strcmp(pb->type, "vtep")) {
+        /* Cannot handle changes to vtep lports (yet). */
+        return false;
+    }
+
     ofctrl_remove_flows(flow_table, &pb->header_.uuid);
 
     if (!strcmp(pb->type, "external")) {
@@ -1553,6 +1558,8 @@  physical_handle_flows_for_lport(const struct sbrec_port_binding *pb,
                               p_ctx->chassis, flow_table, &ofpacts);
         ofpbuf_uninit(&ofpacts);
     }
+
+    return true;
 }
 
 void
diff --git a/controller/physical.h b/controller/physical.h
index c4540ad7f..ee4b1ae1f 100644
--- a/controller/physical.h
+++ b/controller/physical.h
@@ -65,7 +65,7 @@  void physical_run(struct physical_ctx *,
                   struct ovn_desired_flow_table *);
 void physical_handle_mc_group_changes(struct physical_ctx *,
                                       struct ovn_desired_flow_table *);
-void physical_handle_flows_for_lport(const struct sbrec_port_binding *,
+bool physical_handle_flows_for_lport(const struct sbrec_port_binding *,
                                      bool removed,
                                      struct physical_ctx *,
                                      struct ovn_desired_flow_table *);