usrloc/nathelper: Refactor code for cachedb-only pinging

modules/nathelper/nathelper.c

@@ -1367,7 +1367,7 @@ nh_timer(unsigned int ticks, void *timer_idx)
 	struct socket_info* send_sock;
 	unsigned int flags;
 	struct proxy_l next_hop;
-	uint64_t contact_id=0;
+	ucontact_coords ct_coords;
 
 	udomain_t *d;
 
@@ -1435,8 +1435,8 @@ nh_timer(unsigned int ticks, void *timer_idx)
 			cp = (char*)cp + sizeof(next_hop);
 
 			if (STORE_BRANCH_CTID) {
-				memcpy(&contact_id, cp, sizeof(contact_id));
-				cp = (char*)cp + sizeof(contact_id);
+				memcpy(&ct_coords, cp, sizeof ct_coords);
+				cp = (char*)cp + sizeof ct_coords;
 			}
 
 			if (next_hop.proto != PROTO_NONE && next_hop.proto != PROTO_UDP &&
@@ -1469,7 +1469,7 @@ nh_timer(unsigned int ticks, void *timer_idx)
 
 			if ((flags & sipping_flag) &&
 			    (opt.s = build_sipping(d, &c, send_sock, &path, &opt.len,
-			                         contact_id, ctid_match_enabled(flags)))) {
+			                         ct_coords, ctid_match_enabled(flags)))) {
 				if (msg_send(send_sock, next_hop.proto, &to, 0, opt.s, opt.len, NULL) < 0) {
 					LM_ERR("sip msg_send failed\n");
 				}
@@ -1677,7 +1677,7 @@ static void
 ping_checker_timer(unsigned int ticks, void *timer_idx)
 {
 	time_t ctime;
-	uint64_t _contact_id;
+	ucontact_coords ct_coords;
 
 	udomain_t *_d;
 	struct nh_table *table;
@@ -1743,38 +1743,39 @@ ping_checker_timer(unsigned int ticks, void *timer_idx)
 			/* ping confirmed and unlinked from hash; only free the cell */
 			prev = cell;
 			cell = cell->tnext;
+			ul.free_ucontact_coords(prev->ct_coords);
 			shm_free(prev);
 			continue;
 		}
 
-
 		/* we need lock since we don't know whether we will remove this
 		 * cell from the list or not */
 		lock_hash(cell->hash_id);
 
 		/* for these cells threshold has been exceeded */
-		LM_DBG("cell with cid %llu has %d unresponded pings\n",
-				(long long unsigned int)cell->contact_id, cell->not_responded);
+		LM_DBG("cell with ucoords %llu has %d unresponded pings\n",
+		       (unsigned long long)cell->ct_coords, cell->not_responded);
 		cell->not_responded++;
 
 		if (cell->not_responded >= max_pings_lost) {
-			LM_DBG("cell with cid %llu exceeded max pings threshold! removing...\n",
-					(long long unsigned int)cell->contact_id);
-			_contact_id = cell->contact_id;
+			LM_DBG("cell with ucoords %llu exceeded max failed pings! "
+			       "removing...\n", (unsigned long long)cell->ct_coords);
+			ct_coords = cell->ct_coords;
 			_d = cell->d;
 
-			remove_given_cell(cell, &table->entries[cell->hash_id]);
+			remove_from_hash(cell);
 
 			/* we put the lock on cell which now moved into prev */
 			unlock_hash(cell->hash_id);
 
 			prev = cell;
 			cell = cell->tnext;
 
+			ul.free_ucontact_coords(prev->ct_coords);
 			shm_free(prev);
 
-			if (rm_on_to_flag && ul.delete_ucontact_from_id &&
-				ul.delete_ucontact_from_id(_d, _contact_id, 0) < 0) {
+			if (rm_on_to_flag && ul.delete_ucontact_from_coords &&
+				ul.delete_ucontact_from_coords(_d, ct_coords, 0) < 0) {
 				/* we keep going since it might work for other contacts */
 				LM_ERR("failed to remove ucontact from db\n");
 			}

modules/nathelper/nh_table.c

@@ -86,7 +86,8 @@ struct nh_table* get_htable(void)
 	return n_table;
 }
 
-struct ping_cell *build_p_cell(int hash_id, udomain_t* d, uint64_t contact_id)
+struct ping_cell *build_p_cell(int hash_id, udomain_t* d,
+                               ucontact_coords ct_coords)
 {
 	struct ping_cell *cell;
 
@@ -101,7 +102,7 @@ struct ping_cell *build_p_cell(int hash_id, udomain_t* d, uint64_t contact_id)
 	cell->hash_id   = hash_id;
 	cell->timestamp = now;
 	cell->d         = d;
-	cell->contact_id = contact_id;
+	cell->ct_coords = ct_coords;
 
 	return cell;
 }
@@ -125,29 +126,28 @@ void insert_into_hash( struct ping_cell* p_cell)
 	entry->first = p_cell;
 }
 
-struct ping_cell *get_cell(int hash_id, uint64_t contact_id)
+struct ping_cell *get_cell(int hash_id, ucontact_coords coords)
 {
 	struct nh_entry *entry;
-	struct ping_cell* cell=NULL;
+	struct ping_cell *cell;
 
 	entry = &n_table->entries[hash_id];
 	cell = entry->first;
 
 	for (cell=entry->first; cell; cell = cell->next) {
-		if (cell->contact_id == contact_id)
+		if (cell->ct_coords == coords)
 			return cell;
 	}
 
 	return NULL;
 }
 
-/*
- * needs to be called under lock
- * only removes cell from hash
- * also frees the cell
- */
-void remove_given_cell(struct ping_cell *cell, struct nh_entry *entry)
+/* must be called under lock */
+void remove_from_hash(struct ping_cell *cell)
 {
+	struct nh_entry *entry;
+
+	entry = &n_table->entries[cell->hash_id];
 
 	if (cell == entry->first && cell == entry->last) {
 		entry->first = entry->last = 0;
@@ -162,4 +162,3 @@ void remove_given_cell(struct ping_cell *cell, struct nh_entry *entry)
 		cell->next->prev = cell->prev;
 	}
 }
-

modules/nathelper/nh_table.h

@@ -45,7 +45,7 @@ struct ping_cell {
 
 	udomain_t* d;
 
-	uint64_t contact_id;
+	ucontact_coords ct_coords; /* !< helps locate the associated contact */
 
 	unsigned int timestamp; /* !< timestamp when ping was sent */
 	char not_responded; /* !< number of pings not responded to */
@@ -98,7 +98,7 @@ void free_hash_table();
 struct ping_cell *build_p_cell(int hash_id, udomain_t* d, uint64_t contact_id);
 void insert_into_hash( struct ping_cell* p_cell);
 struct ping_cell *get_cell(int hash_id, uint64_t contact_id);
-void remove_given_cell(struct ping_cell *cell, struct nh_entry *entry);
+void remove_from_hash(struct ping_cell *cell);
 
 
 #endif

modules/nathelper/sip_pinger.h

@@ -95,10 +95,10 @@ static void init_sip_ping(int _match_ctid)
 
 static int parse_branch(str branch)
 {
-	unsigned int hash_id;
-	int cid_len, sipping_latency;
+	unsigned int hash_id, label;
+	int sipping_latency;
 	char *end;
-	uint64_t contact_id=0;
+	ucontact_coords ct_coords;
 	struct timeval timeval_st;
 
 	struct ping_cell *p_cell;
@@ -114,37 +114,28 @@ static int parse_branch(str branch)
 	branch.len -= BMAGIC_LEN;
 
 	end = q_memchr(branch.s, '.', branch.len);
-	if (0 == end) {
+	if (!end) {
 		/* if reverse hex2int succeeds on this it's a simple
 		 * ping based on sipping_callid_cnt label */
-		if (reverse_hex2int(branch.s, end-branch.s, &hash_id)==0)
+		if (reverse_hex2int(branch.s, end - branch.s, &label) == 0)
 			return 0;
 
 		return 1;
 	}
 
-	reverse_hex2int(branch.s, end-branch.s, &hash_id);
+	reverse_hex2int(branch.s, end - branch.s, &hash_id);
 
-	branch.len -= (end-branch.s + 1);
-	branch.s = end+1;
+	branch.len -= (end - branch.s + 1);
+	branch.s = end + 1;
 
-
-	if (0 == end)
-		return 1;
-
-	end = q_memchr(branch.s, '.', branch.len);
-	cid_len = end-branch.s;
-	reverse_hex2int64(branch.s, cid_len, 1/* request unsafe parsing */,
-		&contact_id);
 	/* reverse_hex2int64() cannot fail in unsafe mode and it will return 
 	   whatever it was able to parse (0 if nothing )*/
-
-	/* we don't parse the label since we don't need it */
+	reverse_hex2int64(branch.s, branch.len, 1, &ct_coords);
 
 	lock_hash(hash_id);
-	if ((p_cell=get_cell(hash_id, contact_id))==NULL) {
-		LM_WARN("received ping response for a removed contact"
-				" with contact id %llu\n", (long long unsigned int)contact_id);
+	p_cell = get_cell(hash_id, ct_coords);
+	if (!p_cell) {
+		LM_WARN("received ping response for a removed contact\n");
 		unlock_hash(hash_id);
 		return 0;
 	}
@@ -155,10 +146,9 @@ static int parse_branch(str branch)
 
 	LM_DBG("update_sipping_latency with %d us\n", sipping_latency);
 	if (ul.update_sipping_latency &&
-		ul.update_sipping_latency(p_cell->d, p_cell->contact_id,
-		                          sipping_latency) < 0) {
+	    ul.update_sipping_latency(p_cell->d, ct_coords, sipping_latency) < 0) {
 		/* we keep going since it might work for other contacts */
-		LM_ERR("failed to update_sipping_latency ucontact from db\n");
+		LM_ERR("failed to update ucontact sipping_latency\n");
 	}
 	/* when we receive answer to a ping we consider all pings sent
 	 * confirmed, because what we want to know is that the contact
@@ -168,10 +158,8 @@ static int parse_branch(str branch)
 	/* mark for removal */
 	p_cell->timestamp = 0;
 
-	remove_given_cell(p_cell, &get_htable()->entries[p_cell->hash_id]);
-
+	remove_from_hash(p_cell);
 	unlock_hash(hash_id);
-
 	return 0;
 }
 
@@ -223,7 +211,7 @@ static int sipping_rpl_filter(struct sip_msg *rpl)
 
 static inline int
 build_branch(char *branch, int *size,
-		str *curi, udomain_t *d, uint64_t contact_id, int match_ctid)
+		str *curi, udomain_t *d, ucontact_coords ct_coords, int match_ctid)
 {
 	int hash_id, ret, label;
 	time_t timestamp;
@@ -241,8 +229,8 @@ build_branch(char *branch, int *size,
 		gettimeofday(&timeval_st, NULL);
 
 		lock_hash(hash_id);
-		if ((p_cell=get_cell(hash_id, contact_id))==NULL) {
-			if (0 == (p_cell = build_p_cell(hash_id, d, contact_id))) {
+		if ((p_cell=get_cell(hash_id, ct_coords))==NULL) {
+			if (0 == (p_cell = build_p_cell(hash_id, d, ct_coords))) {
 				unlock_hash(hash_id);
 				goto out_memfault;
 			}
@@ -288,18 +276,15 @@ build_branch(char *branch, int *size,
 		*branch = '.';
 		branch++;
 
-		ret=int64_2reverse_hex(&branch, size, contact_id);
+		ret=int64_2reverse_hex(&branch, size, ct_coords);
+		if (ret < 0)
+			goto out_nospace;
+	} else {
+		ret=int2reverse_hex(&branch, size, label);
 		if (ret < 0)
 			goto out_nospace;
-
-		*branch = '.';
-		branch++;
 	}
 
-	ret=int2reverse_hex(&branch, size, label);
-	if (ret < 0)
-		goto out_nospace;
-
 	*branch = '\0';
 
 	return 0;
@@ -317,7 +302,7 @@ build_branch(char *branch, int *size,
 /* build the buffer of a SIP ping request */
 static inline char*
 build_sipping(udomain_t *d, str *curi, struct socket_info* s,str *path,
-		int *len_p, uint64_t contact_id, int match_ctid)
+		int *len_p, ucontact_coords ct_coords, int match_ctid)
 {
 #define s_len(_s) (sizeof(_s)-1)
 	static char buf[MAX_SIPPING_SIZE];
@@ -335,7 +320,7 @@ build_sipping(udomain_t *d, str *curi, struct socket_info* s,str *path,
 	bbuild += sizeof(BSTART) - 1;
 	bsize -= (bbuild - branch);
 
-	build_branch( bbuild, &bsize, curi, d, contact_id, match_ctid);
+	build_branch( bbuild, &bsize, curi, d, ct_coords, match_ctid);
 
 	sbranch.s = branch;
 	sbranch.len = strlen(branch);