[openib-general] Re: [PATCH] RE: compliancy issue?

[Michael S. Tsirkin]

Quoting r. Sean Hefty <sean.hefty at intel.com>:
> Subject: [PATCH] RE: compliancy issue?
> 
> >CA4-24.2.3: The connecting peer shall terminate the connection attempt
> >if ExtMaxAdverts of the HAH is set to zero.
> >
> >This means that SDP must examine the HAH before RTU is sent.
> >But, CMA currently sends RTU from cma_rep_recv, before notifying
> >the user.
> 
> Can you try this simple patch and see if it fixes your problem?  You will
> need to call rdma_accept() or rdma_reject() after receiving a CONNECT_RESPONSE
> event.  The conn_param to rdma_accept() should be NULL.

OK, I just tested and this works for me. Here's the SDP patch to do what you
described. The code actually got cleaner now: its convenient to get
different events on active versus passive side - previously I had
to check a flag to figure out what does ESTABLISHED mean.

I still think it makes sense to do this for all ULPs and not just SDP, but oh
well.

Signed-off-by: Michael S. Tsirkin <mst at mellanox.co.il>

Index: linux-2.6.16/drivers/infiniband/ulp/sdp/sdp_cma.c
===================================================================
--- linux-2.6.16.orig/drivers/infiniband/ulp/sdp/sdp_cma.c	2006-05-16 15:22:00.000000000 +0300
+++ linux-2.6.16/drivers/infiniband/ulp/sdp/sdp_cma.c	2006-05-16 15:25:03.000000000 +0300
@@ -237,9 +237,9 @@ int sdp_connect_handler(struct sock *sk,
 	return 0;
 }
 
-int sdp_connected_handler(struct sock *sk, struct rdma_cm_event *event)
+static int sdp_response_handler(struct sock *sk, struct rdma_cm_event *event)
 {
-	struct sock *parent;
+	struct sdp_hah *h;
 	sdp_dbg(sk, "%s\n", __func__);
 
 	sk->sk_state = TCP_ESTABLISHED;
@@ -250,23 +250,37 @@ int sdp_connected_handler(struct sock *s
 	if (sock_flag(sk, SOCK_DEAD))
 		return 0;
 
+	h = event->private_data;
+	sdp_sk(sk)->bufs = ntohs(h->bsdh.bufs);
+	sdp_sk(sk)->xmit_size_goal = ntohl(h->actrcvsz) -
+		sizeof(struct sdp_bsdh);
+
+	sdp_dbg(sk, "%s bufs %d xmit_size_goal %d\n", __func__,
+		sdp_sk(sk)->bufs,
+		sdp_sk(sk)->xmit_size_goal);
+
+	ib_req_notify_cq(sdp_sk(sk)->qp->send_cq, IB_CQ_NEXT_COMP);
+
+	sk->sk_state_change(sk);
+	sk_wake_async(sk, 0, POLL_OUT);
+	return 0;
+}
+
+int sdp_connected_handler(struct sock *sk, struct rdma_cm_event *event)
+{
+	struct sock *parent;
+	sdp_dbg(sk, "%s\n", __func__);
+
 	parent = sdp_sk(sk)->parent;
-	if (!parent) {
-		struct sdp_hah *h = event->private_data;
-		sdp_sk(sk)->bufs = ntohs(h->bsdh.bufs);
-		sdp_sk(sk)->xmit_size_goal = ntohl(h->actrcvsz) -
-			sizeof(struct sdp_bsdh);
-
-		sdp_dbg(sk, "%s bufs %d xmit_size_goal %d\n", __func__,
-			sdp_sk(sk)->bufs,
-			sdp_sk(sk)->xmit_size_goal);
+	BUG_ON(!parent);
 
-		ib_req_notify_cq(sdp_sk(sk)->qp->send_cq, IB_CQ_NEXT_COMP);
+	sk->sk_state = TCP_ESTABLISHED;
+
+	/* TODO: If SOCK_KEEPOPEN set, need to reset and start
+	   keepalive timer here */
 
-		sk->sk_state_change(sk);
-		sk_wake_async(sk, 0, POLL_OUT);
+	if (sock_flag(sk, SOCK_DEAD))
 		return 0;
-	}
 
 	lock_sock(parent);
 	if (sk_acceptq_is_full(parent)) {
@@ -292,11 +306,6 @@ void sdp_disconnected_handler(struct soc
 	sdp_dbg(sk, "%s\n", __func__);
 }
 
-void sdp_response_handler(struct sock *sk)
-{
-	sdp_dbg(sk, "%s\n", __func__);
-}
-
 int sdp_cma_handler(struct rdma_cm_id *id, struct rdma_cm_event *event)
 {
 	struct rdma_conn_param conn_param;
@@ -388,7 +397,11 @@ int sdp_cma_handler(struct rdma_cm_id *i
 		break;
 	case RDMA_CM_EVENT_CONNECT_RESPONSE:
 		sdp_dbg(sk, "RDMA_CM_EVENT_CONNECT_RESPONSE\n");
-		sdp_response_handler(sk);
+		rc = sdp_response_handler(sk, event);
+		if (rc)
+			rdma_reject(id, NULL, 0);
+		else
+			rc = rdma_accept(id, NULL);
 		break;
 	case RDMA_CM_EVENT_CONNECT_ERROR:
 		sdp_dbg(sk, "RDMA_CM_EVENT_CONNECT_ERROR\n");

-- 
MST