From patchwork Fri Sep 25 20:49:46 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Joe Stringer X-Patchwork-Id: 522990 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 3D44E140281 for ; Sat, 26 Sep 2015 06:50:03 +1000 (AEST) Received: from archives.nicira.com (localhost [127.0.0.1]) by archives.nicira.com (Postfix) with ESMTP id E16FD10C08; Fri, 25 Sep 2015 13:49:56 -0700 (PDT) X-Original-To: dev@openvswitch.org Delivered-To: dev@openvswitch.org Received: from mx1e4.cudamail.com (mx1.cudamail.com [69.90.118.67]) by archives.nicira.com (Postfix) with ESMTPS id 9F8AB10BFF for ; Fri, 25 Sep 2015 13:49:55 -0700 (PDT) Received: from bar5.cudamail.com (unknown [192.168.21.12]) by mx1e4.cudamail.com (Postfix) with ESMTPS id EAEBB1E0530 for ; Fri, 25 Sep 2015 14:49:54 -0600 (MDT) X-ASG-Debug-ID: 1443214194-09eadd11e04ae090001-byXFYA Received: from mx1-pf1.cudamail.com ([192.168.24.1]) by bar5.cudamail.com with ESMTP id EEcrJsUojagylnYd (version=TLSv1 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO) for ; Fri, 25 Sep 2015 14:49:54 -0600 (MDT) X-Barracuda-Envelope-From: joestringer@nicira.com X-Barracuda-RBL-Trusted-Forwarder: 192.168.24.1 Received: from unknown (HELO mail-pa0-f45.google.com) (209.85.220.45) by mx1-pf1.cudamail.com with ESMTPS (RC4-SHA encrypted); 25 Sep 2015 20:49:53 -0000 Received-SPF: unknown (mx1-pf1.cudamail.com: Multiple SPF records returned) X-Barracuda-RBL-Trusted-Forwarder: 209.85.220.45 Received: by pacfv12 with SMTP id fv12so117484021pac.2 for ; Fri, 25 Sep 2015 13:49:53 -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=+Z9PoURIFB57hQP/8EX30OfplssUFQHjVo4S1Nj+rrU=; b=bly2C6qy4GTnRk+h1+IuT0DgtQJtQ4egaT/fQOML3jRj9TPpyZd/L9X4npCB5N6m3M COGejQyy4ou8jSouwr+OB9VYb5IxXsVgDKhqACkwX+0JuWv5eAicOP1krSPRDxnWAOm/ r/kZrn8bN8/RitezUoJIgSq1NvNqMi1qHB0xqCr/W0EsFYIaQF5KnRp9TBxCIMZsicSE 6LYBKNhQYwcWpJPmzVVY9jo6eQ/e38u7FRID5SQx6594ApyeaG4Xl181Vk4R7siIOsGd 23G6h1Z9tEg+yLdvQgY9juv4ZPcTkIjXqLd1rSxnjiWnasrukq7gsgk9sf1cp3C6B6nz N+1g== X-Gm-Message-State: ALoCoQnSns7rgrs0eUNcp2iTdQDBnTCwxFlqrWOe9mix3orKb5vJhKpreEHmB18kDri5eyPjD9qG X-Received: by 10.66.121.229 with SMTP id ln5mr9659049pab.133.1443214193002; Fri, 25 Sep 2015 13:49:53 -0700 (PDT) Received: from localhost.localdomain ([208.91.2.4]) by smtp.gmail.com with ESMTPSA id xa4sm5519782pac.28.2015.09.25.13.49.52 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Fri, 25 Sep 2015 13:49:52 -0700 (PDT) X-CudaMail-Envelope-Sender: joestringer@nicira.com X-Barracuda-Apparent-Source-IP: 208.91.2.4 From: Joe Stringer To: dev@openvswitch.org X-CudaMail-Whitelist-To: dev@openvswitch.org X-CudaMail-MID: CM-E1-924078175 X-CudaMail-DTE: 092515 X-CudaMail-Originating-IP: 209.85.220.45 Date: Fri, 25 Sep 2015 13:49:46 -0700 X-ASG-Orig-Subj: [##CM-E1-924078175##][PATCH 2/2] ofp-actions: Make parse_reg_load() accept set_field syntax. Message-Id: <1443214186-50469-2-git-send-email-joestringer@nicira.com> X-Mailer: git-send-email 2.1.4 In-Reply-To: <1443214186-50469-1-git-send-email-joestringer@nicira.com> References: <1443214186-50469-1-git-send-email-joestringer@nicira.com> X-Barracuda-Connect: UNKNOWN[192.168.24.1] X-Barracuda-Start-Time: 1443214194 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 Subject: [ovs-dev] [PATCH 2/2] ofp-actions: Make parse_reg_load() accept set_field syntax. 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" Commit 7eb4b1f1d "ofp-actions: Support OF1.5 (draft) masked Set-Field, merge with reg_load." purportedly merged the set_field and reg_load fields, including the commandline parsing to use the metafield syntax like "load:192.168.0.1->OXM_OF_IPV4_SRC". However, the actual reg_load parsing code was not updated. This patch takes advantage of the newly refactored parse_set_field() helpers to make parse_reg_load() conform to the documentation, which allows more thorough validation of inputs, and allows reg_load to parse fields that are wider than 64 bits, as well as masked fields. Note that this disallows syntax that was previously allowed, such as: load:0x0a000001->OXM_OF_IPV4_SRC Signed-off-by: Joe Stringer --- Note that v2.4.0 includes the aforementioned commit; we should consider either backporting this fix to branch-2.4, or reverting the documentation modification on branch-2.4 to be consistent with the previous syntax. --- NEWS | 3 +++ lib/ofp-actions.c | 57 +++++++++++++++++++++++++++++++++++++-------------- tests/ofproto-dpif.at | 6 +++--- 3 files changed, 48 insertions(+), 18 deletions(-) diff --git a/NEWS b/NEWS index ca22c8e..ef0348b 100644 --- a/NEWS +++ b/NEWS @@ -21,6 +21,9 @@ Post-v2.4.0 targets to run a new system testsuite. These tests can be run inside a Vagrant box. See INSTALL.md for details - Dropped support for GRE64 tunnel. + - ovs-ofctl syntax for the "reg_load" action now validates inputs to + ensure they are of the format that the field accepts. Simply loading + hexadecimal values into any field will no longer work. v2.4.0 - 20 Aug 2015 diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c index d33e429..153c712 100644 --- a/lib/ofp-actions.c +++ b/lib/ofp-actions.c @@ -2571,36 +2571,63 @@ error: } static char * OVS_WARN_UNUSED_RESULT -parse_reg_load(char *arg, struct ofpbuf *ofpacts) +parse_reg_load__(char *arg, struct ofpbuf *ofpacts) { struct ofpact_set_field *sf = ofpact_put_reg_load(ofpacts); - const char *full_arg = arg; - uint64_t value = strtoull(arg, (char **) &arg, 0); + union mf_value immediate, mask; + const struct mf_field *mf; + char *key, *value, *delim; struct mf_subfield dst; - char *error; + char *error = NULL; - if (strncmp(arg, "->", 2)) { - return xasprintf("%s: missing `->' following value", full_arg); + error = set_field_split_str(arg, &key, &value, &delim); + if (error) { + return error; } - arg += 2; - error = mf_parse_subfield(&dst, arg); + + error = mf_parse_subfield(&dst, key); if (error) { return error; } + delim[0] = '\0'; + mf = dst.field; - if (dst.n_bits < 64 && (value >> dst.n_bits) != 0) { - return xasprintf("%s: value %"PRIu64" does not fit into %d bits", - full_arg, value, dst.n_bits); + error = set_field_parse__(mf, key, value, &immediate, &mask); + if (error) { + return error; } - sf->field = dst.field; + if (!bitwise_is_all_zeros(&immediate, mf->n_bytes, dst.n_bits, + mf->n_bytes * 8 - dst.n_bits)) { + struct ds ds; + + ds_init(&ds); + mf_format(mf, &immediate, &mask, &ds); + error = xasprintf("%s: value %s does not fit into %d bits", + arg, ds_cstr(&ds), dst.n_bits); + ds_destroy(&ds); + return error; + } + + sf->field = mf; memset(&sf->value, 0, sizeof sf->value); - bitwise_put(value, &sf->value, dst.field->n_bytes, dst.ofs, dst.n_bits); - bitwise_put(UINT64_MAX, &sf->mask, - dst.field->n_bytes, dst.ofs, dst.n_bits); + bitwise_copy(&immediate, mf->n_bytes, 0, &sf->value, mf->n_bytes, + dst.ofs, dst.n_bits); + bitwise_copy(&mask, mf->n_bytes, 0, &sf->mask, mf->n_bytes, + dst.ofs, dst.n_bits); + return NULL; } +static char * OVS_WARN_UNUSED_RESULT +parse_reg_load(char *arg, struct ofpbuf *ofpacts) +{ + char *copy = xstrdup(arg); + char *error = parse_reg_load__(copy, ofpacts); + free(copy); + return error; +} + static void format_SET_FIELD(const struct ofpact_set_field *a, struct ds *s) { diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at index 9609d2d..c211a50 100644 --- a/tests/ofproto-dpif.at +++ b/tests/ofproto-dpif.at @@ -1292,7 +1292,7 @@ cookie=0x7 table=5 in_port=84 actions=load:5->NXM_NX_REG4[[]],load:6->NXM_NX_TUN cookie=0x8 table=6 in_port=85 actions=mod_tp_src:85,controller,resubmit(86,7) cookie=0x9 table=7 in_port=86 actions=mod_tp_dst:86,controller,controller cookie=0xa dl_src=40:44:44:44:44:41 actions=mod_vlan_vid:99,mod_vlan_pcp:1,controller -cookie=0xd dl_src=80:88:88:88:88:88 arp actions=load:2->OXM_OF_ARP_OP[[]],controller,load:0xc0a88001->OXM_OF_ARP_SPA[[]],controller,load:0x404444444441->OXM_OF_ARP_THA[[]],load:0x01010101->OXM_OF_ARP_SPA[[]],load:0x02020202->OXM_OF_ARP_TPA[[]],controller +cookie=0xd dl_src=80:88:88:88:88:88 arp actions=load:2->OXM_OF_ARP_OP[[]],controller,load:192.168.128.1->OXM_OF_ARP_SPA[[]],controller,load:40:44:44:44:44:41->OXM_OF_ARP_THA[[]],load:1.1.1.1->OXM_OF_ARP_SPA[[]],load:2.2.2.2->OXM_OF_ARP_TPA[[]],controller ]) AT_CHECK([ovs-ofctl add-flows br0 flows.txt]) @@ -1564,7 +1564,7 @@ cookie=0xd dl_src=60:66:66:66:66:66 actions=pop_mpls:0x0800,controller cookie=0xc dl_src=70:77:77:77:77:77 actions=push_mpls:0x8848,load:1000->OXM_OF_MPLS_LABEL[[]],load:7->OXM_OF_MPLS_TC[[]],controller cookie=0xd dl_src=60:66:66:66:00:01 actions=pop_mpls:0x0800,dec_ttl,controller -cookie=0xd dl_src=60:66:66:66:00:02 actions=pop_mpls:0x0800,load:0xa000001->OXM_OF_IPV4_DST[[]],controller +cookie=0xd dl_src=60:66:66:66:00:02 actions=pop_mpls:0x0800,load:10.0.0.1->OXM_OF_IPV4_DST[[]],controller cookie=0xd dl_src=60:66:66:66:00:03 actions=pop_mpls:0x0800,move:OXM_OF_IPV4_DST[[]]->OXM_OF_IPV4_SRC[[]],controller cookie=0xd dl_src=60:66:66:66:00:04 actions=pop_mpls:0x0800,push:OXM_OF_IPV4_DST[[]],pop:OXM_OF_IPV4_SRC[[]],controller cookie=0xd dl_src=60:66:66:66:00:05 actions=pop_mpls:0x0800,multipath(eth_src,50,modulo_n,256,0,OXM_OF_IPV4_SRC[[0..7]]),controller @@ -3020,7 +3020,7 @@ OVS_VSWITCHD_START ADD_OF_PORTS([br0], [1], [2]) ovs-vsctl -- set Interface p2 type=dummy options:pcap=p2.pcap -ovs-ofctl add-flow br0 'in_port=1,arp actions=load:2->OXM_OF_ARP_OP[[]],2,load:0xc0a88001->OXM_OF_ARP_SPA[[]],2,load:0x404444444441->OXM_OF_ARP_THA[[]],2' +ovs-ofctl add-flow br0 'in_port=1,arp actions=load:2->OXM_OF_ARP_OP[[]],2,load:192.168.128.1->OXM_OF_ARP_SPA[[]],2,load:40:44:44:44:44:41->OXM_OF_ARP_THA[[]],2' # Input some packets that should follow the arp modification slow-path. for i in 1 2 3; do