[openib-general] [PATCH][SDP] CM disconnect.

Libor Michalek libor at topspin.com
Mon Feb 28 17:55:34 PST 2005


  Fixup and simplify CM disconnect interface, and in the process get
rid of sdp_post.c. Plus convert a few more functions from int to void
return. 

  With this patch all the major SDP/CM code paths should be working
correctly, including connection cleanup. So if you see connections
that are not getting cleaned up, let me know.

  There's still more to be done to simplify the connection state
transitions. Specifically merge conn->istate and conn->state since
there's alot of redundant information between the two, and checking
for divergence between the two which cannot occur.

 9 files changed, 83 insertions(+), 241 deletions(-)

-Libor

Signed-off-by: Libor Michalek <libor at topspin.com>

Index: Makefile
===================================================================
--- Makefile	(revision 1931)
+++ Makefile	(working copy)
@@ -20,7 +20,6 @@
 	sdp_send.o \
 	sdp_recv.o \
 	sdp_event.o \
-	sdp_post.o \
 	sdp_pass.o \
 	sdp_actv.o \
 	sdp_link.o
Index: sdp_proto.h
===================================================================
--- sdp_proto.h	(revision 1931)
+++ sdp_proto.h	(working copy)
@@ -342,14 +342,6 @@
 /*
  * post functions
  */
-int sdp_cm_listen_start(struct sdev_root *dev_root);
-
-int sdp_cm_listen_stop(struct sdev_root *dev_root);
-
-int sdp_cm_disconnect(struct sdp_opt *conn);
-
-int sdp_cm_reject(struct sdp_opt *conn);
-
 int sdp_recv_flush(struct sdp_opt *conn);
 
 int sdp_send_flush(struct sdp_opt *conn);
@@ -619,7 +611,7 @@
 /*
  * sdp_conn_state_dump - dump state information to the log
  */
-static inline int sdp_conn_state_dump(struct sdp_opt *conn)
+static inline void sdp_conn_state_dump(struct sdp_opt *conn)
 {
 #ifdef _SDP_CONN_STATE_REC
 	int counter;
@@ -629,7 +621,7 @@
 	if (conn->state_rec.state[0] == SDP_CONN_ST_INVALID) {
 		sdp_dbg_init("STATE:   No state history. <%d>",
 			     conn->state_rec.value);
-		return 0;
+		return;
 	}
 
 	for (counter = 0;
@@ -641,7 +633,6 @@
 			     conn->state_rec.file[counter],
 			     conn->state_rec.line[counter]);
 #endif
-	return 0;
 }
 
 #endif /* _SDP_PROTO_H */
Index: sdp_rcvd.c
===================================================================
--- sdp_rcvd.c	(revision 1931)
+++ sdp_rcvd.c	(working copy)
@@ -87,13 +87,14 @@
 		/*
 		 * Begin IB/CM disconnect
 		 */
-		result = sdp_cm_disconnect(conn);
+		result = ib_send_cm_dreq(conn->cm_id, NULL, 0);
 		/*
 		 * if the remote DREQ was already received, but unprocessed, 
 		 * do not treat it as an error
 		 */
