From bc695194582468092d00fd1d1c39a2b4c64cf407 Mon Sep 17 00:00:00 2001 From: Peter Portante Date: Mon, 29 Sep 2025 15:56:21 -0400 Subject: [PATCH 1/2] Restore use of `writev()` system call The use of the sequential `write()` system calls to replace `writev()` in commit 29d17be might be simpler to understand, but can lead to unwanted CPU overhead when a high frequency of newlines exists in the data. The original bugfix for the log file corruption was the proper reset of the `iovcnt` on error. We restore the reset but always do it as part of the `writev_buffer_flush()` method which avoids "leaking" knowledge of how it works. Signed-off-by: Peter Portante --- src/ctr_logging.c | 70 ++++++++++++++++++++++++++++++----------------- src/utils.h | 2 -- 2 files changed, 45 insertions(+), 27 deletions(-) diff --git a/src/ctr_logging.c b/src/ctr_logging.c index 56c83b6c..797b469f 100644 --- a/src/ctr_logging.c +++ b/src/ctr_logging.c @@ -77,6 +77,8 @@ static size_t container_tag_len; static char *syslog_identifier = NULL; static size_t syslog_identifier_len; +#define WRITEV_BUFFER_N_IOV 128 + typedef struct { int iovcnt; struct iovec iov[WRITEV_BUFFER_N_IOV]; @@ -551,12 +553,6 @@ static int write_k8s_log(stdpipe_t pipe, const char *buf, ssize_t buflen) if (writev_buffer_flush(k8s_log_fd, &bufv) < 0) { nwarn("failed to flush buffer to log"); } - /* - * Always reset the buffer after rotation to ensure clean state - * with the new file descriptor. Any unflushed data is lost, but - * this prevents corruption of subsequent log entries. - */ - bufv.iovcnt = 0; reopen_k8s_file(); } @@ -629,29 +625,53 @@ static bool get_line_len(ptrdiff_t *line_len, const char *buf, ssize_t buflen) static ssize_t writev_buffer_flush(int fd, writev_buffer_t *buf) { - size_t count = 0; - - for (int i = 0; i < buf->iovcnt; i++) { - const char *ptr = buf->iov[i].iov_base; - size_t remaining = buf->iov[i].iov_len; - - while (remaining > 0) { - ssize_t written = write(fd, ptr, remaining); - if (written < 0) { - if (errno == EINTR) - continue; - return -1; - } - if (written == 0) - return -1; + ssize_t count = 0; + int iovcnt = buf->iovcnt; + struct iovec *iov = buf->iov; + + /* + * By definition, flushing the buffers will either be entirely successful, or will fail at some point + * along the way. There is no facility to attempt to retry a writev() system call outside of an EINTR + * errno. Therefore, no matter the outcome, always reset the writev_buffer_t data structure. + */ + buf->iovcnt = 0; - ptr += written; - remaining -= written; - count += written; + while (iovcnt > 0) { + ssize_t res; + do { + res = writev(fd, iov, iovcnt); + } while (res == -1 && errno == EINTR); + + if (res <= 0) { + /* + * Any unflushed data is lost (this would be a good place to add a counter for how many times + * this occurs and another count for how much data is lost). + * + * Note that if writev() returns a 0, this logic considers it an error. + */ + return -1; + } + + count += res; + + while (res > 0) { + size_t iov_len = iov->iov_len; + size_t from_this = MIN((size_t)res, iov_len); + res -= from_this; + iov_len -= from_this; + + if (iov_len == 0) { + iov++; + iovcnt--; + /* continue, res still > 0 */ + } else { + iov->iov_len = iov_len; + iov->iov_base += from_this; + /* break, res is 0 */ + } } } - buf->iovcnt = 0; return count; } diff --git a/src/utils.h b/src/utils.h index 39d3f0aa..00acb9c4 100644 --- a/src/utils.h +++ b/src/utils.h @@ -255,8 +255,6 @@ static inline void hashtable_free_cleanup(GHashTable **tbl) #define _cleanup_gerror_ _cleanup_(gerror_free_cleanup) -#define WRITEV_BUFFER_N_IOV 128 - ssize_t write_all(int fd, const void *buf, size_t count); int set_subreaper(gboolean enabled); From 4bb9dab56e8e7ce82b9b4f623c7ba76b030ecd1f Mon Sep 17 00:00:00 2001 From: Peter Portante Date: Mon, 6 Oct 2025 22:25:38 -0400 Subject: [PATCH 2/2] Document testing package requirements Adding `systemd-devel` for Fedora derivatives. Signed-off-by: Peter Portante --- README.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/README.md b/README.md index d0bab9c6..16771a8b 100644 --- a/README.md +++ b/README.md @@ -45,6 +45,7 @@ sudo yum install -y \ glib2-devel \ glibc-devel \ libseccomp-devel \ + systemd-devel \ make \ pkgconfig \ runc @@ -87,6 +88,17 @@ Note, to run conmon, you'll also need to have an OCI compliant runtime installed, like [runc](https://github.com/opencontainers/runc) or [crun](https://github.com/containers/crun). +## Testing + +Once you have successfully built `conmon`, run the tests using: + +``` shell +make test +``` + +Note that you'll also need the `bats` and `socat` packages install if not +present. + ## Static build It is possible to build a statically linked binary of conmon by using