rds: ib: Implement proper cm_id compare

RDS attempts to establish if two rdma_cm_ids actually are the
same. This was implemented by comparing their addresses. But, an
rdma_cm_id may be destroyed and allocated again. Thus, a *new* id, but
the address could still be the same as the *old* one.

Solved by using an idr id as the cm_id's context. Added helper
functions to create and destroy rdma_cm_ids and comparing the
rdma_cm_ids.

Orabug: 29391909

Signed-off-by: Håkon Bugge <haakon.bugge@oracle.com>
Tested-by: Rosa Lopez <rosa.lopez@oracle.com>
Reviewed-by: Hans Westgaard Ry <hans.westgaard.ry@oracle.com>
Signed-off-by: Brian Maly <brian.maly@oracle.com>

diff --git a/net/rds/ib.c b/net/rds/ib.c
index 7fccc6bac305e8f49a37866fa3a73e3ff5898784..04db8522c9239c11117da058ffbeffaaa824c5de 100644 (file)
--- a/net/rds/ib.c
+++ b/net/rds/ib.c
@@ -541,8 +541,8 @@ static int rds_ib_laddr_check(struct net *net, const struct in6_addr *addr,
        /* Create a CMA ID and try to bind it. This catches both
         * IB and iWARP capable NICs.
         */
-       cm_id = rdma_create_id(rds_rdma_cm_event_handler,
-                              NULL, RDMA_PS_TCP, IB_QPT_RC);
+       cm_id = rds_ib_rdma_create_id(rds_rdma_cm_event_handler,
+                                     NULL, RDMA_PS_TCP, IB_QPT_RC);
        if (IS_ERR(cm_id))
                return -EADDRNOTAVAIL;
 
@@ -594,7 +594,7 @@ static int rds_ib_laddr_check(struct net *net, const struct in6_addr *addr,
                 addr, scope_id, ret,
                 cm_id->device ? cm_id->device->node_type : -1);
 
-       rdma_destroy_id(cm_id);
+       rds_ib_rdma_destroy_id(cm_id);
 
        return ret;
 }

