core: remove syn_branch functionality for calculating Via branch parameter

* remove syn_branch parameter and functionality from core and tm module for
  calculating the Via branch parameter
* reported from Richard Brady, rnbrady at gmail dot com to sr-dev
* kamailio is not standard compliant with this setting enabled (RFC 3261,
  17.2.3 and 16.11) for stateless forwarding of replies
* the performance reason that motivated this functionality are today not
  valid anymore, even embedded systems have more than enough power to
  calculate MD5 and other modules uses more expensive operations anyway
* adapt a bunch of example and test configuration that used this parameter,
  it has been also removed from the core cookbook wiki

cfg.lex

@@ -393,7 +393,6 @@ CHILDREN children
 SOCKET_WORKERS socket_workers
 CHECK_VIA	check_via
 PHONE2TEL	phone2tel
-SYN_BRANCH syn_branch
 MEMLOG		"memlog"|"mem_log"
 MEMDBG		"memdbg"|"mem_dbg"
 MEMSUM		"mem_summary"
@@ -792,7 +791,6 @@ IMPORTFILE      "import_file"
 <INITIAL>{SOCKET_WORKERS}	{ count(); yylval.strval=yytext; return SOCKET_WORKERS; }
 <INITIAL>{CHECK_VIA}	{ count(); yylval.strval=yytext; return CHECK_VIA; }
 <INITIAL>{PHONE2TEL}	{ count(); yylval.strval=yytext; return PHONE2TEL; }
-<INITIAL>{SYN_BRANCH}	{ count(); yylval.strval=yytext; return SYN_BRANCH; }
 <INITIAL>{MEMLOG}	{ count(); yylval.strval=yytext; return MEMLOG; }
 <INITIAL>{MEMDBG}	{ count(); yylval.strval=yytext; return MEMDBG; }
 <INITIAL>{MEMSUM}	{ count(); yylval.strval=yytext; return MEMSUM; }

cfg.y

@@ -440,7 +440,6 @@ extern char *finame;
 %token SOCKET_WORKERS
 %token CHECK_VIA
 %token PHONE2TEL
-%token SYN_BRANCH
 %token MEMLOG
 %token MEMDBG
 %token MEMSUM
@@ -958,8 +957,6 @@ assign_stm:
 	| CHECK_VIA EQUAL error { yyerror("boolean value expected"); }
 	| PHONE2TEL EQUAL NUMBER { phone2tel=$3; }
 	| PHONE2TEL EQUAL error { yyerror("boolean value expected"); }
-	| SYN_BRANCH EQUAL NUMBER { syn_branch=$3; }
-	| SYN_BRANCH EQUAL error { yyerror("boolean value expected"); }
 	| MEMLOG EQUAL intno { default_core_cfg.memlog=$3; }
 	| MEMLOG EQUAL error { yyerror("int value expected"); }
 	| MEMDBG EQUAL intno { default_core_cfg.memdbg=$3; }

examples/pcscf/kamailio.cfg

@@ -46,7 +46,6 @@ sip_warning=no
 children=64
 #!endif
 
-syn_branch=0
 # Locks all ser pages into memory making it unswappable (in general one 
 # doesn't want his sip proxy swapped out )
 mlock_pages=yes

examples/websocket.cfg

@@ -45,8 +45,6 @@ tcp_connection_lifetime=3604
 tcp_accept_no_cl=yes
 tcp_rd_buf_size=16384
 
-syn_branch=0
-
 #!ifdef LOCAL_TEST_RUN
 debug=2
 mpath="modules"

forward.c

@@ -535,31 +535,23 @@ int forward_request(struct sip_msg* msg, str* dst, unsigned short port,
 		}
 	}/* dst */
 	send_info->send_flags=msg->fwd_send_flags;
-	/* calculate branch for outbound request;  if syn_branch is turned off,
+	/* calculate branch for outbound request;
 	   calculate is from transaction key, i.e., as an md5 of From/To/CallID/
 	   CSeq exactly the same way as TM does; good for reboot -- than messages
 	   belonging to transaction lost due to reboot will still be forwarded
 	   with the same branch parameter and will be match-able downstream
-	
-	   if it is turned on, we don't care about reboot; we simply put a simple
-	   value in there; better for performance
 	*/
