netinet6: Add ip6_hdr_pseudo{} to ip6.h; diff reduce and apply.
Needs ReviewPublic
Actions

Authored by bms on Mar 4 2026, 11:11 PM.

Details

Reviewers

glebius
imp
pouria

Group Reviewers

network

Summary

Stop rolling our own ip6_hdr_pseudo{}. This actually lives in <netinet/ip6.h>.
Use it in netipsec/xform_tcp.c, with a C99 block-scope designated-initializer.
There are probably other refactoring candidates.
The CTASSERT()s were obtained from NetBSD and added to ip6_input.c.

Prerequisite for merging Andre Oppermann's (unfinished) TCP-AO implementation.

Obtained from: NetBSD, OpenBSD

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 71189
Build 68072: arc lint + arc unit

Event Timeline

bms created this revision.Mar 4 2026, 11:11 PM

Herald added subscribers: melifaro, ae. · View Herald TranscriptMar 4 2026, 11:11 PM

bms requested review of this revision.Mar 4 2026, 11:11 PM

Harbormaster completed remote builds in B71189: Diff 173159.Mar 4 2026, 11:11 PM

https://sourcegraph.com/r/github.com/NetBSD/src@fa6492c255d21e5cf54f55623aa7146c0652e1dd/-/blob/sys/netinet/ip6.h?L121:8-121:22
https://sourcegraph.com/r/github.com/openbsd/src@773346f9ab1e2d6d0bef5b5f2019c0df58d4ed68/-/blob/sys/netinet/ip6.h?L116:8-116:22
https://github.com/freebsd/freebsd-legacy/commit/3e306683756b8ad0f110d8c3318a914ccd9d9006

bz edited reviewers, added: network; removed: bz.Mar 4 2026, 11:36 PM

Alternatively the entire function can be rewritten in declarative manner:

	const struct ip6_hdr *ip6 = mtod(m, struct ip6_hdr *);
	struct ip6_hdr_pseudo ip6ph = {
		.ip6ph_src = ip6->ip6_src,
		.ip6ph_dst = ip6->ip6_dst,
		.ip6ph_len = htonl(m->m_pkthdr.len - (ip6->ip6_nxt == IPPROTO_UDP ? sizeof(struct ip6) + sizeof(struct udphdr) : sizeof(struct ip6)),
		.ip6ph_nxt = htonl(IPPROTO_TCP),
	};

	MD5Update(ctx, &ip6ph, sizeof(ip6ph));

	return (ntohl(ip6ph.ip6ph_len));
}

I am not strongly suggesting that, up to your taste. The feature of such implementation, is that no extra zeroing before assignment is performed.

sys/netinet/ip6.h
113–116	If it is nonstandard and kernel only, let's just put into netinet6/ip6_var.h and no need for XXX :)
120–122	Please use C99 `uintXX_t` types instead of 8-ish BSD `u_intXX_t` in any new code.
sys/netinet6/ip6_input.c
129–133	We no longer use CTASSERT() in new code. It was used back before we were supporting old compilers. Now we use the standard C11 keyword. Question: do we want to assert it equal to the magic number 40 or do we want to assert two structures are of equal size?
sys/netipsec/xform_tcp.c
159–161	This declaration zeroes the entire struct. If this was the intent, then you need just `struct ip6_hdr_pseudo ip6ph = {}`.
174	You lost `htonl()` around IPPROTO_TCP. Is this intentional?
175	The `char *` cast is not needed at all.

bms added inline comments.Mar 5 2026, 2:21 AM

sys/netinet/ip6.h
113–116	See below.
120–122	The intent was diff reduction with other BSDs (and to facilitate porting Andre's existing code, which requires a whole bunch of sys/crypto additions downstreamed from OpenBSD). If you are stipulating that C99 types are now required, that is likely to affect future such merges. I've been away for nearly a decade, so I don't know what the consensus is here.
sys/netinet6/ip6_input.c
129–133	I assume you mean it needs to be _Static_assert() instead then. Again, new to many C99 constructs. Yes, we are checking that the struct was not packed or aligned incorrectly.
sys/netipsec/xform_tcp.c
159–161	I'm new to some of the C99 constructs so I will have to take your word for that.
174	Look again -- IPv6 next header fields are 8 bits. htonl() was an artefact of the use of a 32-bit wide field in the previous hand-rolled ip6 pseudo header.
175	That was already there, diff is non-semantic so it's picking up on my change of local struct name.

glebius added inline comments.Mar 5 2026, 2:46 AM

sys/netinet/ip6.h
120–122	IMHO, after 30 years of independent development, there is little sense left to keep diffs to other BSDs minimal. The code bases diverged a lot! The automatic patch application from OpenBSD was already not possible 15 years ago, when I was working on pf. Even if you are lucky and a patch applied, very high chance it will not work as intended. Only thoughtful merging is possible. Of course, we should not create diffs just to create diffs. But if on one side of scale is reducing standard header pollution and on the other side is reducing diff to OpenBSD, the first outweighs the latter. Same for using C99 types instead of non-standard types. This is my IMHO, but to my knowledge it is aligned with most active developers today. Feel free to bring more reviewers to discuss this point in general.
sys/netipsec/xform_tcp.c
174	Look again -- IPv6 next header fields are 8 bits. htonl() was an artefact of the use of a 32-bit wide field in the previous hand-rolled ip6 pseudo header. Indeed, thanks!
175	That was already there, diff is non-semantic so it's picking up on my change of local struct name. Since you are changing this line, please improve it and remove the unneeded cast.