On Mon, Jan 20, 2025 at 5:14 PM Ales Musil <[email protected]> wrote:

>
>
> 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
>

By the look of it there is actually conflict, I'll resolve it and send v2.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to