[ovs-dev] [PATCH] ovn-controller: Fix table 33 flows for containers.

[Gurucharan Shetty]

I will send a v2.

On Mon, Sep 28, 2015 at 12:28 PM, Gurucharan Shetty <shettyg at 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