On Mon, Sep 19, 2016 at 8:48 AM, Ben Pfaff <b...@ovn.org> wrote:
> On Thu, Sep 15, 2016 at 09:28:37AM -0700, Jesse Gross wrote:
>> When using tunnel TLVs (at the moment, this means Geneve options), a
>> controller must first map the class and type onto an appropriate OXM
>> field so that it can be used in OVS flow operations. This table is
>> managed using OpenFlow extensions.
>> The original code that added support for TLVs made the mapping table
>> global as a simplification. However, this is not really logically
>> correct as the OpenFlow management commands are operating on a per-bridge
>> basis. This removes the original limitation to make the table per-bridge.
>> One nice result of this change is that it is generally clearer whether
>> the tunnel metadata is in datapath or OpenFlow format. Rather than
>> allowing ad-hoc format changes and trying to handle both formats in the
>> tunnel metadata functions, the format is more clearly separated by function.
>> Datapaths (both kernel and userspace) use datapath format and it is not
>> changed during the upcall process. At the beginning of action translation,
>> tunnel metadata is converted to OpenFlow format and flows and wildcards
>> are translated back at the end of the process.
>> As an additional benefit, this change improves performance in some flow
>> setup situations by keeping the tunnel metadata in the original packet
>> format in more cases. This helps when copies need to be made as the amount
>> of data touched is only what is present in the packet rather than the
>> maximum amount of metadata supported.
>> Co-authored-by: Madhu Challa <cha...@noironetworks.com>
>> Signed-off-by: Madhu Challa <cha...@noironetworks.com>
>> Signed-off-by: Jesse Gross <je...@kernel.org>
>> ---
>> This is a rebased version of the previously posted patch which no longer
>> applies cleanly.
>> v2: Pass 'struct tun_table' instead of 'struct ofproto' for functions that
>>     need to access tunnel metadata.
> Thanks a lot Jesse.  I like this much better and, surprisingly, it
> yields a net reduction in code.
> I think that this should update NEWS to reflect the new feature.
> This appears to cause a new test failure:
>     2235: ovn -- 3 HVs, 1 VIFs/HV, 1 software GW, 1 LS    FAILED (ovn.at:1942)
> I couldn't immediately figure out why.  With the patch this test seems
> to fail consistently (10 failures out of 10) and without it it succeeds
> (0 failures out of 10).
> If you can figure something out there,
> Acked-by: Ben Pfaff <b...@ovn.org>

The test failure is the result of a silent merge conflict after the
patch was sent out introduced by "ofproto-dpif-xlate: Fix treatment of
mirrors across patch port.". That patch moved the initialization of a
variable to after some new code that was added by this patch.

I fixed that up, added an item to NEWS and applied this to master.

Thanks for reviewing it!
dev mailing list

Reply via email to