[ovs-dev] [PATCH 01/16] User-Space MPLS actions and matches

Simon Horman horms at verge.net.au
Fri Jan 25 04:44:39 UTC 2013

On Tue, Jan 22, 2013 at 03:49:42PM -0800, Ben Pfaff wrote:
> On Thu, Jan 17, 2013 at 02:26:06PM +0900, Simon Horman wrote:
> > > On Tue, Jan 15, 2013 at 11:05:45PM -0800, Ben Pfaff wrote:
> > > > parse_l3_onward() could use flow_innermost_dl_type() since that's what
> > > > it's effectively calculating as 'dl_type' (maybe it should be
> > > > 'inner_dl_type' or 'innermost_dl_type').
> > 
> > I'm not so sure about this.
> > 
> > flow_innermost_dl_type(), in its current incantation, is used to
> > obtain the innermost dl_type of a struct flow. In other words,
> > it reads flow->encap_dl_type.
> > 
> > On the other hand, parse_l3_onward() may write flow->encap_dl_type
> > and never reads it.
> Before parse_l3_onward() gets called, flow->dl_type is set and
> flow->encap_dl_type is 0, so 'dl_type' is initialized with the
> innermost ethertype at that point.
> After the first block in parse_l3_onward() runs, flow->encap_dl_type
> may now be initialized to the inner ethertype and, if so, 'dl_type' is
> updated to the same value.  So 'dl_type' is also initialized with the
> innermost ethertype at that point too.
> I think it's the same behavior, either way, so I'm not going to press
> the point, but if you see a difference in behavior then please let me
> know.

Thanks, I think that I see what you are getting at.

I have moved things around a little so that is only set
as the result of flow_innermost_dl_type(flow) after the
first (MPLS) block.

I think this aids readability and there should be no behavioural
change to parse_l3_onward().

> (It now occurs to me that we should probably rename this function
> parse_l2_5_onward().)


More information about the dev mailing list