On Sun, Apr 28, 2013 at 11:29 AM, Rajahalme, Jarno (NSN - FI/Espoo) <[email protected]> wrote: > On Apr 27, 2013, at 0:43 , ext Jesse Gross wrote: >> On Thu, Apr 18, 2013 at 8:07 AM, Jarno Rajahalme >> <[email protected]> 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. _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
