From patchwork Tue Sep 29 20:40:25 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Joe Stringer X-Patchwork-Id: 524015 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 31E7B140180 for ; Wed, 30 Sep 2015 06:41:09 +1000 (AEST) Received: from archives.nicira.com (localhost [127.0.0.1]) by archives.nicira.com (Postfix) with ESMTP id 4D1EB106CC; Tue, 29 Sep 2015 13:40:59 -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 67A51106C9 for ; Tue, 29 Sep 2015 13:40:58 -0700 (PDT) Received: from bar5.cudamail.com (unknown [192.168.21.12]) by mx1e4.cudamail.com (Postfix) with ESMTPS id DF5D31E0072 for ; Tue, 29 Sep 2015 14:40:57 -0600 (MDT) X-ASG-Debug-ID: 1443559257-09eadd11e06d2180001-byXFYA Received: from mx1-pf2.cudamail.com ([192.168.24.2]) by bar5.cudamail.com with ESMTP id C2eZ6tr7wiz0lk2H (version=TLSv1 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO) for ; Tue, 29 Sep 2015 14:40:57 -0600 (MDT) X-Barracuda-Envelope-From: joestringer@nicira.com X-Barracuda-RBL-Trusted-Forwarder: 192.168.24.2 Received: from unknown (HELO mail-pa0-f52.google.com) (209.85.220.52) by mx1-pf2.cudamail.com with ESMTPS (RC4-SHA encrypted); 29 Sep 2015 20:40:57 -0000 Received-SPF: unknown (mx1-pf2.cudamail.com: Multiple SPF records returned) X-Barracuda-RBL-Trusted-Forwarder: 209.85.220.52 Received: by padhy16 with SMTP id hy16so15874674pad.1 for ; Tue, 29 Sep 2015 13:40:56 -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=TmAWthvJdU2HBVvTaUHxpq/aLk5QKwSDnF79RklTU2KQJ5caI4I6Y5zcmraf2mg7+4 JZD4jA7eGecQ4098QV2LmB5KyIW29uk7M5A1eoBIHy+rwOYUuaJad0JyCx/l5hgarHLX QJEcQtZ97EalX2eSwHu4mz0ehTHclpSk6w2MClmUAMrBw3ZPRv+sX43CuT4Vrw1PT4Xb kldEDQYk+giDpCXmVuBrIkZ7fOUHCWLKgUxGr4TS+/CQVYPsVKpgHqVaQMR43ZiF0SHH h4iD35OqfeGRgg9WvwE5RWsFcx446dvVDGSmrRAx26c0pereUQ+btqhbYtZC/hN6Z1Ev u/rg== X-Gm-Message-State: ALoCoQl4MnRAGPUTHWSm60yipcG83n4wcVA1zdg4KokfIGoRFSITfUNAhQV9Zu359W/vOCKmvhRe X-Received: by 10.68.197.97 with SMTP id it1mr46633pbc.4.1443559256561; Tue, 29 Sep 2015 13:40:56 -0700 (PDT) Received: from localhost.localdomain ([208.91.2.4]) by smtp.gmail.com with ESMTPSA id fe8sm27450431pab.40.2015.09.29.13.40.54 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Tue, 29 Sep 2015 13:40:56 -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-E2-928101458 X-CudaMail-DTE: 092915 X-CudaMail-Originating-IP: 209.85.220.52 Date: Tue, 29 Sep 2015 13:40:25 -0700 X-ASG-Orig-Subj: [##CM-E2-928101458##][PATCHv3 02/11] ofp-actions: Make parse_reg_load() accept set_field syntax. Message-Id: <1443559234-7330-3-git-send-email-joestringer@nicira.com> X-Mailer: git-send-email 2.1.4 In-Reply-To: <1443559234-7330-1-git-send-email-joestringer@nicira.com> References: <1443559234-7330-1-git-send-email-joestringer@nicira.com> X-Barracuda-Connect: UNKNOWN[192.168.24.2] X-Barracuda-Start-Time: 1443559257 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] [PATCHv3 02/11] 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