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 <ho...@verge.net.au>
---
v8
* As suggested by Ben Pfaff
- Treat actions as a set regardless of the context in which
they are called. There is some room for interpretation of
the spec in this area but this simpler approach
is being taken for now. This reverts the behaviour
of this patch in this regard to that of v3.
- Reduce alignment of action_list_stub in xlate_group_bucket()
from 64 to 8.
- Correct whitespace in xlate_all_group()
* Tweak select statement to call NOT_REACHED() if default case
is reached: it shouldn't be under any circumstances.
* Set OFPGFC_CHAINING bit of ofproto->ogf.capabilities in
ofproto_create() as this feature is supported by this patch.
v7
* Rebase for Ben Pfaff's rework of
"Add support for write-actions"
v6
* Make parameters of group_dpif_get_buckets() const
* Make bucket parameter of xlate_group_bucket* const
* Use consistent quoting stile in tests
v5
* No change
v4
* Rebase
* As suggested by Jarno Rajahalme
- Stop translation of actions if looking up the
group of a group actions fails. Previously an attempt
was made to send a packet_in message instead.
- Process action buckets of groups for write list as a list of actions
rather than a set. The action bucket should only be treated as a set if
the group action is part of the action set
* group parameter of group_dpif_get_buckets() should not
be const as the address of one of its fields is assigned
to another parameter.
* Correct cloning logic in xlate_all_group().
It is the flow that should be reset not the base_flow.
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 | 79 +++++++++++++++++++++++++++++++++++++++++++-
ofproto/ofproto-dpif.c | 35 ++++++++++++++++++++
ofproto/ofproto-dpif.h | 10 ++++++
ofproto/ofproto.c | 3 ++
tests/ofproto-dpif.at | 48 +++++++++++++++++++++++++++
tests/ofproto.at | 2 +-
6 files changed, 175 insertions(+), 2 deletions(-)
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 735d96b..287bdce 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -1787,6 +1787,81 @@ xlate_table_action(struct xlate_ctx *ctx,
}
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)
{
@@ -2315,7 +2390,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
--- 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;
+}
/* Sends 'packet' out 'ofport'.
* May modify 'packet'.
diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h
index c93d37c..51cb38f 100644
--- 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 229fc55..719aff0 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -528,6 +528,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) ENUM |
#include "ofp-util.def"
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index 9e0ea4b..d99b190 100644
--- 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 03ad346..3652cf2 100644
--- 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
--
1.8.4
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
