registrar: Fix ping latency-based contact sorting

serialize_branches() was designed to always sort branches in descending
"q" value order. This patch adds an optional serialize_branches() parameter
which disables this behavior.

Author: liviuchircu

action.c

@@ -1958,27 +1958,18 @@ int do_action(struct action* a, struct sip_msg* msg)
 			break;
 		case SERIALIZE_BRANCHES_T:
 			script_trace("core", "serialize_branches", msg, a->file, a->line) ;
-			if (a->elem[0].type!=NUMBER_ST){
-				LM_ALERT("BUG in serialize_branches argument"
-						" type: %d\n", a->elem[0].type);
-				ret=E_BUG;
-				break;
-			}
-			if (serialize_branches(msg,(int)a->elem[0].u.number)!=0) {
+			if (serialize_branches(msg,(int)a->elem[0].u.number,
+						a->elem[1].u.data ? (int)a->elem[1].u.number : 0)!=0) {
 				LM_ERR("serialize_branches failed\n");
 				ret=E_UNSPEC;
 				break;
 			}
-			ret=1; /* continue processing */
+			ret=1;
 			break;
 		case NEXT_BRANCHES_T:
 			script_trace("core", "next_branches", msg, a->file, a->line) ;
-			if ((ret=next_branches(msg))<0) {
-				LM_ERR("next_branches failed\n");
-				ret=E_UNSPEC;
-				break;
-			}
-			/* continue processing */
+			if ((ret = next_branches(msg)) < 0)
+				LM_DBG("no more branches\n");
 			break;
 		case EQ_T:
 		case COLONEQ_T:

cfg.y

@@ -2316,8 +2316,13 @@ cmd:	 FORWARD LPAREN STRING RPAREN	{ mk_action2( $$, FORWARD_T,
 								}
 		| FORCE_SEND_SOCKET error {$$=0; yyerror("missing '(' or ')' ?"); }
 		| SERIALIZE_BRANCHES LPAREN NUMBER RPAREN {
+								mk_action1( $$, SERIALIZE_BRANCHES_T,
+									NUMBER_ST, (void*)(long)$3);
+								}
+		| SERIALIZE_BRANCHES LPAREN NUMBER COMMA NUMBER RPAREN {
 								mk_action2( $$, SERIALIZE_BRANCHES_T,
-									NUMBER_ST, 0, (void*)(long)$3, 0);
+									NUMBER_ST, NUMBER_ST,
+									(void*)(long)$3, (void*)(long)$5);
 								}
 		| SERIALIZE_BRANCHES LPAREN error RPAREN {$$=0; yyerror("bad argument,"
 								" number expected");

modules/registrar/lookup.c

@@ -73,11 +73,21 @@ static str branch_uris[MAX_BRANCHES-1];
 ucontact_t **sorted_cts; /* always has an extra terminating NULL ptr */
 int sorted_cts_sz = 20;
 
-static int cmp_ucontact(const void *ct1, const void *ct2)
+static int cmp_ucontact(const void *_ct1, const void *_ct2)
 {
-	return
-		(*(ucontact_t **)ct1)->sipping_latency -
-		(*(ucontact_t **)ct2)->sipping_latency;
+	ucontact_t *ct1 = *(ucontact_t **)_ct1, *ct2 = *(ucontact_t **)_ct2;
+
+	if (ct1->sipping_latency == 0) {
+		if (ct2->sipping_latency == 0)
+			return 0;
+
+		return 1;
+	}
+
+	if (ct2->sipping_latency == 0)
+		return -1;
+
+	return ct1->sipping_latency - ct2->sipping_latency;
 }
 
 ucontact_t **sanitize_contacts(ucontact_t *contacts, int flags, int max_latency)

serialize.c

@@ -72,7 +72,7 @@ int init_serialization(void)
  * contact is the last one in its priority class.  Finally, removes
  * all branches from destination set.
  */
-int serialize_branches(struct sip_msg *msg, int clean_before )
+int serialize_branches(struct sip_msg *msg, int clean_before, int keep_order)
 {
 	static struct serial_contact contacts[MAX_BRANCHES];
 	int n, last, first, i, prev;
@@ -100,7 +100,7 @@ int serialize_branches(struct sip_msg *msg, int clean_before )
 			break;
 	}
 
-	if (branch.s == 0) {
+	if (branch.s == 0 && !keep_order) {
 		LM_DBG("nothing to do - all same q!\n");
 		return 0;
 	}
@@ -185,8 +185,15 @@ int serialize_branches(struct sip_msg *msg, int clean_before )
 		contacts[n].enc_info = enc_info;
 		contacts[n].q = q;
 
-		/* insert based on q */
-		for (i = first, prev=-1; i != -1 && contacts[i].q < q; prev=i,i = contacts[i].next);
+		if (keep_order) {
+			contacts[n].next = first;
+			first = n++;
+			continue;
+		}
+
+		/* insert based on ascending q values, so add_avp() reverses them */
+		for (i = first, prev = -1;
+		     i != -1 && contacts[i].q < q; prev = i ,i = contacts[i].next);
 
 		if (i == -1) {
 			/* append */
@@ -209,7 +216,7 @@ int serialize_branches(struct sip_msg *msg, int clean_before )
 
 	/* Assign values for q_flags */
 	for (i = first; contacts[i].next != -1; i = contacts[i].next) {
-		if (contacts[i].q < contacts[contacts[i].next].q)
+		if (keep_order || contacts[i].q < contacts[contacts[i].next].q)
 			contacts[contacts[i].next].q_flag = Q_FLAG;
 		else
 			contacts[contacts[i].next].q_flag = 0;

serialize.h

@@ -42,7 +42,7 @@ int init_serialization();
 /*! \brief converts the destination set (for parallel forking) into AVPS used
  * for serial forking.
  */
-int serialize_branches(struct sip_msg *msg, int clean_before );
+int serialize_branches(struct sip_msg *msg, int clean_before, int keep_order);
 
 
 /*! \brief gets the next branches for serial forking