[ovs-dev] [PATCH v5 3/3] Translation of indirect and all groups
Simon Horman
horms at verge.net.au
Thu Oct 10 00:15:33 UTC 2013
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 at verge.net.au>
---
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 | 101 ++++++++++++++++++++++++++++++++++++++++++-
ofproto/ofproto-dpif.c | 34 +++++++++++++++
ofproto/ofproto-dpif.h | 10 +++++
ofproto/ofproto.c | 2 +
tests/ofproto-dpif.at | 48 ++++++++++++++++++++
5 files changed, 194 insertions(+), 1 deletion(-)
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index a5db2d7..683042f 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -180,6 +180,7 @@ struct xlate_ctx {
uint16_t user_cookie_offset;/* Used for user_action_cookie fixup. */
bool exit; /* No further actions should be processed. */
+ bool xlating_action_set; /* True if the action set is being translated. */
struct list action_set; /* Action set.
* An ordered list of struct ofpbuf
* containing ofpacts to add to the action. */
@@ -1770,6 +1771,99 @@ xlate_table_action(struct xlate_ctx *ctx,
}
static void
+xlate_group_bucket_set(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_group_bucket(struct xlate_ctx *ctx, struct ofputil_bucket *bucket)
+{
+ if (ctx->xlating_action_set) {
+ xlate_group_bucket_set(ctx, bucket);
+ } else {
+ do_xlate_actions(bucket->ofpacts, bucket->ofpacts_len, ctx);
+ }
+}
+
+static void
+xlate_all_group(struct xlate_ctx *ctx, struct group_dpif *group)
+{
+ struct ofputil_bucket *bucket;
+ 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:
+ default:
+ /* XXX not yet implemented */
+ break;
+ }
+ group_dpif_release(group);
+}
+
+static bool
+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 {
+ return true;
+ }
+ } 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);
+ return true;
+ }
+
+ return false;
+}
+
+static void
xlate_ofpact_resubmit(struct xlate_ctx *ctx,
const struct ofpact_resubmit *resubmit)
{
@@ -2290,7 +2384,9 @@ xlate_action_set(struct xlate_ctx *ctx)
ofpacts_list_to_action_set(&ofpact_buf, &ctx->action_set);
action_set_clear(ctx);
+ ctx->xlating_action_set = true;
do_xlate_actions(ofpact_buf.data, ofpact_buf.size, ctx);
+ ctx->xlating_action_set = false;
ofpbuf_uninit(&ofpact_buf);
}
@@ -2318,7 +2414,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:
@@ -2790,6 +2888,7 @@ xlate_actions__(struct xlate_in *xin, struct xlate_out *xout)
ctx.orig_skb_priority = flow->skb_priority;
ctx.table_id = 0;
ctx.exit = false;
+ ctx.xlating_action_set = false;
ctx.mpls_depth_delta = 0;
list_init(&ctx.action_set);
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 6a4149f..8bfaf48 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -4904,6 +4904,40 @@ 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(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 0863efd..a5362ee 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.
*
@@ -87,6 +88,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(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 8e4f300..1709fcc 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -523,6 +523,8 @@ 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.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 75abcbf..02184ac 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -61,6 +61,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: 10,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])
--
1.8.4
More information about the dev
mailing list