Hi Stipe,

did you really tested this patch? I don't think so...

in smsc2_route(...):
/* function to route outgoing SMS'es
 *
 * If finds a good one, puts into it and returns SMSCCONN_SUCCESS
 * If finds only bad ones, but acceptable, queues and
* returns SMSCCONN_QUEUED (like all acceptable currently disconnected) * if message acceptable but queues full returns SMSCCONN_FAILED_QFULL and
 * message is not destroyed.
 * If cannot find nothing at all, returns SMSCCONN_FAILED_DISCARDED and
 * message is NOT destroyed (otherwise it is)
 */

and then at the end of the function:
    msg_destroy(msg);
    return SMSCCONN_SUCCESS;
}

so you have 100% bum in bb_boxc.

Beside implementation error I don't think this will work as you would like. As far as message accepted by bearerbox you want return smsc-id in ack. That's ok. But what do you want todo if the same message is then temporarily undelivered by this smsc and you make retry? What is todo there, you already returned "wrong" smsc- id to smsbox.
So this feature just doesn't make any sense for me.

Thanks,
Alex

Am 20.03.2009 um 14:53 schrieb Stipe Tolj:

Hi list,

I'd like to introduce an easy concept where bearerbox can inform smsbox
connections about the final smsc-id the msg has been assigned to.

Bearerbox simply re-writes the msg->sms.smsc_id value as soon as the
smsc2_rout() routing has occurred and we're drilled down to the assigned smsc
connection.

That value is used then in a new field for the ack msg type:

   mack->ack.smsc_id = octstr_duplicate(msg->sms.smsc_id);

and the ack is send as usual to the boxc connection. The smsbox can then (as example usage) put that final routing decision to the HTTP response body:

   answer = octstr_format("0: Accepted for delivery by smsc-id '%s'",

Please apply against CVS HEAD, test and comment.

Thanks,
Stipe

BTW, the core of the patch is pretty minimal, while the patch file includes some
source code re-formating to comply to our CodingStyle.

--
-------------------------------------------------------------------
Kölner Landstrasse 419
40589 Düsseldorf, NRW, Germany

tolj.org system architecture      Kannel Software Foundation (KSF)
http://www.tolj.org/              http://www.kannel.org/

mailto:st_{at}_tolj.org           mailto:stolj_{at}_kannel.org
-------------------------------------------------------------------
### Eclipse Workspace Patch 1.0
#P gateway-cvs-head
Index: gw/bb_boxc.c
===================================================================
RCS file: /home/cvs/gateway/gw/bb_boxc.c,v
retrieving revision 1.94
diff -u -r1.94 bb_boxc.c
--- gw/bb_boxc.c        2 Mar 2009 01:35:38 -0000       1.94
+++ gw/bb_boxc.c        20 Mar 2009 02:25:47 -0000
@@ -215,8 +215,9 @@
    int rc;

    /*
-     * save modifies ID and time, so if the smsbox uses it, save
-     * it FIRST for the reply message!!!
+     * Make sure we use the same UUID and time for the
+     * ack response message towards smsbox.
+     * A smsbox connection logic may depend on it.
     */
    mack = msg_create(ack);
    gw_assert(mack != NULL);
@@ -225,31 +226,43 @@

    store_save(msg);

+    /* call our message router */
    rc = smsc2_rout(msg, 0);
-    switch(rc) {
+
+    /* provide the routing information via ack
+     * to the attached smsbox connection */
+    mack->ack.smsc_id = octstr_duplicate(msg->sms.smsc_id);
+
+    switch (rc) {
        case SMSCCONN_SUCCESS:
-           mack->ack.nack = ack_success;
-           break;
+            mack->ack.nack = ack_success;
+            break;
+
        case SMSCCONN_QUEUED:
-           mack->ack.nack = ack_buffered;
-           break;
+            mack->ack.nack = ack_buffered;
+            break;
+
        case SMSCCONN_FAILED_DISCARDED: /* no router at all */
        case SMSCCONN_FAILED_QFULL: /* queue full */
-           warning(0, "Message rejected by bearerbox, %s!",
- (rc == SMSCCONN_FAILED_DISCARDED) ? "no router" : "queue full");
-           /*
-            * first create nack for store-file, in order to delete
-            * message from store-file.
-            */
- store_save_ack(msg, (rc == SMSCCONN_FAILED_QFULL ? ack_failed_tmp : ack_failed)); - mack->ack.nack = (rc == SMSCCONN_FAILED_QFULL ? ack_failed_tmp : ack_failed);
-
-           /* destroy original message */
-           msg_destroy(msg);
-           break;
+            warning(0, "Message rejected by bearerbox, %s!",
+ (rc == SMSCCONN_FAILED_DISCARDED) ? "no router" : "queue full");
+
+            /*
+             * First create nack for store-file, in order to delete
+             * message from store-file.
+             */
+ store_save_ack(msg, (rc == SMSCCONN_FAILED_QFULL ? ack_failed_tmp : ack_failed)); + mack->ack.nack = (rc == SMSCCONN_FAILED_QFULL ? ack_failed_tmp : ack_failed);
+
+            /* destroy original message */
+            msg_destroy(msg);
+            break;
+
+        default:
+            break;
    }

-    /* put ack into incoming queue of conn */
+    /* put ack into incoming queue of conn (smsbox connection) */
    send_msg(conn, mack);
    msg_destroy(mack);
}
Index: gw/smsbox.c
===================================================================
RCS file: /home/cvs/gateway/gw/smsbox.c,v
retrieving revision 1.282
diff -u -r1.282 smsbox.c
--- gw/smsbox.c 14 Jan 2009 11:11:46 -0000      1.282
+++ gw/smsbox.c 20 Mar 2009 02:25:50 -0000
@@ -184,33 +184,45 @@
        octstr_destroy(os);
        return;
    }
