I will send a v2.
On Mon, Sep 28, 2015 at 12:28 PM, Gurucharan Shetty <shet...@nicira.com> wrote:
>> 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
