| Message ID | ca2185d1f1e9008922d1b344f3887d400a1d6053.1477336045.git.swise@opengridcomputing.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
Looks good,
Reviewed-by: Sagi Grimberg <sagi@grimbeg.me>
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Oct 24, 2016 at 11:59:03AM -0700, Steve Wise wrote: > rdma_reject_msg() returns a pointer to a string message associated with > the transport reject reason codes. > > Signed-off-by: Steve Wise <swise@opengridcomputing.com> Looks good, Reviewed-by: Christoph Hellwig <hch@lst.de> (and /me notes the difference in error numbers between iWarp and the IB universe.. Stunning) -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10/24/2016 12:09 PM, Steve Wise wrote: > + [IB_CM_REJ_RDC_NOT_EXIST] = "rdc not exist", Please change this into "RDC does not exist". > + [IB_CM_REJ_INVALID_GID] = "invalid gid", > + [IB_CM_REJ_INVALID_LID] = "invalid lid", > + [IB_CM_REJ_INVALID_SL] = "invalid sl", > + [IB_CM_REJ_INVALID_TRAFFIC_CLASS] = "invalid traffic class", > + [IB_CM_REJ_INVALID_HOP_LIMIT] = "invalid hop limit", > + [IB_CM_REJ_INVALID_PACKET_RATE] = "invalid packet rate", > + [IB_CM_REJ_INVALID_ALT_GID] = "invalid alt gid", > + [IB_CM_REJ_INVALID_ALT_LID] = "invalid alt lid", > + [IB_CM_REJ_INVALID_ALT_SL] = "invalid alt sl", > + [IB_CM_REJ_INVALID_ALT_TRAFFIC_CLASS] = "invalid alt traffic class", > + [IB_CM_REJ_INVALID_ALT_HOP_LIMIT] = "invalid alt hop limit", > + [IB_CM_REJ_INVALID_ALT_PACKET_RATE] = "invalid alt packet rate", > + [IB_CM_REJ_PORT_CM_REDIRECT] = "port cm redirect", > + [IB_CM_REJ_PORT_REDIRECT] = "port redirect", > + [IB_CM_REJ_INVALID_MTU] = "invalid mtu", > + [IB_CM_REJ_INSUFFICIENT_RESP_RESOURCES] = "insufficient resp resources", > + [IB_CM_REJ_CONSUMER_DEFINED] = "consumer defined", > + [IB_CM_REJ_INVALID_RNR_RETRY] = "invalid rnr retry", > + [IB_CM_REJ_DUPLICATE_LOCAL_COMM_ID] = "duplicate local comm id", > + [IB_CM_REJ_INVALID_CLASS_VERSION] = "invalid class version", > + [IB_CM_REJ_INVALID_FLOW_LABEL] = "invalid flow label", > + [IB_CM_REJ_INVALID_ALT_FLOW_LABEL] = "invalid alt flow label", Other error messages capitalize GID, LID, CM, ID, RNR and SL so please do this here too. > +const char *__attribute_const__ ibcm_reject_msg(int reason) > +{ > + size_t index = reason; > + > + if (index >= ARRAY_SIZE(ibcm_rej_reason_strs) || > + !ibcm_rej_reason_strs[index]) > + return "unrecognized reason"; > + else > + return ibcm_rej_reason_strs[index]; > +} Please consider using positive logic - this means negating the if-condition and swapping the if- and else-parts. > +const char *__attribute_const__ rdma_reject_msg(struct rdma_cm_id *id, > + int reason) > +{ > + if (rdma_ib_or_roce(id->device, id->port_num)) > + return ibcm_reject_msg(reason); > + > + if (rdma_protocol_iwarp(id->device, id->port_num)) > + return iwcm_reject_msg(reason); > + > + WARN_ON_ONCE(1); > + return "unrecognized reason"; Have you considered to return "unrecognized transport" here instead? > +const char *__attribute_const__ iwcm_reject_msg(int reason) > +{ > + size_t index; > + > + /* iWARP uses negative errnos */ > + index = -reason; > + > + if (index >= ARRAY_SIZE(iwcm_rej_reason_strs) || > + !iwcm_rej_reason_strs[index]) > + return "unrecognized reason"; > + else > + return iwcm_rej_reason_strs[index]; > +} Also for this function, please consider using positive logic. Thanks, Bart. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message----- > From: Bart Van Assche [mailto:bart.vanassche@sandisk.com] > Sent: Tuesday, October 25, 2016 12:58 PM > To: Steve Wise; dledford@redhat.com; sean.hefty@intel.com > Cc: linux-rdma@vger.kernel.org; linux-nvme@lists.infradead.org; > sagi@grimberg.me; hch@lst.de; axboe@fb.com; santosh.shilimkar@oracle.com > Subject: Re: [PATCH v3 1/6] rdma_cm: add rdma_reject_msg() helper function > > On 10/24/2016 12:09 PM, Steve Wise wrote: > > + [IB_CM_REJ_RDC_NOT_EXIST] = "rdc not exist", > > Please change this into "RDC does not exist". > ok > > + [IB_CM_REJ_INVALID_GID] = "invalid gid", > > + [IB_CM_REJ_INVALID_LID] = "invalid lid", > > + [IB_CM_REJ_INVALID_SL] = "invalid sl", > > + [IB_CM_REJ_INVALID_TRAFFIC_CLASS] = "invalid traffic class", > > + [IB_CM_REJ_INVALID_HOP_LIMIT] = "invalid hop limit", > > + [IB_CM_REJ_INVALID_PACKET_RATE] = "invalid packet rate", > > + [IB_CM_REJ_INVALID_ALT_GID] = "invalid alt gid", > > + [IB_CM_REJ_INVALID_ALT_LID] = "invalid alt lid", > > + [IB_CM_REJ_INVALID_ALT_SL] = "invalid alt sl", > > + [IB_CM_REJ_INVALID_ALT_TRAFFIC_CLASS] = "invalid alt traffic class", > > + [IB_CM_REJ_INVALID_ALT_HOP_LIMIT] = "invalid alt hop limit", > > + [IB_CM_REJ_INVALID_ALT_PACKET_RATE] = "invalid alt packet rate", > > + [IB_CM_REJ_PORT_CM_REDIRECT] = "port cm redirect", > > + [IB_CM_REJ_PORT_REDIRECT] = "port redirect", > > + [IB_CM_REJ_INVALID_MTU] = "invalid mtu", > > + [IB_CM_REJ_INSUFFICIENT_RESP_RESOURCES] = "insufficient resp > resources", > > + [IB_CM_REJ_CONSUMER_DEFINED] = "consumer defined", > > + [IB_CM_REJ_INVALID_RNR_RETRY] = "invalid rnr retry", > > + [IB_CM_REJ_DUPLICATE_LOCAL_COMM_ID] = "duplicate local comm > id", > > + [IB_CM_REJ_INVALID_CLASS_VERSION] = "invalid class version", > > + [IB_CM_REJ_INVALID_FLOW_LABEL] = "invalid flow label", > > + [IB_CM_REJ_INVALID_ALT_FLOW_LABEL] = "invalid alt flow label", > > Other error messages capitalize GID, LID, CM, ID, RNR and SL so please > do this here too. > ok > > +const char *__attribute_const__ ibcm_reject_msg(int reason) > > +{ > > + size_t index = reason; > > + > > + if (index >= ARRAY_SIZE(ibcm_rej_reason_strs) || > > + !ibcm_rej_reason_strs[index]) > > + return "unrecognized reason"; > > + else > > + return ibcm_rej_reason_strs[index]; > > +} > > Please consider using positive logic - this means negating the > if-condition and swapping the if- and else-parts. yea that might be even clearer. > > > +const char *__attribute_const__ rdma_reject_msg(struct rdma_cm_id *id, > > + int reason) > > +{ > > + if (rdma_ib_or_roce(id->device, id->port_num)) > > + return ibcm_reject_msg(reason); > > + > > + if (rdma_protocol_iwarp(id->device, id->port_num)) > > + return iwcm_reject_msg(reason); > > + > > + WARN_ON_ONCE(1); > > + return "unrecognized reason"; > > Have you considered to return "unrecognized transport" here instead? > I can do that. > > +const char *__attribute_const__ iwcm_reject_msg(int reason) > > +{ > > + size_t index; > > + > > + /* iWARP uses negative errnos */ > > + index = -reason; > > + > > + if (index >= ARRAY_SIZE(iwcm_rej_reason_strs) || > > + !iwcm_rej_reason_strs[index]) > > + return "unrecognized reason"; > > + else > > + return iwcm_rej_reason_strs[index]; > > +} > > Also for this function, please consider using positive logic. > > Thanks, > > Bart. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/infiniband/core/cm.c b/drivers/infiniband/core/cm.c index c995255..98bcc9c 100644 --- a/drivers/infiniband/core/cm.c +++ b/drivers/infiniband/core/cm.c @@ -57,6 +57,54 @@ MODULE_AUTHOR("Sean Hefty"); MODULE_DESCRIPTION("InfiniBand CM"); MODULE_LICENSE("Dual BSD/GPL"); +static const char * const ibcm_rej_reason_strs[] = { + [IB_CM_REJ_NO_QP] = "no qp", + [IB_CM_REJ_NO_EEC] = "no eec", + [IB_CM_REJ_NO_RESOURCES] = "no resources", + [IB_CM_REJ_TIMEOUT] = "timeout", + [IB_CM_REJ_UNSUPPORTED] = "unsupported", + [IB_CM_REJ_INVALID_COMM_ID] = "invalid comm id", + [IB_CM_REJ_INVALID_COMM_INSTANCE] = "invalid comm instance", + [IB_CM_REJ_INVALID_SERVICE_ID] = "invalid service id", + [IB_CM_REJ_INVALID_TRANSPORT_TYPE] = "invalid transport type", + [IB_CM_REJ_STALE_CONN] = "stale conn", + [IB_CM_REJ_RDC_NOT_EXIST] = "rdc not exist", + [IB_CM_REJ_INVALID_GID] = "invalid gid", + [IB_CM_REJ_INVALID_LID] = "invalid lid", + [IB_CM_REJ_INVALID_SL] = "invalid sl", + [IB_CM_REJ_INVALID_TRAFFIC_CLASS] = "invalid traffic class", + [IB_CM_REJ_INVALID_HOP_LIMIT] = "invalid hop limit", + [IB_CM_REJ_INVALID_PACKET_RATE] = "invalid packet rate", + [IB_CM_REJ_INVALID_ALT_GID] = "invalid alt gid", + [IB_CM_REJ_INVALID_ALT_LID] = "invalid alt lid", + [IB_CM_REJ_INVALID_ALT_SL] = "invalid alt sl", + [IB_CM_REJ_INVALID_ALT_TRAFFIC_CLASS] = "invalid alt traffic class", + [IB_CM_REJ_INVALID_ALT_HOP_LIMIT] = "invalid alt hop limit", + [IB_CM_REJ_INVALID_ALT_PACKET_RATE] = "invalid alt packet rate", + [IB_CM_REJ_PORT_CM_REDIRECT] = "port cm redirect", + [IB_CM_REJ_PORT_REDIRECT] = "port redirect", + [IB_CM_REJ_INVALID_MTU] = "invalid mtu", + [IB_CM_REJ_INSUFFICIENT_RESP_RESOURCES] = "insufficient resp resources", + [IB_CM_REJ_CONSUMER_DEFINED] = "consumer defined", + [IB_CM_REJ_INVALID_RNR_RETRY] = "invalid rnr retry", + [IB_CM_REJ_DUPLICATE_LOCAL_COMM_ID] = "duplicate local comm id", + [IB_CM_REJ_INVALID_CLASS_VERSION] = "invalid class version", + [IB_CM_REJ_INVALID_FLOW_LABEL] = "invalid flow label", + [IB_CM_REJ_INVALID_ALT_FLOW_LABEL] = "invalid alt flow label", +}; + +const char *__attribute_const__ ibcm_reject_msg(int reason) +{ + size_t index = reason; + + if (index >= ARRAY_SIZE(ibcm_rej_reason_strs) || + !ibcm_rej_reason_strs[index]) + return "unrecognized reason"; + else + return ibcm_rej_reason_strs[index]; +} +EXPORT_SYMBOL(ibcm_reject_msg); + static void cm_add_one(struct ib_device *device); static void cm_remove_one(struct ib_device *device, void *client_data); diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c index 5f65a78..831e94b 100644 --- a/drivers/infiniband/core/cma.c +++ b/drivers/infiniband/core/cma.c @@ -101,6 +101,20 @@ const char *__attribute_const__ rdma_event_msg(enum rdma_cm_event_type event) } EXPORT_SYMBOL(rdma_event_msg); +const char *__attribute_const__ rdma_reject_msg(struct rdma_cm_id *id, + int reason) +{ + if (rdma_ib_or_roce(id->device, id->port_num)) + return ibcm_reject_msg(reason); + + if (rdma_protocol_iwarp(id->device, id->port_num)) + return iwcm_reject_msg(reason); + + WARN_ON_ONCE(1); + return "unrecognized reason"; +} +EXPORT_SYMBOL(rdma_reject_msg); + static void cma_add_one(struct ib_device *device); static void cma_remove_one(struct ib_device *device, void *client_data); diff --git a/drivers/infiniband/core/iwcm.c b/drivers/infiniband/core/iwcm.c index 357624f..8d01253 100644 --- a/drivers/infiniband/core/iwcm.c +++ b/drivers/infiniband/core/iwcm.c @@ -59,6 +59,27 @@ MODULE_AUTHOR("Tom Tucker"); MODULE_DESCRIPTION("iWARP CM"); MODULE_LICENSE("Dual BSD/GPL"); +static const char * const iwcm_rej_reason_strs[] = { + [ECONNRESET] = "reset by remote host", + [ECONNREFUSED] = "refused by remote application", + [ETIMEDOUT] = "setup timeout", +}; + +const char *__attribute_const__ iwcm_reject_msg(int reason) +{ + size_t index; + + /* iWARP uses negative errnos */ + index = -reason; + + if (index >= ARRAY_SIZE(iwcm_rej_reason_strs) || + !iwcm_rej_reason_strs[index]) + return "unrecognized reason"; + else + return iwcm_rej_reason_strs[index]; +} +EXPORT_SYMBOL(iwcm_reject_msg); + static struct ibnl_client_cbs iwcm_nl_cb_table[] = { [RDMA_NL_IWPM_REG_PID] = {.dump = iwpm_register_pid_cb}, [RDMA_NL_IWPM_ADD_MAPPING] = {.dump = iwpm_add_mapping_cb}, diff --git a/include/rdma/ib_cm.h b/include/rdma/ib_cm.h index 92a7d85..b49258b 100644 --- a/include/rdma/ib_cm.h +++ b/include/rdma/ib_cm.h @@ -603,4 +603,10 @@ struct ib_cm_sidr_rep_param { int ib_send_cm_sidr_rep(struct ib_cm_id *cm_id, struct ib_cm_sidr_rep_param *param); +/** + * ibcm_reject_msg - return a pointer to a reject message string. + * @reason: Value returned in the REJECT event status field. + */ +const char *__attribute_const__ ibcm_reject_msg(int reason); + #endif /* IB_CM_H */ diff --git a/include/rdma/iw_cm.h b/include/rdma/iw_cm.h index 6d0065c..5cd7701 100644 --- a/include/rdma/iw_cm.h +++ b/include/rdma/iw_cm.h @@ -253,4 +253,10 @@ int iw_cm_disconnect(struct iw_cm_id *cm_id, int abrupt); int iw_cm_init_qp_attr(struct iw_cm_id *cm_id, struct ib_qp_attr *qp_attr, int *qp_attr_mask); +/** + * iwcm_reject_msg - return a pointer to a reject message string. + * @reason: Value returned in the REJECT event status field. + */ +const char *__attribute_const__ iwcm_reject_msg(int reason); + #endif /* IW_CM_H */ diff --git a/include/rdma/rdma_cm.h b/include/rdma/rdma_cm.h index 81fb1d1..f11a768 100644 --- a/include/rdma/rdma_cm.h +++ b/include/rdma/rdma_cm.h @@ -388,4 +388,12 @@ int rdma_set_afonly(struct rdma_cm_id *id, int afonly); */ __be64 rdma_get_service_id(struct rdma_cm_id *id, struct sockaddr *addr); +/** + * rdma_reject_msg - return a pointer to a reject message string. + * @id: Communication identifier that received the REJECT event. + * @reason: Value returned in the REJECT event status field. + */ +const char *__attribute_const__ rdma_reject_msg(struct rdma_cm_id *id, + int reason); + #endif /* RDMA_CM_H */
rdma_reject_msg() returns a pointer to a string message associated with the transport reject reason codes. Signed-off-by: Steve Wise <swise@opengridcomputing.com> --- drivers/infiniband/core/cm.c | 48 ++++++++++++++++++++++++++++++++++++++++++ drivers/infiniband/core/cma.c | 14 ++++++++++++ drivers/infiniband/core/iwcm.c | 21 ++++++++++++++++++ include/rdma/ib_cm.h | 6 ++++++ include/rdma/iw_cm.h | 6 ++++++ include/rdma/rdma_cm.h | 8 +++++++ 6 files changed, 103 insertions(+)