On Thu, Sep 10, 2015 at 01:18:55PM -0700, Ben Pfaff wrote:
> Otherwise duplicate bucket IDs cause linked list loops and other nastiness
> because the ofputil_bucket_find() in the OFPG15_BUCKET_LAST case later in
> copy_buckets_for_insert_bucket() will find the new bucket instead of the
> old one and the list_splice() call becomes nonsensical.
> 
> Reported-by: Ray Li <rayli1...@gmail.com>
> Reported-at: 
> http://openvswitch.org/pipermail/discuss/2015-September/018731.html
> Signed-off-by: Ben Pfaff <b...@nicira.com>

Reviewed-by: Simon Horman <simon.hor...@netronome.com>

> ---
>  AUTHORS           | 1 +
>  ofproto/ofproto.c | 2 +-
>  tests/ofproto.at  | 8 ++++++++
>  3 files changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/AUTHORS b/AUTHORS
> index a7f40bb..75ca32a 100644
> --- a/AUTHORS
> +++ b/AUTHORS
> @@ -339,6 +339,7 @@ Pratap Reddy            pre...@nicira.com
>  Ralf Heiringhoff        r...@frosty-geek.net
>  Ram Jothikumar          rjothiku...@nicira.com
>  Ramana Reddy            gtvrre...@gmail.com
> +Ray Li                  rayli1...@gmail.com
>  RishiRaj Maulick        rishi.raj2...@gmail.com
>  Rob Sherwood            rob.sherw...@bigswitch.com
>  Robert Strickler        anomal...@gmail.com
> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> index c7dd8a2..c884948 100644
> --- a/ofproto/ofproto.c
> +++ b/ofproto/ofproto.c
> @@ -6368,7 +6368,7 @@ copy_buckets_for_insert_bucket(const struct ofgroup 
> *ofgroup,
>  
>      ofputil_bucket_clone_list(&new_ofgroup->buckets, &ofgroup->buckets, 
> NULL);
>  
> -    if (ofputil_bucket_check_duplicate_id(&ofgroup->buckets)) {
> +    if (ofputil_bucket_check_duplicate_id(&new_ofgroup->buckets)) {
>              VLOG_INFO_RL(&rl, "Duplicate bucket id");
>              return OFPERR_OFPGMFC_BUCKET_EXISTS;
>      }
> diff --git a/tests/ofproto.at b/tests/ofproto.at
> index 7e80293..0f3a55e 100644
> --- a/tests/ofproto.at
> +++ b/tests/ofproto.at
> @@ -438,6 +438,14 @@ AT_CHECK([STRIP_XIDS stdout], [0], [dnl
>  OFPST_GROUP_DESC reply (OF1.5):
>   
> group_id=1234,type=all,bucket=bucket_id:0,actions=output:0,bucket=bucket_id:1,actions=output:1,bucket=bucket_id:10,actions=output:10,bucket=bucket_id:11,actions=output:11,bucket=bucket_id:14,actions=output:14,bucket=bucket_id:15,actions=output:15
>  ])
> +
> +# Verify that duplicate bucket IDs are rejected.
> +AT_CHECK([ovs-ofctl -O OpenFlow15 -vwarn insert-buckets br0 
> group_id=1234,command_bucket_id=last,bucket=bucket_id:14,actions=output:14,bucket=bucket_id:15,actions=output:15],
>  [1], [], [stderr])
> +AT_CHECK([STRIP_XIDS stderr | sed '/truncated/,$d'], [0], [dnl
> +OFPT_ERROR (OF1.5): OFPGMFC_BUCKET_EXISTS
> +OFPT_GROUP_MOD (OF1.5):
> +])
> +
>  AT_CHECK([ovs-ofctl -O OpenFlow15 -vwarn insert-buckets br0 
> group_id=1234,command_bucket_id=15])
>  AT_CHECK([ovs-ofctl -O OpenFlow15 -vwarn dump-groups br0], [0], [stdout])
>  AT_CHECK([STRIP_XIDS stdout], [0], [dnl
> -- 
> 2.1.3
> 
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to