26 Nov
2019
26 Nov
'19
9:55 a.m.
Hi, thanks for the quick response!
For future pull requests I will strictly separate bug fixes and new
features.
For SND_F_FORCE_SOCKET - I am using ipsec functionality with UDP or TCP.
For TCP I need to force send to existing socket (in combination with
tcp_reuse_port=yes) to pass the Request message through existing ipsec
tunnel. I think that change will not reflect other user, because it's valid
only for PROTO_TCP and PROTO_WS in combination with ipsec_forward() (using
of ipsec tunnels). For ipsec and UDP that is not a problem. I can add an
option (ipsec_force_send_socket or something like this) to be clear when
force socket is used or not.
For Register reply with code 200 - supported and secagree headers are
needed only for that message.
For locking - I could do refactor in some of the next changes.
Cheers!
On Mon, 25 Nov 2019 at 20:40, Henning Westerholt <notifications(a)github.com>
wrote:
Hello, thanks for the pull request.
Please separate the bugfixes (that needs to be backported) from functional
additions next time. I assume that the bugfix is in spi_gen.c and also one
related to the log in int add_supported_secagree_header(struct sip_msg* m)
. After merging I can manually backport.
About the functional change:
- in ipsec_forward() add supported and require secagree headers only
for Register reply with code 200. Set SND_F_FORCE_SOCKET for request
messages.
Two questions:
- Was the previous behaviour to apply it for all SIP methods wrong?
- Are there cases where one does not want to force the send socket for
the messages?
I am just trying to sort out if the change might break other people
configs and some parameter needs to be added.
I have one general remark related to the locking in the module. It uses
directly the pthread function calls. Usually all modules should use the
kamailio locking interface (in core/locking.h). The simple lock is probably
ok for this module. Would be great if you could convert it to this in one
of your next changes. You can look to e.g. to the ims_usrloc* modules to
have a look to the implementation.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<https://github.com/kamailio/kamailio/pull/2148?email_source=notifications&email_token=ALKTZB45LFAJXW6DCGLXXDDQVQLZ7A5CNFSM4JRKYG72YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEFDMKNI#issuecomment-558286133>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ALKTZB2Y2HGQDAYPZRHKXYDQVQLZ7ANCNFSM4JRKYG7Q>
.
--
Best Regards,
Aleksandar Yosifov
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/pull/2148#issuecomment-558528872