From patchwork Fri Jul 14 19:42:49 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Gregory Rose X-Patchwork-Id: 788755 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from mail.linuxfoundation.org (mail.linuxfoundation.org [140.211.169.12]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3x8NQ92fK9z9s2G for ; Sat, 15 Jul 2017 05:42:57 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="lhJ9O6O7"; dkim-atps=neutral Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id 66539C0C; Fri, 14 Jul 2017 19:42:55 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@mail.linuxfoundation.org Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org [172.17.192.35]) by mail.linuxfoundation.org (Postfix) with ESMTPS id 8C138BB3 for ; Fri, 14 Jul 2017 19:42:54 +0000 (UTC) X-Greylist: whitelisted by SQLgrey-1.7.6 Received: from mail-pf0-f194.google.com (mail-pf0-f194.google.com [209.85.192.194]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id 2015CCD for ; Fri, 14 Jul 2017 19:42:54 +0000 (UTC) Received: by mail-pf0-f194.google.com with SMTP id z6so11986411pfk.3 for ; Fri, 14 Jul 2017 12:42:53 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id; bh=QS+FSORXUQs6ZYhs02TUQTy3EJa9RKh3q1zVYqVU6ew=; b=lhJ9O6O7WWYlgxniHjTJ5b0TJtYw1uIT/wH2GOO2ntzYQKrZOZ994BDyD5Hlw38xtA yTbIpuwnorxXgOzbco/9ofhySZZstXBFa036O938j+EqFAeFd7ACW4SDruP34gdGCZRB ArEZCbSs/EljfhV/Ejyj6isVdFjB6qU/vZvWROA02Jj1pI1M/sa7K5H95G2KNUdX/Xeh oYoaFCuy0UN9Ccj9/g+L65wQHE1d+vIbgaifacmLX/cqZrD4GJVz0mhP1Zas3Q9jmmVK 7dl0unl3brQnyzBffpLq5nAfkyRzMwicXxxJJ+gqgC4/HE8TnZoKcj0RaZe4dT3DDXnL Uqdg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=QS+FSORXUQs6ZYhs02TUQTy3EJa9RKh3q1zVYqVU6ew=; b=suDuSWDZEri4R1jyTxWa8nxgpk5JwNkhLxnAjbLf7XuTmagaOl3HyB3sBPMQ67hhL7 6RQ68vY01daC6bRQSnOIdLISCbfw8xLMmcseZF4GERrRuj4/SE9DoWRTs6TicQkC7JzA Tf0rIXQ2Xpl2Sv/hHfA1jw93VMOx39vYZm3ASKHvejlZ4uLxGX7vwDkY7xBu7NyKy6hO L0LcX8Vuqgo8l43pSElGsZb/9+qPW4Lj7xHac/26VOpG/P9X7A+0CMVUEokbI+IQhTl+ Znf4VWzXqFVFbswq+4K5zyx3wcwPkHyD42BUbe7r4QVxOXpZxQ8rgXRdDtjjicQmHRna KNKw== X-Gm-Message-State: AIVw110k+zSpYd45O7uR9l56Y2drqnccbMQ0IIjchbgEtT3ob+luuYdS Exsl5/uLfeMyEjK9 X-Received: by 10.101.83.135 with SMTP id x7mr17071092pgq.63.1500061373671; Fri, 14 Jul 2017 12:42:53 -0700 (PDT) Received: from gizo.domain (184-100-139-69.ptld.qwest.net. [184.100.139.69]) by smtp.gmail.com with ESMTPSA id f24sm21711675pff.74.2017.07.14.12.42.52 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 14 Jul 2017 12:42:52 -0700 (PDT) From: Greg Rose To: netdev@vger.kernel.org Date: Fri, 14 Jul 2017 12:42:49 -0700 Message-Id: <1500061369-27918-1-git-send-email-gvrose8192@gmail.com> X-Mailer: git-send-email 1.8.3.1 X-Spam-Status: No, score=-1.2 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_ENVFROM_END_DIGIT,FREEMAIL_FROM, RCVD_IN_DNSWL_NONE,RCVD_IN_SORBS_SPAM autolearn=no version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on smtp1.linux-foundation.org Cc: dev@openvswitch.org Subject: [ovs-dev] [PATCH V3 net] openvswitch: Fix for force/commit action failures X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: ovs-dev-bounces@openvswitch.org Errors-To: ovs-dev-bounces@openvswitch.org When there is an established connection in direction A->B, it is possible to receive a packet on port B which then executes ct(commit,force) without first performing ct() - ie, a lookup. In this case, we would expect that this packet can delete the existing entry so that we can commit a connection with direction B->A. However, currently we only perform a check in skb_nfct_cached() for whether OVS_CS_F_TRACKED is set and OVS_CS_F_INVALID is not set, ie that a lookup previously occurred. In the above scenario, a lookup has not occurred but we should still be able to statelessly look up the existing entry and potentially delete the entry if it is in the opposite direction. This patch extends the check to also hint that if the action has the force flag set, then we will lookup the existing entry so that the force check at the end of skb_nfct_cached has the ability to delete the connection. Fixes: dd41d330b03 ("openvswitch: Add force commit.") CC: Pravin Shelar CC: dev@openvswitch.org Signed-off-by: Joe Stringer Signed-off-by: Greg Rose --- V2: Make sure nf_conntrack_in() is called for force case V3: Fix compiler warning for newer compilers --- net/openvswitch/conntrack.c | 51 ++++++++++++++++++++++++++++++++------------- 1 file changed, 36 insertions(+), 15 deletions(-) diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c index 08679eb..e3c4c6c 100644 --- a/net/openvswitch/conntrack.c +++ b/net/openvswitch/conntrack.c @@ -629,6 +629,34 @@ static int handle_fragments(struct net *net, struct sw_flow_key *key, return ct; } +static +struct nf_conn *ovs_ct_executed(struct net *net, + const struct sw_flow_key *key, + const struct ovs_conntrack_info *info, + struct sk_buff *skb, + bool *ct_executed) +{ + struct nf_conn *ct = NULL; + + /* If no ct, check if we have evidence that an existing conntrack entry + * might be found for this skb. This happens when we lose a skb->_nfct + * due to an upcall, or if the direction is being forced. If the + * connection was not confirmed, it is not cached and needs to be run + * through conntrack again. + */ + *ct_executed = (key->ct_state & OVS_CS_F_TRACKED) && + !(key->ct_state & OVS_CS_F_INVALID) && + (key->ct_zone == info->zone.id); + + if (*ct_executed || (!key->ct_state && info->force)) { + ct = ovs_ct_find_existing(net, &info->zone, info->family, skb, + !!(key->ct_state & + OVS_CS_F_NAT_MASK)); + } + + return ct; +} + /* Determine whether skb->_nfct is equal to the result of conntrack lookup. */ static bool skb_nfct_cached(struct net *net, const struct sw_flow_key *key, @@ -637,24 +665,17 @@ static bool skb_nfct_cached(struct net *net, { enum ip_conntrack_info ctinfo; struct nf_conn *ct; + bool ct_executed = true; ct = nf_ct_get(skb, &ctinfo); - /* If no ct, check if we have evidence that an existing conntrack entry - * might be found for this skb. This happens when we lose a skb->_nfct - * due to an upcall. If the connection was not confirmed, it is not - * cached and needs to be run through conntrack again. - */ - if (!ct && key->ct_state & OVS_CS_F_TRACKED && - !(key->ct_state & OVS_CS_F_INVALID) && - key->ct_zone == info->zone.id) { - ct = ovs_ct_find_existing(net, &info->zone, info->family, skb, - !!(key->ct_state - & OVS_CS_F_NAT_MASK)); - if (ct) - nf_ct_get(skb, &ctinfo); - } if (!ct) + ct = ovs_ct_executed(net, key, info, skb, &ct_executed); + + if (ct) + nf_ct_get(skb, &ctinfo); + else return false; + if (!net_eq(net, read_pnet(&ct->ct_net))) return false; if (!nf_ct_zone_equal_any(info->ct, nf_ct_zone(ct))) @@ -679,7 +700,7 @@ static bool skb_nfct_cached(struct net *net, return false; } - return true; + return ct_executed; } #ifdef CONFIG_NF_NAT_NEEDED