> -----Original Message----- > From: Bart Van Assche [mailto:bart.vanassche@xxxxxxxxxxx] > Sent: Tuesday, October 25, 2016 12:58 PM > To: Steve Wise; dledford@xxxxxxxxxx; sean.hefty@xxxxxxxxx > Cc: linux-rdma@xxxxxxxxxxxxxxx; linux-nvme@xxxxxxxxxxxxxxxxxxx; > sagi@xxxxxxxxxxx; hch@xxxxxx; axboe@xxxxxx; santosh.shilimkar@xxxxxxxxxx > 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@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