-    /* XXX  this should be fixed so that we really wait for DLR
-     *      SMSC accept/deny before doing this - but that is far
-     *      more slower, a bit more complex, and is done later on
+
+    /*
+     * TODO: move from 'semi-waiting' to 'full-waiting' method.
+     *
+     * Actually this waiting mechanism is 'semi-waiting'. All that we
+     * are sure of is that bearerbox was able to assign the message
+     * in the lower smsc module layer to a specific smsc connection,
+     * and queued for delivery. This is no real ACK state from the
+     * SMSC side itself.
+     *
+ * This would imply waiting for the corresponding submit_sm_resp PDU + * i.e. of the SMPP protocol. This coudl be only done efficiently via
+     * asynchronous callback passing of the messages. -- st.
     */

    switch (msg->ack.nack) {
-      case ack_success:
-        status = HTTP_ACCEPTED;
-        answer = octstr_create("0: Accepted for delivery");
-        break;
-      case ack_buffered:
-        status = HTTP_ACCEPTED;
-        answer = octstr_create("3: Queued for later delivery");
-        break;
-      case ack_failed:
-        status = HTTP_FORBIDDEN;
-        answer = octstr_create("Not routable. Do not try again.");
-        break;
-      case ack_failed_tmp:
-        status = HTTP_SERVICE_UNAVAILABLE;
-        answer = octstr_create("Temporal failure, try again later.");
-        break;
-      default:
-       error(0, "Strange reply from bearerbox!");
-        status = HTTP_SERVICE_UNAVAILABLE;
-        answer = octstr_create("Temporal failure, try again later.");
-        break;
+        case ack_success:
+            status = HTTP_ACCEPTED;
+ answer = octstr_format("0: Accepted for delivery by smsc-id '%s'",
+                        octstr_get_cstr(msg->ack.smsc_id));
+            break;
+        case ack_buffered:
+            status = HTTP_ACCEPTED;
+ answer = octstr_format("3: Queued for later delivery by smsc-id '%s'",
+                         octstr_get_cstr(msg->ack.smsc_id));
+            break;
+        case ack_failed:
+            status = HTTP_FORBIDDEN;
+ answer = octstr_create("Not routable. Do not try again.");
+            break;
+        case ack_failed_tmp:
+            status = HTTP_SERVICE_UNAVAILABLE;
+ answer = octstr_create("Temporal failure, try again later.");
+            break;
+        default:
+            error(0, "Strange reply from bearerbox!");
+            status = HTTP_SERVICE_UNAVAILABLE;
+ answer = octstr_create("Temporal failure, try again later.");
+            break;
    }

    http_send_reply(client, status, sendsms_reply_hdrs, answer);
