[ovs-dev] [PATCH 4/7] Keep all of tunnel metadata in flow.

[Rajahalme, Jarno (NSN - FI/Espoo)]

On Apr 28, 2013, at 21:29 , ext Rajahalme, Jarno (NSN - FI/Espoo) wrote:
> 
> On Apr 27, 2013, at 0:43 , ext Jesse Gross wrote:
> 
>> On Thu, Apr 18, 2013 at 8:07 AM, Jarno Rajahalme
>> <jarno.rajahalme at nsn.com> wrote:
>>> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
>>> index 33b09c6..b88a2d4 100644
>>> --- a/ofproto/ofproto-dpif.c
>>> +++ b/ofproto/ofproto-dpif.c
>>> @@ -6849,23 +6847,23 @@ action_xlate_ctx_init(struct action_xlate_ctx *ctx,
>>> +     * - Tunnel metadata as received is retained in both 'flow' and
>>> +     *   'base_flow'.  This will prevent packet output with the exact same
>>> +     *   headers as it was received with (which would not make sense since
>>> +     *   the destination would be us).  In effect, this emulates the kernel
>>> +     *   behavior of clearing the tunnel metadata before action processing.
>> 
>> This makes me nervous because it desynchronizes userspace's view from
>> the kernel's. Historically, this has caused problems (the ones that
>> Ben cited) when we try to generate actions based on what we think has
>> changed. In this case, the effect is likely fairly limited - you won't
>> be able to send a packet with the exact same parameters that it came
>> in on but it doesn't really seem like a good direction.
> 
> In retrospect it would be cleaner to zero out tunnel metadata in 'base_flow'
> (as before) and keep it only in 'flow' to allow later matching on it.
> This way 'base_flow' would reflect how kernel sees tunnel metadata at the
> start of action processing (none). The fact that 'flow' is different from
> 'base_flow' in this regard does not matter, as tunnel metadata is only ever
> committed to kernel with tunnel output, and tnl_port_send() will take care
> of setting the flow.tunnel as need be.
> 
> Another thing that I came to think only when reading Ben's new tutorial:
> Output to the input port is skipped. This would be a problem if you only
> have one generic flow based GRE port (as enabled by the last patch in
> this series), and you should forward GRE input to another GRE output.

read: "... to another GRE destination."

> This could be fixed by always allowing output (in xlate_output_action())
> to a tunnel that has cfg.ip_dst_flow set, even if the output port is the
> same as the input port. Thoughts?
> 

Note to self: It might be prudent to consider carrying and decrementing
the tunnel ttl from a tunnel input to tunnel output to cut out possible loops,
at least in this case of forwarding back to flow-based tunnel input port.

  Jarno