[ovs-dev] [RFC v3 10/19] classifer: Convert struct flow flow_metadata to use tun_key

[Simon Horman]

On Thu, May 17, 2012 at 06:10:54PM -0700, Ben Pfaff wrote:
> On Fri, May 18, 2012 at 09:29:09AM +0900, Simon Horman wrote:
> > On Thu, May 17, 2012 at 09:48:23AM -0700, Ben Pfaff wrote:
> > > On Thu, May 17, 2012 at 05:59:09PM +0900, Simon Horman wrote:
> > > > This allows the tun_key tp be bassed throughout user-space,
> > > > attached to a flow. This is the essence of flow-based tunneling.
> > > > 
> > > > This does not add tun_key or wildcards, other than the existing match for
> > > > the tun_id. It is envisaged that most if not all fields of the tun_key
> > > > could be wildcarded.
> > > > 
> > > > Cc: Kyle Mestery <kmestery at cisco.com>
> > > > Signed-off-by: Simon Horman <horms at verge.net.au>
> > > 
> > > This patch looks troublesome for layering reasons.
> > > lib/classifier.[ch] and lib/flow.[ch] are in a layer that is supposed
> > > to be ofproto-provider agnostic.  That is, among other constraints, it
> > > should not depend on Linux kernel headers.  We may have to come up
> > > with a more neutral way to represent the tun_key for general userspace
> > > purposes than to use ovs_key_ipv4_tunnel.
> > 
> > Understood.
> > 
> > I think that the crux of the problem you point out lies in the use
> > of struct ovs_key_ipv4_tunnel, which is defined in linux/openvswitch.h.
> > 
> > An approach that I had considered, and one that may be woth revisiting,
> > is to either define a different structure (perhaps the same interlally
> > but with a different name) or simply break out the fields so that
> > the user-space flow structure would have tun_id, tun_ipv4_dst,... fields.
> > 
> > Although the elemtns of tun_key do relate to the linux implementation
> > (elsewhere in this series), I think that conceptually they are not
> > ofproto-provider specific.  GRE, for instance, has such parameters
> > regardless of the implementation.
> 
> I think I agree with all that.
> 
> I think I'd be OK with either using a different structure or breaking
> out the fields.  The latter *might* work better for alignment (it's
> nice not to have holes in struct flow) but it's probably not a big
> deal either way.

I agree, I'll see what seems to work best when I implement the change.