Although it's unlikely it's possible that passt-repair could encounter an
error on some but not all the fds in a batch it is given. At present,
passt-repair will die in this situation, meaning that passt knows something
went wrong, but doesn't know the state of the fds it passed in the last
batch.
Change the passt-repair protocol, so that instead of replying with a copy
of the command byte, it replies with the number of fds that were
successfuly handled. We always process the fds in the order given, and
bail out on the first error, so this is sufficient to tell passt the state
of all the fds it passed.
For now we don't use that extra information in passt, we just update it to
understand the new protocol.
Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au>
---
passt-repair.c | 29 +++++++++++++++++++++++------
repair.c | 11 +++++++----
2 files changed, 30 insertions(+), 10 deletions(-)
diff --git a/passt-repair.c b/passt-repair.c
index 64b926bd..f35e74ba 100644
--- a/passt-repair.c
+++ b/passt-repair.c
@@ -16,6 +16,7 @@
* off. Reply by echoing the command. Exit on EOF.
*/
+#include <assert.h>
#include <sys/prctl.h>
#include <sys/types.h>
#include <sys/socket.h>
@@ -38,6 +39,7 @@
#include "seccomp_repair.h"
#define SCM_MAX_FD 253 /* From Linux kernel (include/net/scm.h), not in UAPI */
+static_assert(SCM_MAX_FD < UCHAR_MAX, "Batch sizes must fit in 8 bits");
#define die(status, ...) \
do { \
@@ -77,6 +79,7 @@ int main(int argc, char **argv)
struct cmsghdr *cmsg;
struct msghdr msg;
struct iovec iov;
+ uint8_t reply;
size_t fdlen;
int op;
@@ -140,16 +143,30 @@ loop:
for (i = 0; i < n; i++) {
if (setsockopt(fds[i], SOL_TCP, TCP_REPAIR, &op, sizeof(op))) {
- die_errno("Setting TCP_REPAIR to %i on socket %i",
- op, fds[i]);
+ fprintf(stderr,
+ "Setting TCP_REPAIR to %i on socket %i: %d\n",
+ op, fds[i], errno);
+ break;
}
+ }
+
+ if (i < n)
+ fprintf(stderr, "Failed to handle %d/%d sockets", n - i, n);
+
+ reply = i;
- /* Close _our_ copy */
- close(fds[i]);
+ /* Close all _our_, even if we failed to setsockopt() on some */
+ for (i = 0; i < n; i++) {
+ if (close(fds[i]))
+ /* This is unlikely, but would be painful to debug if it
+ * ever did happen and we didn't report it.
+ */
+ die_errno("Couldn't close socket %i", fds[i]);
}
- /* Confirm setting by echoing the command back */
- if (send(s, &cmd, sizeof(cmd), 0) < 0)
+
+ /* Confirm by sending number of fds succesfully handled back */
+ if (send(s, &reply, sizeof(reply), 0) < 0)
die_errno("Reply to %i", op);
goto loop;
diff --git a/repair.c b/repair.c
index 3ee089f0..343faa22 100644
--- a/repair.c
+++ b/repair.c
@@ -34,7 +34,7 @@ static int repair_fds[SCM_MAX_FD];
static int8_t repair_cmd;
/* Number of pending file descriptors set in @repair_fds */
-static int repair_nfds;
+static unsigned repair_nfds;
/**
* repair_sock_init() - Start listening for connections on helper socket
@@ -151,7 +151,8 @@ int repair_flush(struct ctx *c)
struct iovec iov = { &repair_cmd, sizeof(repair_cmd) };
struct cmsghdr *cmsg;
struct msghdr msg;
- int8_t reply;
+ unsigned nfds;
+ uint8_t reply;
if (!repair_nfds)
return 0;
@@ -169,6 +170,7 @@ int repair_flush(struct ctx *c)
cmsg->cmsg_len = CMSG_LEN(sizeof(int) * repair_nfds);
memcpy(CMSG_DATA(cmsg), repair_fds, sizeof(int) * repair_nfds);
+ nfds = repair_nfds;
repair_nfds = 0;
if (sendmsg(c->fd_repair, &msg, 0) < 0) {
@@ -185,8 +187,9 @@ int repair_flush(struct ctx *c)
return ret;
}
- if (reply != repair_cmd) {
- err("Unexpected reply from TCP_REPAIR helper: %d", reply);
+ if (reply != nfds) {
+ err("TCP_REPAIR helper was unable to process %d fds",
+ nfds - reply);
repair_close(c);
return -ENXIO;
}
--
2.48.1