[ovs-dev] [PATCH v4] ovs:group: support to insert bucket with weight value for select type
Ben Pfaff
blp at ovn.org
Thu Dec 27 20:07:29 UTC 2018
On Mon, Dec 17, 2018 at 12:24:04PM +0800, solomon wrote:
> 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.
>
>
> v3--v4:
> 1. remove the redundant space in testcase。
> ref:https://api.travis-ci.org/v3/job/467942792/log.txt
>
> v2-->v3:
> 1. add testcase.
>
> v1-->v2:
> 1. fix warning from 0-day robot.
>
>
> Signed-off-by: solomon <liwei.solomon at gmail.com>
Thanks for the revision.
I think that, if we apply this, then every OFPTGC15_INSERT_BUCKET that
ovs-ofctl sends will include a weight. That is OK if it is talking to
the latest ovs-vswitchd, but anything older will reject it. So, I think
that it is better if OFPTGC15_INSERT_BUCKET only includes a weight if
the user supplied one. I think that we can do that easily, by checking
for a nonzero weight instead of the command, and if we do that then we
don't need the command anyway. So here is what I would suggest folding
in:
diff --git a/lib/ofp-group.c b/lib/ofp-group.c
index d9eba735da58..92ff35842986 100644
--- a/lib/ofp-group.c
+++ b/lib/ofp-group.c
@@ -1177,8 +1177,7 @@ 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,
- int group_command)
+ struct ofpbuf *openflow, enum ofp_version ofp_version)
{
struct ofp15_bucket *ob;
size_t start, actions_start, actions_len;
@@ -1190,9 +1189,7 @@ ofputil_put_ofp15_bucket(const struct ofputil_bucket *bucket,
ofpacts_put_openflow_actions(bucket->ofpacts, bucket->ofpacts_len,
openflow, ofp_version);
actions_len = openflow->size - actions_start;
-
- if (group_type == OFPGT11_SELECT
- || group_command == OFPGC15_INSERT_BUCKET) {
+ if (group_type == OFPGT11_SELECT || bucket->weight) {
ofpprop_put_u16(openflow, OFPGBPT15_WEIGHT, bucket->weight);
}
if (bucket->watch_port != OFPP_ANY) {
@@ -1269,7 +1266,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, -1);
+ gds->type, reply, version);
}
ogds = ofpbuf_at_assert(reply, start_ogds, sizeof *ogds);
ogds->type = gds->type;
@@ -2069,8 +2066,7 @@ 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,
- gm->command);
+ ofputil_put_ofp15_bucket(bucket, bucket_id, gm->type, b, ofp_version);
}
ogm = ofpbuf_at_assert(b, start_ogm, sizeof *ogm);
ogm->command = htons(gm->command != OFPGC11_ADD_OR_MOD || group_existed < 0
diff --git a/tests/ofproto.at b/tests/ofproto.at
index 0af4fd731573..15a2914836d4 100644
--- a/tests/ofproto.at
+++ b/tests/ofproto.at
@@ -679,7 +679,7 @@ AT_CHECK([ovs-ofctl -O OpenFlow11 -vwarn insert-buckets br0 group_id=1234,comman
[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.
+# 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
More information about the dev
mailing list