On Thu, 19 Feb 2026 13:44:54 -0500 Peter Foley wrote:

Support build systems like bazel that check that headers are self-contained.

Signed-off-by: Peter Foley

Thanks for the patch, Peter! It looks obviously correct to me, but I still have a question: do you happen to have a Bazel BUILD file somewhere (we could also add it to passt's contrib/ if it's not in any other repository) to check future changes against it? Otherwise we risk breaking Bazel builds again soon. I'll review and apply within a couple of days regardless of that. -- Stefano

On Mon, 23 Feb 2026 16:33:32 +1100 David Gibson wrote:

On Thu, Feb 19, 2026 at 01:44:54PM -0500, Peter Foley wrote:

...

Support build systems like bazel that check that headers are self-contained.

Signed-off-by: Peter Foley

There are kind of two schools of thoughts on headers. One is that every header should #include anything it relies on. The other is that headers should #include nothing, and .c files should includde everything they need in the right order. The advantages of the second approach are that it makes it easier to keep #includes in .c files minimal, and makes circular dependencies more obvious and easier to dientanble.

We've kinda sorta been using approach two in passt, but not entirely, and honestly, it's not really working.

I would argue it *is* pretty much working, because it builds without warnings against glibc and musl, with several versions of gcc and Clang, on a large number of distributions and architectures, which is what it needs to do. There are currently two warnings with (unreleased) gcc 16-ish and glibc, I still have to post patches for them, but they have nothing to do with includes. That being said, sure, it's not either approach and admittedly kind of arbitrary and rather messy.

So I'm happy to convert to the former approach. However, if we're adding #includes in the headers so they're self contained, then we should be able to also *remove* a bunch of #includes from .c files (and other .h files) which were previously only there to satisfy the indirect dependencies.

Just for clarity, while I agree, this patch does *not* magically make that Peter's job. :) I'd say that making it build with Bazel is more useful at this stage so I would happily accept this patch by itself (I just need to find a moment to try out builds on musl and on a couple of distributions, first). The cleanup you propose can also be done independently at a later point, also because I'm fairly sure there are a bunch of left-over includes (also/mostly from myself) even before this change. Note that this kind of cleanup would also take a bit of testing that we currently can't automate, for example building against musl on Alpine or Void Linux. -- Stefano

On Mon, Feb 23, 2026 at 11:45 AM Peter Foley wrote:

The primary BUILD file is in Google's internal repository, so I can't share that. An OSS bazel version looks like https://github.com/pefoley2/passt/commit/4f89da6f05c84c9f171689541fd81549b48... Unfortunately in my quick testing, the OSS bazel build doesn't actually catch the same layering check violations that Google's internal "Blaze" variant of bazel does. So I'm not sure how helpful it would be.

I poked at this some more, and clang-include-cleaner seems to be able to do a good job of determining whether the headers compile stand-alone. I had to make some more fixes to get there though: https://github.com/pefoley2/passt/commit/5067d86e567851db24dad515cd36b536272... If you want, I can fold the two "include fixing" commits together and re-send.

On Mon, Feb 23, 2026 at 12:35 PM Stefano Brivio wrote:

By the way, we already run clang-tidy tests ('make clang-tidy') as part of our tests (test/build/static_checkers.sh).

Would it be just a matter of enabling the misc-include-cleaner in the list of tests we give clang-tidy from the Makefile:

https://clang.llvm.org/extra/clang-tidy/checks/misc/include-cleaner.html

? I haven't tried.

It appears so, a local change to pass -checks=misc-include-cleaner to clang-tidy resulted in a ton of errors like: /usr/local/google/home/pefoley/passt/vu_common.c:277:2: error: no header providing "iov_from_buf" is directly included [misc-include-cleaner,-warnings-as-errors] 277 | iov_from_buf(in_sg, elem_cnt, VNET_HLEN, buf, total); | ^ /usr/local/google/home/pefoley/passt/vu_common.c:301:15: error: no header providing "ETH_ZLEN" is directly included [misc-include-cleaner,-warnings-as-errors] 9 | if (l2len >= ETH_ZLEN) | ^ /usr/local/google/home/pefoley/passt/vu_common.c:304:2: error: no header providing "memset" is directly included [misc-include-cleaner,-warnings-as-errors] 9 | memset((char *)iov->iov_base + iov->iov_len, 0, ETH_ZLEN - l2len);

...

If you want, I can fold the two "include fixing" commits together and re-send.

