Skip Menu |

This queue is for tickets about the Net-SIP CPAN distribution.

Report information
The Basics
Id: 122925
Status: resolved
Priority: 0/
Queue: Net-SIP

People
Owner: Steffen_Ullrich [...] genua.de
Requestors: richard.carver [...] cloudmont.co.uk
Cc:
AdminCc:

Bug Information
Severity: (no value)
Broken in: (no value)
Fixed in: (no value)



Subject: Allow extending of StatelessProxy __forward_packet_final method
Date: Tue, 29 Aug 2017 13:34:44 +0000
To: "bug-Net-SIP [...] rt.cpan.org" <bug-Net-SIP [...] rt.cpan.org>
From: Richard Carver <richard.carver [...] cloudmont.co.uk>
Net::SIP::StatelessProxy doesn't call __forward_packet_final in a way that allows it to be overridden in an inherited class. The following patch makes this possible. The reason I need this is that one side of my SIP proxy uses a URI which has multiple AAAA records for load balancing, and often my CANCEL requests are sent to a different IP address to the INVITE and are not recognised. In order to fix this I am caching the DNS results for the URI lookup on a per-call basis so that all requests for a call-id go to the same IP, but each new call does a fresh DNS lookup. To do this I have a class that inherits from Net::SIP::StatelessProxy. It overrides __forward_packet_final to cache the result and overrides __forward_request_getdaddr to use the cached value if needed. --- /usr/local/share/perl5/Net/SIP/StatelessProxy.pm.orig 2017-08-29 11:52:38.207617148 +0000 +++ /usr/local/share/perl5/Net/SIP/StatelessProxy.pm 2017-08-29 11:53:58.815522681 +0000 @@ -339,7 +339,7 @@ ip_parts2string($entry->{dst_addr}[0])); return; } - __forward_packet_final( $self,$entry ); + $self->__forward_packet_final( $entry ); } @@ -457,7 +457,7 @@ [ \&__forward_request_2,$self,$entry ] ); } else { - __forward_packet_final($self,$entry); + $self->__forward_packet_final($entry); } } @@ -483,7 +483,7 @@ return unless @$dst_addr; # nothing could be resolved - __forward_packet_final( $self,$entry ); + $self->__forward_packet_final( $entry ); }
Am Mi 30. Aug 2017, 01:09:26, richard.carver@cloudmont.co.uk schrieb: Show quoted text
> Net::SIP::StatelessProxy doesn't call __forward_packet_final in a way > that allows it to be overridden in an inherited class. The following > patch makes this possible. > > The reason I need this is that one side of my SIP proxy uses a URI > which has multiple AAAA records for load balancing, and often my > CANCEL requests are sent to a different IP address to the INVITE and > are not recognised. In order to fix this I am caching the DNS results > for the URI lookup on a per-call basis so that all requests for a > call-id go to the same IP, but each new call does a fresh DNS lookup. > To do this I have a class that inherits from Net::SIP::StatelessProxy. > It overrides __forward_packet_final to cache the result and overrides > __forward_request_getdaddr to use the cached value if needed. >
I don't think this is the correct approach. The case of a multihomed client you describe should in my opinion be instead handled by adding a received and rport parameter to the Via header and then using these information to send the response back to the correct client. This last part should already be implemented and I've tried to implement the first part (adding the parameters to the Via header) in branch rt122925. Could you please check if this solves the problem you have: https://github.com/noxxi/p5-net-sip/tree/rt122925 Regards, Steffen
Subject: RE: [rt.cpan.org #122925] Allow extending of StatelessProxy __forward_packet_final method
Date: Wed, 15 Nov 2017 15:37:55 +0000
To: "bug-Net-SIP [...] rt.cpan.org" <bug-Net-SIP [...] rt.cpan.org>
From: Richard Carver <richard.carver [...] cloudmont.co.uk>
Sorry for the delay in responding. I think the problem you are solving may not the problem I am having, or else I have misunderstood. This is my scenario: A <--> B (Net::SIP proxy) <--> C A is a single server. B is a single server instance of Net::SIP::StatelessProxy with my additional business logic. C is a collection of SIP proxy servers using DNS A record round-robin for load balancing. The DNS server for C returns a single A record with a 30s TTL chosen from a pool of A records that it has available. After the 30s TTL expires another lookup will be made and a different single A record will be returned. This is out of my control to influence. A sends an INVITE to B. B has business logic to work out where to send it, and sends it to C using a FQDN. This maps to a single IP address and is sent to that server. A then sends a CANCEL to B. This has no Route headers and a single Via header inserted by A. As there is no route information in this CANCEL it goes through the same business logic and is sent to C using DNS lookup. If the 30s TTL has expired then this DNS lookup will return a different IP address and the CANCEL will be sent to a different server that has no knowledge of the dialog. For BYE there would be Route headers and this would not be an issue - it's just the CANCEL scenario where I need to cache the DNS lookup. Regards, Richard Carver Cloudmont Limited Tel: 033 0001 0436 Mob: +44 (0) 7803 005932 Email/Skype for Business: Richard.Carver@cloudmont.co.uk Show quoted text
-----Original Message----- From: Steffen Ullrich via RT [mailto:bug-Net-SIP@rt.cpan.org] Sent: 21 October 2017 20:00 To: Richard Carver <richard.carver@cloudmont.co.uk> Subject: [rt.cpan.org #122925] Allow extending of StatelessProxy __forward_packet_final method <URL: https://rt.cpan.org/Ticket/Display.html?id=122925 > Am Mi 30. Aug 2017, 01:09:26, richard.carver@cloudmont.co.uk schrieb:
> Net::SIP::StatelessProxy doesn't call __forward_packet_final in a way > that allows it to be overridden in an inherited class. The following > patch makes this possible. > > The reason I need this is that one side of my SIP proxy uses a URI > which has multiple AAAA records for load balancing, and often my > CANCEL requests are sent to a different IP address to the INVITE and > are not recognised. In order to fix this I am caching the DNS results > for the URI lookup on a per-call basis so that all requests for a > call-id go to the same IP, but each new call does a fresh DNS lookup. > To do this I have a class that inherits from Net::SIP::StatelessProxy. > It overrides __forward_packet_final to cache the result and overrides > __forward_request_getdaddr to use the cached value if needed. >
I don't think this is the correct approach. The case of a multihomed client you describe should in my opinion be instead handled by adding a received and rport parameter to the Via header and then using these information to send the response back to the correct client. This last part should already be implemented and I've tried to implement the first part (adding the parameters to the Via header) in branch rt122925. Could you please check if this solves the problem you have: https://github.com/noxxi/p5-net-sip/tree/rt122925 Regards, Steffen
Am Mi 15. Nov 2017, 13:13:11, richard.carver@cloudmont.co.uk schrieb: Show quoted text
> Sorry for the delay in responding. I think the problem you are solving > may not the problem I am having, or else I have misunderstood. > ...
Now I think I've understood the problem. I'm still not really comfortable with this path because it let other rely on the specific behavior of an internal function (i.e. no official API). Still, I've made the change in commit 22b9728f. But be aware that this is still no official API you are using so the chance is higher that things will inadvertently break in the future.
Subject: RE: [rt.cpan.org #122925] Allow extending of StatelessProxy __forward_packet_final method
Date: Wed, 22 Nov 2017 08:24:59 +0000
To: "bug-Net-SIP [...] rt.cpan.org" <bug-Net-SIP [...] rt.cpan.org>
From: Richard Carver <richard.carver [...] cloudmont.co.uk>
I agree that it is risky extending an internal function like this. Net::SIP:: Dispatcher supports providing an alternative resolver - what do you think about updating Net::SIP:: Dispatcher to pass the call-id or packet or some other identifying information into the resolver so I could implement this per-call cache there? Show quoted text
-----Original Message----- From: Steffen Ullrich via RT [mailto:bug-Net-SIP@rt.cpan.org] Sent: 22 November 2017 08:08 To: Richard Carver <richard.carver@cloudmont.co.uk> Subject: [rt.cpan.org #122925] Allow extending of StatelessProxy __forward_packet_final method <URL: https://rt.cpan.org/Ticket/Display.html?id=122925 > Am Mi 15. Nov 2017, 13:13:11, richard.carver@cloudmont.co.uk schrieb:
> Sorry for the delay in responding. I think the problem you are solving > may not the problem I am having, or else I have misunderstood. > ...
Now I think I've understood the problem. I'm still not really comfortable with this path because it let other rely on the specific behavior of an internal function (i.e. no official API). Still, I've made the change in commit 22b9728f. But be aware that this is still no official API you are using so the chance is higher that things will inadvertently break in the future.
Am Mi 22. Nov 2017, 03:25:32, richard.carver@cloudmont.co.uk schrieb: Show quoted text
> I agree that it is risky extending an internal function like this. > > Net::SIP:: Dispatcher supports providing an alternative resolver - > what do you think about updating Net::SIP:: Dispatcher to pass the > call-id or packet or some other identifying information into the > resolver so I could implement this per-call cache there?
While this might work I think this is just another hack specifically for your use case. I think the main problem is that you are trying to use StatelessProxy in some stateful way, i.e. against the explicit stateless design.