Improve iptables timeout behaviour.
1. Increase the default timeout from 1s to 5s. This is necessary
for as long as our version of iptables sleeps for 1 second at
a time while the iptables lock is contended.
2. When a timeout occurs, kill the process to ensure that if it
recovers, any output is not returned to subsequent commands.
Add corresponding unit tests.
While I'm at it:
- Ensure that iptables commands that take an output string clear
the output string before appending to it. Otherwise, callers
that passed the same output string object to two separate
iptables commands would think the second command returned both
outputs. This does not affect any existing callers.
- Delete some unused code.
Bug: 35634318
Test: netd_{unit,integration}_test pass
Change-Id: Ife3dfd328ea82f2e93fb903fcf3660a13078b7b5
diff --git a/server/IptablesRestoreController.cpp b/server/IptablesRestoreController.cpp
index 9eb023b..406983a 100644
--- a/server/IptablesRestoreController.cpp
+++ b/server/IptablesRestoreController.cpp
@@ -34,10 +34,9 @@
constexpr size_t PING_SIZE = sizeof(PING) - 1;
-// TODO: This mirrors &gCtls.iptablesRestoreCtrl in production and is duplicated
-// here to aid testing. It allows us to unit-test IptablesRestoreController without
-// needing to construct a fully fledged Controllers object.
-/* static */ IptablesRestoreController* sInstance = nullptr;
+// Not compile-time constants because they are changed by the unit tests.
+int IptablesRestoreController::MAX_RETRIES = 50;
+int IptablesRestoreController::POLL_TIMEOUT_MS = 100;
class IptablesProcess {
public:
@@ -266,14 +265,6 @@
process->errBuf.clear();
}
-// The maximum number of times we poll(2) for a response on our set of polled
-// fds. Chosen so that the overall timeout is 1s.
-static constexpr int MAX_RETRIES = 10;
-
-// The timeout (in millis) for each call to poll. The maximum wait is
-// |POLL_TIMEOUT_MS * MAX_RETRIES|. Chosen so that the overall timeout is 1s.
-static constexpr int POLL_TIMEOUT_MS = 100;
-
/* static */
bool IptablesRestoreController::drainAndWaitForAck(const std::unique_ptr<IptablesProcess> &process,
const std::string& command,
@@ -341,6 +332,9 @@
if (!receivedAck && !process->processTerminated) {
ALOGE("Timed out waiting for response from iptables process %d", process->pid);
+ // Kill the process so that if it eventually recovers, we don't misinterpret the ping
+ // response (or any output) of the command we just sent as coming from future commands.
+ process->stop();
}
maybeLogStderr(process, command);
@@ -355,6 +349,8 @@
std::string buffer;
if (output == nullptr) {
output = &buffer;
+ } else {
+ output->clear();
}
int res = 0;