patch-2.4.21 linux-2.4.21/net/irda/irnet/irnet_irda.c

• Lines: 113
• Date: 2003-06-13 07:51:39.000000000 -0700
• Orig file: linux-2.4.20/net/irda/irnet/irnet_irda.c
• Orig date: 2002-11-28 15:53:16.000000000 -0800

diff -urN linux-2.4.20/net/irda/irnet/irnet_irda.c linux-2.4.21/net/irda/irnet/irnet_irda.c
@@ -1340,46 +1340,80 @@
       return;
     }
 
-  /* Socket connecting ?
-   * Clear up flag : prevent irnet_disconnect_indication() to mess up tsap */
-  if(test_and_clear_bit(0, &new->ttp_connect))
-    {
-      /* The socket is trying to connect to the other end and may have sent
-       * a IrTTP connection request and is waiting for a connection response
-       * (that may never come).
-       * Now, the pain is that the socket may have opened a tsap and is
-       * waiting on it, while the other end is trying to connect to it on
-       * another tsap.
-       */
-      DERROR(IRDA_CB_ERROR, "Socket already connecting. Ouch !\n");
+  /* The following code is a bit tricky, so need comments ;-)
+   */
+  /* If ttp_connect is set, the socket is trying to connect to the other
+   * end and may have sent a IrTTP connection request and is waiting for
+   * a connection response (that may never come).
+   * Now, the pain is that the socket may have opened a tsap and is
+   * waiting on it, while the other end is trying to connect to it on
+   * another tsap.
+   * Because IrNET can be peer to peer, we need to workaround this.
+   * Furthermore, the way the irnetd script is implemented, the
+   * target will create a second IrNET connection back to the
+   * originator and expect the originator to bind this new connection
+   * to the original PPPD instance.
+   * And of course, if we don't use irnetd, we can have a race when
+   * both side try to connect simultaneously, which could leave both
+   * connections half closed (yuck).
+   * Conclusions :
+   *	1) The "originator" must accept the new connection and get rid
+   *	   of the old one so that irnetd works
+   *	2) One side must deny the new connection to avoid races,
+   *	   but both side must agree on which side it is...
+   * Most often, the originator is primary at the LAP layer.
+   * Jean II
+   */
+  /* Now, let's look at the way I wrote the test...
+   * We need to clear up the ttp_connect flag atomically to prevent
+   * irnet_disconnect_indication() to mess up the tsap we are going to close.
+   * We want to clear the ttp_connect flag only if we close the tsap,
+   * otherwise we will never close it, so we need to check for primary
+   * *before* doing the test on the flag.
+   * And of course, ALLOW_SIMULT_CONNECT can disable this entirely...
+   * Jean II
+   */
+
+  /* Socket already connecting ? On primary ? */
+  if(0
 #ifdef ALLOW_SIMULT_CONNECT
-      /* Cleanup the TSAP if necessary - IrIAP will be cleaned up later */
+     || ((irttp_is_primary(server->tsap) == 1)	/* primary */
+	 && (test_and_clear_bit(0, &new->ttp_connect)))
+#endif /* ALLOW_SIMULT_CONNECT */
+     )
+    {
+      DERROR(IRDA_CB_ERROR, "Socket already connecting, but going to reuse it !\n");
+
+      /* Cleanup the old TSAP if necessary - IrIAP will be cleaned up later */
       if(new->tsap != NULL)
 	{
-	  /* Close the connection the new socket was attempting.
-	   * This seems to be safe... */
+	  /* Close the old connection the new socket was attempting,
+	   * so that we can hook it up to the new connection.
+	   * It's now safe to do it... */
 	  irttp_close_tsap(new->tsap);
 	  new->tsap = NULL;
 	}
-      /* Note : no return, fall through... */
-#else /* ALLOW_SIMULT_CONNECT */
-      irnet_disconnect_server(server, skb);
-      return;
-#endif /* ALLOW_SIMULT_CONNECT */
     }
   else
-    /* If socket is not connecting or connected, tsap should be NULL */
-    if(new->tsap != NULL)
-      {
-	/* If we are here, we are also in irnet_disconnect_indication(),
-	 * and it's a nice race condition... On the other hand, we can't be
-	 * in irda_irnet_destroy() otherwise we would not have found the
-	 * socket in the hashbin. */
-	/* Better get out of here, otherwise we will mess up tsaps ! */
-	DERROR(IRDA_CB_ERROR, "Race condition detected, abort connect...\n");
-	irnet_disconnect_server(server, skb);
-	return;
-      }
+    {
+      /* Three options :
+       * 1) socket was not connecting or connected : ttp_connect should be 0.
+       * 2) we don't want to connect the socket because we are secondary or
+       * ALLOW_SIMULT_CONNECT is undefined. ttp_connect should be 1.
+       * 3) we are half way in irnet_disconnect_indication(), and it's a
+       * nice race condition... Fortunately, we can detect that by checking
+       * if tsap is still alive. On the other hand, we can't be in
+       * irda_irnet_destroy() otherwise we would not have found this
+       * socket in the hashbin.
+       * Jean II */
+      if((test_bit(0, &new->ttp_connect)) || (new->tsap != NULL))
+	{
+	  /* Don't mess this socket, somebody else in in charge... */
+	  DERROR(IRDA_CB_ERROR, "Race condition detected, socket in use, abort connect...\n");
+	  irnet_disconnect_server(server, skb);
+	  return;
+	}
+    }
 
   /* So : at this point, we have a socket, and it is idle. Good ! */
   irnet_connect_socket(server, new, qos, max_sdu_size, max_header_size);