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

Jesse Gross jesse at nicira.com
Mon Apr 29 17:49:35 UTC 2013


On Sun, Apr 28, 2013 at 11:29 AM, Rajahalme, Jarno (NSN - FI/Espoo)
<jarno.rajahalme at nsn.com> 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.

That sounds good to me.

> 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.
> 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?

I know that Ben was thinking about ways to relax this check anyways,
so it might be good to see how that fits with this.



More information about the dev mailing list