From patchwork Fri Oct 16 10:50:48 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Horman X-Patchwork-Id: 531216 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from archives.nicira.com (li376-54.members.linode.com [96.126.127.54]) by ozlabs.org (Postfix) with ESMTP id 9A6141402D0 for ; Fri, 16 Oct 2015 21:52:49 +1100 (AEDT) Received: from archives.nicira.com (localhost [127.0.0.1]) by archives.nicira.com (Postfix) with ESMTP id 7B7D010C9A; Fri, 16 Oct 2015 03:52:48 -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 D577710C99 for ; Fri, 16 Oct 2015 03:52:46 -0700 (PDT) Received: from bar4.cudamail.com (bar2 [192.168.15.2]) by mx3v1.cudamail.com (Postfix) with ESMTP id 54A93618205 for ; Fri, 16 Oct 2015 04:52:46 -0600 (MDT) X-ASG-Debug-ID: 1444992765-03dc210f8d40b50001-byXFYA Received: from mx3-pf3.cudamail.com ([192.168.14.3]) by bar4.cudamail.com with ESMTP id p3cWihbWXEzCxhHY (version=TLSv1 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO) for ; Fri, 16 Oct 2015 04:52:45 -0600 (MDT) X-Barracuda-Envelope-From: simon.horman@netronome.com X-Barracuda-RBL-Trusted-Forwarder: 192.168.14.3 Received: from unknown (HELO mail-pa0-f54.google.com) (209.85.220.54) by mx3-pf3.cudamail.com with ESMTPS (RC4-SHA encrypted); 16 Oct 2015 10:52:40 -0000 Received-SPF: none (mx3-pf3.cudamail.com: domain at netronome.com does not designate permitted sender hosts) X-Barracuda-RBL-Trusted-Forwarder: 209.85.220.54 Received: by pabrc13 with SMTP id rc13so117925340pab.0 for ; Fri, 16 Oct 2015 03:52:40 -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:in-reply-to :references; bh=X1Httzvch5mrKBFrP7QNGDqf0vWv2kKkljWxkqtK/LU=; b=MPvZuNHJvT1/LJdAGeBviR7Qd3fmVEQRFtYln4jEwpNlAXXah7MFcVNSsWeIbCqKNx 8duzTxylsBLH8P+ffe8rlZ83V/QHo8555faYwkF4C31PTkTGIqYm3oi2bcC3MgOX8R1m IRtFqsOkBN5dlOew6loYVqjyFdxOuqIPeP0umaq/byr/adGF5ZGyU1WfpBlDEfKVHQl9 2801Qcy1KVjBCmWFnJUeccwAgRKYL96VV492Rwjb9nBiB5A5bH3kZsw5pjkCLiecpGVu 8fg79pkmVjUolRIb4y2KYb+7zI/r9ma/OKtC/5dd/xX4yUdJiZdkD9usCGKg8RY6Pda/ HNpg== X-Gm-Message-State: ALoCoQl5kRjJno8WoO3EpWL1iY4sl5cLoAyDdMnc0ZvGEvxC6K1sU+jqMRdwyTJQE5Hr8/XiLWKK X-Received: by 10.68.227.227 with SMTP id sd3mr16046795pbc.116.1444992760020; Fri, 16 Oct 2015 03:52:40 -0700 (PDT) Received: from penelope.isobedori.kobe.vergenet.net (g1-27-253-251-7.bmobile.ne.jp. [27.253.251.7]) by smtp.gmail.com with ESMTPSA id bo5sm20525555pbb.76.2015.10.16.03.52.35 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Fri, 16 Oct 2015 03:52:38 -0700 (PDT) X-CudaMail-Envelope-Sender: simon.horman@netronome.com X-Barracuda-Apparent-Source-IP: 27.253.251.7 From: Simon Horman To: dev@openvswitch.org X-CudaMail-MID: CM-V3-1015005760 X-CudaMail-DTE: 101615 X-CudaMail-Originating-IP: 209.85.220.54 Date: Fri, 16 Oct 2015 19:50:48 +0900 X-ASG-Orig-Subj: [##CM-V3-1015005760##][PATCH 2/2] ofproto: correct group fields command line option parsing Message-Id: <1444992648-27107-3-git-send-email-simon.horman@netronome.com> X-Mailer: git-send-email 2.1.4 In-Reply-To: <1444992648-27107-1-git-send-email-simon.horman@netronome.com> References: <1444992648-27107-1-git-send-email-simon.horman@netronome.com> X-GBUdb-Analysis: 0, 209.85.220.54, Ugly c=0.476605 p=-0.431373 Source Normal X-MessageSniffer-Rules: 0-0-0-19351-c X-Barracuda-Connect: UNKNOWN[192.168.14.3] X-Barracuda-Start-Time: 1444992765 X-Barracuda-Encrypted: DHE-RSA-AES256-SHA X-Barracuda-URL: https://web.cudamail.com:443/cgi-mod/mark.cgi X-Virus-Scanned: by bsmtpd at cudamail.com X-Barracuda-BRTS-Status: 1 X-Barracuda-Spam-Score: 0.60 X-Barracuda-Spam-Status: No, SCORE=0.60 using per-user scores of TAG_LEVEL=3.0 QUARANTINE_LEVEL=1000.0 KILL_LEVEL=3.0 tests=BSF_SC5_MJ1963, RDNS_NONE X-Barracuda-Spam-Report: Code version 3.2, rules version 3.2.3.23542 Rule breakdown below pts rule name description ---- ---------------------- -------------------------------------------------- 0.10 RDNS_NONE Delivered to trusted network by a host with no rDNS 0.50 BSF_SC5_MJ1963 Custom Rule MJ1963 Cc: Simon Horman Subject: [ovs-dev] [PATCH 2/2] ofproto: correct group fields command line option parsing 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" 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 --- lib/ofp-parse.c | 112 ++++++++++++++++++++++++++++++------------------------- tests/ofproto.at | 6 +-- 2 files changed, 65 insertions(+), 53 deletions(-) diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c index 843765650a44..647e019db35d 100644 --- a/lib/ofp-parse.c +++ b/lib/ofp-parse.c @@ -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; diff --git a/tests/ofproto.at b/tests/ofproto.at index 8699c4a29ed9..e0c4b68ec7f9 100644 --- a/tests/ofproto.at +++ b/tests/ofproto.at @@ -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])