[ovs-dev] [PATCH 1/2] Improve dl_type checking in ofpact_check__
Simon Horman
horms at verge.net.au
Thu May 2 04:43:23 UTC 2013
On Wed, May 01, 2013 at 09:19:46PM -0700, Jesse Gross wrote:
> On Wed, May 1, 2013 at 1:11 AM, Simon Horman <horms at verge.net.au> wrote:
> > Some actions require an MPLS dl_type while others require a non-MPLS
> > dl_type. Enforce this for actions handled in ofpact_check__().
> >
> > Update the following actions to require an non-MPLS dl_type:
> >
> > set_ipv4_src
> > set_ipv4_dst
> > set_ipv4_dscp
> > set_l4_src_port
> > set_l4_dst_port
> > dec_ttl
> >
> > Update the following actions to require an MPLS dl_type:
> >
> > set_mpls_ttl
> > dec_mpls_ttl
> > pop_mpls
> >
> > Update handling of the following actions to use the dl_type set by MPLS
> > push and pop actions if it differs from the orginal dl_type. This allows
> > their existing checks to encorce dl_type pre-requisites correctly.
> >
> > output_reg
> > bundle
> > reg_move
> > stack_push
> > stack_pop
> > learn
> > multipath
> >
> > Signed-off-by: Simon Horman <horms at verge.net.au>
>
> I think this is somewhat inconsistent with the handling of other
> OpenFlow actions in the presence of wildcards. For example, a
> completely wildcarded flow will allow a set TCP port action to succeed
> while a set MPLS action will fail.
>
> Personally, I'm in favor of stricter checking but that's not what has
> been done historically. We do have those stricter checks for
> extensions like the register actions and we could do the same for MPLS
> although that seems like it would be very confusing.
>
> Regardless, I think we could probably also assume that updating the
> flow is the common case and do it once at the top of the loop rather
> than for each action type.
Thanks, I will update this patch to:
* Not include the non-flow changes.
Personally I am ambivalent to them, but I can address them later
if you like.
* Update the flow at the top of the loop.
More information about the dev
mailing list