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@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 .