diff mbox series

[v6,4/4] rdma_rxe: use netlink messages to add/delete links

Message ID e63cec57a13b40731730778d71b66cc9f1ca513a.1544033819.git.swise@opengridcomputing.com (mailing list archive)
State Superseded
Headers show
Series Dynamic rdma link creation | expand

Commit Message

Steve Wise Dec. 5, 2018, 3:14 p.m. UTC 
Add support for the RDMA_NLDEV_CMD_NEWLINK/DELLINK messages which allow
dynamically adding new RXE links.  Deprecate the old module options
for now.

Cc: Moni Shoua <monis@mellanox.com>
Reviewed-by: Yanjun Zhu <yanjun.zhu@oracle.com>
Signed-off-by: Steve Wise <swise@opengridcomputing.com>
---
 drivers/infiniband/sw/rxe/rxe.c       | 66 +++++++++++++++++++++++++++++++++--
 drivers/infiniband/sw/rxe/rxe.h       |  3 +-
 drivers/infiniband/sw/rxe/rxe_net.c   | 21 +++++++++--
 drivers/infiniband/sw/rxe/rxe_net.h   |  2 +-
 drivers/infiniband/sw/rxe/rxe_sysfs.c |  6 ++--
 drivers/infiniband/sw/rxe/rxe_verbs.c |  4 +--
 drivers/infiniband/sw/rxe/rxe_verbs.h |  2 +-
 7 files changed, 92 insertions(+), 12 deletions(-)

Comments