Yes, thanks, that would be appreciated. I would first try to settle on a convenient way to keep Blaze/Bazel happy for the future, though.

As I mentioned earlies, Bazel appears to be happy withough the include fixes, it's just Google's internal Blaze varient that has some kind of stricter checking that falls over. I can send the change out for supporting the Bazel build, but I think using misc-include-cleaner would be a better plan.

-- Stefano

On Mon, 23 Feb 2026 13:08:07 -0500 Peter Foley wrote:

On Mon, Feb 23, 2026 at 12:35 PM Stefano Brivio wrote:

...

By the way, we already run clang-tidy tests ('make clang-tidy') as part of our tests (test/build/static_checkers.sh).

Would it be just a matter of enabling the misc-include-cleaner in the list of tests we give clang-tidy from the Makefile:

https://clang.llvm.org/extra/clang-tidy/checks/misc/include-cleaner.html

? I haven't tried.

It appears so, a local change to pass -checks=misc-include-cleaner to clang-tidy resulted in a ton of errors like: /usr/local/google/home/pefoley/passt/vu_common.c:277:2: error: no header providing "iov_from_buf" is directly included

[...]

Hmm, "nice". I guess we should find out if it's reasonable / doable to "fix" all those.

...

...

If you want, I can fold the two "include fixing" commits together and re-send.

Yes, thanks, that would be appreciated. I would first try to settle on a convenient way to keep Blaze/Bazel happy for the future, though.

As I mentioned earlies, Bazel appears to be happy withough the include fixes, it's just Google's internal Blaze varient that has some kind of stricter checking that falls over.

Oh, I didn't understand, I thought Bazel reported a non-empty subset of the failures from Blaze.

I can send the change out for supporting the Bazel build, but I think using misc-include-cleaner would be a better plan.

Yeah, I think so too... as long as it's not hundreds of additional warnings / fixes we need to take care of, it's definitely less effort and should be less clunky too. -- Stefano

On Mon, 23 Feb 2026 15:22:20 -0500 Peter Foley wrote:

On Mon, Feb 23, 2026 at 2:05 PM Stefano Brivio wrote:

...

Hmm, "nice". I guess we should find out if it's reasonable / doable to "fix" all those.

I tried just doing clang-include-cleaner --edit, but it's unfortunately c++ centric, so it added a bunch of stuff like <cstdint> I can try manually fixing that up and seeing what happens.

By the way, I won't have a chance to try this before a couple of days, if needed, but another thought: if we end up adding/changing hundreds of include lines as a result, maybe the cleanup David mentioned would actually be in scope at that point, even from a mere perspective of "noise" we would add, or effort you're spending anyway (let's make it fully worth it I'd say). In any case we could keep a clean clang-include-cleaner output as second step. I think the priorities here should be 1. keep/make things working for everybody while 2. avoiding the risk of recurring fix-ups for future changes and 3. make things pretty/readable/elegant, exactly in this order. If we can tackle all of them together, great, otherwise we can take care of 1. first, and 2. and 3. later. -- Stefano

On Mon, 23 Feb 2026 16:37:54 -0500 Peter Foley wrote:

On Mon, Feb 23, 2026 at 3:47 PM Stefano Brivio wrote:

...

By the way, I won't have a chance to try this before a couple of days, if needed, but another thought: if we end up adding/changing hundreds of include lines as a result, maybe the cleanup David mentioned would actually be in scope at that point, even from a mere perspective of "noise" we would add, or effort you're spending anyway (let's make it fully worth it I'd say).

I tried running clang-include-cleaner and then massaging it to not include c++ only headers or stuff from bits/ That wound up with: 64 files changed, 349 insertions(+), 189 deletions(-)

Unfortunatly include-cleaner isn't smart enough to handle things like #include instead of So absent adding pragma annotations to glibc headers, I'm not sure a clean clang-include-cleaner check is possible.

We could always add suppressions for clang-tidy checks, we already have a bunch, see NOLINTNEXTLINE directives in the code.

That diff is https://github.com/pefoley2/passt/commit/6ae0bcb2bbdc10384346dda547db60f80c8... . I can send it as a proper patch as well if you're interested, but it's a lot of churn and I'm not sure how to prevent back-sliding...

Ouch... yeah. Let me have another look tomorrow (Tuesday) but I think it only makes sense if we use that as a chance to entirely switch to approach #2 David was mentioning (minus perhaps some exceptions), including all the related clean-ups. I'm totally for it by the way, if you can take care of it. Otherwise I'd recommend sticking to a more conservative approach for the moment being. -- Stefano

