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->base_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 <ho...@verge.net.au>

---
v3
* Rebase for "ofproto-dpif: Hide struct rule_dpif internally"
* Update for hidden group_dpif
* Corrected typo in all groups test

v2
* First post
---
 ofproto/ofproto-dpif-xlate.c | 116 +++++++++++++++++++++++++++++++++++++------
 ofproto/ofproto-dpif.c       |  36 ++++++++++++++
 ofproto/ofproto-dpif.h       |  11 ++++
 ofproto/ofproto.c            |   2 +
 tests/ofproto-dpif.at        |  24 +++++++++
 5 files changed, 175 insertions(+), 14 deletions(-)

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 359aa10..9704ecd 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -1693,6 +1693,24 @@ xlate_recursively(struct xlate_ctx *ctx, struct 
rule_dpif *rule)
 }
 
 static void
+xlate_miss(struct xlate_ctx *ctx)
+{
+        struct xport *xport;
+        struct rule_dpif *rule;
+
+        /* XXX
+         * check if table configuration flags
+         * OFPTC_TABLE_MISS_CONTROLLER, default.
+         * OFPTC_TABLE_MISS_CONTINUE,
+         * OFPTC_TABLE_MISS_DROP
+         * When OF1.0, OFPTC_TABLE_MISS_CONTINUE is used. What to do? */
+        xport = get_ofp_port(ctx->xbridge, ctx->xin->flow.in_port.ofp_port);
+        choose_miss_rule(xport ? xport->config : 0, ctx->xbridge->miss_rule,
+                         ctx->xbridge->no_packet_in_rule, &rule);
+        xlate_recursively(ctx, rule);
+}
+
+static void
 xlate_table_action(struct xlate_ctx *ctx,
                    ofp_port_t in_port, uint8_t table_id, bool may_packet_in)
 {
@@ -1720,19 +1738,7 @@ xlate_table_action(struct xlate_ctx *ctx,
         if (got_rule) {
             xlate_recursively(ctx, rule);
         } else if (may_packet_in) {
-            struct xport *xport;
-
-            /* XXX
-             * check if table configuration flags
-             * OFPTC_TABLE_MISS_CONTROLLER, default.
-             * OFPTC_TABLE_MISS_CONTINUE,
-             * OFPTC_TABLE_MISS_DROP
-             * When OF1.0, OFPTC_TABLE_MISS_CONTINUE is used. What to do? */
-            xport = get_ofp_port(ctx->xbridge, 
ctx->xin->flow.in_port.ofp_port);
-            choose_miss_rule(xport ? xport->config : 0,
-                             ctx->xbridge->miss_rule,
-                             ctx->xbridge->no_packet_in_rule, &rule);
-            xlate_recursively(ctx, rule);
+            xlate_miss(ctx);
         }
 
         ctx->table_id = old_table_id;
@@ -1745,6 +1751,88 @@ xlate_table_action(struct xlate_ctx *ctx,
 }
 
 static void
+xlate_group_bucket(struct xlate_ctx *ctx, struct ofputil_bucket *bucket)
+{
+    struct ofpbuf actions_out, actions_in_buf;
+    struct list actions_in = LIST_INITIALIZER(&actions_in);
+
+    ofpbuf_use_const(&actions_in_buf, bucket->ofpacts, bucket->ofpacts_len);
+    list_init(&actions_in_buf.list_node);
+    list_insert(&actions_in, &actions_in_buf.list_node);
+
+    ofpbuf_init(&actions_out, actions_in_buf.size);
+
+    ofpacts_list_to_action_set(&actions_out, &actions_in);
+    ctx->recurse++;
+    do_xlate_actions(actions_out.data, actions_out.size, ctx);
+    ctx->recurse--;
+
+    ofpbuf_uninit(&actions_out);
+    ofpbuf_uninit(&actions_in_buf);
+}
+
+static void
+xlate_all_group(struct xlate_ctx *ctx, struct group_dpif *group)
+    OVS_REQ_RDLOCK(&group->up.rwlock)
+{
+    struct ofputil_bucket *bucket;
+    struct list *buckets;
+    struct flow old_base_flow = ctx->base_flow;
+
+    group_dpif_get_buckets(group, &buckets);
+
+    LIST_FOR_EACH(bucket, list_node, buckets) {
+        xlate_group_bucket(ctx, bucket);
+        /* Roll back base_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->base_flow = old_base_flow;
+    }
+}
+
+static void
+xlate_group_action__(struct xlate_ctx *ctx, struct group_dpif *group)
+    OVS_RELEASES(&group->up.rwlock)
+{
+    switch (group_dpif_get_type(group)) {
+    case OFPGT11_ALL:
+    case OFPGT11_INDIRECT:
+        xlate_all_group(ctx, group);
+        break;
+    case OFPGT11_SELECT:
+    case OFPGT11_FF:
+    default:
+        /* XXX not yet implemented */
+        break;
+    }
+    group_dpif_release(group);
+}
+
+static void
+xlate_group_action(struct xlate_ctx *ctx, uint32_t group_id)
+{
+    if (ctx->recurse < MAX_RESUBMIT_RECURSION) {
+        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 {
+            xlate_miss(ctx);
+        }
+    } else {
+        static struct vlog_rate_limit recurse_rl = VLOG_RATE_LIMIT_INIT(1, 1);
+
+        VLOG_ERR_RL(&recurse_rl, "resubmit actions recursed over %d times",
+                    MAX_RESUBMIT_RECURSION);
+    }
+}
+
+static void
 xlate_ofpact_resubmit(struct xlate_ctx *ctx,
                       const struct ofpact_resubmit *resubmit)
 {
@@ -2238,7 +2326,7 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t 
ofpacts_len,
             break;
 
         case OFPACT_GROUP:
-            /* XXX not yet implemented */
+            xlate_group_action(ctx, ofpact_get_GROUP(a)->group_id);
             break;
 
         case OFPACT_CONTROLLER:
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 22d44b8..3b84793 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -5159,6 +5159,42 @@ 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_NO_THREAD_SAFETY_ANALYSIS
+{
+    if (group) {
+        ofproto_group_release(&group->up);
+    }
+}
+
+void
+group_dpif_get_buckets(const struct group_dpif *group, struct list **buckets)
+{
+    *buckets = &group->up.buckets;
+}
+
+enum ofp11_group_type
+group_dpif_get_type(const struct group_dpif *group)
+{
+    return group->up.type;
+}
 
 /* Sends 'packet' out 'ofport'.
  * May modify 'packet'.
diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h
index eb724d5..1808ec6 100644
--- a/ofproto/ofproto-dpif.h
+++ b/ofproto/ofproto-dpif.h
@@ -31,6 +31,7 @@ struct ofproto_dpif;
 struct ofport_dpif;
 struct dpif_backer;
 struct OVS_LOCKABLE rule_dpif;
+struct OVS_LOCKABLE group_dpif;
 
 /* Ofproto-dpif -- DPIF based ofproto implementation.
  *
@@ -91,6 +92,16 @@ void choose_miss_rule(enum ofputil_port_config,
                       struct rule_dpif **rule)
     OVS_ACQ_RDLOCK(*rule);
 
+bool group_dpif_lookup(struct ofproto_dpif *ofproto, uint32_t group_id,
+                       struct group_dpif **group)
+    OVS_TRY_RDLOCK(true, *group);
+
+void group_dpif_release(struct group_dpif *group) OVS_RELEASES(group);
+
+void group_dpif_get_buckets(const struct group_dpif *group,
+                            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 b94fd8e..6713a8a 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -460,6 +460,8 @@ ofproto_create(const char *datapath_name, const char 
*datapath_type,
     ofproto->min_mtu = INT_MAX;
     ovs_rwlock_init(&ofproto->groups_rwlock);
     hmap_init(&ofproto->groups);
+    ofproto->ogf.max_groups[OFPGT11_ALL] = OFPG_MAX;
+    ofproto->ogf.max_groups[OFPGT11_INDIRECT] = OFPG_MAX;
 
     error = ofproto->ofproto_class->construct(ofproto);
     if (error) {
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index 56cc775..582e679 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -61,6 +61,30 @@ AT_CHECK([tail -1 stdout], [0],
 OVS_VSWITCHD_STOP
 AT_CLEANUP
 
+AT_SETUP([ofproto-dpif - all group])
+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.3.90,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])
+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 - registers])
 OVS_VSWITCHD_START
 ADD_OF_PORTS([br0], [20], [21], [22], [33], [90])
-- 
1.8.4

_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to