[ovs-dev] [PATCH] ofp-actions: Consider pop MPLS to non-MPLS ethtype as consistent

Simon Horman horms at verge.net.au
Wed Feb 19 12:01:05 UTC 2014


On Tue, Feb 18, 2014 at 10:43:27AM -0800, Jarno Rajahalme wrote:
> 
> On Feb 16, 2014, at 5:11 PM, Simon Horman <horms at verge.net.au> wrote:
> 
> > On Mon, Feb 17, 2014 at 10:06:55AM +0900, Simon Horman wrote:
> >> Remove the restriction that pop MPLS to an ethtype is
> >> considered inconsistent.
> >> 
> >> As it happens this is allowed for OpenFlow 1.0, and thus
> >> the tests in the Open vSwtich test-suite, by virtue of
> >> inconsistent actions being allowed in the case of OpenFlow 1.0.
> >> 
> >> Its not clear to me why they should be considered inconsistent
> >> or disallowed for other OpenFlow versions.
> >> 
> >> This was found using Ryu tests via the new make check-ryu target.
> >> It allows all of the "POP_MPLS"[1] tests to pass where they previously
> >> failed.
> >> 
> >> [1] http://osrg.github.io/ryu-certification/switch/ovs
> >> 
> >> Signed-off-by: Simon Horman <horms at verge.net.au>
> >> ---
> >> lib/ofp-actions.c | 3 ---
> >> 1 file changed, 3 deletions(-)
> >> 
> >> diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
> >> index 781c3a1..623c726 100644
> >> --- a/lib/ofp-actions.c
> >> +++ b/lib/ofp-actions.c
> >> @@ -2068,9 +2068,6 @@ ofpact_check__(enum ofputil_protocol *usable_protocols, struct ofpact *a,
> >> 
> >>     case OFPACT_POP_MPLS:
> >>         flow->dl_type = ofpact_get_POP_MPLS(a)->ethertype;
> >> -        if (!eth_type_mpls(flow->dl_type)) {
> >> -            inconsistent_match(usable_protocols);
> >> -        }
> >>         return 0;
> >> 
> >>     case OFPACT_SAMPLE:
> > 
> > Looking over this another time, I wonder if the eth_type_mpls()
> > check should be moved to above the flow->dl_type = ofpact_get_POP_MPLS(a)->ethertype
> > line. pop MPLS on a non MPLS packet would indeed be inconsistent.
> 
> 
> That sounds right to me,

Thanks, I will send an updated patch.



More information about the dev mailing list