Download raw body.
IP6_EXTHDR_GET pass mbuf pointer
On Thu, May 22, 2025 at 05:23:48PM +0200, Claudio Jeker wrote:
> On Fri, May 23, 2025 at 12:07:46AM +0900, Alexander Bluhm wrote:
> > On Thu, May 22, 2025 at 04:25:57PM +0200, Claudio Jeker wrote:
> > > Would it make sense to actually kill this macro and make it a proper
> > > function? This thing is a monster and it probably makes things slower if
> > > anything.
> >
> > I was considering this. Problem is that typ is passed. Then the
> > return pointer is casted accordingly. Maybe we get can get away
> > with void pointer and the compiler doing its job.
>
> (val) = (typ)(mtod((*(mp)), caddr_t) + (off)); \
> (val) = (typ)NULL; \
>
> Urgh, these casts are just horrible. I think using a void * as return
> works equally well.
>
> > static inline void *ip6_exthdr_get(struct mbuf **mp, int off, int len);
> > th = ip6_exthdr_get(mp, iphlen, sizeof(*th));
>
> Please use a size_t len argument. Also I'm not sure if you gain anything
> from static inline. Would be an interesting experiment.
Here is the ip6_exthdr_get() function diff.
I make it static inline in netinet/ip6.h, mainly because we have
no proper C file for it. There is no ip6.c. And it has to be in
netinet as it is used by TCP and UDP in kernel without IPv6. Then
the namespace ip6_ would be inconsistent. Keeping it in in6.h
avoids all these naming problems. And the inline function is not
very long.
I moved the panic("m_pulldown malfunction") as kassert into
m_pulldown(). Then it is only implemented once. The code is
complicated enough that checking makes some sense.
Regarding the type of len, I don't know. A lot of sizes in the
network stack are passed around as signed int. At least that avoids
type confusion when comparing them against each other. len is
passed to m_pulldown() which also takes an int. The callers of
ip6_exthdr_get() pass int or size_t to off or len inconsistently.
I think it is best to keep everthing int as it is.
I have also wrapped an ugly a long line in the diff's context.
ok?
bluhm
Index: kern/uipc_mbuf2.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/kern/uipc_mbuf2.c,v
diff -u -p -r1.47 uipc_mbuf2.c
--- kern/uipc_mbuf2.c 29 Aug 2024 16:42:30 -0000 1.47
+++ kern/uipc_mbuf2.c 24 May 2025 14:02:05 -0000
@@ -213,6 +213,7 @@ m_pulldown(struct mbuf *m, int off, int
off = 0;
ok:
+ KASSERT(n->m_len >= off + len);
if (offp)
*offp = off;
return (n);
Index: netinet/ip6.h
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip6.h,v
diff -u -p -r1.22 ip6.h
--- netinet/ip6.h 24 May 2025 12:27:23 -0000 1.22
+++ netinet/ip6.h 24 May 2025 14:21:57 -0000
@@ -267,28 +267,27 @@ struct ip6_frag {
#ifdef _KERNEL
/*
- * IP6_EXTHDR_GET ensures that intermediate protocol header (from "off" to
- * "len") is located in single mbuf, on contiguous memory region.
+ * ip6_exthdr_get() ensures that intermediate protocol header (from "off"
+ * to "len") is located in single mbuf, on contiguous memory region.
* The pointer to the region will be returned to pointer variable "val",
* with type "typ".
*/
-#define IP6_EXTHDR_GET(val, typ, mp, off, len) \
-do { \
- struct mbuf *t; \
- int tmp; \
- if ((*(mp))->m_len >= (off) + (len)) \
- (val) = (typ)(mtod((*(mp)), caddr_t) + (off)); \
- else { \
- t = m_pulldown((*(mp)), (off), (len), &tmp); \
- if (t) { \
- if (t->m_len < tmp + (len)) \
- panic("m_pulldown malfunction"); \
- (val) = (typ)(mtod(t, caddr_t) + tmp); \
- } else { \
- (val) = (typ)NULL; \
- (*(mp)) = NULL; \
- } \
- } \
-} while (/* CONSTCOND */ 0)
+
+static inline void *
+ip6_exthdr_get(struct mbuf **mp, int off, int len)
+{
+ struct mbuf *t;
+ int toff;
+
+ if ((*mp)->m_len >= off + len)
+ return (mtod(*mp, caddr_t) + off);
+
+ t = m_pulldown(*mp, off, len, &toff);
+ if (t == NULL) {
+ *mp = NULL;
+ return (NULL);
+ }
+ return (mtod(t, caddr_t) + toff);
+}
#endif /* _KERNEL */
#endif /* _NETINET_IP6_H_ */
Index: netinet/tcp_input.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/tcp_input.c,v
diff -u -p -r1.448 tcp_input.c
--- netinet/tcp_input.c 24 May 2025 12:27:23 -0000 1.448
+++ netinet/tcp_input.c 24 May 2025 14:08:47 -0000
@@ -448,8 +448,8 @@ tcp_input_solocked(struct mbuf **mp, int
* Get IP and TCP header together in first mbuf.
* Note: IP leaves IP header in first mbuf.
*/
- IP6_EXTHDR_GET(th, struct tcphdr *, mp, iphlen, sizeof(*th));
- if (!th) {
+ th = ip6_exthdr_get(mp, iphlen, sizeof(*th));
+ if (th == NULL) {
tcpstat_inc(tcps_rcvshort);
return IPPROTO_DONE;
}
@@ -532,8 +532,8 @@ tcp_input_solocked(struct mbuf **mp, int
}
tlen -= off;
if (off > sizeof(struct tcphdr)) {
- IP6_EXTHDR_GET(th, struct tcphdr *, mp, iphlen, off);
- if (!th) {
+ th = ip6_exthdr_get(mp, iphlen, off);
+ if (th == NULL) {
tcpstat_inc(tcps_rcvshort);
return IPPROTO_DONE;
}
Index: netinet/udp_usrreq.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/udp_usrreq.c,v
diff -u -p -r1.338 udp_usrreq.c
--- netinet/udp_usrreq.c 24 May 2025 12:27:23 -0000 1.338
+++ netinet/udp_usrreq.c 24 May 2025 14:09:18 -0000
@@ -213,8 +213,8 @@ udp_input(struct mbuf **mp, int *offp, i
udpstat_inc(udps_ipackets);
- IP6_EXTHDR_GET(uh, struct udphdr *, mp, iphlen, sizeof(struct udphdr));
- if (!uh) {
+ uh = ip6_exthdr_get(mp, iphlen, sizeof(struct udphdr));
+ if (uh == NULL) {
udpstat_inc(udps_hdrops);
return IPPROTO_DONE;
}
Index: netinet6/dest6.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/dest6.c,v
diff -u -p -r1.22 dest6.c
--- netinet6/dest6.c 24 May 2025 12:27:23 -0000 1.22
+++ netinet6/dest6.c 24 May 2025 14:10:09 -0000
@@ -57,12 +57,12 @@ dest6_input(struct mbuf **mp, int *offp,
u_int8_t *opt;
/* validation of the length of the header */
- IP6_EXTHDR_GET(dstopts, struct ip6_dest *, mp, off, sizeof(*dstopts));
+ dstopts = ip6_exthdr_get(mp, off, sizeof(*dstopts));
if (dstopts == NULL)
return IPPROTO_DONE;
dstoptlen = (dstopts->ip6d_len + 1) << 3;
- IP6_EXTHDR_GET(dstopts, struct ip6_dest *, mp, off, dstoptlen);
+ dstopts = ip6_exthdr_get(mp, off, dstoptlen);
if (dstopts == NULL)
return IPPROTO_DONE;
off += dstoptlen;
Index: netinet6/frag6.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/frag6.c,v
diff -u -p -r1.91 frag6.c
--- netinet6/frag6.c 24 May 2025 12:27:23 -0000 1.91
+++ netinet6/frag6.c 24 May 2025 14:15:34 -0000
@@ -125,7 +125,7 @@ frag6_input(struct mbuf **mp, int *offp,
u_int8_t ecn, ecn0;
ip6 = mtod(*mp, struct ip6_hdr *);
- IP6_EXTHDR_GET(ip6f, struct ip6_frag *, mp, offset, sizeof(*ip6f));
+ ip6f = ip6_exthdr_get(mp, offset, sizeof(*ip6f));
if (ip6f == NULL)
return IPPROTO_DONE;
@@ -451,8 +451,7 @@ frag6_input(struct mbuf **mp, int *offp,
int prvnxt = ip6_get_prevhdr(*mp, offset);
uint8_t *prvnxtp;
- IP6_EXTHDR_GET(prvnxtp, uint8_t *, mp, prvnxt,
- sizeof(*prvnxtp));
+ prvnxtp = ip6_exthdr_get(mp, prvnxt, sizeof(*prvnxtp));
if (prvnxtp == NULL)
goto dropfrag;
*prvnxtp = nxt;
Index: netinet6/icmp6.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/icmp6.c,v
diff -u -p -r1.265 icmp6.c
--- netinet6/icmp6.c 24 May 2025 12:27:23 -0000 1.265
+++ netinet6/icmp6.c 24 May 2025 14:14:28 -0000
@@ -281,8 +281,7 @@ icmp6_do_error(struct mbuf *m, int type,
if (off >= 0 && nxt == IPPROTO_ICMPV6) {
struct icmp6_hdr *icp;
- IP6_EXTHDR_GET(icp, struct icmp6_hdr *, &m, off,
- sizeof(*icp));
+ icp = ip6_exthdr_get(&m, off, sizeof(*icp));
if (icp == NULL) {
icmp6stat_inc(icp6s_tooshort);
return (NULL);
@@ -407,7 +406,7 @@ icmp6_input(struct mbuf **mp, int *offp,
/*
* calculate the checksum
*/
- IP6_EXTHDR_GET(icmp6, struct icmp6_hdr *, mp, off, sizeof(*icmp6));
+ icmp6 = ip6_exthdr_get(mp, off, sizeof(*icmp6));
if (icmp6 == NULL) {
icmp6stat_inc(icp6s_tooshort);
return IPPROTO_DONE;
@@ -588,8 +587,7 @@ icmp6_input(struct mbuf **mp, int *offp,
n->m_next = n0;
} else {
deliverecho:
- IP6_EXTHDR_GET(nicmp6, struct icmp6_hdr *, &n, off,
- sizeof(*nicmp6));
+ nicmp6 = ip6_exthdr_get(&n, off, sizeof(*nicmp6));
noff = off;
}
if (n) {
@@ -762,8 +760,8 @@ icmp6_notify_error(struct mbuf *m, int o
icmp6stat_inc(icp6s_tooshort);
goto freeit;
}
- IP6_EXTHDR_GET(icmp6, struct icmp6_hdr *, &m, off,
- sizeof(*icmp6) + sizeof(struct ip6_hdr));
+ icmp6 = ip6_exthdr_get(&m, off,
+ sizeof(*icmp6) + sizeof(struct ip6_hdr));
if (icmp6 == NULL) {
icmp6stat_inc(icp6s_tooshort);
return (-1);
@@ -791,8 +789,7 @@ icmp6_notify_error(struct mbuf *m, int o
case IPPROTO_HOPOPTS:
case IPPROTO_DSTOPTS:
case IPPROTO_AH:
- IP6_EXTHDR_GET(eh, struct ip6_ext *, &m,
- eoff, sizeof(*eh));
+ eh = ip6_exthdr_get(&m, eoff, sizeof(*eh));
if (eh == NULL) {
icmp6stat_inc(icp6s_tooshort);
return (-1);
@@ -813,8 +810,7 @@ icmp6_notify_error(struct mbuf *m, int o
* information that depends on the final
* destination (e.g. path MTU).
*/
- IP6_EXTHDR_GET(rth, struct ip6_rthdr *, &m,
- eoff, sizeof(*rth));
+ rth = ip6_exthdr_get(&m, eoff, sizeof(*rth));
if (rth == NULL) {
icmp6stat_inc(icp6s_tooshort);
return (-1);
@@ -832,24 +828,23 @@ icmp6_notify_error(struct mbuf *m, int o
rth->ip6r_type == IPV6_RTHDR_TYPE_0) {
int hops;
- IP6_EXTHDR_GET(rth0,
- struct ip6_rthdr0 *, &m,
- eoff, rthlen);
+ rth0 = ip6_exthdr_get(&m, eoff, rthlen);
if (rth0 == NULL) {
icmp6stat_inc(icp6s_tooshort);
return (-1);
}
/* just ignore a bogus header */
if ((rth0->ip6r0_len % 2) == 0 &&
- (hops = rth0->ip6r0_len/2))
- finaldst = (struct in6_addr *)(rth0 + 1) + (hops - 1);
+ (hops = rth0->ip6r0_len/2)) {
+ finaldst = (struct in6_addr *)
+ (rth0 + 1) + (hops - 1);
+ }
}
eoff += rthlen;
nxt = rth->ip6r_nxt;
break;
case IPPROTO_FRAGMENT:
- IP6_EXTHDR_GET(fh, struct ip6_frag *, &m,
- eoff, sizeof(*fh));
+ fh = ip6_exthdr_get(&m, eoff, sizeof(*fh));
if (fh == NULL) {
icmp6stat_inc(icp6s_tooshort);
return (-1);
@@ -879,8 +874,8 @@ icmp6_notify_error(struct mbuf *m, int o
}
}
notify:
- IP6_EXTHDR_GET(icmp6, struct icmp6_hdr *, &m, off,
- sizeof(*icmp6) + sizeof(struct ip6_hdr));
+ icmp6 = ip6_exthdr_get(&m, off,
+ sizeof(*icmp6) + sizeof(struct ip6_hdr));
if (icmp6 == NULL) {
icmp6stat_inc(icp6s_tooshort);
return (-1);
@@ -1204,7 +1199,7 @@ icmp6_redirect_input(struct mbuf *m, int
if (!(ifp->if_xflags & IFXF_AUTOCONF6))
goto freeit;
- IP6_EXTHDR_GET(nd_rd, struct nd_redirect *, &m, off, icmp6len);
+ nd_rd = ip6_exthdr_get(&m, off, icmp6len);
if (nd_rd == NULL) {
icmp6stat_inc(icp6s_tooshort);
if_put(ifp);
Index: netinet6/ip6_input.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/ip6_input.c,v
diff -u -p -r1.271 ip6_input.c
--- netinet6/ip6_input.c 24 May 2025 12:27:23 -0000 1.271
+++ netinet6/ip6_input.c 24 May 2025 14:19:44 -0000
@@ -685,8 +685,8 @@ ip6_hbhchcheck(struct mbuf **mp, int *of
(caddr_t)&ip6->ip6_plen - (caddr_t)ip6);
goto bad;
}
- IP6_EXTHDR_GET(hbh, struct ip6_hbh *, mp,
- sizeof(struct ip6_hdr), sizeof(struct ip6_hbh));
+ hbh = ip6_exthdr_get(mp, sizeof(struct ip6_hdr),
+ sizeof(struct ip6_hbh));
if (hbh == NULL) {
ip6stat_inc(ip6s_tooshort);
goto bad;
@@ -814,15 +814,14 @@ ip6_hopopts_input(struct mbuf **mp, int
struct ip6_hbh *hbh;
/* validation of the length of the header */
- IP6_EXTHDR_GET(hbh, struct ip6_hbh *, mp,
- sizeof(struct ip6_hdr), sizeof(struct ip6_hbh));
+ hbh = ip6_exthdr_get(mp, sizeof(struct ip6_hdr),
+ sizeof(struct ip6_hbh));
if (hbh == NULL) {
ip6stat_inc(ip6s_tooshort);
return -1;
}
hbhlen = (hbh->ip6h_len + 1) << 3;
- IP6_EXTHDR_GET(hbh, struct ip6_hbh *, mp, sizeof(struct ip6_hdr),
- hbhlen);
+ hbh = ip6_exthdr_get(mp, sizeof(struct ip6_hdr), hbhlen);
if (hbh == NULL) {
ip6stat_inc(ip6s_tooshort);
return -1;
Index: netinet6/mld6.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/mld6.c,v
diff -u -p -r1.66 mld6.c
--- netinet6/mld6.c 24 May 2025 12:27:23 -0000 1.66
+++ netinet6/mld6.c 24 May 2025 14:15:51 -0000
@@ -179,7 +179,7 @@ mld6_input(struct mbuf *m, int off)
/* XXX: These are necessary for KAME's link-local hack */
struct in6_addr all_nodes = IN6ADDR_LINKLOCAL_ALLNODES_INIT;
- IP6_EXTHDR_GET(mldh, struct mld_hdr *, &m, off, sizeof(*mldh));
+ mldh = ip6_exthdr_get(&m, off, sizeof(*mldh));
if (mldh == NULL) {
icmp6stat_inc(icp6s_tooshort);
return;
Index: netinet6/nd6_nbr.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/nd6_nbr.c,v
diff -u -p -r1.159 nd6_nbr.c
--- netinet6/nd6_nbr.c 24 May 2025 12:27:23 -0000 1.159
+++ netinet6/nd6_nbr.c 24 May 2025 14:16:32 -0000
@@ -117,7 +117,7 @@ nd6_ns_input(struct mbuf *m, int off, in
if (ifp == NULL)
goto freeit;
- IP6_EXTHDR_GET(nd_ns, struct nd_neighbor_solicit *, &m, off, icmp6len);
+ nd_ns = ip6_exthdr_get(&m, off, icmp6len);
if (nd_ns == NULL) {
icmp6stat_inc(icp6s_tooshort);
if_put(ifp);
@@ -540,7 +540,7 @@ nd6_na_input(struct mbuf *m, int off, in
if (ip6->ip6_hlim != 255)
goto bad;
- IP6_EXTHDR_GET(nd_na, struct nd_neighbor_advert *, &m, off, icmp6len);
+ nd_na = ip6_exthdr_get(&m, off, icmp6len);
if (nd_na == NULL) {
icmp6stat_inc(icp6s_tooshort);
if_put(ifp);
Index: netinet6/nd6_rtr.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/nd6_rtr.c,v
diff -u -p -r1.174 nd6_rtr.c
--- netinet6/nd6_rtr.c 24 May 2025 12:27:24 -0000 1.174
+++ netinet6/nd6_rtr.c 24 May 2025 14:17:58 -0000
@@ -92,8 +92,7 @@ nd6_rtr_cache(struct mbuf *m, int off, i
if (IN6_IS_ADDR_UNSPECIFIED(&saddr6))
goto freeit;
- IP6_EXTHDR_GET(nd_rs, struct nd_router_solicit *, &m, off,
- icmp6len);
+ nd_rs = ip6_exthdr_get(&m, off, icmp6len);
if (nd_rs == NULL) {
icmp6stat_inc(icp6s_tooshort);
return;
@@ -109,8 +108,7 @@ nd6_rtr_cache(struct mbuf *m, int off, i
if (!IN6_IS_ADDR_LINKLOCAL(&saddr6))
goto bad;
- IP6_EXTHDR_GET(nd_ra, struct nd_router_advert *, &m, off,
- icmp6len);
+ nd_ra = ip6_exthdr_get(&m, off, icmp6len);
if (nd_ra == NULL) {
icmp6stat_inc(icp6s_tooshort);
return;
Index: netinet6/raw_ip6.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/raw_ip6.c,v
diff -u -p -r1.191 raw_ip6.c
--- netinet6/raw_ip6.c 24 May 2025 12:27:24 -0000 1.191
+++ netinet6/raw_ip6.c 24 May 2025 14:20:04 -0000
@@ -147,8 +147,7 @@ rip6_input(struct mbuf **mp, int *offp,
if (proto == IPPROTO_ICMPV6) {
struct icmp6_hdr *icmp6;
- IP6_EXTHDR_GET(icmp6, struct icmp6_hdr *, mp, *offp,
- sizeof(*icmp6));
+ icmp6 = ip6_exthdr_get(mp, *offp, sizeof(*icmp6));
if (icmp6 == NULL)
return IPPROTO_DONE;
type = icmp6->icmp6_type;
Index: netinet6/route6.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/route6.c,v
diff -u -p -r1.24 route6.c
--- netinet6/route6.c 24 May 2025 12:27:24 -0000 1.24
+++ netinet6/route6.c 24 May 2025 14:20:27 -0000
@@ -59,7 +59,7 @@ route6_input(struct mbuf **mp, int *offp
int off = *offp, rhlen;
ip6 = mtod(*mp, struct ip6_hdr *);
- IP6_EXTHDR_GET(rh, struct ip6_rthdr *, mp, off, sizeof(*rh));
+ rh = ip6_exthdr_get(mp, off, sizeof(*rh));
if (rh == NULL) {
ip6stat_inc(ip6s_tooshort);
return IPPROTO_DONE;
IP6_EXTHDR_GET pass mbuf pointer