From patchwork Fri Oct 16 15:21:06 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ben Pfaff X-Patchwork-Id: 531391 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from archives.nicira.com (unknown [IPv6:2600:3c00::f03c:91ff:fe6e:bdf7]) by ozlabs.org (Postfix) with ESMTP id B33E6140B0F for ; Sat, 17 Oct 2015 02:21:15 +1100 (AEDT) Received: from archives.nicira.com (localhost [127.0.0.1]) by archives.nicira.com (Postfix) with ESMTP id 8428410C3B; Fri, 16 Oct 2015 08:21:14 -0700 (PDT) X-Original-To: dev@openvswitch.org Delivered-To: dev@openvswitch.org Received: from mx3v1.cudamail.com (mx3.cudamail.com [64.34.241.5]) by archives.nicira.com (Postfix) with ESMTPS id 7287A10C35 for ; Fri, 16 Oct 2015 08:21:13 -0700 (PDT) Received: from bar3.cudamail.com (bar1 [192.168.15.1]) by mx3v1.cudamail.com (Postfix) with ESMTP id 951286181BD for ; Fri, 16 Oct 2015 09:21:11 -0600 (MDT) X-ASG-Debug-ID: 1445008870-03dd7b105e59b80001-byXFYA Received: from mx3-pf1.cudamail.com ([192.168.14.2]) by bar3.cudamail.com with ESMTP id TfOBF3z1w8BVRcN6 (version=TLSv1 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO) for ; Fri, 16 Oct 2015 09:21:10 -0600 (MDT) X-Barracuda-Envelope-From: blp@nicira.com X-Barracuda-RBL-Trusted-Forwarder: 192.168.14.2 Received: from unknown (HELO mail-pa0-f45.google.com) (209.85.220.45) by mx3-pf1.cudamail.com with ESMTPS (RC4-SHA encrypted); 16 Oct 2015 15:21:10 -0000 Received-SPF: unknown (mx3-pf1.cudamail.com: Multiple SPF records returned) X-Barracuda-RBL-Trusted-Forwarder: 209.85.220.45 Received: by pabrc13 with SMTP id rc13so123538587pab.0 for ; Fri, 16 Oct 2015 08:21:09 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=TGbEldt7YUnRCKWJeaeJZmsMx5kic8TgZWjxgcCVd6g=; b=Rn3KSyHpjJ7T7rYxoUSzTOIah4+/zZ6FXOQeHT0JepLxMysLyQ8BCWW7mgGINtd6H6 B8vO09wz40ZRR0ZgjrnWp6GCxJxTJEfVcHcXaHrGG68tcwDanRHIvds30gJ83w3zW2uE tvb4L9kUuVl7/5FwdRNufcN0KJ3xWkfSW4dfNicnemAVVjqCeex4Aj1K8Vt6CT1iPna4 mAW42DdUSmb1ZDzwv8yVPJOooRIe8k1Rfsoh6sJWJV7RGcSS9mGhLiBO0cJ4Wh9pA7Fo 1fqQofP2WmxYnFikIHxih7lkj2alXB5+ZR097HMYwNCFNLJIQmEc2gzKEhsuAe3eV7Ud OjkQ== X-Gm-Message-State: ALoCoQnjDNz0xIrmdu0MNNDMDH157kWO63SxjAJ80JpgYOLTVofv9g2/xCXC5bdnLWnqGr/cto4Q X-Received: by 10.68.139.194 with SMTP id ra2mr17439492pbb.6.1445008869749; Fri, 16 Oct 2015 08:21:09 -0700 (PDT) Received: from sigabrt.gateway.sonic.net (173-228-112-197.dsl.dynamic.fusionbroadband.com. [173.228.112.197]) by smtp.gmail.com with ESMTPSA id xz5sm21853551pbb.12.2015.10.16.08.21.08 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Fri, 16 Oct 2015 08:21:08 -0700 (PDT) X-CudaMail-Envelope-Sender: blp@nicira.com X-Barracuda-Apparent-Source-IP: 173.228.112.197 From: Ben Pfaff To: dev@openvswitch.org X-CudaMail-Whitelist-To: dev@openvswitch.org X-CudaMail-MID: CM-V1-1015025035 X-CudaMail-DTE: 101615 X-CudaMail-Originating-IP: 209.85.220.45 Date: Fri, 16 Oct 2015 08:21:06 -0700 X-ASG-Orig-Subj: [##CM-V1-1015025035##][PATCH] ofp-parse: Fix parsing, formatting of multiple fields in NTR extension. Message-Id: <1445008866-5572-1-git-send-email-blp@nicira.com> X-Mailer: git-send-email 2.1.3 X-Barracuda-Connect: UNKNOWN[192.168.14.2] X-Barracuda-Start-Time: 1445008870 X-Barracuda-Encrypted: DHE-RSA-AES256-SHA X-Barracuda-URL: https://web.cudamail.com:443/cgi-mod/mark.cgi X-ASG-Whitelist: Header =?UTF-8?B?eFwtY3VkYW1haWxcLXdoaXRlbGlzdFwtdG8=?= X-Virus-Scanned: by bsmtpd at cudamail.com X-Barracuda-BRTS-Status: 1 Cc: Ben Pfaff , Simon Horman Subject: [ovs-dev] [PATCH] ofp-parse: Fix parsing, formatting of multiple fields in NTR extension. X-BeenThere: dev@openvswitch.org X-Mailman-Version: 2.1.16 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: dev-bounces@openvswitch.org Sender: "dev" Until now, the only way to specify multiple fields in the "fields" parameter for the Netronome groups extension, was to specify "fields" more than once, e.g. fields=eth_dst,fields=ip_dst However, this wasn't documented and the code in ofp-print didn't use it, generating output that couldn't be parsed. This commit fixes the situation by introducing a more straightforward syntax, e.g. fields(eth_dst,ip_dst), documents it, and adjusts ofp-print code to use it when there is more than one field (it retains the previous format for backward compatibility when there is exactly one field) CC: Simon Horman Signed-off-by: Ben Pfaff Acked-by: Simon Horman --- lib/ofp-parse.c | 25 ++++++------------------- lib/ofp-print.c | 22 ++++++++++------------ tests/ofp-print.at | 2 +- tests/ofproto.at | 4 ++-- utilities/ovs-ofctl.8.in | 6 +++++- 5 files changed, 24 insertions(+), 35 deletions(-) diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c index 8437656..214cc36 100644 --- a/lib/ofp-parse.c +++ b/lib/ofp-parse.c @@ -1228,24 +1228,20 @@ static char * OVS_WARN_UNUSED_RESULT parse_select_group_field(char *s, struct field_array *fa, enum ofputil_protocol *usable_protocols) { - char *save_ptr = NULL; - char *name; + char *name, *value_str; - for (name = strtok_r(s, "=, \t\r\n", &save_ptr); name; - name = strtok_r(NULL, "=, \t\r\n", &save_ptr)) { + while (ofputil_parse_key_value(&s, &name, &value_str)) { const struct mf_field *mf = mf_from_name(name); 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); } - value_str = strtok_r(NULL, ", \t\r\n", &save_ptr); - if (value_str) { + if (*value_str) { error = mf_parse_value(mf, value_str, &value); if (error) { return error; @@ -1297,10 +1293,8 @@ parse_ofp_group_mod_str__(struct ofputil_group_mod *gm, uint16_t command, F_COMMAND_BUCKET_ID = 1 << 2, F_COMMAND_BUCKET_ID_ALL = 1 << 3, } fields; - char *save_ptr = NULL; bool had_type = false; bool had_command_bucket_id = false; - char *name; struct ofputil_bucket *bucket; char *error = NULL; @@ -1358,16 +1352,9 @@ 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)) { - char *value; - - value = strtok_r(NULL, ", \t\r\n", &save_ptr); - if (!value) { - error = xasprintf("field %s missing value", name); - goto out; - } - + char *pos = string; + char *name, *value; + while (ofputil_parse_key_value(&pos, &name, &value)) { if (!strcmp(name, "command_bucket_id")) { if (!(fields & F_COMMAND_BUCKET_ID)) { error = xstrdup("command bucket id is not needed"); diff --git a/lib/ofp-print.c b/lib/ofp-print.c index d0c94ce..240ba84 100644 --- a/lib/ofp-print.c +++ b/lib/ofp-print.c @@ -2352,22 +2352,20 @@ ofp_print_group(struct ds *s, uint32_t group_id, uint8_t type, } if (props->selection_method[0]) { - size_t mark, start; - - ds_put_format(s, ",selection_method=%s,", props->selection_method); + ds_put_format(s, ",selection_method=%s", props->selection_method); if (props->selection_method_param) { - ds_put_format(s, "selection_method_param=%"PRIu64",", + ds_put_format(s, ",selection_method_param=%"PRIu64, props->selection_method_param); } - /* Allow rewinding to immediately before the trailing ',' */ - mark = s->length - 1; - - ds_put_cstr(s, "fields="); - start = s->length; - oxm_format_field_array(s, &props->fields); - if (s->length == start) { - ds_truncate(s, mark); + size_t n = bitmap_count1(props->fields.used.bm, MFF_N_IDS); + if (n == 1) { + ds_put_cstr(s, ",fields="); + oxm_format_field_array(s, &props->fields); + } else if (n > 1) { + ds_put_cstr(s, ",fields("); + oxm_format_field_array(s, &props->fields); + ds_put_char(s, ')'); } } diff --git a/tests/ofp-print.at b/tests/ofp-print.at index 8cdcead..fd9c2d4 100644 --- a/tests/ofp-print.at +++ b/tests/ofp-print.at @@ -2043,7 +2043,7 @@ ff ff 00 3b 00 00 15 40 00 00 00 01 00 00 00 00 \ 14 01 ff 00 00 00 00 00 \ "], [0], [dnl OFPST_GROUP_DESC reply (OF1.5) (xid=0x2): - group_id=8192,type=select,selection_method=hash,fields=ip_dst=255.255.255.0,nw_proto,tcp_src,bucket=bucket_id:0,weight:100,watch_port:1,actions=output:1,bucket=bucket_id:1,weight:200,watch_port:2,actions=output:2,bucket=bucket_id:2,weight:200,watch_port:3,actions=output:3 + group_id=8192,type=select,selection_method=hash,fields(ip_dst=255.255.255.0,nw_proto,tcp_src),bucket=bucket_id:0,weight:100,watch_port:1,actions=output:1,bucket=bucket_id:1,weight:200,watch_port:2,actions=output:2,bucket=bucket_id:2,weight:200,watch_port:3,actions=output:3 ]) AT_CLEANUP diff --git a/tests/ofproto.at b/tests/ofproto.at index 5e4441c..beeb65a 100644 --- a/tests/ofproto.at +++ b/tests/ofproto.at @@ -343,14 +343,14 @@ 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=1234,type=select,selection_method=hash,bucket=output:10,bucket=output:11 +group_id=1234,type=select,selection_method=hash,fields(eth_dst,ip_dst,tcp_dst),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 ]) AT_CHECK([ovs-ofctl -O OpenFlow15 -vwarn add-groups br0 groups.txt]) AT_CHECK([ovs-ofctl -F OXM-OpenFlow15 -O OpenFlow15 -vwarn dump-groups br0 1234], [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:0,actions=output:10,bucket=bucket_id:1,actions=output:11 + group_id=1234,type=select,selection_method=hash,fields(eth_dst,ip_dst,tcp_dst),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]) diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in index a6087f6..dc81e7e 100644 --- a/utilities/ovs-ofctl.8.in +++ b/utilities/ovs-ofctl.8.in @@ -2596,6 +2596,9 @@ This field is optional for \fBadd\-group\fR, \fBadd\-groups\fR and \fBmod\-group\fR commands on groups of type \fBselect\fR. Prohibited otherwise. The default value is the empty string. .IP +Other than the empty string, \fBhash\fR is currently the only defined +selection method. +.IP This option will use a Netronome OpenFlow extension which is only supported when using Open vSwitch 2.4 and later with OpenFlow 1.5 and later. @@ -2609,7 +2612,8 @@ Prohibited otherwise. The default value is zero. This option will use a Netronome OpenFlow extension which is only supported when using Open vSwitch 2.4 and later with OpenFlow 1.5 and later. -.IP \fBfields\fR=\fIparam\fR +.IP \fBfields\fR=\fIfield\fR +.IQ \fBfields(\fIfield\fR[\fB=\fImask\fR]\fR...\fB)\fR The field parameters to selection method selected by the \fBselection_method\fR field. The syntax is described in \fBFlow Syntax\fR with the additional restrictions that if a value is provided it is