Module: kamailio Branch: master Commit: 27bed654dcc4e9dd543a7f2f92e2174478bf7f26 URL: https://github.com/kamailio/kamailio/commit/27bed654dcc4e9dd543a7f2f92e21744...
Author: Federico Cabiddu federico.cabiddu@gmail.com Committer: Federico Cabiddu federico.cabiddu@gmail.com Date: 2015-09-04T15:12:29+02:00
tm: check all the outgoing branches when appending a new branch
---
Modified: modules/tm/t_append_branches.c
---
Diff: https://github.com/kamailio/kamailio/commit/27bed654dcc4e9dd543a7f2f92e21744... Patch: https://github.com/kamailio/kamailio/commit/27bed654dcc4e9dd543a7f2f92e21744...
---
diff --git a/modules/tm/t_append_branches.c b/modules/tm/t_append_branches.c index 8060021..f16c672 100644 --- a/modules/tm/t_append_branches.c +++ b/modules/tm/t_append_branches.c @@ -23,6 +23,9 @@ * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, * EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. * + * History: + * ------- + * 2014-09-09 first release of t_append_branches * */
@@ -106,8 +109,6 @@ int t_append_branches(void) { set_branch_route(t->on_branch_delayed); }
- outgoings = t->nr_of_outgoings; - /* not really sure that the following is needed */
set_branch_iterator(nr_branches-1); @@ -116,7 +117,7 @@ int t_append_branches(void) { &bflags, &si, &ruid, &instance, &location_ua))) { LM_DBG("Current uri %.*s\n",current_uri.len, current_uri.s);
- for (i=0; i<=nr_branches; i++) { + for (i=0; i<outgoings; i++) { if (t->uac[i].ruid.len == ruid.len && !memcmp(t->uac[i].ruid.s, ruid.s, ruid.len)) { LM_DBG("branch already added [%.*s]\n", ruid.len, ruid.s); @@ -133,6 +134,8 @@ int t_append_branches(void) { &path, 0, si, orig_msg->fwd_send_flags, orig_msg->rcv.proto, (dst_uri.len)?-1:UAC_SKIP_BR_DST_F, &instance, &ruid, &location_ua); + + LM_DBG("added branch [%.*s] with ruid [%.*s]\n", current_uri.len, current_uri.s, ruid.len, ruid.s);
/* test if cancel was received meanwhile */ if (t->flags & T_CANCELED) goto canceled;
Hello,
you re-introduced the history at the top of the file, which was removed from tm module and going to be removed from everywhere in the code, as most of it is irrelevant. The changes are better tracked via git now, with the commit message and git diff/blame being very handy.
Cheers, Daniel
On 04/09/15 15:13, Federico Cabiddu wrote:
Module: kamailio Branch: master Commit: 27bed654dcc4e9dd543a7f2f92e2174478bf7f26 URL: https://github.com/kamailio/kamailio/commit/27bed654dcc4e9dd543a7f2f92e21744...
Author: Federico Cabiddu federico.cabiddu@gmail.com Committer: Federico Cabiddu federico.cabiddu@gmail.com Date: 2015-09-04T15:12:29+02:00
tm: check all the outgoing branches when appending a new branch
Modified: modules/tm/t_append_branches.c
Diff: https://github.com/kamailio/kamailio/commit/27bed654dcc4e9dd543a7f2f92e21744... Patch: https://github.com/kamailio/kamailio/commit/27bed654dcc4e9dd543a7f2f92e21744...
diff --git a/modules/tm/t_append_branches.c b/modules/tm/t_append_branches.c index 8060021..f16c672 100644 --- a/modules/tm/t_append_branches.c +++ b/modules/tm/t_append_branches.c @@ -23,6 +23,9 @@
- NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE,
- EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- History:
*/
- 2014-09-09 first release of t_append_branches
@@ -106,8 +109,6 @@ int t_append_branches(void) { set_branch_route(t->on_branch_delayed); }
outgoings = t->nr_of_outgoings;
/* not really sure that the following is needed */
set_branch_iterator(nr_branches-1);
@@ -116,7 +117,7 @@ int t_append_branches(void) { &bflags, &si, &ruid, &instance, &location_ua))) { LM_DBG("Current uri %.*s\n",current_uri.len, current_uri.s);
for (i=0; i<=nr_branches; i++) {
for (i=0; i<outgoings; i++) { if (t->uac[i].ruid.len == ruid.len && !memcmp(t->uac[i].ruid.s, ruid.s, ruid.len)) { LM_DBG("branch already added [%.*s]\n", ruid.len, ruid.s);@@ -133,6 +134,8 @@ int t_append_branches(void) { &path, 0, si, orig_msg->fwd_send_flags, orig_msg->rcv.proto, (dst_uri.len)?-1:UAC_SKIP_BR_DST_F, &instance, &ruid, &location_ua);
LM_DBG("added branch [%.*s] with ruid [%.*s]\n", current_uri.len, current_uri.s, ruid.len, ruid.s);/* test if cancel was received meanwhile */ if (t->flags & T_CANCELED) goto canceled;
sr-dev mailing list sr-dev@lists.sip-router.org http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
I'm sorry, I didn't see it with git difftool before committing. Do you want that I clean the file?
Cheers,
Federico
On Fri, Sep 4, 2015 at 3:18 PM, Daniel-Constantin Mierla miconda@gmail.com wrote:
Hello,
you re-introduced the history at the top of the file, which was removed from tm module and going to be removed from everywhere in the code, as most of it is irrelevant. The changes are better tracked via git now, with the commit message and git diff/blame being very handy.
Cheers, Daniel
On 04/09/15 15:13, Federico Cabiddu wrote:
Module: kamailio Branch: master Commit: 27bed654dcc4e9dd543a7f2f92e2174478bf7f26 URL:
https://github.com/kamailio/kamailio/commit/27bed654dcc4e9dd543a7f2f92e21744...
Author: Federico Cabiddu federico.cabiddu@gmail.com Committer: Federico Cabiddu federico.cabiddu@gmail.com Date: 2015-09-04T15:12:29+02:00
tm: check all the outgoing branches when appending a new branch
Modified: modules/tm/t_append_branches.c
Diff:
https://github.com/kamailio/kamailio/commit/27bed654dcc4e9dd543a7f2f92e21744...
Patch:
https://github.com/kamailio/kamailio/commit/27bed654dcc4e9dd543a7f2f92e21744...
diff --git a/modules/tm/t_append_branches.c
b/modules/tm/t_append_branches.c
index 8060021..f16c672 100644 --- a/modules/tm/t_append_branches.c +++ b/modules/tm/t_append_branches.c @@ -23,6 +23,9 @@
- NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
SOFTWARE,
- EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- History:
*/
- 2014-09-09 first release of t_append_branches
@@ -106,8 +109,6 @@ int t_append_branches(void) { set_branch_route(t->on_branch_delayed); }
outgoings = t->nr_of_outgoings;/* not really sure that the following is needed */ set_branch_iterator(nr_branches-1);@@ -116,7 +117,7 @@ int t_append_branches(void) {
&bflags, &si, &ruid, &instance, &location_ua))) {LM_DBG("Current uri %.*s\n",current_uri.len,current_uri.s);
for (i=0; i<=nr_branches; i++) {
for (i=0; i<outgoings; i++) { if (t->uac[i].ruid.len == ruid.len && !memcmp(t->uac[i].ruid.s,ruid.s, ruid.len)) {
LM_DBG("branch already added [%.*s]\n",ruid.len, ruid.s);
@@ -133,6 +134,8 @@ int t_append_branches(void) { &path, 0, si,
orig_msg->fwd_send_flags,
orig_msg->rcv.proto,(dst_uri.len)?-1:UAC_SKIP_BR_DST_F, &instance,
&ruid, &location_ua);
LM_DBG("added branch [%.*s] with ruid [%.*s]\n",current_uri.len, current_uri.s, ruid.len, ruid.s);
/* test if cancel was received meanwhile */ if (t->flags & T_CANCELED) goto canceled;
sr-dev mailing list sr-dev@lists.sip-router.org http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
-- Daniel-Constantin Mierla http://twitter.com/#!/miconda - http://www.linkedin.com/in/miconda Book: SIP Routing With Kamailio - http://www.asipto.com
It is not a big problem, just wanted to underline it to be aware of. And it would be good to be removed, indeed.
As I looked at the code in the file with this occasion, if you want to go through all outgoing branches in the destination set, then you need to do:
init_branch_iterator();
instead of:
set_branch_iterator(nr_branches-1);
That will look at all next hops that are already in destination set.
Assuming that you mainly use this function when you get a new registration and have one new outgoing branch, then nr_branches-1 can result to be 0. However, if for what so ever reason you get more branches, then you will use only one with existing code. As I got it from the code, you check if the destination is already used, so just going again from branch 0 in destination set looks like what you wanted to do. Maybe I am wrong ...
Cheers, Daniel
On 04/09/15 15:21, Federico Cabiddu wrote:
I'm sorry, I didn't see it with git difftool before committing. Do you want that I clean the file?
Cheers,
Federico
On Fri, Sep 4, 2015 at 3:18 PM, Daniel-Constantin Mierla <miconda@gmail.com mailto:miconda@gmail.com> wrote:
Hello, you re-introduced the history at the top of the file, which was removed from tm module and going to be removed from everywhere in the code, as most of it is irrelevant. The changes are better tracked via git now, with the commit message and git diff/blame being very handy. Cheers, Daniel On 04/09/15 15:13, Federico Cabiddu wrote: > Module: kamailio > Branch: master > Commit: 27bed654dcc4e9dd543a7f2f92e2174478bf7f26 > URL: https://github.com/kamailio/kamailio/commit/27bed654dcc4e9dd543a7f2f92e2174478bf7f26 > > Author: Federico Cabiddu <federico.cabiddu@gmail.com <mailto:federico.cabiddu@gmail.com>> > Committer: Federico Cabiddu <federico.cabiddu@gmail.com <mailto:federico.cabiddu@gmail.com>> > Date: 2015-09-04T15:12:29+02:00 > > tm: check all the outgoing branches when appending a new branch > > --- > > Modified: modules/tm/t_append_branches.c > > --- > > Diff: https://github.com/kamailio/kamailio/commit/27bed654dcc4e9dd543a7f2f92e2174478bf7f26.diff > Patch: https://github.com/kamailio/kamailio/commit/27bed654dcc4e9dd543a7f2f92e2174478bf7f26.patch > > --- > > diff --git a/modules/tm/t_append_branches.c b/modules/tm/t_append_branches.c > index 8060021..f16c672 100644 > --- a/modules/tm/t_append_branches.c > +++ b/modules/tm/t_append_branches.c > @@ -23,6 +23,9 @@ > * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, > * EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. > * > + * History: > + * ------- > + * 2014-09-09 first release of t_append_branches > * > */ > > @@ -106,8 +109,6 @@ int t_append_branches(void) { > set_branch_route(t->on_branch_delayed); > } > > - outgoings = t->nr_of_outgoings; > - > /* not really sure that the following is needed */ > > set_branch_iterator(nr_branches-1); > @@ -116,7 +117,7 @@ int t_append_branches(void) { > &bflags, &si, &ruid, &instance, &location_ua))) { > LM_DBG("Current uri %.*s\n",current_uri.len, current_uri.s); > > - for (i=0; i<=nr_branches; i++) { > + for (i=0; i<outgoings; i++) { > if (t->uac[i].ruid.len == ruid.len > && !memcmp(t->uac[i].ruid.s, ruid.s, ruid.len)) { > LM_DBG("branch already added [%.*s]\n", ruid.len, ruid.s); > @@ -133,6 +134,8 @@ int t_append_branches(void) { > &path, 0, si, orig_msg->fwd_send_flags, > orig_msg->rcv.proto, (dst_uri.len)?-1:UAC_SKIP_BR_DST_F, &instance, > &ruid, &location_ua); > + > + LM_DBG("added branch [%.*s] with ruid [%.*s]\n", current_uri.len, current_uri.s, ruid.len, ruid.s); > > /* test if cancel was received meanwhile */ > if (t->flags & T_CANCELED) goto canceled; > > > _______________________________________________ > sr-dev mailing list > sr-dev@lists.sip-router.org <mailto:sr-dev@lists.sip-router.org> > http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev -- Daniel-Constantin Mierla http://twitter.com/#!/miconda <http://twitter.com/#%21/miconda> - http://www.linkedin.com/in/miconda Book: SIP Routing With Kamailio - http://www.asipto.com
Thank you Daniel for the suggestion. I realized that I misunderstood the meaning of nr_branches. My intention was to use it as a "starting point" to check only the newly created branches in the destination set. I will change it according to your suggestion and also remove the history if you agree.
Thank you,
Federico
On Fri, Sep 4, 2015 at 3:27 PM, Daniel-Constantin Mierla miconda@gmail.com wrote:
It is not a big problem, just wanted to underline it to be aware of. And it would be good to be removed, indeed.
As I looked at the code in the file with this occasion, if you want to go through all outgoing branches in the destination set, then you need to do:
init_branch_iterator();
instead of:
set_branch_iterator(nr_branches-1);
That will look at all next hops that are already in destination set.
Assuming that you mainly use this function when you get a new registration and have one new outgoing branch, then nr_branches-1 can result to be 0. However, if for what so ever reason you get more branches, then you will use only one with existing code. As I got it from the code, you check if the destination is already used, so just going again from branch 0 in destination set looks like what you wanted to do. Maybe I am wrong ...
Cheers, Daniel
On 04/09/15 15:21, Federico Cabiddu wrote:
I'm sorry, I didn't see it with git difftool before committing. Do you want that I clean the file?
Cheers,
Federico
On Fri, Sep 4, 2015 at 3:18 PM, Daniel-Constantin Mierla < miconda@gmail.commiconda@gmail.com> wrote:
Hello,
you re-introduced the history at the top of the file, which was removed from tm module and going to be removed from everywhere in the code, as most of it is irrelevant. The changes are better tracked via git now, with the commit message and git diff/blame being very handy.
Cheers, Daniel
On 04/09/15 15:13, Federico Cabiddu wrote:
Module: kamailio Branch: master Commit: 27bed654dcc4e9dd543a7f2f92e2174478bf7f26 URL:
https://github.com/kamailio/kamailio/commit/27bed654dcc4e9dd543a7f2f92e21744...
Author: Federico Cabiddu < federico.cabiddu@gmail.com
federico.cabiddu@gmail.com>
Committer: Federico Cabiddu < federico.cabiddu@gmail.com
federico.cabiddu@gmail.com>
Date: 2015-09-04T15:12:29+02:00
tm: check all the outgoing branches when appending a new branch
Modified: modules/tm/t_append_branches.c
Diff:
https://github.com/kamailio/kamailio/commit/27bed654dcc4e9dd543a7f2f92e21744...
Patch:
https://github.com/kamailio/kamailio/commit/27bed654dcc4e9dd543a7f2f92e21744...
diff --git a/modules/tm/t_append_branches.c
b/modules/tm/t_append_branches.c
index 8060021..f16c672 100644 --- a/modules/tm/t_append_branches.c +++ b/modules/tm/t_append_branches.c @@ -23,6 +23,9 @@
- NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
SOFTWARE,
- EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- History:
*/
- 2014-09-09 first release of t_append_branches
@@ -106,8 +109,6 @@ int t_append_branches(void) { set_branch_route(t->on_branch_delayed); }
outgoings = t->nr_of_outgoings;/* not really sure that the following is needed */ set_branch_iterator(nr_branches-1);@@ -116,7 +117,7 @@ int t_append_branches(void) {
&bflags, &si, &ruid, &instance, &location_ua))) {LM_DBG("Current uri %.*s\n",current_uri.len,current_uri.s);
for (i=0; i<=nr_branches; i++) {
for (i=0; i<outgoings; i++) { if (t->uac[i].ruid.len == ruid.len && !memcmp(t->uac[i].ruid.s,ruid.s, ruid.len)) {
LM_DBG("branch already added [%.*s]\n",ruid.len, ruid.s);
@@ -133,6 +134,8 @@ int t_append_branches(void) { &path, 0, si,
orig_msg->fwd_send_flags,
orig_msg->rcv.proto,(dst_uri.len)?-1:UAC_SKIP_BR_DST_F, &instance,
&ruid, &location_ua);
LM_DBG("added branch [%.*s] with ruid [%.*s]\n",current_uri.len, current_uri.s, ruid.len, ruid.s);
/* test if cancel was received meanwhile */ if (t->flags & T_CANCELED) goto canceled;
sr-dev mailing list sr-dev@lists.sip-router.org http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
-- Daniel-Constantin Mierla http://twitter.com/#!/miconda - http://www.linkedin.com/in/miconda Book: SIP Routing With Kamailio - http://www.asipto.com http://www.asipto.com
-- Daniel-Constantin Mierlahttp://twitter.com/#!/miconda - http://www.linkedin.com/in/miconda Book: SIP Routing With Kamailio - http://www.asipto.com
The history must be removed, it was decided and lost of parts were updated with it removed.
Regarding the number of branches, it is a matter of the context when you want to look at the new branches. As you already iterate through the transaction structure to see if a destination was used, then the safest is to iterate from 0. That because in a failure_route, the core destination set table is again starting from 0.
Cheers, Daniel
On 04/09/15 15:37, Federico Cabiddu wrote:
Thank you Daniel for the suggestion. I realized that I misunderstood the meaning of nr_branches. My intention was to use it as a "starting point" to check only the newly created branches in the destination set. I will change it according to your suggestion and also remove the history if you agree.
Thank you,
Federico
On Fri, Sep 4, 2015 at 3:27 PM, Daniel-Constantin Mierla <miconda@gmail.com mailto:miconda@gmail.com> wrote:
It is not a big problem, just wanted to underline it to be aware of. And it would be good to be removed, indeed. As I looked at the code in the file with this occasion, if you want to go through all outgoing branches in the destination set, then you need to do: init_branch_iterator(); instead of: set_branch_iterator(nr_branches-1); That will look at all next hops that are already in destination set. Assuming that you mainly use this function when you get a new registration and have one new outgoing branch, then nr_branches-1 can result to be 0. However, if for what so ever reason you get more branches, then you will use only one with existing code. As I got it from the code, you check if the destination is already used, so just going again from branch 0 in destination set looks like what you wanted to do. Maybe I am wrong ... Cheers, Daniel On 04/09/15 15:21, Federico Cabiddu wrote:I'm sorry, I didn't see it with git difftool before committing. Do you want that I clean the file? Cheers, Federico On Fri, Sep 4, 2015 at 3:18 PM, Daniel-Constantin Mierla <miconda@gmail.com <mailto:miconda@gmail.com>> wrote: Hello, you re-introduced the history at the top of the file, which was removed from tm module and going to be removed from everywhere in the code, as most of it is irrelevant. The changes are better tracked via git now, with the commit message and git diff/blame being very handy. Cheers, Daniel On 04/09/15 15:13, Federico Cabiddu wrote: > Module: kamailio > Branch: master > Commit: 27bed654dcc4e9dd543a7f2f92e2174478bf7f26 > URL: https://github.com/kamailio/kamailio/commit/27bed654dcc4e9dd543a7f2f92e2174478bf7f26 > > Author: Federico Cabiddu <federico.cabiddu@gmail.com <mailto:federico.cabiddu@gmail.com>> > Committer: Federico Cabiddu <federico.cabiddu@gmail.com <mailto:federico.cabiddu@gmail.com>> > Date: 2015-09-04T15:12:29+02:00 > > tm: check all the outgoing branches when appending a new branch > > --- > > Modified: modules/tm/t_append_branches.c > > --- > > Diff: https://github.com/kamailio/kamailio/commit/27bed654dcc4e9dd543a7f2f92e2174478bf7f26.diff > Patch: https://github.com/kamailio/kamailio/commit/27bed654dcc4e9dd543a7f2f92e2174478bf7f26.patch > > --- > > diff --git a/modules/tm/t_append_branches.c b/modules/tm/t_append_branches.c > index 8060021..f16c672 100644 > --- a/modules/tm/t_append_branches.c > +++ b/modules/tm/t_append_branches.c > @@ -23,6 +23,9 @@ > * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, > * EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. > * > + * History: > + * ------- > + * 2014-09-09 first release of t_append_branches > * > */ > > @@ -106,8 +109,6 @@ int t_append_branches(void) { > set_branch_route(t->on_branch_delayed); > } > > - outgoings = t->nr_of_outgoings; > - > /* not really sure that the following is needed */ > > set_branch_iterator(nr_branches-1); > @@ -116,7 +117,7 @@ int t_append_branches(void) { > &bflags, &si, &ruid, &instance, &location_ua))) { > LM_DBG("Current uri %.*s\n",current_uri.len, current_uri.s); > > - for (i=0; i<=nr_branches; i++) { > + for (i=0; i<outgoings; i++) { > if (t->uac[i].ruid.len == ruid.len > && !memcmp(t->uac[i].ruid.s, ruid.s, ruid.len)) { > LM_DBG("branch already added [%.*s]\n", ruid.len, ruid.s); > @@ -133,6 +134,8 @@ int t_append_branches(void) { > &path, 0, si, orig_msg->fwd_send_flags, > orig_msg->rcv.proto, (dst_uri.len)?-1:UAC_SKIP_BR_DST_F, &instance, > &ruid, &location_ua); > + > + LM_DBG("added branch [%.*s] with ruid [%.*s]\n", current_uri.len, current_uri.s, ruid.len, ruid.s); > > /* test if cancel was received meanwhile */ > if (t->flags & T_CANCELED) goto canceled; > > > _______________________________________________ > sr-dev mailing list > sr-dev@lists.sip-router.org <mailto:sr-dev@lists.sip-router.org> > http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev -- Daniel-Constantin Mierla http://twitter.com/#!/miconda <http://twitter.com/#%21/miconda> - http://www.linkedin.com/in/miconda Book: SIP Routing With Kamailio - http://www.asipto.com-- Daniel-Constantin Mierla http://twitter.com/#!/miconda <http://twitter.com/#%21/miconda> - http://www.linkedin.com/in/miconda Book: SIP Routing With Kamailio - http://www.asipto.com
-
Daniel-Constantin Mierla -
Federico Cabiddu