-	if (syn_branch ) {
-	        memcpy(msg->add_to_branch_s, "z9hG4bKcydzigwkX", 16);
-		msg->add_to_branch_len=16;
-	} else {
-		if (!char_msg_val( msg, md5 )) 	{ /* parses transaction key */
-			LOG(L_ERR, "ERROR: forward_request: char_msg_val failed\n");
-			ret=E_UNSPEC;
-			goto error;
-		}
-		msg->hash_index=hash( msg->callid->body, get_cseq(msg)->number);
-		if (!branch_builder( msg->hash_index, 0, md5, 0 /* 0-th branch */,
-					msg->add_to_branch_s, &msg->add_to_branch_len )) {
-			LOG(L_ERR, "ERROR: forward_request: branch_builder failed\n");
-			ret=E_UNSPEC;
-			goto error;
-		}
+	if (!char_msg_val( msg, md5 )) 	{ /* parses transaction key */
+		LOG(L_ERR, "ERROR: forward_request: char_msg_val failed\n");
+		ret=E_UNSPEC;
+		goto error;
+	}
+	msg->hash_index=hash( msg->callid->body, get_cseq(msg)->number);
+	if (!branch_builder( msg->hash_index, 0, md5, 0 /* 0-th branch */,
+				msg->add_to_branch_s, &msg->add_to_branch_len )) {
+		LOG(L_ERR, "ERROR: forward_request: branch_builder failed\n");
+		ret=E_UNSPEC;
+		goto error;
 	}
 	/* try to send the message until success or all the ips are exhausted
 	 *  (if dns lookup is performed && the dns cache used ) */

globals.h

@@ -111,7 +111,6 @@ extern int dont_daemonize;
 extern int check_via;
 extern int phone2tel;
 extern int received_dns;
-extern int syn_branch;
 /* extern int process_no; */
 extern int child_rank;
 extern int sip_warning;

main.c

@@ -383,8 +383,6 @@ int config_check = 0;
 int check_via =  0;
 /* translate user=phone URIs to TEL URIs */
 int phone2tel = 1;
-/* shall use stateful synonym branches? faster but not reboot-safe */
-int syn_branch = 1;
 /* debugging level for timer debugging */
 int timerlog = L_WARN;
 /* should replies include extensive warnings? by default no,

modules/tm/h_table.c

@@ -256,27 +256,25 @@ static inline void init_synonym_id( struct cell *t )
 	char *c;
 	unsigned int myrand;
 
-	if (!syn_branch) {
-		p_msg=t->uas.request;
-		if (p_msg) {
-			/* char value of a proxied transaction is
-			   calculated out of header-fields forming
-			   transaction key
-			*/
-			char_msg_val( p_msg, t->md5 );
-		} else {
-			/* char value for a UAC transaction is created
-			   randomly -- UAC is an originating stateful element 
-			   which cannot be refreshed, so the value can be
-			   anything
-			*/
-			/* HACK : not long enough */
-			myrand=rand();
-			c=t->md5;
-			size=MD5_LEN;
-			memset(c, '0', size );
-			int2reverse_hex( &c, &size, myrand );
-		}
+	p_msg=t->uas.request;
+	if (p_msg) {
+		/* char value of a proxied transaction is
+		   calculated out of header-fields forming
+		   transaction key
+		*/
+		char_msg_val( p_msg, t->md5 );
+	} else {
+		/* char value for a UAC transaction is created
+		   randomly -- UAC is an originating stateful element
+		   which cannot be refreshed, so the value can be
+		   anything
+		*/
+		/* HACK : not long enough */
+		myrand=rand();
+		c=t->md5;
+		size=MD5_LEN;
+		memset(c, '0', size );
+		int2reverse_hex( &c, &size, myrand );
 	}
 }
 
@@ -309,10 +307,9 @@ struct cell*  build_cell( struct sip_msg* p_msg )
 	sr_xavp_t** xold;
 #endif
 
-	/* allocs a new cell */
-	/* if syn_branch==0 add space for md5 (MD5_LEN -sizeof(struct cell.md5)) */
+	/* allocs a new cell, add space for md5 (MD5_LEN - sizeof(struct cell.md5)) */
 	new_cell = (struct cell*)shm_malloc( sizeof( struct cell )+
-			((MD5_LEN-sizeof(((struct cell*)0)->md5))&((syn_branch!=0)-1)) );
+			MD5_LEN-sizeof(((struct cell*)0)->md5) );
 	if  ( !new_cell ) {
 		ser_error=E_OUT_OF_MEM;
 		return NULL;

modules/tm/h_table.h

@@ -455,8 +455,8 @@ typedef struct cell
 	 /* The route to take for each downstream branch separately */
 	unsigned short on_branch;
 
-	/* place holder for MD5checksum  (meaningful only if syn_branch=0) */
-	char md5[0]; /* if syn_branch==0 then MD5_LEN bytes are extra alloc'ed*/
+	/* place holder for MD5checksum, MD5_LEN bytes are extra alloc'ed */
+	char md5[0];
 
 } tm_cell_t;
 

modules/tm/t_lookup.c

@@ -13,15 +13,8 @@
  * reply and both  of them are constructed by different software.
  * 
  * As for reply matching, we match based on branch value -- that is
- * faster too. There are two versions .. with SYNONYMs #define
- * enabled, the branch includes ordinal number of a transaction
- * in a synonym list in hash table and is somewhat faster but
- * not reboot-resilient. SYNONYMs turned off are little slower
- * but work across reboots as well.
- *
- * The branch parameter is formed as follows:
- * SYNONYMS  on: hash.synonym.branch
- * SYNONYMS off: hash.md5.branch
+ * faster too.
+ * The branch parameter is formed as follows: hash.md5.branch
  *
  * -jiri
  *