-		if (result < 0) {
-			sdp_dbg_warn(conn, "Error <%d> CM disconnect", result);
+		if (result) {
+			sdp_dbg_warn(conn, "Error <%d> sending CM DREQ",
+				     result);
 
 			if (result != -EPROTO)
 				goto error;
Index: sdp_sent.c
===================================================================
--- sdp_sent.c	(revision 1931)
+++ sdp_sent.c	(working copy)
@@ -65,8 +65,8 @@
 		/*
 		 * Begin IB/CM disconnect
 		 */
-		result = sdp_cm_disconnect(conn);
-		if (result < 0) {
+		result = ib_send_cm_dreq(conn->cm_id, NULL, 0);
+		if (result) {
 			sdp_dbg_warn(conn, "Error <%d> posting CM disconnect",
 				     result);
 			goto error;
@@ -92,13 +92,14 @@
 		/*
 		 * Begin IB/CM disconnect
 		 */
-		result = sdp_cm_disconnect(conn);
+		result = ib_send_cm_dreq(conn->cm_id, NULL, 0);
 		/*
 		 * if the remote DREQ was already received, but unprocessed, do
 		 * not treat it as an error
 		 */
-		if (result < 0)
-			sdp_dbg_warn(conn, "Error <%d> CM disconnect", result);
+		if (result)
+			sdp_dbg_warn(conn, "Error <%d> sending CM DREQ",
+				     result);
 
 		break;
 	default:
@@ -126,9 +127,9 @@
 	SDP_EXPECT((conn->state == SDP_CONN_ST_ERROR_STRM));
 	SDP_CONN_ST_SET(conn, SDP_CONN_ST_ERROR_STRM);
 
-	result = sdp_cm_disconnect(conn);
-	if (result < 0)
-		sdp_dbg_warn(conn, "Error <%d> CM disconnect", result);
+	result = ib_send_cm_dreq(conn->cm_id, NULL, 0);
+	if (result)
+		sdp_dbg_warn(conn, "Error <%d> sending CM DREQ", result);
 
 	return result;
 }
Index: sdp_wall.c
===================================================================
--- sdp_wall.c	(revision 1931)
+++ sdp_wall.c	(working copy)
@@ -43,7 +43,6 @@
  */
 int sdp_wall_send_close(struct sdp_opt *conn)
 {
-	struct sdpc_buff *buff;
 	int result;
 
 	sdp_dbg_ctrl(conn, "Close send. src <%08x:%04x> dst <%08x:%04x>",
@@ -61,13 +60,7 @@
 		/*
 		 * clear out the sent HelloAck message
 		 */
-		buff = sdp_buff_q_get_head(&conn->send_post);
-		if (!buff)
-			sdp_dbg_warn(conn, "Error, hello ack missing.");
-		else {
-			result = sdp_buff_pool_put(buff);
-			SDP_EXPECT(result >= 0);
-		}
+		(void)sdp_buff_pool_put(sdp_buff_q_get_head(&conn->send_post));
 		/*
 		 * fall through
 		 */
@@ -105,9 +98,7 @@
 	/*
 	 * abortive close.
 	 */
-	SDP_CONN_ST_SET(conn, SDP_CONN_ST_ERROR_STRM);
-
-	result = sdp_cm_disconnect(conn);
+	result = sdp_wall_send_abort(conn);
 	if (result < 0)
 		sdp_dbg_warn(conn, "Error <%d> CM disconnect send", result);
 
@@ -157,9 +148,7 @@
 	/*
 	 * abortive close.
 	 */
-	SDP_CONN_ST_SET(conn, SDP_CONN_ST_ERROR_STRM);
-
-	result = sdp_cm_disconnect(conn);
+	result = sdp_wall_send_abort(conn);
 	if (result < 0)
 		sdp_dbg_warn(conn, "Error <%d> CM disconnect send", result);
 
@@ -262,8 +251,8 @@
 	 */
 	SDP_CONN_ST_SET(conn, SDP_CONN_ST_ERROR_STRM);
 
-	result = sdp_cm_disconnect(conn);
-	if (result < 0)
+	result = ib_send_cm_dreq(conn->cm_id, NULL, 0);
+	if (result)
 		sdp_dbg_warn(conn, "Error <%d> CM disconnect send", result);
 
 	return 0;
Index: sdp_conn.h
===================================================================
--- sdp_conn.h	(revision 1931)
+++ sdp_conn.h	(working copy)
@@ -465,8 +465,8 @@
 extern void sdp_conn_internal_lock(struct sdp_opt *conn, unsigned long *flags);
 extern void sdp_conn_internal_unlock(struct sdp_opt *conn);
 extern void sdp_conn_relock(struct sdp_opt *conn);
+extern void sdp_conn_destruct(struct sdp_opt *conn);
 extern int sdp_conn_cq_drain(struct ib_cq *cq, struct sdp_opt *conn);
-extern int sdp_conn_destruct(struct sdp_opt *conn);
 
 #define SDP_CONN_LOCK_IRQ(conn, flags) \
         spin_lock_irqsave(&((conn)->lock.slock), flags)
@@ -515,7 +515,7 @@
 static inline void sdp_conn_put(struct sdp_opt *conn)
 {
 	if (atomic_dec_and_test(&conn->refcnt))
-		(void)sdp_conn_destruct(conn);
+		sdp_conn_destruct(conn);
 }
 
 /*
Index: sdp_conn.c
===================================================================
--- sdp_conn.c	(revision 1931)
+++ sdp_conn.c	(working copy)
@@ -646,17 +646,11 @@
 /*
  * sdp_conn_destruct - final destructor for connection.
  */
-int sdp_conn_destruct(struct sdp_opt *conn)
+void sdp_conn_destruct(struct sdp_opt *conn)
 {
-	int result = 0;
 	int dump = 0;
+	int result;
 
-	if (!conn) {
-		sdp_dbg_warn(NULL, "sk destruct, no connection!");
-		result = -EINVAL;
-		goto done;
-	}
-
 	sdp_dbg_ctrl(conn, "DESTRUCT. <%08x:%04x> <%08x:%04x> <%u:%u>",
 		     conn->src_addr, conn->src_port,
 		     conn->dst_addr, conn->dst_port,
@@ -746,7 +740,7 @@
 	 * in case CM/IB are still tracking this connection.
 	 */
 	if (conn->cm_id) {
-		sdp_dbg_ctrl(conn, "CM local id <%d>", conn->cm_id->local_id);
+		sdp_dbg_warn(conn, "destroying CM ID.");
 
 		result = ib_destroy_cm_id(conn->cm_id);
 		if (result)
@@ -759,6 +753,9 @@
 	if (atomic_read(&conn->refcnt) < 0)
 		sdp_dbg_warn(conn, "destruct low ref count <%04x>",
 			     atomic_read(&conn->refcnt));
+
+	if (dump)
+		sdp_conn_state_dump(conn);
 	/*
 	 * free the OS socket structure
 	 */
@@ -770,16 +767,6 @@
 	}
 
 	kmem_cache_free(dev_root_s.conn_cache, conn);
-
-	result = 0;
-done:
-
-	if (dump) {
-		result = sdp_conn_state_dump(conn);
-		SDP_EXPECT(result >= 0);
-	}
-
-	return result;
 }
 
 /*
@@ -1995,15 +1982,29 @@
 	/*
 	 * start listening
 	 */
-	result = sdp_cm_listen_start(&dev_root_s);
-	if (result < 0) {
-		sdp_warn("Error <%d> listening for connections on HCA.",
-			 result);
+	dev_root_s.listen_id = ib_create_cm_id(sdp_cm_event_handler,
+					       (void *)SDP_DEV_SK_INVALID);
+	if (!dev_root_s.listen_id) {
+		sdp_warn("Failed to create listen connection identifier.");
+		result = -ENOMEM;
+		goto error_cm_id;
+	}
+
+	result = ib_cm_listen(dev_root_s.listen_id,
+		              cpu_to_be64(SDP_MSG_SERVICE_ID_VALUE),
+			      cpu_to_be64(SDP_MSG_SERVICE_ID_MASK));
+	if (result) {
+		sdp_warn("Error <%d> listening for SDP connections", result);
 		goto error_listen;
+
 	}
 
+	sdp_dbg_init("Started listening for SDP connection requests");
+
 	return 0;
 error_listen:
+	(void)ib_destroy_cm_id(dev_root_s.listen_id);
+error_cm_id:
 	kmem_cache_destroy(dev_root_s.sock_cache);
 error_sock:
 	kmem_cache_destroy(dev_root_s.conn_cache);
@@ -2044,6 +2045,10 @@
 	dev_root_s.sk_array--;
 	free_pages((unsigned long)dev_root_s.sk_array, dev_root_s.sk_ordr);
 	/*
+	 * delete IOCB table
+	 */
+	sdp_main_iocb_cleanup();
+	/*
 	 * delete conn cache
 	 */
 	kmem_cache_destroy(dev_root_s.conn_cache);
@@ -2054,11 +2059,7 @@
 	/*
 	 * stop listening
 	 */
-	(void)sdp_cm_listen_stop(&dev_root_s);
-	/*
-	 * delete IOCB table
-	 */
-	sdp_main_iocb_cleanup();
+	(void)ib_destroy_cm_id(dev_root_s.listen_id);
 
 	return 0;
 }
Index: sdp_event.c
===================================================================
--- sdp_event.c	(revision 1931)
+++ sdp_event.c	(working copy)
@@ -236,11 +236,13 @@
 		break;
 	}
 
+	conn->cm_id = NULL;
 	sdp_conn_put(conn); /* CM reference */
-	return 0;
+	return -ENOENT;     /* ensure CM cleans-up identifier. */
 }
 
-static int sdp_cm_established(struct ib_cm_id *cm_id, struct ib_cm_event *event,
+static int sdp_cm_established(struct ib_cm_id *cm_id,
+			      struct ib_cm_event *event,
 			      struct sdp_opt *conn)
 {
 	int result = 0;
@@ -306,6 +308,28 @@
 	return result;
 }
 
+static int sdp_cm_dreq_rcvd(struct ib_cm_id *cm_id, struct ib_cm_event *event,
+			    struct sdp_opt *conn)
+{
+	int result = 0;
+
+	sdp_dbg_ctrl(conn, "CM DREQ RCVD. commID <%08x> event <%d>",
+		     cm_id->local_id, event->event);
+	/*
+	 * Respond with a DREP.
+	 */
+	result = ib_send_cm_drep(conn->cm_id, NULL, 0);
+	if (result) {
+		sdp_dbg_warn(conn, "Error <%d> sending CM DREP", result);
+
+		sdp_wall_recv_drop(conn);
+
+		conn->cm_id = NULL;
+		sdp_conn_put(conn);	/* CM reference */
+	}
+
+	return result;
+}
 /*
  * sdp_cm_timewait - handler for connection Time Wait completion
  */
@@ -326,7 +350,7 @@
 			       conn->ca->dma_device,
 			       PCI_DMA_FROMDEVICE);
 	/*
-	 * check state
+	 * process state changes.
 	 */
 	switch (conn->state) {
 	case SDP_CONN_ST_ERROR_STRM:
@@ -423,6 +447,10 @@
 		result = sdp_cm_established(cm_id, event, conn);
 		break;
 	case IB_CM_DREQ_RCVD:
+		result = sdp_cm_dreq_rcvd(cm_id, event, conn);
+		if (result)
+			break;
+		/* fall through on success to handle state transition */
 	case IB_CM_TIMEWAIT:
 		result = sdp_cm_timewait(cm_id, event, conn);
 		break;
@@ -441,7 +469,7 @@
 			/*
 			 * dump connection state if it is being recorded.
 			 */
-			(void)sdp_conn_state_dump(conn);
+			sdp_conn_state_dump(conn);
 		}
 
 		sdp_conn_unlock(conn);
Index: sdp_post.c
===================================================================
--- sdp_post.c	(revision 1931)
+++ sdp_post.c	(working copy)
@@ -1,168 +0,0 @@
-/*
- * Copyright (c) 2005 Topspin Communications.  All rights reserved.
- *
- * This software is available to you under a choice of one of two
- * licenses.  You may choose to be licensed under the terms of the GNU
- * General Public License (GPL) Version 2, available from the file
- * COPYING in the main directory of this source tree, or the
- * OpenIB.org BSD license below:
- *
- *     Redistribution and use in source and binary forms, with or
- *     without modification, are permitted provided that the following
- *     conditions are met:
- *
- *      - Redistributions of source code must retain the above
- *        copyright notice, this list of conditions and the following
- *        disclaimer.
- *
- *      - Redistributions in binary form must reproduce the above
- *        copyright notice, this list of conditions and the following
- *        disclaimer in the documentation and/or other materials
- *        provided with the distribution.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
- * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
- * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
- * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
- * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
- * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
- * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
- * SOFTWARE.
- *
- * $Id$
- */
-
-#include "sdp_main.h"
-
-/*
- * sdp_cm_listen_start - start listening on all possible socket ports
- */
-int sdp_cm_listen_start(struct sdev_root *dev_root)
-{
-	int result = 0;
-
-	if (dev_root->listen_id)
-		sdp_dbg_warn(NULL, "Already listening for connections.");
-
-	dev_root->listen_id = ib_create_cm_id(sdp_cm_event_handler,
-					      (void *)SDP_DEV_SK_INVALID);
-	if (!dev_root->listen_id)
-		return -ENOMEM;
-	/*
-	 * start listening
-	 */
-	result = ib_cm_listen(dev_root->listen_id,
-		              cpu_to_be64(SDP_MSG_SERVICE_ID_VALUE),
-			      cpu_to_be64(SDP_MSG_SERVICE_ID_MASK));
-	if (result < 0) {
-		sdp_dbg_warn(NULL, "Error <%d> listening for SDP connections",
-			     result);
-
-		(void)ib_destroy_cm_id(dev_root->listen_id);
-		dev_root->listen_id = NULL;
-	} else
-		sdp_dbg_init("Started listening for SDP connection requests");
-
-	return result;
-}
-
-/*
- * sdp_cm_listen_stop - stop listening on all possible socket ports
- */
-int sdp_cm_listen_stop(struct sdev_root *dev_root)
-{
-	int result = 0;
-
-	result = ib_destroy_cm_id(dev_root->listen_id);
-	if (result < 0)
-		sdp_dbg_warn(NULL, "Error <%d> stopping listen", result);
-	else
-		sdp_dbg_init("Stopped listening for SDP connections");
-
-	dev_root->listen_id = NULL;
-
-	return result;
-}
-
-/*
- * do_cm_disconnect - initiate a disconnect request using the CM
- */
-static void do_cm_disconnect(void *arg)
-{
-	struct sdp_opt *conn = (struct sdp_opt *)arg;
-	int result;
-	
-	if (!conn) {
-		sdp_dbg_warn(NULL, "Error, posting disconnect for NULL conn");
-		return;
-	}
-
-	sdp_dbg_ctrl(conn, "Defered disconnect <%08x>", conn->cm_id->local_id);
-	/*
-	 * send a disconnect request using the connection manager
-	 */
-	result = ib_send_cm_dreq(conn->cm_id, NULL, 0);
-	if (result < 0)
-		sdp_dbg_warn(NULL, "Error <%d> CM disconnect request", result);
-
-	sdp_conn_put(conn);
-}
-
-/*
- * do_cm_reject - initiate a reject request using the CM
- */
-static void do_cm_reject(void *arg)
-{
-	struct sdp_opt *conn = (struct sdp_opt *)arg;
-	int result;
-
-	if (!conn)
-		sdp_dbg_warn(NULL, "Error, posting reject for NULL conn");
-
-	sdp_dbg_ctrl(conn, "Defered reject <%08x>", conn->cm_id->local_id);
-	/*
-	 * send a reject request using the connection manager
-	 */
-	result = ib_send_cm_rej(conn->cm_id, 
-				IB_CM_REJ_CONSUMER_DEFINED,
-				NULL, 0, NULL, 0);
-	if (result < 0)
-		sdp_dbg_warn(NULL, "Error <%d> CM reject request", result);
-
-	sdp_conn_put(conn);
-}
-/*
- * sdp_cm_deferred_generic - initiate a defered request using the CM
- */
-static int sdp_cm_deferred_generic(struct sdp_opt *conn,
-				   void (*defer_func)(void *arg))
-{
-	/*
-	 * send a potentially defered failed request.
-	 */
-	sdp_conn_hold(conn);
-
-	if (in_atomic() || irqs_disabled()) {
-		INIT_WORK(&conn->cm_exec, defer_func, (void *)conn);
-		schedule_work(&conn->cm_exec);
-	} else
-		defer_func(conn);
-
-	return 0;
-}
-
-/*
- * sdp_cm_disconnect - initiate a disconnect request using the CM
-*/
-int sdp_cm_disconnect(struct sdp_opt *conn)
-{
-	return sdp_cm_deferred_generic(conn, do_cm_disconnect);
-}
-
-/*
- * sdp_cm_reject - initiate a reject request using the CM
- */
-int sdp_cm_reject(struct sdp_opt *conn)
-{
-	return sdp_cm_deferred_generic(conn, do_cm_reject);
-}



More information about the general mailing list