* [PATCH] Add missing includes to headers
@ 2026-02-19 17:45 Peter Foley
2026-02-21 17:57 ` Stefano Brivio
2026-02-23 5:33 ` David Gibson
0 siblings, 2 replies; 5+ messages in thread
From: Peter Foley @ 2026-02-19 17:45 UTC (permalink / raw)
To: passt-dev; +Cc: Peter Foley
Support build systems like bazel that check that headers are
self-contained.
Signed-off-by: Peter Foley <pefoley@google.com>
---
flow.h | 6 ++++++
flow_table.h | 1 +
icmp_flow.h | 2 ++
inany.h | 5 +++++
ip.h | 2 ++
linux_dep.h | 3 +++
pif.h | 4 ++++
seccomp.sh | 5 +++++
siphash.h | 3 +++
tap.h | 5 +++++
tcp_conn.h | 4 ++++
tcp_internal.h | 5 +++++
udp_internal.h | 3 +++
util.c | 1 +
14 files changed, 49 insertions(+)
diff --git a/flow.h b/flow.h
index d636358..897c9ea 100644
--- a/flow.h
+++ b/flow.h
@@ -7,6 +7,12 @@
#ifndef FLOW_H
#define FLOW_H
+#include <stdint.h>
+#include <netinet/in.h>
+
+#include "inany.h"
+#include "util.h"
+
#define FLOW_TIMER_INTERVAL 1000 /* ms */
/**
diff --git a/flow_table.h b/flow_table.h
index 73de13b..8fb7b5c 100644
--- a/flow_table.h
+++ b/flow_table.h
@@ -7,6 +7,7 @@
#ifndef FLOW_TABLE_H
#define FLOW_TABLE_H
+#include "pif.h"
#include "tcp_conn.h"
#include "icmp_flow.h"
#include "udp_flow.h"
diff --git a/icmp_flow.h b/icmp_flow.h
index fb93801..3af98be 100644
--- a/icmp_flow.h
+++ b/icmp_flow.h
@@ -7,6 +7,8 @@
#ifndef ICMP_FLOW_H
#define ICMP_FLOW_H
+#include "flow.h"
+
/**
* struct icmp_ping_flow - Descriptor for a flow of ping requests/replies
* @f: Generic flow information
diff --git a/inany.h b/inany.h
index b02c289..c7de094 100644
--- a/inany.h
+++ b/inany.h
@@ -9,6 +9,11 @@
#ifndef INANY_H
#define INANY_H
+#include <string.h>
+
+#include "ip.h"
+#include "siphash.h"
+
struct siphash_state;
/** union inany_addr - Represents either an IPv4 or IPv6 address
diff --git a/ip.h b/ip.h
index a8043c2..3be2d4e 100644
--- a/ip.h
+++ b/ip.h
@@ -9,6 +9,8 @@
#include <netinet/ip.h>
#include <netinet/ip6.h>
+#include "util.h"
+
#define IN4_IS_ADDR_UNSPECIFIED(a) \
(((struct in_addr *)(a))->s_addr == htonl_constant(INADDR_ANY))
#define IN4_IS_ADDR_BROADCAST(a) \
diff --git a/linux_dep.h b/linux_dep.h
index 89e590c..3f8184b 100644
--- a/linux_dep.h
+++ b/linux_dep.h
@@ -7,6 +7,9 @@
#ifndef LINUX_DEP_H
#define LINUX_DEP_H
+#include <stdint.h>
+#include <unistd.h>
+
/* struct tcp_info_linux - Information from Linux TCP_INFO getsockopt()
*
* Largely derived from include/linux/tcp.h in the Linux kernel
diff --git a/pif.h b/pif.h
index 0f7f667..7c755bd 100644
--- a/pif.h
+++ b/pif.h
@@ -7,6 +7,10 @@
#ifndef PIF_H
#define PIF_H
+#include <netinet/in.h>
+
+#include "epoll_type.h"
+
union inany_addr;
union sockaddr_inany;
diff --git a/seccomp.sh b/seccomp.sh
index 60ebe84..5347586 100755
--- a/seccomp.sh
+++ b/seccomp.sh
@@ -34,6 +34,11 @@ AUDIT_ARCH="AUDIT_ARCH_$(echo ${ARCH} | tr '[a-z]' '[A-Z]' \
HEADER="/* This file was automatically generated by $(basename ${0}) */
+#include <stddef.h>
+#include <linux/audit.h>
+#include <linux/filter.h>
+#include <linux/seccomp.h>
+
#ifndef AUDIT_ARCH_PPC64LE
#define AUDIT_ARCH_PPC64LE (AUDIT_ARCH_PPC64 | __AUDIT_ARCH_LE)
#endif"
diff --git a/siphash.h b/siphash.h
index e760236..bbddcac 100644
--- a/siphash.h
+++ b/siphash.h
@@ -44,6 +44,9 @@
#ifndef SIPHASH_H
#define SIPHASH_H
+#include <stddef.h>
+#include <stdint.h>
+
/**
* struct siphash_state - Internal state of siphash calculation
*/
diff --git a/tap.h b/tap.h
index cc780d1..07ca096 100644
--- a/tap.h
+++ b/tap.h
@@ -6,6 +6,11 @@
#ifndef TAP_H
#define TAP_H
+#include <stddef.h>
+#include <stdint.h>
+
+#include "passt.h"
+
/** L2_MAX_LEN_PASTA - Maximum frame length for pasta mode (with L2 header)
*
* The kernel tuntap device imposes a maximum frame size of 65535 including
diff --git a/tcp_conn.h b/tcp_conn.h
index 21cea10..d4d0139 100644
--- a/tcp_conn.h
+++ b/tcp_conn.h
@@ -9,6 +9,10 @@
#ifndef TCP_CONN_H
#define TCP_CONN_H
+#include <stdint.h>
+
+#include "flow.h"
+
/**
* struct tcp_tap_conn - Descriptor for a TCP connection (not spliced)
* @f: Generic flow information
diff --git a/tcp_internal.h b/tcp_internal.h
index 518913b..591e58c 100644
--- a/tcp_internal.h
+++ b/tcp_internal.h
@@ -6,6 +6,11 @@
#ifndef TCP_INTERNAL_H
#define TCP_INTERNAL_H
+#include <stdint.h>
+#include <netinet/tcp.h>
+
+#include "util.h"
+
#define MAX_WS 8
#define MAX_WINDOW (1 << (16 + (MAX_WS)))
diff --git a/udp_internal.h b/udp_internal.h
index 0a8fe49..64e4577 100644
--- a/udp_internal.h
+++ b/udp_internal.h
@@ -6,6 +6,9 @@
#ifndef UDP_INTERNAL_H
#define UDP_INTERNAL_H
+#include <netinet/in.h>
+#include <netinet/udp.h>
+
#include "tap.h" /* needed by udp_meta_t */
/**
diff --git a/util.c b/util.c
index a48f727..db27431 100644
--- a/util.c
+++ b/util.c
@@ -25,6 +25,7 @@
#include <errno.h>
#include <stdbool.h>
#include <linux/errqueue.h>
+#include <linux/in6.h>
#include <getopt.h>
#include "linux_dep.h"
--
2.53.0.371.g1d285c8824-goog
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Add missing includes to headers
2026-02-19 17:45 [PATCH] Add missing includes to headers Peter Foley
@ 2026-02-21 17:57 ` Stefano Brivio
[not found] ` <CAAAKUPN=GPDp84tQAv4Kpxs-AzKR44pDkWda-AbXWaUomYN5eg@mail.gmail.com>
2026-02-23 5:33 ` David Gibson
1 sibling, 1 reply; 5+ messages in thread
From: Stefano Brivio @ 2026-02-21 17:57 UTC (permalink / raw)
To: Peter Foley; +Cc: passt-dev
On Thu, 19 Feb 2026 13:44:54 -0500
Peter Foley <pefoley@google.com> wrote:
> Support build systems like bazel that check that headers are
> self-contained.
>
> Signed-off-by: Peter Foley <pefoley@google.com>
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Add missing includes to headers
2026-02-19 17:45 [PATCH] Add missing includes to headers Peter Foley
2026-02-21 17:57 ` Stefano Brivio
@ 2026-02-23 5:33 ` David Gibson
2026-02-23 16:32 ` Stefano Brivio
1 sibling, 1 reply; 5+ messages in thread
From: David Gibson @ 2026-02-23 5:33 UTC (permalink / raw)
To: Peter Foley; +Cc: passt-dev
[-- Attachment #1: Type: text/plain, Size: 6615 bytes --]
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 <pefoley@google.com>
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. 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.
> ---
> flow.h | 6 ++++++
> flow_table.h | 1 +
> icmp_flow.h | 2 ++
> inany.h | 5 +++++
> ip.h | 2 ++
> linux_dep.h | 3 +++
> pif.h | 4 ++++
> seccomp.sh | 5 +++++
> siphash.h | 3 +++
> tap.h | 5 +++++
> tcp_conn.h | 4 ++++
> tcp_internal.h | 5 +++++
> udp_internal.h | 3 +++
> util.c | 1 +
> 14 files changed, 49 insertions(+)
>
> diff --git a/flow.h b/flow.h
> index d636358..897c9ea 100644
> --- a/flow.h
> +++ b/flow.h
> @@ -7,6 +7,12 @@
> #ifndef FLOW_H
> #define FLOW_H
>
> +#include <stdint.h>
> +#include <netinet/in.h>
> +
> +#include "inany.h"
> +#include "util.h"
> +
> #define FLOW_TIMER_INTERVAL 1000 /* ms */
>
> /**
> diff --git a/flow_table.h b/flow_table.h
> index 73de13b..8fb7b5c 100644
> --- a/flow_table.h
> +++ b/flow_table.h
> @@ -7,6 +7,7 @@
> #ifndef FLOW_TABLE_H
> #define FLOW_TABLE_H
>
> +#include "pif.h"
> #include "tcp_conn.h"
> #include "icmp_flow.h"
> #include "udp_flow.h"
> diff --git a/icmp_flow.h b/icmp_flow.h
> index fb93801..3af98be 100644
> --- a/icmp_flow.h
> +++ b/icmp_flow.h
> @@ -7,6 +7,8 @@
> #ifndef ICMP_FLOW_H
> #define ICMP_FLOW_H
>
> +#include "flow.h"
> +
> /**
> * struct icmp_ping_flow - Descriptor for a flow of ping requests/replies
> * @f: Generic flow information
> diff --git a/inany.h b/inany.h
> index b02c289..c7de094 100644
> --- a/inany.h
> +++ b/inany.h
> @@ -9,6 +9,11 @@
> #ifndef INANY_H
> #define INANY_H
>
> +#include <string.h>
> +
> +#include "ip.h"
> +#include "siphash.h"
> +
> struct siphash_state;
>
> /** union inany_addr - Represents either an IPv4 or IPv6 address
> diff --git a/ip.h b/ip.h
> index a8043c2..3be2d4e 100644
> --- a/ip.h
> +++ b/ip.h
> @@ -9,6 +9,8 @@
> #include <netinet/ip.h>
> #include <netinet/ip6.h>
>
> +#include "util.h"
> +
> #define IN4_IS_ADDR_UNSPECIFIED(a) \
> (((struct in_addr *)(a))->s_addr == htonl_constant(INADDR_ANY))
> #define IN4_IS_ADDR_BROADCAST(a) \
> diff --git a/linux_dep.h b/linux_dep.h
> index 89e590c..3f8184b 100644
> --- a/linux_dep.h
> +++ b/linux_dep.h
> @@ -7,6 +7,9 @@
> #ifndef LINUX_DEP_H
> #define LINUX_DEP_H
>
> +#include <stdint.h>
> +#include <unistd.h>
> +
> /* struct tcp_info_linux - Information from Linux TCP_INFO getsockopt()
> *
> * Largely derived from include/linux/tcp.h in the Linux kernel
> diff --git a/pif.h b/pif.h
> index 0f7f667..7c755bd 100644
> --- a/pif.h
> +++ b/pif.h
> @@ -7,6 +7,10 @@
> #ifndef PIF_H
> #define PIF_H
>
> +#include <netinet/in.h>
> +
> +#include "epoll_type.h"
> +
> union inany_addr;
> union sockaddr_inany;
>
> diff --git a/seccomp.sh b/seccomp.sh
> index 60ebe84..5347586 100755
> --- a/seccomp.sh
> +++ b/seccomp.sh
> @@ -34,6 +34,11 @@ AUDIT_ARCH="AUDIT_ARCH_$(echo ${ARCH} | tr '[a-z]' '[A-Z]' \
>
> HEADER="/* This file was automatically generated by $(basename ${0}) */
>
> +#include <stddef.h>
> +#include <linux/audit.h>
> +#include <linux/filter.h>
> +#include <linux/seccomp.h>
> +
> #ifndef AUDIT_ARCH_PPC64LE
> #define AUDIT_ARCH_PPC64LE (AUDIT_ARCH_PPC64 | __AUDIT_ARCH_LE)
> #endif"
> diff --git a/siphash.h b/siphash.h
> index e760236..bbddcac 100644
> --- a/siphash.h
> +++ b/siphash.h
> @@ -44,6 +44,9 @@
> #ifndef SIPHASH_H
> #define SIPHASH_H
>
> +#include <stddef.h>
> +#include <stdint.h>
> +
> /**
> * struct siphash_state - Internal state of siphash calculation
> */
> diff --git a/tap.h b/tap.h
> index cc780d1..07ca096 100644
> --- a/tap.h
> +++ b/tap.h
> @@ -6,6 +6,11 @@
> #ifndef TAP_H
> #define TAP_H
>
> +#include <stddef.h>
> +#include <stdint.h>
> +
> +#include "passt.h"
> +
> /** L2_MAX_LEN_PASTA - Maximum frame length for pasta mode (with L2 header)
> *
> * The kernel tuntap device imposes a maximum frame size of 65535 including
> diff --git a/tcp_conn.h b/tcp_conn.h
> index 21cea10..d4d0139 100644
> --- a/tcp_conn.h
> +++ b/tcp_conn.h
> @@ -9,6 +9,10 @@
> #ifndef TCP_CONN_H
> #define TCP_CONN_H
>
> +#include <stdint.h>
> +
> +#include "flow.h"
> +
> /**
> * struct tcp_tap_conn - Descriptor for a TCP connection (not spliced)
> * @f: Generic flow information
> diff --git a/tcp_internal.h b/tcp_internal.h
> index 518913b..591e58c 100644
> --- a/tcp_internal.h
> +++ b/tcp_internal.h
> @@ -6,6 +6,11 @@
> #ifndef TCP_INTERNAL_H
> #define TCP_INTERNAL_H
>
> +#include <stdint.h>
> +#include <netinet/tcp.h>
> +
> +#include "util.h"
> +
> #define MAX_WS 8
> #define MAX_WINDOW (1 << (16 + (MAX_WS)))
>
> diff --git a/udp_internal.h b/udp_internal.h
> index 0a8fe49..64e4577 100644
> --- a/udp_internal.h
> +++ b/udp_internal.h
> @@ -6,6 +6,9 @@
> #ifndef UDP_INTERNAL_H
> #define UDP_INTERNAL_H
>
> +#include <netinet/in.h>
> +#include <netinet/udp.h>
> +
> #include "tap.h" /* needed by udp_meta_t */
>
> /**
> diff --git a/util.c b/util.c
> index a48f727..db27431 100644
> --- a/util.c
> +++ b/util.c
> @@ -25,6 +25,7 @@
> #include <errno.h>
> #include <stdbool.h>
> #include <linux/errqueue.h>
> +#include <linux/in6.h>
> #include <getopt.h>
>
> #include "linux_dep.h"
> --
> 2.53.0.371.g1d285c8824-goog
>
--
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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Add missing includes to headers
2026-02-23 5:33 ` David Gibson
@ 2026-02-23 16:32 ` Stefano Brivio
0 siblings, 0 replies; 5+ messages in thread
From: Stefano Brivio @ 2026-02-23 16:32 UTC (permalink / raw)
To: David Gibson; +Cc: Peter Foley, passt-dev
On Mon, 23 Feb 2026 16:33:32 +1100
David Gibson <david@gibson.dropbear.id.au> 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 <pefoley@google.com>
>
> 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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Add missing includes to headers
[not found] ` <CAAAKUPMP8goRHqk0VDn6UDWmyyPXpzs37UuCLL8x7wDp10tY6A@mail.gmail.com>
@ 2026-02-23 17:35 ` Stefano Brivio
0 siblings, 0 replies; 5+ messages in thread
From: Stefano Brivio @ 2026-02-23 17:35 UTC (permalink / raw)
To: Peter Foley; +Cc: passt-dev
On Mon, 23 Feb 2026 12:23:08 -0500
Peter Foley <pefoley@google.com> wrote:
> On Mon, Feb 23, 2026 at 11:45 AM Peter Foley <pefoley@google.com> 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/4f89da6f05c84c9f171689541fd81549b4801270
> > 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/5067d86e567851db24dad515cd36b53627266ba6
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.
> 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.
--
Stefano
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-02-23 17:35 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-02-19 17:45 [PATCH] Add missing includes to headers Peter Foley
2026-02-21 17:57 ` Stefano Brivio
[not found] ` <CAAAKUPN=GPDp84tQAv4Kpxs-AzKR44pDkWda-AbXWaUomYN5eg@mail.gmail.com>
[not found] ` <CAAAKUPMP8goRHqk0VDn6UDWmyyPXpzs37UuCLL8x7wDp10tY6A@mail.gmail.com>
2026-02-23 17:35 ` Stefano Brivio
2026-02-23 5:33 ` David Gibson
2026-02-23 16:32 ` Stefano Brivio
Code repositories for project(s) associated with this public inbox
https://passt.top/passt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for IMAP folder(s).