On Mon, Jan 20, 2025 at 5:10 PM Numan Siddique <[email protected]> wrote:

> On Mon, Jan 20, 2025 at 4:16 AM Ales Musil <[email protected]> wrote:
> >
> > Simplify incremental processing for port bindings especially
> > with peer ports.
> >
> > Signed-off-by: Ales Musil <[email protected]>
> > ---
> >  controller/physical.c | 19 ++++++-------------
> >  1 file changed, 6 insertions(+), 13 deletions(-)
> >
> > diff --git a/controller/physical.c b/controller/physical.c
> > index 69298e1e4..ae44cb1f4 100644
> > --- a/controller/physical.c
> > +++ b/controller/physical.c
> > @@ -2413,22 +2413,15 @@ physical_handle_flows_for_lport(const struct
> sbrec_port_binding *pb,
> >      }
> >
> >      /* Always update pb and the configured peer for patch ports. */
> > -    if (!removed || !strcmp(pb->type, "patch")) {
> > +    if (!removed) {
> >          physical_eval_port_binding(p_ctx, pb, flow_table);
> >      }
> >
> > -    if (!strcmp(pb->type, "patch")) {
> > -        if (removed) {
> > -            ofctrl_remove_flows(flow_table, &pb->header_.uuid);
> > -        }
> > -        const struct sbrec_port_binding *peer =
> > -            get_binding_peer(p_ctx->sbrec_port_binding_by_name, pb);
> > -        if (peer) {
> > -            physical_eval_port_binding(p_ctx, peer, flow_table);
> > -            if (removed) {
> > -                ofctrl_remove_flows(flow_table, &peer->header_.uuid);
> > -            }
> > -        }
>
> Thanks Ales for the patch.  This makes total sense.
> While applying a patch eariler, I missed that ofctrl_remove_flows() is
> called at the beginning of
> the function.
>
>
> > +    const struct sbrec_port_binding *peer =
> > +        get_binding_peer(p_ctx->sbrec_port_binding_by_name, pb);
>
> Before calling get_binding_peer() I think we can check the type to be
> LP_PATCH to avoid
> unnecessary smap_get() and lport_lookup_by_name().  What do you think ?
>
> Like
>
> const struct sbrec_port_binding *peer = (type == LP_PATCH ?
> get_binding_peer(...) : NULL;
>

Good point, do you want me to send v2?


> Thanks
> Numan
>
> > +    if (peer) {
> > +        ofctrl_remove_flows(flow_table, &peer->header_.uuid);
> > +        physical_eval_port_binding(p_ctx, peer, flow_table);
> >      }
> >
> >      return true;
> > --
> > 2.47.1
> >
> > _______________________________________________
> > dev mailing list
> > [email protected]
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
Thanks,
Ales
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to