20 Jun
2023
20 Jun
'23
6:21 p.m.
fixed incorrect source IP address selection for the SIP messages sending procedure when
TCP transport is used or for UDP with the 'mhomed' setting set as
'mhomed=1'.
<!-- Kamailio Pull Request Template -->
<!--
IMPORTANT:
- for detailed contributing guidelines, read:
https://github.com/kamailio/kamailio/blob/master/.github/CONTRIBUTING.md
- pull requests must be done to master branch, unless they are backports
of fixes from master branch to a stable branch
- backports to stable branches must be done with 'git cherry-pick -x
...'
- code is contributed under BSD for core and main components (tm, sl, auth, tls)
- code is contributed GPLv2 or a compatible license for the other components
- GPL code is contributed with OpenSSL licensing exception
-->
#### Pre-Submission Checklist
<!-- Go over all points below, and after creating the PR, tick all the checkboxes
that apply -->
<!-- All points should be verified, otherwise, read the CONTRIBUTING guidelines
from above-->
<!-- If you're unsure about any of these, don't hesitate to ask on
sr-dev mailing list -->
- [x] Commit message has the format required by CONTRIBUTING guide
- [ ] Commits are split per component (core, individual modules, libs, utils, ...)
- [ ] Each component has a single commit (if not, squash them into one commit)
- [ ] No commits to README files for modules (changes must be done to docbook files
in `doc/` subfolder, the README file is autogenerated)
#### Type Of Change
- [x] Small bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds new functionality)
- [ ] Breaking change (fix or feature that would change existing functionality)
#### Checklist:
<!-- Go over all points below, and after creating the PR, tick the checkboxes that
apply -->
- [ ] PR should be backported to stable branches
- [x] Tested changes locally
- [x] Related to issue #3486(replace XXXX with an open issue number)
#### Description
<!-- Describe your changes in detail -->
The current service behavior is as follows: if OS contains several network interfaces and
the service has a listening TCP socket (or UDP with 'mhomed=1' setting) on
the second (third, fourth and so on) interface, then the outbound SIP messages will be
always sent from the first (default) interface.
These changes make the outbound SIP messages always going through the
network_interface/address, which is taken for listening by service.
You can view, comment on, or merge this pull request online at:
https://github.com/kamailio/kamailio/pull/3490
-- Commit Summary --
* core: fixed wrong network interface selection.
-- File Changes --
M src/core/forward.c (7)
M src/core/udp_server.c (37)
M src/core/udp_server.h (6)
M src/main.c (2)
-- Patch Links --
https://github.com/kamailio/kamailio/pull/3490.patch
https://github.com/kamailio/kamailio/pull/3490.diff
--
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/pull/3490
You are receiving this because you are subscribed to this thread.
Message ID: &lt;kamailio/kamailio/pull/3490(a)github.com&gt;
20 Jun
20 Jun
6:29 p.m.
There is a note that tcp_set_src_addr() should be used only on init:
- https://github.com/kamailio/kamailio/blob/master/src/core/tcp_main.c#L217
Are you sure it does not have side effects, when there is a mix of forcing and not-forcing
send socket?
Is it compatible with the old behaviour when the global parameters
tcp_source_ipv4/tcp_source_ipv6 were supposed to be global for all traffic?
--
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/pull/3490#issuecomment-1599024379
You are receiving this because you are subscribed to this thread.
Message ID: <kamailio/kamailio/pull/3490/c1599024379(a)github.com>
23 Jun
23 Jun
1:29 p.m.
@ivanuschak pushed 1 commit.
6323eb264c0d9cee7f1a6fc028bd7453b26f5cc8 core: fixed wrong network interface selection.
--
View it on GitHub:
https://github.com/kamailio/kamailio/pull/3490/files/8d5804162022c38fbf93aa…
You are receiving this because you are subscribed to this thread.
Message ID: <kamailio/kamailio/pull/3490/push/14085842137(a)github.com>
1:52 p.m.
_tcp_set_src_addr()_/_udp_set_src_addr()_ function calls have been moved to the _main()_
after _fix_all_socket_list()_ invocation. This place is where service is being initiated
and not yet entered into the main loop.
_udp_source_ipv6_/_tcp_source_ipv6_/_udp_source_ipv4_/_tcp_source_ipv4_ are global
variables and they are initiated on the service init step and then there are no changes
for this variables.
Regarding
Is it compatible with the old behaviour when the
global parameters tcp_source_ipv4/tcp_source_ipv6 were supposed to be global for all
traffic?
Is there some old behavior, which can be enabled by some option? If yes,
then the compatibility with this mode should be kept, right?
I didn't find these variables (_tcp_source_ipv4_/_tcp_source_ipv6_) are initialized
somewhere in the source code, but they are used, so it looks like its initialization is
needed.
--
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/pull/3490#issuecomment-1604103519
You are receiving this because you are subscribed to this thread.
Message ID: <kamailio/kamailio/pull/3490/c1604103519(a)github.com>
2:22 p.m.
The `tcp_source_ipv4` and `tcp_source_ipv6` are set by config parser:
- https://github.com/kamailio/kamailio/blob/master/src/core/cfg.y#L1203-L1222
Your commit likely is overwriting them.
--
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/pull/3490#issuecomment-1604140463
You are receiving this because you are subscribed to this thread.
Message ID: <kamailio/kamailio/pull/3490/c1604140463(a)github.com>
2:24 p.m.
@miconda commented on this pull request.
@@ -2864,6 +2864,16 @@ int main(int argc, char** argv)
fprintf(stderr, "failed to initialize list addresses\n");
goto error;
}
+ for(struct socket_info* si=udp_listen; si; si=si->next){
+ udp_set_src_addr(&si->address);
+ }
This loop results is setting the global variables to the last IPv4 or IPv6 listen
addresses in the list, right?
Same happens with the loop for tcp below.
--
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/pull/3490#pullrequestreview-1494965107
You are receiving this because you are subscribed to this thread.
Message ID: <kamailio/kamailio/pull/3490/review/1494965107(a)github.com>
5:21 p.m.
Yes, you're correct the `tcp_source_ipv4` and `tcp_source_ipv6` are set by config
parser. My commit replaces this logic. This replacing is not correct, since these vars are
configurable from the config file. I'll remove the logic for TCP.
But I don't see similar logic for UDP, i.e. there is no default IP addresses.
Is there similar source IP addresses for UDP?
--
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/pull/3490#issuecomment-1604354086
You are receiving this because you are subscribed to this thread.
Message ID: <kamailio/kamailio/pull/3490/c1604354086(a)github.com>
5:25 p.m.
@ivanuschak commented on this pull request.
@@ -2864,6 +2864,16 @@ int main(int argc, char** argv)
fprintf(stderr, "failed to initialize list addresses\n");
goto error;
}
+ for(struct socket_info* si=udp_listen; si; si=si->next){
+ udp_set_src_addr(&si->address);
+ }
Yes, correct. The loop result is setting the global variables to the last IPv4 or IPv6
listen addresses in the list. The same for UDP.
But these loops are not correct since these source IP addresses are set up by config file
(at least for TCP), these loops just replace the source IP addresses. I will change it.
--
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/pull/3490#discussion_r1239887201
You are receiving this because you are subscribed to this thread.
Message ID: <kamailio/kamailio/pull/3490/review/1495254884(a)github.com>
26 Jun
26 Jun
4:42 p.m.
I haven't added `tcp_source_ipv4` or `tcp_source_ipv6`, neither used them, so I am not
sure what was the main purpose for them. I also don't know why there is no equivalent
for udp, maybe one should be introduced. But more or less from the beginning of the
project, the rule to reuse the incoming socket for outgoing traffic was in place.
Maybe the next steps should be:
- add also for udp the corresponding core parameters to be set from config
- add a new parameter like `proto_source_mode` which when set to 0 (the default) the
current behaviour is preserved and when set to 1 the one you want is done, if that is
solving issues you are facing
In this way we have backward compatibility by default and you have an option to change
that behaviour for your specific needs.
--
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/pull/3490#issuecomment-1607500234
You are receiving this because you are subscribed to this thread.
Message ID: <kamailio/kamailio/pull/3490/c1607500234(a)github.com>
24 Jul
24 Jul
2:10 p.m.
@ivanuschak pushed 1 commit.
d0944d1c9a97d7962dc757a28e24e2dfcbd0746d Fixed wrong network interface selection for SIP
messages forwarding.
--
View it on GitHub:
https://github.com/kamailio/kamailio/pull/3490/files/6323eb264c0d9cee7f1a6f…
You are receiving this because you are subscribed to this thread.
Message ID: <kamailio/kamailio/pull/3490/push/14424464395(a)github.com>
7:56 p.m.
@ivanuschak pushed 1 commit.
f26d2cd25f9d3ef953f61f84ceae5cb2a6c65d48 Fixed wrong network interface selection for SIP
messages forwarding.
--
View it on GitHub:
https://github.com/kamailio/kamailio/pull/3490/files/d0944d1c9a97d7962dc757…
You are receiving this because you are subscribed to this thread.
Message ID: <kamailio/kamailio/pull/3490/push/14428942972(a)github.com>
25 Jul
25 Jul
4:54 p.m.
@ivanuschak pushed 1 commit.
f7cf3be73dab3eaa9648dd274fcff1a302be2208 core: fixed wrong network interface selection.
--
View it on GitHub:
https://github.com/kamailio/kamailio/pull/3490/files/f26d2cd25f9d3ef953f61f…
You are receiving this because you are subscribed to this thread.
Message ID: <kamailio/kamailio/pull/3490/push/14440875200(a)github.com>
5:01 p.m.
@ivanuschak pushed 1 commit.
c62fc6ce9b72048cc2a7b3d2883994bd7226bc5d core: fixed wrong network interface selection.
--
View it on GitHub:
https://github.com/kamailio/kamailio/pull/3490/files/f7cf3be73dab3eaa9648dd…
You are receiving this because you are subscribed to this thread.
Message ID: <kamailio/kamailio/pull/3490/push/14440967658(a)github.com>
5:06 p.m.
@ivanuschak pushed 1 commit.
37e4196bc816ce5aa588f6f1ff7e6be7fed457ee core: fixed wrong network interface selection.
--
View it on GitHub:
https://github.com/kamailio/kamailio/pull/3490/files/c62fc6ce9b72048cc2a7b3…
You are receiving this because you are subscribed to this thread.
Message ID: <kamailio/kamailio/pull/3490/push/14441037960(a)github.com>
5:18 p.m.
@ivanuschak pushed 1 commit.
b6eb3a53f826bb3cc9e68b93308c50f1ecae49a4 core: fixed wrong network interface selection.
--
View it on GitHub:
https://github.com/kamailio/kamailio/pull/3490/files/37e4196bc816ce5aa588f6…
You are receiving this because you are subscribed to this thread.
Message ID: <kamailio/kamailio/pull/3490/push/14441222881(a)github.com>
5:24 p.m.
@ivanuschak pushed 1 commit.
d42f8e68fa396f7daca74fbce1eb256e76baf040 core: fixed wrong network interface selection.
--
View it on GitHub:
https://github.com/kamailio/kamailio/pull/3490/files/b6eb3a53f826bb3cc9e68b…
You are receiving this because you are subscribed to this thread.
Message ID: <kamailio/kamailio/pull/3490/push/14441319474(a)github.com>
5:30 p.m.
Implementation was re-worked. Now for _tcp_do_connect_ function if _from_ parameter is
null then the sending socket is bound on the first listening address/port, which matches
the required address family (IPv4/IPv6).
The same approach is implemented for UDP sockets when 'mhomed=1'.
--
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/pull/3490#issuecomment-1649958226
You are receiving this because you are subscribed to this thread.
Message ID: <kamailio/kamailio/pull/3490/c1649958226(a)github.com>
7:02 p.m.
@sergey-safarov commented on this pull request.
+inline static int find_listening_sock_info(
+ int s, union sockaddr_union **from, int type)
+{
+ struct ip_addr ip;
+ struct socket_info *si = NULL;
+ su2ip_addr(&ip, *from);
+
+ si = find_si(&ip, 0, type);
+
+ if(unlikely(si == 0)) {
+ si = find_sock_info_by_address_family(type, ip.af);
+ if(si) {
+ int optval = 1;
+ su2ip_addr(&ip, &si->su);
+ *from = &si->su;
+ if(setsockopt(s, SOL_SOCKET, SO_REUSEADDR, (void *)&optval,
Current implementation requires kernel with `SO_REUSEADDR` and `SO_REUSEPORT`.
I have checked sources and found several places where this features also required.
I have a question.
Can we relay on statement "all kernels provide `SO_REUSEADDR` and
`SO_REUSEPORT`" functionality?
--
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/pull/3490#pullrequestreview-1545852256
You are receiving this because you are subscribed to this thread.
Message ID: <kamailio/kamailio/pull/3490/review/1545852256(a)github.com>
25 Sep
25 Sep
6:04 p.m.
Merged #3490 into master.
--
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/pull/3490#event-10464017734
You are receiving this because you are subscribed to this thread.
Message ID: <kamailio/kamailio/pull/3490/issue_event/10464017734(a)github.com>
428
days inactive
525
days old
18 comments
3 participants
participants (3)
-
Daniel-Constantin Mierla -
Ivanuschak Nikolay
-
sergey-safarov