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

[Jesse Gross]

On Thu, Apr 18, 2013 at 8:07 AM, Jarno Rajahalme
<jarno.rajahalme at nsn.com> wrote:
> diff --git a/lib/match.c b/lib/match.c
> index 2aa4e89..222e5d7 100644
> --- a/lib/match.c
> +++ b/lib/match.c
> @@ -133,13 +133,9 @@ match_wc_init(struct match *match, const struct flow *flow)
>  void
>  match_init_exact(struct match *match, const struct flow *flow)
>  {
> -    ovs_be64 tun_id = flow->tunnel.tun_id;
> -
>      match->flow = *flow;
>      match->flow.skb_priority = 0;
>      match->flow.skb_mark = 0;
> -    memset(&match->flow.tunnel, 0, sizeof match->flow.tunnel);
> -    match->flow.tunnel.tun_id = tun_id;

Since we're not allowing matching on these fields yet, it seems like
semantically it makes more sense to include this change as part of the
last patch. In this case, it probably doesn't matter much in practice
though.

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