The "insert buckets" and "delete buckets" operations on a group should not change the group's type or properties, but the implementation did this by mistake. This fixes the problem.
Reported-by: shivani dommeti <[email protected]> Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2017-December/045830.html Signed-off-by: Ben Pfaff <[email protected]> --- AUTHORS.rst | 1 + ofproto/ofproto.c | 17 +++++++++++++++++ tests/ofproto.at | 25 +++++++++++++++++++++++++ 3 files changed, 43 insertions(+) diff --git a/AUTHORS.rst b/AUTHORS.rst index 075b2f72d47d..979cfd7bce41 100644 --- a/AUTHORS.rst +++ b/AUTHORS.rst @@ -589,6 +589,7 @@ likunyun [email protected] meishengxin [email protected] neeraj mehta [email protected] rahim entezari [email protected] +shivani dommeti [email protected] weizj [email protected] 俊 赵 [email protected] 冯全树(Crab) [email protected] diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index 82c2bb27d348..84eb18e901ed 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -6905,6 +6905,13 @@ handle_queue_get_config_request(struct ofconn *ofconn, return error; } +/* Allocates, initializes, and constructs a new group in 'ofproto', obtaining + * all the attributes for it from 'gm', and stores a pointer to it in + * '*ofgroup'. Makes the new group visible from the flow table starting from + * 'version'. + * + * Returns 0 if successful, otherwise an error code. If there is an error then + * '*ofgroup' is indeterminate upon return. */ static enum ofperr init_group(struct ofproto *ofproto, const struct ofputil_group_mod *gm, ovs_version_t version, struct ofgroup **ofgroup) @@ -7113,6 +7120,16 @@ modify_group_start(struct ofproto *ofproto, struct ofproto_group_mod *ogm) return OFPERR_OFPGMFC_UNKNOWN_GROUP; } + /* Inserting or deleting a bucket should not change the group's type or + * properties, so change the group mod so that these aspects match the old + * group. (See EXT-570.) */ + if (ogm->gm.command == OFPGC15_INSERT_BUCKET || + ogm->gm.command == OFPGC15_REMOVE_BUCKET) { + ogm->gm.type = old_group->type; + ofputil_group_properties_destroy(&ogm->gm.props); + ofputil_group_properties_copy(&ogm->gm.props, &old_group->props); + } + if (old_group->type != ogm->gm.type && (ofproto->n_groups[ogm->gm.type] >= ofproto->ogf.max_groups[ogm->gm.type])) { diff --git a/tests/ofproto.at b/tests/ofproto.at index c19a3d10daed..74a30cbefe3b 100644 --- a/tests/ofproto.at +++ b/tests/ofproto.at @@ -621,7 +621,32 @@ 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:12,actions=output:12,bucket=bucket_id:13,actions=output:13,bucket=bucket_id:14,actions=output:14,bucket=bucket_id:15,actions=output:15,bucket=bucket_id:20,actions=output:20,bucket=bucket_id:21,actions=output:21 ]) +# Delete groups. +AT_CHECK([ovs-ofctl -O OpenFlow15 del-groups br0]) + +# Add "fast_failover" group, then insert a bucket into it and make +# sure that the type of the group doesn't change. (There was a bug +# that caused this to happen.) +AT_CHECK([ovs-ofctl -O OpenFlow15 -vwarn add-group br0 group_id=1234,type=ff]) +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=ff +]) +AT_CHECK([ovs-ofctl -O OpenFlow15 -vwarn insert-buckets br0 group_id=1234,command_bucket_id=first,bucket=bucket_id:20,actions=output:20,bucket=bucket_id:21,actions=output:21]) +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=ff,bucket=bucket_id:20,actions=output:20,bucket=bucket_id:21,actions=output:21 +]) + # Negative tests. +AT_CHECK([ovs-ofctl -O OpenFlow15 -vwarn insert-buckets br0 group_id=1234,command_bucket_id=123,type=indirect,bucket=bucket_id:0,actions=output:0,bucket=bucket_id:1,actions=output:1], [1], [], + [ovs-ofctl: type is not needed +]) +AT_CHECK([ovs-ofctl -O OpenFlow15 -vwarn insert-buckets br0 group_id=1234,command_bucket_id=123,selection_method=dp_hash,type=indirect,bucket=bucket_id:0,actions=output:0,bucket=bucket_id:1,actions=output:1], [1], [], + [ovs-ofctl: selection method is not needed +]) AT_CHECK([ovs-ofctl -O OpenFlow15 -vwarn insert-buckets br0 group_id=1234,command_bucket_id=0xffffff01,bucket=bucket_id:0,actions=output:0,bucket=bucket_id:1,actions=output:1], [1], [], [ovs-ofctl: invalid command bucket id 4294967041 ]) -- 2.10.2 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
