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>
> .
>
--
Best Regards,
Aleksandar Yosifov
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.