From patchwork Fri Jul 14 16:10:39 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Gregory Rose X-Patchwork-Id: 788616 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3x8Hjb3dl0z9s2s for ; Sat, 15 Jul 2017 02:10:59 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="VAcTUiIZ"; dkim-atps=neutral Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754584AbdGNQKr (ORCPT ); Fri, 14 Jul 2017 12:10:47 -0400 Received: from mail-pf0-f196.google.com ([209.85.192.196]:34432 "EHLO mail-pf0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754572AbdGNQKp (ORCPT ); Fri, 14 Jul 2017 12:10:45 -0400 Received: by mail-pf0-f196.google.com with SMTP id c24so11483606pfe.1 for ; Fri, 14 Jul 2017 09:10:45 -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=qHshaJXVHAPYNYu9FgpDP8rJSmdMh7DtWUpo8JKD2Ao=; b=VAcTUiIZsajJ4MIFDbAqiF6rqj+bp+CwpmyCtReUxUv7Vp+2MfyZWs4dX6OHvyfr5G fQwDVBGn0b+uDodzhwklZEyrBTaS5W70ZntusCRbNbAg2L8vvT1c7TNhIxyv5cbilw/T MrRu4Btb9Bm1B1BGiBh6jeKb6llhY19NAcN97bRFOkmry3ms9BlQfvAL/dRDRLLfvYD8 3DwJvERvN9AfjpkBArZsz+b+XZcSE4Ll5y1tAZ+q/qCb2Yx+0FAe7QIHw/pFHfmoHYN/ y/hOI1QGK/yIeoM/vpZmihA+eRsZbUcp5eAUTNDGBfQAAR/GcsMgPMGRdhCZ7Dq69Om3 II6w== 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=qHshaJXVHAPYNYu9FgpDP8rJSmdMh7DtWUpo8JKD2Ao=; b=rPXxiTzdJncjobzO3j92aivswKZjKs8FwAxavRjSY6FsADdLY+LIkm7/h86tXM4bzS 4MvejkAfmkwgENL3svKTPGg9KOkkh1ABsB5IWzIqHjuN6Fk6pdaq0oAHG9t1KkIF9HBf JxqeAmoEaBSIR8LRJtfCnP322k/F+jyG8dLdY/DtvJa6VgjcaJ5Rezt7tMiJCCdwNBcw KHSqssr5uyxUQ9LQZP74ndGJmD5rgpvK88c3p2BYyFe9n/TM1Mig2o8lza4QuwOR6hbk eyag0Sx//zk0OO+1z4kqI9sFmCq3OoAqwWtfCTOzjRQqAlnG4T1SK6VGal6MNT6tQZiw hD/g== X-Gm-Message-State: AIVw112AzXMACHdRIMFMqnCQaAcCWW84Mk8CuyN3uENnOOowWJFyGW0U bt2GB2Y6A6bi8Ae3 X-Received: by 10.99.250.69 with SMTP id g5mr15539756pgk.11.1500048644512; Fri, 14 Jul 2017 09:10:44 -0700 (PDT) Received: from gizo.domain (184-100-139-69.ptld.qwest.net. [184.100.139.69]) by smtp.gmail.com with ESMTPSA id 79sm19622417pfn.51.2017.07.14.09.10.42 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 14 Jul 2017 09:10:43 -0700 (PDT) From: Greg Rose To: netdev@vger.kernel.org Cc: Greg Rose , Pravin Shelar , dev@openvswitch.org, Joe Stringer Subject: [PATCH net] openvswitch: Fix for force/commit action failures Date: Fri, 14 Jul 2017 09:10:39 -0700 Message-Id: <1500048639-24169-1-git-send-email-gvrose8192@gmail.com> X-Mailer: git-send-email 1.8.3.1 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.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 Reviewed-by: Joe Stringer --- net/openvswitch/conntrack.c | 50 +++++++++++++++++++++++++++++++-------------- 1 file changed, 35 insertions(+), 15 deletions(-) diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c index 08679eb..1260f2b 100644 --- a/net/openvswitch/conntrack.c +++ b/net/openvswitch/conntrack.c @@ -629,6 +629,33 @@ static int handle_fragments(struct net *net, struct sw_flow_key *key, return ct; } +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 +664,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 +699,7 @@ static bool skb_nfct_cached(struct net *net, return false; } - return true; + return ct_executed; } #ifdef CONFIG_NF_NAT_NEEDED