On Mon, Feb 23, 2026 at 01:08:07PM -0500, Peter Foley wrote:

On Mon, Feb 23, 2026 at 12:35 PM Stefano Brivio wrote:

...

By the way, we already run clang-tidy tests ('make clang-tidy') as part of our tests (test/build/static_checkers.sh).

Would it be just a matter of enabling the misc-include-cleaner in the list of tests we give clang-tidy from the Makefile:

https://clang.llvm.org/extra/clang-tidy/checks/misc/include-cleaner.html

? I haven't tried.

It appears so, a local change to pass -checks=misc-include-cleaner to clang-tidy resulted in a ton of errors like: /usr/local/google/home/pefoley/passt/vu_common.c:277:2: error: no header providing "iov_from_buf" is directly included [misc-include-cleaner,-warnings-as-errors] 277 | iov_from_buf(in_sg, elem_cnt, VNET_HLEN, buf, total); | ^ /usr/local/google/home/pefoley/passt/vu_common.c:301:15: error: no header providing "ETH_ZLEN" is directly included [misc-include-cleaner,-warnings-as-errors] 9 | if (l2len >= ETH_ZLEN) | ^ /usr/local/google/home/pefoley/passt/vu_common.c:304:2: error: no header providing "memset" is directly included [misc-include-cleaner,-warnings-as-errors] 9 | memset((char *)iov->iov_base + iov->iov_len, 0, ETH_ZLEN - l2len);

Unfortunately, I suspect this isn't going to work out. It may do some of the checks we want here, but unfortunately it makes some very silly warnings in relation to the standard library, which is why we disabled it in the first place. See commit 3be9e0010ea7329ae0f3707f67ac4cf0bac13d54 I guess we should retry and see if the latest LLVM is better about this, though.

...

...

If you want, I can fold the two "include fixing" commits together and re-send.

Yes, thanks, that would be appreciated. I would first try to settle on a convenient way to keep Blaze/Bazel happy for the future, though.

As I mentioned earlies, Bazel appears to be happy withough the include fixes, it's just Google's internal Blaze varient that has some kind of stricter checking that falls over. I can send the change out for supporting the Bazel build, but I think using misc-include-cleaner would be a better plan.

...

-- Stefano

-- David Gibson (he or they) | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you, not the other way | around. http://www.ozlabs.org/~dgibson

On Tue, Feb 24, 2026 at 4:33 AM Stefano Brivio wrote:

I guess:

https://github.com/pefoley2/passt/commit/6ae0bcb2bbdc10384346dda547db60f80c8...

makes that mostly pass with a recent LLVM version? If we need a dozen of suppressions (maybe even 20-30?) I would say it's sustainable, but more than that and it will start looking messy.

Ideally glibc would add the necessary annotations to their bits/ headers to make include-cleaner do the right thing, but I'm not sure whether they'd want to do that in the first place. And even if they did, it would take a while for a "fixed" version to roll out. Let alone other libc implementations like musl.

I suppose another track would be to add special-case logic to clang-tidy to not recommend bits headers, but I don't know if the LLVM maintainers would like that either.

On Tue, Feb 24, 2026 at 12:53 PM Stefano Brivio wrote:

What I was suggesting was to sprinkle the code with beauties such as:

/* NOLINTNEXTLINE(misc-include-cleaner) */

