[PATCH] CIMD2 fix and userguide (overrides last one)

"Angel Fradejas" <[email protected]>
Newsgroups gmane.comp.mobile.kannel.devel
Message-ID <[email protected]>
This patch fixes some problems with the SMSCConn CIMD2 driver, for example
high CPU load in a connection with no traffic at all (thanks to Dziugas
Baltrunas for pointing it out), and some other things.

Also included an update for the user guide.

This patch overrides last one I sent yesterday.

Stipe please, commit.


Angel FRADEJAS
Director de Investigación y Desarrollo
Mediafusión España, S.A.
[email protected]
www.mediafusion.es
Tel. +34 91 252 32 00
Fax +34 91 572 27 08
----------------------------------------------------------------------------
----
Mediafusión España, S.A. ®
Aviso legal:
Este mensaje electrónico está dirigido únicamente a la(s) dirección(es)
indicadas anteriormente; el carácter confidencial, personal e intransferible
del mismo está protegido legalmente. Cualquier revelación, uso o reenvío no
autorizado, completo o en parte, está prohibido. Si ha recibido este mensaje
por equivocación, notifíquelo inmediatamente a la persona que lo ha enviado
y borre el mensaje original junto con sus ficheros anexos sin leerlo ni
grabarlo, total o parcialmente.
Gracias.
cimd2_fix2.diff (application/octet-stream, 10.1 KB)
? .cvsignore
? test/test_dbpool
Index: doc/userguide/userguide.xml
===================================================================
RCS file: /home/cvs/gateway/doc/userguide/userguide.xml,v
retrieving revision 1.214
diff -u -r1.214 userguide.xml
--- doc/userguide/userguide.xml	20 Mar 2003 09:16:11 -0000	1.214
+++ doc/userguide/userguide.xml	26 Mar 2003 08:49:42 -0000
@@ -2122,7 +2122,7 @@
 smsc-username = foo
 smsc-password = bar
 keepalive = 5 
-sender-prefix = "12345"
+my-number = "12345"
 </programlisting>
 
  <informaltable frame="none">
@@ -2161,13 +2161,13 @@
      <entry><literal>number</literal></entry>
      <entry valign="bottom">
        SMSC connection will not be left idle for longer than this many
-       minutes.  The right value to use depends on how eager the SMSC
-       is to close idle connections.  5 minutes is a good guess.  If you
-       see many unexplained reconnects, try lowering this value.
+       seconds.  The right value to use depends on how eager the SMSC
+       is to close idle connections. If you see many unexplained 
+       reconnects, try lowering this value.
        Set it to 0 to disable this feature.
      </entry></row>
 
-   <row><entry><literal>sender-prefix</literal></entry>
+   <row><entry><literal>my-number</literal></entry>
       <entry><literal>string</literal></entry>
       <entry valign="bottom">
         The number that the SMSC will add in front of the sender number
@@ -2175,13 +2175,20 @@
         a message, it will remove this prefix from the sender number
         so that the SMSC will add it again.  If the prefix was not present,
         Kannel will log a warning and will not send the sender number.
-        If <literal>sender-prefix</literal> is not set, or is set
+        If <literal>my-number</literal> is not set, or is set
         to <literal>"never"</literal>, then Kannel will not send the
         sender number to the SMSC at all.
         If you want Kannel to pass all sender numbers to the SMSC
         unchanged, then just set <literal>sender-prefix</literal> to the
         empty string <literal>""</literal>.
       </entry></row>
+
+   <row><entry><literal>our-port</literal></entry>
+     <entry><literal>port-number</literal></entry>
+     <entry valign="bottom">
+       Optional port number in which to bind the connection in our
+       end.
+     </entry></row>
 
    </tbody></tgroup></informaltable>
    
Index: gw/smsc/smsc_cimd2.c
===================================================================
RCS file: /home/cvs/gateway/gw/smsc/smsc_cimd2.c,v
retrieving revision 1.7
diff -u -r1.7 smsc_cimd2.c
--- gw/smsc/smsc_cimd2.c	25 Mar 2003 10:16:21 -0000	1.7
+++ gw/smsc/smsc_cimd2.c	26 Mar 2003 08:49:43 -0000
@@ -59,8 +59,7 @@
 
     List *outgoing_queue;
     SMSCConn *conn;
