Hi Thanks very much for the quick feedback. My comments inline.
Regards Richard. On 6 July 2013 19:48, Daniel-Constantin Mierla <[email protected]> wrote: > Hello, > > the patch cannot be applied as it is. The comments contain references to > request processing, which is obviously wrong. Then I have seen code > specific to requests, like managing the request uri or dst uri -- these > fields shouldn't be set for replies, it's ok as safety to clear them up, > but not cloning. > I've gone through the code (and comments) in detail and removed all incorrect references to request code. I've also removed the dst/request URI management from the faked_resp method. Attached is the patch for these changes. > > From the patch I see the new function are only for internal usage, not for > config file. Are they supposed to be used only in the transaction context > (i.e., after the transaction is matched for reply)? > This patch closely follows the pattern of the t_suspend(), t_continue() functions that are for programmatic use only. We use the t_suspend_reply() and t_continue_reply() functions in the ims_qos module so did not need to expose it to the config file. I suppose the suspend functionality for requests and responses could be exposed to the config file if required - but it has not been done in this patch. > > Cheers, > Daniel > > > On 7/4/13 5:24 PM, Richard Good wrote: > > Hi > > I've finally got working functionality for t-suspend and t_continue for > SIP responses in the tmp/tm_async_reply_support branch. > > I've memory and load tested the branch and have also isolated the changes > so that if there are problems they will only be experienced if you use the > new API functions. > > However keeping mind that TM can be tricky to program (as mentioned in > the documentation ;)) - I've included below a summary of the changes and a > git diff against the latest master. So the TM experts can have a look and > see if there are any obvious problems. > > If there are no complaints or queries I will commit the changes to master > next week Friday (July 12). > > Regards > Richard. > > Change summary: > 1. parser/msg_parser.h: added new message state FL_RPL_SUSPENDED > 2. tm_load.c: new api functions: *t_suspend_reply*, *t_continue_reply*and > *t_cancel_suspend_reply* > 3. t_suspend.c/h: new methods: *t_suspend_reply*, *t_continue_reply* and > *t_cancel_suspend_reply > *4. tm/t_reply.c: if msg is in state FL_RPL_SUSPENDED then we skip > sending the reply on > 5. tm/t_reply.c/h: new methods to fake env for responses: *faked_env_resp, > faked_resp, free_faked_resp > * > 6. API documentation to include new functions: *t_suspend_reply*, * > t_continue_reply* and *t_cancel_suspend_reply > > * > * > * > * > * > > > > > > On 18 March 2013 14:13, Richard Good <[email protected]> wrote: > >> Hi Daniel and other TM experts >> >> We have been working on a prototype for being able to use the t_suspend() >> and t_continue() methods for SIP responses. The version in branch >> /tmp/tm_async_reply_support is working, though still needs to be 100% >> tested and only works for 1 branch (i.e. no forking) >> >> Would it be possible for you to have a look at this branch code and see >> if we are on the right track. The code is working but TM (as mentioned in >> the documentation ;)) can be tricky to program and we want to make sure we >> are not breaking anything, as the goal is to get this into the master >> branch. >> >> The changes we have done: >> >> 1. parser/msg_parser.h: added new message state FL_RPL_SUSPENDED >> 2. tm/h_table.h: added int suspended_request to ua_server and int >> suspended_reply to ua_client >> 3. tm/t_suspend.c:t_suspend >> 1. Changed suspend method to check if the message is a request or >> response and set the suspended_request/suspended_reply variable >> accordingly >> 2. If its a request we do as the existing code >> 3. If its a response we get the correct uac branch, clone the msg >> into t->uac[branch].reply and set the msg state to FL_RPL_SUSPENDED >> 4. tm/t_reply.c: if msg is in state FL_RPL_SUSPENDED then we skip >> sending the reply on >> 5. tm/t_suspend.c:t_continue >> 1. Check if this uas was suspended - if so this is a continue for >> a request so continue with existing code >> 2. If not then we search for the branch that was suspended >> 3. Once branch is found unsuspend the message state >> 4. do pre-script, run_top_route and post scripts >> 5. Relay the reply using basically the same code we skipped in >> step 4 >> >> First next step after we confirm we are on the right track is to add >> forking support - this will require a new t_continue method where the >> calling application passes in the branch of the suspended reply when >> wanting to continue. >> >> Any comments/feedback would be appreciated. >> >> Regards >> >> Richard. >> >> >> >> >> >> >> >> On 11 March 2013 14:34, Daniel-Constantin Mierla <[email protected]>wrote: >> >>> Hello, >>> >>> t_suspend() and t_continue() are only for requests at this moment. >>> >>> Cheers, >>> Daniel >>> >>> >>> On 3/6/13 2:49 PM, Richard Good wrote: >>> >>> Hi >>> >>> We are using the t_suspend and t_continue functionality to >>> asynchronously send Diameter messages. >>> >>> This works nicely when we suspend and resume on a SIP request. However >>> in some circumstances we want to send a Diameter message on a SIP response. >>> >>> I have found that the SIP responses continue to be relayed despite being >>> suspended and returning 0 to the cfg file. >>> >>> I am investigating the cause but before I get any deeper does anyone >>> know if the t_suspend and t_continue functionality in the tm module can be >>> used on a SIP response? >>> >>> Regards >>> Richard. >>> >>> >>> This email is subject to the disclaimer of Smile Communications (PTY) >>> Ltd. at http://www.smilecoms.com/disclaimer >>> >>> _______________________________________________ >>> sr-dev mailing >>> [email protected]http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev >>> >>> >>> -- >>> Daniel-Constantin Mierla - >>> http://www.asipto.comhttp://twitter.com/#!/miconda - >>> http://www.linkedin.com/in/miconda >>> Kamailio World Conference, April 16-17, 2013, Berlin >>> - http://conference.kamailio.com - >>> >>> >>> _______________________________________________ >>> sr-dev mailing list >>> [email protected] >>> http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev >>> >>> >> >> >> > This email is subject to the disclaimer of Smile Communications at > http://www.smilecoms.com/disclaimer > > > > -- > Daniel-Constantin Mierla - http://www.asipto.comhttp://twitter.com/#!/miconda > - http://www.linkedin.com/in/miconda > > This email is subject to the disclaimer of Smile Communications at http://www.smilecoms.com/disclaimer
tm_async_reply_support.patch_2
Description: Binary data
_______________________________________________ sr-dev mailing list [email protected] http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
