Re: [PATCH] Throttling support for AT smsc

Vincent CHAVANIS <[email protected]>
Newsgroups gmane.comp.mobile.kannel.devel
Organization TELEMAQUE
Message-ID <[email protected]>
here is the corrected patch.

Vincent.



Le 15/06/2011 13:42, Alexander Malysh a écrit :
> Hi Vincent,
>
> patch doesn't apply:
> 	fatal: corrupt patch at line 59
>
> and this change is not what you want...
>
>       if (privdata->modem == NULL || privdata->modem->need_sleep)
> -        sleep(1);
> +        gwthread_sleep(1);
>
> you want to sleep 1 second because modem needs this. if you use gwthread_sleep other threads
> could wake up this thread.
>
> Thanks,
> Alex
>
> P.S. Otherwise patch looks good.
>
> Am 15.06.2011 um 13:00 schrieb Vincent CHAVANIS:
>
>> Hi,
>>
>> Sorry for the delay,
>> here is the new patch with alex suggestion with some optimizations
>>
>> The patch has been edited manually, so telle me if it does not apply.
>>
>> regards
>>
>> Vincent.
>>
>>
>> Le 20/05/2011 20:16, Alexander Malysh a écrit :
>>> Hi Vincent,
>>>
>>> this is very easy:
>>> a) if you sleep for throughput you don't receive any MOs
>>> b) if you have something to send, this thread will be waked up any time and therefore there is no throughput garantee
>>>
>>> if you use load_XXX then all these issues can be avoided.
>>>
>>> Alex
>>>
>>> Am 19.05.2011 um 13:04 schrieb Vincent CHAVANIS:
>>>
>>>> Hi Alex,
>>>>
>>>> AT commands are synchronous and there is only one thread
>>>> for sending and receving messages
>>>>
>>>> Why should i implement the load_XX function to do the trick ?
>>>> (there is no need here to modify the PrivAT2data structure)
>>>>
>>>> Vincent
>>>>
>>>>
>>>>
>>>> Le 18/05/2011 20:50, Alexander Malysh a écrit :
>>>>> Hi,
>>>>>
>>>>> why don't you use load_XX support? See smsc_smpp as example.
>>>>> With only sleep thread will wakeup if new messages available for sending
>>>>> and therefore you don't guarantee throughput.
>>>>>
>>>>> Alex
>>>>>
>>>>> Am 18.05.2011 um 14:48 schrieb Vincent CHAVANIS:
>>>>>
>>>>>> Hi all,
>>>>>>
>>>>>> Here is a small patch to support throttling on AT smsc
>>>>>>
>>>>>> Any comment ?
>>>>>>
>>>>>> regards
>>>>>>
>>>>>> Vincent.
>>>>>> <smsc_at_throttling.txt>
>>>>>
>>>>
>> <througputv2_at.txt>
>
througput_at_v3.txt (text/plain, 3.7 KB)
--- /gateway-cvs/gw/smsc/smsc_at.h      2010-10-07 16:26:11.194141461 +0200
+++ /gateway/gw/smsc/smsc_at.h  2011-06-15 14:21:50.276042157 +0200
@@ -55,7 +55,7 @@
  */ 
 
 /*
- * gw/smsc_at2.h
+ * gw/smsc_at.h
  *
  * New driver for serial connected AT based
  * devices.
@@ -68,6 +68,7 @@
 #define SMSC_AT2_H
 
 #include "gwlib/gwlib.h"
+#include "load.h"
 
 /* maximum data to attempt to read in one go */
 #define        MAX_READ        1023
@@ -149,6 +150,7 @@
     int rawtcp_port;
     int is_serial; /* false if device is rawtcp */ 
     int use_telnet; /* use telnet escape sequences */
+    Load *load;
  } PrivAT2data;
