From patchwork Wed Jun 29 13:35:30 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jarno Rajahalme X-Patchwork-Id: 642045 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from archives.nicira.com (archives.nicira.com [96.126.127.54]) by ozlabs.org (Postfix) with ESMTP id 3rfkHW5qkDz9t0y for ; Wed, 29 Jun 2016 23:37:11 +1000 (AEST) Received: from archives.nicira.com (localhost [127.0.0.1]) by archives.nicira.com (Postfix) with ESMTP id 926C210992; Wed, 29 Jun 2016 06:37:06 -0700 (PDT) X-Original-To: dev@openvswitch.org Delivered-To: dev@openvswitch.org Received: from mx1e3.cudamail.com (mx1.cudamail.com [69.90.118.67]) by archives.nicira.com (Postfix) with ESMTPS id 5190910879 for ; Wed, 29 Jun 2016 06:37:05 -0700 (PDT) Received: from bar5.cudamail.com (localhost [127.0.0.1]) by mx1e3.cudamail.com (Postfix) with ESMTPS id CF61E420577 for ; Wed, 29 Jun 2016 07:37:04 -0600 (MDT) X-ASG-Debug-ID: 1467207423-09eadd73997f1f0001-byXFYA Received: from mx3-pf3.cudamail.com ([192.168.14.3]) by bar5.cudamail.com with ESMTP id ePpmpIPqIRSM0aSF (version=TLSv1 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO) for ; Wed, 29 Jun 2016 07:37:04 -0600 (MDT) X-Barracuda-Envelope-From: jarno@ovn.org X-Barracuda-RBL-Trusted-Forwarder: 192.168.14.3 Received: from unknown (HELO relay6-d.mail.gandi.net) (217.70.183.198) by mx3-pf3.cudamail.com with ESMTPS (DHE-RSA-AES256-SHA encrypted); 29 Jun 2016 13:37:03 -0000 Received-SPF: pass (mx3-pf3.cudamail.com: SPF record at ovn.org designates 217.70.183.198 as permitted sender) X-Barracuda-Apparent-Source-IP: 217.70.183.198 X-Barracuda-RBL-IP: 217.70.183.198 Received: from mfilter28-d.gandi.net (mfilter28-d.gandi.net [217.70.178.159]) by relay6-d.mail.gandi.net (Postfix) with ESMTP id 52386FB8CD; Wed, 29 Jun 2016 15:37:02 +0200 (CEST) X-Virus-Scanned: Debian amavisd-new at mfilter28-d.gandi.net Received: from relay6-d.mail.gandi.net ([IPv6:::ffff:217.70.183.198]) by mfilter28-d.gandi.net (mfilter28-d.gandi.net [::ffff:10.0.15.180]) (amavisd-new, port 10024) with ESMTP id dmptEmhXC97I; Wed, 29 Jun 2016 15:37:00 +0200 (CEST) X-Originating-IP: 83.145.195.18 Received: from sc9-mailhost1.vmware.com (unknown [83.145.195.18]) (Authenticated sender: jarno@ovn.org) by relay6-d.mail.gandi.net (Postfix) with ESMTPSA id 2C1ADFB8D6; Wed, 29 Jun 2016 15:36:59 +0200 (CEST) X-CudaMail-Envelope-Sender: jarno@ovn.org From: Jarno Rajahalme To: dev@openvswitch.org X-CudaMail-Whitelist-To: dev@openvswitch.org X-CudaMail-MID: CM-V3-628013283 X-CudaMail-DTE: 062916 X-CudaMail-Originating-IP: 217.70.183.198 Date: Wed, 29 Jun 2016 06:35:30 -0700 X-ASG-Orig-Subj: [##CM-V3-628013283##][PATCH 2/2] datapath: Only set mark and labels with a commit flag. Message-Id: <1467207330-4730-2-git-send-email-jarno@ovn.org> X-Mailer: git-send-email 2.1.4 In-Reply-To: <1467207330-4730-1-git-send-email-jarno@ovn.org> References: <1467207330-4730-1-git-send-email-jarno@ovn.org> X-Barracuda-Connect: UNKNOWN[192.168.14.3] X-Barracuda-Start-Time: 1467207424 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] datapath: Only set mark and labels with a commit flag. 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" Upstream commit: commit 7d904c7bcd51f72579c0c3134a50896c5a3efb9f Author: Jarno Rajahalme Date: Tue Jun 21 14:59:38 2016 -0700 openvswitch: Only set mark and labels with a commit flag. Only set conntrack mark or labels when the commit flag is specified. This makes sure we can not set them before the connection has been persisted, as in that case the mark and labels would be lost in an event of an userspace upcall. OVS userspace already requires the commit flag to accept setting ct_mark and/or ct_labels. Validate for this in the kernel API. Signed-off-by: Jarno Rajahalme Signed-off-by: David S. Miller Signed-off-by: Jarno Rajahalme Acked-by: Joe Stringer --- datapath/conntrack.c | 76 +++++++++++++++++++++++++++++++++++----------------- 1 file changed, 51 insertions(+), 25 deletions(-) diff --git a/datapath/conntrack.c b/datapath/conntrack.c index 55f9867..9a79e73 100644 --- a/datapath/conntrack.c +++ b/datapath/conntrack.c @@ -879,6 +879,42 @@ static bool labels_nonzero(const struct ovs_key_ct_labels *labels) return false; } +/* Lookup connection and confirm if unconfirmed. */ +static int ovs_ct_commit(struct net *net, struct sw_flow_key *key, + const struct ovs_conntrack_info *info, + struct sk_buff *skb) +{ + int err; + + err = __ovs_ct_lookup(net, key, info, skb); + if (err) + return err; + + /* Apply changes before confirming the connection so that the initial + * conntrack NEW netlink event carries the values given in the CT + * action. + */ + if (info->mark.mask) { + err = ovs_ct_set_mark(skb, key, info->mark.value, + info->mark.mask); + if (err) + return err; + } + if (labels_nonzero(&info->labels.mask)) { + err = ovs_ct_set_labels(skb, key, &info->labels.value, + &info->labels.mask); + if (err) + return err; + } + /* This will take care of sending queued events even if the connection + * is already confirmed. + */ + if (nf_conntrack_confirm(skb) != NF_ACCEPT) + return -EINVAL; + + return 0; +} + /* Returns 0 on success, -EINPROGRESS if 'skb' is stolen, or other nonzero * value if 'skb' is freed. */ @@ -900,34 +936,10 @@ int ovs_ct_execute(struct net *net, struct sk_buff *skb, } if (info->commit) - err = __ovs_ct_lookup(net, key, info, skb); + err = ovs_ct_commit(net, key, info, skb); else err = ovs_ct_lookup(net, key, info, skb); - if (err) - goto err; - /* Apply changes before confirming the connection so that the initial - * conntrack NEW netlink event carries the values given in the CT - * action. - */ - if (info->mark.mask) { - err = ovs_ct_set_mark(skb, key, info->mark.value, - info->mark.mask); - if (err) - goto err; - } - if (labels_nonzero(&info->labels.mask)) { - err = ovs_ct_set_labels(skb, key, &info->labels.value, - &info->labels.mask); - if (err) - goto err; - } - /* This will take care of sending queued events even if the connection - * is already confirmed. - */ - if (info->commit && nf_conntrack_confirm(skb) != NF_ACCEPT) - err = -EINVAL; -err: skb_push(skb, nh_ofs); if (err) kfree_skb(skb); @@ -1189,6 +1201,20 @@ static int parse_ct(const struct nlattr *attr, struct ovs_conntrack_info *info, } } +#ifdef CONFIG_NF_CONNTRACK_MARK + if (!info->commit && info->mark.mask) { + OVS_NLERR(log, + "Setting conntrack mark requires 'commit' flag."); + return -EINVAL; + } +#endif +#ifdef CONFIG_NF_CONNTRACK_LABELS + if (!info->commit && labels_nonzero(&info->labels.mask)) { + OVS_NLERR(log, + "Setting conntrack labels requires 'commit' flag."); + return -EINVAL; + } +#endif if (rem > 0) { OVS_NLERR(log, "Conntrack attr has %d unknown bytes", rem); return -EINVAL;