[sr-dev] [kamailio/kamailio] ims_ipsec_pcscf: fixed warnings, added a new config param (#2148)

[alexyosifov]

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 at 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.kamailio.org/pipermail/sr-dev/attachments/20191126/a0c788fb/attachment.html>