ofproto-dpif: Translation of indirect and all groups

Allow translation of indirect and all groups.  Also allow insertion of
indirect and all groups by changing the maximum permitted number in the
groups table from 0 to OFPG_MAX.

Implementation note:

After translating the actions for each bucket ctx->flow is reset to its
state prior to translation of the buckets actions. This is equivalent to
cloning the bucket before applying actions. This is my interpretation of the
OpenFlow 1.3.2 specification section 5.6.1 Group Types, which includes the
following text. I believe there is room for other interpretations.

* On all groups: "The packet is effectively cloned for each bucket; one
  packet is processed for each bucket of the group."
* On indirect groups: "This group type is effectively identical to an
  all group with one bucket."

Signed-off-by: Simon Horman <horms@verge.net.au>
Signed-off-by: Ben Pfaff <blp@nicira.com>

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index d300ec5..75ba53c 100644 (file)
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -1786,6 +1786,81 @@ xlate_table_action(struct xlate_ctx *ctx,
     ctx->exit = true;
 }
 
+static void
+xlate_group_bucket(struct xlate_ctx *ctx, const struct ofputil_bucket *bucket)
+{
+    uint64_t action_list_stub[1024 / 8];
+    struct ofpbuf action_list, action_set;
+
+    ofpbuf_use_const(&action_set, bucket->ofpacts, bucket->ofpacts_len);
+    ofpbuf_use_stub(&action_list, action_list_stub, sizeof action_list_stub);
+
+    ofpacts_execute_action_set(&action_list, &action_set);
+    ctx->recurse++;
+    do_xlate_actions(action_list.data, action_list.size, ctx);
+    ctx->recurse--;
+
+    ofpbuf_uninit(&action_set);
+    ofpbuf_uninit(&action_list);
+}
+
+static void
+xlate_all_group(struct xlate_ctx *ctx, struct group_dpif *group)
+{
+    const struct ofputil_bucket *bucket;
+    const struct list *buckets;
+    struct flow old_flow = ctx->xin->flow;
+
+    group_dpif_get_buckets(group, &buckets);
+
+    LIST_FOR_EACH (bucket, list_node, buckets) {
+        xlate_group_bucket(ctx, bucket);
+        /* Roll back flow to previous state.
+         * This is equivalent to cloning the packet for each bucket.
+         *
+         * As a side effect any subsequently applied actions will
+         * also effectively be applied to a clone of the packet taken
+         * just before applying the all or indirect group. */
+        ctx->xin->flow = old_flow;
+    }
+}
+
+static void
+xlate_group_action__(struct xlate_ctx *ctx, struct group_dpif *group)
+{
+    switch (group_dpif_get_type(group)) {
+    case OFPGT11_ALL:
+    case OFPGT11_INDIRECT:
+        xlate_all_group(ctx, group);
+        break;
+    case OFPGT11_SELECT:
+    case OFPGT11_FF:
+        /* XXX not yet implemented */
+        break;
+    default:
+        NOT_REACHED();
+    }
+    group_dpif_release(group);
+}
+
+static bool
+xlate_group_action(struct xlate_ctx *ctx, uint32_t group_id)
+{
+    if (xlate_resubmit_resource_check(ctx)) {
+        struct group_dpif *group;
+        bool got_group;
+
+        got_group = group_dpif_lookup(ctx->xbridge->ofproto, group_id, &group);
+        if (got_group) {
+            xlate_group_action__(ctx, group);
+        } else {
+            return true;
+        }
+    }
+
+    return false;
+}
+
 static void
 xlate_ofpact_resubmit(struct xlate_ctx *ctx,
                       const struct ofpact_resubmit *resubmit)
@@ -2365,7 +2440,9 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,
             break;
 
         case OFPACT_GROUP:
-            /* XXX not yet implemented */
+            if (xlate_group_action(ctx, ofpact_get_GROUP(a)->group_id)) {
+                return;
+            }
             break;
 
         case OFPACT_CONTROLLER:

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 277074c..341a63c 100644 (file)
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -4907,6 +4907,41 @@ group_get_stats(const struct ofgroup *group_, struct ofputil_group_stats *ogs)
 
     return 0;
 }
