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


<p style="font-size:small;-webkit-text-size-adjust:none;color:#666;">—<br />You are receiving this because you are subscribed to this thread.<br />Reply to this email directly, <a href="https://github.com/kamailio/kamailio/pull/2148?email_source=notifications&email_token=ABO7UZLIAE6EVOJXFCTPTJTQVTQAJA5CNFSM4JRKYG72YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEFFHS2A#issuecomment-558528872">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/ABO7UZLAEZROKNWFHEDS72DQVTQAJANCNFSM4JRKYG7Q">unsubscribe</a>.<img src="https://github.com/notifications/beacon/ABO7UZJZ5VKF2D2DS4AOM73QVTQAJA5CNFSM4JRKYG72YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEFFHS2A.gif" height="1" width="1" alt="" /></p>
<script type="application/ld+json">[
{
"@context": "http://schema.org",
"@type": "EmailMessage",
"potentialAction": {
"@type": "ViewAction",
"target": "https://github.com/kamailio/kamailio/pull/2148?email_source=notifications\u0026email_token=ABO7UZLIAE6EVOJXFCTPTJTQVTQAJA5CNFSM4JRKYG72YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEFFHS2A#issuecomment-558528872",
"url": "https://github.com/kamailio/kamailio/pull/2148?email_source=notifications\u0026email_token=ABO7UZLIAE6EVOJXFCTPTJTQVTQAJA5CNFSM4JRKYG72YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEFFHS2A#issuecomment-558528872",
"name": "View Pull Request"
},
"description": "View this Pull Request on GitHub",
"publisher": {
"@type": "Organization",
"name": "GitHub",
"url": "https://github.com"
}
}
]</script>