[ovs-dev] ofproto-dpif-xlate: Fix translation of groups with no buckets.

Message ID 20180902163043.11210-1-blp@ovn.org
State Accepted
Headers show
Series
  • [ovs-dev] ofproto-dpif-xlate: Fix translation of groups with no buckets.
Related show

Commit Message

Ben Pfaff Sept. 2, 2018, 4:30 p.m.
A group can have no buckets, in which case ovs_list_back() assert-fails.
This fixes the problem.

Found by OFTest.

Fixes: a04e58881e25 ("ofproto-dpif-xlate: Simplify translation for groups.")
Signed-off-by: Ben Pfaff <blp@ovn.org>
---
 ofproto/ofproto-dpif-xlate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Eelco Chaudron Sept. 13, 2018, 2:58 p.m. | #1
On 2 Sep 2018, at 18:30, Ben Pfaff wrote:

> A group can have no buckets, in which case ovs_list_back() 
> assert-fails.
> This fixes the problem.
>
> Found by OFTest.
>
> Fixes: a04e58881e25 ("ofproto-dpif-xlate: Simplify translation for 
> groups.")
> Signed-off-by: Ben Pfaff <blp@ovn.org>

The change looks fine to me.

In addition the issue was found in one of our regressions, and this 
patch solved it.
https://bugzilla.redhat.com/show_bug.cgi?id=1626488

Acked-by: Eelco Chaudron <echaudro@redhat.com>

> ---
>  ofproto/ofproto-dpif-xlate.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/ofproto/ofproto-dpif-xlate.c 
> b/ofproto/ofproto-dpif-xlate.c
> index e26f6c8f554a..507e14dd0d00 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -4488,7 +4488,7 @@ xlate_group_action__(struct xlate_ctx *ctx, 
> struct group_dpif *group,
>                       bool is_last_action)
>  {
>      if (group->up.type == OFPGT11_ALL || group->up.type == 
> OFPGT11_INDIRECT) {
> -        struct ovs_list *last_bucket = 
> ovs_list_back(&group->up.buckets);
> +        struct ovs_list *last_bucket = group->up.buckets.prev;
>          struct ofputil_bucket *bucket;
>          LIST_FOR_EACH (bucket, list_node, &group->up.buckets) {
>              bool is_last_bucket = &bucket->list_node == last_bucket;
> -- 
> 2.16.1
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Ben Pfaff Sept. 18, 2018, 4:42 a.m. | #2
On Thu, Sep 13, 2018 at 04:58:08PM +0200, Eelco Chaudron wrote:
> 
> 
> On 2 Sep 2018, at 18:30, Ben Pfaff wrote:
> 
> >A group can have no buckets, in which case ovs_list_back() assert-fails.
> >This fixes the problem.
> >
> >Found by OFTest.
> >
> >Fixes: a04e58881e25 ("ofproto-dpif-xlate: Simplify translation for
> >groups.")
> >Signed-off-by: Ben Pfaff <blp@ovn.org>
> 
> The change looks fine to me.
> 
> In addition the issue was found in one of our regressions, and this patch
> solved it.
> https://bugzilla.redhat.com/show_bug.cgi?id=1626488
> 
> Acked-by: Eelco Chaudron <echaudro@redhat.com>

Thanks.  Applied to master and branch-2.10.

Patch

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index e26f6c8f554a..507e14dd0d00 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -4488,7 +4488,7 @@  xlate_group_action__(struct xlate_ctx *ctx, struct group_dpif *group,
                      bool is_last_action)
 {
     if (group->up.type == OFPGT11_ALL || group->up.type == OFPGT11_INDIRECT) {
-        struct ovs_list *last_bucket = ovs_list_back(&group->up.buckets);
+        struct ovs_list *last_bucket = group->up.buckets.prev;
         struct ofputil_bucket *bucket;
         LIST_FOR_EACH (bucket, list_node, &group->up.buckets) {
             bool is_last_bucket = &bucket->list_node == last_bucket;