-    int receiver_thread;
-    int sender_thread;
+    int io_thread;
     int quitting;
     List *stopped; /* list-trick for suspend/isolate */
 
@@ -1492,6 +1491,10 @@
     response = packet_create(request->operation + RESPONSE, request->seq);
     packet_set_checksum(response);
 
+    debug("bb.sms.cimd2", 0, "CIMD2[%s]: sending <%s>",
+          octstr_get_cstr(pdata->conn->id),
+          octstr_get_cstr(response->data));
+
     /* Don't check errors here because if there is something
      * wrong with the socket, the main loop will detect it. */
     octstr_write_to_socket(pdata->socket, response->data);
@@ -1635,9 +1638,9 @@
     if (pdata->socket < 0) {
         warning(0, "CIMD2[%s]: cimd2_request: socket not open.",
                 octstr_get_cstr(conn->id));
-        goto io_error;
+        return -2;        
     }
-
+    
 retransmit:
     packet_set_send_sequence(request, pdata);
     packet_set_checksum(request);
@@ -1765,7 +1768,7 @@
                 octstr_get_cstr(conn->id));
         cimd2_close_socket(pdata);
     }
-
+    
     pdata->socket = tcpip_connect_to_server_with_port(
                                             octstr_get_cstr(pdata->host),
                                             pdata->port,
@@ -1906,11 +1909,6 @@
     return ret;
 }
 
-/* The bearerbox really doesn't like it if pending_smsmessage returns
- * an error code.  We work around it until the bearerbox is rewritten.
- * Record the error here, and return it in cimd2_receive_msg.  Return
- * "message available" if there is an error so that cimd2_receive_msg
- * is called. */
 static int cimd2_receive_msg(SMSCConn *conn, Msg **msg)
 {
     PrivData *pdata = conn->data;
@@ -1931,7 +1929,7 @@
          * way. */
         return 0;
     }
-
+    
     ret = read_available(pdata->socket, 0);
     if (ret == 0) {
         if (pdata->keepalive > 0 && pdata->next_ping < time(NULL)) {
@@ -1961,6 +1959,7 @@
         return -1;
     }
 
+
     for (;;) {
         packet = packet_extract(pdata->inbuffer,conn);
         if (!packet)
@@ -2064,7 +2063,7 @@
 }
 
 
-static void sms_receiver(void *arg)
+static void io_thread (void *arg)
 {
     Msg       *msg;
     SMSCConn  *conn = arg;
@@ -2079,49 +2078,6 @@
     
         list_consume(pdata->stopped); /* block here if suspended/isolated */
       
-        if (conn->status == SMSCCONN_ACTIVE) {
-            msg = sms_receive(conn);
-            if (msg) {
-                debug("bb.sms.cimd2", 0, "CIMD2[%s]: new message received",
-                      octstr_get_cstr(conn->id));
-                sleep = 0.0001;
-                bb_smscconn_receive(conn, msg);
-                continue;
-            }
-        }
-        /* note that this implementations means that we sleep even
-         * when we fail connection.. but time is very short, anyway
-         */
-        gwthread_sleep(sleep);
-        /* gradually sleep longer and longer times until something starts to
-         * happen - this of course reduces response time, but that's better than
-         * extensive CPU usage when it is not used
-         */
-        sleep *= 2;
-        if (sleep >= 2.0)
-            sleep = 1.999999;
-    }
-}
-
-
-static void sms_sender(void *arg)
-{
-    Msg       *msg;
-    SMSCConn  *conn = arg;
-    PrivData *pdata = conn->data;
-    double sleep = 0.00001;
-    
-    mutex_lock(conn->flow_mutex);
-    conn->status = SMSCCONN_CONNECTING;
-    mutex_unlock(conn->flow_mutex);
-
-    /* Make sure we log into our own log-file if defined */
-    log_thread_to(conn->log_idx);
-
-    while (!pdata->quitting) {
-    
-        list_consume(pdata->stopped); /* block here if suspended/isolated */
-
         /* check that connection is active */
         if (conn->status != SMSCCONN_ACTIVE) {
             if (cimd2_login(conn) != 0) { 
@@ -2140,25 +2096,48 @@
             bb_smscconn_connected(conn);
             mutex_unlock(conn->flow_mutex);
         }
-        
-        if (pdata->quitting) 
-            break;
 
+        /* receive messages */
+        do { 
+            msg = sms_receive(conn);
+            if (msg) {
+                sleep = 0;
+                debug("bb.sms.cimd2", 0, "CIMD2[%s]: new message received",
+                      octstr_get_cstr(conn->id));
+                bb_smscconn_receive(conn, msg);
+            }
+        } while (msg);
+ 
         /* send messages */
         do {
             msg = list_extract_first(pdata->outgoing_queue);
             if (msg) {
+                sleep = 0;
                 if (cimd2_submit_msg(conn,msg) != 0) break;
             }
-            else {
-                gwthread_sleep(sleep);
-            }
         } while (msg);
+ 
+        if (sleep > 0) {
+
+            /* note that this implementations means that we sleep even
+             * when we fail connection.. but time is very short, anyway
+             */
+            gwthread_sleep(sleep);
+            /* gradually sleep longer and longer times until something starts to
+             * happen - this of course reduces response time, but that's better than
+             * extensive CPU usage when it is not used
+             */
+            sleep *= 2;
+            if (sleep >= 2.0)
+                sleep = 1.999999;
+        }
+        else {
+            sleep = 0.0001;
+        }
     }
 }
 
 
-
 static int cimd2_add_msg_cb (SMSCConn *conn, Msg *sms)
 {
     PrivData *pdata = conn->data;
@@ -2166,7 +2145,7 @@
 
     copy = msg_duplicate(sms);
     list_produce(pdata->outgoing_queue, copy);
-    gwthread_wakeup(pdata->sender_thread);
+    gwthread_wakeup(pdata->io_thread);
 
     return 0;
 }
@@ -2199,14 +2178,9 @@
         conn->is_stopped = 0;
     }
     
