solomon <[email protected]> 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 <[email protected]> > ---
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 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
