W dniu 10.03.2019 o 08:10, Otto Moerbeek pisze:
On Sat, Mar 09, 2019 at 11:19:11PM +0100, Michał Koc wrote:
W dniu 09.03.2019 o 12:43, Otto Moerbeek pisze:
On Sat, Mar 09, 2019 at 12:10:34PM +0100, Michał Koc wrote:
W dniu 09.03.2019 o 08:15, Otto Moerbeek pisze:
On Fri, Mar 08, 2019 at 12:03:25PM +0100, Michał Koc wrote:
Hi all,
We have a triple redundant vpn gateway setup with sasyncd running and tons
of tunnels, about 1000 flows.
Looking at the graph of memory usage, you can clearly see that something is
sucking up the memory.
The graph can be viewed here: https://pasteboard.co/I4sjzQ8.jpg
Looking at the ps, sasyncd shows huge memory consumption:
USER PID %CPU %MEM VSZ RSS TT STAT
STARTED TIME COMMAND
_isakmpd 33560 0.0 17.0 699264 708508 ?? S
26Feb19 6:58.81 /usr/sbin/sasyncd
It only happens on the master node. Slaves do not show such a behavior.
There is nothing about sasyncd in the logs.
After sasyncd restart memory consumption is minimal, but tends to grow.
Is it normal ? or am I missing something ?
Best regards
M.K.
This is not normal. You could try to run with -vv to see if some error
path is taken that triggers a leak.
-Otto
Should I look for something specific ?
The log grows pretty fast and it looks like it could contain some security
data which I wouldn't like to post online.
The statistics of the log(about 2 hours) looks like this:
carp_init: 1
config: 7
monitor_get_pfkey_snap: 4
monitor_loop: 1
net: 1
net_connect: 3
net_ctl: 4
net_disconnect_peer: 3
net_handle_messages: 2
net_queue: 91780
net_read: 10
net_send_messages: 39192
pfkey_send_flush: 4
pfkey_snapshot: 6832
timer_add: 19
timer_run: 18
Best regards
M.K.
Just the counts does not reveal anything. I did a quick audit of the
memory allocation logic of sasyncd and did not spot an error. If you
do not want to post the logs, you'll neeed to analyze them yourself.
This requires matching the log lines to the code and tracking where
stuff gets allocated and deallocated. Some digging could reveal the error.
I used to run sasyncd, but I do no longer. Settig up a test env is
quite some work I do not have time for.
-Otto
First of all, thank You for your time. I know it's one of the most valuable
resource.
We have done some analysis and we have found the problem.
The problem is that the very master machine exists as a peer in it's config.
The purpose of this is to make all of the config files to be as similar as
possible for all of the members of the cluster.
Removing it from peers fixes the problem.
So there are three main roads we can follow:
1. Fix sasyncd to recognize self and handle it properly (a result)
2. It should be mentioned in manual, not to set self as a peer (an excuse)
3. We can change our internal config handling (no one else benefits)
What would You recommend as a next step ? we will do much to follow road 1,
but we might need help, eg. code review and some guidance to meet OpenBSD
needs
Furthermore if we follow road 1, is there any chance to get the reviewed,
correct, accepted fix into the tree ?
Best regards
M.K.
Good you found the problem.
As for number 1, in general it is hard to determine if an IP is your
own (or redirects to yourself), especially since network interfaces
can be added and changed dynamically. But a starup check against
getifaddrs(3) would catch the most obvious I'm my own peer cases.
A dynamic form of loop detection would be nice, it would require
adding a hostid or equivalent. That is not normally set these days and
also would change the wire format of the messages.
But even if you start work on code, a manual page change warning
against this would be a good thing.
If you post diffs to tech@ with a proper explanation they will be
picked up and reviewed, either by me or somee other developer.
BTW, I noticed there are order dependencies in sasyncd.cnf that are
not mentioned in the man page. Also the example in
src/etc/examples/sasyncd.conf does not work if you just uncomment the
lines because of those order dependencies. So there's more room for
improvement in the man page and example config.
-Otto
Correct me please if I am wrong, but in my opinion vpn gateways are
usually static setups, because of design, security, network,
documentation, politics and other reasons
So startup check should at least give a clue when something goes wrong.
Of course setting up some kind of host id would be best solution, we
will get back to it in less busy time.
The patch below makes the following changes:
1. adds startup check for peers being listed in local addresses
2. adds proper log messages in cases of peer being local address or
duplicate
3. fixes duplicate handling, which was incorrect
4. changes name of "duplicate" variable to "valid" to allow it to used
in more cases than just duplicate
Best regards
M.K.
Index: conf.y
===================================================================
RCS file: /cvs/src/usr.sbin/sasyncd/conf.y,v
retrieving revision 1.19
diff -u -p -r1.19 conf.y
--- conf.y 9 Apr 2017 02:40:24 -0000 1.19
+++ conf.y 10 Mar 2019 13:39:04 -0000
@@ -30,8 +30,10 @@
#include <sys/types.h>
#include <sys/stat.h>
#include <sys/socket.h>
+#include <arpa/inet.h>
#include <ctype.h>
#include <fcntl.h>
+#include <ifaddrs.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
@@ -172,17 +174,50 @@ setting : INTERFACE STRING
| PEER STRING
{
struct syncpeer *peer;
- int duplicate = 0;
+ int valid = 1;
+ struct ifaddrs *ifap = 0, *ifa;
+ char *ifaddrname = 0;
+
+ if (getifaddrs(&ifap) == 0) {
+ for (ifa = ifap; ifa && valid; ifa =
ifa->ifa_next) {
+ if (!ifa->ifa_addr)
+ continue;
+
+ switch (ifa->ifa_addr->sa_family) {
+ case AF_INET:
+ ifaddrname =
malloc(INET_ADDRSTRLEN);
+ if (ifaddrname)
+ inet_ntop(AF_INET, &((struct
sockaddr_in *)ifa->ifa_addr)->sin_addr, ifaddrname, INET_ADDRSTRLEN);
+ break;
+ case AF_INET6:
+ ifaddrname =
malloc(INET6_ADDRSTRLEN);
+ if (ifaddrname)
+ inet_ntop(AF_INET6, &((struct
sockaddr_in6 *)ifa->ifa_addr)->sin6_addr, ifaddrname, INET6_ADDRSTRLEN);
+ break;
+ }
+
+ if(ifaddrname) {
+ if (strcmp($2, ifaddrname) ==
0) {
+ log_msg(2, "config: local
peer %s", $2);
+ valid = 0;
+ }
+ free(ifaddrname);
+ ifaddrname = 0;
+ }
+ }
+ freeifaddrs(ifap);
+ }
- for (peer = LIST_FIRST(&cfgstate.peerlist); peer;
- peer = LIST_NEXT(peer, link))
- if (strcmp($2, peer->name) == 0) {
- duplicate++;
+ if (valid)
+ for (peer = LIST_FIRST(&cfgstate.peerlist); peer;
+ peer = LIST_NEXT(peer, link))
+ if (strcmp($2, peer->name) == 0) {
+ log_msg(2, "config: duplicate peer %s",
$2);
+ valid = 0;
break;
- }
- if (duplicate)
- free($2);
- else {
+ }
+
+ if (valid) {
peer = calloc(1, sizeof *peer);
if (!peer) {
log_err("config: calloc(1, %lu) "
@@ -191,10 +226,11 @@ setting : INTERFACE STRING
YYERROR;
}
peer->name = $2;
- }
- LIST_INSERT_HEAD(&cfgstate.peerlist, peer, link);
- cfgstate.peercnt++;
- log_msg(2, "config: add peer %s", peer->name);
+ LIST_INSERT_HEAD(&cfgstate.peerlist, peer,
link);
+ cfgstate.peercnt++;
+ log_msg(2, "config: add peer %s", peer->name);
+ } else
+ free($2);
}
| LISTEN ON STRING af port
{
