From patchwork Wed Sep 23 01:17:25 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jesse Gross X-Patchwork-Id: 521528 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 09C8F140180 for ; Wed, 23 Sep 2015 11:17:33 +1000 (AEST) Received: from archives.nicira.com (localhost [127.0.0.1]) by archives.nicira.com (Postfix) with ESMTP id 52E5A10A2B; Tue, 22 Sep 2015 18:17:33 -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 25D91101F5 for ; Tue, 22 Sep 2015 18:17:32 -0700 (PDT) Received: from bar4.cudamail.com (bar2 [192.168.15.2]) by mx3v1.cudamail.com (Postfix) with ESMTP id A7198618C42 for ; Tue, 22 Sep 2015 19:17:31 -0600 (MDT) X-ASG-Debug-ID: 1442971051-03dc211a5202660001-byXFYA Received: from mx3-pf1.cudamail.com ([192.168.14.2]) by bar4.cudamail.com with ESMTP id 5J1X979BHEKekBou (version=TLSv1 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO) for ; Tue, 22 Sep 2015 19:17:31 -0600 (MDT) X-Barracuda-Envelope-From: jesse@nicira.com X-Barracuda-RBL-Trusted-Forwarder: 192.168.14.2 Received: from unknown (HELO mail-pa0-f41.google.com) (209.85.220.41) by mx3-pf1.cudamail.com with ESMTPS (RC4-SHA encrypted); 23 Sep 2015 01:17:30 -0000 Received-SPF: unknown (mx3-pf1.cudamail.com: Multiple SPF records returned) X-Barracuda-RBL-Trusted-Forwarder: 209.85.220.41 Received: by padhy16 with SMTP id hy16so24680422pad.1 for ; Tue, 22 Sep 2015 18:17:30 -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:subject:date:message-id; bh=EKWgIAu81PcklZMuMjvRDrDYqd+3JPeTSiGiTot1Vno=; b=Q14/s31TgVY2vS0tJ6LioP43m0Limz7HcgM+oVlFhjwLYqvCmg4pg5bZjl+XPrsJfY WHK1I8IvS3eBsHCKLjohq72Gt50bVzVvUoaz+YMvcpdk/kslS8s8F91cnS7SYTCo483g vH/UASx+hgMbu7vb3Ri8Sd6E0FTQdJeFLi1RQQGwK5+LUH5Dga36Uj6a5G9cAOd8uIRF cV7qtG4YlGhlaHsyHg/BD3bYs8RPReevPaaNO/RxezP29F8V+K1KMCllY4C8xhUGVPsY hwn2TduZ4VsJkOhMvJAXVSzgjZVJBakWNpfCZHPe+v0u9ZFN6f8kg6tdtfXUCg5LcybR OY6A== X-Gm-Message-State: ALoCoQmXmQbztZk43f2bkxn4Zrmrl72k7Rk6J6aPVEWCSTlNZZTDkR4StXeGYsB+RNwbZFil4Lba X-Received: by 10.66.252.5 with SMTP id zo5mr33419077pac.96.1442971050522; Tue, 22 Sep 2015 18:17:30 -0700 (PDT) Received: from localhost.localdomain (c-71-202-123-143.hsd1.ca.comcast.net. [71.202.123.143]) by smtp.gmail.com with ESMTPSA id bz4sm4579654pbd.6.2015.09.22.18.17.28 for (version=TLSv1/SSLv3 cipher=OTHER); Tue, 22 Sep 2015 18:17:29 -0700 (PDT) X-CudaMail-Envelope-Sender: jesse@nicira.com X-Barracuda-Apparent-Source-IP: 71.202.123.143 From: Jesse Gross To: dev@openvswitch.org X-CudaMail-Whitelist-To: dev@openvswitch.org X-CudaMail-MID: CM-V1-921063726 X-CudaMail-DTE: 092215 X-CudaMail-Originating-IP: 209.85.220.41 Date: Tue, 22 Sep 2015 18:17:25 -0700 X-ASG-Orig-Subj: [##CM-V1-921063726##][PATCH] datapath: Backport "openvswitch: Zero flows on allocation." Message-Id: <1442971045-86082-1-git-send-email-jesse@nicira.com> X-Mailer: git-send-email 2.1.4 X-Barracuda-Connect: UNKNOWN[192.168.14.2] X-Barracuda-Start-Time: 1442971051 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] datapath: Backport "openvswitch: Zero flows on allocation." 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: openvswitch: Zero flows on allocation. When support for megaflows was introduced, OVS needed to start installing flows with a mask applied to them. Since masking is an expensive operation, OVS also had an optimization that would only take the parts of the flow keys that were covered by a non-zero mask. The values stored in the remaining pieces should not matter because they are masked out. While this works fine for the purposes of matching (which must always look at the mask), serialization to netlink can be problematic. Since the flow and the mask are serialized separately, the uninitialized portions of the flow can be encoded with whatever values happen to be present. In terms of functionality, this has little effect since these fields will be masked out by definition. However, it leaks kernel memory to userspace, which is a potential security vulnerability. It is also possible that other code paths could look at the masked key and get uninitialized data, although this does not currently appear to be an issue in practice. This removes the mask optimization for flows that are being installed. This was always intended to be the case as the mask optimizations were really targetting per-packet flow operations. Fixes: 03f0d916 ("openvswitch: Mega flow implementation") Signed-off-by: Jesse Gross Acked-by: Pravin B Shelar Signed-off-by: David S. Miller Upstream: ae5f2fb1 ("openvswitch: Zero flows on allocation.") Signed-off-by: Jesse Gross Acked-by: Pravin B Shelar --- datapath/datapath.c | 4 ++-- datapath/flow_table.c | 23 ++++++++++++----------- datapath/flow_table.h | 2 +- 3 files changed, 15 insertions(+), 14 deletions(-) diff --git a/datapath/datapath.c b/datapath/datapath.c index 1115649..5f36242 100644 --- a/datapath/datapath.c +++ b/datapath/datapath.c @@ -930,7 +930,7 @@ static int ovs_flow_cmd_new(struct sk_buff *skb, struct genl_info *info) if (error) goto err_kfree_flow; - ovs_flow_mask_key(&new_flow->key, &key, &mask); + ovs_flow_mask_key(&new_flow->key, &key, true, &mask); /* Extract flow identifier. */ error = ovs_nla_get_identifier(&new_flow->id, a[OVS_FLOW_ATTR_UFID], @@ -1057,7 +1057,7 @@ static struct sw_flow_actions *get_flow_actions(const struct nlattr *a, struct sw_flow_key masked_key; int error; - ovs_flow_mask_key(&masked_key, key, mask); + ovs_flow_mask_key(&masked_key, key, true, mask); error = ovs_nla_copy_actions(a, &masked_key, &acts, log); if (error) { OVS_NLERR(log, diff --git a/datapath/flow_table.c b/datapath/flow_table.c index c76f2a1..eeadf86 100644 --- a/datapath/flow_table.c +++ b/datapath/flow_table.c @@ -63,20 +63,21 @@ static u16 range_n_bytes(const struct sw_flow_key_range *range) } void ovs_flow_mask_key(struct sw_flow_key *dst, const struct sw_flow_key *src, - const struct sw_flow_mask *mask) + bool full, const struct sw_flow_mask *mask) { - const long *m = (const long *)((const u8 *)&mask->key + - mask->range.start); - const long *s = (const long *)((const u8 *)src + - mask->range.start); - long *d = (long *)((u8 *)dst + mask->range.start); + int start = full ? 0 : mask->range.start; + int len = full ? sizeof *dst : range_n_bytes(&mask->range); + const long *m = (const long *)((const u8 *)&mask->key + start); + const long *s = (const long *)((const u8 *)src + start); + long *d = (long *)((u8 *)dst + start); int i; - /* The memory outside of the 'mask->range' are not set since - * further operations on 'dst' only uses contents within - * 'mask->range'. + /* If 'full' is true then all of 'dst' is fully initialized. Otherwise, + * if 'full' is false the memory outside of the 'mask->range' is left + * uninitialized. This can be used as an optimization when further + * operations on 'dst' only use contents within 'mask->range'. */ - for (i = 0; i < range_n_bytes(&mask->range); i += sizeof(long)) + for (i = 0; i < len; i += sizeof(long)) *d++ = *s++ & *m++; } @@ -554,7 +555,7 @@ static struct sw_flow *masked_flow_lookup(struct table_instance *ti, u32 hash; struct sw_flow_key masked_key; - ovs_flow_mask_key(&masked_key, unmasked, mask); + ovs_flow_mask_key(&masked_key, unmasked, false, mask); hash = flow_hash(&masked_key, &mask->range); head = find_bucket(ti, hash); (*n_mask_hit)++; diff --git a/datapath/flow_table.h b/datapath/flow_table.h index 70d04be..8fa99d8 100644 --- a/datapath/flow_table.h +++ b/datapath/flow_table.h @@ -99,5 +99,5 @@ struct sw_flow *ovs_flow_tbl_lookup_ufid(struct flow_table *, bool ovs_flow_cmp(const struct sw_flow *, const struct sw_flow_match *); void ovs_flow_mask_key(struct sw_flow_key *dst, const struct sw_flow_key *src, - const struct sw_flow_mask *mask); + bool full, const struct sw_flow_mask *mask); #endif /* flow_table.h */