-    if (pdata->sender_thread != -1) {
-        gwthread_wakeup(pdata->sender_thread);
-        gwthread_join(pdata->sender_thread); 
-    }
-    
-    if (pdata->receiver_thread != -1) {
-        gwthread_wakeup(pdata->receiver_thread);
-        gwthread_join(pdata->receiver_thread); 
+    if (pdata->io_thread != -1) {
+        gwthread_wakeup(pdata->io_thread);
+        gwthread_join(pdata->io_thread); 
     }
 
     cimd2_close_socket(pdata);
@@ -2225,7 +2199,7 @@
 
     list_remove_producer(pdata->stopped);
     /* in case there are messages in the buffer already */
-    gwthread_wakeup(pdata->receiver_thread);
+    gwthread_wakeup(pdata->io_thread);
     debug("bb.sms", 0, "SMSCConn CIMD2 %s, start called",
           octstr_get_cstr(conn->id));
 }
@@ -2337,26 +2311,17 @@
                 maxlen);
     }
 
-    pdata->receiver_thread = gwthread_create(sms_receiver, conn);
-    pdata->sender_thread = gwthread_create(sms_sender, conn);
+    pdata->io_thread = gwthread_create(io_thread, conn);
 
-    if ((pdata->receiver_thread == -1) ||  
-        (pdata->sender_thread == -1)) {  
+    if (pdata->io_thread == -1) {  
 
-        error(0,"CIMD2[%s]: Couldn't start I/O threads.",
+        error(0,"CIMD2[%s]: Couldn't start I/O thread.",
               octstr_get_cstr(conn->id));
         pdata->quitting = 1;
-        if (pdata->sender_thread != -1) {
-            gwthread_wakeup(pdata->sender_thread);
-            gwthread_join(pdata->sender_thread);
-        }  
-        if (pdata->receiver_thread != -1) {
-            gwthread_wakeup(pdata->receiver_thread);
-            gwthread_join(pdata->receiver_thread);
-        }
+        gwthread_wakeup(pdata->io_thread);
+        gwthread_join(pdata->io_thread);
         cimd2_destroy(pdata);
         return -1;  
-
     } 
 
     conn->send_msg = cimd2_add_msg_cb;
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.