From: Håkon Bugge Date: Fri, 5 Apr 2019 13:09:46 +0000 (+0200) Subject: rds: ib: Implement proper cm_id compare X-Git-Tag: v4.1.12-124.31.3~110 X-Git-Url: https://www.infradead.org/git/?a=commitdiff_plain;h=0cdb52d4b2c681db3a4b6caf7a4a5ebe5ace38c7;p=users%2Fjedix%2Flinux-maple.git 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 Tested-by: Rosa Lopez Reviewed-by: Hans Westgaard Ry Signed-off-by: Brian Maly --- diff --git a/net/rds/ib.c b/net/rds/ib.c index 7fccc6bac305e..04db8522c9239 100644 --- 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 ebf890f123da7..919e3f0444f10 100644 --- 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 4171249a288cf..1e6deedc89d11 100644 --- 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 5d1a127de6cf4..af3062e35da64 100644 --- 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 a58d9d61ec797..568a8590d27d1 100644 --- 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 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 8b95f1d446818..728f1937b74c5 100644 --- 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)