RDMA/netlink: Remove netlink clients infrastructure
authorLeon Romanovsky <leonro@mellanox.com>
Mon, 5 Jun 2017 07:20:11 +0000 (10:20 +0300)
committerLeon Romanovsky <leon@kernel.org>
Thu, 10 Aug 2017 10:13:06 +0000 (13:13 +0300)
RDMA netlink has a complicated infrastructure for dynamically
registering and de-registering netlink clients to the NETLINK_RDMA
group. The complicated portion of this code is not widely used because
2 of the 3 current clients are statically compiled together with
netlink.c. The infrastructure, therefore, is deemed overkill.

Refactor the code to eliminate the dynamically added clients. Now all
clients are pre-registered in a client array at compile time, and at run
time they merely check-in with the infrastructure to pass their callback
table for inclusion in the pre-sized client array.

This also allows for future cleanups and removal of unneeded code in the
iwcm* netlink handler.

Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
Reviewed-by: Chien Tin Tung <chien.tin.tung@intel.com>
drivers/infiniband/core/cma.c
drivers/infiniband/core/core_priv.h
drivers/infiniband/core/device.c
drivers/infiniband/core/iwcm.c
drivers/infiniband/core/netlink.c
include/rdma/rdma_netlink.h

index ca4135c596baa5477516275935ba41dcc2dc454a..2a16a559bdda1cf8caf0bc7967962841deba94c9 100644 (file)
@@ -4512,9 +4512,7 @@ static int __init cma_init(void)
        if (ret)
                goto err;
 
-       if (ibnl_add_client(RDMA_NL_RDMA_CM, ARRAY_SIZE(cma_cb_table),
-                           cma_cb_table))
-               pr_warn("RDMA CMA: failed to add netlink callback\n");
+       rdma_nl_register(RDMA_NL_RDMA_CM, cma_cb_table);
        cma_configfs_init();
 
        return 0;
