[openib-general] Re: UCM accesses internal cm_id state
Libor Michalek
libor at topspin.com
Tue Jun 7 15:56:46 PDT 2005
On Thu, Jun 02, 2005 at 12:55:34PM -0700, Sean Hefty wrote:
> The ucm code in ib_ucm_event_handler() reads the cm_id->state information in
> response to an event occurrence. The cm_id state can change dynamically in
> response to an event, and in general should not be accessed. (I.e. the
> current cm_id state may not match up with the event being reported.)
>
> Access to the state requires coordination with the CM code itself, so trying
> to rely on it to drive external states will eventually lead to problems.
> Clients of the CM should either maintain their own state information or
> perform operations based on the occurrence of the reported event.
>
> I had originally exposed the state only for debugging purposes. A comment
> was added that it should only be used for internal/debug use, but I will
> make a note to remove the state from the externally visible cm_id structure.
Makes sense to me. This patch removes the state variable from both
kernel and userspace UCM code. I'll also work on a patch for SDP when
I get a chance, but it should be fairly straigh forward.
-Libor
Index: linux-kernel/infiniband/include/ib_user_cm.h
===================================================================
--- linux-kernel/infiniband/include/ib_user_cm.h (revision 2564)
+++ linux-kernel/infiniband/include/ib_user_cm.h (working copy)
@@ -90,8 +90,6 @@
struct ib_ucm_attr_id_resp {
__u64 service_id;
__u64 service_mask;
- __u32 state;
- __u32 lap_state;
__u32 local_id;
__u32 remote_id;
};
@@ -310,7 +308,6 @@
struct ib_ucm_event_resp {
__u32 id;
- __u32 state;
__u32 event;
__u32 present;
union {
Index: linux-kernel/infiniband/core/ucm.c
===================================================================
--- linux-kernel/infiniband/core/ucm.c (revision 2564)
+++ linux-kernel/infiniband/core/ucm.c (working copy)
@@ -58,7 +58,7 @@
static struct semaphore ctx_id_mutex;
static struct idr ctx_id_table;
-static int ctx_id_rover = 0;
+static int ctx_id_rover = 0;
static struct ib_ucm_context *ib_ucm_ctx_get(int id)
{
@@ -428,7 +428,6 @@
uevent->resp.id = id;
uevent->resp.event = event->event;
- uevent->resp.state = cm_id->state;
result = ib_ucm_event_process(event, uevent);
if (result)
@@ -652,8 +651,6 @@
resp.service_id = ctx->cm_id->service_id;
resp.service_mask = ctx->cm_id->service_mask;
- resp.state = ctx->cm_id->state;
- resp.lap_state = ctx->cm_id->lap_state;
resp.local_id = ctx->cm_id->local_id;
resp.remote_id = ctx->cm_id->remote_id;
Index: userspace/libibcm/include/infiniband/cm.h
===================================================================
--- userspace/libibcm/include/infiniband/cm.h (revision 2521)
+++ userspace/libibcm/include/infiniband/cm.h (working copy)
@@ -39,33 +39,6 @@
#include <infiniband/verbs.h>
#include <infiniband/sa.h>
-enum ib_cm_state {
- IB_CM_IDLE,
- IB_CM_LISTEN,
- IB_CM_REQ_SENT,
- IB_CM_REQ_RCVD,
- IB_CM_MRA_REQ_SENT,
- IB_CM_MRA_REQ_RCVD,
- IB_CM_REP_SENT,
- IB_CM_REP_RCVD,
- IB_CM_MRA_REP_SENT,
- IB_CM_MRA_REP_RCVD,
- IB_CM_ESTABLISHED,
- IB_CM_DREQ_SENT,
- IB_CM_DREQ_RCVD,
- IB_CM_TIMEWAIT,
- IB_CM_SIDR_REQ_SENT,
- IB_CM_SIDR_REQ_RCVD
-};
-
-enum ib_cm_lap_state {
- IB_CM_LAP_IDLE,
- IB_CM_LAP_SENT,
- IB_CM_LAP_RCVD,
- IB_CM_MRA_LAP_SENT,
- IB_CM_MRA_LAP_RCVD,
-};
-
enum ib_cm_event_type {
IB_CM_REQ_ERROR,
IB_CM_REQ_RECEIVED,
@@ -240,7 +213,6 @@
struct ib_cm_event {
uint32_t cm_id;
enum ib_cm_event_type event;
- enum ib_cm_state state;
union {
struct ib_cm_req_event_param req_rcvd;
struct ib_cm_rep_event_param rep_rcvd;
@@ -313,8 +285,6 @@
struct ib_cm_attr_param {
uint64_t service_id;
uint64_t service_mask;
- enum ib_cm_state state;
- enum ib_cm_lap_state lap_state;
uint32_t local_id;
uint32_t remote_id;
};
Index: userspace/libibcm/include/infiniband/cm_abi.h
===================================================================
--- userspace/libibcm/include/infiniband/cm_abi.h (revision 2521)
+++ userspace/libibcm/include/infiniband/cm_abi.h (working copy)
@@ -94,8 +94,6 @@
struct cm_abi_attr_id_resp {
__u64 service_id;
__u64 service_mask;
- __u32 state;
- __u32 lap_state;
__u32 local_id;
__u32 remote_id;
};
@@ -314,7 +312,6 @@
struct cm_abi_event_resp {
__u32 id;
- __u32 state;
__u32 event;
__u32 present;
union {
Index: userspace/libibcm/src/cm.c
===================================================================
--- userspace/libibcm/src/cm.c (revision 2521)
+++ userspace/libibcm/src/cm.c (working copy)
@@ -188,8 +188,6 @@
param->service_id = resp->service_id;
param->service_mask = resp->service_mask;
- param->state = resp->state;
- param->lap_state = resp->lap_state;
param->local_id = resp->local_id;
param->remote_id = resp->remote_id;
@@ -773,7 +771,6 @@
evt->cm_id = resp->id;
evt->event = resp->event;
- evt->state = resp->state;
if (resp->present & CM_ABI_PRES_PRIMARY) {
Index: userspace/libibcm/examples/simple.c
===================================================================
--- userspace/libibcm/examples/simple.c (revision 2521)
+++ userspace/libibcm/examples/simple.c (working copy)
@@ -168,11 +168,10 @@
goto done;
}
- printf("CM ID <%d> Event <%d> State <%d>\n",
- event->cm_id, event->event, event->state);
+ printf("CM ID <%d> Event <%d>\n", event->cm_id, event->event);
- switch (event->state) {
- case IB_CM_REQ_RCVD:
+ switch (event->event) {
+ case IB_CM_REQ_RECEIVED:
result = ib_cm_destroy_id(cm_id);
if (result < 0) {
@@ -205,7 +204,7 @@
}
break;
- case IB_CM_REP_RCVD:
+ case IB_CM_REP_RECEIVED:
result = ib_cm_send_rtu(cm_id, NULL, 0);
if (result < 0) {
@@ -215,7 +214,7 @@
}
break;
- case IB_CM_ESTABLISHED:
+ case IB_CM_RTU_RECEIVED:
result = ib_cm_send_dreq(cm_id, NULL, 0);
if (result < 0) {
@@ -225,7 +224,7 @@
}
break;
- case IB_CM_DREQ_RCVD:
+ case IB_CM_DREQ_RECEIVED:
result = ib_cm_send_drep(cm_id, NULL, 0);
if (result < 0) {
@@ -235,15 +234,14 @@
}
break;
- case IB_CM_TIMEWAIT:
+ case IB_CM_DREP_RECEIVED:
break;
- case IB_CM_IDLE:
+ case IB_CM_TIMEWAIT_EXIT:
status = 1;
break;
default:
status = EINVAL;
- printf("Unhandled state <%d:%d>\n",
- event->state, event->event);
+ printf("Unhandled event <%d>\n", event->event);
break;
}
More information about the general
mailing list