[openib-general] [PATCH] SDP passive connect rework
Libor Michalek
libor at topspin.com
Fri Feb 25 17:41:37 PST 2005
This patch simplifies the passive connection process and reworks
the connection reference counting, to better match the CM. Next step
is the disconnect path.
-Libor
Signed-off-by: Libor Michalek <libor at topspin.com>
Index: sdp_proto.h
===================================================================
--- sdp_proto.h (revision 1922)
+++ sdp_proto.h (working copy)
@@ -120,17 +120,13 @@
int sdp_wall_send_abort(struct sdp_opt *conn);
-int sdp_wall_recv_confirm(struct sdp_opt *conn);
-
-int sdp_wall_recv_failed(struct sdp_opt *conn, int error);
-
int sdp_wall_recv_close(struct sdp_opt *conn);
int sdp_wall_recv_closing(struct sdp_opt *conn);
int sdp_wall_recv_abort(struct sdp_opt *conn);
-int sdp_wall_recv_drop(struct sdp_opt *conn);
+void sdp_wall_recv_drop(struct sdp_opt *conn);
int sdp_wall_abort(struct sdp_opt *conn);
@@ -336,6 +332,10 @@
/*
* passive connect functions
*/
+void sdp_cm_pass_error(struct sdp_opt *conn, int error);
+
+int sdp_cm_pass_establish(struct sdp_opt *conn);
+
int sdp_cm_req_handler(struct ib_cm_id *cm_id,
struct ib_cm_event *event);
Index: sdp_actv.c
===================================================================
--- sdp_actv.c (revision 1922)
+++ sdp_actv.c (working copy)
@@ -104,9 +104,9 @@
}
/*
- * _sdp_actv_conn_establish - process an accepted connection request.
+ * _sdp_cm_actv_establish - process an accepted connection request.
*/
-static int _sdp_actv_conn_establish(struct sdp_opt *conn)
+static int _sdp_cm_actv_establish(struct sdp_opt *conn)
{
struct ib_qp_attr *qp_attr;
int attr_mask = 0;
@@ -275,9 +275,6 @@
{
struct sdp_msg_hello_ack *hello_ack;
int result = -EPROTO;
-
- if (NULL == conn)
- return -EINVAL;
if (cm_id != conn->cm_id) {
sdp_dbg_warn(conn, "REP comm ID mismatch. <%08x:%08x>",
@@ -325,7 +322,7 @@
*/
(void)sdp_buff_pool_put(sdp_buff_q_get_head(&conn->send_post));
- result = _sdp_actv_conn_establish(conn);
+ result = _sdp_cm_actv_establish(conn);
if (0 > result) {
sdp_dbg_warn(conn, "Error <%d> accept receive failed", result);
goto error;
Index: sdp_conn.c
===================================================================
--- sdp_conn.c (revision 1922)
+++ sdp_conn.c (working copy)
@@ -99,7 +99,7 @@
/*
* up ref until we release. One ref for GW and one for INET.
*/
- sdp_conn_hold(accept_conn); /* INET reference */
+ sdp_conn_hold(accept_conn); /* AcceptQueue INET reference */
return 0;
}
Index: sdp_wall.c
===================================================================
--- sdp_wall.c (revision 1922)
+++ sdp_wall.c (working copy)
@@ -227,6 +227,8 @@
* instead of canceling allow the path completion to
* determine that the socket has moved to an error state.
*/
+ SDP_CONN_ST_SET(conn, SDP_CONN_ST_CLOSED);
+ break;
case SDP_CONN_ST_REQ_SENT:
case SDP_CONN_ST_REP_SENT:
/*
@@ -272,103 +274,6 @@
*/
/*
- * sdp_wall_recv_confirm - callback to confirm accepeted passive open
- */
-int sdp_wall_recv_confirm(struct sdp_opt *conn)
-{
- struct ib_qp_attr *qp_attr;
- int attr_mask = 0;
- int result = 0;
-
- sdp_dbg_ctrl(conn, "Passive Establish src <%08x:%04x> dst <%08x:%04x>",
- conn->src_addr, conn->src_port,
- conn->dst_addr, conn->dst_port);
-
- qp_attr = kmalloc(sizeof(*qp_attr), GFP_KERNEL);
- if (!qp_attr) {
- result = -ENOMEM;
- goto error;
- }
-
- memset(qp_attr, 0, sizeof(*qp_attr));
-
- qp_attr->qp_state = IB_QPS_RTS;
-
- result = ib_cm_init_qp_attr(conn->cm_id, qp_attr, &attr_mask);
- if (result) {
- sdp_dbg_warn(conn, "Error <%d> QP attributes for RTS",
- result);
- goto error;
- }
-
- result = ib_modify_qp(conn->qp, qp_attr, attr_mask);
- kfree(qp_attr);
- if (result)
- goto error;
-
- switch (conn->istate) {
- case SDP_SOCK_ST_ACCEPTING:
- conn->istate = SDP_SOCK_ST_ACCEPTED;
- conn->send_buf = SDP_INET_SEND_SIZE;
-
- break;
- case SDP_SOCK_ST_ACCEPTED:
- conn->istate = SDP_SOCK_ST_ESTABLISHED;
- conn->send_buf = SDP_INET_SEND_SIZE;
-
- sdp_inet_wake_send(conn->sk);
-
- break;
- default:
- result = -EPROTO;
- goto error;
- }
-
- return 0;
-error:
- conn->istate = SDP_SOCK_ST_ERROR;
-
- sdp_inet_wake_error(conn->sk);
-
- sdp_conn_put(conn); /* CM sk reference */
-
- return result;
-}
-
-/*
- * sdp_wall_recv_failed - callback to notify accepted open of a failure
- */
-int sdp_wall_recv_failed(struct sdp_opt *conn, int error)
-{
- sdp_dbg_ctrl(conn, "Failed recv. src <%08x:%04x> dst <%08x:%04x> <%d>",
- conn->src_addr, conn->src_port,
- conn->dst_addr, conn->dst_port, error);
- /*
- * the connection has failed, move to error, and notify anyone
- * waiting of the state change.
- */
- SDP_EXPECT((SDP_SOCK_ST_ACCEPTED == conn->istate));
-
- switch (conn->istate) {
- default:
- SDP_CONN_SET_ERR(conn, error);
- conn->sk->sk_socket->state = SS_UNCONNECTED;
- sdp_inet_wake_error(conn->sk);
- /*
- * fall through
- */
- case SDP_SOCK_ST_ACCEPTING:
- conn->istate = SDP_SOCK_ST_ERROR;
- conn->shutdown = SHUTDOWN_MASK;
- break;
- }
-
- sdp_conn_put(conn); /* CM reference */
-
- return 0;
-}
-
-/*
* sdp_wall_recv_close - callback to accept an active close
*/
int sdp_wall_recv_close(struct sdp_opt *conn)
@@ -482,20 +387,16 @@
/*
* sdp_wall_recv_drop - drop SDP protocol reference to socket
*/
-int sdp_wall_recv_drop(struct sdp_opt *conn)
+void sdp_wall_recv_drop(struct sdp_opt *conn)
{
int result;
sdp_dbg_ctrl(conn, "Drop. src <%08x:%04x> dst <%08x:%04x>",
conn->src_addr, conn->src_port,
conn->dst_addr, conn->dst_port);
- /*
- * Lock Note:
- * This function should only be called from the process context,
- * because AcceptQueueRemove will take the process context connection
- * lock. Generally this function is only called as a result of CM
- * state transitions.
- */
+
+ conn->send_buf = 0;
+
switch (conn->istate) {
case SDP_SOCK_ST_ACCEPTING:
/*
@@ -533,11 +434,7 @@
break;
}
- conn->send_buf = 0;
-
- sdp_conn_put(conn); /* CM sk reference */
-
- return 0;
+ return;
}
/*
Index: sdp_pass.c
===================================================================
--- sdp_pass.c (revision 1922)
+++ sdp_pass.c (working copy)
@@ -35,8 +35,116 @@
#include "sdp_main.h"
/*
- * _sdp_cm_accept - respond to a connection attempt with an ack
+ * handle incomming passive connection error. (REJ)
*/
+void sdp_cm_pass_error(struct sdp_opt *conn, int error)
+{
+ int result;
+
+ sdp_dbg_ctrl(conn,
+ "passive error. src <%08x:%04x> dst <%08x:%04x> <%d>",
+ conn->src_addr, conn->src_port,
+ conn->dst_addr, conn->dst_port, error);
+ /*
+ * the connection has failed, move to error, and notify anyone
+ * waiting of the state change. remove connection from listen
+ * queue if possible
+ */
+ result = sdp_inet_accept_q_remove(conn);
+ if (!result)
+ sdp_conn_put(conn); /* AcceptQueue INET reference */
+
+ SDP_CONN_SET_ERR(conn, error);
+ conn->istate = SDP_SOCK_ST_ERROR;
+ conn->shutdown = SHUTDOWN_MASK;
+ conn->send_buf = 0;
+
+ if (conn->sk->sk_socket)
+ conn->sk->sk_socket->state = SS_UNCONNECTED;
+
+ sdp_iocb_q_cancel_all(conn, (0 - error));
+ sdp_inet_wake_error(conn->sk);
+}
+
+/*
+ * handle incomming passive connection establishment. (RTU)
+ */
+int sdp_cm_pass_establish(struct sdp_opt *conn)
+{
+ struct ib_qp_attr *qp_attr;
+ int attr_mask = 0;
+ int result = 0;
+
+ sdp_dbg_ctrl(conn, "Passive Establish src <%08x:%04x> dst <%08x:%04x>",
+ conn->src_addr, conn->src_port,
+ conn->dst_addr, conn->dst_port);
+ /*
+ * free hello ack message
+ */
+ (void)sdp_buff_pool_put(sdp_buff_q_get_head(&conn->send_post));
+
+ SDP_CONN_ST_SET(conn, SDP_CONN_ST_ESTABLISHED);
+
+ qp_attr = kmalloc(sizeof(*qp_attr), GFP_KERNEL);
+ if (!qp_attr) {
+ result = -ENOMEM;
+ goto done;
+ }
+
+ memset(qp_attr, 0, sizeof(*qp_attr));
+ qp_attr->qp_state = IB_QPS_RTS;
+
+ result = ib_cm_init_qp_attr(conn->cm_id, qp_attr, &attr_mask);
+ if (result) {
+ sdp_dbg_warn(conn, "Error <%d> QP attributes for RTS",
+ result);
+ goto error;
+ }
+
+ result = ib_modify_qp(conn->qp, qp_attr, attr_mask);
+ if (result) {
+ sdp_dbg_warn(conn, "Error <%d> modifiying QP to RTS", result);
+ goto error;
+ }
+
+ conn->send_buf = SDP_INET_SEND_SIZE;
+
+ switch (conn->istate) {
+ case SDP_SOCK_ST_ACCEPTING:
+ conn->istate = SDP_SOCK_ST_ACCEPTED;
+ break;
+ case SDP_SOCK_ST_ACCEPTED:
+ conn->istate = SDP_SOCK_ST_ESTABLISHED;
+ break;
+ }
+
+ result = sdp_send_flush(conn);
+ if (0 > result) {
+ sdp_dbg_warn(conn, "Error <%d> flushing sends.", result);
+ goto error;
+ }
+
+ result = sdp_recv_flush(conn);
+ if (0 > result) {
+ sdp_dbg_warn(conn, "Error <%d> flushing receives.", result);
+ goto error;
+ }
+
+ sdp_inet_wake_send(conn->sk);
+
+ kfree(qp_attr);
+ return 0;
+error:
+ kfree(qp_attr);
+done:
+ sdp_cm_pass_error(conn, (0 - result));
+ SDP_CONN_ST_SET(conn, SDP_CONN_ST_ERROR_STRM);
+
+ return result;
+}
+/*
+ * Functions to handle incomming passive connection requests. (REQ)
+ */
static int _sdp_cm_accept(struct sdp_opt *conn)
{
struct ib_cm_rep_param param;
@@ -46,9 +154,9 @@
int qp_mask = 0;
int result;
int expect;
-
/*
- * build listen response headers
+ * Accept connection, build listen response headers and send
+ * a REP message to remote peer.
*/
if (sizeof(struct sdp_msg_hello_ack) > conn->recv_size) {
sdp_dbg_warn(conn, "buffer size <%d> too small. <%Zu>",
@@ -180,16 +288,16 @@
return result;
}
-/*
- * _sdp_cm_listen_lookup - match a listener with an incomming conn
- */
static int _sdp_cm_listen_lookup(struct sdp_opt *conn)
{
struct sdp_opt *listen_conn;
struct sock *listen_sk;
struct sock *sk;
int result;
-
+ /*
+ * match a listener with an incomming conn, and generate
+ * accept message and state on success.
+ */
sdp_dbg_ctrl(conn,
"listen match for conn. src <%08x:%04x> dst <%08x:%04x>",
conn->src_addr, conn->src_port,
@@ -296,9 +404,6 @@
return result;
}
-/*
- * _sdp_cm_hello_check - validate the hello header
- */
static int _sdp_cm_hello_check(struct sdp_msg_hello *msg_hello)
{
/*
@@ -365,9 +470,6 @@
return 0; /* success */
}
-/*
- * sdp_cm_req_handler - handler for passive connection open completion
- */
int sdp_cm_req_handler(struct ib_cm_id *cm_id, struct ib_cm_event *event)
{
struct sdp_msg_hello *msg_hello = event->private_data;
@@ -469,9 +571,13 @@
sdp_conn_unlock(conn);
return 0;
error:
+ SDP_CONN_ST_SET(conn, SDP_CONN_ST_CLOSED);
conn->cm_id = NULL; /* cm_id destroyed by CM on error result. */
sdp_conn_unlock(conn);
- sdp_conn_put(conn); /* CM sk reference, will destory */
+ sdp_conn_put(conn); /* CM reference */
done:
+ (void)ib_send_cm_rej(cm_id,
+ IB_CM_REJ_CONSUMER_DEFINED,
+ NULL, 0, NULL, 0);
return result;
}
Index: sdp_event.c
===================================================================
--- sdp_event.c (revision 1922)
+++ sdp_event.c (working copy)
@@ -198,56 +198,26 @@
* Connection establishment IB/CM callback functions
*/
-/*
- * _sdp_cm_idle - handler for connection idle completion
- */
static int _sdp_cm_idle(struct ib_cm_id *cm_id,
struct ib_cm_event *event,
struct sdp_opt *conn)
{
- int result = 0;
- int expect;
-
- if (NULL == conn)
- return -EINVAL;
- /*
- * IDLE should only be called after some other action on the comm_id,
- * which means the callback argument will be a SDP conn, since the
- * only time it is not a SDP conn is the first callback in a passive
- * open.
- */
sdp_dbg_ctrl(conn, "CM IDLE. commID <%08x> event <%d> status <%d>",
cm_id->local_id, event->event, event->param.send_status);
/*
* check state
*/
switch (conn->state) {
- case SDP_CONN_ST_REQ_SENT: /* active open, Hello msg sent */
+ case SDP_CONN_ST_REQ_SENT:
conn->state = SDP_CONN_ST_CLOSED;
sdp_cm_actv_error(conn, ECONNREFUSED);
- sdp_conn_put(conn); /* CM reference */
-
break;
- case SDP_CONN_ST_REP_SENT: /* passive open, Hello ack msg sent */
- result = sdp_wall_recv_failed(conn, ECONNREFUSED);
- if (0 > result) {
+ case SDP_CONN_ST_REP_SENT:
+ conn->state = SDP_CONN_ST_CLOSED;
- sdp_dbg_warn(conn, "Error <%d> receiving CM failed",
- result);
- goto error;
- }
-
+ sdp_cm_pass_error(conn, ECONNREFUSED);
break;
- case SDP_CONN_ST_REQ_RECV: /* passive open, Hello msg recv'd */
- /*
- * connection state is outstanding to the gateway, so the
- * connection cannot be destroyed, until the gateway responds.
- * The connection is marked for destruction. No return error,
- * since this is the last CM callback.
- */
- SDP_CONN_ST_SET(conn, SDP_CONN_ST_ERROR_CM);
- break;
case SDP_CONN_ST_TIME_WAIT_1:
case SDP_CONN_ST_ESTABLISHED:
sdp_dbg_warn(conn, "Unexpected connection state");
@@ -259,44 +229,24 @@
/*
* Connection is finally dead. Drop the CM reference
*/
- result = sdp_wall_recv_drop(conn);
- SDP_EXPECT(!(0 > result));
-
+ sdp_wall_recv_drop(conn);
break;
default:
sdp_warn("Unknown conn state. conn <%d> state <%04x:%04x>",
conn->hashent, conn->istate, conn->state);
- result = -EINVAL;
- goto error;
break;
}
+ sdp_conn_put(conn); /* CM reference */
return 0;
-error:
- /*
- * failed to notify INET of SDP termination of this connection,
- * last attempt to drop the CM reference.
- */
- expect = sdp_wall_recv_drop(conn);
- SDP_EXPECT(!(0 > expect));
-
- return result;
}
-/*
- * _sdp_cm_established - handler for connection established completion
- */
static int _sdp_cm_established(struct ib_cm_id *cm_id,
struct ib_cm_event *event,
struct sdp_opt *conn)
{
int result = 0;
- int expect;
- struct sdpc_buff *buff;
- if (NULL == conn)
- return -EINVAL;
-
sdp_dbg_ctrl(conn, "CM ESTABLISHED. commID <%08x>", cm_id->local_id);
/*
* release disconnects.
@@ -306,49 +256,27 @@
* check state
*/
switch (conn->state) {
- case SDP_CONN_ST_REP_SENT: /* passive open, Hello ack msg sent */
- buff = sdp_buff_q_get_head(&conn->send_post);
- if (NULL == buff)
- sdp_dbg_warn(conn, "hello ack missing in send pool");
- else
- (void)sdp_buff_pool_put(buff);
-
- SDP_CONN_ST_SET(conn, SDP_CONN_ST_ESTABLISHED);
-
- result = sdp_wall_recv_confirm(conn);
+ case SDP_CONN_ST_REP_SENT:
+ result = sdp_cm_pass_establish(conn);
+ if (!result)
+ break;
+ /*
+ * on error fall through to disconnect
+ */
+ case SDP_CONN_ST_ERROR_STRM:
+ /*
+ * Begin abortive disconnect.
+ * Leave state unchanged, time_wait and idle will handle the
+ * existing state correctly.
+ */
+ result = ib_send_cm_dreq(conn->cm_id, NULL, 0);
if (0 > result) {
- sdp_dbg_warn(conn, "Error <%d> confirming conn state",
+ sdp_dbg_warn(NULL, "Error <%d> sending CM DREQ",
result);
- /*
- * CM sk reference released by Confirm
- * error processing
- */
goto done;
}
- result = sdp_send_flush(conn);
- if (0 > result) {
- sdp_dbg_warn(conn, "Error <%d> flushing send queue.",
- result);
- goto error;
- }
-
- result = sdp_recv_flush(conn);
- if (0 > result) {
- sdp_dbg_warn(conn, "Error <%d> flushing receives.",
- result);
- goto error;
- }
-
break;
- case SDP_CONN_ST_RTU_SENT: /* TODO: no longer approriate. */
- /*
- * Set state.
- */
- SDP_CONN_ST_SET(conn, SDP_CONN_ST_ESTABLISHED);
- /*
- * fall through
- */
case SDP_CONN_ST_DIS_PEND_1:
/* active open, and active close */
case SDP_CONN_ST_DIS_PEND_R:
@@ -362,18 +290,6 @@
}
break;
- case SDP_CONN_ST_ERROR_STRM:
- /*
- * Sockets has released reference, begin abortive disconnect.
- * Leave state unchanged, time_wait and idle will handle the
- * existing state correctly.
- */
- result = sdp_cm_disconnect(conn);
- if (0 > result)
- sdp_dbg_warn(conn, "Error <%d> posting CM disconnect",
- result);
-
- break;
default:
sdp_warn("Unexpected conn state. conn <%d> state <%04x:%04x>",
@@ -385,10 +301,10 @@
return 0;
error:
- expect = sdp_wall_recv_drop(conn);
- SDP_EXPECT(!(0 > expect));
+ sdp_wall_recv_drop(conn);
done:
conn->cm_id = NULL;
+ sdp_conn_put(conn); /* CM reference */
return result;
}
@@ -400,11 +316,7 @@
struct sdp_opt *conn)
{
int result = 0;
- int expect;
- if (NULL == conn)
- return -EINVAL;
-
sdp_dbg_ctrl(conn, "CM TIME WAIT. commID <%08x> event <%d>",
cm_id->local_id, event->event);
/*
@@ -468,22 +380,17 @@
return 0;
error:
- expect = sdp_wall_recv_drop(conn);
- SDP_EXPECT(!(0 > expect));
+ sdp_wall_recv_drop(conn);
conn->cm_id = NULL;
+ sdp_conn_put(conn); /* CM reference */
return result;
}
/*
* Primary Connection Managment callback function
*/
-
-/*
- * sdp_cm_event_handler - handler for CM state transitions request
- */
-int sdp_cm_event_handler(struct ib_cm_id *cm_id,
- struct ib_cm_event *event)
+int sdp_cm_event_handler(struct ib_cm_id *cm_id, struct ib_cm_event *event)
{
s32 hashent = (unsigned long)cm_id->context;
struct sdp_opt *conn = NULL;
@@ -498,10 +405,12 @@
if (NULL != conn)
sdp_conn_lock(conn);
else
- if (IB_CM_REQ_RCVD != cm_id->state)
+ if (IB_CM_REQ_RCVD != cm_id->state) {
sdp_dbg_warn(NULL,
"No conn <%d> CM state <%d> event <%d>",
hashent, cm_id->state, event->event);
+ return -EINVAL;
+ }
switch (cm_id->state) {
case IB_CM_REQ_RCVD:
@@ -544,3 +453,7 @@
return result;
}
+
+
+
+
More information about the general
mailing list