Thanks, applied to master.

On Mon, Jun 22, 2015 at 09:38:52AM -0700, Alex Wang wrote:
> Acked-by: Alex Wang <[email protected]>
> 
> On Mon, Jun 22, 2015 at 8:40 AM, Ben Pfaff <[email protected]> wrote:
> 
> > 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
> >
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev

Reply via email to