From patchwork Fri Aug 10 23:58:44 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Roland Dreier X-Patchwork-Id: 1307081 Return-Path: X-Original-To: patchwork-linux-rdma@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork1.kernel.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by patchwork1.kernel.org (Postfix) with ESMTP id 3CB423FC66 for ; Fri, 10 Aug 2012 23:59:10 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1760402Ab2HJX7I (ORCPT ); Fri, 10 Aug 2012 19:59:08 -0400 Received: from na3sys010aog101.obsmtp.com ([74.125.245.70]:59412 "HELO na3sys010aog101.obsmtp.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1760378Ab2HJX7G (ORCPT ); Fri, 10 Aug 2012 19:59:06 -0400 Received: from mail-qa0-f47.google.com ([209.85.216.47]) (using TLSv1) by na3sys010aob101.postini.com ([74.125.244.12]) with SMTP ID DSNKUCWgSWibnvSkNGzqM4KcS5TMdr2L6sbh@postini.com; Fri, 10 Aug 2012 16:59:06 PDT Received: by qaat11 with SMTP id t11so1548369qaa.6 for ; Fri, 10 Aug 2012 16:59:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=purestorage.com; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc:content-type; bh=BhvjTZGG4/SJIHgcf3ck3FcqqbQZRC9J/IrJHmqRVkU=; b=juzTB+8gUdcsKZLz6kXSq/RAnel0GX/7uaDFJ9udg+xWk3zMudOcOb009yiuuhupbg 4FEVhiaZ8cjJNa6i3CTVSA+t52WyepjM9IL5mcSvrNuJTMmMD1NKYRi7xBO2dqG3z6Y7 /maMRx1xmjKjrDxvIGokWw//4OcWSlqrWj0zw= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc:content-type:x-gm-message-state; bh=BhvjTZGG4/SJIHgcf3ck3FcqqbQZRC9J/IrJHmqRVkU=; b=XYxkBK2EmArCt5ac0NGqdz4Z4Bll0FrDr7xfnhYwryk/nXG3l8dAzxqG1RoLNQQ9oN yIxVj/UpqSJY+4PKMEyu2IjWZs3tQO5KF+rSjWnCtKGSFrPmpcFb0l0qJf8ixNdabIzY 5DHB1ZmbI+olEG9i2nqjFO9ddAtfx68/hwAbo0d2zTMc/9416NPdQFq0CSlbpk+TsxzX na2XQOsGvTtV7FFVKxax+4IN3rkuduEHnqLNU5YoCG5Bh+F2P2Mmk4fWv3A/aykOKXrA qrNXhFFqorDHcyvJk7yXFHa0RYcOvTuwJ7vN9sNunuTE01yYEJu4Xun9s/GSXc/9/swo adYw== Received: by 10.229.135.193 with SMTP id o1mr2269554qct.100.1344643145121; Fri, 10 Aug 2012 16:59:05 -0700 (PDT) MIME-Version: 1.0 Received: by 10.229.31.201 with HTTP; Fri, 10 Aug 2012 16:58:44 -0700 (PDT) In-Reply-To: <9ac93bde-c809-45c2-86e2-a50ea6a1020f@exht1.ad.emulex.com> References: <9ac93bde-c809-45c2-86e2-a50ea6a1020f@exht1.ad.emulex.com> From: Roland Dreier Date: Fri, 10 Aug 2012 16:58:44 -0700 Message-ID: Subject: Re: [PATCH] RDMA/ocrdma: Fixed CONFIG_VLAN_8021Q. To: Parav Pandit Cc: linux-rdma@vger.kernel.org X-Gm-Message-State: ALoCoQkSIl2BI7fhplV3lsAtTf4RhdK3Ig3z7BCqwmklw4llqHwMETmeDI7gl2Hrx7fYcyBtU35I Sender: linux-rdma-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-rdma@vger.kernel.org On Fri, Aug 10, 2012 at 9:28 AM, Parav Pandit wrote: > Fixed avoiding checking real vlan dev in scenario > when VLAN is disabled and ipv6 is enabled. It would be a nice touch to acknowledge Fengguang with a Reported-by: > Signed-off-by: Parav Pandit > --- > drivers/infiniband/hw/ocrdma/ocrdma_main.c | 16 ++++++++++++---- > 1 files changed, 12 insertions(+), 4 deletions(-) > > diff --git a/drivers/infiniband/hw/ocrdma/ocrdma_main.c b/drivers/infiniband/hw/ocrdma/ocrdma_main.c > index 5a04452..7146ffd 100644 > --- a/drivers/infiniband/hw/ocrdma/ocrdma_main.c > +++ b/drivers/infiniband/hw/ocrdma/ocrdma_main.c > @@ -161,7 +161,7 @@ static void ocrdma_add_default_sgid(struct ocrdma_dev *dev) > ocrdma_get_guid(dev, &sgid->raw[8]); > } > > -#if defined(CONFIG_VLAN_8021Q) || defined(CONFIG_VLAN_8021Q_MODULE) > +#if IS_ENABLED(CONFIG_VLAN_8021Q) || IS_ENABLED(CONFIG_VLAN_8021Q_MODULE) a single IS_ENABLED() tests both CONFIG_xxx and CONFIG_xxx_MODULE > static void ocrdma_add_vlan_sgids(struct ocrdma_dev *dev) > { > struct net_device *netdev, *tmp; > @@ -202,8 +202,16 @@ static int ocrdma_build_sgid_tbl(struct ocrdma_dev *dev) > return 0; > } > > -#if IS_ENABLED(CONFIG_IPV6) || IS_ENABLED(CONFIG_VLAN_8021Q) > +static struct net_device *ocrdma_get_real_netdev(struct net_device *netdev) > +{ > +#if IS_ENABLED(CONFIG_VLAN_8021Q) || IS_ENABLED(CONFIG_VLAN_8021Q_MODULE) > + return vlan_dev_real_dev(netdev); > +#else > + return netdev; > +#endif > +} This is kind of crazy: it's a big helper that you only use in one spot, and the whole point of IS_ENABLED() is that it is usable in C code too -- so you could at least write this without using the preprocessor. But I don't think it really helps to split out this wrapper. > +#if IS_ENABLED(CONFIG_IPV6) > static int ocrdma_inet6addr_event(struct notifier_block *notifier, > unsigned long event, void *ptr) > { > @@ -217,7 +225,7 @@ static int ocrdma_inet6addr_event(struct notifier_block *notifier, > bool is_vlan = false; > u16 vid = 0; > > - netdev = vlan_dev_real_dev(event_netdev); > + netdev = ocrdma_get_real_netdev(event_netdev); > if (netdev != event_netdev) { > is_vlan = true; > vid = vlan_dev_vlan_id(event_netdev); > @@ -262,7 +270,7 @@ static struct notifier_block ocrdma_inet6addr_notifier = { > .notifier_call = ocrdma_inet6addr_event > }; > > -#endif /* IPV6 and VLAN */ > +#endif /* IPV6 */ > > static enum rdma_link_layer ocrdma_link_layer(struct ib_device *device, > u8 port_num) How about this simpler version: From d549f55f2e132e3d1f1288ce4231f45f12988bbf Mon Sep 17 00:00:00 2001 From: Roland Dreier Date: Fri, 10 Aug 2012 16:52:13 -0700 Subject: [PATCH] RDMA/ocrdma: Don't call vlan_dev_real_dev() for non-VLAN netdevs If CONFIG_VLAN_8021Q is not set, then vlan_dev_real_dev() just goes BUG(), so we shouldn't call it unless we're actually dealing with a VLAN netdev. Reported-by: Fengguang Wu Signed-off-by: Roland Dreier --- drivers/infiniband/hw/ocrdma/ocrdma_main.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) if (dev->nic_info.netdev == netdev) { diff --git a/drivers/infiniband/hw/ocrdma/ocrdma_main.c b/drivers/infiniband/hw/ocrdma/ocrdma_main.c index 5a04452..c4e0131 100644 --- a/drivers/infiniband/hw/ocrdma/ocrdma_main.c +++ b/drivers/infiniband/hw/ocrdma/ocrdma_main.c @@ -161,7 +161,7 @@ static void ocrdma_add_default_sgid(struct ocrdma_dev *dev) ocrdma_get_guid(dev, &sgid->raw[8]); } -#if defined(CONFIG_VLAN_8021Q) || defined(CONFIG_VLAN_8021Q_MODULE) +#if IS_ENABLED(CONFIG_VLAN_8021Q) static void ocrdma_add_vlan_sgids(struct ocrdma_dev *dev) { struct net_device *netdev, *tmp; @@ -202,14 +202,13 @@ static int ocrdma_build_sgid_tbl(struct ocrdma_dev *dev) return 0; } -#if IS_ENABLED(CONFIG_IPV6) || IS_ENABLED(CONFIG_VLAN_8021Q) +#if IS_ENABLED(CONFIG_IPV6) static int ocrdma_inet6addr_event(struct notifier_block *notifier, unsigned long event, void *ptr) { struct inet6_ifaddr *ifa = (struct inet6_ifaddr *)ptr; - struct net_device *event_netdev = ifa->idev->dev; - struct net_device *netdev = NULL; + struct net_device *netdev = ifa->idev->dev; struct ib_event gid_event; struct ocrdma_dev *dev; bool found = false; @@ -217,11 +216,12 @@ static int ocrdma_inet6addr_event(struct notifier_block *notifier, bool is_vlan = false; u16 vid = 0; - netdev = vlan_dev_real_dev(event_netdev); - if (netdev != event_netdev) { - is_vlan = true; - vid = vlan_dev_vlan_id(event_netdev); + is_vlan = netdev->priv_flags & IFF_802_1Q_VLAN; + if (is_vlan) { + vid = vlan_dev_vlan_id(netdev); + netdev = vlan_dev_real_dev(netdev); } + rcu_read_lock(); list_for_each_entry_rcu(dev, &ocrdma_dev_list, entry) {