From a0c9ca3b0624453e337e863b83d3c9826e502c38 Mon Sep 17 00:00:00 2001 From: Arlin Davis Date: Thu, 17 Mar 2016 16:06:35 -0700 Subject: [PATCH] scm: backward compatibility issue with MTU negotiation The scm provider builds the CM reply message on stack and doesnt memset to zero so resv fields are unknown. The client cannot check mtu/resv field for MTU adjustments. Bump provider CM message version to DCM_VER_MTU and add check for appropriate version. Signed-off-by: Arlin Davis --- dapl/openib_common/dapl_ib_common.h | 6 ++++- dapl/openib_scm/cm.c | 40 +++++++++++++++++------------ 2 files changed, 29 insertions(+), 17 deletions(-) diff --git a/dapl/openib_common/dapl_ib_common.h b/dapl/openib_common/dapl_ib_common.h index 8ff898f..a2ff41c 100644 --- a/dapl/openib_common/dapl_ib_common.h +++ b/dapl/openib_common/dapl_ib_common.h @@ -105,9 +105,13 @@ typedef struct ibv_context *ib_hca_handle_t; typedef ib_hca_handle_t dapl_ibal_ca_t; /* QP info to exchange, wire protocol version for these CM's */ +/* Version 7, RDMA READ attributes */ /* Version 8, 24-bit port space and rtns value */ -#define DCM_VER 8 +/* Version 9, MTU negotiation per QP */ +#define DCM_VER 9 +#define DCM_VER_MTU 9 #define DCM_VER_XPS 8 /* extended port space, rtns */ +#define DCM_VER_RR 7 /* RDMA Read attributes */ #define DCM_VER_MIN 6 /* backward compatibility limit */ /* CM private data areas, same for all operations */ diff --git a/dapl/openib_scm/cm.c b/dapl/openib_scm/cm.c index 35164ef..a23c421 100644 --- a/dapl/openib_scm/cm.c +++ b/dapl/openib_scm/cm.c @@ -592,12 +592,14 @@ static void dapli_socket_connected(dp_ib_cm_handle_t cm_ptr, int err) dapl_log(DAPL_DBG_TYPE_CM, " CONN_REQ: (%d) SRC lid=0x%x," - " qpn=0x%x, psize=%d GID %s\n", + " qpn=0x%x, psize=%d GID %s rd_in %d rtns %d mtu %d rsv %d\n", exp, ntohs(cm_ptr->msg.saddr.ib.lid), ntohl(cm_ptr->msg.saddr.ib.qpn), ntohs(cm_ptr->msg.p_size), inet_ntop(AF_INET6, &cm_ptr->hca->ib_trans.gid, - gid_str, sizeof(gid_str))); + gid_str, sizeof(gid_str)), + cm_ptr->msg.rd_in, cm_ptr->msg.rtns, + cm_ptr->msg.mtu, cm_ptr->msg.resv[0]); DAPL_CNTR(((DAPL_IA *)dapl_llist_peek_head(&cm_ptr->hca->ia_list_head)), DCNT_IA_CM_REQ_TX); return; @@ -743,8 +745,10 @@ static void dapli_socket_connect_rtu(dp_ib_cm_handle_t cm_ptr) exp += SCM_BC_DIFF; } - dapl_log(DAPL_DBG_TYPE_CM, " CONN_REP_IN: ver %d cm_sz %d, p_sz %d\n", - ntohs(cm_ptr->msg.ver), exp, ntohs(cm_ptr->msg.p_size)); + dapl_log(DAPL_DBG_TYPE_CM, + " CONN_REP_IN: ver %d cm_sz %d, p_sz %d rd_in %d mtu %d rsv %d\n", + ntohs(cm_ptr->msg.ver), exp, ntohs(cm_ptr->msg.p_size), + cm_ptr->msg.rd_in, cm_ptr->msg.mtu, cm_ptr->msg.resv[0]); if (len != exp || ntohs(cm_ptr->msg.ver) < DCM_VER_MIN) { int err = dapl_socket_errno(); @@ -844,13 +848,13 @@ static void dapli_socket_connect_rtu(dp_ib_cm_handle_t cm_ptr) } /* rdma_out, initiator, cannot exceed remote rdma_in max */ - if (ntohs(cm_ptr->msg.ver) >= 7) + if (ntohs(cm_ptr->msg.ver) >= DCM_VER_RR) ep_ptr->param.ep_attr.max_rdma_read_out = DAPL_MIN(ep_ptr->param.ep_attr.max_rdma_read_out, cm_ptr->msg.rd_in); /* Set QP MTU, if negotiated. 2K for compatibility */ - ep_ptr->qp_handle->mtu = cm_ptr->msg.mtu ? + ep_ptr->qp_handle->mtu = (ntohs(cm_ptr->msg.ver) >= DCM_VER_MTU) ? DAPL_MIN(cm_ptr->msg.mtu, cm_ptr->hca->ib_trans.ib_cm.mtu): getenv("DAPL_IB_MTU") ? cm_ptr->hca->ib_trans.ib_cm.mtu : IBV_MTU_2048; @@ -1186,14 +1190,17 @@ static void dapli_socket_accept_data(ib_cm_srvc_handle_t acm_ptr) DAPL_CNTR(((DAPL_IA *)dapl_llist_peek_head(&acm_ptr->hca->ia_list_head)), DCNT_IA_CM_REQ_RX); - dapl_dbg_log(DAPL_DBG_TYPE_CM, - " ACCEPT: DST %s %x lid=0x%x, qpn=0x%x, psz=%d\n", - inet_ntoa(((struct sockaddr_in *) - &acm_ptr->msg.daddr.so)->sin_addr), - ntohs(((struct sockaddr_in *) - &acm_ptr->msg.daddr.so)->sin_port), - ntohs(acm_ptr->msg.saddr.ib.lid), - ntohl(acm_ptr->msg.saddr.ib.qpn), exp); + dapl_log(DAPL_DBG_TYPE_CM, + " ACCEPT: DST %s %x lid=0x%x, qpn=0x%x, psz=%d" + " rd_in %d mtu %d rsv %d \n", + inet_ntoa(((struct sockaddr_in *) + &acm_ptr->msg.daddr.so)->sin_addr), + ntohs(((struct sockaddr_in *) + &acm_ptr->msg.daddr.so)->sin_port), + ntohs(acm_ptr->msg.saddr.ib.lid), + ntohl(acm_ptr->msg.saddr.ib.qpn), exp, + acm_ptr->msg.rd_in, acm_ptr->msg.mtu, + acm_ptr->msg.resv[0]); #ifdef DAT_EXTENSIONS if (acm_ptr->msg.saddr.ib.qp_type == IBV_QPT_UD) { @@ -1274,7 +1281,7 @@ dapli_socket_accept_usr(DAPL_EP * ep_ptr, } #endif /* rdma_out, initiator, cannot exceed remote rdma_in max */ - if (ntohs(cm_ptr->msg.ver) >= 7) + if (ntohs(cm_ptr->msg.ver) >= DCM_VER_RR) ep_ptr->param.ep_attr.max_rdma_read_out = DAPL_MIN(ep_ptr->param.ep_attr.max_rdma_read_out, cm_ptr->msg.rd_in); @@ -1283,7 +1290,7 @@ dapli_socket_accept_usr(DAPL_EP * ep_ptr, exp += SCM_BC_DIFF; /* Set QP MTU, if negotiated. 2K for compatibility */ - ep_ptr->qp_handle->mtu = cm_ptr->msg.mtu ? + ep_ptr->qp_handle->mtu = (ntohs(cm_ptr->msg.ver) >= DCM_VER_MTU) ? DAPL_MIN(cm_ptr->msg.mtu, cm_ptr->hca->ib_trans.ib_cm.mtu): getenv("DAPL_IB_MTU") ? cm_ptr->hca->ib_trans.ib_cm.mtu : IBV_MTU_2048; @@ -1323,6 +1330,7 @@ dapli_socket_accept_usr(DAPL_EP * ep_ptr, sizeof(union dcm_addr)); /* send our QP info, IA address, pdata. Don't overwrite dst data */ + (void)dapl_os_memzero(&local, sizeof(ib_cm_msg_t)); local.ver = htons(DCM_VER); local.op = htons(DCM_REP); local.rd_in = ep_ptr->param.ep_attr.max_rdma_read_in; -- 2.46.0