From patchwork Thu Jul 3 14:29:44 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Estrin, Alex" X-Patchwork-Id: 4474321 Return-Path: X-Original-To: patchwork-linux-rdma@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 843519F387 for ; Thu, 3 Jul 2014 14:30:06 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 7BAE620396 for ; Thu, 3 Jul 2014 14:30:05 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id DCD262039E for ; Thu, 3 Jul 2014 14:30:02 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965144AbaGCO3x (ORCPT ); Thu, 3 Jul 2014 10:29:53 -0400 Received: from mga02.intel.com ([134.134.136.20]:5126 "EHLO mga02.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S964928AbaGCO3v (ORCPT ); Thu, 3 Jul 2014 10:29:51 -0400 Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga101.jf.intel.com with ESMTP; 03 Jul 2014 07:29:50 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.01,595,1400050800"; d="scan'208";a="567902677" Received: from sedona.ch.intel.com ([143.182.228.65]) by orsmga002.jf.intel.com with ESMTP; 03 Jul 2014 07:29:46 -0700 Received: from phlsvsles11.ph.intel.com (phlsvsles11.ph.intel.com [10.228.195.43]) by sedona.ch.intel.com (8.13.6/8.14.3/Standard MailSET/Hub) with ESMTP id s63ETjIC020663; Thu, 3 Jul 2014 07:29:45 -0700 Received: from phlsvslse11.ph.intel.com (localhost [127.0.0.1]) by phlsvsles11.ph.intel.com with ESMTP id s63ETiTF022946; Thu, 3 Jul 2014 10:29:44 -0400 Subject: [PATCH v6 1/1] IPoIB: Avoid multicast join attempts when having invalid p_key To: Roland Dreier From: Alex Estrin Cc: linux-rdma@vger.kernel.org Date: Thu, 03 Jul 2014 10:29:44 -0400 Message-ID: <20140703142944.22920.12822.stgit@phlsvslse11.ph.intel.com> User-Agent: StGit/0.16 MIME-Version: 1.0 Sender: linux-rdma-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-rdma@vger.kernel.org X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Currently, the parent interface keeps sending broadcast group join requests even if p_key index 0 is invalid, which for itself is possible/common in virtualized environment where a VF has been probed to VM but the actual p_key configuration has not yet been assigned by the management software. This creates unnecessary noise on the fabric and in the kernel logs: ib0: multicast join failed for ff12:401b:8000:0000:0000:0000:ffff:ffff, status -22 The original code run the multicast task regardless of the actual p_key value, which can be avoided. The fix is to re-init resources and bring interface up only if p_key index 0 is valid either when starting up or on PKEY_CHANGE event. Fixes: c290414169 ('IPoIB: Fix pkey change flow for virtualization environments') Reviewed-by: Ira Weiny Signed-off-by: Alex Estrin --- Changes from v5: -reinit child interface pkey_index before pkey lookup & compare. Changes from v4: - streamline child interface pkey event handling, - shutdown of pkey polling thread depends on PKEY_STOP flag state only. Original ipoib_ib_dev_down() could leave polling thread active if PKEY_ASSIGNED flag was set. That could create a racing condition on followed re-initialization of QP resources. Changes from v3: Updated chagelog and fixed alignemnt as suggested by Or Gerlitz Changes from v2: Handle pkey change event for a case when interface is down (pointed out by Erez Shitrit ). Changes from v1: p_key check for 'Invalid' value was moved to ipoib_pkey_dev_check_presence() that is used now in ipoib_ib_dev_open() for p_key validation. --- drivers/infiniband/ulp/ipoib/ipoib_ib.c | 60 ++++++++++++++++--------------- 1 files changed, 30 insertions(+), 30 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/infiniband/ulp/ipoib/ipoib_ib.c b/drivers/infiniband/ulp/ipoib/ipoib_ib.c index 6a7003d..b297352 100644 --- a/drivers/infiniband/ulp/ipoib/ipoib_ib.c +++ b/drivers/infiniband/ulp/ipoib/ipoib_ib.c @@ -52,6 +52,7 @@ MODULE_PARM_DESC(data_debug_level, #endif static DEFINE_MUTEX(pkey_mutex); +static void ipoib_pkey_dev_check_presence(struct net_device *dev); struct ipoib_ah *ipoib_create_ah(struct net_device *dev, struct ib_pd *pd, struct ib_ah_attr *attr) @@ -669,12 +670,13 @@ int ipoib_ib_dev_open(struct net_device *dev) struct ipoib_dev_priv *priv = netdev_priv(dev); int ret; - if (ib_find_pkey(priv->ca, priv->port, priv->pkey, &priv->pkey_index)) { - ipoib_warn(priv, "P_Key 0x%04x not found\n", priv->pkey); - clear_bit(IPOIB_PKEY_ASSIGNED, &priv->flags); + ipoib_pkey_dev_check_presence(dev); + + if (!test_bit(IPOIB_PKEY_ASSIGNED, &priv->flags)) { + ipoib_warn(priv, "P_Key 0x%04x is %s\n", priv->pkey, + !(priv->pkey & 0x7fff) ? "Invalid" : "not found"); return -1; } - set_bit(IPOIB_PKEY_ASSIGNED, &priv->flags); ret = ipoib_init_qp(dev); if (ret) { @@ -712,9 +714,10 @@ dev_stop: static void ipoib_pkey_dev_check_presence(struct net_device *dev) { struct ipoib_dev_priv *priv = netdev_priv(dev); - u16 pkey_index = 0; - if (ib_find_pkey(priv->ca, priv->port, priv->pkey, &pkey_index)) + if (!(priv->pkey & 0x7fff) || + ib_find_pkey(priv->ca, priv->port, priv->pkey, + &priv->pkey_index)) clear_bit(IPOIB_PKEY_ASSIGNED, &priv->flags); else set_bit(IPOIB_PKEY_ASSIGNED, &priv->flags); @@ -746,12 +749,10 @@ int ipoib_ib_dev_down(struct net_device *dev, int flush) netif_carrier_off(dev); /* Shutdown the P_Key thread if still active */ - if (!test_bit(IPOIB_PKEY_ASSIGNED, &priv->flags)) { - mutex_lock(&pkey_mutex); - set_bit(IPOIB_PKEY_STOP, &priv->flags); + mutex_lock(&pkey_mutex); + if (!test_and_set_bit(IPOIB_PKEY_STOP, &priv->flags)) cancel_delayed_work_sync(&priv->pkey_poll_task); - mutex_unlock(&pkey_mutex); - } + mutex_unlock(&pkey_mutex); ipoib_mcast_stop_thread(dev, flush); ipoib_mcast_dev_flush(dev); @@ -972,7 +973,7 @@ static void __ipoib_ib_dev_flush(struct ipoib_dev_priv *priv, { struct ipoib_dev_priv *cpriv; struct net_device *dev = priv->dev; - u16 new_index; + u16 old_index; int result; down_read(&priv->vlan_rwsem); @@ -986,16 +987,17 @@ static void __ipoib_ib_dev_flush(struct ipoib_dev_priv *priv, up_read(&priv->vlan_rwsem); - if (!test_bit(IPOIB_FLAG_INITIALIZED, &priv->flags)) { - /* for non-child devices must check/update the pkey value here */ - if (level == IPOIB_FLUSH_HEAVY && - !test_bit(IPOIB_FLAG_SUBINTERFACE, &priv->flags)) - update_parent_pkey(priv); + if (!test_bit(IPOIB_FLAG_INITIALIZED, &priv->flags) && + level != IPOIB_FLUSH_HEAVY) { ipoib_dbg(priv, "Not flushing - IPOIB_FLAG_INITIALIZED not set.\n"); return; } if (!test_bit(IPOIB_FLAG_ADMIN_UP, &priv->flags)) { + /* interface is down. update pkey and leave. */ + if (level == IPOIB_FLUSH_HEAVY && + !test_bit(IPOIB_FLAG_SUBINTERFACE, &priv->flags)) + update_parent_pkey(priv); ipoib_dbg(priv, "Not flushing - IPOIB_FLAG_ADMIN_UP not set.\n"); return; } @@ -1005,20 +1007,16 @@ static void __ipoib_ib_dev_flush(struct ipoib_dev_priv *priv, * (parent) devices should always takes what present in pkey index 0 */ if (test_bit(IPOIB_FLAG_SUBINTERFACE, &priv->flags)) { - if (ib_find_pkey(priv->ca, priv->port, priv->pkey, &new_index)) { - clear_bit(IPOIB_PKEY_ASSIGNED, &priv->flags); - ipoib_ib_dev_down(dev, 0); - ipoib_ib_dev_stop(dev, 0); - if (ipoib_pkey_dev_delay_open(dev)) - return; - } - /* restart QP only if P_Key index is changed */ - if (test_and_set_bit(IPOIB_PKEY_ASSIGNED, &priv->flags) && - new_index == priv->pkey_index) { + old_index = priv->pkey_index; + priv->pkey_index = 0; + ipoib_pkey_dev_check_presence(dev); + + if (test_bit(IPOIB_PKEY_ASSIGNED, &priv->flags) && + old_index == priv->pkey_index) { + /* restart QP only if P_Key index is changed */ ipoib_dbg(priv, "Not flushing - P_Key index not changed.\n"); return; } - priv->pkey_index = new_index; } else { result = update_parent_pkey(priv); /* restart QP only if P_Key value changed */ @@ -1038,8 +1036,10 @@ static void __ipoib_ib_dev_flush(struct ipoib_dev_priv *priv, ipoib_ib_dev_down(dev, 0); if (level == IPOIB_FLUSH_HEAVY) { - ipoib_ib_dev_stop(dev, 0); - ipoib_ib_dev_open(dev); + if (test_bit(IPOIB_FLAG_INITIALIZED, &priv->flags)) + ipoib_ib_dev_stop(dev, 0); + if (ipoib_ib_dev_open(dev) != 0) + return; } /*