From patchwork Fri Sep 18 20:02:40 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: gavin_remaley@selinc.com X-Patchwork-Id: 519570 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 D5829140787 for ; Sat, 19 Sep 2015 06:03:31 +1000 (AEST) Received: from archives.nicira.com (localhost [127.0.0.1]) by archives.nicira.com (Postfix) with ESMTP id 032F110C30; Fri, 18 Sep 2015 13:03:31 -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 DB14D10C30 for ; Fri, 18 Sep 2015 13:03:29 -0700 (PDT) Received: from bar3.cudamail.com (bar1 [192.168.15.1]) by mx3v1.cudamail.com (Postfix) with ESMTP id 1A6E16193E3 for ; Fri, 18 Sep 2015 14:03:29 -0600 (MDT) X-ASG-Debug-ID: 1442606608-03dd7b59580c8b0001-byXFYA Received: from mx3-pf1.cudamail.com ([192.168.14.2]) by bar3.cudamail.com with ESMTP id XoDR1so2xAAGbIyH (version=TLSv1 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO) for ; Fri, 18 Sep 2015 14:03:28 -0600 (MDT) X-Barracuda-Envelope-From: gavin_remaley@selinc.com X-Barracuda-RBL-Trusted-Forwarder: 192.168.14.2 Received: from unknown (HELO volare.selinc.com) (74.117.214.143) by mx3-pf1.cudamail.com with SMTP; 18 Sep 2015 20:02:42 -0000 Received-SPF: pass (mx3-pf1.cudamail.com: SPF record at selinc.com designates 74.117.214.143 as permitted sender) X-Barracuda-Apparent-Source-IP: 74.117.214.143 X-Barracuda-RBL-IP: 74.117.214.143 Received: from edison.ad.selinc.com (edison.ad.selinc.com [10.100.0.15]) by volare.selinc.com (8.15.0.59/8.14.7) with ESMTP id t8IK2fTt008702; Fri, 18 Sep 2015 13:02:41 -0700 In-Reply-To: <1441751019-8625-1-git-send-email-blp@nicira.com> References: <1441751019-8625-1-git-send-email-blp@nicira.com> To: Ben Pfaff MIME-Version: 1.0 X-CudaMail-MID: CM-V1-917048582 X-CudaMail-DTE: 091815 X-CudaMail-Originating-IP: 74.117.214.143 X-KeepSent: 5086A0D9:F9FB9FE8-88257EC4:006CBF7A; type=4; name=$KeepSent X-ASG-Orig-Subj: [##CM-V1-917048582##]Re: [PATCH 1/3] ofp-actions: Look inside write_actions for output ports and groups. X-Mailer: Lotus Notes Release 8.5.2 August 10, 2010 Message-ID: X-CudaMail-Envelope-Sender: gavin_remaley@selinc.com From: gavin_remaley@selinc.com Date: Fri, 18 Sep 2015 14:02:40 -0600 X-MIMETrack: Serialize by Router on Edison/SEL(Release 9.0.1FP3 HF515|April 25, 2015) at 09/18/2015 01:02:41 PM, Serialize complete at 09/18/2015 01:02:41 PM X-Proofpoint-Virus-Version: vendor=nai engine=5700 definitions=7928 signatures=670636 X-GBUdb-Analysis: 0, 74.117.214.143, Ugly c=0 p=0 Source New X-MessageSniffer-Rules: 0-0-0-18008-c X-Barracuda-Connect: UNKNOWN[192.168.14.2] X-Barracuda-Start-Time: 1442606608 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.10 X-Barracuda-Spam-Status: No, SCORE=0.10 using per-user scores of TAG_LEVEL=3.0 QUARANTINE_LEVEL=1000.0 KILL_LEVEL=3.0 tests=HTML_MESSAGE, NO_REAL_NAME, RDNS_NONE X-Barracuda-Spam-Report: Code version 3.2, rules version 3.2.3.22678 Rule breakdown below pts rule name description ---- ---------------------- -------------------------------------------------- 0.00 NO_REAL_NAME From: does not include a real name 0.00 HTML_MESSAGE BODY: HTML included in message 0.10 RDNS_NONE Delivered to trusted network by a host with no rDNS X-Content-Filtered-By: Mailman/MimeDel 2.1.16 Cc: dev@openvswitch.org Subject: Re: [ovs-dev] [PATCH 1/3] ofp-actions: Look inside write_actions for output ports and groups. 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: , Errors-To: dev-bounces@openvswitch.org Sender: "dev" This patch looks good, though it looks like the general issue with examining the Actions within a Write Actions instruction has several instances beyond what is addressed by this patch. I have noticed that an invalid Group ID within Write Actions is not caught (Flow is programmed with no error). That appears to be a problem in ofproto.c -> ofproto_check_ofpacts. A similar fix seemed to address that issue. I did a global search for OFPACT_FOR_EACH and examined the action processing. I see several other potential problems with Write Actions. ofp-actions.c -> ofpacts_check ofp-actions.c -> ofpacts_get_meter ofproto.c -> ofproto_check_ofpacts Thanks for your help. From: Ben Pfaff To: dev@openvswitch.org Cc: Ben Pfaff , Gavin Remaley Date: 09/08/2015 04:55 PM Subject: [PATCH 1/3] ofp-actions: Look inside write_actions for output ports and groups. The out_port and out_group matches only looked at apply_actions instructions, but my interpretation of the OpenFlow spec is that they should also look inside write_actions. Reported-by: Gavin Remaley Signed-off-by: Ben Pfaff --- AUTHORS | 1 + lib/ofp-actions.c | 21 +++++++++++++++++---- 2 files changed, 18 insertions(+), 4 deletions(-) case OFPACT_OUTPUT_REG: case OFPACT_BUNDLE: @@ -6113,7 +6119,6 @@ ofpact_outputs_to_port(const struct ofpact *ofpact, ofp_port_t port) case OFPACT_POP_MPLS: case OFPACT_SAMPLE: case OFPACT_CLEAR_ACTIONS: - case OFPACT_WRITE_ACTIONS: case OFPACT_GOTO_TABLE: case OFPACT_METER: case OFPACT_GROUP: @@ -6149,9 +6154,17 @@ ofpacts_output_to_group(const struct ofpact *ofpacts, size_t ofpacts_len, const struct ofpact *a; OFPACT_FOR_EACH (a, ofpacts, ofpacts_len) { - if (a->type == OFPACT_GROUP - && ofpact_get_GROUP(a)->group_id == group_id) { - return true; + if (a->type == OFPACT_GROUP) { + if (ofpact_get_GROUP(a)->group_id == group_id) { + return true; + } + } else if (a->type == OFPACT_WRITE_ACTIONS) { + const struct ofpact_nest *nested = ofpact_get_WRITE_ACTIONS(a); + if (ofpacts_output_to_group(nested->actions, + ofpact_nest_get_action_len(nested), + group_id)) { + return true; + } } } diff --git a/AUTHORS b/AUTHORS index a7f40bb..0c4d020 100644 --- a/AUTHORS +++ b/AUTHORS @@ -259,6 +259,7 @@ Eivind Bulie Haanaes Eric Lopez elopez@nicira.com Frido Roose fr.roose@gmail.com Gaetano Catalli gaetano.catalli@gmail.com +Gavin Remaley gavin_remaley@selinc.com George Shuklin amarao@desunote.ru Gerald Rogers gerald.rogers@intel.com Ghanem Bahri bahri.ghanem@gmail.com diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c index c46ce97..ee686c1 100644 --- a/lib/ofp-actions.c +++ b/lib/ofp-actions.c @@ -6072,6 +6072,12 @@ ofpact_outputs_to_port(const struct ofpact *ofpact, ofp_port_t port) return ofpact_get_ENQUEUE(ofpact)->port == port; case OFPACT_CONTROLLER: return port == OFPP_CONTROLLER; + case OFPACT_WRITE_ACTIONS: { + const struct ofpact_nest *nested = ofpact_get_WRITE_ACTIONS(ofpact); + return ofpacts_output_to_port(nested->actions, + ofpact_nest_get_action_len(nested), + port); + }