Unfortunatly it needs to go before the usage, not the include. e.g. /usr/local/google/home/pefoley/passt/util.c:122:21: error: no header providing "SOL_SOCKET" is directly included [misc-include-cleaner,-warnings-as-errors] 15 | if (setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &y, sizeof(y))) | ^ /usr/local/google/home/pefoley/passt/util.c:122:33: error: no header providing "SO_REUSEADDR" is directly included [misc-include-cleaner,-warnings-as-errors] 122 | if (setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &y, sizeof(y))) | ^ /usr/local/google/home/pefoley/passt/util.c:143:34: error: no header providing "SO_BINDTODEVICE" is directly included [misc-include-cleaner,-warnings-as-errors] 143 | if (setsockopt(fd, SOL_SOCKET, SO_BINDTODEVICE, | ^ /usr/local/google/home/pefoley/passt/util.c:323:32: error: no header providing "SO_SNDBUF" is directly included [misc-include-cleaner,-warnings-as-errors] 323 | if (setsockopt(s, SOL_SOCKET, SO_SNDBUF, &v, sizeof(v)) || | ^ /usr/local/google/home/pefoley/passt/util.c:329:32: error: no header providing "SO_RCVBUF" is directly included [misc-include-cleaner,-warnings-as-errors] 329 | if (setsockopt(s, SOL_SOCKET, SO_RCVBUF, &v, sizeof(v)) || | ^ /usr/local/google/home/pefoley/passt/util.c:855:42: error: no header providing "iovec" is directly included [misc-include-cleaner,-warnings-as-errors] 15 | int write_remainder(int fd, const struct iovec *iov, size_t iovcnt, size_t skip) | ^ /usr/local/google/home/pefoley/passt/util.c:1070:15: error: no header providing "option" is directly included [misc-include-cleaner,-warnings-as-errors] 15 | const struct option optfd[] = { { "fd", required_argument, NULL, 'F' }, | ^ /usr/local/google/home/pefoley/passt/util.c:1070:42: error: no header providing "required_argument" is directly included [misc-include-cleaner,-warnings-as-errors] 1070 | const struct option optfd[] = { { "fd", required_argument, NULL, 'F' }, | ^ /usr/local/google/home/pefoley/passt/util.c:1077:10: error: no header providing "getopt_long" is directly included [misc-include-cleaner,-warnings-as-errors] 1077 | name = getopt_long(argc, argv, "-:F:", optfd, NULL); | ^ /usr/local/google/home/pefoley/passt/util.c:1081:16: error: no header providing "optarg" is directly included [misc-include-cleaner,-warnings-as-errors] 15 | fd = strtol(optarg, NULL, 0); | ^

before each "offending" include line... assuming it works, and assuming we need perhaps 20-30 of them. But if it's a lot more, then that's not a reasonable option either.

I get "72 warnings treated as errors" when running clang-tidy with my current set of patches.

-- Stefano

Ugh. Yet another alternative could be to enable misc-include-cleaner for headers only, which would probably need a separate invocation of clang-tidy.

I'm not sure if that will work at all though. If it doesn't, I'm out of ideas... maybe we should simply go back to your original patch, in that case, and you'll just get to "fix" things as they break (hopefully infrequently) in the future.

I tried that, it didn't go well either: /usr/local/google/home/pefoley/passt/util.h:140:5: error: no header

On Tue, Feb 24, 2026 at 3:03 PM Stefano Brivio wrote: providing "__BYTE_ORDER" is directly included [misc-include-cleaner,-warnings-as-errors] 9 | #if __BYTE_ORDER == __BIG_ENDIAN | ^ /usr/local/google/home/pefoley/passt/util.h:140:21: error: no header providing "__BIG_ENDIAN" is directly included [misc-include-cleaner,-warnings-as-errors] 140 | #if __BYTE_ORDER == __BIG_ENDIAN | ^ /usr/local/google/home/pefoley/passt/util.h:241:42: error: no header providing "iovec" is directly included [misc-include-cleaner,-warnings-as-errors] 9 | int write_remainder(int fd, const struct iovec *iov, size_t iovcnt, size_t skip); | ^ /usr/local/google/home/pefoley/passt/util.h:254:35: error: no header providing "sa_family_t" is directly included [misc-include-cleaner,-warnings-as-errors] 9 | static inline const char *af_name(sa_family_t af) | I'm inclined to forget about trying to fix everything and just do the minimal set of changes to make the tooling happy with parsing the headers.

-- Stefano

On Tue, Feb 24, 2026 at 4:18 PM Stefano Brivio wrote:

...

I'm inclined to forget about trying to fix everything and just do the minimal set of changes to make the tooling happy with parsing the headers.

Yeah, at this point I'd totally support that.

Does the most recent patch I sent look good to apply? That should be the "minimal" include happiness version.

On Mon, 23 Feb 2026 13:11:19 -0500 Peter Foley wrote:

Support build systems like bazel that check that headers are self-contained.

Also update includes so that clang-include-cleaner succeeds.

Tested with: clang-include-cleaner-19 --extra-arg=-D_GNU_SOURCE --extra-arg=-DPAGE_SIZE=4096 --extra-arg=-DVERSION=\"git\" --extra-arg=-DHAS_GETRANDOM *.h *.c

Signed-off-by: Peter Foley

Tested some more (especially with musl), and applied, thanks. At this point, with David's series applied as well, things should build cleanly for you. -- Stefano