ofproto-dpif-xlate: Add support for OpenFlow group chaining.

Requested-by: Vinllen Chen <cvinllen@gmail.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Alex Wang <alexw@nicira.com>

diff --git a/AUTHORS b/AUTHORS
index d439fda..2240576 100644 (file)
--- a/AUTHORS
+++ b/AUTHORS
@@ -356,6 +356,7 @@ Torbjorn Tornkvist      kruskakli@gmail.com
 Valentin Bud            valentin@hackaserver.com
 Vasiliy Tolstov         v.tolstov@selfip.ru
 Vasu Dasari             vdasari@gmail.com
+Vinllen Chen            cvinllen@gmail.com
 Vishal Swarankar        vishal.swarnkar@gmail.com
 Vjekoslav Brajkovic     balkan@cs.washington.edu
 Voravit T.              voravit@kth.se

diff --git a/NEWS b/NEWS
index e9d1afa..a0664dc 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -1,5 +1,8 @@
 Post-v2.4.0
 ---------------------
+   - OpenFlow:
+     * Group chaining (where one OpenFlow group triggers another) is
+       now supported.
 
 
 v2.4.0 - xx xxx xxxx

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index d3ae567..8c68273 100644 (file)
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -3330,6 +3330,7 @@ xlate_select_group(struct xlate_ctx *ctx, struct group_dpif *group)
 static void
 xlate_group_action__(struct xlate_ctx *ctx, struct group_dpif *group)
 {
+    bool was_in_group = ctx->in_group;
     ctx->in_group = true;
 
     switch (group_dpif_get_type(group)) {
@@ -3348,37 +3349,13 @@ xlate_group_action__(struct xlate_ctx *ctx, struct group_dpif *group)
     }
     group_dpif_unref(group);
 
-    ctx->in_group = false;
-}
-
-static bool
-xlate_group_resource_check(struct xlate_ctx *ctx)
-{
-    if (!xlate_resubmit_resource_check(ctx)) {
-        return false;
-    } else if (ctx->in_group) {
-        /* Prevent nested translation of OpenFlow groups.
-         *
-         * OpenFlow allows this restriction.  We enforce this restriction only
-         * because, with the current architecture, we would otherwise have to
-         * take a possibly recursive read lock on the ofgroup rwlock, which is
-         * unsafe given that POSIX allows taking a read lock to block if there
-         * is a thread blocked on taking the write lock.  Other solutions
-         * without this restriction are also possible, but seem unwarranted
-         * given the current limited use of groups. */
-        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
-
-        VLOG_ERR_RL(&rl, "cannot recursively translate OpenFlow group");
-        return false;
-    } else {
-        return true;
-    }
+    ctx->in_group = was_in_group;
 }
 
 static bool
 xlate_group_action(struct xlate_ctx *ctx, uint32_t group_id)
 {
-    if (xlate_group_resource_check(ctx)) {
+    if (xlate_resubmit_resource_check(ctx)) {
         struct group_dpif *group;
         bool got_group;
 

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 8e68b0a..05a80b7 100644 (file)
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -4105,19 +4105,6 @@ static enum ofperr
 group_construct(struct ofgroup *group_)
 {
     struct group_dpif *group = group_dpif_cast(group_);
-    const struct ofputil_bucket *bucket;
-
-    /* Prevent group chaining because our locking structure makes it hard to
-     * implement deadlock-free.  (See xlate_group_resource_check().) */
-    LIST_FOR_EACH (bucket, list_node, &group->up.buckets) {
-        const struct ofpact *a;
-
-        OFPACT_FOR_EACH (a, bucket->ofpacts, bucket->ofpacts_len) {
-            if (a->type == OFPACT_GROUP) {
-                return OFPERR_OFPGMFC_CHAINING_UNSUPPORTED;
-            }
-        }
-    }
 
     ovs_mutex_init_adaptive(&group->stats_mutex);
     ovs_mutex_lock(&group->stats_mutex);

diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index bd1b21c..7f20786 100644 (file)
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -294,13 +294,15 @@ Datapath actions: 10,set(tcp(src=91)),11
 OVS_VSWITCHD_STOP
 AT_CLEANUP
 
-AT_SETUP([ofproto-dpif - group chaining not supported])
+AT_SETUP([ofproto-dpif - group chaining])
 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,group:123,bucket=output:11'],
-  [1], [], [stderr])
-AT_CHECK([STRIP_XIDS stderr | sed 1q], [0],
-  [OFPT_ERROR (OF1.2): OFPGMFC_CHAINING_UNSUPPORTED
+AT_CHECK([ovs-ofctl -O OpenFlow12 add-group br0 'group_id=1234,type=all,bucket=set_field:192.168.3.90->ip_src,group:123,bucket=output:11'])
+AT_CHECK([ovs-ofctl -O OpenFlow12 add-group br0 'group_id=123,type=all,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: set(ipv4(src=192.168.3.90)),10,set(ipv4(src=192.168.0.1)),11
 ])
 OVS_VSWITCHD_STOP
 AT_CLEANUP