This patch could still use a review.
On Fri, Jun 05, 2015 at 10:45:35PM -0700, Ben Pfaff wrote: > Requested-by: Vinllen Chen <[email protected]> > Signed-off-by: Ben Pfaff <[email protected]> > --- > AUTHORS | 1 + > ofproto/ofproto-dpif-xlate.c | 29 +++-------------------------- > ofproto/ofproto-dpif.c | 13 ------------- > tests/ofproto-dpif.at | 12 +++++++----- > 4 files changed, 11 insertions(+), 44 deletions(-) > > diff --git a/AUTHORS b/AUTHORS > index 28899a8..d600a90 100644 > --- a/AUTHORS > +++ b/AUTHORS > @@ -352,6 +352,7 @@ Torbjorn Tornkvist [email protected] > Valentin Bud [email protected] > Vasiliy Tolstov [email protected] > Vasu Dasari [email protected] > +Vinllen Chen [email protected] > Vishal Swarankar [email protected] > Vjekoslav Brajkovic [email protected] > Voravit T. [email protected] > diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c > index 71b8bef..933191e 100644 > --- a/ofproto/ofproto-dpif-xlate.c > +++ b/ofproto/ofproto-dpif-xlate.c > @@ -3280,6 +3280,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)) { > @@ -3298,37 +3299,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 22e5d5f..d913f33 100644 > --- a/ofproto/ofproto-dpif.c > +++ b/ofproto/ofproto-dpif.c > @@ -4045,19 +4045,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 b5a9ad9..4784a38 100644 > --- 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 > -- > 2.1.3 > _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
