Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
freebsd-relayd: Fix problems with HCE/PFE/relay desync after reload
It has been observed that relayd will sometimes exit with an assertion
failure immediately after its config has been reloaded (i.e., SIGHUP was
sent or "relayctl reload" was used). Usually it's because the PFE was
handling a host status update (IMSG_HOST_STATUS) and failed to find the
host in question or the check counts aren't matching.
relayd configuration reloads are implemented by the parent, which
re-reads the configuration file, builds up a new set of data structures
(hosts, tables, redirects, etc.), and sends copies to each worker.
The sequence is:
1. parent -> IMSG_CTL_RESET -> workers, causing workers to free all of
the existing data structures.
2. parent -> IMSG_CFG_{TABLE,HOST,...} -> workers, giving state to
workers.
3. parent -> IMSG_CFG_DONE -> workers, indicating that it's done with
the configuration update.
4. workers -> IMSG_CFG_DONE -> parent, indicating workers are ready to
start.
5. parent -> IMSG_CFG_START -> workers, only once all messages in step 4
have been received by the parent.
The structures sent in step 2 each have a unique ID (modulo 32-bit
object ID rollover), so a host object ID is only valid in one
"configuration epoch". When a reload starts, a new configuration epoch
begins, and all IDs belonging to the previous epoch become invalid.
The basic problem is that there is no global ordering of IPC messages
sent between workers and the parent. Each process in relayd receives
messages from multiple queues, and these can be interleaved or delayed
indefinitely. For instance, IMSG_HOST_STATUS (sent from HCE -> PFE, and
PFE -> relay) refers to a host by ID, but there is nothing ensuring that
the status update is handled in the same epoch in which it was sent.
Such mismatches result in the assertion failures mentioned above.
One solution is simply to remove checks and drop messages if they refer
to non-existent objects. However, this might hide bugs and generally
makes the code harder to reason about. My solution is to make the
notion of configuration epoch explicit in relayd worker state, and to
tag certain messages with the current epoch. Then workers can decide
what to do about mismatches. So:
1. Add a 64-bit epoch counter, incremented in step 1 of the reload
procedure above. That is, each worker has a notion of the current
epoch, incremented when IMSG_CTL_RESET is received.
2. When the HCE schedules host checks, it stamps the host with the
current epoch. When IMSG_HOST_STATUS is set to the PFE and to
relays, the receiver drops the message if the epoch doesn't match.
This ensures that old host status messages are correctly ignored.
3. "script" checks involve a round-trip through the parent process which
isn't cancelled when the HCE receives IMSG_CTL_RESET. Thus, imbue
script state with the current epoch as well.
4. Ensure that the PFE and HCE actually disable events when CTL_RESET is
received. Previously, hce_disable_events() and pfe_disable_events()
weren't getting called at all.
5. Make sure that relayctl can't be used to schedule a check while a
configuration reload is in progress.
This was sufficient to eliminate assertion failures in a stress test
which spams relayd with SIGHUP and relayctl poll commands. I suspect
that my coverage of item 5 above isn't wide enough; that is, other
relayctl commands (e.g., relayctl table disable) might still cause
problems.- Loading branch information
