diff mbox

[v5,1/6] IB/core, cma: Nice log-friendly string helpers

Message ID 1431432329-859-2-git-send-email-sagig@mellanox.com (mailing list archive)
State Changes Requested
Headers show

Commit Message

Sagi Grimberg May 12, 2015, 12:05 p.m. UTC
Some of us keep revisiting the code to decode enumerations that
appear in out logs. Let's borrow the nice logging helpers that
exists in xprtrdma and rds for CMA events, IB events and WC statuses.

Reviewed-by: Yann Droneaud <ydroneaud@opteya.com>
Reviewed-by: Bart Van Assche <bart.vanassche@sandisk.com>
Signed-off-by: Sagi Grimberg <sagig@mellanox.com>
---
 drivers/infiniband/core/cma.c   |   28 +++++++++++++++++
 drivers/infiniband/core/verbs.c |   65 +++++++++++++++++++++++++++++++++++++++
 include/rdma/ib_verbs.h         |    4 ++
 include/rdma/rdma_cm.h          |    2 +
 4 files changed, 99 insertions(+), 0 deletions(-)

Comments

Or Gerlitz May 12, 2015, 12:59 p.m. UTC | #1
On Tue, May 12, 2015 at 3:05 PM, Sagi Grimberg <sagig@mellanox.com> wrote:
> Some of us keep revisiting the code to decode enumerations that
> appear in out logs. Let's borrow the nice logging helpers that
> exists in xprtrdma and rds for CMA events, IB events and WC statuses.

[...]

> +static const char * const cma_events[] = {
> +       [RDMA_CM_EVENT_ADDR_RESOLVED]   = "ADDR_RESOLVED",
> +       [RDMA_CM_EVENT_ADDR_ERROR]      = "ADDR_ERROR",
> +       [RDMA_CM_EVENT_ROUTE_RESOLVED]  = "ROUTE_RESOLVED",
> +       [RDMA_CM_EVENT_ROUTE_ERROR]     = "ROUTE_ERROR",
> +       [RDMA_CM_EVENT_CONNECT_REQUEST] = "CONNECT_REQUEST",
> +       [RDMA_CM_EVENT_CONNECT_RESPONSE]= "CONNECT_RESPONSE",
> +       [RDMA_CM_EVENT_CONNECT_ERROR]   = "CONNECT_ERROR",
> +       [RDMA_CM_EVENT_UNREACHABLE]     = "UNREACHABLE",
> +       [RDMA_CM_EVENT_REJECTED]        = "REJECTED",
> +       [RDMA_CM_EVENT_ESTABLISHED]     = "ESTABLISHED",
> +       [RDMA_CM_EVENT_DISCONNECTED]    = "DISCONNECTED",
> +       [RDMA_CM_EVENT_DEVICE_REMOVAL]  = "DEVICE_REMOVAL",
> +       [RDMA_CM_EVENT_MULTICAST_JOIN]  = "MULTICAST_JOIN",
> +       [RDMA_CM_EVENT_MULTICAST_ERROR] = "MULTICAST_ERROR",
> +       [RDMA_CM_EVENT_ADDR_CHANGE]     = "ADDR_CHANGE",
> +       [RDMA_CM_EVENT_TIMEWAIT_EXIT]   = "TIMEWAIT_EXIT",
> +};

Sagi,

My vote goes to using easy-to-digest human-readable non-capital
letters strings here and
elsewhere (IB events, etc),  as done today by the NFS code and
generally by errno based
helpers such as strerror, e.g

"address resolved"
"address error"
"route resolved"

Will be happy to hear what others think

Or.

