On 05/05/2015 02:26 PM, Justin Pettit wrote:
> 
>> On May 5, 2015, at 6:59 AM, Russell Bryant <rbry...@redhat.com> wrote:
>>
>> ovn-controller updates the chassis column of the Bindings table in
>> OVN_Southbound when a logical port appears on the local switch.  A
>> logical port that has a parent will never appear on a switch managed
>> by ovn-controller.  When a parent port appears, all child container
>> ports should be updated as being on that chassis, as well.
> 
> Good catch.
> 
>>     SBREC_BINDINGS_FOR_EACH(bindings_rec, ctx->ovnsb_idl) {
>>         if (sset_find_and_delete(&lports, bindings_rec->logical_port)) {
>> +            sset_add(&found_lports, bindings_rec->logical_port);
> 
> Instead of adding to "found_lports" as they're being removed from
> "lports", what about just doing an sset_clone() after the call to
> get_local_iface_ids() into a new sset called "all_lports"?

Ah, nice.

> 
>>             if (!strcmp(bindings_rec->chassis, ctx->chassis_id)) {
>>                 continue;
>>             }
>> @@ -99,6 +101,15 @@ bindings_run(struct controller_ctx *ctx)
>>                           ctx->chassis_id);
>>             }
>>             sbrec_bindings_set_chassis(bindings_rec, ctx->chassis_id);
>> +        } else if (bindings_rec->parent_port && 
>> bindings_rec->parent_port[0] &&
>> +                (sset_contains(&lports, bindings_rec->parent_port) ||
>> +                 sset_contains(&found_lports, bindings_rec->parent_port))) {
> 
> Then, you only need to do a single sset_contains() call on "all_lports" here.

Right, much better.

> 
>> +            if (bindings_rec->chassis[0]) {
>> +                VLOG_INFO("Changing chassis for lport %s from %s to %s",
>> +                          bindings_rec->logical_port, bindings_rec->chassis,
>> +                          ctx->chassis_id);
>> +            }
> 
> Won't this be logged even if the chassis is not changed?

Oops, yes.  I copied this from code above it that was handling regular
ports.  However, I neglected to copy the check to see if the chassis is
already set properly and continuing in that case.

I'll incorporate these changes and send another rev.

Thanks for the feedback!

-- 
Russell Bryant
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to