@@ -244,32 +256,35 @@
        else if (msg == NULL) /* just to be sure, may not happens */
            break;

-       if (msg_type(msg) == admin) {
-           if (msg->admin.command == cmd_shutdown) {
-               info(0, "Bearerbox told us to die");
-               program_status = shutting_down;
-           } else if (msg->admin.command == cmd_restart) {
-               info(0, "Bearerbox told us to restart");
-               restart = 1;
-               program_status = shutting_down;
-           }
-           /*
-            * XXXX here should be suspend/resume, add RSN
-            */
-           msg_destroy(msg);
-       } else if (msg_type(msg) == sms) {
-           if (total == 0)
-               start = time(NULL);
-           total++;
-           gwlist_produce(smsbox_requests, msg);
-       } else if (msg_type(msg) == ack) {
-           if (!immediate_sendsms_reply)
-               delayed_http_reply(msg);
-           msg_destroy(msg);
-       } else {
-           warning(0, "Received other message than sms/admin, ignoring!");
-           msg_destroy(msg);
-       }
+        if (msg_type(msg) == admin) {
+            if (msg->admin.command == cmd_shutdown) {
+                info(0, "Bearerbox told us to die");
+                program_status = shutting_down;
+            } else if (msg->admin.command == cmd_restart) {
+                info(0, "Bearerbox told us to restart");
+                restart = 1;
+                program_status = shutting_down;
+            }
+            /*
+             * TODO: here should be suspend/resume, add RSN
+             */
+            msg_destroy(msg);
+        }
+        else if (msg_type(msg) == sms) {
+            if (total == 0)
+                start = time(NULL);
+            total++;
+            gwlist_produce(smsbox_requests, msg);
+        }
+        else if (msg_type(msg) == ack) {
+            if (!immediate_sendsms_reply)
+                delayed_http_reply(msg);
+            msg_destroy(msg);
+        }
+        else {
+ warning(0, "Received other message than sms/admin, ignoring!");
+            msg_destroy(msg);
+        }
    }
    secs = difftime(time(NULL), start);
    info(0, "Received (and handled?) %d requests in %d seconds "
Index: gw/smscconn.c
===================================================================
RCS file: /home/cvs/gateway/gw/smscconn.c,v
retrieving revision 1.59
diff -u -r1.59 smscconn.c
--- gw/smscconn.c       12 Jan 2009 16:46:56 -0000      1.59
+++ gw/smscconn.c       20 Mar 2009 02:25:50 -0000
@@ -499,6 +499,7 @@
{
    int ret = -1;
    List *parts = NULL;
+    const Octstr *cid;

    gw_assert(conn != NULL);
    mutex_lock(conn->flow_mutex);
@@ -506,6 +507,17 @@
        mutex_unlock(conn->flow_mutex);
        return -1;
    }
+
+    /*
+     * We're done now with routing and have our smsc connection
+     * assigned to be used. In order to get accurate logging of
+     * the smsc-id the messages goes, and for the ACK/NACK msg
+     * towards smsbox we replace the value here.
+     */
+    if (conn && (cid = smscconn_id(conn))) {
+        octstr_destroy(msg->sms.smsc_id);
+        msg->sms.smsc_id = octstr_duplicate(cid);
+    }

/* if this a retry of splitted message, don't unify prefix and don't try to split */
    if (msg->sms.split_parts == NULL) {
Index: gw/msg-decl.h
===================================================================
RCS file: /home/cvs/gateway/gw/msg-decl.h,v
retrieving revision 1.37
diff -u -r1.37 msg-decl.h
--- gw/msg-decl.h       14 Jan 2009 11:11:46 -0000      1.37
+++ gw/msg-decl.h       20 Mar 2009 02:25:47 -0000
@@ -118,6 +118,7 @@
                INTEGER(nack);
                INTEGER(time);
                UUID(id);
+        OCTSTR(smsc_id);
        })

MSG(wdp_datagram,


Reply via email to