diff --git a/net/rds/ib.h b/net/rds/ib.h
index ebf890f123da76a55538cca47d0f88d741951713..919e3f0444f10b28cb5e7377ca1e28d6ec956359 100644 (file)
--- a/net/rds/ib.h
+++ b/net/rds/ib.h
@@ -577,6 +577,70 @@ struct rds_ib_statistics {
 
 extern struct workqueue_struct *rds_ib_wq;
 
+static inline struct rds_connection *rds_ib_map_conn(struct rds_connection *conn)
+{
+       int id;
+
+       mutex_lock(&cm_id_map_lock);
+       id = idr_alloc_cyclic(&cm_id_map, conn, 0, 0, GFP_KERNEL);
+       mutex_unlock(&cm_id_map_lock);
+
+       if (id < 0)
+               return ERR_PTR(id);
+
+       return (struct rds_connection *)(unsigned long)id;
+}
+
+static inline struct rdma_cm_id *rds_ib_rdma_create_id(rdma_cm_event_handler event_handler,
+                                                      void *context, enum rdma_port_space ps,
+                                                      enum ib_qp_type qp_type)
+{
+       return rdma_create_id(event_handler,
+                             rds_ib_map_conn(context), ps, qp_type);
+}
+
+static inline void rds_ib_rdma_destroy_id(struct rdma_cm_id *cm_id)
+{
+       mutex_lock(&cm_id_map_lock);
+       (void)idr_remove(&cm_id_map, (int)(u64)cm_id->context);
+       mutex_unlock(&cm_id_map_lock);
+       rdma_destroy_id(cm_id);
+}
+
+static inline struct rds_connection *rds_ib_get_conn(struct rdma_cm_id *cm_id)
+{
+       struct rds_connection *conn;
+
+       mutex_lock(&cm_id_map_lock);
+       conn = idr_find(&cm_id_map, (unsigned long)cm_id->context);
+       mutex_unlock(&cm_id_map_lock);
+
+       return conn;
+}
+
+static inline bool rds_ib_same_cm_id(struct rds_ib_connection *ic, struct rdma_cm_id *cm_id)
+{
+       if (ic) {
+               if (ic->i_cm_id != cm_id) {
+                       rds_rtd_ptr(RDS_RTD_CM_EXT,
+                                   "conn %p ic->cm_id %p NE cm_id %p\n",
+                                   ic->conn, ic->i_cm_id, cm_id);
+                       return false;
+               }
+               if (ic->i_cm_id->context != cm_id->context) {
+                       rds_rtd_ptr(RDS_RTD_CM_EXT,
+                                   "conn %p ic->cm_id %p cm_id %p ctx1 %p NE ctx2 %p\n",
+                                   ic->conn, ic->i_cm_id, cm_id,
+                                   ic->i_cm_id->context, cm_id->context);
+                       return false;
+               }
+
+               return true;
+       }
+
+       return false;
+}
+
 /*
  * Fake ib_dma_sync_sg_for_{cpu,device} as long as ib_verbs.h
  * doesn't define it.

diff --git a/net/rds/ib_cm.c b/net/rds/ib_cm.c
index 4171249a288cfbaffdc5765828b4692528bd451a..1e6deedc89d11d3f228363b7307805cd4e0233ac 100644 (file)
--- a/net/rds/ib_cm.c
+++ b/net/rds/ib_cm.c
@@ -1092,7 +1092,7 @@ int rds_ib_cm_handle_connect(struct rdma_cm_id *cm_id,
                 * in both of the cases below, the conn is half setup.
                 * we need to make sure the lower layers don't destroy it
                 */
-               if (ic && ic->i_cm_id == cm_id)
+               if (rds_ib_same_cm_id(ic, cm_id))
                        destroy = 0;
                if (rds_conn_state(conn) == RDS_CONN_UP) {
                        rds_rtd(RDS_RTD_CM_EXT_P,
@@ -1164,11 +1164,11 @@ int rds_ib_cm_handle_connect(struct rdma_cm_id *cm_id,
                rds_send_drop_acked(conn, be64_to_cpu(dp_cmn->ricpc_ack_seq),
                                    NULL);
 
-       BUG_ON(cm_id->context);
+       BUG_ON(rds_ib_get_conn(cm_id));
        BUG_ON(ic->i_cm_id);
 
        ic->i_cm_id = cm_id;
-       cm_id->context = conn;
+       cm_id->context = rds_ib_map_conn(conn);
 
        /* We got halfway through setting up the ib_connection, if we
         * fail now, we have to take the long route out of this mess. */
@@ -1232,7 +1232,7 @@ void rds_ib_conn_destroy_init(struct rds_connection *conn)
 
 int rds_ib_cm_initiate_connect(struct rdma_cm_id *cm_id, bool isv6)
 {
-       struct rds_connection *conn = cm_id->context;
+       struct rds_connection *conn = rds_ib_get_conn(cm_id);
        struct rds_ib_connection *ic = conn->c_transport_data;
        struct rdma_conn_param conn_param;
        union rds_ib_conn_priv dp;
@@ -1291,7 +1291,7 @@ out:
         * the cm_id. We should certainly not do it as long as we still
         * "own" the cm_id. */
        if (ret) {
-               if (ic->i_cm_id == cm_id)
+               if (rds_ib_same_cm_id(ic, cm_id))
                        ret = 0;
        }
 
@@ -1318,12 +1318,12 @@ int rds_ib_conn_path_connect(struct rds_conn_path *cp)
                handler = rds6_rdma_cm_event_handler;
        else
                handler = rds_rdma_cm_event_handler;
-       ic->i_cm_id = rdma_create_id(handler, conn, RDMA_PS_TCP, IB_QPT_RC);
+       ic->i_cm_id = rds_ib_rdma_create_id(handler, conn, RDMA_PS_TCP, IB_QPT_RC);
 
        if (IS_ERR(ic->i_cm_id)) {
                ret = PTR_ERR(ic->i_cm_id);
                ic->i_cm_id = NULL;
-               rds_rtd(RDS_RTD_ERR, "rdma_create_id() failed: %d\n", ret);
+               rds_rtd(RDS_RTD_ERR, "rds_ib_rdma_create_id() failed: %d\n", ret);
                goto out;
        }
 
@@ -1366,7 +1366,7 @@ int rds_ib_conn_path_connect(struct rds_conn_path *cp)
        if (ret) {
                rds_rtd(RDS_RTD_ERR, "addr resolve failed for cm id %p: %d\n",
                        ic->i_cm_id, ret);
-               rdma_destroy_id(ic->i_cm_id);
+               rds_ib_rdma_destroy_id(ic->i_cm_id);
 
                ic->i_cm_id = NULL;
        }
@@ -1476,7 +1476,7 @@ void rds_ib_conn_path_shutdown(struct rds_conn_path *cp)
                if (ic->i_recvs)
                        rds_ib_recv_clear_ring(ic);
 
-               rdma_destroy_id(ic->i_cm_id);
+               rds_ib_rdma_destroy_id(ic->i_cm_id);
 
                /*
                 * Move connection back to the nodev list.

diff --git a/net/rds/ib_send.c b/net/rds/ib_send.c
index 5d1a127de6cf4cbf4ff93962e4125369120f72a5..af3062e35da643fc70c079d1a82c2cabb9c67e3f 100644 (file)
--- a/net/rds/ib_send.c
+++ b/net/rds/ib_send.c
@@ -444,7 +444,7 @@ try_again:
                avail--;
 
        if (avail < wanted) {
-               struct rds_connection *conn = ic->i_cm_id->context;
+               struct rds_connection *conn = rds_ib_get_conn(ic->i_cm_id);
 
                /* Oops, there aren't that many credits left! */
                set_bit(RDS_LL_SEND_FULL, &conn->c_flags);

diff --git a/net/rds/rdma_transport.c b/net/rds/rdma_transport.c
index a58d9d61ec7977423610d916daefec2b75e70d7f..568a8590d27d1e6f1486d57c8632be2d1a1202da 100644 (file)
--- a/net/rds/rdma_transport.c
+++ b/net/rds/rdma_transport.c
@@ -43,6 +43,8 @@
 
 #define RDS_REJ_CONSUMER_DEFINED 28
 
+struct mutex cm_id_map_lock;
+DEFINE_IDR(cm_id_map);
 /* Global IPv4 and IPv6 RDS RDMA listener cm_id */
 static struct rdma_cm_id *rds_rdma_listen_id;
 static struct rdma_cm_id *rds6_rdma_listen_id;
@@ -87,11 +89,12 @@ int rds_rdma_cm_event_handler_cmn(struct rdma_cm_id *cm_id,
                                  bool isv6)
 {
        /* this can be null in the listening path */
-       struct rds_connection *conn = cm_id->context;
-       struct rds_transport *trans;
+       struct rds_connection *conn;
+       struct rds_transport *trans = &rds_ib_transport;
        int ret = 0;
        int *err;
 
+       conn = rds_ib_get_conn(cm_id);
        if (conn)
                rds_rtd_ptr(RDS_RTD_CM,
                            "conn %p state %s cm_id %p <%pI6c,%pI6c,%d> handling event %u (%s) priv_dta_len %d\n",
@@ -166,7 +169,7 @@ int rds_rdma_cm_event_handler_cmn(struct rdma_cm_id *cm_id,
                                            "conn %p <%pI6c,%pI6c,%d> dropping connection after rdma_resolve_route failure %d\n",
                                            conn, &conn->c_laddr, &conn->c_faddr, conn->c_tos, ret);
                                ibic = conn->c_transport_data;
-                               if (ibic && ibic->i_cm_id == cm_id)
+                               if (rds_ib_same_cm_id(ibic, cm_id))
                                        ibic->i_cm_id = NULL;
                                rds_conn_drop(conn, DR_IB_RESOLVE_ROUTE_FAIL);
                        }
@@ -184,7 +187,7 @@ int rds_rdma_cm_event_handler_cmn(struct rdma_cm_id *cm_id,
                        struct rds_ib_connection *ibic;
 
                        ibic = conn->c_transport_data;
-                       if (ibic && ibic->i_cm_id == cm_id) {
+                       if (rds_ib_same_cm_id(ibic, cm_id)) {
                                /* ibacm caches the path record without considering the tos/sl.
                                 * It is considered a match if the <src,dest> matches the
                                 * cache. In order to create qp with the correct sl/vl, RDS
@@ -375,10 +378,10 @@ static int rds_rdma_listen_init_common(rdma_cm_event_handler handler,
        struct rdma_cm_id *cm_id;
        int ret;
 
-       cm_id = rdma_create_id(handler, NULL, RDMA_PS_TCP, IB_QPT_RC);
+       cm_id = rds_ib_rdma_create_id(handler, NULL, RDMA_PS_TCP, IB_QPT_RC);
        if (IS_ERR(cm_id)) {
                ret = PTR_ERR(cm_id);
-               printk(KERN_ERR "RDS/RDMA: failed to setup listener, rdma_create_id() returned %d\n",
+               printk(KERN_ERR "RDS/RDMA: failed to setup listener, rds_ib_rdma_create_id() returned %d\n",
                       ret);
                return ret;
        }
@@ -409,7 +412,7 @@ static int rds_rdma_listen_init_common(rdma_cm_event_handler handler,
        cm_id = NULL;
 out:
        if (cm_id)
-               rdma_destroy_id(cm_id);
+               rds_ib_rdma_destroy_id(cm_id);
        return ret;
 }
 
@@ -452,12 +455,12 @@ static void rds_rdma_listen_stop(void)
 {
        if (rds_rdma_listen_id) {
                rdsdebug("cm %p\n", rds_rdma_listen_id);
-               rdma_destroy_id(rds_rdma_listen_id);
+               rds_ib_rdma_destroy_id(rds_rdma_listen_id);
                rds_rdma_listen_id = NULL;
        }
        if (rds6_rdma_listen_id) {
                rdsdebug("cm %p\n", rds6_rdma_listen_id);
-               rdma_destroy_id(rds6_rdma_listen_id);
+               rds_ib_rdma_destroy_id(rds6_rdma_listen_id);
                rds6_rdma_listen_id = NULL;
        }
 }
@@ -468,6 +471,8 @@ int __init rds_rdma_init(void)
 {
        int ret;
 
+       mutex_init(&cm_id_map_lock);
+
        ret = rds_ib_init();
        if (ret)
                goto out;

diff --git a/net/rds/rdma_transport.h b/net/rds/rdma_transport.h
index 8b95f1d446818763d75cca72ae4071b6e39a6fd9..728f1937b74c5dff81c7b2c0030be3aabe47bf9a 100644 (file)
--- a/net/rds/rdma_transport.h
+++ b/net/rds/rdma_transport.h
@@ -7,6 +7,9 @@
 
 #define RDS_RDMA_RESOLVE_TIMEOUT_MS     RDS_RECONNECT_RETRY_MS
 
+extern struct mutex            cm_id_map_lock;
+extern struct idr              cm_id_map;
+
 /* Per IB specification 7.7.3, service level is a 4-bit field. */
 #define TOS_TO_SL(tos) ((tos) & 0xF)