@@ -892,16 +885,12 @@ int t_reply_matching( struct sip_msg *p_msg , int *p_branch )
 
 	char *loopi;
 	int loopl;
-	char *syni;
-	int synl;
 	
 	short is_cancel;
 
 	/* make compiler warnings happy */
 	loopi=0;
 	loopl=0;
-	syni=0;
-	synl=0;
 
 	/* split the branch into pieces: loop_detection_check(ignored),
 	 hash_table_id, synonym_id, branch_id */
@@ -928,25 +917,14 @@ int t_reply_matching( struct sip_msg *p_msg , int *p_branch )
 	hashi=p;
 	p=n+1;scan_space--;
 
-	if (!syn_branch) {
-		/* md5 value */
-		n=eat_token2_end( p, p+scan_space, BRANCH_SEPARATOR );
-		loopl = n-p;
-		scan_space-= loopl;
-		if (n==p || scan_space<2 || *n!=BRANCH_SEPARATOR) 
-			goto nomatch2;
-		loopi=p;
-		p=n+1; scan_space--;
-	} else {
-		/* synonym id */
-		n=eat_token2_end( p, p+scan_space, BRANCH_SEPARATOR);
-		synl=n-p;
-		scan_space-=synl;
-		if (!synl || scan_space<2 || *n!=BRANCH_SEPARATOR) 
-			goto nomatch2;
-		syni=p;
-		p=n+1;scan_space--;
-	}
+	/* md5 value */
+	n=eat_token2_end( p, p+scan_space, BRANCH_SEPARATOR );
+	loopl = n-p;
+	scan_space-= loopl;
+	if (n==p || scan_space<2 || *n!=BRANCH_SEPARATOR)
+		goto nomatch2;
+	loopi=p;
+	p=n+1; scan_space--;
 
 	/* branch id  -  should exceed the scan_space */
 	n=eat_token_end( p, p+scan_space );
@@ -959,8 +937,7 @@ int t_reply_matching( struct sip_msg *p_msg , int *p_branch )
 		||hash_index>=TABLE_ENTRIES
 		|| reverse_hex2int(branchi, branchl, &branch_id)<0
 		||branch_id>=MAX_BRANCHES
-		|| (syn_branch ? (reverse_hex2int(syni, synl, &entry_label))<0 
-			: loopl!=MD5_LEN ))
+		|| loopl!=MD5_LEN)
 	) {
 		DBG("DEBUG: t_reply_matching: poor reply labels %d label %d "
 			"branch %d\n", hash_index, entry_label, branch_id );
@@ -983,14 +960,8 @@ int t_reply_matching( struct sip_msg *p_msg , int *p_branch )
 	/* all the transactions from the entry are compared */
 	clist_foreach(hash_bucket, p_cell, next_c){
 		prefetch_loc_r(p_cell->next_c, 1);
-		/* first look if branch matches */
-		if (likely(syn_branch)) {
-			if (p_cell->label != entry_label) 
-				continue;
-		} else {
-			if ( memcmp(p_cell->md5, loopi,MD5_LEN)!=0)
+		if ( memcmp(p_cell->md5, loopi,MD5_LEN)!=0)
 					continue;
-		}
 
 		/* sanity check ... too high branch ? */
 		if (unlikely(branch_id>=p_cell->nr_of_outgoings))

modules/tm/t_msgbuilder.c

@@ -1612,11 +1612,7 @@ char* build_uac_req(str* method, str* headers, str* body, dlg_t* dialog, int bra
 int t_calc_branch(struct cell *t, 
 	int b, char *branch, int *branch_len)
 {
-	return syn_branch ?
-		branch_builder( t->hash_index,
-			t->label, 0,
-			b, branch, branch_len )
-		: branch_builder( t->hash_index,
+	return branch_builder( t->hash_index,
 			0, t->md5,
 			b, branch, branch_len );
 }

test/sf.cfg

@@ -18,7 +18,6 @@ rev_dns=no	# (cmd. line: -R)
 port=5060
 #port=8060
 
-syn_branch=1
 fifo="/tmp/ser_fifo"
 
 # advertise IP address in Via (as opposed to advertising DNS name

test/stress.cfg

@@ -11,7 +11,6 @@ fork=1
 log_stderror=yes	# (cmd line: -E)
 check_via=no # (cmd. line: -v)
 dns=no # (cmd. line: -r)
-syn_branch=1
 reply_to_via=0
 fifo="/tmp/ser_fifo"
 

test/tcp.cfg

@@ -25,7 +25,6 @@ rev_dns=off      # (cmd. line: -R)
 # for more info: sip_router -h
 alias=iptel.org
 alias="foo.bar"
-syn_branch=0
 fifo="/tmp/ser_fifo"
 
 #modules