@@ -1225,62 +1225,54 @@ parse_bucket_str(struct ofputil_bucket *bucket, char *str_, uint8_t group_type,
}
static char * OVS_WARN_UNUSED_RESULT
-parse_select_group_field(char *s, struct field_array *fa,
+parse_select_group_field(const char *name, const char *value_str,
+ struct field_array *fa,
enum ofputil_protocol *usable_protocols)
{
- char *save_ptr = NULL;
- char *name;
+ const struct mf_field *mf = mf_from_name(name);
- for (name = strtok_r(s, "=, \t\r\n", &save_ptr); name;
- name = strtok_r(NULL, "=, \t\r\n", &save_ptr)) {
- const struct mf_field *mf = mf_from_name(name);
+ if (mf) {
+ char *error;
+ union mf_value value;
- if (mf) {
- char *error;
- const char *value_str;
- union mf_value value;
+ if (bitmap_is_set(fa->used.bm, mf->id)) {
+ return xasprintf("%s: duplicate field", name);
+ }
- if (bitmap_is_set(fa->used.bm, mf->id)) {
- return xasprintf("%s: duplicate field", name);
+ if (value_str) {
+ error = mf_parse_value(mf, value_str, &value);
+ if (error) {
+ return error;
}
- value_str = strtok_r(NULL, ", \t\r\n", &save_ptr);
- if (value_str) {
- error = mf_parse_value(mf, value_str, &value);
- if (error) {
- return error;
- }
-
- /* The mask cannot be all-zeros */
- if (!mf_is_tun_metadata(mf) &&
- is_all_zeros(&value, mf->n_bytes)) {
- return xasprintf("%s: values are wildcards here "
- "and must not be all-zeros", s);
- }
+ /* The mask cannot be all-zeros */
+ if (!mf_is_tun_metadata(mf) &&
+ is_all_zeros(&value, mf->n_bytes)) {
+ return xasprintf("%s: values are wildcards here "
+ "and must not be all-zeros", name);
+ }
- /* The values parsed are masks for fields used
- * by the selection method */
- if (!mf_is_mask_valid(mf, &value)) {
- return xasprintf("%s: invalid mask for field %s",
- value_str, mf->name);
- }
- } else {
- memset(&value, 0xff, mf->n_bytes);
+ /* The values parsed are masks for fields used
+ * by the selection method */
+ if (!mf_is_mask_valid(mf, &value)) {
+ return xasprintf("%s: invalid mask for field", name);
}
+ } else {
+ memset(&value, 0xff, mf->n_bytes);
+ }
- field_array_set(mf->id, &value, fa);
+ field_array_set(mf->id, &value, fa);
- if (is_all_ones(&value, mf->n_bytes)) {
- *usable_protocols &= mf->usable_protocols_exact;
- } else if (mf->usable_protocols_bitwise == mf->usable_protocols_cidr
- || ip_is_cidr(value.be32)) {
- *usable_protocols &= mf->usable_protocols_cidr;
- } else {
- *usable_protocols &= mf->usable_protocols_bitwise;
- }
+ if (is_all_ones(&value, mf->n_bytes)) {
+ *usable_protocols &= mf->usable_protocols_exact;
+ } else if (mf->usable_protocols_bitwise == mf->usable_protocols_cidr
+ || ip_is_cidr(value.be32)) {
+ *usable_protocols &= mf->usable_protocols_cidr;
} else {
- return xasprintf("%s: unknown field %s", s, name);
+ *usable_protocols &= mf->usable_protocols_bitwise;
}
+ } else {
+ return xasprintf("unknown select group field %s", name);
}
return NULL;
@@ -1300,6 +1292,7 @@ parse_ofp_group_mod_str__(struct ofputil_group_mod *gm, uint16_t command,
char *save_ptr = NULL;
bool had_type = false;
bool had_command_bucket_id = false;
+ bool parsing_fields = false;
char *name;
struct ofputil_bucket *bucket;
char *error = NULL;
@@ -1358,14 +1351,29 @@ parse_ofp_group_mod_str__(struct ofputil_group_mod *gm, uint16_t command,
}
/* Parse everything before the buckets. */
- for (name = strtok_r(string, "=, \t\r\n", &save_ptr); name;
- name = strtok_r(NULL, "=, \t\r\n", &save_ptr)) {
+ for (name = strtok_r(string, ", \t\r\n", &save_ptr); name;
+ name = strtok_r(NULL, ", \t\r\n", &save_ptr)) {
char *value;
- value = strtok_r(NULL, ", \t\r\n", &save_ptr);
+ if (parsing_fields &&
+ (!strcmp(name, "command_bucket_id") ||
+ !strcmp(name, "group_id") ||
+ !strcmp(name, "type") ||
+ !strcmp(name, "selection_method") ||
+ !strcmp(name, "selection_method") ||
+ !strcmp(name, "selection_method_param"))) {
+ parsing_fields = false;
+ }
+
+again:
+ value = strchr(name, '=');
if (!value) {
- error = xasprintf("field %s missing value", name);
- goto out;
+ if (!parsing_fields) {
+ error = xasprintf("field %s missing value", name);
+ goto out;
+ }
+ } else {
+ *(value)++ = '\0';
}
if (!strcmp(name, "command_bucket_id")) {
@@ -1462,12 +1470,16 @@ parse_ofp_group_mod_str__(struct ofputil_group_mod *gm, uint16_t command,
error = xstrdup("fields are not needed");
goto out;
}
- error = parse_select_group_field(value, &gm->props.fields,
+ *usable_protocols &= OFPUTIL_P_OF15_UP;
+ parsing_fields = true;
+ name = value;
+ goto again;
+ } else if (parsing_fields) {
+ error = parse_select_group_field(name, value, &gm->props.fields,
usable_protocols);
if (error) {
goto out;
}
- *usable_protocols &= OFPUTIL_P_OF15_UP;
} else {
error = xasprintf("unknown keyword %s", name);
goto out;
@@ -343,7 +343,7 @@ dnl Actions definition listed in both supported formats (w/ actions=)
AT_SETUP([ofproto - del group (OpenFlow 1.5)])
OVS_VSWITCHD_START
AT_DATA([groups.txt], [dnl
-group_id=1233,type=select,selection_method=hash,bucket=output:10,bucket=output:11
+group_id=1233,type=select,fields=eth_src,eth_dst=ff:ff:ff:00:00:00,selection_method=hash,bucket=output:10,bucket=output:11
group_id=1234,type=select,selection_method=hash,bucket=output:10,bucket=output:11
group_id=1235,type=all,bucket=actions=output:12,bucket=bucket_id:0,actions=output:10,bucket=bucket_id:1,actions=output:11
])
@@ -358,14 +358,14 @@ 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=1235,type=all,bucket=bucket_id:2,actions=output:12,bucket=bucket_id:0,actions=output:10,bucket=bucket_id:1,actions=output:11
- group_id=1233,type=select,selection_method=hash,bucket=bucket_id:0,actions=output:10,bucket=bucket_id:1,actions=output:11
+ group_id=1233,type=select,selection_method=hash,fields=eth_src,eth_dst=ff:ff:ff:00:00:00,bucket=bucket_id:0,actions=output:10,bucket=bucket_id:1,actions=output:11
])
AT_CHECK([ovs-ofctl -O OpenFlow15 -vwarn del-groups br0 group_id=1234])
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=1235,type=all,bucket=bucket_id:2,actions=output:12,bucket=bucket_id:0,actions=output:10,bucket=bucket_id:1,actions=output:11
- group_id=1233,type=select,selection_method=hash,bucket=bucket_id:0,actions=output:10,bucket=bucket_id:1,actions=output:11
+ group_id=1233,type=select,selection_method=hash,fields=eth_src,eth_dst=ff:ff:ff:00:00:00,bucket=bucket_id:0,actions=output:10,bucket=bucket_id:1,actions=output:11
])
AT_CHECK([ovs-ofctl -O OpenFlow15 -vwarn del-groups br0], [0])
AT_CHECK([ovs-ofctl -O OpenFlow15 -vwarn dump-groups br0], [0], [stdout])
This corrects the parsing of 'fields' specified for groups on the command line. 'fields' may be used in conjunction with the Netronome selection method extension to describe which fields of the flow should be used as by the selection method. This patch corrects two problems with the current implementation as compared to the documentation in the ovs-ofctl man page. * Allows parsing of more than one field * Allows parsing of masks for fields Fixes: 18ac06d3546e ("ofp-util: Encoding and decoding of (draft) OpenFlow 1.5 group messages.") Signed-off-by: Simon Horman <simon.horman@netronome.com> --- lib/ofp-parse.c | 112 ++++++++++++++++++++++++++++++------------------------- tests/ofproto.at | 6 +-- 2 files changed, 65 insertions(+), 53 deletions(-)