Re: [sr-dev] git:master: dialog(k): postpone setting tm callbacks until T is created

Hi Sven,

sorry, took a while to get back to this subject.

The fix seems ok, being tested a bit in my side. I didn't wanted to backport it immediately, just to give a second thought about it. The purpose was to backport, of course.

Btw, have you had a chance to test it?

If you need it in 3.2 quickly, go ahead and cherry-pick it on stable branch. If not, I will do it, probably soon as well.

Cheers, Daniel

On 11/2/11 4:27 PM, Sven Knoblich wrote:

Hi Daniel, i saw your changes regarding the stateless reply in dialog-module. Should we push this commit as well into the 3.2 branch?

Thanks for your reply.

Bye, Sven

Am 27.10.2011 16:16, schrieb Daniel-Constantin Mierla:

...

Module: sip-router Branch: master Commit: ad4cfe8a3410059bc5c0b4951e49a952c4b01dfe URL:http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=ad4cfe8a...

Author: Daniel-Constantin Mierlamiconda@gmail.com Committer: Daniel-Constantin Mierlamiconda@gmail.com Date: Thu Oct 27 07:17:52 2011 +0200

dialog(k): postpone setting tm callbacks until T is created

• add dialog in tm callbacks when transaction is created
• when using dlg_manage(), if dialog does not makes it to transaction, clean it up
• fixes case when stateless reply is used after dlg_manage() to create a new dialog

---

modules_k/dialog/dialog.c | 3 +- modules_k/dialog/dlg_handlers.c | 95 ++++++++++++++++++++++++++------------- modules_k/dialog/dlg_hash.h | 2 + modules_k/dialog/dlg_profile.c | 7 +++- 4 files changed, 73 insertions(+), 34 deletions(-)

diff --git a/modules_k/dialog/dialog.c b/modules_k/dialog/dialog.c index 7d8f239..14e8d81 100644 --- a/modules_k/dialog/dialog.c +++ b/modules_k/dialog/dialog.c @@ -507,7 +507,8 @@ static int mod_init(void) }

if (initial_cbs_inscript != 0&& initial_cbs_inscript != 1) {

• LM_ERR("invalid parameter for running initial callbacks in-script (must be either 0 or 1)\n");
  

• LM_ERR("invalid parameter for running initial callbacks in-script"
  

• 		" (must be either 0 or 1)\n");
  

  return -1; }

diff --git a/modules_k/dialog/dlg_handlers.c b/modules_k/dialog/dlg_handlers.c index ccc95f1..701d80c 100644 --- a/modules_k/dialog/dlg_handlers.c +++ b/modules_k/dialog/dlg_handlers.c @@ -106,6 +106,8 @@ static unsigned int CURR_DLG_ID = 0xffffffff; /*!< current dialog id */ /*! separator inside the record-route paramter */ #define DLG_SEPARATOR '.'

+int dlg_set_tm_callbacks(tm_cell_t *t, sip_msg_t *req, dlg_cell_t *dlg,

• int mode);
  
  

  /*!

  • \brief Initialize the dialog handlers

@@ -609,19 +611,28 @@ static inline int pre_match_parse( struct sip_msg *req, str *callid, */ void dlg_onreq(struct cell* t, int type, struct tmcb_params *param) {

• struct sip_msg *req = param->req;

• sip_msg_t *req = param->req;

• if (!initial_cbs_inscript) {

• if (spiral_detected == 1)
  

• 	run_dlg_callbacks( DLGCB_SPIRALED, current_dlg_pointer, req, NULL, DLG_DIR_DOWNSTREAM, 0);
  

• else if (spiral_detected == 0)
  

• 	run_create_callbacks( current_dlg_pointer, req);
  

• }
• if((req->flags&dlg_flag)!=dlg_flag)

• return;
  

• if (current_dlg_pointer!=NULL)

• if(req->first_line.u.request.method_value != METHOD_INVITE) return;

• dlg_new_dialog(req, t, 1);

• if (current_dlg_pointer!=NULL) {

• if (!initial_cbs_inscript) {
  

• 	if (spiral_detected == 1)
  

• 		run_dlg_callbacks( DLGCB_SPIRALED, current_dlg_pointer,
  

• 				req, NULL, DLG_DIR_DOWNSTREAM, 0);
  

• 	else if (spiral_detected == 0)
  

• 		run_create_callbacks( current_dlg_pointer, req);
  

• }
  

• }
• if (current_dlg_pointer==NULL) {

• if((req->flags&dlg_flag)!=dlg_flag)
  

• 	return;
  

• dlg_new_dialog(req, t, 1);
  

• }
• if (current_dlg_pointer!=NULL) {

• dlg_set_tm_callbacks(t, req, current_dlg_pointer, spiral_detected);
  

• } }

