[ovs-dev] [PATCH 1/3] ofp-actions: Look inside write_actions for output ports and groups.

gavin_remaley at selinc.com gavin_remaley at selinc.com
Fri Sep 18 20:02:40 UTC 2015


This patch looks good, though it looks like the general issue with 
examining the Actions within a Write Actions instruction has several 
instances beyond what is addressed by this patch.

I have noticed that an invalid Group ID within Write Actions is not caught 
(Flow is programmed with no error).  That appears to be a problem in 
ofproto.c -> ofproto_check_ofpacts.  A similar fix seemed to address that 
issue.

I did a global search for OFPACT_FOR_EACH and examined the action 
processing.  I see several other potential problems with Write Actions.

ofp-actions.c -> ofpacts_check
ofp-actions.c -> ofpacts_get_meter
ofproto.c -> ofproto_check_ofpacts

Thanks for your help.



From:   Ben Pfaff <blp at nicira.com>
To:     dev at openvswitch.org
Cc:     Ben Pfaff <blp at nicira.com>, Gavin Remaley 
<gavin_remaley at selinc.com>
Date:   09/08/2015 04:55 PM
Subject:        [PATCH 1/3] ofp-actions: Look inside write_actions for 
output ports and groups.



The out_port and out_group matches only looked at apply_actions
instructions, but my interpretation of the OpenFlow spec is that they
should also look inside write_actions.

Reported-by: Gavin Remaley <gavin_remaley at selinc.com>
Signed-off-by: Ben Pfaff <blp at nicira.com>
---
 AUTHORS           |  1 +
 lib/ofp-actions.c | 21 +++++++++++++++++----
 2 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/AUTHORS b/AUTHORS
index a7f40bb..0c4d020 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -259,6 +259,7 @@ Eivind Bulie Haanaes
 Eric Lopez              elopez at nicira.com
 Frido Roose             fr.roose at gmail.com
 Gaetano Catalli         gaetano.catalli at gmail.com
+Gavin Remaley           gavin_remaley at selinc.com
 George Shuklin          amarao at desunote.ru
 Gerald Rogers           gerald.rogers at intel.com
 Ghanem Bahri            bahri.ghanem at gmail.com
diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index c46ce97..ee686c1 100644
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -6072,6 +6072,12 @@ ofpact_outputs_to_port(const struct ofpact *ofpact, 
ofp_port_t port)
         return ofpact_get_ENQUEUE(ofpact)->port == port;
     case OFPACT_CONTROLLER:
         return port == OFPP_CONTROLLER;
+    case OFPACT_WRITE_ACTIONS: {
+        const struct ofpact_nest *nested = 
ofpact_get_WRITE_ACTIONS(ofpact);
+        return ofpacts_output_to_port(nested->actions,
+                                      ofpact_nest_get_action_len(nested),
+                                      port);
+    }
 
     case OFPACT_OUTPUT_REG:
     case OFPACT_BUNDLE:
@@ -6113,7 +6119,6 @@ ofpact_outputs_to_port(const struct ofpact *ofpact, 
ofp_port_t port)
     case OFPACT_POP_MPLS:
     case OFPACT_SAMPLE:
     case OFPACT_CLEAR_ACTIONS:
-    case OFPACT_WRITE_ACTIONS:
     case OFPACT_GOTO_TABLE:
     case OFPACT_METER:
     case OFPACT_GROUP:
@@ -6149,9 +6154,17 @@ ofpacts_output_to_group(const struct ofpact 
*ofpacts, size_t ofpacts_len,
     const struct ofpact *a;
 
     OFPACT_FOR_EACH (a, ofpacts, ofpacts_len) {
-        if (a->type == OFPACT_GROUP
-            && ofpact_get_GROUP(a)->group_id == group_id) {
-            return true;
+        if (a->type == OFPACT_GROUP) {
+            if (ofpact_get_GROUP(a)->group_id == group_id) {
+                return true;
+            }
+        } else if (a->type == OFPACT_WRITE_ACTIONS) {
+            const struct ofpact_nest *nested = 
ofpact_get_WRITE_ACTIONS(a);
+            if (ofpacts_output_to_group(nested->actions,
+ ofpact_nest_get_action_len(nested),
+                                        group_id)) {
+                return true;
+            }
         }
     }
 
-- 
2.1.3





More information about the dev mailing list