[ovs-dev] [PATCH v3] ovs:group: support to insert bucket with weight value for select type

Aaron Conole aconole at bytheb.org
Sat Dec 15 22:09:05 UTC 2018


solomon <liwei.solomon at gmail.com> writes:

> After creating a group with hash select type,then  we need to insert a new bucket with weight,
> But,it fails. Commands are as following:
>
> # ovs-ofctl  -O OpenFlow15 add-group br0 "group_id=10, type=select, selection_method=hash,fields=tcp_src, bucket=bucket_id=10,weight:99,actions=output:1, bucket=bucket_id=20,weight:199,actions=output:1 "
>
> # ovs-ofctl -O OpenFlow15 insert-buckets br0 "group_id=10,type=select command_bucket_id=last,bucket=bucket_id=3,weight=100,actions=output:1"
> ovs-ofctl: type is not needed
>
> # ovs-ofctl -O OpenFlow15 insert-buckets br0 "group_id=10 command_bucket_id=last,bucket=bucket_id=3,weight=100,actions=output:1"
> ovs-ofctl: Only select groups can have bucket weights.
>
>
> This patch can help us. However, for other types that are not select, the check of the parameters
> is not strict, but it does not affect their function, because other types do not use this weight parameter.
>
>
> v2-->v3:
> 1. add testcase.
>
> v1-->v2:
> 1. fix warning from 0-day robot.
>
>
> Signed-off-by: solomon <liwei.solomon at gmail.com>
> ---

Travis build seems to fail with this, and it looks related:

https://travis-ci.org/ovsrobot/ovs/jobs/467942792

But, I haven't taken a look at the code (technically, I'm on a frozen
swamp somewhere in NH not supposed to be looking after work related
things, but if you don't tell my wife, I won't either :).

>  lib/ofp-group.c  | 17 +++++++++++------
>  tests/ofproto.at | 14 ++++++++++++++
>  2 files changed, 25 insertions(+), 6 deletions(-)
>
> diff --git a/lib/ofp-group.c b/lib/ofp-group.c
> index c9e95ad4c..d9eba735d 100644
> --- a/lib/ofp-group.c
> +++ b/lib/ofp-group.c
> @@ -1043,7 +1043,8 @@ parse_ofp_group_mod_str__(struct ofputil_group_mod *gm, int command,
>          }
>          ovs_list_push_back(&gm->buckets, &bucket->list_node);
>  
> -        if (gm->type != OFPGT11_SELECT && bucket->weight) {
> +        if (gm->command != OFPGC15_INSERT_BUCKET
> +            && gm->type != OFPGT11_SELECT && bucket->weight) {
>              error = xstrdup("Only select groups can have bucket weights.");
>              goto out;
>          }
> @@ -1176,7 +1177,8 @@ ofputil_put_ofp11_bucket(const struct ofputil_bucket *bucket,
>  static void
>  ofputil_put_ofp15_bucket(const struct ofputil_bucket *bucket,
>                           uint32_t bucket_id, enum ofp11_group_type group_type,
> -                         struct ofpbuf *openflow, enum ofp_version ofp_version)
> +                         struct ofpbuf *openflow, enum ofp_version ofp_version,
> +                         int group_command)
>  {
>      struct ofp15_bucket *ob;
>      size_t start, actions_start, actions_len;
> @@ -1189,7 +1191,8 @@ ofputil_put_ofp15_bucket(const struct ofputil_bucket *bucket,
>                                   openflow, ofp_version);
>      actions_len = openflow->size - actions_start;
>  
> -    if (group_type == OFPGT11_SELECT) {
> +    if (group_type == OFPGT11_SELECT
> +        || group_command == OFPGC15_INSERT_BUCKET) {
>          ofpprop_put_u16(openflow, OFPGBPT15_WEIGHT, bucket->weight);
>      }
>      if (bucket->watch_port != OFPP_ANY) {
> @@ -1266,7 +1269,7 @@ ofputil_append_ofp15_group_desc_reply(const struct ofputil_group_desc *gds,
>      start_buckets = reply->size;
>      LIST_FOR_EACH (bucket, list_node, buckets) {
>          ofputil_put_ofp15_bucket(bucket, bucket->bucket_id,
> -                                 gds->type, reply, version);
> +                                 gds->type, reply, version, -1);
>      }
>      ogds = ofpbuf_at_assert(reply, start_ogds, sizeof *ogds);
>      ogds->type = gds->type;
> @@ -2066,7 +2069,8 @@ ofputil_encode_ofp15_group_mod(enum ofp_version ofp_version,
>              bucket_id = bucket->bucket_id;
>          }
>  
> -        ofputil_put_ofp15_bucket(bucket, bucket_id, gm->type, b, ofp_version);
> +        ofputil_put_ofp15_bucket(bucket, bucket_id, gm->type, b, ofp_version,
> +                                 gm->command);
>      }
>      ogm = ofpbuf_at_assert(b, start_ogm, sizeof *ogm);
>      ogm->command = htons(gm->command != OFPGC11_ADD_OR_MOD || group_existed < 0
> @@ -2251,7 +2255,8 @@ ofputil_check_group_mod(const struct ofputil_group_mod *gm)
>  
>      struct ofputil_bucket *bucket;
>      LIST_FOR_EACH (bucket, list_node, &gm->buckets) {
> -        if (bucket->weight && gm->type != OFPGT11_SELECT) {
> +        if (bucket->weight && gm->type != OFPGT11_SELECT
> +            && gm->command != OFPGC15_INSERT_BUCKET) {
>              return OFPERR_OFPGMFC_INVALID_GROUP;
>          }
>  
> diff --git a/tests/ofproto.at b/tests/ofproto.at
> index 9819bc577..7a6598010 100644
> --- a/tests/ofproto.at
> +++ b/tests/ofproto.at
> @@ -678,6 +678,20 @@ AT_CHECK([ovs-ofctl -O OpenFlow15 -vwarn insert-buckets br0 group_id=1234,comman
>  AT_CHECK([ovs-ofctl -O OpenFlow11 -vwarn insert-buckets br0 group_id=1234,command_bucket_id=first,bucket=bucket_id:0,actions=output:0,bucket=bucket_id:1,actions=output:1], [1], [],
>    [ovs-ofctl: insert-bucket needs OpenFlow 1.5 or later ('-O OpenFlow15')
>  ])
> +
> +#Verify insert-buckets command to insert bucket with weight value for select group.
> +AT_CHECK([ovs-ofctl -O OpenFlow15 --strict del-groups br0 group_id=1234])
> +AT_DATA([groups.txt], [dnl
> +group_id=1234,type=select,selection_method=hash,bucket=bucket_id=1,weight:100,output:11
> +])
> +AT_CHECK([ovs-ofctl -O OpenFlow15 -vwarn add-groups br0 groups.txt])
> +AT_CHECK([ovs-ofctl -O OpenFlow15 insert-buckets br0 group_id=1234,command_bucket_id=last,bucket=bucket_id=2,weight=100,actions=output:11])
> +AT_CHECK([ovs-ofctl -O OpenFlow15 -vwarn dump-groups br0], [0], [stdout])
> +AT_CHECK([strip_xids < stdout], [0], [dnl
> +OFPST_GROUP_DESC reply (OF1.5):
> + group_id=1234,type=select,selection_method=hash,bucket=bucket_id:1,weight:100,actions=output:11,bucket=bucket_id:2,weigh    t:100,actions=output:11
> +])
> +
>  OVS_VSWITCHD_STOP
>  AT_CLEANUP


More information about the dev mailing list