From patchwork Sat Nov 25 14:02:12 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: zhangliping X-Patchwork-Id: 841245 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=163.com header.i=@163.com header.b="ifKoGpCZ"; dkim-atps=neutral Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3ykZXs5c3Tz9ryv for ; Sun, 26 Nov 2017 01:03:41 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751754AbdKYODj (ORCPT ); Sat, 25 Nov 2017 09:03:39 -0500 Received: from m12-14.163.com ([220.181.12.14]:46991 "EHLO m12-14.163.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751714AbdKYODi (ORCPT ); Sat, 25 Nov 2017 09:03:38 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=163.com; s=s110527; h=From:Subject:Date:Message-Id; bh=NK9z1US8SCr0N6vbb7 p0mSjbori/hhGwGu8DPE2+IlA=; b=ifKoGpCZ0kiRbFnpBT2shOcGMgQvoZCWgq 3gw3PkFBv73QFrXZrcULipsgJRRotwhqPDnJwYWdIPRrTmMXbf+/kpZzipe7v/Y/ tX3GTphrF19cN3/Pbq+E4AV6uGGUVdCmbAj1jbqqSd8RxEkaYyDuwQ+MUK9qEkMV Co1iDjaBI= Received: from localhost.localdomain (unknown [180.164.180.9]) by smtp10 (Coremail) with SMTP id DsCowACXJKUeeBlawOzgBA--.43426S2; Sat, 25 Nov 2017 22:03:27 +0800 (CST) From: zhangliping To: pshelar@nicira.com, davem@davemloft.net, netdev@vger.kernel.org Cc: zhangliping Subject: [PATCH net] openvswitch: fix the incorrect flow action alloc size Date: Sat, 25 Nov 2017 22:02:12 +0800 Message-Id: <20171125140212.105418-1-zhanglkk1990@163.com> X-Mailer: git-send-email 2.13.4 X-CM-TRANSID: DsCowACXJKUeeBlawOzgBA--.43426S2 X-Coremail-Antispam: 1Uf129KBjvJXoWxXw4DXryfKrW8urWUWrykZrb_yoW5Wr1kpF 4vk34rXr1DAr17Wr40kr1kGFy5CFn5ArZ8CFWUGa48Zw1UKrykKayDKrW2kF98CFWfAr9x XryDKr1fCFWkAFJanT9S1TB71UUUUU7qnTZGkaVYY2UrUUUUjbIjqfuFe4nvWSU5nxnvy2 9KBjDUYxBIdaVFxhVjvjDU0xZFpf9x07bwVyxUUUUU= X-Originating-IP: [180.164.180.9] X-CM-SenderInfo: x2kd0w5onnimizq6il2tof0z/1tbivg+bZFZcPGGD0AAAsK Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org From: zhangliping If we want to add a datapath flow, which has more than 500 vxlan outputs' action, we will get the following error reports: openvswitch: netlink: Flow action size 32832 bytes exceeds max openvswitch: netlink: Flow action size 32832 bytes exceeds max openvswitch: netlink: Actions may not be safe on all matching packets ... ... It seems that we can simply enlarge the MAX_ACTIONS_BUFSIZE to fix it, but this is not the root cause. For example, for a vxlan output action, we need about 60 bytes for the nlattr, but after it is converted to the flow action, it only occupies 24 bytes. This means that we can still support more than 1000 vxlan output actions for a single datapath flow under the the current 32k max limitation. So even if the nla_len(attr) is larger than MAX_ACTIONS_BUFSIZE, we shouldn't report EINVAL and keep it move on, as the judgement can be done by the reserve_sfa_size. Signed-off-by: zhangliping Acked-by: Pravin B Shelar --- net/openvswitch/flow_netlink.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c index dc42479..624ea74 100644 --- a/net/openvswitch/flow_netlink.c +++ b/net/openvswitch/flow_netlink.c @@ -2241,14 +2241,11 @@ int ovs_nla_put_mask(const struct sw_flow *flow, struct sk_buff *skb) #define MAX_ACTIONS_BUFSIZE (32 * 1024) -static struct sw_flow_actions *nla_alloc_flow_actions(int size, bool log) +static struct sw_flow_actions *nla_alloc_flow_actions(int size) { struct sw_flow_actions *sfa; - if (size > MAX_ACTIONS_BUFSIZE) { - OVS_NLERR(log, "Flow action size %u bytes exceeds max", size); - return ERR_PTR(-EINVAL); - } + WARN_ON_ONCE(size > MAX_ACTIONS_BUFSIZE); sfa = kmalloc(sizeof(*sfa) + size, GFP_KERNEL); if (!sfa) @@ -2321,12 +2318,15 @@ static struct nlattr *reserve_sfa_size(struct sw_flow_actions **sfa, new_acts_size = ksize(*sfa) * 2; if (new_acts_size > MAX_ACTIONS_BUFSIZE) { - if ((MAX_ACTIONS_BUFSIZE - next_offset) < req_size) + if ((MAX_ACTIONS_BUFSIZE - next_offset) < req_size) { + OVS_NLERR(log, "Flow action size exceeds max %u", + MAX_ACTIONS_BUFSIZE); return ERR_PTR(-EMSGSIZE); + } new_acts_size = MAX_ACTIONS_BUFSIZE; } - acts = nla_alloc_flow_actions(new_acts_size, log); + acts = nla_alloc_flow_actions(new_acts_size); if (IS_ERR(acts)) return (void *)acts; @@ -3059,7 +3059,7 @@ int ovs_nla_copy_actions(struct net *net, const struct nlattr *attr, { int err; - *sfa = nla_alloc_flow_actions(nla_len(attr), log); + *sfa = nla_alloc_flow_actions(min(nla_len(attr), MAX_ACTIONS_BUFSIZE)); if (IS_ERR(*sfa)) return PTR_ERR(*sfa);