On Fri, Jan 25, 2013 at 01:44:39PM +0900, Simon Horman wrote: > 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().) > > > Done
Thanks on both counts. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev