On Tue, Oct 30, 2012 at 09:59:24AM +0900, Simon Horman wrote:
> On Mon, Oct 29, 2012 at 09:58:58AM -0700, Ben Pfaff wrote:
> > On Sat, Oct 27, 2012 at 03:05:57PM +0900, Simon Horman wrote:
> > > There are (or at least will be) cases where this check can produce false
> > > positives. For example, a flow which matches a non-MPLS packet and then
> > > applies an MPLS push action followed by an action to load the MPLS label.
> >
> > So, I remember the discussion, but I don't remember coming to exactly
> > this conclusion.
> >
> > The problem seems to be, at this point, MPLS specific, because for
> > most prerequisites, whether it is satisfied cannot be affected by
> > actions. For example, no action can transform a TCP packet into a
> > non-TCP packet, and vice versa.
> >
> > My thought is, then, for now we should define a new function to test
> > whether a prerequisite could ever be satisfied given a particular
> > flow. For all prerequisites other than MPLS, this would return the
> > same thing as mf_are_prereqs_ok(); for MPLS, it could just return
> > true.
>
> I think that I am comfortable with that approach, I will implement it
> and see how things go.
Hi Ben,
I had an idea about how to fix this particular problem a different way.
The idea seems to work, I would appreciate your feedback.
----------------------------------------------------------------
[RFC] Handle pre-requisites of MPLS set-field/load actions
mpls_push and mpls_pop actions may modify the dl_type of a flow.
This needs to be taken into account when checking the pre-requisites
of set-field/load actions.
This patch implements this by tracking the dl_type changes of
mpls_push and mpls_pop actions and passing an updated flow to
nxm_reg_load_check().
If acceptable, this change should be merged partially into
"User-Space MPLS actions and matches" and partially into
"Add support for copy_ttl_out action".
Signed-off-by: Simon Horman <ho...@verge.net.au>
---
lib/ofp-actions.c | 20 ++++++++++++++++----
tests/ofproto-dpif.at | 4 ++--
2 files changed, 18 insertions(+), 6 deletions(-)
diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index 7e59484..df07357 100644
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -1112,7 +1112,8 @@ exit:
}
static enum ofperr
-ofpact_check__(const struct ofpact *a, const struct flow *flow, int max_ports)
+ofpact_check__(const struct ofpact *a, const struct flow *flow, int max_ports,
+ ovs_be16 *dl_type)
{
const struct ofpact_enqueue *enqueue;
@@ -1153,8 +1154,11 @@ ofpact_check__(const struct ofpact *a, const struct flow
*flow, int max_ports)
case OFPACT_REG_MOVE:
return nxm_reg_move_check(ofpact_get_REG_MOVE(a), flow);
- case OFPACT_REG_LOAD:
- return nxm_reg_load_check(ofpact_get_REG_LOAD(a), flow);
+ case OFPACT_REG_LOAD: {
+ struct flow updated_flow = *flow;
+ updated_flow.dl_type = *dl_type;
+ return nxm_reg_load_check(ofpact_get_REG_LOAD(a), &updated_flow);
+ }
case OFPACT_DEC_TTL:
case OFPACT_COPY_TTL_IN:
@@ -1179,8 +1183,14 @@ ofpact_check__(const struct ofpact *a, const struct flow
*flow, int max_ports)
case OFPACT_NOTE:
case OFPACT_EXIT:
+ return 0;
+
case OFPACT_PUSH_MPLS:
+ *dl_type = ofpact_get_PUSH_MPLS(a)->ethertype;
+ return 0;
+
case OFPACT_POP_MPLS:
+ *dl_type = ofpact_get_POP_MPLS(a)->ethertype;
return 0;
case OFPACT_CLEAR_ACTIONS:
@@ -1201,9 +1211,11 @@ ofpacts_check(const struct ofpact ofpacts[], size_t
ofpacts_len,
const struct flow *flow, int max_ports)
{
const struct ofpact *a;
+ ovs_be16 dl_type = flow->dl_type;
OFPACT_FOR_EACH (a, ofpacts, ofpacts_len) {
- enum ofperr error = ofpact_check__(a, flow, max_ports);
+ enum ofperr error = ofpact_check__(a, flow, max_ports,
+ &dl_type);
if (error) {
return error;
}
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index 24f04d4..642b261 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -256,8 +256,8 @@ cookie=0xa dl_src=40:44:44:44:44:46
actions=push_mpls:0x8847,load:10->OXM_OF_MPL
cookie=0xa dl_src=40:44:44:44:44:47
actions=push_mpls:0x8847,load:10->OXM_OF_MPLS_LABEL[[]],load:3->OXM_OF_MPLS_TC[[]],dec_mpls_ttl,set_mpls_ttl(10),controller
cookie=0xa dl_src=40:44:44:44:44:48
actions=push_mpls:0x8847,load:10->OXM_OF_MPLS_LABEL[[]],load:3->OXM_OF_MPLS_TC[[]],set_mpls_ttl(10),dec_mpls_ttl,controller
cookie=0xa dl_src=40:44:44:44:44:49
actions=push_mpls:0x8847,load:10->OXM_OF_MPLS_LABEL[[]],load:3->OXM_OF_MPLS_TC[[]],set_mpls_ttl(10),dec_mpls_ttl,copy_ttl_out,controller
-cookie=0xb dl_src=50:55:55:55:55:55
actions=load:1000->OXM_OF_MPLS_LABEL[[]],controller
-cookie=0xb dl_src=50:55:55:55:55:56
actions=load:1000->OXM_OF_MPLS_LABEL[[]],copy_ttl_out,controller
+cookie=0xb dl_src=50:55:55:55:55:55 dl_type=0x8847
actions=load:1000->OXM_OF_MPLS_LABEL[[]],controller
+cookie=0xb dl_src=50:55:55:55:55:56 dl_type=0x8847
actions=load:1000->OXM_OF_MPLS_LABEL[[]],copy_ttl_out,controller
cookie=0xd dl_src=60:66:66:66:66:66 actions=pop_mpls:0x0800,controller
cookie=0xd dl_src=60:66:66:66:66:67
actions=copy_ttl_in,dec_ttl,pop_mpls:0x0800,dec_ttl,controller
cookie=0xc dl_src=70:77:77:77:77:77
actions=push_mpls:0x8848,load:1000->OXM_OF_MPLS_LABEL[[]],load:7->OXM_OF_MPLS_TC[[]],controller
--
1.7.10.4
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
