> From first read of the patch, it looks like this doesn't change
> anything. The above code still seems to provide the same result. The
> important part is actually in code not changed. There is code later
> that uses whether or not the port is in lport_to_ofport to determine if
> it's local or not.
Right. I took the approach to prevent future bugs that may add code based on
the name 'lport_to_ofport' which can indicate that the simap has all
the lport to ofport mappings. Instead it only has local vm backed
logical ports.
So presumably, I can simply change the name to 'vif_to_ofport'.
>
>> if (simap_contains(&lport_to_ofport, port->logical_port)) {
>> put_load(port->tunnel_key, MFF_LOG_OUTPORT, 0, 32, &ofpacts);
>> put_resubmit(OFTABLE_DROP_LOOPBACK, &ofpacts);
>
> I think I'd actually prefer just fixing that code to handle the
> parent_port case. It prevents having to add another full iteration of
> port bindings, which could have many thousands of entries.
>
> I think this would fix it, but I haven't tested it.
Yes, it should fix it. I am fine with either approach.
>
>
>> diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c
>> index bdb02da..5cc700e 100644
>> --- a/ovn/controller/physical.c
>> +++ b/ovn/controller/physical.c
>> @@ -471,7 +471,8 @@ physical_run(struct controller_ctx *ctx, enum
>> mf_field_id mff_ovn_geneve,
>> continue;
>> }
>>
>> - if (simap_contains(&lport_to_ofport, port->logical_port)) {
>> + if (simap_contains(&lport_to_ofport,
>> + port->parent_port ? port->parent_port :
>> port->logical_port)) {
>> put_load(port->tunnel_key, MFF_LOG_OUTPORT, 0, 32,
>> &ofpacts);
>> put_resubmit(OFTABLE_DROP_LOOPBACK, &ofpacts);
>> } else if (port->chassis) {
>
> --
> Russell Bryant
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
