From 5f936a387fae32f4a4f7c11a9cbd5666b31ef9e7 Mon Sep 17 00:00:00 2001 From: Stefan Mititelu Date: Mon, 16 Nov 2015 10:49:36 +0200 Subject: [PATCH] rtpengine: Fix deletion for branching scenarios - hash table entry contains callid, viabranch - hash table lookup based on callid, viabranch (useful for branching scenarios); keep doing the hash table remove right away - remove op param when select_rtpp_node(); not needed --- modules/rtpengine/rtpengine.c | 69 +++++++++-------- modules/rtpengine/rtpengine_hash.c | 116 +++++++++++++++++++---------- modules/rtpengine/rtpengine_hash.h | 12 ++- 3 files changed, 121 insertions(+), 76 deletions(-) diff --git a/modules/rtpengine/rtpengine.c b/modules/rtpengine/rtpengine.c index a11e38459bf..7fa328365f8 100644 --- a/modules/rtpengine/rtpengine.c +++ b/modules/rtpengine/rtpengine.c @@ -193,9 +193,9 @@ static int rtpengine_offer_answer(struct sip_msg *msg, const char *flags, int op static int fixup_set_id(void ** param, int param_no); static int set_rtpengine_set_f(struct sip_msg * msg, char * str1, char * str2); static struct rtpp_set * select_rtpp_set(int id_set); -static struct rtpp_node *select_rtpp_node_new(str, int, int); -static struct rtpp_node *select_rtpp_node_old(str, int, int); -static struct rtpp_node *select_rtpp_node(str, int, int); +static struct rtpp_node *select_rtpp_node_new(str, str, int); +static struct rtpp_node *select_rtpp_node_old(str, str, int); +static struct rtpp_node *select_rtpp_node(str, str, int); static char *send_rtpp_command(struct rtpp_node *, bencode_item_t *, int *); static int get_extra_id(struct sip_msg* msg, str *id_str); @@ -1859,7 +1859,8 @@ static bencode_item_t *rtpp_function_call(bencode_buffer_t *bencbuf, struct sip_ { struct ng_flags_parse ng_flags; bencode_item_t *item, *resp; - str callid, from_tag, to_tag, body, viabranch, error; + str callid = STR_NULL, from_tag = STR_NULL, to_tag = STR_NULL, viabranch = STR_NULL; + str body = STR_NULL, error = STR_NULL; int ret, queried_nodes; struct rtpp_node *node; char *cp; @@ -1992,7 +1993,7 @@ static bencode_item_t *rtpp_function_call(bencode_buffer_t *bencbuf, struct sip_ LM_ERR("queried nodes limit reached\n"); goto error; } - node = select_rtpp_node(callid, 1, op); + node = select_rtpp_node(callid, viabranch, 1); if (!node) { LM_ERR("no available proxies\n"); goto error; @@ -2036,12 +2037,12 @@ static bencode_item_t *rtpp_function_call(bencode_buffer_t *bencbuf, struct sip_ if (op == OP_DELETE) { /* Delete the key<->value from the hashtable */ - if (!rtpengine_hash_table_remove(&callid)) { - LM_ERR("rtpengine hash table failed to remove entry for callen=%d callid=%.*s\n", - callid.len, callid.len, callid.s); + if (!rtpengine_hash_table_remove(callid, viabranch)) { + LM_ERR("rtpengine hash table failed to remove entry for callen=%d callid=%.*s viabranch=%.*s\n", + callid.len, callid.len, callid.s, viabranch.len, viabranch.s); } else { - LM_DBG("rtpengine hash table remove entry for callen=%d callid=%.*s\n", - callid.len, callid.len, callid.s); + LM_DBG("rtpengine hash table remove entry for callen=%d callid=%.*s viabranch=%.*s\n", + callid.len, callid.len, callid.s, viabranch.len, viabranch.s); } } @@ -2278,7 +2279,7 @@ static struct rtpp_set * select_rtpp_set(int id_set ){ * run the selection algorithm and return the new selected node */ static struct rtpp_node * -select_rtpp_node_new(str callid, int do_test, int op) +select_rtpp_node_new(str callid, str viabranch, int do_test) { struct rtpp_node* node; unsigned i, sum, sumcut, weight_sum; @@ -2350,18 +2351,25 @@ select_rtpp_node_new(str callid, int do_test, int op) } /* build the entry */ - struct rtpengine_hash_entry *entry = shm_malloc(sizeof(struct rtpp_node)); + struct rtpengine_hash_entry *entry = shm_malloc(sizeof(struct rtpengine_hash_entry)); if (!entry) { - LM_ERR("rtpengine hash table fail to create entry for calllen=%d callid=%.*s\n", - callid.len, callid.len, callid.s); + LM_ERR("rtpengine hash table fail to create entry for calllen=%d callid=%.*s viabranch=%.*s\n", + callid.len, callid.len, callid.s, viabranch.len, viabranch.s); return node; } + memset(entry, 0, sizeof(struct rtpengine_hash_entry)); /* fill the entry */ if (shm_str_dup(&entry->callid, &callid) < 0) { LM_ERR("rtpengine hash table fail to duplicate calllen=%d callid=%.*s\n", callid.len, callid.len, callid.s); - shm_free(entry); + rtpengine_hash_table_free_entry(entry); + return node; + } + if (shm_str_dup(&entry->viabranch, &viabranch) < 0) { + LM_ERR("rtpengine hash table fail to duplicate calllen=%d viabranch=%.*s\n", + callid.len, viabranch.len, viabranch.s); + rtpengine_hash_table_free_entry(entry); return node; } entry->node = node; @@ -2369,15 +2377,14 @@ select_rtpp_node_new(str callid, int do_test, int op) entry->tout = get_ticks() + hash_table_tout; /* insert the key<->entry from the hashtable */ - if (!rtpengine_hash_table_insert(&callid, entry)) { - LM_ERR("rtpengine hash table fail to insert node=%.*s for calllen=%d callid=%.*s\n", - node->rn_url.len, node->rn_url.s, callid.len, callid.len, callid.s); - shm_free(entry->callid.s); - shm_free(entry); + if (!rtpengine_hash_table_insert(callid, viabranch, entry)) { + LM_ERR("rtpengine hash table fail to insert node=%.*s for calllen=%d callid=%.*s viabranch=%.*s\n", + node->rn_url.len, node->rn_url.s, callid.len, callid.len, callid.s, viabranch.len, viabranch.s); + rtpengine_hash_table_free_entry(entry); return node; } else { - LM_DBG("rtpengine hash table insert node=%.*s for calllen=%d callid=%.*s\n", - node->rn_url.len, node->rn_url.s, callid.len, callid.len, callid.s); + LM_DBG("rtpengine hash table insert node=%.*s for calllen=%d callid=%.*s viabranch=%.*s\n", + node->rn_url.len, node->rn_url.s, callid.len, callid.len, callid.s, viabranch.len, viabranch.s); } /* return selected node */ @@ -2388,19 +2395,19 @@ select_rtpp_node_new(str callid, int do_test, int op) * lookup the hastable (key=callid value=node) and get the old node (e.g. for answer/delete) */ static struct rtpp_node * -select_rtpp_node_old(str callid, int do_test, int op) +select_rtpp_node_old(str callid, str viabranch, int do_test) { struct rtpp_node *node = NULL; - node = rtpengine_hash_table_lookup(&callid); + node = rtpengine_hash_table_lookup(callid, viabranch); if (!node) { - LM_NOTICE("rtpengine hash table lookup failed to find node for calllen=%d callid=%.*s\n", - callid.len, callid.len, callid.s); + LM_NOTICE("rtpengine hash table lookup failed to find node for calllen=%d callid=%.*s viabranch=%.*s\n", + callid.len, callid.len, callid.s, viabranch.len, viabranch.s); return NULL; } else { - LM_DBG("rtpengine hash table lookup find node=%.*s for calllen=%d callid=%.*s\n", - node->rn_url.len, node->rn_url.s, callid.len, callid.len, callid.s); + LM_DBG("rtpengine hash table lookup find node=%.*s for calllen=%d callid=%.*s viabranch=%.*s\n", + node->rn_url.len, node->rn_url.s, callid.len, callid.len, callid.s, viabranch.len, viabranch.s); } return node; @@ -2411,7 +2418,7 @@ select_rtpp_node_old(str callid, int do_test, int op) * the call if some proxies were disabled or enabled (e.g. kamctl command) */ static struct rtpp_node * -select_rtpp_node(str callid, int do_test, int op) +select_rtpp_node(str callid, str viabranch, int do_test) { struct rtpp_node *node = NULL; @@ -2421,12 +2428,12 @@ select_rtpp_node(str callid, int do_test, int op) } // lookup node - node = select_rtpp_node_old(callid, do_test, op); + node = select_rtpp_node_old(callid, viabranch, do_test); // check node if (!node) { // run the selection algorithm - node = select_rtpp_node_new(callid, do_test, op); + node = select_rtpp_node_new(callid, viabranch, do_test); // check node if (!node) { diff --git a/modules/rtpengine/rtpengine_hash.c b/modules/rtpengine/rtpengine_hash.c index 566a9607eda..027fd8a26f0 100644 --- a/modules/rtpengine/rtpengine_hash.c +++ b/modules/rtpengine/rtpengine_hash.c @@ -11,25 +11,25 @@ static struct rtpengine_hash_table *rtpengine_hash_table; static int hash_table_size; /* from sipwise rtpengine */ -static int str_cmp_str(const str *a, const str *b) { - if (a->len < b->len) +static int str_cmp_str(const str a, const str b) { + if (a.len < b.len) return -1; - if (a->len > b->len) + if (a.len > b.len) return 1; - if (a->len == 0 && b->len == 0) + if (a.len == 0 && b.len == 0) return 0; - return memcmp(a->s, b->s, a->len); + return memcmp(a.s, b.s, a.len); } /* from sipwise rtpengine */ -static int str_equal(str *a, str *b) { +static int str_equal(str a, str b) { return (str_cmp_str(a, b) == 0); } /* from sipwise rtpengine */ -static unsigned int str_hash(str *s) { +static unsigned int str_hash(str s) { unsigned int ret = 5381; - str it = *s; + str it = s; while (it.len > 0) { ret = (ret << 5) + ret + *it.s; @@ -40,7 +40,7 @@ static unsigned int str_hash(str *s) { return ret % hash_table_size; } -/* rtpengine glib hash API */ +/* rtpengine hash API */ int rtpengine_hash_table_init(int size) { int i; @@ -58,6 +58,7 @@ int rtpengine_hash_table_init(int size) { LM_ERR("no shm left to create rtpengine_hash_table\n"); return 0; } + memset(rtpengine_hash_table, 0, sizeof(struct rtpengine_hash_table)); // init hashtable entry_list rtpengine_hash_table->entry_list = shm_malloc(hash_table_size * sizeof(struct rtpengine_hash_entry)); @@ -97,7 +98,6 @@ int rtpengine_hash_table_init(int size) { int rtpengine_hash_table_destroy() { int i; - struct rtpengine_hash_entry *entry, *last_entry; // check rtpengine hashtable if (!rtpengine_hash_table) { @@ -113,16 +113,11 @@ int rtpengine_hash_table_destroy() { return 0; } - // destroy hashtable entry_list[i] lock_get(rtpengine_hash_lock); + + // destroy hashtable entry_list[i] for (i = 0; i < hash_table_size; i++) { - entry = rtpengine_hash_table->entry_list[i]; - while (entry) { - last_entry = entry; - entry = entry->next; - shm_free(last_entry->callid.s); - shm_free(last_entry); - } + rtpengine_hash_table_free_entry_list(rtpengine_hash_table->entry_list[i]); } // destroy hashtable entry_list @@ -132,6 +127,7 @@ int rtpengine_hash_table_destroy() { // destroy hashtable shm_free(rtpengine_hash_table); rtpengine_hash_table = NULL; + lock_release(rtpengine_hash_lock); // destroy lock @@ -145,7 +141,7 @@ int rtpengine_hash_table_destroy() { return 1; } -int rtpengine_hash_table_insert(str *key, struct rtpengine_hash_entry *value) { +int rtpengine_hash_table_insert(str callid, str viabranch, struct rtpengine_hash_entry *value) { struct rtpengine_hash_entry *entry, *last_entry; struct rtpengine_hash_entry *new_entry = (struct rtpengine_hash_entry *) value; unsigned int hash_index; @@ -163,18 +159,21 @@ int rtpengine_hash_table_insert(str *key, struct rtpengine_hash_entry *value) { } // get entry list - hash_index = str_hash(key); + hash_index = str_hash(callid); entry = rtpengine_hash_table->entry_list[hash_index]; last_entry = entry; // lock lock_get(rtpengine_hash_lock); while (entry) { - // if key found, don't add new entry - if (str_equal(&entry->callid, &new_entry->callid)) { + // if found, don't add new entry + if (str_equal(entry->callid, new_entry->callid) && + str_equal(entry->viabranch, new_entry->viabranch)) { // unlock lock_release(rtpengine_hash_lock); - LM_ERR("Call id %.*s already in hashtable, ignore new value", entry->callid.len, entry->callid.s); + LM_NOTICE("callid=%.*s, viabranch=%.*s already in hashtable, ignore new value", + entry->callid.len, entry->callid.s, + entry->viabranch.len, entry->viabranch.s); return 0; } @@ -184,8 +183,7 @@ int rtpengine_hash_table_insert(str *key, struct rtpengine_hash_entry *value) { last_entry->next = entry->next; // free current entry; entry points to unknown - shm_free(entry->callid.s); - shm_free(entry); + rtpengine_hash_table_free_entry(entry); // set pointers entry = last_entry; @@ -210,7 +208,7 @@ int rtpengine_hash_table_insert(str *key, struct rtpengine_hash_entry *value) { return 1; } -int rtpengine_hash_table_remove(str *key) { +int rtpengine_hash_table_remove(str callid, str viabranch) { struct rtpengine_hash_entry *entry, *last_entry; unsigned int hash_index; @@ -227,19 +225,19 @@ int rtpengine_hash_table_remove(str *key) { } // get first entry from entry list; jump over unused list head - hash_index = str_hash(key); + hash_index = str_hash(callid); entry = rtpengine_hash_table->entry_list[hash_index]; last_entry = entry; // lock lock_get(rtpengine_hash_lock); while (entry) { - // if key found, delete entry - if (str_equal(&entry->callid, (str *)key)) { + // if callid found, delete entry + if (str_equal(entry->callid, callid) && + str_equal(entry->viabranch, viabranch)) { // free entry last_entry->next = entry->next; - shm_free(entry->callid.s); - shm_free(entry); + rtpengine_hash_table_free_entry(entry); // update total rtpengine_hash_table->total--; @@ -256,8 +254,7 @@ int rtpengine_hash_table_remove(str *key) { last_entry->next = entry->next; // free current entry; entry points to unknown - shm_free(entry->callid.s); - shm_free(entry); + rtpengine_hash_table_free_entry(entry); // set pointers entry = last_entry; @@ -276,7 +273,7 @@ int rtpengine_hash_table_remove(str *key) { return 0; } -struct rtpp_node *rtpengine_hash_table_lookup(str *key) { +struct rtpp_node *rtpengine_hash_table_lookup(str callid, str viabranch) { struct rtpengine_hash_entry *entry, *last_entry; unsigned int hash_index; struct rtpp_node *node; @@ -294,15 +291,16 @@ struct rtpp_node *rtpengine_hash_table_lookup(str *key) { } // get first entry from entry list; jump over unused list head - hash_index = str_hash(key); + hash_index = str_hash(callid); entry = rtpengine_hash_table->entry_list[hash_index]; last_entry = entry; // lock lock_get(rtpengine_hash_lock); while (entry) { - // if key found, return entry - if (str_equal(&entry->callid, (str *)key)) { + // if callid found, return entry + if (str_equal(entry->callid, callid) && + str_equal(entry->viabranch, viabranch)) { node = entry->node; // unlock @@ -317,8 +315,7 @@ struct rtpp_node *rtpengine_hash_table_lookup(str *key) { last_entry->next = entry->next; // free current entry; entry points to unknown - shm_free(entry->callid.s); - shm_free(entry); + rtpengine_hash_table_free_entry(entry); // set pointers entry = last_entry; @@ -369,8 +366,7 @@ void rtpengine_hash_table_print() { last_entry->next = entry->next; // free current entry; entry points to unknown - shm_free(entry->callid.s); - shm_free(entry); + rtpengine_hash_table_free_entry(entry); // set pointers entry = last_entry; @@ -401,3 +397,41 @@ unsigned int rtpengine_hash_table_total() { return rtpengine_hash_table->total; } + +void rtpengine_hash_table_free_entry(struct rtpengine_hash_entry *entry) { + if (!entry) { + return ; + } + + // free callid + if (entry->callid.s) { + shm_free(entry->callid.s); + } + + // free viabranch + if (entry->viabranch.s) { + shm_free(entry->viabranch.s); + } + + // free entry + shm_free(entry); + + return ; +} + +void rtpengine_hash_table_free_entry_list(struct rtpengine_hash_entry *entry_list) { + struct rtpengine_hash_entry *entry, *last_entry; + + if (!entry_list) { + return ; + } + + entry = entry_list; + while (entry) { + last_entry = entry; + entry = entry->next; + rtpengine_hash_table_free_entry(last_entry); + } + + return ; +} diff --git a/modules/rtpengine/rtpengine_hash.h b/modules/rtpengine/rtpengine_hash.h index c0a9ed8c6bc..2d1ab4545f1 100644 --- a/modules/rtpengine/rtpengine_hash.h +++ b/modules/rtpengine/rtpengine_hash.h @@ -6,10 +6,11 @@ /* table entry */ struct rtpengine_hash_entry { - unsigned int tout; // call timeout str callid; // call callid + str viabranch; // call viabranch struct rtpp_node *node; // call selected node + unsigned int tout; // call timeout struct rtpengine_hash_entry *next; // call next }; @@ -22,10 +23,13 @@ struct rtpengine_hash_table { int rtpengine_hash_table_init(int size); int rtpengine_hash_table_destroy(); -int rtpengine_hash_table_insert(str *key, struct rtpengine_hash_entry *value); -int rtpengine_hash_table_remove(str *key); -struct rtpp_node *rtpengine_hash_table_lookup(str *key); +int rtpengine_hash_table_insert(str callid, str viabranch, struct rtpengine_hash_entry *value); +int rtpengine_hash_table_remove(str callid, str viabranch); +struct rtpp_node *rtpengine_hash_table_lookup(str callid, str viabranch); void rtpengine_hash_table_print(); unsigned int rtpengine_hash_table_total(); +void rtpengine_hash_table_free_entry(struct rtpengine_hash_entry *entry); +void rtpengine_hash_table_free_entry_list(struct rtpengine_hash_entry *entry_list); + #endif