Leon Romanovsky Dec. 6, 2018, 7:02 a.m. UTC | #1 
On Wed, Dec 05, 2018 at 07:14:20AM -0800, Steve Wise wrote:
> Add support for the RDMA_NLDEV_CMD_NEWLINK/DELLINK messages which allow
> dynamically adding new RXE links.  Deprecate the old module options
> for now.
>
> Cc: Moni Shoua <monis@mellanox.com>
> Reviewed-by: Yanjun Zhu <yanjun.zhu@oracle.com>
> Signed-off-by: Steve Wise <swise@opengridcomputing.com>
> ---
>  drivers/infiniband/sw/rxe/rxe.c       | 66 +++++++++++++++++++++++++++++++++--
>  drivers/infiniband/sw/rxe/rxe.h       |  3 +-
>  drivers/infiniband/sw/rxe/rxe_net.c   | 21 +++++++++--
>  drivers/infiniband/sw/rxe/rxe_net.h   |  2 +-
>  drivers/infiniband/sw/rxe/rxe_sysfs.c |  6 ++--
>  drivers/infiniband/sw/rxe/rxe_verbs.c |  4 +--
>  drivers/infiniband/sw/rxe/rxe_verbs.h |  2 +-
>  7 files changed, 92 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/infiniband/sw/rxe/rxe.c b/drivers/infiniband/sw/rxe/rxe.c
> index 19d8e522b72b..b98141442cb2 100644
> --- a/drivers/infiniband/sw/rxe/rxe.c
> +++ b/drivers/infiniband/sw/rxe/rxe.c
> @@ -31,6 +31,7 @@
>   * SOFTWARE.
>   */
>
> +#include <rdma/rdma_netlink.h>
>  #include <net/addrconf.h>
>  #include "rxe.h"
>  #include "rxe_loc.h"
> @@ -309,7 +310,7 @@ void rxe_set_mtu(struct rxe_dev *rxe, unsigned int ndev_mtu)
>  /* called by ifc layer to create new rxe device.
>   * The caller should allocate memory for rxe by calling ib_alloc_device.
>   */
> -int rxe_add(struct rxe_dev *rxe, unsigned int mtu)
> +int rxe_add(struct rxe_dev *rxe, unsigned int mtu, const char *ibdev_name)
>  {
>  	int err;
>
> @@ -321,7 +322,7 @@ int rxe_add(struct rxe_dev *rxe, unsigned int mtu)
>
>  	rxe_set_mtu(rxe, mtu);
>
> -	err = rxe_register_device(rxe);
> +	err = rxe_register_device(rxe, ibdev_name);
>  	if (err)
>  		goto err1;
>
> @@ -332,6 +333,63 @@ int rxe_add(struct rxe_dev *rxe, unsigned int mtu)
>  	return err;
>  }
>
> +static struct ib_device *rxe_newlink(const char *ibdev_name,
> +				     const char *ndev_name)
> +{
> +	struct net_device *ndev = NULL;
> +	struct rxe_dev *rxe;
> +	int err = 0;
> +
> +	ndev = dev_get_by_name(&init_net, ndev_name);
> +	if (!ndev) {
> +		pr_err("interface %s not found\n", ndev_name);
> +		err = -ENODEV;
> +		goto err;
> +	}
> +
> +	rxe = net_to_rxe(ndev);
> +	if (rxe) {
> +		pr_err("already configured on %s\n", ndev_name);
> +		err = -EEXIST;
> +		rxe_dev_put(rxe);
> +		goto err;
> +	}

Shouldn't the code above be part of core logic instead of being in driver?

Thanks
Steve Wise Dec. 6, 2018, 3:39 p.m. UTC | #2 
On 12/6/2018 1:02 AM, Leon Romanovsky wrote:
> On Wed, Dec 05, 2018 at 07:14:20AM -0800, Steve Wise wrote:
>> Add support for the RDMA_NLDEV_CMD_NEWLINK/DELLINK messages which allow
>> dynamically adding new RXE links.  Deprecate the old module options
>> for now.
>>
>> Cc: Moni Shoua <monis@mellanox.com>
>> Reviewed-by: Yanjun Zhu <yanjun.zhu@oracle.com>
>> Signed-off-by: Steve Wise <swise@opengridcomputing.com>
>> ---
>>  drivers/infiniband/sw/rxe/rxe.c       | 66 +++++++++++++++++++++++++++++++++--
>>  drivers/infiniband/sw/rxe/rxe.h       |  3 +-
>>  drivers/infiniband/sw/rxe/rxe_net.c   | 21 +++++++++--
>>  drivers/infiniband/sw/rxe/rxe_net.h   |  2 +-
>>  drivers/infiniband/sw/rxe/rxe_sysfs.c |  6 ++--
>>  drivers/infiniband/sw/rxe/rxe_verbs.c |  4 +--
>>  drivers/infiniband/sw/rxe/rxe_verbs.h |  2 +-
>>  7 files changed, 92 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/infiniband/sw/rxe/rxe.c b/drivers/infiniband/sw/rxe/rxe.c
>> index 19d8e522b72b..b98141442cb2 100644
>> --- a/drivers/infiniband/sw/rxe/rxe.c
>> +++ b/drivers/infiniband/sw/rxe/rxe.c
>> @@ -31,6 +31,7 @@
>>   * SOFTWARE.
>>   */
>>
>> +#include <rdma/rdma_netlink.h>
>>  #include <net/addrconf.h>
>>  #include "rxe.h"
>>  #include "rxe_loc.h"
>> @@ -309,7 +310,7 @@ void rxe_set_mtu(struct rxe_dev *rxe, unsigned int ndev_mtu)
>>  /* called by ifc layer to create new rxe device.
>>   * The caller should allocate memory for rxe by calling ib_alloc_device.
>>   */
>> -int rxe_add(struct rxe_dev *rxe, unsigned int mtu)
>> +int rxe_add(struct rxe_dev *rxe, unsigned int mtu, const char *ibdev_name)
>>  {
>>  	int err;
>>
>> @@ -321,7 +322,7 @@ int rxe_add(struct rxe_dev *rxe, unsigned int mtu)
>>
>>  	rxe_set_mtu(rxe, mtu);
>>
>> -	err = rxe_register_device(rxe);
>> +	err = rxe_register_device(rxe, ibdev_name);
>>  	if (err)
>>  		goto err1;
>>
>> @@ -332,6 +333,63 @@ int rxe_add(struct rxe_dev *rxe, unsigned int mtu)
>>  	return err;
>>  }
>>
>> +static struct ib_device *rxe_newlink(const char *ibdev_name,
>> +				     const char *ndev_name)
>> +{
>> +	struct net_device *ndev = NULL;
>> +	struct rxe_dev *rxe;
>> +	int err = 0;
>> +
>> +	ndev = dev_get_by_name(&init_net, ndev_name);
>> +	if (!ndev) {
>> +		pr_err("interface %s not found\n", ndev_name);
>> +		err = -ENODEV;
>> +		goto err;
>> +	}
>> +
>> +	rxe = net_to_rxe(ndev);
>> +	if (rxe) {
>> +		pr_err("already configured on %s\n", ndev_name);
>> +		err = -EEXIST;
>> +		rxe_dev_put(rxe);
>> +		goto err;
>> +	}
> Shouldn't the code above be part of core logic instead of being in driver?
>
> Thanks

So nldev_newlink() would find the net device and pass it to the
ops->newlink function instead of passing the net device name?  That
sounds good.

Thanks!
diff mbox series

Patch

diff --git a/drivers/infiniband/sw/rxe/rxe.c b/drivers/infiniband/sw/rxe/rxe.c
index 19d8e522b72b..b98141442cb2 100644
--- a/drivers/infiniband/sw/rxe/rxe.c
+++ b/drivers/infiniband/sw/rxe/rxe.c
@@ -31,6 +31,7 @@ 
  * SOFTWARE.
  */
 
+#include <rdma/rdma_netlink.h>
 #include <net/addrconf.h>
 #include "rxe.h"
 #include "rxe_loc.h"
@@ -309,7 +310,7 @@  void rxe_set_mtu(struct rxe_dev *rxe, unsigned int ndev_mtu)
 /* called by ifc layer to create new rxe device.
  * The caller should allocate memory for rxe by calling ib_alloc_device.
  */
-int rxe_add(struct rxe_dev *rxe, unsigned int mtu)
+int rxe_add(struct rxe_dev *rxe, unsigned int mtu, const char *ibdev_name)
 {
 	int err;
 
@@ -321,7 +322,7 @@  int rxe_add(struct rxe_dev *rxe, unsigned int mtu)
 
 	rxe_set_mtu(rxe, mtu);
 
-	err = rxe_register_device(rxe);
+	err = rxe_register_device(rxe, ibdev_name);
 	if (err)
 		goto err1;
 
@@ -332,6 +333,63 @@  int rxe_add(struct rxe_dev *rxe, unsigned int mtu)
 	return err;
 }
 
+static struct ib_device *rxe_newlink(const char *ibdev_name,
+				     const char *ndev_name)
+{
+	struct net_device *ndev = NULL;
+	struct rxe_dev *rxe;
+	int err = 0;
+
+	ndev = dev_get_by_name(&init_net, ndev_name);
+	if (!ndev) {
+		pr_err("interface %s not found\n", ndev_name);
+		err = -ENODEV;
+		goto err;
+	}
+
+	rxe = net_to_rxe(ndev);
+	if (rxe) {
+		pr_err("already configured on %s\n", ndev_name);
+		err = -EEXIST;
+		rxe_dev_put(rxe);
+		goto err;
+	}
+
+	rxe = rxe_net_add(ibdev_name, ndev);
+	if (!rxe) {
+		pr_err("failed to add %s\n", ndev_name);
+		err = -EINVAL;
+		goto err;
+	}
+
+	if (netif_running(ndev) && netif_carrier_ok(ndev))
+		rxe_port_up(rxe);
+	else
+		rxe_port_down(rxe);
+	pr_info("added %s to %s\n", rxe->ib_dev.name, ndev->name);
+err:
+	if (ndev)
+		dev_put(ndev);
+	return err ? ERR_PTR(err) : &rxe->ib_dev;
+}
+
+static int rxe_dellink(struct ib_device *device)
+{
+	struct rxe_dev *rxe = ibdev_to_rxe(device);
+
+	if (!rxe)
+		return -ENODEV;
+	rxe_net_remove(rxe);
+	rxe_dev_put(rxe);
+	return 0;
+}
+
+static struct rdma_link_ops rxe_link_ops = {
+	.type = "rxe",
+	.newlink = rxe_newlink,
+	.dellink = rxe_dellink,
+};
+
 static int __init rxe_module_init(void)
 {
 	int err;
@@ -347,12 +405,14 @@  static int __init rxe_module_init(void)
 	if (err)
 		return err;
 
+	rdma_link_register(&rxe_link_ops);
 	pr_info("loaded\n");
 	return 0;
 }
 
 static void __exit rxe_module_exit(void)
 {
+	rdma_link_unregister(&rxe_link_ops);
 	rxe_remove_all();
 	rxe_net_exit();
 	rxe_cache_exit();
@@ -362,3 +422,5 @@  static void __exit rxe_module_exit(void)
 
 late_initcall(rxe_module_init);
 module_exit(rxe_module_exit);
+
+MODULE_ALIAS_RDMA_LINK("rxe");
diff --git a/drivers/infiniband/sw/rxe/rxe.h b/drivers/infiniband/sw/rxe/rxe.h
index e0c0dce80bbf..b8e754af6310 100644
--- a/drivers/infiniband/sw/rxe/rxe.h
+++ b/drivers/infiniband/sw/rxe/rxe.h
@@ -95,7 +95,7 @@  static inline u32 rxe_crc32(struct rxe_dev *rxe,
 
 void rxe_set_mtu(struct rxe_dev *rxe, unsigned int dev_mtu);
 
-int rxe_add(struct rxe_dev *rxe, unsigned int mtu);
+int rxe_add(struct rxe_dev *rxe, unsigned int mtu, const char *ibdev_name);
 void rxe_remove_all(void);
 
 void rxe_rcv(struct sk_buff *skb);
@@ -106,6 +106,7 @@  static inline void rxe_dev_put(struct rxe_dev *rxe)
 }
 struct rxe_dev *net_to_rxe(struct net_device *ndev);
 struct rxe_dev *get_rxe_by_name(const char *name);
+struct rxe_dev *ibdev_to_rxe(struct ib_device *device);
 
 void rxe_port_up(struct rxe_dev *rxe);
 void rxe_port_down(struct rxe_dev *rxe);
diff --git a/drivers/infiniband/sw/rxe/rxe_net.c b/drivers/infiniband/sw/rxe/rxe_net.c
index f1b559d728b6..7897bcb61394 100644
--- a/drivers/infiniband/sw/rxe/rxe_net.c
+++ b/drivers/infiniband/sw/rxe/rxe_net.c
@@ -83,6 +83,23 @@  struct rxe_dev *get_rxe_by_name(const char *name)
 	return found;
 }
 
+struct rxe_dev *ibdev_to_rxe(struct ib_device *device)
+{
+	struct rxe_dev *rxe;
+	struct rxe_dev *found = NULL;
+
+	spin_lock_bh(&dev_list_lock);
+	list_for_each_entry(rxe, &rxe_dev_list, list) {
+		if (&rxe->ib_dev == device) {
+			found = rxe;
+			kref_get(&rxe->ref_cnt);
+			break;
+		}
+	}
+	spin_unlock_bh(&dev_list_lock);
+
+	return found;
+}
 
 static struct rxe_recv_sockets recv_sockets;
 
@@ -552,7 +569,7 @@  enum rdma_link_layer rxe_link_layer(struct rxe_dev *rxe, unsigned int port_num)
 	return IB_LINK_LAYER_ETHERNET;
 }
 
-struct rxe_dev *rxe_net_add(struct net_device *ndev)
+struct rxe_dev *rxe_net_add(const char *ibdev_name, struct net_device *ndev)
 {
 	int err;
 	struct rxe_dev *rxe = NULL;
@@ -563,7 +580,7 @@  struct rxe_dev *rxe_net_add(struct net_device *ndev)
 
 	rxe->ndev = ndev;
 
-	err = rxe_add(rxe, ndev->mtu);
+	err = rxe_add(rxe, ndev->mtu, ibdev_name);
 	if (err) {
 		ib_dealloc_device(&rxe->ib_dev);
 		return NULL;
diff --git a/drivers/infiniband/sw/rxe/rxe_net.h b/drivers/infiniband/sw/rxe/rxe_net.h
index feb1d715468c..ced836f346ee 100644
--- a/drivers/infiniband/sw/rxe/rxe_net.h
+++ b/drivers/infiniband/sw/rxe/rxe_net.h
@@ -43,7 +43,7 @@  struct rxe_recv_sockets {
 	struct socket *sk6;
 };
 
-struct rxe_dev *rxe_net_add(struct net_device *ndev);
+struct rxe_dev *rxe_net_add(const char *ibdev_name, struct net_device *ndev);
 void rxe_net_remove(struct rxe_dev *rxe);
 
 int rxe_net_init(void);
diff --git a/drivers/infiniband/sw/rxe/rxe_sysfs.c b/drivers/infiniband/sw/rxe/rxe_sysfs.c
index fc52eb3d1856..1a3410647854 100644
--- a/drivers/infiniband/sw/rxe/rxe_sysfs.c
+++ b/drivers/infiniband/sw/rxe/rxe_sysfs.c
@@ -97,7 +97,7 @@  static int rxe_param_set_add(const char *val, const struct kernel_param *kp)
 		goto err;
 	}
 
-	rxe = rxe_net_add(ndev);
+	rxe = rxe_net_add("rxe%d", ndev);
 	if (!rxe) {
 		pr_err("failed to add %s\n", intf);
 		err = -EINVAL;
@@ -152,6 +152,6 @@  static int rxe_param_set_remove(const char *val, const struct kernel_param *kp)
 };
 
 module_param_cb(add, &rxe_add_ops, NULL, 0200);
-MODULE_PARM_DESC(add, "Create RXE device over network interface");
+MODULE_PARM_DESC(add, "DEPRECATED.  Create RXE device over network interface");
 module_param_cb(remove, &rxe_remove_ops, NULL, 0200);
-MODULE_PARM_DESC(remove, "Remove RXE device over network interface");
+MODULE_PARM_DESC(remove, "DEPRECATED.  Remove RXE device over network interface");
diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.c b/drivers/infiniband/sw/rxe/rxe_verbs.c
index 30817c79ba96..3536ca4627cd 100644
--- a/drivers/infiniband/sw/rxe/rxe_verbs.c
+++ b/drivers/infiniband/sw/rxe/rxe_verbs.c
@@ -1165,7 +1165,7 @@  static ssize_t parent_show(struct device *device,
 	.attrs = rxe_dev_attributes,
 };
 
-int rxe_register_device(struct rxe_dev *rxe)
+int rxe_register_device(struct rxe_dev *rxe, const char *ibdev_name)
 {
 	int err;
 	struct ib_device *dev = &rxe->ib_dev;
@@ -1273,7 +1273,7 @@  int rxe_register_device(struct rxe_dev *rxe)
 
 	rdma_set_device_sysfs_group(dev, &rxe_attr_group);
 	dev->driver_id = RDMA_DRIVER_RXE;
-	err = ib_register_device(dev, "rxe%d", NULL);
+	err = ib_register_device(dev, ibdev_name, NULL);
 	if (err) {
 		pr_warn("%s failed with error %d\n", __func__, err);
 		goto err1;
diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.h b/drivers/infiniband/sw/rxe/rxe_verbs.h
index 32c8ecb707d1..01e2e5604137 100644
--- a/drivers/infiniband/sw/rxe/rxe_verbs.h
+++ b/drivers/infiniband/sw/rxe/rxe_verbs.h
@@ -467,7 +467,7 @@  static inline struct rxe_mem *to_rmw(struct ib_mw *mw)
 	return mw ? container_of(mw, struct rxe_mem, ibmw) : NULL;
 }
 
-int rxe_register_device(struct rxe_dev *rxe);
+int rxe_register_device(struct rxe_dev *rxe, const char *ibdev_name);
 void rxe_unregister_device(struct rxe_dev *rxe);
 
 void rxe_mc_cleanup(struct rxe_pool_entry *arg);