[PATCH rdma-next 0/5] IB device rename support

[PATCH rdma-next 0/5] IB device rename support

[Leon Romanovsky]

From: Leon Romanovsky <leonro@mellanox.com>

Hi,

This series introduce long-waiting feature - "IB device rename".
Such feature gives and option to rename user visible IB device name from
vendor specific name (e.g. mlx5_0) to anything else.

The user space component through rdmatool will follow this series.

[leonro@server /]$ lspci |grep -i Ether
00:08.0 Ethernet controller: Red Hat, Inc. Virtio network device
00:09.0 Ethernet controller: Mellanox Technologies MT27700 Family [ConnectX-4]
[leonro@server /]$ sudo rdma dev
1: mlx5_0: node_type ca fw 3.8.9999 node_guid 5254:00c0:fe12:3455 sys_image_guid 5254:00c0:fe12:3455
[leonro@server /]$ sudo rdma dev set mlx5_0 name hfi1_0
[leonro@server /]$ sudo rdma dev
1: hfi1_0: node_type ca fw 3.8.9999 node_guid 5254:00c0:fe12:3455 sys_image_guid 5254:00c0:fe12:3455

First patch introduces getter/setter to access names, i didn't convert
all drivers to stop using name directly, because they don't base their
decision on "name", and use this print only and can print truncated name
if renaming is done at the same time as logging.

Second patch updates SMC to use IB device index instead of name.

Third patch globally converts all drivers to new allocation name
routine.

Forth and fifth patches are actually implement and exports through RDMA
netlink the rename routines.

It uses exported by device_rename() function, despite the comment from
2010, which warns about downsides of this function, the netdev is still
uses, so we will use too.

There is one patch/series which was dropped from this submission -
conversion of SElinux from being IB device name to be IB device index
based. It simply needs more special care and more testing.

This series was tested with mlx5 devices with/without traffic and with
non-modified rdma-core.

Dennis,
I didn't touch hfi1, but I'm not sure if it is needed.

Thanks

Leon Romanovsky (5):
  RDMA/core: Provide getter and setter to access IB device name
  net/smc: Use IB device index instead of name
  RDMA: Convert IB drivers to name allocation routine
  RDMA/core: Implement IB device rename function
  RDMA/nldev: Allow IB device rename through RDMA netlink

 drivers/infiniband/core/core_priv.h            |  1 +
 drivers/infiniband/core/device.c               | 52 +++++++++++++++++++++++---
 drivers/infiniband/core/nldev.c                | 33 ++++++++++++++++
 drivers/infiniband/hw/bnxt_re/main.c           |  6 ++-
 drivers/infiniband/hw/cxgb3/iwch_provider.c    |  5 ++-
 drivers/infiniband/hw/cxgb4/provider.c         |  5 ++-
 drivers/infiniband/hw/hns/hns_roce_main.c      |  4 +-
 drivers/infiniband/hw/i40iw/i40iw_verbs.c      |  7 +++-
 drivers/infiniband/hw/mlx4/main.c              |  7 +++-
 drivers/infiniband/hw/mlx5/main.c              |  4 +-
 drivers/infiniband/hw/mthca/mthca_provider.c   |  5 ++-
 drivers/infiniband/hw/nes/nes_verbs.c          |  6 ++-
 drivers/infiniband/hw/ocrdma/ocrdma_main.c     |  7 +++-
 drivers/infiniband/hw/qedr/main.c              |  4 +-
 drivers/infiniband/hw/usnic/usnic_ib_main.c    |  5 ++-
 drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c |  9 +++--
 drivers/infiniband/sw/rxe/rxe_verbs.c          |  5 ++-
 include/rdma/ib_verbs.h                        |  8 +++-
 include/uapi/rdma/rdma_netlink.h               |  3 +-
 net/smc/smc_diag.c                             |  6 +--
 net/smc/smc_pnet.c                             | 27 +++++++------
 21 files changed, 171 insertions(+), 38 deletions(-)

[PATCH rdma-next 1/5] RDMA/core: Provide getter and setter to access IB device name

[Leon Romanovsky]

From: Leon Romanovsky <leonro@mellanox.com>

Prepare IB device name field to rename operation by ensuring that all
accesses to it are protected with lock and users don't see part of name.

The protection is done with global device_lock because it is used in
allocation and deallocation phases. At this stage, this lock is not
busy and easily can be moved to be per-device, once it will be needed.

Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
 drivers/infiniband/core/device.c | 24 +++++++++++++++++++++++-
 include/rdma/ib_verbs.h          |  8 +++++++-
 2 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
index 5a680a88aa87..3270cde6d806 100644
--- a/drivers/infiniband/core/device.c
+++ b/drivers/infiniband/core/device.c
@@ -170,6 +170,14 @@ static struct ib_device *__ib_device_get_by_name(const char *name)
 	return NULL;
 }

+void ib_device_get_name(struct ib_device *ibdev, char *name)
+{
+	down_read(&lists_rwsem);
+	strlcpy(name, ibdev->name, IB_DEVICE_NAME_MAX);
+	up_read(&lists_rwsem);
+}
+EXPORT_SYMBOL(ib_device_get_name);
+
 static int alloc_name(char *name)
 {
 	unsigned long *inuse;
@@ -202,6 +210,21 @@ static int alloc_name(char *name)
 	return 0;
 }

