Skip to content

Changes

Page history
rename designs to Designs authored by Andrei Pavel's avatar Andrei Pavel
Hide whitespace changes
Inline Side-by-side
Designs/Run-external-script-hook.md 0 → 100644
View page @ e3c1f9fd
# WIP!
Requirements:
1. the implementation needs to support to send back and forth parameters (using environment and maybe stdout and stderr data) and be able to set SKIP/DROP flag using 'next_step' variable and return status.
2. the call to the external script needs to be able to support blocking (synchronous)/non-blocking (fire and forget) mode.
(the term async is misleading and should refer to handling the response when the operation finishes and would require something like packet parking)
3. if calling the script synchronously, a timer must be started so that on timeout kea can kill the child process (kea should not block indefinitely if the script hangs)
4. in synchronous mode (also a good debugging feature also) the library would use a pipe and send back the stdout and stderr of the child (either by using popen or pipe + dup2)
5. the library wrapper script should receive a list of IN parameters and OUT parameters and print the OUT parameters on stdout before returning the control to Kea (the only way to receive update from the script is through stdout and stderr)
6. the output of the script must be parsed to update the OUT parameters, the return status and the 'next_step' for SKIP/DROP flag
7. kea does not create files with close on exec flag, and if the child process does exit (not _exit) buffers can be written twice to opened files so we would need to close all fds in child before calling execv
8. spawn process and wait should be called on the same thread or from any thread (most positive, but might depend on the implementation), but it can be main or any other thread
9. only the main thread should have SIGCHLD unmasked, all other threads must block the signal
10. even though all hook points can be implemented, the initial implementation should implement only a subset (TBD):
dhcpv4
```
subnet4_select COULD
called from: Dhcpv4Srv::acceptDirectRequest Dhcpv4Srv::processDiscover Dhcpv4Srv::processInform Dhcpv4Srv::processRequest
IN: query4 subnet4 subnet4collection
OUT: subnet4
lease4_select COULD
called from: Dhcpv4Srv::processDiscover Dhcpv4Srv::processRequest
IN: query4 subnet4 fake_allocation lease4
OUT: lease4
lease4_renew MUST
called from: Dhcpv4Srv::processDiscover Dhcpv4Srv::processRequest
IN: query4 subnet4 clientid hwaddr lease4
OUT: NONE
lease4_expire MUST
called from: AllocEngine::reclaimExpiredLeases4
IN: lease4 remove_lease
OUT: NONE
lease4_recover MUST
called from: AllocEngine::reclaimExpiredLease
IN: lease4
OUT: NONE
leases4_committed MUST
called from: Dhcpv4Srv::processPacket
IN: query4 leases4 deleted_leases4
OUT: NONE
lease4_release MUST
called from: Dhcpv4Srv::processRelease
IN: query4 lease4
OUT: NONE
lease4_decline MUST
called from: Dhcpv4Srv::processDecline
IN: query4 lease4
OUT: NONE
```
dhcpv6
```
subnet6_select COULD
called from: Dhcpv6Srv::processPacket
IN: query6 subnet6 subnet6collection
OUT: subnet6
lease6_select COULD
called from: Dhcpv6Srv::processSolicit Dhcpv6Srv::processRenew Dhcpv6Srv::processRebind
IN: query6 subnet6 fake_allocation lease6
OUT: lease6
lease6_renew MUST
called from: Dhcpv6Srv::processRenew Dhcpv6Srv::processRebind
IN: query6 lease6 ia_na/ia_pd
OUT: NONE
lease6_rebind MUST
called from: Dhcpv6Srv::processRenew Dhcpv6Srv::processRebind
IN: query6 lease6 ia_na/ia_pd
OUT: NONE
lease6_expire MUST
called from: AllocEngine::reclaimExpiredLeases6
IN: lease6 remove_lease
OUT: NONE
lease6_recover MUST
called from: AllocEngine::reclaimExpiredLease
IN: lease6
OUT: NONE
leases6_committed MUST
called from: Dhcpv6Srv::processPacket
IN: query6 leases6 deleted_leases6
OUT: NONE
lease6_release MUST
called from: Dhcpv6Srv::processRelease
IN: query6 lease6
OUT: NONE
lease6_decline MUST
called from: Dhcpv6Srv::processDecline
IN: query6 lease6
OUT: NONE
```
hook points ignored:
dhcpv4
```
dhcp4_srv_configured
called from: ControlledDhcpv4Srv::processConfig
IN: io_context network_state json_config server_config
OUT: NONE
command_processed
called from:
IN:
OUT:
cb4_updated
called from:
IN:
OUT:
pkt4_receive
called from: Dhcpv4Srv::processPacket
IN: query4
OUT: query4
buffer4_receive
called from: Dhcpv4Srv::processPacket
IN: query4
OUT: query4
host4_identifier
called from: Dhcpv4Srv::processDiscover Dhcpv4Srv::processInform Dhcpv4Srv::processRequest
IN: query4 id_type id_value
OUT: id_type id_value
buffer4_send
called from: Dhcpv4Srv::processPacket
IN: response4
OUT: response4
pkt4_send
called from: Dhcpv4Srv::processPacket
IN: query4 response4
OUT: NONE
```
dhcpv6
```
dhcp6_srv_configured
called from: ControlledDhcpv6Srv::processConfig
IN: io_context network_state json_config server_config
OUT: NONE
command_processed
called from:
IN:
OUT:
cb6_updated
called from:
IN:
OUT:
pkt6_receive
called from: Dhcpv6Srv::processPacket
IN: query6
OUT: query6
buffer6_receive
called from: Dhcpv6Srv::processPacket
IN: query6
OUT: query6
host6_identifier
called from: Dhcpv6Srv::processPacket
IN: query6 id_type id_value
OUT: id_type id_value
buffer6_send
called from: Dhcpv6Srv::processPacket
IN: response6
OUT: response6
pkt6_send
called from: Dhcpv6Srv::processPacket
IN: query6 response6
OUT: NONE
```
As all hooks of interest only use the lease4/lease6 types and query4/query6, only some data related to the lease and packet structure must be exported to the hook.
Most of the hook points don't have any OUT parameters except next_step which sets SKIP/DROP flags.
This indicated that there is not strong need for parsing the stdout received back to update parameters.
The only relevant hook points remain:
dhcpv4
```
lease4_renew
lease4_expire
lease4_recover ?
leases4_committed
lease4_release
lease4_decline
```
dhcpv6
```
lease6_renew
lease6_rebind
lease6_expire
lease6_recover ?
leases6_committed
lease6_release
lease6_decline
```
Adding support for synchronous and stdout and stderr parsing in ProcessSpawn
To be able to spawn synchronous child processes, the ProcessSpawn needs to extend it's functionality by adding:
pid_t ProcessSpawn::spawn(bool sync, uint32_t timeout)
This function should spawn a process synchronously/blocking or non-blocking as specified in the 'sync' parameter.
If the 'sync' is set to true, the function should use the timeout (nonzero) value to start the child process, and use the timeout to wait for it. If the timeout is reached, a kill signal will be send to the child. In any case, after the child has ended, the process stdout and stderr will be collected. The status of the child process will be collected using waitpid by the interrupt handler ProcessSpawnImpl::waitForProcess just like before.
The spawn function should create two pipes (out_pipe and err_pipe) and use that as the child stdout and stderr FDs using dup2.
Another pipe must be created to synchronously wake up after the process has finished processing (sync_pipe).
A thing that must be taken into account is that the main thread is the only thread with unmasked SIGCHLD so the waitForProcess can only be called on this thread.
As ProcessSpawn::spawn and ProcessSpawn::wait can be called on any thread, any communication with the main thread must be thread safe but also use only async-signal-safe functions.
Using a mutex to protect any container would not help as lock and unlock can block indefinitely.
To simplify the handling of SIGCHLD without interfering with multiple threads, the ProcessSpawn::spawn will need to fork. The created child will need to unmask the SIGCHLD. By doing this, the child process can handle it's own child process and the SIGCHLD on the only thread created by fork. It can fork again a child which will do execv and wait on a select with timeout. The select will exit either on the SIGCHLD interrupt on on timeout.
If the timeout expires, the child process will be killed.
After the child process exited, the sync_pipe is used to wake up the Kea thread calling ProcessSpawn::spawn
To wait for the process to finish, the ProcessSpawn::wait will be used (presented below) by ProcessSpawn::spawn internally.
If the 'sync' is set to false, the previous implementation (current master branch) can be used (fire and forget) and the return status of the script can be handled by the ProcessSpawnImpl::waitForProcess function without any extra steps.
To have consistency over all spawned child processes we can extend the functionality and allow to wait for processes started in non-blocking mode, so the ProcessSpawn::wait can be called with the specific PID at any time after ProcessSpawn::spawn was called.
ProcessSpawn::wait(pid_t, std::ostringstream, std::ostringstream)
To wait on the process to finish, the calling thread will block waiting on read on a pipe (sync_pipe).
The stdout of the child process can be optionally collected from the ProcessSpawn::wait function.
An example of using pipe with signal SIGCHLD is described:
https://stackoverflow.com/questions/282176/waitpid-equivalent-with-timeout
Using a condition variable and catching the interrupt in the waiting thread is not necessary and by running multiple threads, there would be difficult to filter out which wait condition variable has consumed it's predicate or not.
For reference, waiting using condition variables and signals:
https://docs.oracle.com/cd/E19683-01/806-6867/gen-24356/index.html
```
ProcessSpawn::spawn(...) {
int pipefd[2];
if (pipe(pipefd) == -1) {
isc_throw(isc::Unexpected, "failed to create sync pipe");
}
int outpipefd[2];
if (pipe(outpipefd) == -1) {
close(pipefd[0]);
close(pipefd[1]);
isc_throw(isc::Unexpected, "failed to create stdout pipe");
}
int errpipefd[2];
if (pipe(errpipefd) == -1) {
close(pipefd[0]);
close(pipefd[1]);
close(outpipefd[0]);
close(outpipefd[1]);
isc_throw(isc::Unexpected, "failed to create stderr pipe");
}
pid_t intermediate_pid = fork();
if (intermediate_pid == 0) {
// intermediate child
// first close all opened FDs and leave only sync, stdout and stderr pipes opened
struct rlimit fd_limit;
memset(&df_limit, 0, sizeof(fd_limit));
getrlimit(RLIMIT_NOFILE, &fd_limit);
uint32_t max_fd = fd_limit.rlim_max;
for (int i = 3; i < max_fd; ++i) {
// don't close the write end of the sync, stdout and stderr pipes
// as they will be used later
if (i == pipefd[1] || i == outpipefd[1] || i == errpipefd[1]) {
continue;
}
close(i);
}
// second update proper SIGNAL MASK
sigset_t new_set;
// mask all signals
sigfillset(&new_set);
pthread_sigmask(mask, &new_set, 0);
// unmask SIGCHLD
sigemptyset(&new_set);
sigaddset(&new_set, SIGCHLD);
pthread_sigmask(mask, &new_set, 0);
// third close stdin and set stdout and stderr pipes as stdout and stderr
int devnullfd = open("/dev/null", O_RDWR, 0);
// set stdin as devnull
(void)dup2(devnullfd, 0);
(void)close(devnullfd);
// set write end of the pipe as stdout
(void)dup2(outpipefd[1], 1);
// close outpipefd[1]
(void)close(outpipefd[1]);
// set write end of the pipe as stderr
(void)dup2(errpipefd[1], 2);
// close errpipefd[1]
(void)close(errpipefd[1]);
pid_t worker_pid = fork();
if (worker_pid == 0) {
// actual process to run the external program
execv(...);
_exit(0);
} else if (worker_pid < 0) {
write(pipefd[1], "", 1);
_exit(0);
}
struct timeval tv;
int retval;
tv.tv_sec = timeout;
tv.tv_usec = 0;
// sleep for timeout using select or wake on SIGCHLD
retval = select(0, NULL, NULL, NULL, &tv);
if (retval == -1) {
// SIGCHLD is the only signal that could have waken the process
// nothing left to do
} else if (retval == 0) {
// timeout elapsed
kill(worker_pid, SIGKILL);
} else {
// should never happen
}
// collect the child process status
waitpid(worker_pid);
// send sync signal to wake calling process thread
write(pipefd[1], "", 1);
_exit(0);
} else if (intermediate_pid > 0) {
// initial thread
int n = 0;
do {
char buf;
n = read(pipefd[0], &buf, 1);
if (n == 1) {
// read stdout and stderr
...
break;
}
} while (n == -1 && errno == EINTR);
}
close(pipefd[1]);
close(outpipefd[1]);
close(errpipefd[1]);
close(pipefd[0]);
close(outpipefd[0]);
close(errpipefd[0]);
// the intermediate_pid process state will be cleared by ProcessSpawnImpl::waitForProcess
// so no zombie process is left.
}
```
The hook library will contain a single instance of a RunScript class.
The class should be able to store information related to the script name and path, calling the script synchronously or not, and verify execution privileges on the file.
The instance will be created on the 'load' hook point.
```
class RunScript {
public:
/// @brief Constructor.
RunScript(std::string full_name);
/// @brief Destructor.
~RunScript();
/// @brief Call external script.
///
/// This function calls run_script_wrapper.sh internally.
///
/// @param hook_point_name The name of the hook point which will be handled by
/// the external script.
/// @param [in] in_params A map with the input parameters of the hook point.
/// @params [in|out] out_params A map with the names of the parameters that can to be
/// exported by the script.
void runScriptWithParameters(std::string& hook_point_name,
const std::map<std::string, std::string>& in_params,
std::map<std::string, std::string>& out_params);
private:
/// @prief The external script name.
std::string script_name_;
/// @brief The
bool sync_;
};
```
The Implementation will use ProcessSpawn internally and call an additional script (run_script_wrapper.sh) which is used to explicitly print the output parameters on stdout.
The wrapper script required parameters are:
```
script_name hook_point_name
with environment variables set for
key=value for 'list_of_in_params'
and
hook_parameters="space separated keys" for 'list_of_out_params'
```
eg:
```
run_script_test.sh lease4_select
with environment variables:
next_step=continue
query4=""
subnet4=""
fake_allocation=false
lease4=192.168.0.5
hook_parameters="lease4 next_step"
```
The script looks something like this:
```
if test ${#} -lt 2; then
echo "Usage: ${0} script_name hook_point_name"
echo " All variables used by the script must be available"
echo " as environment variables."
echo " All variables specified in the 'hook_parameters'"
echo " variable will be printed to stdout."
exit 1
fi
# set parameters
script_name=${1}
function_name=${2}
# calling script with 'source' so that we can get back updated variables
source ${script_name} ${function_name}
# mark parameter export after script ended
echo "START_EXPORTING_HOOK_PARAMETERS"
# print output parameters on stdout so that the Run Script hook can parse them
for parameter in ${hook_parameters}; do
echo "${parameter}=${!parameter}"
done
# mark parameter export after script ended
echo "END_EXPORTING_HOOK_PARAMETERS"
```
After the script is run, on stdout the Run Script hook will be able to parse:
```
...
START_EXPORTING_HOOK_PARAMETERS
lease4=192.168.0.5
next_step=drop
END_EXPORTING_HOOK_PARAMETERS
```
The output parameters should be delimited on stdout by specific labels:
```
START_EXPORTING_HOOK_PARAMETERS
...
END_EXPORTING_HOOK_PARAMETERS
```
An example of a script which handles 'lease4_select' hook point would look like:
```
add_ipv4_routes()
{
if [ "${lease4}" = "192.168.0.5" ]; then
next_step=drop
else
# Add interface route towards client
...
fi
}
case "$1" in
"lease4_select")
# Only add route if fake_allocation is set to 0 aka request/renew
[ "${fake_allocation}" = "0" ] || break
add_ipv4_routes
;;
esac
```
Configuration of the library should look like:
```
{
"Dhcp4": {
"hook_libraries": [
{
"library": "/usr/local/lib/libdhcp_run_script.so",
"parameters": {
"name": "/path_to/script_name.sh",
"sync": false
}
}
]
}
}
```
Francis initial comment:
the child process status must be collected even when it is not used, i.e. please to not create zombie processes. This is done by calling one of the wait system calls on the delivery of the SIGCHLD signal. This last point is the source of the incompatibility with multi-threading: signals are process (vs thread) things and are delivered to one of the threads which do not block it so the only way to direct a signal to a specific thread is to block it in all threads at the exception of one (usually the main thread or a dedicated one0. This means that two threads can not easily run a script in parallel...
Francis historical point:
in ~2004/2005 I worked on a DHCPv6 PD only implementation which used an external script to collect and piggyback RADIUS information in the relay. It used popen() i.e. the variant of system() which gives more that the status. It was used too to manage routes so I strongly insist on the report of the status to Kea: if there is an error the response must not be sent. So there are use cases for both the asynchronous (not waiting) and synchronous (wait) versions of the script feature. The only point I relax this requirement is that the synchronous version does not need to be supported with multi threading in a first step.
Francis point about the process spawn code:
The src/lib/util/process_spawn.h has this warning about the ProcessSpawn class
```
Only one instance of the ProcessSpawn class may exist
at the given time. Creating additional instance would cause an
attempt to register a new SIGCHLD signal handler and, as a
consequence, the new ProcessSpawn object will fail to create.
```
It seems that at least one system signals an error....
The solution is e.g. to make ProcessSpawn a singleton.
Francis point about signal handlers:
signal handlers can not be writen in C++ (e.g. allocation dead locks) and have a very limited
list of system calls and C standard functions which can be called. So it is not possible to do directly what we want / need in the handler itself. Fortunately this issue is not new: we fixed twice it in Kea. The best solution is to use the boost ASIO signal service (it sends the signal number over a pipe, BTW there are more things available in the handler but very system dependent... the last time I saw a code using this signal context was many years ago) as we currently do in src/lib/process/io_service_signal.{h,cc}. This can mean to reorganize a bit the code i.e. moving files between directory but this service is very convenient and adds a lot of flexibility which greatly help in further steps.
Some details:
- the status must be collected because not doing this makes finished children zombies (the alternative is to explicitly ignore (which is not the default) the SIGCHLD signal and of course this is not compatible)
- it is not required to call wait() in the signal handler, the only requirement is to call it somewhere when a child finished
- the SIGCHLD signal is just a notification and does not give the number of finished children so the status collection uses WNOHANG and loops until there is no more child
- the process state object must be protected against concurrent access: in the current code the only issue is an asynchronous call to the signal handler so the protection is implemented by blocking the signal. Of course this should be replaced by a standard mutex: again moving to the boost ASIO signal service is critical here as mutexes can not be used in a signal handler
Francis clear state point:
clearState() is never called so we have a memory leak. I propose two things:
- add a flag in the state saying that the spawn() caller is not interested by the status so the state entry can be freed by in the wait loop.
- add as a convention that a spawn() caller interested by the status should free the entry
Francis extension proposals:
We need something to handle timeouts in particular for synchronous scripts. I do not know what to send to the shell running the script.
To handle synchronous script from a service thread the thread waits for the script completion. We can use a pipe (aka watch socket) or a condition variable. It seems the second is a bit better because for instance we can use wait_for or wait_until to handle the timeout in the same call. In all cases the process state class should be extended.