> +
> +const char *rdma_event_msg(enum rdma_cm_event_type event)
> +{
> +       size_t index = event;
> +
> +       return (index < ARRAY_SIZE(cma_events) && cma_events[index]) ?
> +                       cma_events[index] : "UNRECOGNIZED_EVENT";
> +}
> +EXPORT_SYMBOL(rdma_event_msg);
> +
>  static void cma_add_one(struct ib_device *device);
>  static void cma_remove_one(struct ib_device *device);
>
> diff --git a/drivers/infiniband/core/verbs.c b/drivers/infiniband/core/verbs.c
> index f93eb8d..e366a52 100644
> --- a/drivers/infiniband/core/verbs.c
> +++ b/drivers/infiniband/core/verbs.c
> @@ -48,6 +48,71 @@
>
>  #include "core_priv.h"
>
> +static const char * const ib_events[] = {
> +       [IB_EVENT_CQ_ERR]               = "CQ_ERR",
> +       [IB_EVENT_QP_FATAL]             = "QP_FATAL",
> +       [IB_EVENT_QP_REQ_ERR]           = "QP_REQ_ERR",
> +       [IB_EVENT_QP_ACCESS_ERR]        = "QP_ACCESS_ERR",
> +       [IB_EVENT_COMM_EST]             = "COMM_EST",
> +       [IB_EVENT_SQ_DRAINED]           = "SQ_DRAINED",
> +       [IB_EVENT_PATH_MIG]             = "PATH_MIG",
> +       [IB_EVENT_PATH_MIG_ERR]         = "PATH_MIG_ERR",
> +       [IB_EVENT_DEVICE_FATAL]         = "DEVICE_FATAL",
> +       [IB_EVENT_PORT_ACTIVE]          = "PORT_ACTIVE",
> +       [IB_EVENT_PORT_ERR]             = "PORT_ERR",
> +       [IB_EVENT_LID_CHANGE]           = "LID_CHANGE",
> +       [IB_EVENT_PKEY_CHANGE]          = "PKEY_CHANGE",
> +       [IB_EVENT_SM_CHANGE]            = "SM_CHANGE",
> +       [IB_EVENT_SRQ_ERR]              = "SRQ_ERR",
> +       [IB_EVENT_SRQ_LIMIT_REACHED]    = "SRQ_LIMIT_REACHED",
> +       [IB_EVENT_QP_LAST_WQE_REACHED]  = "QP_LAST_WQE_REACHED",
> +       [IB_EVENT_CLIENT_REREGISTER]    = "CLIENT_REREGISTER",
> +       [IB_EVENT_GID_CHANGE]           = "GID_CHANGE",
> +};
> +
> +const char *ib_event_msg(enum ib_event_type event)
> +{
> +       size_t index = event;
> +
> +       return (index < ARRAY_SIZE(ib_events) && ib_events[index]) ?
> +                       ib_events[index] : "UNRECOGNIZED_EVENT";
> +}
> +EXPORT_SYMBOL(ib_event_msg);
> +
> +static const char * const wc_statuses[] = {
> +       [IB_WC_SUCCESS]                 = "SUCCESS",
> +       [IB_WC_LOC_LEN_ERR]             = "LOC_LEN_ERR",
> +       [IB_WC_LOC_QP_OP_ERR]           = "LOC_QP_OP_ERR",
> +       [IB_WC_LOC_EEC_OP_ERR]          = "LOC_EEC_OP_ERR",
> +       [IB_WC_LOC_PROT_ERR]            = "LOC_PROT_ERR",
> +       [IB_WC_WR_FLUSH_ERR]            = "WR_FLUSH_ERR",
> +       [IB_WC_MW_BIND_ERR]             = "MW_BIND_ERR",
> +       [IB_WC_BAD_RESP_ERR]            = "BAD_RESP_ERR",
> +       [IB_WC_LOC_ACCESS_ERR]          = "LOC_ACCESS_ERR",
> +       [IB_WC_REM_INV_REQ_ERR]         = "REM_INV_REQ_ERR",
> +       [IB_WC_REM_ACCESS_ERR]          = "REM_ACCESS_ERR",
> +       [IB_WC_REM_OP_ERR]              = "REM_OP_ERR",
> +       [IB_WC_RETRY_EXC_ERR]           = "RETRY_EXC_ERR",
> +       [IB_WC_RNR_RETRY_EXC_ERR]       = "RNR_RETRY_EXC_ERR",
> +       [IB_WC_LOC_RDD_VIOL_ERR]        = "LOC_RDD_VIOL_ERR",
> +       [IB_WC_REM_INV_RD_REQ_ERR]      = "REM_INV_RD_REQ_ERR",
> +       [IB_WC_REM_ABORT_ERR]           = "REM_ABORT_ERR",
> +       [IB_WC_INV_EECN_ERR]            = "INV_EECN_ERR",
> +       [IB_WC_INV_EEC_STATE_ERR]       = "INV_EEC_STATE_ERR",
> +       [IB_WC_FATAL_ERR]               = "FATAL_ERR",
> +       [IB_WC_RESP_TIMEOUT_ERR]        = "RESP_TIMEOUT_ERR",
> +       [IB_WC_GENERAL_ERR]             = "GENERAL_ERR",
> +};
> +
> +const char *ib_wc_status_msg(enum ib_wc_status status)
> +{
> +       size_t index = status;
> +
> +       return (index < ARRAY_SIZE(wc_statuses) && wc_statuses[index]) ?
> +                       wc_statuses[index] : "UNRECOGNIZED_STATUS";
> +}
> +EXPORT_SYMBOL(ib_wc_status_msg);
> +
>  __attribute_const__ int ib_rate_to_mult(enum ib_rate rate)
>  {
>         switch (rate) {
> diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
> index 65994a1..672fc8f 100644
> --- a/include/rdma/ib_verbs.h
> +++ b/include/rdma/ib_verbs.h
> @@ -412,6 +412,8 @@ enum ib_event_type {
>         IB_EVENT_GID_CHANGE,
>  };
>
> +__attribute_const__ const char *ib_event_msg(enum ib_event_type event);
> +
>  struct ib_event {
>         struct ib_device        *device;
>         union {
> @@ -663,6 +665,8 @@ enum ib_wc_status {
>         IB_WC_GENERAL_ERR
>  };
>
> +__attribute_const__ const char *ib_wc_status_msg(enum ib_wc_status status);
> +
>  enum ib_wc_opcode {
>         IB_WC_SEND,
>         IB_WC_RDMA_WRITE,
> diff --git a/include/rdma/rdma_cm.h b/include/rdma/rdma_cm.h
> index 1ed2088..c92522c 100644
> --- a/include/rdma/rdma_cm.h
> +++ b/include/rdma/rdma_cm.h
> @@ -62,6 +62,8 @@ enum rdma_cm_event_type {
>         RDMA_CM_EVENT_TIMEWAIT_EXIT
>  };
>
> +__attribute_const__ const char *rdma_event_msg(enum rdma_cm_event_type event);
> +
>  enum rdma_port_space {
>         RDMA_PS_SDP   = 0x0001,
>         RDMA_PS_IPOIB = 0x0002,
> --
> 1.7.1
>
> --
> 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
--
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
Sagi Grimberg May 12, 2015, 1:33 p.m. UTC | #2
On 5/12/2015 3:59 PM, Or Gerlitz wrote:
> On Tue, May 12, 2015 at 3:05 PM, Sagi Grimberg <sagig@mellanox.com> wrote:
>> Some of us keep revisiting the code to decode enumerations that
>> appear in out logs. Let's borrow the nice logging helpers that
>> exists in xprtrdma and rds for CMA events, IB events and WC statuses.
>
> [...]
>
>> +static const char * const cma_events[] = {
>> +       [RDMA_CM_EVENT_ADDR_RESOLVED]   = "ADDR_RESOLVED",
>> +       [RDMA_CM_EVENT_ADDR_ERROR]      = "ADDR_ERROR",
>> +       [RDMA_CM_EVENT_ROUTE_RESOLVED]  = "ROUTE_RESOLVED",
>> +       [RDMA_CM_EVENT_ROUTE_ERROR]     = "ROUTE_ERROR",
>> +       [RDMA_CM_EVENT_CONNECT_REQUEST] = "CONNECT_REQUEST",
>> +       [RDMA_CM_EVENT_CONNECT_RESPONSE]= "CONNECT_RESPONSE",
>> +       [RDMA_CM_EVENT_CONNECT_ERROR]   = "CONNECT_ERROR",
>> +       [RDMA_CM_EVENT_UNREACHABLE]     = "UNREACHABLE",
>> +       [RDMA_CM_EVENT_REJECTED]        = "REJECTED",
>> +       [RDMA_CM_EVENT_ESTABLISHED]     = "ESTABLISHED",
>> +       [RDMA_CM_EVENT_DISCONNECTED]    = "DISCONNECTED",
>> +       [RDMA_CM_EVENT_DEVICE_REMOVAL]  = "DEVICE_REMOVAL",
>> +       [RDMA_CM_EVENT_MULTICAST_JOIN]  = "MULTICAST_JOIN",
>> +       [RDMA_CM_EVENT_MULTICAST_ERROR] = "MULTICAST_ERROR",
>> +       [RDMA_CM_EVENT_ADDR_CHANGE]     = "ADDR_CHANGE",
>> +       [RDMA_CM_EVENT_TIMEWAIT_EXIT]   = "TIMEWAIT_EXIT",
>> +};
>
> Sagi,
>
> My vote goes to using easy-to-digest human-readable non-capital
> letters strings here and
> elsewhere (IB events, etc),  as done today by the NFS code and
> generally by errno based
> helpers such as strerror, e.g
>
> "address resolved"
> "address error"
> "route resolved"
>
> Will be happy to hear what others think

Let's do that before I post v6...
--
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
Chuck Lever May 12, 2015, 1:56 p.m. UTC | #3
On May 12, 2015, at 8:59 AM, Or Gerlitz <gerlitz.or@gmail.com> wrote:

> On Tue, May 12, 2015 at 3:05 PM, Sagi Grimberg <sagig@mellanox.com> wrote:
>> Some of us keep revisiting the code to decode enumerations that
>> appear in out logs. Let's borrow the nice logging helpers that
>> exists in xprtrdma and rds for CMA events, IB events and WC statuses.
> 
> [...]
> 
>> +static const char * const cma_events[] = {
>> +       [RDMA_CM_EVENT_ADDR_RESOLVED]   = "ADDR_RESOLVED",
>> +       [RDMA_CM_EVENT_ADDR_ERROR]      = "ADDR_ERROR",
>> +       [RDMA_CM_EVENT_ROUTE_RESOLVED]  = "ROUTE_RESOLVED",
>> +       [RDMA_CM_EVENT_ROUTE_ERROR]     = "ROUTE_ERROR",
>> +       [RDMA_CM_EVENT_CONNECT_REQUEST] = "CONNECT_REQUEST",
>> +       [RDMA_CM_EVENT_CONNECT_RESPONSE]= "CONNECT_RESPONSE",
>> +       [RDMA_CM_EVENT_CONNECT_ERROR]   = "CONNECT_ERROR",
>> +       [RDMA_CM_EVENT_UNREACHABLE]     = "UNREACHABLE",
>> +       [RDMA_CM_EVENT_REJECTED]        = "REJECTED",
>> +       [RDMA_CM_EVENT_ESTABLISHED]     = "ESTABLISHED",
>> +       [RDMA_CM_EVENT_DISCONNECTED]    = "DISCONNECTED",
>> +       [RDMA_CM_EVENT_DEVICE_REMOVAL]  = "DEVICE_REMOVAL",
>> +       [RDMA_CM_EVENT_MULTICAST_JOIN]  = "MULTICAST_JOIN",
>> +       [RDMA_CM_EVENT_MULTICAST_ERROR] = "MULTICAST_ERROR",
>> +       [RDMA_CM_EVENT_ADDR_CHANGE]     = "ADDR_CHANGE",
>> +       [RDMA_CM_EVENT_TIMEWAIT_EXIT]   = "TIMEWAIT_EXIT",
>> +};
> 
> Sagi,
> 
> My vote goes to using easy-to-digest human-readable non-capital
> letters strings here and
> elsewhere (IB events, etc),  as done today by the NFS code and
> generally by errno based
> helpers such as strerror, e.g
> 
> "address resolved"
> "address error"
> "route resolved"
> 
> Will be happy to hear what others think

I generally prefer spelled-out strings over symbol names.

For the strings in xprtrdma, I looked up the symbols in the IBTA spec,
and used part of the description of each error. It’s obvious in most
cases, but there are one or two that are a little odd (eg. MW_BIND_ERR
is “memory management operation error”).

> Or.
> 
>> +
>> +const char *rdma_event_msg(enum rdma_cm_event_type event)
>> +{
>> +       size_t index = event;
>> +
>> +       return (index < ARRAY_SIZE(cma_events) && cma_events[index]) ?
>> +                       cma_events[index] : "UNRECOGNIZED_EVENT";
>> +}
>> +EXPORT_SYMBOL(rdma_event_msg);
>> +
>> static void cma_add_one(struct ib_device *device);
>> static void cma_remove_one(struct ib_device *device);
>> 
>> diff --git a/drivers/infiniband/core/verbs.c b/drivers/infiniband/core/verbs.c
>> index f93eb8d..e366a52 100644
>> --- a/drivers/infiniband/core/verbs.c
>> +++ b/drivers/infiniband/core/verbs.c
>> @@ -48,6 +48,71 @@
>> 
>> #include "core_priv.h"
>> 
>> +static const char * const ib_events[] = {
>> +       [IB_EVENT_CQ_ERR]               = "CQ_ERR",
>> +       [IB_EVENT_QP_FATAL]             = "QP_FATAL",
>> +       [IB_EVENT_QP_REQ_ERR]           = "QP_REQ_ERR",
>> +       [IB_EVENT_QP_ACCESS_ERR]        = "QP_ACCESS_ERR",
>> +       [IB_EVENT_COMM_EST]             = "COMM_EST",
>> +       [IB_EVENT_SQ_DRAINED]           = "SQ_DRAINED",
>> +       [IB_EVENT_PATH_MIG]             = "PATH_MIG",
>> +       [IB_EVENT_PATH_MIG_ERR]         = "PATH_MIG_ERR",
>> +       [IB_EVENT_DEVICE_FATAL]         = "DEVICE_FATAL",
>> +       [IB_EVENT_PORT_ACTIVE]          = "PORT_ACTIVE",
>> +       [IB_EVENT_PORT_ERR]             = "PORT_ERR",
>> +       [IB_EVENT_LID_CHANGE]           = "LID_CHANGE",
>> +       [IB_EVENT_PKEY_CHANGE]          = "PKEY_CHANGE",
>> +       [IB_EVENT_SM_CHANGE]            = "SM_CHANGE",
>> +       [IB_EVENT_SRQ_ERR]              = "SRQ_ERR",
>> +       [IB_EVENT_SRQ_LIMIT_REACHED]    = "SRQ_LIMIT_REACHED",
>> +       [IB_EVENT_QP_LAST_WQE_REACHED]  = "QP_LAST_WQE_REACHED",
>> +       [IB_EVENT_CLIENT_REREGISTER]    = "CLIENT_REREGISTER",
>> +       [IB_EVENT_GID_CHANGE]           = "GID_CHANGE",
>> +};
>> +
>> +const char *ib_event_msg(enum ib_event_type event)
>> +{
>> +       size_t index = event;
>> +
>> +       return (index < ARRAY_SIZE(ib_events) && ib_events[index]) ?
>> +                       ib_events[index] : "UNRECOGNIZED_EVENT";
>> +}
>> +EXPORT_SYMBOL(ib_event_msg);
>> +
>> +static const char * const wc_statuses[] = {
>> +       [IB_WC_SUCCESS]                 = "SUCCESS",
>> +       [IB_WC_LOC_LEN_ERR]             = "LOC_LEN_ERR",
>> +       [IB_WC_LOC_QP_OP_ERR]           = "LOC_QP_OP_ERR",
>> +       [IB_WC_LOC_EEC_OP_ERR]          = "LOC_EEC_OP_ERR",
>> +       [IB_WC_LOC_PROT_ERR]            = "LOC_PROT_ERR",
>> +       [IB_WC_WR_FLUSH_ERR]            = "WR_FLUSH_ERR",
>> +       [IB_WC_MW_BIND_ERR]             = "MW_BIND_ERR",
>> +       [IB_WC_BAD_RESP_ERR]            = "BAD_RESP_ERR",
>> +       [IB_WC_LOC_ACCESS_ERR]          = "LOC_ACCESS_ERR",
>> +       [IB_WC_REM_INV_REQ_ERR]         = "REM_INV_REQ_ERR",
>> +       [IB_WC_REM_ACCESS_ERR]          = "REM_ACCESS_ERR",
>> +       [IB_WC_REM_OP_ERR]              = "REM_OP_ERR",
>> +       [IB_WC_RETRY_EXC_ERR]           = "RETRY_EXC_ERR",
>> +       [IB_WC_RNR_RETRY_EXC_ERR]       = "RNR_RETRY_EXC_ERR",
>> +       [IB_WC_LOC_RDD_VIOL_ERR]        = "LOC_RDD_VIOL_ERR",
>> +       [IB_WC_REM_INV_RD_REQ_ERR]      = "REM_INV_RD_REQ_ERR",
>> +       [IB_WC_REM_ABORT_ERR]           = "REM_ABORT_ERR",
>> +       [IB_WC_INV_EECN_ERR]            = "INV_EECN_ERR",
>> +       [IB_WC_INV_EEC_STATE_ERR]       = "INV_EEC_STATE_ERR",
>> +       [IB_WC_FATAL_ERR]               = "FATAL_ERR",
>> +       [IB_WC_RESP_TIMEOUT_ERR]        = "RESP_TIMEOUT_ERR",
>> +       [IB_WC_GENERAL_ERR]             = "GENERAL_ERR",
>> +};
>> +
>> +const char *ib_wc_status_msg(enum ib_wc_status status)
>> +{
>> +       size_t index = status;
>> +
>> +       return (index < ARRAY_SIZE(wc_statuses) && wc_statuses[index]) ?
>> +                       wc_statuses[index] : "UNRECOGNIZED_STATUS";
>> +}
>> +EXPORT_SYMBOL(ib_wc_status_msg);
>> +
>> __attribute_const__ int ib_rate_to_mult(enum ib_rate rate)
>> {
>>        switch (rate) {
>> diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
>> index 65994a1..672fc8f 100644
>> --- a/include/rdma/ib_verbs.h
>> +++ b/include/rdma/ib_verbs.h
>> @@ -412,6 +412,8 @@ enum ib_event_type {
>>        IB_EVENT_GID_CHANGE,
>> };
>> 
>> +__attribute_const__ const char *ib_event_msg(enum ib_event_type event);
>> +
>> struct ib_event {
>>        struct ib_device        *device;
>>        union {
>> @@ -663,6 +665,8 @@ enum ib_wc_status {
>>        IB_WC_GENERAL_ERR
>> };
>> 
>> +__attribute_const__ const char *ib_wc_status_msg(enum ib_wc_status status);
>> +
>> enum ib_wc_opcode {
>>        IB_WC_SEND,
>>        IB_WC_RDMA_WRITE,
>> diff --git a/include/rdma/rdma_cm.h b/include/rdma/rdma_cm.h
>> index 1ed2088..c92522c 100644
>> --- a/include/rdma/rdma_cm.h
>> +++ b/include/rdma/rdma_cm.h
>> @@ -62,6 +62,8 @@ enum rdma_cm_event_type {
>>        RDMA_CM_EVENT_TIMEWAIT_EXIT
>> };
>> 
>> +__attribute_const__ const char *rdma_event_msg(enum rdma_cm_event_type event);
>> +
>> enum rdma_port_space {
>>        RDMA_PS_SDP   = 0x0001,
>>        RDMA_PS_IPOIB = 0x0002,
>> --
>> 1.7.1
>> 
>> --
>> 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
> --
> 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

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com



--
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
Doug Ledford May 12, 2015, 5:21 p.m. UTC | #4
On Tue, 2015-05-12 at 09:56 -0400, Chuck Lever wrote:
> On May 12, 2015, at 8:59 AM, Or Gerlitz <gerlitz.or@gmail.com> wrote:
> 
> > On Tue, May 12, 2015 at 3:05 PM, Sagi Grimberg <sagig@mellanox.com> wrote:
> >> Some of us keep revisiting the code to decode enumerations that
> >> appear in out logs. Let's borrow the nice logging helpers that
> >> exists in xprtrdma and rds for CMA events, IB events and WC statuses.
> > 
> > [...]
> > 
> >> +static const char * const cma_events[] = {
> >> +       [RDMA_CM_EVENT_ADDR_RESOLVED]   = "ADDR_RESOLVED",
> >> +       [RDMA_CM_EVENT_ADDR_ERROR]      = "ADDR_ERROR",
> >> +       [RDMA_CM_EVENT_ROUTE_RESOLVED]  = "ROUTE_RESOLVED",
> >> +       [RDMA_CM_EVENT_ROUTE_ERROR]     = "ROUTE_ERROR",
> >> +       [RDMA_CM_EVENT_CONNECT_REQUEST] = "CONNECT_REQUEST",
> >> +       [RDMA_CM_EVENT_CONNECT_RESPONSE]= "CONNECT_RESPONSE",
> >> +       [RDMA_CM_EVENT_CONNECT_ERROR]   = "CONNECT_ERROR",
> >> +       [RDMA_CM_EVENT_UNREACHABLE]     = "UNREACHABLE",
> >> +       [RDMA_CM_EVENT_REJECTED]        = "REJECTED",
> >> +       [RDMA_CM_EVENT_ESTABLISHED]     = "ESTABLISHED",
> >> +       [RDMA_CM_EVENT_DISCONNECTED]    = "DISCONNECTED",
> >> +       [RDMA_CM_EVENT_DEVICE_REMOVAL]  = "DEVICE_REMOVAL",
> >> +       [RDMA_CM_EVENT_MULTICAST_JOIN]  = "MULTICAST_JOIN",
> >> +       [RDMA_CM_EVENT_MULTICAST_ERROR] = "MULTICAST_ERROR",
> >> +       [RDMA_CM_EVENT_ADDR_CHANGE]     = "ADDR_CHANGE",
> >> +       [RDMA_CM_EVENT_TIMEWAIT_EXIT]   = "TIMEWAIT_EXIT",
> >> +};
> > 
> > Sagi,
> > 
> > My vote goes to using easy-to-digest human-readable non-capital
> > letters strings here and
> > elsewhere (IB events, etc),  as done today by the NFS code and
> > generally by errno based
> > helpers such as strerror, e.g
> > 
> > "address resolved"
> > "address error"
> > "route resolved"
> > 
> > Will be happy to hear what others think
> 
> I generally prefer spelled-out strings over symbol names.
> 
> For the strings in xprtrdma, I looked up the symbols in the IBTA spec,
> and used part of the description of each error. It’s obvious in most
> cases, but there are one or two that are a little odd (eg. MW_BIND_ERR
> is “memory management operation error”).

Likewise, I prefer the more complete description to a simple
regurgitation of C preprocessor symbols.  Aside from that, I'm glad to
see this patchset come along because I hate looking up error numbers
too.
diff mbox

Patch

diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
index d570030..ef9f277 100644
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
@@ -65,6 +65,34 @@  MODULE_LICENSE("Dual BSD/GPL");
 #define CMA_CM_MRA_SETTING (IB_CM_MRA_FLAG_DELAY | 24)
 #define CMA_IBOE_PACKET_LIFETIME 18
 
+static const char * const cma_events[] = {
+	[RDMA_CM_EVENT_ADDR_RESOLVED]	= "ADDR_RESOLVED",
+	[RDMA_CM_EVENT_ADDR_ERROR]	= "ADDR_ERROR",
+	[RDMA_CM_EVENT_ROUTE_RESOLVED]	= "ROUTE_RESOLVED",
+	[RDMA_CM_EVENT_ROUTE_ERROR]	= "ROUTE_ERROR",
+	[RDMA_CM_EVENT_CONNECT_REQUEST]	= "CONNECT_REQUEST",
+	[RDMA_CM_EVENT_CONNECT_RESPONSE]= "CONNECT_RESPONSE",
+	[RDMA_CM_EVENT_CONNECT_ERROR]	= "CONNECT_ERROR",
+	[RDMA_CM_EVENT_UNREACHABLE]	= "UNREACHABLE",
+	[RDMA_CM_EVENT_REJECTED]	= "REJECTED",
+	[RDMA_CM_EVENT_ESTABLISHED]	= "ESTABLISHED",
+	[RDMA_CM_EVENT_DISCONNECTED]	= "DISCONNECTED",
+	[RDMA_CM_EVENT_DEVICE_REMOVAL]	= "DEVICE_REMOVAL",
+	[RDMA_CM_EVENT_MULTICAST_JOIN]	= "MULTICAST_JOIN",
+	[RDMA_CM_EVENT_MULTICAST_ERROR]	= "MULTICAST_ERROR",
+	[RDMA_CM_EVENT_ADDR_CHANGE]	= "ADDR_CHANGE",
+	[RDMA_CM_EVENT_TIMEWAIT_EXIT]	= "TIMEWAIT_EXIT",
+};
+
+const char *rdma_event_msg(enum rdma_cm_event_type event)
+{
+	size_t index = event;
+
+	return (index < ARRAY_SIZE(cma_events) && cma_events[index]) ?
+			cma_events[index] : "UNRECOGNIZED_EVENT";
+}
+EXPORT_SYMBOL(rdma_event_msg);
+
 static void cma_add_one(struct ib_device *device);
 static void cma_remove_one(struct ib_device *device);
 
diff --git a/drivers/infiniband/core/verbs.c b/drivers/infiniband/core/verbs.c
index f93eb8d..e366a52 100644
--- a/drivers/infiniband/core/verbs.c
+++ b/drivers/infiniband/core/verbs.c
@@ -48,6 +48,71 @@ 
 
 #include "core_priv.h"
 
+static const char * const ib_events[] = {
+	[IB_EVENT_CQ_ERR]		= "CQ_ERR",
+	[IB_EVENT_QP_FATAL]		= "QP_FATAL",
+	[IB_EVENT_QP_REQ_ERR]		= "QP_REQ_ERR",
+	[IB_EVENT_QP_ACCESS_ERR]	= "QP_ACCESS_ERR",
+	[IB_EVENT_COMM_EST]		= "COMM_EST",
+	[IB_EVENT_SQ_DRAINED]		= "SQ_DRAINED",
+	[IB_EVENT_PATH_MIG]		= "PATH_MIG",
+	[IB_EVENT_PATH_MIG_ERR]		= "PATH_MIG_ERR",
+	[IB_EVENT_DEVICE_FATAL]		= "DEVICE_FATAL",
+	[IB_EVENT_PORT_ACTIVE]		= "PORT_ACTIVE",
+	[IB_EVENT_PORT_ERR]		= "PORT_ERR",
+	[IB_EVENT_LID_CHANGE]		= "LID_CHANGE",
+	[IB_EVENT_PKEY_CHANGE]		= "PKEY_CHANGE",
+	[IB_EVENT_SM_CHANGE]		= "SM_CHANGE",
+	[IB_EVENT_SRQ_ERR]		= "SRQ_ERR",
+	[IB_EVENT_SRQ_LIMIT_REACHED]	= "SRQ_LIMIT_REACHED",
+	[IB_EVENT_QP_LAST_WQE_REACHED]	= "QP_LAST_WQE_REACHED",
+	[IB_EVENT_CLIENT_REREGISTER]	= "CLIENT_REREGISTER",
+	[IB_EVENT_GID_CHANGE]		= "GID_CHANGE",
+};
+
+const char *ib_event_msg(enum ib_event_type event)
+{
+	size_t index = event;
+
+	return (index < ARRAY_SIZE(ib_events) && ib_events[index]) ?
+			ib_events[index] : "UNRECOGNIZED_EVENT";
+}
+EXPORT_SYMBOL(ib_event_msg);
+
+static const char * const wc_statuses[] = {
+	[IB_WC_SUCCESS]			= "SUCCESS",
+	[IB_WC_LOC_LEN_ERR]		= "LOC_LEN_ERR",
+	[IB_WC_LOC_QP_OP_ERR]		= "LOC_QP_OP_ERR",
+	[IB_WC_LOC_EEC_OP_ERR]		= "LOC_EEC_OP_ERR",
+	[IB_WC_LOC_PROT_ERR]		= "LOC_PROT_ERR",
+	[IB_WC_WR_FLUSH_ERR]		= "WR_FLUSH_ERR",
+	[IB_WC_MW_BIND_ERR]		= "MW_BIND_ERR",
+	[IB_WC_BAD_RESP_ERR]		= "BAD_RESP_ERR",
+	[IB_WC_LOC_ACCESS_ERR]		= "LOC_ACCESS_ERR",
+	[IB_WC_REM_INV_REQ_ERR]		= "REM_INV_REQ_ERR",
+	[IB_WC_REM_ACCESS_ERR]		= "REM_ACCESS_ERR",
+	[IB_WC_REM_OP_ERR]		= "REM_OP_ERR",
+	[IB_WC_RETRY_EXC_ERR]		= "RETRY_EXC_ERR",
+	[IB_WC_RNR_RETRY_EXC_ERR]	= "RNR_RETRY_EXC_ERR",
+	[IB_WC_LOC_RDD_VIOL_ERR]	= "LOC_RDD_VIOL_ERR",
+	[IB_WC_REM_INV_RD_REQ_ERR]	= "REM_INV_RD_REQ_ERR",
+	[IB_WC_REM_ABORT_ERR]		= "REM_ABORT_ERR",
+	[IB_WC_INV_EECN_ERR]		= "INV_EECN_ERR",
+	[IB_WC_INV_EEC_STATE_ERR]	= "INV_EEC_STATE_ERR",
+	[IB_WC_FATAL_ERR]		= "FATAL_ERR",
+	[IB_WC_RESP_TIMEOUT_ERR]	= "RESP_TIMEOUT_ERR",
+	[IB_WC_GENERAL_ERR]		= "GENERAL_ERR",
+};
+
+const char *ib_wc_status_msg(enum ib_wc_status status)
+{
+	size_t index = status;
+
+	return (index < ARRAY_SIZE(wc_statuses) && wc_statuses[index]) ?
+			wc_statuses[index] : "UNRECOGNIZED_STATUS";
+}
+EXPORT_SYMBOL(ib_wc_status_msg);
+
 __attribute_const__ int ib_rate_to_mult(enum ib_rate rate)
 {
 	switch (rate) {
diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
index 65994a1..672fc8f 100644
--- a/include/rdma/ib_verbs.h
+++ b/include/rdma/ib_verbs.h
@@ -412,6 +412,8 @@  enum ib_event_type {
 	IB_EVENT_GID_CHANGE,
 };
 
+__attribute_const__ const char *ib_event_msg(enum ib_event_type event);
+
 struct ib_event {
 	struct ib_device	*device;
 	union {
@@ -663,6 +665,8 @@  enum ib_wc_status {
 	IB_WC_GENERAL_ERR
 };
 
+__attribute_const__ const char *ib_wc_status_msg(enum ib_wc_status status);
+
 enum ib_wc_opcode {
 	IB_WC_SEND,
 	IB_WC_RDMA_WRITE,
diff --git a/include/rdma/rdma_cm.h b/include/rdma/rdma_cm.h
index 1ed2088..c92522c 100644
--- a/include/rdma/rdma_cm.h
+++ b/include/rdma/rdma_cm.h
@@ -62,6 +62,8 @@  enum rdma_cm_event_type {
 	RDMA_CM_EVENT_TIMEWAIT_EXIT
 };
 
+__attribute_const__ const char *rdma_event_msg(enum rdma_cm_event_type event);
+
 enum rdma_port_space {
 	RDMA_PS_SDP   = 0x0001,
 	RDMA_PS_IPOIB = 0x0002,