[ovs-dev] ofp-actions.c: expand ofpacts_output_to_group to cover Write Actions that include Group action
diff mbox

Message ID OFD287FC2B.3AB649A1-ON88257EB3.00733A88-87257EB3.007719F7@selinc.com
State Superseded
Headers show

Commit Message

gavin_remaley@selinc.com Sept. 1, 2015, 9:40 p.m. UTC
When a Group is deleted, all Flows that reference the deleted Group should
also be deleted (automatically).  Flows that reference a Group via Write
Actions are not found by the function ofpacts_output_to_group and are
therefore not deleted.

Testing: I only tested that the patch fixed the described problem.  I did 
no
other testing to verify that it could not possibly break anything.  There
appears to be only one call to the changed function so probability seems
low.

Signed-off-by: Gavin Remaley <gavin_remaley@selinc.com>

---
 lib/ofp-actions.c | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

         }
     }

Comments

Ben Pfaff Sept. 8, 2015, 10:26 p.m. UTC | #1
On Tue, Sep 01, 2015 at 03:40:53PM -0600, gavin_remaley@selinc.com wrote:
> When a Group is deleted, all Flows that reference the deleted Group should
> also be deleted (automatically).  Flows that reference a Group via Write
> Actions are not found by the function ofpacts_output_to_group and are
> therefore not deleted.
> 
> Testing: I only tested that the patch fixed the described problem.  I did 
> no
> other testing to verify that it could not possibly break anything.  There
> appears to be only one call to the changed function so probability seems
> low.
> 
> Signed-off-by: Gavin Remaley <gavin_remaley@selinc.com>

We can do this a little more simply, and at the same time fix the same
problem for out_port.  I sent out a patch series, starting here:
        http://openvswitch.org/pipermail/dev/2015-September/059719.html

Patch
diff mbox

diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index ea47ff9..b575cbc 100644
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -3183,9 +3183,26 @@  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) {
+            /* Get the nested actions */
+            const struct ofpact_nest *nested_actions = 
ofpact_get_WRITE_ACTIONS(a);
+            struct ofpact const *nested;
+ 
+            /* Loop through all the nested actions and check for Group */
+            OFPACT_FOR_EACH (nested,
+                             nested_actions->actions,
+                             ofpact_nest_get_action_len(nested_actions)) 
{
+                if (nested->type == OFPACT_GROUP &&
+                    ofpact_get_GROUP(nested)->group_id == group_id) {
+                    return true;
+                }
+ 
+            }