[ovs-dev] [PATCH 04/16] actions: Allow secondary decoding of a flow

Ben Pfaff blp at nicira.com
Wed Feb 13 16:42:53 UTC 2013


On Wed, Feb 13, 2013 at 11:47:46PM +0900, Simon Horman wrote:
> On Tue, Feb 12, 2013 at 09:42:52AM -0800, Ben Pfaff wrote:
> > On Wed, Feb 06, 2013 at 10:53:55PM +0900, Simon Horman wrote:
> > > Actions may provide information to allow further decoding of
> > > a flow.
> > > 
> > > For example:
> > > 
> > > In the case of MPLS, L3 and L4 information may not initially be decoded
> > > from the frame as the ethernet type of the frame is an MPLS type and no
> > > information is known about the type of the inner frame.
> > > 
> > > However, the type of the inner frame may be provided by an mpls_pop action
> > > in which case L3 and L4 information may be decoded providing a finer
> > > grained match than is otherwise possible.
> > > 
> > > Signed-off-by: Simon Horman <horms at verge.net.au>
> > 
> > Why does odp_flow_key_from_flow() take a separate encap_dl_type
> > parameter instead of using flow->encap_dl_type?  There may be a good
> > reason, but I don't understand yet.
> 
> Having looked over the code paths again I believe that is now an artifact
> of previous development cycles and can be removed. I'll see about doing so.

OK, thanks.

> > The actions_allow_l3_extraction() check is probably not sufficient in
> > facet_check_consistency(), because it misses ofpacts found via
> > "resubmit".  We will probably need to record this in xlate_actions().
> 
> Thanks. I had not considered resubmit.
> 
> To clarify your suggestion: as xlate_actions() traverses all rules we can
> use it, perhaps using a return value, use it to determine if there l3
> extraction is possible or not more accurately and possibly more efficiently
> than using actions_allow_l3_extraction(). If so, yes, I agree and I'll make it
> so.

I'd guess that updating something in action_xlate_ctx would work better
than a return value but otherwise that's what I meant.

> In "ofproto: Allow actions richer matches based on actions"
> is used again in handle_flow_miss_without_facet() via
> handle_flow_miss_l3_extraction(). It seems that resubmit needs to be taken
> into account there too.

OK.

> > By the way, I think that we may need to adjust how MPLS information is
> > included in ODP flow key data.  Reading datapath/README, I think that we
> > may need to use a format like:
> > 
> >         eth(...), eth_type(0x8847), mpls(...), encap(ip(...), tcp(...))
> > 
> > whereas I believe we are currently omitting the encap and simply writing
> > 
> >         eth(...), eth_type(0x8847), mpls(...), ip(...), tcp(...)
> > 
> > I think the latter will make kernel ABI compatibility problematic.
> 
> Yes, the patch-set currently uses the latter.
> 
> I did once have a go at implementing the former, but without much success.
> I can try again. But before I do, I would like to know if encap() can
> be nested. I ask this because there can (in theory) be multiple MPLS LSEs.
> And also I believe that VLAN and MPLS can co-exist in a single frame.

Yes, encap() can nest.  What kind of problem did you have before?



More information about the dev mailing list