All of lore.kernel.org
 help / color / mirror / Atom feed
From: Leon Romanovsky <leon@kernel.org>
To: Doug Ledford <dledford@redhat.com>, Jason Gunthorpe <jgg@mellanox.com>
Cc: Leon Romanovsky <leonro@mellanox.com>,
	RDMA mailing list <linux-rdma@vger.kernel.org>,
	Denis Drozdov <denisd@mellanox.com>,
	Erez Shitrit <erezsh@mellanox.com>,
	Saeed Mahameed <saeedm@mellanox.com>,
	linux-netdev <netdev@vger.kernel.org>
Subject: [PATCH rdma-next 05/10] IB/ipoib: Move init code to ndo_init
Date: Sun, 29 Jul 2018 11:34:55 +0300	[thread overview]
Message-ID: <20180729083500.5352-6-leon@kernel.org> (raw)
In-Reply-To: <20180729083500.5352-1-leon@kernel.org>

From: Jason Gunthorpe <jgg@mellanox.com>

Now that we have a proper ndo_uninit, move code that naturally pairs
with the ndo_uninit into ndo_init. This allows the netdev core to natually
handle ordering.

This fixes the situation where register_netdev can fail before calling
ndo_init, in which case it wouldn't call ndo_uninit either.

Also move a bunch of duplicated init code that is shared between child
and parent for clarity. Now the child and parent register functions look
very similar.

Signed-off-by: Jason Gunthorpe <jgg@mellanox.com>
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
 drivers/infiniband/ulp/ipoib/ipoib.h         |   3 -
 drivers/infiniband/ulp/ipoib/ipoib_main.c    | 193 +++++++++++++++------------
 drivers/infiniband/ulp/ipoib/ipoib_netlink.c |   6 -
 drivers/infiniband/ulp/ipoib/ipoib_vlan.c    |  31 +----
 4 files changed, 114 insertions(+), 119 deletions(-)

diff --git a/drivers/infiniband/ulp/ipoib/ipoib.h b/drivers/infiniband/ulp/ipoib/ipoib.h
index 04fc5ad1b69f..02ad1a60dc80 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib.h
+++ b/drivers/infiniband/ulp/ipoib/ipoib.h
@@ -508,8 +508,6 @@ void ipoib_ib_dev_down(struct net_device *dev);
 int ipoib_ib_dev_stop_default(struct net_device *dev);
 void ipoib_pkey_dev_check_presence(struct net_device *dev);
 
-int ipoib_dev_init(struct net_device *dev, struct ib_device *ca, int port);
-
 void ipoib_mcast_join_task(struct work_struct *work);
 void ipoib_mcast_carrier_on_task(struct work_struct *work);
 void ipoib_mcast_send(struct net_device *dev, u8 *daddr, struct sk_buff *skb);
@@ -597,7 +595,6 @@ void ipoib_pkey_open(struct ipoib_dev_priv *priv);
 void ipoib_drain_cq(struct net_device *dev);
 
 void ipoib_set_ethtool_ops(struct net_device *dev);
-void ipoib_set_dev_features(struct ipoib_dev_priv *priv, struct ib_device *hca);
 
 #define IPOIB_FLAGS_RC		0x80
 #define IPOIB_FLAGS_UC		0x40
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c b/drivers/infiniband/ulp/ipoib/ipoib_main.c
index d4e9951dc539..67ab52eec3e9 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_main.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c
@@ -1741,13 +1741,11 @@ static int ipoib_ioctl(struct net_device *dev, struct ifreq *ifr,
 	return priv->rn_ops->ndo_do_ioctl(dev, ifr, cmd);
 }
 