@@ -4531,7 +4529,7 @@ err_wq:
 static void __exit cma_cleanup(void)
 {
        cma_configfs_exit();
-       ibnl_remove_client(RDMA_NL_RDMA_CM);
+       rdma_nl_unregister(RDMA_NL_RDMA_CM);
        ib_unregister_client(&cma_client);
        unregister_netdevice_notifier(&cma_nb);
        rdma_addr_unregister_client(&addr_client);
index 11ae67514e13946a26c54d8da8c4d8078aa59596..e759c27113cdc16ff3fae0ef2d895dcc6e6c502d 100644 (file)
@@ -179,8 +179,8 @@ void ib_mad_cleanup(void);
 int ib_sa_init(void);
 void ib_sa_cleanup(void);
 
-int ibnl_init(void);
-void ibnl_cleanup(void);
+int rdma_nl_init(void);
+void rdma_nl_exit(void);
 
 /**
  * Check if there are any listeners to the netlink group
index a5dfab6adf495b88e846ee03b815536a7917bf48..d0994cd30eae4eca86215b3e227bd0611b1eb513 100644 (file)
@@ -1086,29 +1086,15 @@ struct net_device *ib_get_net_dev_by_params(struct ib_device *dev,
 }
 EXPORT_SYMBOL(ib_get_net_dev_by_params);
 
-static struct ibnl_client_cbs ibnl_ls_cb_table[] = {
+static const struct ibnl_client_cbs ibnl_ls_cb_table[] = {
        [RDMA_NL_LS_OP_RESOLVE] = {
-               .dump = ib_nl_handle_resolve_resp,
-               .module = THIS_MODULE },
+               .dump = ib_nl_handle_resolve_resp},
        [RDMA_NL_LS_OP_SET_TIMEOUT] = {
-               .dump = ib_nl_handle_set_timeout,
-               .module = THIS_MODULE },
+               .dump = ib_nl_handle_set_timeout},
        [RDMA_NL_LS_OP_IP_RESOLVE] = {
-               .dump = ib_nl_handle_ip_res_resp,
-               .module = THIS_MODULE },
+               .dump = ib_nl_handle_ip_res_resp},
 };
 
-static int ib_add_ibnl_clients(void)
-{
-       return ibnl_add_client(RDMA_NL_LS, ARRAY_SIZE(ibnl_ls_cb_table),
-                              ibnl_ls_cb_table);
-}
-
-static void ib_remove_ibnl_clients(void)
-{
-       ibnl_remove_client(RDMA_NL_LS);
-}
-
 static int __init ib_core_init(void)
 {
        int ret;
@@ -1130,9 +1116,9 @@ static int __init ib_core_init(void)
                goto err_comp;
        }
 
-       ret = ibnl_init();
+       ret = rdma_nl_init();
        if (ret) {
-               pr_warn("Couldn't init IB netlink interface\n");
+               pr_warn("Couldn't init IB netlink interface: err %d\n", ret);
                goto err_sysfs;
        }
 
@@ -1154,24 +1140,17 @@ static int __init ib_core_init(void)
                goto err_mad;
        }
 
-       ret = ib_add_ibnl_clients();
-       if (ret) {
-               pr_warn("Couldn't register ibnl clients\n");
-               goto err_sa;
-       }
-
        ret = register_lsm_notifier(&ibdev_lsm_nb);
        if (ret) {
                pr_warn("Couldn't register LSM notifier. ret %d\n", ret);
-               goto err_ibnl_clients;
+               goto err_sa;
        }
 
+       rdma_nl_register(RDMA_NL_LS, ibnl_ls_cb_table);
        ib_cache_setup();
 
        return 0;
 
-err_ibnl_clients:
-       ib_remove_ibnl_clients();
 err_sa:
        ib_sa_cleanup();
 err_mad:
@@ -1179,7 +1158,7 @@ err_mad:
 err_addr:
        addr_cleanup();
 err_ibnl:
-       ibnl_cleanup();
+       rdma_nl_exit();
 err_sysfs:
        class_unregister(&ib_class);
 err_comp:
@@ -1191,13 +1170,13 @@ err:
 
 static void __exit ib_core_cleanup(void)
 {
-       unregister_lsm_notifier(&ibdev_lsm_nb);
        ib_cache_cleanup();
-       ib_remove_ibnl_clients();
+       rdma_nl_unregister(RDMA_NL_LS);
+       unregister_lsm_notifier(&ibdev_lsm_nb);
        ib_sa_cleanup();
        ib_mad_cleanup();
        addr_cleanup();
-       ibnl_cleanup();
+       rdma_nl_exit();
        class_unregister(&ib_class);
        destroy_workqueue(ib_comp_wq);
        /* Make sure that any pending umem accounting work is done. */
index 31661b5c1743649b47ba6b398c16b327e53a64ed..8599271d8be67629264aea199ac6c99bf1f040d2 100644 (file)
@@ -1175,12 +1175,8 @@ static int __init iw_cm_init(void)
        ret = iwpm_init(RDMA_NL_IWCM);
        if (ret)
                pr_err("iw_cm: couldn't init iwpm\n");
-
-       ret = ibnl_add_client(RDMA_NL_IWCM, ARRAY_SIZE(iwcm_nl_cb_table),
-                             iwcm_nl_cb_table);
-       if (ret)
-               pr_err("iw_cm: couldn't register netlink callbacks\n");
-
+       else
+               rdma_nl_register(RDMA_NL_IWCM, iwcm_nl_cb_table);
        iwcm_wq = alloc_ordered_workqueue("iw_cm_wq", WQ_MEM_RECLAIM);
        if (!iwcm_wq)
                return -ENOMEM;
@@ -1200,7 +1196,7 @@ static void __exit iw_cm_cleanup(void)
 {
        unregister_net_sysctl_table(iwcm_ctl_table_hdr);
        destroy_workqueue(iwcm_wq);
-       ibnl_remove_client(RDMA_NL_IWCM);
+       rdma_nl_unregister(RDMA_NL_IWCM);
        iwpm_exit(RDMA_NL_IWCM);
 }
 
index 0fc50e15ae22523f024af16de3d18eb58c35ffa8..06f7ba31fbdd09dff34a011168e304c6c6eec735 100644 (file)
 #include <rdma/rdma_netlink.h>
 #include "core_priv.h"
 
-struct ibnl_client {
-       struct list_head                list;
-       int                             index;
-       int                             nops;
-       const struct ibnl_client_cbs   *cb_table;
-};
+#include "core_priv.h"
 
-static DEFINE_MUTEX(ibnl_mutex);
+static DEFINE_MUTEX(rdma_nl_mutex);
 static struct sock *nls;
-static LIST_HEAD(client_list);
+static struct {
+       const struct ibnl_client_cbs   *cb_table;
+} rdma_nl_types[RDMA_NL_NUM_CLIENTS];
 
 int ibnl_chk_listeners(unsigned int group)
 {
@@ -57,58 +54,74 @@ int ibnl_chk_listeners(unsigned int group)
        return 0;
 }
 
-int ibnl_add_client(int index, int nops,
-                   const struct ibnl_client_cbs cb_table[])
+static bool is_nl_msg_valid(unsigned int type, unsigned int op)
 {
-       struct ibnl_client *cur;
-       struct ibnl_client *nl_client;
+       static const unsigned int max_num_ops[RDMA_NL_NUM_CLIENTS - 1] = {
+                                 RDMA_NL_RDMA_CM_NUM_OPS,
+                                 RDMA_NL_IWPM_NUM_OPS,
+                                 0,
+                                 RDMA_NL_LS_NUM_OPS,
+                                 0 };
 
-       nl_client = kmalloc(sizeof *nl_client, GFP_KERNEL);
-       if (!nl_client)
-               return -ENOMEM;
+       /*
+        * This BUILD_BUG_ON is intended to catch addition of new
+        * RDMA netlink protocol without updating the array above.
+        */
+       BUILD_BUG_ON(RDMA_NL_NUM_CLIENTS != 6);
 
-       nl_client->index        = index;
-       nl_client->nops         = nops;
-       nl_client->cb_table     = cb_table;
+       if (type > RDMA_NL_NUM_CLIENTS - 1)
+               return false;
 
-       mutex_lock(&ibnl_mutex);
+       return (op < max_num_ops[type - 1]) ? true : false;
+}
 
-       list_for_each_entry(cur, &client_list, list) {
-               if (cur->index == index) {
-                       pr_warn("Client for %d already exists\n", index);
-                       mutex_unlock(&ibnl_mutex);
-                       kfree(nl_client);
-                       return -EINVAL;
-               }
-       }
+static bool is_nl_valid(unsigned int type, unsigned int op)
+{
+       if (!is_nl_msg_valid(type, op) ||
+           !rdma_nl_types[type].cb_table ||
+           !rdma_nl_types[type].cb_table[op].dump)
+               return false;
+       return true;
+}
 
-       list_add_tail(&nl_client->list, &client_list);
+void rdma_nl_register(unsigned int index,
+                     const struct ibnl_client_cbs cb_table[])
+{
+       mutex_lock(&rdma_nl_mutex);
+       if (!is_nl_msg_valid(index, 0)) {
+               /*
+                * All clients are not interesting in success/failure of
+                * this call. They want to see the print to error log and
+                * continue their initialization. Print warning for them,
+                * because it is programmer's error to be here.
+                */
+               mutex_unlock(&rdma_nl_mutex);
+               WARN(true,
+                    "The not-valid %u index was supplied to RDMA netlink\n",
+                    index);
+               return;
+       }
 
-       mutex_unlock(&ibnl_mutex);
+       if (rdma_nl_types[index].cb_table) {
+               mutex_unlock(&rdma_nl_mutex);
+               WARN(true,
+                    "The %u index is already registered in RDMA netlink\n",
+                    index);
+               return;
+       }
 
-       return 0;
+       rdma_nl_types[index].cb_table = cb_table;
+       mutex_unlock(&rdma_nl_mutex);
 }
-EXPORT_SYMBOL(ibnl_add_client);
+EXPORT_SYMBOL(rdma_nl_register);
 
-int ibnl_remove_client(int index)
+void rdma_nl_unregister(unsigned int index)
 {
-       struct ibnl_client *cur, *next;
-
-       mutex_lock(&ibnl_mutex);
-       list_for_each_entry_safe(cur, next, &client_list, list) {
-               if (cur->index == index) {
-                       list_del(&(cur->list));
-                       mutex_unlock(&ibnl_mutex);
-                       kfree(cur);
-                       return 0;
-               }
-       }
-       pr_warn("Can't remove callback for client idx %d. Not found\n", index);
-       mutex_unlock(&ibnl_mutex);
-
-       return -EINVAL;
+       mutex_lock(&rdma_nl_mutex);
+       rdma_nl_types[index].cb_table = NULL;
+       mutex_unlock(&rdma_nl_mutex);
 }
-EXPORT_SYMBOL(ibnl_remove_client);
+EXPORT_SYMBOL(rdma_nl_unregister);
 
 void *ibnl_put_msg(struct sk_buff *skb, struct nlmsghdr **nlh, int seq,
                   int len, int client, int op, int flags)
@@ -149,45 +162,31 @@ EXPORT_SYMBOL(ibnl_put_attr);
 static int ibnl_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh,
                        struct netlink_ext_ack *extack)
 {
-       struct ibnl_client *client;
        int type = nlh->nlmsg_type;
-       int index = RDMA_NL_GET_CLIENT(type);
+       unsigned int index = RDMA_NL_GET_CLIENT(type);
        unsigned int op = RDMA_NL_GET_OP(type);
+       struct netlink_callback cb = {};
+       struct netlink_dump_control c = {};
 
-       list_for_each_entry(client, &client_list, list) {
-               if (client->index == index) {
-                       if (op >= client->nops || !client->cb_table[op].dump)
-                               return -EINVAL;
-
-                       /*
-                        * For response or local service set_timeout request,
-                        * there is no need to use netlink_dump_start.
-                        */
-                       if (!(nlh->nlmsg_flags & NLM_F_REQUEST) ||
-                           (index == RDMA_NL_LS &&
-                            op == RDMA_NL_LS_OP_SET_TIMEOUT)) {
-                               struct netlink_callback cb = {
-                                       .skb = skb,
-                                       .nlh = nlh,
-                                       .dump = client->cb_table[op].dump,
-                                       .module = client->cb_table[op].module,
-                               };
-
-                               return cb.dump(skb, &cb);
-                       }
-
-                       {
-                               struct netlink_dump_control c = {
-                                       .dump = client->cb_table[op].dump,
-                                       .module = client->cb_table[op].module,
-                               };
-                               return netlink_dump_start(nls, skb, nlh, &c);
-                       }
-               }
+       if (!is_nl_valid(index, op))
+               return -EINVAL;
+
+       /*
+        * For response or local service set_timeout request,
+        * there is no need to use netlink_dump_start.
+        */
+       if (!(nlh->nlmsg_flags & NLM_F_REQUEST) ||
+           (index == RDMA_NL_LS && op == RDMA_NL_LS_OP_SET_TIMEOUT)) {
+               cb.skb = skb;
+               cb.nlh = nlh;
+               cb.dump = rdma_nl_types[index].cb_table[op].dump;
+               cb.module = rdma_nl_types[index].cb_table[op].module;
+               return cb.dump(skb, &cb);
        }
 
-       pr_info("Index %d wasn't found in client list\n", index);
-       return -EINVAL;
+       c.dump = rdma_nl_types[index].cb_table[op].dump;
+       c.module = rdma_nl_types[index].cb_table[op].module;
+       return netlink_dump_start(nls, skb, nlh, &c);
 }
 
 static void ibnl_rcv_reply_skb(struct sk_buff *skb)
@@ -221,10 +220,10 @@ static void ibnl_rcv_reply_skb(struct sk_buff *skb)
 
 static void ibnl_rcv(struct sk_buff *skb)
 {
-       mutex_lock(&ibnl_mutex);
+       mutex_lock(&rdma_nl_mutex);
        ibnl_rcv_reply_skb(skb);
        netlink_rcv_skb(skb, &ibnl_rcv_msg);
-       mutex_unlock(&ibnl_mutex);
+       mutex_unlock(&rdma_nl_mutex);
 }
 
 int ibnl_unicast(struct sk_buff *skb, struct nlmsghdr *nlh,
@@ -254,32 +253,26 @@ int ibnl_multicast(struct sk_buff *skb, struct nlmsghdr *nlh,
 }
 EXPORT_SYMBOL(ibnl_multicast);
 
-int __init ibnl_init(void)
+int __init rdma_nl_init(void)
 {
        struct netlink_kernel_cfg cfg = {
                .input  = ibnl_rcv,
        };
 
        nls = netlink_kernel_create(&init_net, NETLINK_RDMA, &cfg);
-       if (!nls) {
-               pr_warn("Failed to create netlink socket\n");
+       if (!nls)
                return -ENOMEM;
-       }
 
        nls->sk_sndtimeo = 10 * HZ;
        return 0;
 }
 
-void ibnl_cleanup(void)
+void rdma_nl_exit(void)
 {
-       struct ibnl_client *cur, *next;
+       int idx;
 
-       mutex_lock(&ibnl_mutex);
-       list_for_each_entry_safe(cur, next, &client_list, list) {
-               list_del(&(cur->list));
-               kfree(cur);
-       }
-       mutex_unlock(&ibnl_mutex);
+       for (idx = 0; idx < RDMA_NL_NUM_CLIENTS; idx++)
+               rdma_nl_unregister(idx);
 
        netlink_kernel_release(nls);
 }
index 5b1466770917a22d5887a6f0b2cbe7bdce16027f..aadf0ab963b2036c81d8607a2cd2b811b1200a08 100644 (file)
@@ -11,23 +11,18 @@ struct ibnl_client_cbs {
 };
 
 /**
- * Add a a client to the list of IB netlink exporters.
+ * Register client in RDMA netlink.
  * @index: Index of the added client
- * @nops: Number of supported ops by the added client.
  * @cb_table: A table for op->callback
- *
- * Returns 0 on success or a negative error code.
  */
-int ibnl_add_client(int index, int nops,
-                   const struct ibnl_client_cbs cb_table[]);
+void rdma_nl_register(unsigned int index,
+                     const struct ibnl_client_cbs cb_table[]);
 
 /**
  * Remove a client from IB netlink.
  * @index: Index of the removed IB client.
- *
- * Returns 0 on success or a negative error code.
  */
-int ibnl_remove_client(int index);
+void rdma_nl_unregister(unsigned int index);
 
 /**
  * Put a new message in a supplied skb.