--- /gateway-cvs/gw/smsc/smsc_at.c      2010-10-07 16:26:11.212892930 +0200
+++ /gateway/gw/smsc/smsc_at.c  2011-06-15 14:23:53.531041068 +0200
@@ -95,6 +95,7 @@
 #include "sms.h"
 #include "dlr.h"
 #include "smsc_at.h"
+#include "load.h"
 
 static Octstr                  *gsm2number(Octstr *pdu);
 static unsigned char   nibble2hex(unsigned char b);
@@ -1432,6 +1433,7 @@
     octstr_destroy(privdata->rawtcp_host);
     gw_prioqueue_destroy(privdata->outgoing_queue, NULL);
     gwlist_destroy(privdata->pending_incoming_messages, octstr_destroy_item);
+    load_destroy(privdata->load);
     gw_free(conn->data);
     conn->data = NULL;
     mutex_lock(conn->flow_mutex);
@@ -1474,24 +1476,20 @@
 
 static long at2_queued_cb(SMSCConn *conn)
 {
-    long ret;
-    PrivAT2data *privdata = conn->data;
-
-    if (conn->status == SMSCCONN_DEAD) /* I'm dead, why would you care ? */
-        return -1;
-
-    ret = gw_prioqueue_len(privdata->outgoing_queue);
+    PrivAT2data *privdata;
 
-    /* use internal queue as load, maybe something else later */
-    conn->load = ret;
-    return ret;
+    privdata = conn->data;
+    conn->load = (privdata ? (conn->status != SMSCCONN_DEAD ?
+                  gw_prioqueue_len(privdata->outgoing_queue) : 0) : 0);
+    return conn->load;
 }
 
 
 static void at2_start_cb(SMSCConn *conn)
 {
-    PrivAT2data *privdata = conn->data;
+    PrivAT2data *privdata;
 
+    privdata = conn->data;
     if (conn->status == SMSCCONN_DISCONNECTED)
         conn->status = SMSCCONN_ACTIVE;
     
@@ -1502,11 +1500,10 @@
 
 static int at2_add_msg_cb(SMSCConn *conn, Msg *sms)
 {
-    PrivAT2data *privdata = conn->data;
-    Msg *copy;
+    PrivAT2data *privdata;
 
-    copy = msg_duplicate(sms);
-    gw_prioqueue_produce(privdata->outgoing_queue, copy);
+    privdata = conn->data;
+    gw_prioqueue_produce(privdata->outgoing_queue, msg_duplicate(sms));
     gwthread_wakeup(privdata->device_thread);
     return 0;
 }
@@ -1628,6 +1625,9 @@
     if (cfg_get_integer((long *) &privdata->max_error_count,  cfg, octstr_imm("max-error-count")) == -1)
         privdata->max_error_count = -1;
 
+    privdata->load = load_create_real(0);
+    load_add_interval(privdata->load, 1);
+
     conn->data = privdata;
     conn->name = octstr_format("AT2[%s]", octstr_get_cstr(privdata->name));
     conn->status = SMSCCONN_CONNECTING;
@@ -2210,10 +2209,16 @@
     if (privdata->modem->enable_mms && gw_prioqueue_len(privdata->outgoing_queue) > 1)
         at2_send_modem_command(privdata, "AT+CMMS=2", 0, 0);
 
-    if ((msg = gw_prioqueue_remove(privdata->outgoing_queue)))
+    if (privdata->conn->throughput > 0 && load_get(privdata->load, 0) >= privdata->conn->throughput) {
+      debug("bb.sms.at2", 0, "AT2[%s]: throughput limit exceeded (load: %.02f, throughput: %.02f)",
+            octstr_get_cstr(privdata->conn->id), load_get(privdata->load, 0), privdata->conn->throughput);
+    } else {           
+      if ((msg = gw_prioqueue_remove(privdata->outgoing_queue))) {
+          load_increase(privdata->load);       
         at2_send_one_message(privdata, msg);
 }
-
+    }
+}
 
 static void at2_send_one_message(PrivAT2data *privdata, Msg *msg)
 {
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.