[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