-int ipoib_dev_init(struct net_device *dev, struct ib_device *ca, int port)
+static int ipoib_dev_init(struct net_device *dev)
 {
 	struct ipoib_dev_priv *priv = ipoib_priv(dev);
 	int ret = -ENOMEM;
 
-	priv->ca = ca;
-	priv->port = port;
 	priv->qp = NULL;
 
 	/*
@@ -1763,7 +1761,7 @@ int ipoib_dev_init(struct net_device *dev, struct ib_device *ca, int port)
 	/* create pd, which used both for control and datapath*/
 	priv->pd = ib_alloc_pd(priv->ca, 0);
 	if (IS_ERR(priv->pd)) {
-		pr_warn("%s: failed to allocate PD\n", ca->name);
+		pr_warn("%s: failed to allocate PD\n", priv->ca->name);
 		goto clean_wq;
 	}
 
@@ -1837,6 +1835,108 @@ static void ipoib_parent_unregister_pre(struct net_device *ndev)
 	flush_workqueue(ipoib_workqueue);
 }
 
+static void ipoib_set_dev_features(struct ipoib_dev_priv *priv)
+{
+	priv->hca_caps = priv->ca->attrs.device_cap_flags;
+
+	if (priv->hca_caps & IB_DEVICE_UD_IP_CSUM) {
+		priv->dev->hw_features |= NETIF_F_IP_CSUM | NETIF_F_RXCSUM;
+
+		if (priv->hca_caps & IB_DEVICE_UD_TSO)
+			priv->dev->hw_features |= NETIF_F_TSO;
+
+		priv->dev->features |= priv->dev->hw_features;
+	}
+}
+
+static int ipoib_parent_init(struct net_device *ndev)
+{
+	struct ipoib_dev_priv *priv = ipoib_priv(ndev);
+	struct ib_port_attr attr;
+	int result;
+
+	result = ib_query_port(priv->ca, priv->port, &attr);
+	if (result) {
+		pr_warn("%s: ib_query_port %d failed\n", priv->ca->name,
+			priv->port);
+		return result;
+	}
+	priv->max_ib_mtu = ib_mtu_enum_to_int(attr.max_mtu);
+
+	result = ib_query_pkey(priv->ca, priv->port, 0, &priv->pkey);
+	if (result) {
+		pr_warn("%s: ib_query_pkey port %d failed (ret = %d)\n",
+			priv->ca->name, priv->port, result);
+		return result;
+	}
+
+	result = rdma_query_gid(priv->ca, priv->port, 0, &priv->local_gid);
+	if (result) {
+		pr_warn("%s: rdma_query_gid port %d failed (ret = %d)\n",
+			priv->ca->name, priv->port, result);
+		return result;
+	}
+	memcpy(priv->dev->dev_addr + 4, priv->local_gid.raw,
+	       sizeof(union ib_gid));
+
+	SET_NETDEV_DEV(priv->dev, priv->ca->dev.parent);
+	priv->dev->dev_id = priv->port - 1;
+
+	return 0;
+}
+
+static void ipoib_child_init(struct net_device *ndev)
+{
+	struct ipoib_dev_priv *priv = ipoib_priv(ndev);
+	struct ipoib_dev_priv *ppriv = ipoib_priv(priv->parent);
+
+	priv->max_ib_mtu = ppriv->max_ib_mtu;
+	set_bit(IPOIB_FLAG_SUBINTERFACE, &priv->flags);
+	memcpy(priv->dev->dev_addr, ppriv->dev->dev_addr, INFINIBAND_ALEN);
+	memcpy(&priv->local_gid, &ppriv->local_gid, sizeof(priv->local_gid));
+}
+
+static int ipoib_ndo_init(struct net_device *ndev)
+{
+	struct ipoib_dev_priv *priv = ipoib_priv(ndev);
+	int rc;
+
+	if (priv->parent) {
+		ipoib_child_init(ndev);
+	} else {
+		rc = ipoib_parent_init(ndev);
+		if (rc)
+			return rc;
+	}
+
+	/* MTU will be reset when mcast join happens */
+	ndev->mtu = IPOIB_UD_MTU(priv->max_ib_mtu);
+	priv->mcast_mtu = priv->admin_mtu = ndev->mtu;
+	ndev->max_mtu = IPOIB_CM_MTU;
+
+	ndev->neigh_priv_len = sizeof(struct ipoib_neigh);
+
+	/*
+	 * Set the full membership bit, so that we join the right
+	 * broadcast group, etc.
+	 */
+	priv->pkey |= 0x8000;
+
+	ndev->broadcast[8] = priv->pkey >> 8;
+	ndev->broadcast[9] = priv->pkey & 0xff;
+	set_bit(IPOIB_FLAG_DEV_ADDR_SET, &priv->flags);
+
+	ipoib_set_dev_features(priv);
+
+	rc = ipoib_dev_init(ndev);
+	if (rc) {
+		pr_warn("%s: failed to initialize device: %s port %d (ret = %d)\n",
+			priv->ca->name, priv->dev->name, priv->port, rc);
+	}
+
+	return 0;
+}
+
 static void ipoib_ndo_uninit(struct net_device *dev)
 {
 	struct ipoib_dev_priv *priv = ipoib_priv(dev), *cpriv, *tcpriv;
@@ -1909,6 +2009,7 @@ static const struct header_ops ipoib_header_ops = {
 };
 
 static const struct net_device_ops ipoib_netdev_ops_pf = {
+	.ndo_init		 = ipoib_ndo_init,
 	.ndo_uninit		 = ipoib_ndo_uninit,
 	.ndo_open		 = ipoib_open,
 	.ndo_stop		 = ipoib_stop,
@@ -1928,6 +2029,7 @@ static const struct net_device_ops ipoib_netdev_ops_pf = {
 };
 
 static const struct net_device_ops ipoib_netdev_ops_vf = {
+	.ndo_init		 = ipoib_ndo_init,
 	.ndo_uninit		 = ipoib_ndo_uninit,
 	.ndo_open		 = ipoib_open,
 	.ndo_stop		 = ipoib_stop,
@@ -2054,6 +2156,9 @@ struct ipoib_dev_priv *ipoib_intf_alloc(struct ib_device *hca, u8 port,
 	if (!priv)
 		return NULL;
 
+	priv->ca = hca;
+	priv->port = port;
+
 	dev = ipoib_get_netdev(hca, port, name);
 	if (!dev)
 		goto free_priv;
@@ -2201,12 +2306,6 @@ static ssize_t create_child(struct device *dev,
 	if (pkey <= 0 || pkey > 0xffff || pkey == 0x8000)
 		return -EINVAL;
 
-	/*
-	 * Set the full membership bit, so that we join the right
-	 * broadcast group, etc.
-	 */
-	pkey |= 0x8000;
-
 	ret = ipoib_vlan_add(to_net_dev(dev), pkey);
 
 	return ret ? ret : count;
@@ -2238,86 +2337,17 @@ int ipoib_add_pkey_attr(struct net_device *dev)
 	return device_create_file(&dev->dev, &dev_attr_pkey);
 }
 
-void ipoib_set_dev_features(struct ipoib_dev_priv *priv, struct ib_device *hca)
-{
-	priv->hca_caps = hca->attrs.device_cap_flags;
-
-	if (priv->hca_caps & IB_DEVICE_UD_IP_CSUM) {
-		priv->dev->hw_features |= NETIF_F_IP_CSUM | NETIF_F_RXCSUM;
-
-		if (priv->hca_caps & IB_DEVICE_UD_TSO)
-			priv->dev->hw_features |= NETIF_F_TSO;
-
-		priv->dev->features |= priv->dev->hw_features;
-	}
-}
-
 static struct net_device *ipoib_add_port(const char *format,
 					 struct ib_device *hca, u8 port)
 {
 	struct ipoib_dev_priv *priv;
-	struct ib_port_attr attr;
 	struct rdma_netdev *rn;
-	int result = -ENOMEM;
+	int result;
 
 	priv = ipoib_intf_alloc(hca, port, format);
 	if (!priv) {
 		pr_warn("%s, %d: ipoib_intf_alloc failed\n", hca->name, port);
-		goto alloc_mem_failed;
-	}
-
-	SET_NETDEV_DEV(priv->dev, hca->dev.parent);
-	priv->dev->dev_id = port - 1;
-
-	result = ib_query_port(hca, port, &attr);
-	if (result) {
-		pr_warn("%s: ib_query_port %d failed\n", hca->name, port);
-		goto device_init_failed;
-	}
-
-	priv->max_ib_mtu = ib_mtu_enum_to_int(attr.max_mtu);
-
-	/* MTU will be reset when mcast join happens */
-	priv->dev->mtu  = IPOIB_UD_MTU(priv->max_ib_mtu);
-	priv->mcast_mtu  = priv->admin_mtu = priv->dev->mtu;
-	priv->dev->max_mtu = IPOIB_CM_MTU;
-
-	priv->dev->neigh_priv_len = sizeof(struct ipoib_neigh);
-
-	result = ib_query_pkey(hca, port, 0, &priv->pkey);
-	if (result) {
-		pr_warn("%s: ib_query_pkey port %d failed (ret = %d)\n",
-			hca->name, port, result);
-		goto device_init_failed;
-	}
-
-	ipoib_set_dev_features(priv, hca);
-
-	/*
-	 * Set the full membership bit, so that we join the right
-	 * broadcast group, etc.
-	 */
-	priv->pkey |= 0x8000;
-
-	priv->dev->broadcast[8] = priv->pkey >> 8;
-	priv->dev->broadcast[9] = priv->pkey & 0xff;
-
-	result = rdma_query_gid(hca, port, 0, &priv->local_gid);
-	if (result) {
-		pr_warn("%s: rdma_query_gid port %d failed (ret = %d)\n",
-			hca->name, port, result);
-		goto device_init_failed;
-	}
-
-	memcpy(priv->dev->dev_addr + 4, priv->local_gid.raw,
-	       sizeof(union ib_gid));
-	set_bit(IPOIB_FLAG_DEV_ADDR_SET, &priv->flags);
-
-	result = ipoib_dev_init(priv->dev, hca, port);
-	if (result) {
-		pr_warn("%s: failed to initialize port %d (ret = %d)\n",
-			hca->name, port, result);
-		goto device_init_failed;
+		return ERR_PTR(-ENOMEM);
 	}
 
 	INIT_IB_EVENT_HANDLER(&priv->event_handler,
@@ -2358,7 +2388,6 @@ static struct net_device *ipoib_add_port(const char *format,
 	rn->free_rdma_netdev(priv->dev);
 	kfree(priv);
 
-alloc_mem_failed:
 	return ERR_PTR(result);
 }
 
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_netlink.c b/drivers/infiniband/ulp/ipoib/ipoib_netlink.c
index 3e44087935ae..a86928a80c08 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_netlink.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_netlink.c
@@ -125,12 +125,6 @@ static int ipoib_new_child_link(struct net *src_net, struct net_device *dev,
 	if (child_pkey == 0 || child_pkey == 0x8000)
 		return -EINVAL;
 
-	/*
-	 * Set the full membership bit, so that we join the right
-	 * broadcast group, etc.
-	 */
-	child_pkey |= 0x8000;
-
 	err = __ipoib_vlan_add(ppriv, ipoib_priv(dev),
 			       child_pkey, IPOIB_RTNL_CHILD);
 
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_vlan.c b/drivers/infiniband/ulp/ipoib/ipoib_vlan.c
index b62ab85c8ead..3103729a73fd 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_vlan.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_vlan.c
@@ -55,35 +55,14 @@ int __ipoib_vlan_add(struct ipoib_dev_priv *ppriv, struct ipoib_dev_priv *priv,
 {
 	int result;
 
-	priv->max_ib_mtu = ppriv->max_ib_mtu;
-	/* MTU will be reset when mcast join happens */
-	priv->dev->mtu   = IPOIB_UD_MTU(priv->max_ib_mtu);
-	priv->mcast_mtu  = priv->admin_mtu = priv->dev->mtu;
 	priv->parent = ppriv->dev;
-	set_bit(IPOIB_FLAG_SUBINTERFACE, &priv->flags);
-
-	ipoib_set_dev_features(priv, ppriv->ca);
-
 	priv->pkey = pkey;
-
-	memcpy(priv->dev->dev_addr, ppriv->dev->dev_addr, INFINIBAND_ALEN);
-	memcpy(&priv->local_gid, &ppriv->local_gid, sizeof(priv->local_gid));
-	set_bit(IPOIB_FLAG_DEV_ADDR_SET, &priv->flags);
-	priv->dev->broadcast[8] = pkey >> 8;
-	priv->dev->broadcast[9] = pkey & 0xff;
-
-	result = ipoib_dev_init(priv->dev, ppriv->ca, ppriv->port);
-	if (result < 0) {
-		ipoib_warn(ppriv, "failed to initialize subinterface: "
-			   "device %s, port %d",
-			   ppriv->ca->name, ppriv->port);
-		goto err;
-	}
+	priv->child_type = type;
 
 	result = register_netdevice(priv->dev);
 	if (result) {
 		ipoib_warn(priv, "failed to initialize; error %i", result);
-		goto err;
+		return result;
 	}
 
 	/* RTNL childs don't need proprietary sysfs entries */
@@ -99,17 +78,13 @@ int __ipoib_vlan_add(struct ipoib_dev_priv *ppriv, struct ipoib_dev_priv *priv,
 			goto sysfs_failed;
 	}
 
-	priv->child_type  = type;
 	list_add_tail(&priv->list, &ppriv->child_intfs);
 
 	return 0;
 
 sysfs_failed:
-	result = -ENOMEM;
 	unregister_netdevice(priv->dev);
-
-err:
-	return result;
+	return -ENOMEM;
 }
 
 int ipoib_vlan_add(struct net_device *pdev, unsigned short pkey)
-- 
2.14.4

  parent reply	other threads:[~2018-07-29  8:34 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-29  8:34 [PATCH rdma-next 00/10] IPoIB uninit Leon Romanovsky
2018-07-29  8:34 ` [PATCH rdma-next 01/10] IB/ipoib: Get rid of IPOIB_FLAG_GOING_DOWN Leon Romanovsky
2018-07-29  8:34 ` [PATCH rdma-next 02/10] IB/ipoib: Use cancel_delayed_work_sync for neigh-clean task Leon Romanovsky
2018-07-29  8:34 ` [PATCH rdma-next 03/10] IB/ipoib: Move all uninit code into ndo_uninit Leon Romanovsky
2018-07-29  8:34 ` [PATCH rdma-next 04/10] IB/ipoib: Make ipoib_neigh_hash_uninit fully fence its work Leon Romanovsky
2018-07-29  8:34 ` Leon Romanovsky [this message]
2018-07-29  8:34 ` [PATCH rdma-next 06/10] RDMA/netdev: Use priv_destructor for netdev cleanup Leon Romanovsky
2018-07-29  8:34 ` [PATCH rdma-next 07/10] IB/ipoib: Get rid of the sysfs_mutex Leon Romanovsky
2018-07-29  8:34 ` [PATCH rdma-next 08/10] IB/ipoib: Do not remove child devices from within the ndo_uninit Leon Romanovsky
2018-07-29  8:34 ` [PATCH rdma-next 09/10] IB/ipoib: Maintain the child_intfs list from ndo_init/uninit Leon Romanovsky
2018-07-29  8:35 ` [PATCH rdma-next 10/10] IB/ipoib: Consolidate checking of the proposed child interface Leon Romanovsky
2018-08-03  2:31 ` [PATCH rdma-next 00/10] IPoIB uninit Jason Gunthorpe

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180729083500.5352-6-leon@kernel.org \
    --to=leon@kernel.org \
    --cc=denisd@mellanox.com \
    --cc=dledford@redhat.com \
    --cc=erezsh@mellanox.com \
    --cc=jgg@mellanox.com \
    --cc=leonro@mellanox.com \
    --cc=linux-rdma@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=saeedm@mellanox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.