+
+bool
+group_dpif_lookup(struct ofproto_dpif *ofproto, uint32_t group_id,
+                  struct group_dpif **group)
+    OVS_TRY_RDLOCK(true, (*group)->up.rwlock)
+{
+    struct ofgroup *ofgroup;
+    bool found;
+
+    *group = NULL;
+    found = ofproto_group_lookup(&ofproto->up, group_id, &ofgroup);
+    *group = found ?  group_dpif_cast(ofgroup) : NULL;
+
+    return found;
+}
+
+void
+group_dpif_release(struct group_dpif *group)
+    OVS_RELEASES(group->up.rwlock)
+{
+    ofproto_group_release(&group->up);
+}
+
+void
+group_dpif_get_buckets(const struct group_dpif *group,
+                       const struct list **buckets)
+{
+    *buckets = &group->up.buckets;
+}
+
+enum ofp11_group_type
+group_dpif_get_type(const struct group_dpif *group)
+{
+    return group->up.type;
+}
 \f
 /* Sends 'packet' out 'ofport'.
  * May modify 'packet'.

diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h
index c93d37c..51cb38f 100644 (file)
--- a/ofproto/ofproto-dpif.h
+++ b/ofproto/ofproto-dpif.h
@@ -32,6 +32,7 @@ struct ofproto_packet_in;
 struct ofport_dpif;
 struct dpif_backer;
 struct OVS_LOCKABLE rule_dpif;
+struct OVS_LOCKABLE group_dpif;
 
 /* Ofproto-dpif -- DPIF based ofproto implementation.
  *
@@ -89,6 +90,15 @@ void choose_miss_rule(enum ofputil_port_config,
                       struct rule_dpif *no_packet_in_rule,
                       struct rule_dpif **rule);
 
+bool group_dpif_lookup(struct ofproto_dpif *ofproto, uint32_t group_id,
+                       struct group_dpif **group);
+
+void group_dpif_release(struct group_dpif *group);
+
+void group_dpif_get_buckets(const struct group_dpif *group,
+                            const struct list **buckets);
+enum ofp11_group_type group_dpif_get_type(const struct group_dpif *group);
+
 bool ofproto_has_vlan_splinters(const struct ofproto_dpif *);
 ofp_port_t vsp_realdev_to_vlandev(const struct ofproto_dpif *,
                                   ofp_port_t realdev_ofp_port,

diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index b626a6c..d123e7c 100644 (file)
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -531,6 +531,9 @@ ofproto_create(const char *datapath_name, const char *datapath_type,
     ovs_rwlock_init(&ofproto->groups_rwlock);
     hmap_init(&ofproto->groups);
     ovs_mutex_unlock(&ofproto_mutex);
+    ofproto->ogf.capabilities = OFPGFC_CHAINING;
+    ofproto->ogf.max_groups[OFPGT11_ALL] = OFPG_MAX;
+    ofproto->ogf.max_groups[OFPGT11_INDIRECT] = OFPG_MAX;
     ofproto->ogf.actions[0] =
 #define OFPAT11_ACTION(ENUM, STRUCT, EXTENSIBLE, NAME) (1 << ENUM) |
 #include "ofp-util.def"

diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index 726ec99..5ba4571 100644 (file)
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -65,6 +65,54 @@ AT_CHECK([tail -1 stdout], [0],
 OVS_VSWITCHD_STOP
 AT_CLEANUP
 
+AT_SETUP([ofproto-dpif - all group in action list])
+OVS_VSWITCHD_START
+ADD_OF_PORTS([br0], [1], [10], [11])
+AT_CHECK([ovs-ofctl -O OpenFlow12 add-group br0 'group_id=1234,type=all,bucket=output:10,set_field:192.168.3.90->ip_src,bucket=output:11'])
+AT_CHECK([ovs-ofctl -O OpenFlow12 add-flow br0 'ip actions=group:1234'])
+AT_CHECK([ovs-appctl ofproto/trace br0 'in_port=1,dl_src=50:54:00:00:00:05,dl_dst=50:54:00:00:00:07,dl_type=0x0800,nw_src=192.168.0.1,nw_dst=192.168.0.2,nw_proto=1,nw_tos=0,nw_ttl=128,icmp_type=8,icmp_code=0'], [0], [stdout])
+AT_CHECK([tail -1 stdout], [0],
+  [Datapath actions: set(ipv4(src=192.168.3.90,dst=192.168.0.2,proto=1,tos=0,ttl=128,frag=no)),10,set(ipv4(src=192.168.0.1,dst=192.168.0.2,proto=1,tos=0,ttl=128,frag=no)),11
+])
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
+AT_SETUP([ofproto-dpif - indirect group in action list])
+OVS_VSWITCHD_START
+ADD_OF_PORTS([br0], [1], [10])
+AT_CHECK([ovs-ofctl -O OpenFlow12 add-group br0 group_id=1234,type=indirect,bucket=output:10])
+AT_CHECK([ovs-ofctl -O OpenFlow12 add-flow br0 'ip actions=group:1234'])
+AT_CHECK([ovs-appctl ofproto/trace br0 'in_port=1,dl_src=50:54:00:00:00:05,dl_dst=50:54:00:00:00:07,dl_type=0x0800,nw_src=192.168.0.1,nw_dst=192.168.0.2,nw_proto=1,nw_tos=0,nw_ttl=128,icmp_type=8,icmp_code=0'], [0], [stdout])
+AT_CHECK([tail -1 stdout], [0],
+  [Datapath actions: 10
+])
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
+AT_SETUP([ofproto-dpif - all group in action set])
+OVS_VSWITCHD_START
+ADD_OF_PORTS([br0], [1], [10], [11])
+AT_CHECK([ovs-ofctl -O OpenFlow12 add-group br0 'group_id=1234,type=all,bucket=output:10,set_field:192.168.3.90->ip_src,bucket=output:11'])
+AT_CHECK([ovs-ofctl -O OpenFlow12 add-flow br0 'ip actions=write_actions(group:1234)'])
+AT_CHECK([ovs-appctl ofproto/trace br0 'in_port=1,dl_src=50:54:00:00:00:05,dl_dst=50:54:00:00:00:07,dl_type=0x0800,nw_src=192.168.0.1,nw_dst=192.168.0.2,nw_proto=1,nw_tos=0,nw_ttl=128,icmp_type=8,icmp_code=0'], [0], [stdout])
+AT_CHECK([tail -1 stdout], [0],
+  [Datapath actions: set(ipv4(src=192.168.3.90,dst=192.168.0.2,proto=1,tos=0,ttl=128,frag=no)),10,set(ipv4(src=192.168.0.1,dst=192.168.0.2,proto=1,tos=0,ttl=128,frag=no)),11
+])
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
+AT_SETUP([ofproto-dpif - indirect group in action set])
+OVS_VSWITCHD_START
+ADD_OF_PORTS([br0], [1], [10])
+AT_CHECK([ovs-ofctl -O OpenFlow12 add-group br0 group_id=1234,type=indirect,bucket=output:10])
+AT_CHECK([ovs-ofctl -O OpenFlow12 add-flow br0 'ip actions=write_actions(group:1234)'])
+AT_CHECK([ovs-appctl ofproto/trace br0 'in_port=1,dl_src=50:54:00:00:00:05,dl_dst=50:54:00:00:00:07,dl_type=0x0800,nw_src=192.168.0.1,nw_dst=192.168.0.2,nw_proto=1,nw_tos=0,nw_ttl=128,icmp_type=8,icmp_code=0'], [0], [stdout])
+AT_CHECK([tail -1 stdout], [0],
+  [Datapath actions: 10
+])
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
 AT_SETUP([ofproto-dpif - registers])
 OVS_VSWITCHD_START
 ADD_OF_PORTS([br0], [20], [21], [22], [33], [90])

diff --git a/tests/ofproto.at b/tests/ofproto.at
index 68dada9..5adb421 100644 (file)
--- a/tests/ofproto.at
+++ b/tests/ofproto.at
@@ -183,7 +183,7 @@ AT_CHECK([STRIP_XIDS stdout], [0], [dnl
 OFPST_GROUP_FEATURES reply (OF1.2):
  Group table:
     Types:  0x0
-    Capabilities:  0x0
+    Capabilities:  0x4
 ])
 OVS_VSWITCHD_STOP
 AT_CLEANUP