Re: [PATCH] misc fixes for smsc_cimd2.c

Hi Per,

Per Skaglund wrote:
> 
> Hi all,
> 
> Here are some very simple fixes for smsc_cimd2 driver. The patch has been
> tested on multiple connections to three different operators. I hope this can
> be commited before 1.3.2.
> 
> Votes for a commit are very welcome ;-)
> 
> Changes:
> 
> 1. Increased the RESPONSE_TIMEOUT constant. The "Guidelines for CIMD
> Application Development" document [http://ncsp.forum.nokia.com/downloads/
> nokia/documents/Guidelines_for_CIMD_Application_Development_v1_0.pdf] states:
> "In the case of a login attempt, applications are required to wait for the
> login response from the ASE for at least one minute. Applications that don't
> follow this rule can overload the comsystem by retrying to connect while SMSC
> is still processing the first request". In my experience this change also
> makes the smsc_cimd2 driver a _lot_ more stable.
> 
> 2. Made sure the sequence number of cimd2 LOGIN packets always is 001. This
> was true the first time Kannel logged in after startup, but smsc_cimd2 uses
> the value of the send_seq counter when setting the sequence number of LOGIN
> packets if there ever is a re-connect. In this situation the server would
> reply to the LOGIN with a NACK (ignored by kannel, see 8). Kannel would be
> able to log in after several login attempts, when the send_seq counter
> eventually rolled over to 001. This fix decreases the time smsc_cimd2 spends
> offline.
> 
> 3. Re-added call to charset_gsm_to_latin1() for 7-bit MO content. Without this
> call Kannel delivers messages to applications in the GSM charset instead of
> latin1. It was removed when the driver was ported to smscconn.
> 
> 4. Added smsc-id in info() calls.
> 
> 5. The driver does no longer discard empty messages. This was an inconsistency
> with other drivers, and is important for application side accounting.
> 
> 6. Checks the sequence number of incoming cimd2 requests. This is a major
> source of the infamous "duplicating MO messages" phenomena. In some situations
> a cimd2 server can fail to receive the DELIVERY_REQUEST_RESPONSE packet sent
> back by Kannel. The server will then issue a new DELIVERY_REQUEST with the
> same sequence number. Reading the cimd2 specs I believe the correct procedure
> for Kannel would be to reply with a NACK packet containing the expected
> sequence number, but I'm not sure how the cimd2 servers would handle that...
> However, a quick workaround is to check the sequence number of each request
> against the sequence number of the previous request. If they match the message
> is discarded, but the DELIVERY_REQUEST_RESPONSE is sent back to satify the
> server.
> 
> 7. The debug row "sending <PDU>" was printed before calling the functions
> packet_set_send_sequence() and packet_set_checksum(), hence the debug output
> displayed wrong sequence number/checksum.
> 
> 8. Fixed a bug to allow the driver to correct the sequence number if the
> server says it was wrong. The code contained a condition with a typo for
> checking that the sequence number provided by the server in the NACK really
> was an odd number.

thanks a lot for submission.

Angel, did you review the patch? Any votes from your side?

Stipe

[email protected]
-------------------------------------------------------------------
Wapme Systems AG

Vogelsanger Weg 80
40470 Düsseldorf

Tel: +49-211-74845-0
Fax: +49-211-74845-299

E-Mail: [email protected]
Internet: http://www.wapme-systems.de
-------------------------------------------------------------------
wapme.net - wherever you are