@@ -782,9 +793,10 @@ int dlg_new_dialog(struct sip_msg *req, struct cell *t, const int run_initial_cb spiral_detected = 1;

          if (run_initial_cbs)

•            run_dlg_callbacks( DLGCB_SPIRALED, dlg, req, NULL, DLG_DIR_DOWNSTREAM, 0);
  

•        // get_dlg has incremented the ref count by 1
  

•        unref_dlg(dlg, 1);
  

•            run_dlg_callbacks( DLGCB_SPIRALED, dlg, req, NULL,
  

• 				DLG_DIR_DOWNSTREAM, 0);
  

•        /* get_dlg() has incremented the ref count by 1
  

• 	 * - it's ok, dlg will be used to set current_dialog_pointer */
       goto finish;
   }
  

  }

@@ -811,7 +823,6 @@ int dlg_new_dialog(struct sip_msg *req, struct cell *t, const int run_initial_cb return -1; }

• /* Populate initial varlist: */ dlg->vars = get_local_varlist_pointer(req, 1);

@@ -826,13 +837,7 @@ int dlg_new_dialog(struct sip_msg *req, struct cell *t, const int run_initial_cb goto error; }

• if ( d_tmb.register_tmcb( req, t,

• 		TMCB_RESPONSE_READY|TMCB_RESPONSE_FWDED,
  

• 		dlg_onreply, (void*)dlg, unref_new_dialog)<0 ) {
  

• LM_ERR("failed to register TMCB\n");
  

• goto error;
  

• }
• // increase reference counter because of registered callback

• /* reference it once for current_dialog_pointer */ ref_dlg(dlg, 1);

  dlg->lifetime = get_dlg_timeout(req);

@@ -847,6 +852,40 @@ int dlg_new_dialog(struct sip_msg *req, struct cell *t, const int run_initial_cb if_update_stat( dlg_enable_stats, processed_dlgs, 1);

finish:

• set_current_dialog(req, dlg);
• _dlg_ctx.dlg = dlg;
• return 0;

+error:

• if (!spiral_detected)

• unref_dlg(dlg,1);               // undo ref regarding linking
  

• return -1;

+}

+/*!

• • \brief add dlg structure to tm callbacks
• • \param t current transaction
• • \param req current sip request
• • \param dlg current dialog
• • \param smode if the sip request was spiraled
• • \return 0 on success, -1 on failure
• */

+int dlg_set_tm_callbacks(tm_cell_t *t, sip_msg_t *req, dlg_cell_t *dlg,

• int smode)
  

+{

• if(smode==0) {

• if ( d_tmb.register_tmcb( req, t,
  

• 		TMCB_RESPONSE_READY|TMCB_RESPONSE_FWDED,
  

• 		dlg_onreply, (void*)dlg, unref_new_dialog)<0 ) {
  

• 	LM_ERR("failed to register TMCB\n");
  

• 	goto error;
  

• }
  

• // increase reference counter because of registered callback
  

• ref_dlg(dlg, 1);
  

• }
• if (t) { // transaction exists ==> keep ref counter large enough to // avoid premature cleanup and ensure proper dialog referencing

@@ -854,9 +893,7 @@ finish: LM_ERR("failed to store dialog in transaction\n"); goto error; }

• }
• else
• {

• } else { // no transaction exists ==> postpone work until we see the // request being forwarded statefully if ( d_tmb.register_tmcb( req, NULL, TMCB_REQUEST_FWDED,

@@ -865,16 +902,10 @@ finish: goto error; } }

• set_current_dialog(req, dlg);
• _dlg_ctx.dlg = dlg;
• ref_dlg(dlg, 1);

• dlg->dflags |= DLG_FLAG_TM;

  return 0;

• error:
• if (!spiral_detected)

• unref_dlg(dlg,1);               // undo ref regarding linking
  

  return -1; }

diff --git a/modules_k/dialog/dlg_hash.h b/modules_k/dialog/dlg_hash.h index ecb6526..9a8854c 100644 --- a/modules_k/dialog/dlg_hash.h +++ b/modules_k/dialog/dlg_hash.h @@ -79,6 +79,8 @@ /* dialog-variable flags (in addition to dialog-flags) */ #define DLG_FLAG_DEL (1<<8) /*!< delete this var */

+#define DLG_FLAG_TM (1<<9) /*!< dialog is set in transaction */

• #define DLG_CALLER_LEG 0 /*!< attribute that belongs to a caller leg */ #define DLG_CALLEE_LEG 1 /*!< attribute that belongs to a callee leg */

diff --git a/modules_k/dialog/dlg_profile.c b/modules_k/dialog/dlg_profile.c index 2e69291..bb20c57 100644 --- a/modules_k/dialog/dlg_profile.c +++ b/modules_k/dialog/dlg_profile.c @@ -312,7 +312,12 @@ int profile_cleanup( struct sip_msg *msg, unsigned int flags, void *param ) { current_dlg_msg_id = 0; if (current_dlg_pointer) {

• unref_dlg( current_dlg_pointer, 1);
  

• if(current_dlg_pointer->dflags&  DLG_FLAG_TM) {
  

• 	unref_dlg( current_dlg_pointer, 1);
  

• } else {
  

• 	/* dialog didn't make it to tm */
  

• 	unref_dlg( current_dlg_pointer, 2);
  

• }
  

  current_dlg_pointer = NULL; } if (current_pending_linkers) {

---

sr-dev mailing list sr-dev@lists.sip-router.org http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev