Skip to content

Commit

Permalink
ipsec: Add a helper function to run commands from the monitor.
Browse files Browse the repository at this point in the history
Until now, functions that needed to call external programs like openssl
or ipsec commands were using subprocess commands directly.  Most of
these calls had no failure checks or any logging making it hard to
understand what is happening inside the daemon when something doesn't
work as intended.

Some commands also had a chance to not read the command output in full.
That might sound like not a big problem, but in practice it causes
ovs-monitor-ipsec to deadlock pluto and itself with certain versions of
Libreswan (mainly Libreswan 5+).  The order of events is following:

 1. ovs-monitor-ipsec calls ipsec status redirecting the output
    to a pipe.
 2. ipsec status calls ipsec whack.
 3. ipsec whack connects to pluto and asks for status.
 4. ovs-monitor-ipsec doesn't read the pipe in full.
 5. ipsec whack blocks on write to the other side of the pipe
    when it runs out of buffer space.
 6. pluto blocks on sendmsg to ipsec whack for the same reason.
 7. ovs-monitor-ipsec calls another ipsec command and blocks
    on connection to pluto.

In this scenario the running process is at the mercy of garbage
collector and it doesn't run because we're blocked on calling another
ipsec command.  All the processes are completely blocked and will not
do any work until ipsec whack is killed.

With this change we're introducing a new function that will be used
for all the external process execution commands and will read the full
output before returning, avoiding the deadlock.  It will also log all
the failures as warnings and the commands themselves at the debug level.

We'll be adding more logic into this function in later commits as well,
so it will not stay that simple.

Signed-off-by: Ilya Maximets <[email protected]>
  • Loading branch information
igsilya committed Oct 29, 2024
1 parent 06b8b9e commit 1fddefd
Showing 1 changed file with 131 additions and 159 deletions.
Loading

0 comments on commit 1fddefd

Please sign in to comment.