+int ib_device_alloc_name(struct ib_device *ibdev, const char *pattern)
+{
+	int ret = 0;
+
+	mutex_lock(&device_mutex);
+	strlcpy(ibdev->name, pattern, IB_DEVICE_NAME_MAX);
+	if (strchr(ibdev->name, '%'))
+		ret = alloc_name(ibdev->name);
+
+	mutex_unlock(&device_mutex);
+
+	return ret;
+}
+EXPORT_SYMBOL(ib_device_alloc_name);
+
 static void ib_device_release(struct device *device)
 {
 	struct ib_device *dev = container_of(device, struct ib_device, dev);
@@ -499,7 +522,6 @@ int ib_register_device(struct ib_device *device,
 		ret = alloc_name(device->name);
 		if (ret)
 			goto out;
-	}

 	if (ib_device_check_mandatory(device)) {
 		ret = -EINVAL;
diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
index e764ed1f6025..66660e7b9854 100644
--- a/include/rdma/ib_verbs.h
+++ b/include/rdma/ib_verbs.h
@@ -2260,6 +2260,11 @@ struct ib_device {
 	/* Do not access @dma_device directly from ULP nor from HW drivers. */
 	struct device                *dma_device;

+	/*
+	 * Do not access @name directly,
+	 * use ib_device_get_name()/ib_device_alloc_name()
+	 * and don't assume that it can't change after access.
+	 */
 	char                          name[IB_DEVICE_NAME_MAX];

 	struct list_head              event_handler_list;
@@ -2638,7 +2643,8 @@ struct ib_device *ib_alloc_device(size_t size);
 void ib_dealloc_device(struct ib_device *device);

 void ib_get_device_fw_str(struct ib_device *device, char *str);
-
+int ib_device_alloc_name(struct ib_device *ibdev, const char *pattern);
+void ib_device_get_name(struct ib_device *ibdev, char *name);
 int ib_register_device(struct ib_device *device,
 		       int (*port_callback)(struct ib_device *,
 					    u8, struct kobject *));

[PATCH rdma-next 2/5] net/smc: Use IB device index instead of name

[Leon Romanovsky]

From: Leon Romanovsky <leonro@mellanox.com>

IB device name is not stable and will be possible to rename in the
following patches, update SMC code to use IB index as a stable
identification.

Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
 net/smc/smc_diag.c |  6 +++---
 net/smc/smc_pnet.c | 27 ++++++++++++++++-----------
 2 files changed, 19 insertions(+), 14 deletions(-)

diff --git a/net/smc/smc_diag.c b/net/smc/smc_diag.c
index dbf64a93d68a..ba8b5a5671ec 100644
--- a/net/smc/smc_diag.c
+++ b/net/smc/smc_diag.c
@@ -156,9 +156,9 @@ static int __smc_diag_dump(struct sock *sk, struct sk_buff *skb,
 			.lnk[0].link_id = smc->conn.lgr->lnk[0].link_id,
 		};

-		memcpy(linfo.lnk[0].ibname,
-		       smc->conn.lgr->lnk[0].smcibdev->ibdev->name,
-		       sizeof(smc->conn.lgr->lnk[0].smcibdev->ibdev->name));
+		ib_device_get_name(smc->conn.lgr->lnk[0].smcibdev->ibdev,
+				   linfo.lnk[0].ibname);
+
 		smc_gid_be16_convert(linfo.lnk[0].gid,
 				     smc->conn.lgr->lnk[0].gid);
 		smc_gid_be16_convert(linfo.lnk[0].peer_gid,
diff --git a/net/smc/smc_pnet.c b/net/smc/smc_pnet.c
index 01c6ce042a1c..67aec5ce6112 100644
--- a/net/smc/smc_pnet.c
+++ b/net/smc/smc_pnet.c
@@ -70,15 +70,14 @@ struct smc_pnetentry {
 	u8 ib_port;
 };

-/* Check if two RDMA device entries are identical. Use device name and port
+/* Check if two RDMA device entries are identical. Use device index and port
  * number for comparison.
  */
-static bool smc_pnet_same_ibname(struct smc_pnetentry *pnetelem, char *ibname,
-				 u8 ibport)
+static bool smc_pnet_same_ibindex(struct smc_pnetentry *pnetelem, u32 ibindex,
+				  u8 ibport)
 {
 	return pnetelem->ib_port == ibport &&
-	       !strncmp(pnetelem->smcibdev->ibdev->name, ibname,
-			sizeof(pnetelem->smcibdev->ibdev->name));
+	       pnetelem->smcibdev->ibdev->index == ibindex;
 }

 /* Find a pnetid in the pnet table.
@@ -179,9 +178,9 @@ static int smc_pnet_enter(struct smc_pnetentry *new_pnetelem)
 			     sizeof(new_pnetelem->pnet_name)) ||
 		    !strncmp(pnetelem->ndev->name, new_pnetelem->ndev->name,
 			     sizeof(new_pnetelem->ndev->name)) ||
-		    smc_pnet_same_ibname(pnetelem,
-					 new_pnetelem->smcibdev->ibdev->name,
-					 new_pnetelem->ib_port)) {
+		    smc_pnet_same_ibindex(pnetelem,
+					  new_pnetelem->smcibdev->ibdev->index,
+					  new_pnetelem->ib_port)) {
 			dev_put(pnetelem->ndev);
 			goto found;
 		}
@@ -227,10 +226,11 @@ static struct smc_ib_device *smc_pnet_find_ib(char *ib_name)

 	spin_lock(&smc_ib_devices.lock);
 	list_for_each_entry(ibdev, &smc_ib_devices.list, list) {
-		if (!strncmp(ibdev->ibdev->name, ib_name,
-			     sizeof(ibdev->ibdev->name))) {
+		char name[IB_DEVICE_NAME_MAX] = {};
+
+		ib_device_get_name(ibdev->ibdev, name);
+		if (!strncmp(name, ib_name, IB_DEVICE_NAME_MAX))
 			goto out;
-		}
 	}
 	ibdev = NULL;
 out:
@@ -267,6 +267,11 @@ static int smc_pnet_fill_entry(struct net *net, struct smc_pnetentry *pnetelem,
 		goto error;

 	rc = -EINVAL;
+	/* NOTE !!!: Sadly enough, but this is part of ABI.
+	 * From day one, the accesses are performed with device names and not
+	 * device indexes for both ETH and IB. It means that this function isn't
+	 * reliable after device renaming.
+	 */
 	if (!tb[SMC_PNETID_IBNAME])
 		goto error;
 	rc = -ENOENT;

[PATCH rdma-next 3/5] RDMA: Convert IB drivers to name allocation routine

[Leon Romanovsky]

From: Leon Romanovsky <leonro@mellanox.com>

Move internal implementation of device name to the IB/core.

Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
 drivers/infiniband/core/device.c               | 5 -----
 drivers/infiniband/hw/bnxt_re/main.c           | 6 +++++-
 drivers/infiniband/hw/cxgb3/iwch_provider.c    | 5 ++++-
 drivers/infiniband/hw/cxgb4/provider.c         | 5 ++++-
 drivers/infiniband/hw/hns/hns_roce_main.c      | 4 +++-
 drivers/infiniband/hw/i40iw/i40iw_verbs.c      | 7 ++++++-
 drivers/infiniband/hw/mlx4/main.c              | 7 ++++++-
 drivers/infiniband/hw/mlx5/main.c              | 4 +++-
 drivers/infiniband/hw/mthca/mthca_provider.c   | 5 ++++-
 drivers/infiniband/hw/nes/nes_verbs.c          | 6 +++++-
 drivers/infiniband/hw/ocrdma/ocrdma_main.c     | 7 ++++++-
 drivers/infiniband/hw/qedr/main.c              | 4 +++-
 drivers/infiniband/hw/usnic/usnic_ib_main.c    | 5 ++++-
 drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c | 9 ++++++---
 drivers/infiniband/sw/rxe/rxe_verbs.c          | 5 ++++-
 15 files changed, 63 insertions(+), 21 deletions(-)

diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
index 3270cde6d806..a9bc2a3f490c 100644
--- a/drivers/infiniband/core/device.c
+++ b/drivers/infiniband/core/device.c
@@ -518,11 +518,6 @@ int ib_register_device(struct ib_device *device,

 	mutex_lock(&device_mutex);

-	if (strchr(device->name, '%')) {
-		ret = alloc_name(device->name);
-		if (ret)
-			goto out;
-
 	if (ib_device_check_mandatory(device)) {
 		ret = -EINVAL;
 		goto out;
diff --git a/drivers/infiniband/hw/bnxt_re/main.c b/drivers/infiniband/hw/bnxt_re/main.c
index 20b9f31052bf..5255e5ffd6f4 100644
--- a/drivers/infiniband/hw/bnxt_re/main.c
+++ b/drivers/infiniband/hw/bnxt_re/main.c
@@ -575,11 +575,15 @@ static void bnxt_re_unregister_ib(struct bnxt_re_dev *rdev)
 static int bnxt_re_register_ib(struct bnxt_re_dev *rdev)
 {
 	struct ib_device *ibdev = &rdev->ibdev;
+	int ret;

 	/* ib device init */
 	ibdev->owner = THIS_MODULE;
 	ibdev->node_type = RDMA_NODE_IB_CA;
-	strlcpy(ibdev->name, "bnxt_re%d", IB_DEVICE_NAME_MAX);
+	ret = ib_device_alloc_name(ibdev, "bnxt_re%d");
+	if (ret)
+		return ret;
+
 	strlcpy(ibdev->node_desc, BNXT_RE_DESC " HCA",
 		strlen(BNXT_RE_DESC) + 5);
 	ibdev->phys_port_cnt = 1;
diff --git a/drivers/infiniband/hw/cxgb3/iwch_provider.c b/drivers/infiniband/hw/cxgb3/iwch_provider.c
index 1b9ff21aa1d5..3680d80036eb 100644
--- a/drivers/infiniband/hw/cxgb3/iwch_provider.c
+++ b/drivers/infiniband/hw/cxgb3/iwch_provider.c
@@ -1319,7 +1319,10 @@ int iwch_register_device(struct iwch_dev *dev)
 	int i;

 	pr_debug("%s iwch_dev %p\n", __func__, dev);
-	strlcpy(dev->ibdev.name, "cxgb3_%d", IB_DEVICE_NAME_MAX);
+	ret = ib_device_alloc_name(&dev->ibdev, "cxgb3_%d");
+	if (ret)
+		return ret;
+
 	memset(&dev->ibdev.node_guid, 0, sizeof(dev->ibdev.node_guid));
 	memcpy(&dev->ibdev.node_guid, dev->rdev.t3cdev_p->lldev->dev_addr, 6);
 	dev->ibdev.owner = THIS_MODULE;
diff --git a/drivers/infiniband/hw/cxgb4/provider.c b/drivers/infiniband/hw/cxgb4/provider.c
index 4eda6872e617..37d40cbdf0dc 100644
--- a/drivers/infiniband/hw/cxgb4/provider.c
+++ b/drivers/infiniband/hw/cxgb4/provider.c
@@ -535,7 +535,10 @@ void c4iw_register_device(struct work_struct *work)
 	struct c4iw_dev *dev = ctx->dev;

 	pr_debug("c4iw_dev %p\n", dev);
-	strlcpy(dev->ibdev.name, "cxgb4_%d", IB_DEVICE_NAME_MAX);
+	ret = ib_device_alloc_name(&dev->ibdev, "cxgb4_%d");
+	if (ret)
+		return;
+
 	memset(&dev->ibdev.node_guid, 0, sizeof(dev->ibdev.node_guid));
 	memcpy(&dev->ibdev.node_guid, dev->rdev.lldi.ports[0]->dev_addr, 6);
 	dev->ibdev.owner = THIS_MODULE;
diff --git a/drivers/infiniband/hw/hns/hns_roce_main.c b/drivers/infiniband/hw/hns/hns_roce_main.c
index 6edb547baee8..e16f72692c83 100644
--- a/drivers/infiniband/hw/hns/hns_roce_main.c
+++ b/drivers/infiniband/hw/hns/hns_roce_main.c
@@ -449,7 +449,9 @@ static int hns_roce_register_device(struct hns_roce_dev *hr_dev)
 	spin_lock_init(&iboe->lock);

 	ib_dev = &hr_dev->ib_dev;
-	strlcpy(ib_dev->name, "hns_%d", IB_DEVICE_NAME_MAX);
+	ret = ib_device_alloc_name(ib_dev, "hns_%d");
+	if (ret)
+		return ret;

 	ib_dev->owner			= THIS_MODULE;
 	ib_dev->node_type		= RDMA_NODE_IB_CA;
diff --git a/drivers/infiniband/hw/i40iw/i40iw_verbs.c b/drivers/infiniband/hw/i40iw/i40iw_verbs.c
index e2e6c74a7452..829be11da4e7 100644
--- a/drivers/infiniband/hw/i40iw/i40iw_verbs.c
+++ b/drivers/infiniband/hw/i40iw/i40iw_verbs.c
@@ -2746,13 +2746,18 @@ static struct i40iw_ib_device *i40iw_init_rdma_device(struct i40iw_device *iwdev
 	struct i40iw_ib_device *iwibdev;
 	struct net_device *netdev = iwdev->netdev;
 	struct pci_dev *pcidev = (struct pci_dev *)iwdev->hw.dev_context;
+	int ret;

 	iwibdev = (struct i40iw_ib_device *)ib_alloc_device(sizeof(*iwibdev));
 	if (!iwibdev) {
 		i40iw_pr_err("iwdev == NULL\n");
 		return NULL;
 	}
-	strlcpy(iwibdev->ibdev.name, "i40iw%d", IB_DEVICE_NAME_MAX);
+
+	ret = ib_device_alloc_name(&iwibdev->ibdev, "i40iw%d");
+	if (ret)
+		return NULL;
+
 	iwibdev->ibdev.owner = THIS_MODULE;
 	iwdev->iwibdev = iwibdev;
 	iwibdev->iwdev = iwdev;
diff --git a/drivers/infiniband/hw/mlx4/main.c b/drivers/infiniband/hw/mlx4/main.c
index bf3cdb88aaf5..8edbc50d298a 100644
--- a/drivers/infiniband/hw/mlx4/main.c
+++ b/drivers/infiniband/hw/mlx4/main.c
@@ -2540,7 +2540,10 @@ static void *mlx4_ib_add(struct mlx4_dev *dev)
 	ibdev->dev = dev;
 	ibdev->bond_next_port	= 0;

-	strlcpy(ibdev->ib_dev.name, "mlx4_%d", IB_DEVICE_NAME_MAX);
+	err = ib_device_alloc_name(&ibdev->ib_dev, "mlx4_%d");
+	if (err)
+		goto err_name;
+
 	ibdev->ib_dev.owner		= THIS_MODULE;
 	ibdev->ib_dev.node_type		= RDMA_NODE_IB_CA;
 	ibdev->ib_dev.local_dma_lkey	= dev->caps.reserved_lkey;
@@ -2882,6 +2885,8 @@ static void *mlx4_ib_add(struct mlx4_dev *dev)

 err_map:
 	mlx4_ib_free_eqs(dev, ibdev);
+
+err_name:
 	iounmap(ibdev->uar_map);

 err_uar:
diff --git a/drivers/infiniband/hw/mlx5/main.c b/drivers/infiniband/hw/mlx5/main.c
index 73782100750d..a4a578ba72ee 100644
--- a/drivers/infiniband/hw/mlx5/main.c
+++ b/drivers/infiniband/hw/mlx5/main.c
@@ -5698,8 +5698,10 @@ int mlx5_ib_stage_init_init(struct mlx5_ib_dev *dev)
 		name = "mlx5_%d";
 	else
 		name = "mlx5_bond_%d";
+	err = ib_device_alloc_name(&dev->ib_dev, name);
+	if (err)
+		goto err_mp;

-	strlcpy(dev->ib_dev.name, name, IB_DEVICE_NAME_MAX);
 	dev->ib_dev.owner		= THIS_MODULE;
 	dev->ib_dev.node_type		= RDMA_NODE_IB_CA;
 	dev->ib_dev.local_dma_lkey	= 0 /* not supported for now */;
diff --git a/drivers/infiniband/hw/mthca/mthca_provider.c b/drivers/infiniband/hw/mthca/mthca_provider.c
index 0d3473b4596e..67ff3ecdf8d5 100644
--- a/drivers/infiniband/hw/mthca/mthca_provider.c
+++ b/drivers/infiniband/hw/mthca/mthca_provider.c
@@ -1198,7 +1198,10 @@ int mthca_register_device(struct mthca_dev *dev)
 	if (ret)
 		return ret;

-	strlcpy(dev->ib_dev.name, "mthca%d", IB_DEVICE_NAME_MAX);
+	ret = ib_device_alloc_name(&dev->ib_dev, "mthca%d");
+	if (ret)
+		return ret;
+
 	dev->ib_dev.owner                = THIS_MODULE;

 	dev->ib_dev.uverbs_abi_ver	 = MTHCA_UVERBS_ABI_VERSION;
diff --git a/drivers/infiniband/hw/nes/nes_verbs.c b/drivers/infiniband/hw/nes/nes_verbs.c
index 6940c7215961..7394d50b02f5 100644
--- a/drivers/infiniband/hw/nes/nes_verbs.c
+++ b/drivers/infiniband/hw/nes/nes_verbs.c
@@ -3635,12 +3635,16 @@ struct nes_ib_device *nes_init_ofa_device(struct net_device *netdev)
 	struct nes_ib_device *nesibdev;
 	struct nes_vnic *nesvnic = netdev_priv(netdev);
 	struct nes_device *nesdev = nesvnic->nesdev;
+	int ret;

 	nesibdev = (struct nes_ib_device *)ib_alloc_device(sizeof(struct nes_ib_device));
 	if (nesibdev == NULL) {
 		return NULL;
 	}
-	strlcpy(nesibdev->ibdev.name, "nes%d", IB_DEVICE_NAME_MAX);
+	ret = ib_device_alloc_name(&nesibdev->ibdev, "nes%d");
+	if (ret)
+		return NULL;
+
 	nesibdev->ibdev.owner = THIS_MODULE;

 	nesibdev->ibdev.node_type = RDMA_NODE_RNIC;
diff --git a/drivers/infiniband/hw/ocrdma/ocrdma_main.c b/drivers/infiniband/hw/ocrdma/ocrdma_main.c
index 7832ee3e0c84..34f7d8f31ee1 100644
--- a/drivers/infiniband/hw/ocrdma/ocrdma_main.c
+++ b/drivers/infiniband/hw/ocrdma/ocrdma_main.c
@@ -116,7 +116,12 @@ static void get_dev_fw_str(struct ib_device *device, char *str)

 static int ocrdma_register_device(struct ocrdma_dev *dev)
 {
-	strlcpy(dev->ibdev.name, "ocrdma%d", IB_DEVICE_NAME_MAX);
+	int ret;
+
+	ret = ib_device_alloc_name(&dev->ibdev, "ocrdma%d");
+	if (ret)
+		return ret;
+
 	ocrdma_get_guid(dev, (u8 *)&dev->ibdev.node_guid);
 	BUILD_BUG_ON(sizeof(OCRDMA_NODE_DESC) > IB_DEVICE_NODE_DESC_MAX);
 	memcpy(dev->ibdev.node_desc, OCRDMA_NODE_DESC,
diff --git a/drivers/infiniband/hw/qedr/main.c b/drivers/infiniband/hw/qedr/main.c
index a0af6d424aed..43e14f6b1c6d 100644
--- a/drivers/infiniband/hw/qedr/main.c
+++ b/drivers/infiniband/hw/qedr/main.c
@@ -170,7 +170,9 @@ static int qedr_register_device(struct qedr_dev *dev)
 {
 	int rc;

-	strlcpy(dev->ibdev.name, "qedr%d", IB_DEVICE_NAME_MAX);
+	rc = ib_device_alloc_name(&dev->ibdev, "qedr%d");
+	if (rc)
+		return rc;

 	dev->ibdev.node_guid = dev->attr.node_guid;
 	memcpy(dev->ibdev.node_desc, QEDR_NODE_DESC, sizeof(QEDR_NODE_DESC));
diff --git a/drivers/infiniband/hw/usnic/usnic_ib_main.c b/drivers/infiniband/hw/usnic/usnic_ib_main.c
index f0538a460328..15f1509cac8d 100644
--- a/drivers/infiniband/hw/usnic/usnic_ib_main.c
+++ b/drivers/infiniband/hw/usnic/usnic_ib_main.c
@@ -335,6 +335,7 @@ static void *usnic_ib_device_add(struct pci_dev *dev)
 	union ib_gid gid;
 	struct in_device *ind;
 	struct net_device *netdev;
+	int ret;

 	usnic_dbg("\n");
 	netdev = pci_get_drvdata(dev);
@@ -364,7 +365,9 @@ static void *usnic_ib_device_add(struct pci_dev *dev)
 	us_ibdev->ib_dev.num_comp_vectors = USNIC_IB_NUM_COMP_VECTORS;
 	us_ibdev->ib_dev.dev.parent = &dev->dev;
 	us_ibdev->ib_dev.uverbs_abi_ver = USNIC_UVERBS_ABI_VERSION;
-	strlcpy(us_ibdev->ib_dev.name, "usnic_%d", IB_DEVICE_NAME_MAX);
+	ret = ib_device_alloc_name(&us_ibdev->ib_dev, "usnic_%d");
+	if (ret)
+		goto err_fwd_dealloc;

 	us_ibdev->ib_dev.uverbs_cmd_mask =
 		(1ull << IB_USER_VERBS_CMD_GET_CONTEXT) |
diff --git a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c
index a5719899f49a..afe84d55bb52 100644
--- a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c
+++ b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c
@@ -159,10 +159,13 @@ static struct net_device *pvrdma_get_netdev(struct ib_device *ibdev,

 static int pvrdma_register_device(struct pvrdma_dev *dev)
 {
-	int ret = -1;
 	int i = 0;
+	int ret;
+
+	ret = ib_device_alloc_name(&dev->ib_dev, "vmw_pvrdma%d");
+	if (ret)
+		return ret;

-	strlcpy(dev->ib_dev.name, "vmw_pvrdma%d", IB_DEVICE_NAME_MAX);
 	dev->ib_dev.node_guid = dev->dsr->caps.node_guid;
 	dev->sys_image_guid = dev->dsr->caps.sys_image_guid;
 	dev->flags = 0;
@@ -235,7 +238,7 @@ static int pvrdma_register_device(struct pvrdma_dev *dev)
 	dev->cq_tbl = kcalloc(dev->dsr->caps.max_cq, sizeof(struct pvrdma_cq *),
 			      GFP_KERNEL);
 	if (!dev->cq_tbl)
-		return ret;
+		return -ENOMEM;
 	spin_lock_init(&dev->cq_tbl_lock);

 	dev->qp_tbl = kcalloc(dev->dsr->caps.max_qp, sizeof(struct pvrdma_qp *),
diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.c b/drivers/infiniband/sw/rxe/rxe_verbs.c
index f5b1e0ad6142..2b017bfeecda 100644
--- a/drivers/infiniband/sw/rxe/rxe_verbs.c
+++ b/drivers/infiniband/sw/rxe/rxe_verbs.c
@@ -1159,7 +1159,10 @@ int rxe_register_device(struct rxe_dev *rxe)
 	struct ib_device *dev = &rxe->ib_dev;
 	struct crypto_shash *tfm;

-	strlcpy(dev->name, "rxe%d", IB_DEVICE_NAME_MAX);
+	err = ib_device_alloc_name(dev, "rxe%d");
+	if (err)
+		return err;
+
 	strlcpy(dev->node_desc, "rxe", sizeof(dev->node_desc));

 	dev->owner = THIS_MODULE;

[PATCH rdma-next 4/5] RDMA/core: Implement IB device rename function

[Leon Romanovsky]

From: Leon Romanovsky <leonro@mellanox.com>

Generic implementation of IB device rename function.

Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
 drivers/infiniband/core/core_priv.h |  1 +
 drivers/infiniband/core/device.c    | 23 +++++++++++++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/drivers/infiniband/core/core_priv.h b/drivers/infiniband/core/core_priv.h
index d7399d5b1cb6..c5881756b799 100644
--- a/drivers/infiniband/core/core_priv.h
+++ b/drivers/infiniband/core/core_priv.h
@@ -87,6 +87,7 @@ int  ib_device_register_sysfs(struct ib_device *device,
 			      int (*port_callback)(struct ib_device *,
 						   u8, struct kobject *));
 void ib_device_unregister_sysfs(struct ib_device *device);
+int ib_device_rename(struct ib_device *ibdev, const char *name);

 typedef void (*roce_netdev_callback)(struct ib_device *device, u8 port,
 	      struct net_device *idev, void *cookie);
diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
index a9bc2a3f490c..36b79cd6b620 100644
--- a/drivers/infiniband/core/device.c
+++ b/drivers/infiniband/core/device.c
@@ -178,6 +178,29 @@ void ib_device_get_name(struct ib_device *ibdev, char *name)
 }
 EXPORT_SYMBOL(ib_device_get_name);

+int ib_device_rename(struct ib_device *ibdev, const char *name)
+{
+	struct ib_device *device;
+	int ret = 0;
+
+	if (!strcmp(name, ibdev->name))
+		return ret;
+
+	mutex_lock(&device_mutex);
+	list_for_each_entry(device, &device_list, core_list) {
+		if (!strcmp(name, device->name)) {
+			ret = -EEXIST;
+			goto out;
+		}
+	}
+
+	strlcpy(ibdev->name, name, IB_DEVICE_NAME_MAX);
+	ret = device_rename(&ibdev->dev, name);
+out:
+	mutex_unlock(&device_mutex);
+	return ret;
+}
+
 static int alloc_name(char *name)
 {
 	unsigned long *inuse;

[PATCH rdma-next 5/5] RDMA/nldev: Allow IB device rename through RDMA netlink

[Leon Romanovsky]

From: Leon Romanovsky <leonro@mellanox.com>

Provide an option to rename IB device name through RDMA netlink and
limit it to users with ADMIN capability only.

Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
 drivers/infiniband/core/nldev.c  | 33 +++++++++++++++++++++++++++++++++
 include/uapi/rdma/rdma_netlink.h |  3 ++-
 2 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/core/nldev.c b/drivers/infiniband/core/nldev.c
index 0385ab438320..64e1d12e9678 100644
--- a/drivers/infiniband/core/nldev.c
+++ b/drivers/infiniband/core/nldev.c
@@ -645,6 +645,35 @@ static int nldev_get_doit(struct sk_buff *skb, struct nlmsghdr *nlh,
 	return err;
 }

+static int nldev_set_doit(struct sk_buff *skb, struct nlmsghdr *nlh,
+			  struct netlink_ext_ack *extack)
+{
+	struct nlattr *tb[RDMA_NLDEV_ATTR_MAX];
+	struct ib_device *device;
+	u32 index;
+	int err;
+
+	err = nlmsg_parse(nlh, 0, tb, RDMA_NLDEV_ATTR_MAX - 1, nldev_policy,
+			  extack);
+	if (err || !tb[RDMA_NLDEV_ATTR_DEV_INDEX])
+		return -EINVAL;
+
+	index = nla_get_u32(tb[RDMA_NLDEV_ATTR_DEV_INDEX]);
+	device = ib_device_get_by_index(index);
+	if (!device)
+		return -EINVAL;
+
+	if (tb[RDMA_NLDEV_ATTR_DEV_NAME]) {
+		char name[IB_DEVICE_NAME_MAX] = {};
+
+		nla_strlcpy(name, tb[RDMA_NLDEV_ATTR_DEV_NAME],
+			    IB_DEVICE_NAME_MAX);
+		return ib_device_rename(device, name);
+	}
+
+	return 0;
+}
+
 static int _nldev_get_dumpit(struct ib_device *device,
 			     struct sk_buff *skb,
 			     struct netlink_callback *cb,
@@ -1077,6 +1106,10 @@ static const struct rdma_nl_cbs nldev_cb_table[RDMA_NLDEV_NUM_OPS] = {
 		.doit = nldev_get_doit,
 		.dump = nldev_get_dumpit,
 	},
+	[RDMA_NLDEV_CMD_SET] = {
+		.doit = nldev_set_doit,
+		.flags = RDMA_NL_ADMIN_PERM,
+	},
 	[RDMA_NLDEV_CMD_PORT_GET] = {
 		.doit = nldev_port_get_doit,
 		.dump = nldev_port_get_dumpit,
diff --git a/include/uapi/rdma/rdma_netlink.h b/include/uapi/rdma/rdma_netlink.h
index edba6351ac13..f9c41bf59efc 100644
--- a/include/uapi/rdma/rdma_netlink.h
+++ b/include/uapi/rdma/rdma_netlink.h
@@ -227,8 +227,9 @@ enum rdma_nldev_command {
 	RDMA_NLDEV_CMD_UNSPEC,

 	RDMA_NLDEV_CMD_GET, /* can dump */
+	RDMA_NLDEV_CMD_SET,

-	/* 2 - 4 are free to use */
+	/* 3 - 4 are free to use */

 	RDMA_NLDEV_CMD_PORT_GET = 5, /* can dump */

Re: [PATCH rdma-next 1/5] RDMA/core: Provide getter and setter to access IB device name

[Steve Wise]

On 9/20/2018 6:21 AM, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@mellanox.com>
> 
> Prepare IB device name field to rename operation by ensuring that all
> accesses to it are protected with lock and users don't see part of name.
> 
> The protection is done with global device_lock because it is used in
> allocation and deallocation phases. At this stage, this lock is not
> busy and easily can be moved to be per-device, once it will be needed.
> 
> Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> ---
>  drivers/infiniband/core/device.c | 24 +++++++++++++++++++++++-
>  include/rdma/ib_verbs.h          |  8 +++++++-
>  2 files changed, 30 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
> index 5a680a88aa87..3270cde6d806 100644
> --- a/drivers/infiniband/core/device.c
> +++ b/drivers/infiniband/core/device.c
> @@ -170,6 +170,14 @@ static struct ib_device *__ib_device_get_by_name(const char *name)
>  	return NULL;
>  }
> 
> +void ib_device_get_name(struct ib_device *ibdev, char *name)
> +{
> +	down_read(&lists_rwsem);
> +	strlcpy(name, ibdev->name, IB_DEVICE_NAME_MAX);
> +	up_read(&lists_rwsem);
> +}
> +EXPORT_SYMBOL(ib_device_get_name);
> +
>  static int alloc_name(char *name)
>  {
>  	unsigned long *inuse;
> @@ -202,6 +210,21 @@ static int alloc_name(char *name)
>  	return 0;
>  }
> 
> +int ib_device_alloc_name(struct ib_device *ibdev, const char *pattern)
> +{
> +	int ret = 0;
> +
> +	mutex_lock(&device_mutex);
> +	strlcpy(ibdev->name, pattern, IB_DEVICE_NAME_MAX);
> +	if (strchr(ibdev->name, '%'))
> +		ret = alloc_name(ibdev->name);
> +
> +	mutex_unlock(&device_mutex);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL(ib_device_alloc_name);
> +
>  static void ib_device_release(struct device *device)
>  {
>  	struct ib_device *dev = container_of(device, struct ib_device, dev);
> @@ -499,7 +522,6 @@ int ib_register_device(struct ib_device *device,
>  		ret = alloc_name(device->name);
>  		if (ret)
>  			goto out;
> -	}

I don't think this is correct...


> 
>  	if (ib_device_check_mandatory(device)) {
>  		ret = -EINVAL;
> diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
> index e764ed1f6025..66660e7b9854 100644
> --- a/include/rdma/ib_verbs.h
> +++ b/include/rdma/ib_verbs.h
> @@ -2260,6 +2260,11 @@ struct ib_device {
>  	/* Do not access @dma_device directly from ULP nor from HW drivers. */
>  	struct device                *dma_device;
> 
> +	/*
> +	 * Do not access @name directly,
> +	 * use ib_device_get_name()/ib_device_alloc_name()
> +	 * and don't assume that it can't change after access.
> +	 */
>  	char                          name[IB_DEVICE_NAME_MAX];
> 
>  	struct list_head              event_handler_list;
> @@ -2638,7 +2643,8 @@ struct ib_device *ib_alloc_device(size_t size);
>  void ib_dealloc_device(struct ib_device *device);
> 
>  void ib_get_device_fw_str(struct ib_device *device, char *str);
> -
> +int ib_device_alloc_name(struct ib_device *ibdev, const char *pattern);
> +void ib_device_get_name(struct ib_device *ibdev, char *name);
>  int ib_register_device(struct ib_device *device,
>  		       int (*port_callback)(struct ib_device *,
>  					    u8, struct kobject *));
> --
> 2.14.4
> 

Re: [PATCH rdma-next 1/5] RDMA/core: Provide getter and setter to access IB device name

[Jason Gunthorpe]

On Thu, Sep 20, 2018 at 02:21:58PM +0300, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@mellanox.com>
> 
> Prepare IB device name field to rename operation by ensuring that all
> accesses to it are protected with lock and users don't see part of name.

Oh dear, no, that isn't going to work, there is too much stuff using
dev_name.. Did you read the comment on device_rename??

https://elixir.bootlin.com/linux/v4.19-rc4/source/drivers/base/core.c#L2715

> The protection is done with global device_lock because it is used in
> allocation and deallocation phases. At this stage, this lock is not
> busy and easily can be moved to be per-device, once it will be needed.
> 
> Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
>  drivers/infiniband/core/device.c | 24 +++++++++++++++++++++++-
>  include/rdma/ib_verbs.h          |  8 +++++++-
>  2 files changed, 30 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
> index 5a680a88aa87..3270cde6d806 100644
> +++ b/drivers/infiniband/core/device.c
> @@ -170,6 +170,14 @@ static struct ib_device *__ib_device_get_by_name(const char *name)
>  	return NULL;
>  }
> 
> +void ib_device_get_name(struct ib_device *ibdev, char *name)
> +{
> +	down_read(&lists_rwsem);
> +	strlcpy(name, ibdev->name, IB_DEVICE_NAME_MAX);
> +	up_read(&lists_rwsem);
> +}
> +EXPORT_SYMBOL(ib_device_get_name);

I think we have to follow netdev and just rely on device_rename()
being 'good enough'.  

Switch everything to use dev_name()/etc rather than try and do
something like this so the responsibility is on the device core to
keep this working, not us.

Turns out I have a series for that for unrelated reasons..

>  static int alloc_name(char *name)
>  {
>  	unsigned long *inuse;
> @@ -202,6 +210,21 @@ static int alloc_name(char *name)
>  	return 0;
>  }
> 
> +int ib_device_alloc_name(struct ib_device *ibdev, const char *pattern)
> +{
> +	int ret = 0;
> +
> +	mutex_lock(&device_mutex);
> +	strlcpy(ibdev->name, pattern, IB_DEVICE_NAME_MAX);
> +	if (strchr(ibdev->name, '%'))
> +		ret = alloc_name(ibdev->name);
> +
> +	mutex_unlock(&device_mutex);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL(ib_device_alloc_name);

Can't call alloc_name() without also adding to the list, this will
allow duplicates.

Jason

Re: [PATCH rdma-next 1/5] RDMA/core: Provide getter and setter to access IB device name

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 2700 bytes --]

On Thu, Sep 20, 2018 at 09:15:41AM -0600, Jason Gunthorpe wrote:
> On Thu, Sep 20, 2018 at 02:21:58PM +0300, Leon Romanovsky wrote:
> > From: Leon Romanovsky <leonro@mellanox.com>
> >
> > Prepare IB device name field to rename operation by ensuring that all
> > accesses to it are protected with lock and users don't see part of name.
>
> Oh dear, no, that isn't going to work, there is too much stuff using
> dev_name.. Did you read the comment on device_rename??
>
> https://elixir.bootlin.com/linux/v4.19-rc4/source/drivers/base/core.c#L2715

Yes, I read, it was mentioned in the cover letter.

>
> > The protection is done with global device_lock because it is used in
> > allocation and deallocation phases. At this stage, this lock is not
> > busy and easily can be moved to be per-device, once it will be needed.
> >
> > Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> >  drivers/infiniband/core/device.c | 24 +++++++++++++++++++++++-
> >  include/rdma/ib_verbs.h          |  8 +++++++-
> >  2 files changed, 30 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
> > index 5a680a88aa87..3270cde6d806 100644
> > +++ b/drivers/infiniband/core/device.c
> > @@ -170,6 +170,14 @@ static struct ib_device *__ib_device_get_by_name(const char *name)
> >  	return NULL;
> >  }
> >
> > +void ib_device_get_name(struct ib_device *ibdev, char *name)
> > +{
> > +	down_read(&lists_rwsem);
> > +	strlcpy(name, ibdev->name, IB_DEVICE_NAME_MAX);
> > +	up_read(&lists_rwsem);
> > +}
> > +EXPORT_SYMBOL(ib_device_get_name);
>
> I think we have to follow netdev and just rely on device_rename()
> being 'good enough'.
>
> Switch everything to use dev_name()/etc rather than try and do
> something like this so the responsibility is on the device core to
> keep this working, not us.
>
> Turns out I have a series for that for unrelated reasons..

And what should I do now with this knowledge?

>
> >  static int alloc_name(char *name)
> >  {
> >  	unsigned long *inuse;
> > @@ -202,6 +210,21 @@ static int alloc_name(char *name)
> >  	return 0;
> >  }
> >
> > +int ib_device_alloc_name(struct ib_device *ibdev, const char *pattern)
> > +{
> > +	int ret = 0;
> > +
> > +	mutex_lock(&device_mutex);
> > +	strlcpy(ibdev->name, pattern, IB_DEVICE_NAME_MAX);
> > +	if (strchr(ibdev->name, '%'))
> > +		ret = alloc_name(ibdev->name);
> > +
> > +	mutex_unlock(&device_mutex);
> > +
> > +	return ret;
> > +}
> > +EXPORT_SYMBOL(ib_device_alloc_name);
>
> Can't call alloc_name() without also adding to the list, this will
> allow duplicates.

I planned to change it in the future by moving to different name scheme
with unique naming.

>
> Jason

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH rdma-next 1/5] RDMA/core: Provide getter and setter to access IB device name

[Jason Gunthorpe]

On Thu, Sep 20, 2018 at 07:40:39PM +0300, Leon Romanovsky wrote:

> > > The protection is done with global device_lock because it is used in
> > > allocation and deallocation phases. At this stage, this lock is not
> > > busy and easily can be moved to be per-device, once it will be needed.
> > >
> > > Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> > >  drivers/infiniband/core/device.c | 24 +++++++++++++++++++++++-
> > >  include/rdma/ib_verbs.h          |  8 +++++++-
> > >  2 files changed, 30 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
> > > index 5a680a88aa87..3270cde6d806 100644
> > > +++ b/drivers/infiniband/core/device.c
> > > @@ -170,6 +170,14 @@ static struct ib_device *__ib_device_get_by_name(const char *name)
> > >  	return NULL;
> > >  }
> > >
> > > +void ib_device_get_name(struct ib_device *ibdev, char *name)
> > > +{
> > > +	down_read(&lists_rwsem);
> > > +	strlcpy(name, ibdev->name, IB_DEVICE_NAME_MAX);
> > > +	up_read(&lists_rwsem);
> > > +}
> > > +EXPORT_SYMBOL(ib_device_get_name);
> >
> > I think we have to follow netdev and just rely on device_rename()
> > being 'good enough'.
> >
> > Switch everything to use dev_name()/etc rather than try and do
> > something like this so the responsibility is on the device core to
> > keep this working, not us.
> >
> > Turns out I have a series for that for unrelated reasons..
> 
> And what should I do now with this knowledge?

Maybe I can send it today..

Jason