From nobody Fri Oct 30 14:14:47 2026 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4jGNQb3hgqz6wx22 for ; Fri, 30 Oct 2026 14:14:47 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange x25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id 4jGNQb2rqLz3CJp for ; Fri, 30 Oct 2026 14:14:47 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1793369687; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=HybljtxNU1vdYpL1npOD7edvj6vjsqJ7vvYDUimuY/Y=; b=pRwtZZe7jqyqVMl9OMoEznKp5z56lRmukYAGYTxW4f3n2FZYZPLqrcTLbMjHC5dLH37fx2 9oO3YIfdlnIA2YWhinDLk3JAsjFNpaLe8ZsThHeEm9sjT6nOGRZoJ0ABz0t4knQuGjo1as 4JeO+rRAO1wo2AgaVuf50dgpnl7FdK7MMElQAOfXiL1rX9ezci5c/0Y1NjWh0Q5w6CldJe 0DgaKUtxi4yMW/LtSEkztf1OmqjO5X4fmRbUkN8DjD+rvOK1wKbdY+i5w3IL248OzcB5tK 4B/50+q3EP8c1IIRsZW5vmSqjQu4rmuMWMZ2bvNfrSZ03IIUvnsznJRetlXotg== ARC-Seal: i=1; a=rsa-sha256; d=freebsd.org; s=dkim; cv=none; t=1793369687; b=qARfwqEftqnCiIFW3lGq0OSau1e30jQVUnTJVghnf6sLbTx+kRd0a3Efmy51gWnkYuaE/P PnM3kNURg4QGiXxn+cSMWAtV+03bcpTXp4sdZApiqU7Oq77eY//od4ozaccs57RuoWt61L ny4LTf78p/9uS+RaAZYf8vRdQvNtlych3p/iwbT5fAI6I/gIDVUMRuVpsBDoly0DNN5ezs pQn/viXjvsEl6JFElAhYJbWSOpl711uUAS2MOdSKLmL5UO+fk7h3TDksrhAOJFp3vjVp6O lzwdKe2QDqL7+byIAd8cBSb8SuUgWeGeK+/QZ1E4ALav14WVIHUBmbjwWviYtQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1793369687; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=HybljtxNU1vdYpL1npOD7edvj6vjsqJ7vvYDUimuY/Y=; b=EHe4yVtz6/JHtN9vM+x4zQSycg2Rvkh7uLWjt1h296uSVnmGr5U6B3B7Ymo7CCsLkygmZ6 e+KGEuyPn4QzIp2foHtn1Jj+wTV4fstvwH/b5i00QYFOjhv0ZtA+CNx3C20lNM5Y74uaa6 N/CVWdYO7tkoS8StdxNn9VywiZY7E9Ul5PwKiq90rdNSMs5n7Xs+dPzg9agjIkPgtc6X8x 0kHl1gkP6Ieg9TA1RAV1njg7MTgkU+/FWnThEaCP2HyXGmIpfnvTiTF6jgnI+nwJ83MHq2 KJuy5Y8TrGLsvDFgw7h7gVeCT1ZLum2ihgwdBXeB9NFMeCw/KfO+C9xFlsXo+w== ARC-Authentication-Results: i=1; mx1.freebsd.org; none Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) by mxrelay.nyi.freebsd.org (Postfix) with ESMTP id 4jGNQb1ynjzVNX for ; Fri, 30 Oct 2026 14:14:47 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from git (uid 1279) (envelope-from git@FreeBSD.org) id 2371d by gitrepo.freebsd.org (DragonFly Mail Agent v0.13+ on gitrepo.freebsd.org); Fri, 30 Oct 2026 14:14:47 +0000 To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Kristof Provost Subject: git: c6a520af95b2 - main - pfsync: improve handling of large packets List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all X-BeenThere: dev-commits-src-all@freebsd.org Sender: owner-dev-commits-src-all@FreeBSD.org List-Id: Commit messages for all branches of the src repository List-Post: List-Help: List-Subscribe: List-Unsubscribe: List-Owner: Precedence: list MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: kp X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: c6a520af95b206d6e6a603e95314398d31f57def Auto-Submitted: auto-generated Date: Fri, 30 Oct 2026 14:14:47 +0000 Message-Id: <6ae4a657.2371d.48ec14f5@gitrepo.freebsd.org> The branch main has been updated by kp: URL: https://cgit.FreeBSD.org/src/commit/?id=c6a520af95b206d6e6a603e95314398d31f57def commit c6a520af95b206d6e6a603e95314398d31f57def Author: Kristof Provost AuthorDate: 2026-10-30 09:48:26 +0000 Commit: Kristof Provost CommitDate: 2026-10-30 14:14:13 +0000 pfsync: improve handling of large packets On setups with pfsync over large MTUs (> 2K) we could fail to parse packets because m_pulldown() fails on packets larger than MCLBYTES. Instead use m_copydata() to copy out the individual chunks we're interested in. This should also fix the flaky pfsync:bulk test case. Depending on the hash seed states would or would not end up synced. PR: 297307 Sponsored by: Rubicon Communications, LLC ("Netgate") --- sys/netpfil/pf/if_pfsync.c | 212 ++++++++++++++++++++------------------------- 1 file changed, 93 insertions(+), 119 deletions(-) diff --git a/sys/netpfil/pf/if_pfsync.c b/sys/netpfil/pf/if_pfsync.c index fa72958788da..97688294bebc 100644 --- a/sys/netpfil/pf/if_pfsync.c +++ b/sys/netpfil/pf/if_pfsync.c @@ -1146,25 +1146,23 @@ done: static int pfsync_in_clr(struct mbuf *m, int offset, int count, int flags, int action) { - struct pfsync_clr *clr; + struct pfsync_clr clr; struct pfi_kkif *kif = NULL; - struct mbuf *mp; - int len = sizeof(*clr) * count; - int i, offp; + int len = sizeof(clr) * count; + int i; u_int32_t creatorid; - mp = m_pulldown(m, offset, len, &offp); - if (mp == NULL) { + if (m->m_pkthdr.len < (offset + len)) { V_pfsyncstats.pfsyncs_badlen++; return (-1); } - clr = (struct pfsync_clr *)(mp->m_data + offp); for (i = 0; i < count; i++) { - creatorid = clr[i].creatorid; + m_copydata(m, offset + sizeof(clr) * i, sizeof(clr), &clr); + creatorid = clr.creatorid; - if (clr[i].ifname[0] != '\0' && - (kif = pfi_kkif_find(clr[i].ifname)) == NULL) + if (clr.ifname[0] != '\0' && + (kif = pfi_kkif_find(clr.ifname)) == NULL) continue; for (int i = 0; i <= V_pf_hashmask; i++) { @@ -1192,9 +1190,8 @@ relock: static int pfsync_in_ins(struct mbuf *m, int offset, int count, int flags, int action) { - struct mbuf *mp; - union pfsync_state_union *sa, *sp; - int i, offp, total_len, msg_version, msg_len; + union pfsync_state_union sp; + int i, total_len, msg_version, msg_len; u_int8_t timeout, direction; sa_family_t af; @@ -1218,34 +1215,32 @@ pfsync_in_ins(struct mbuf *m, int offset, int count, int flags, int action) total_len = msg_len * count; - mp = m_pulldown(m, offset, total_len, &offp); - if (mp == NULL) { + if (m->m_pkthdr.len < (offset + total_len)) { V_pfsyncstats.pfsyncs_badlen++; return (-1); } - sa = (union pfsync_state_union *)(mp->m_data + offp); for (i = 0; i < count; i++) { - sp = (union pfsync_state_union *)((char *)sa + msg_len * i); + m_copydata(m, offset + msg_len * i, msg_len, &sp); switch (msg_version) { case PFSYNC_MSG_VERSION_1301: case PFSYNC_MSG_VERSION_1400: - af = sp->pfs_1301.af; - timeout = sp->pfs_1301.timeout; - direction = sp->pfs_1301.direction; + af = sp.pfs_1301.af; + timeout = sp.pfs_1301.timeout; + direction = sp.pfs_1301.direction; break; case PFSYNC_MSG_VERSION_1500: - af = sp->pfs_1500.wire_af; - timeout = sp->pfs_1500.timeout; - direction = sp->pfs_1500.direction; + af = sp.pfs_1500.wire_af; + timeout = sp.pfs_1500.timeout; + direction = sp.pfs_1500.direction; break; } /* Check for invalid values. */ if (timeout >= PFTM_MAX || - sp->pfs_1301.src.state > PF_TCPS_PROXY_DST || - sp->pfs_1301.dst.state > PF_TCPS_PROXY_DST || + sp.pfs_1301.src.state > PF_TCPS_PROXY_DST || + sp.pfs_1301.dst.state > PF_TCPS_PROXY_DST || direction > PF_OUT || (af != AF_INET && af != AF_INET6)) { if (V_pf_status.debug >= PF_DEBUG_MISC) @@ -1254,7 +1249,7 @@ pfsync_in_ins(struct mbuf *m, int offset, int count, int flags, int action) continue; } - if (pfsync_state_import(sp, flags, msg_version) != 0) + if (pfsync_state_import(&sp, flags, msg_version) != 0) V_pfsyncstats.pfsyncs_badact++; } @@ -1264,24 +1259,20 @@ pfsync_in_ins(struct mbuf *m, int offset, int count, int flags, int action) static int pfsync_in_iack(struct mbuf *m, int offset, int count, int flags, int action) { - struct pfsync_ins_ack *ia, *iaa; + struct pfsync_ins_ack ia; struct pf_kstate *st; + int i; + int len = count * sizeof(struct pfsync_ins_ack); - struct mbuf *mp; - int len = count * sizeof(*ia); - int offp, i; - - mp = m_pulldown(m, offset, len, &offp); - if (mp == NULL) { + if (m->m_pkthdr.len < (offset + len)) { V_pfsyncstats.pfsyncs_badlen++; return (-1); } - iaa = (struct pfsync_ins_ack *)(mp->m_data + offp); for (i = 0; i < count; i++) { - ia = &iaa[i]; + m_copydata(m, offset + sizeof(ia) * i, sizeof(ia), &ia); - st = pf_find_state_byid(ia->id, ia->creatorid); + st = pf_find_state_byid(ia.id, ia.creatorid); if (st == NULL) continue; @@ -1295,7 +1286,7 @@ pfsync_in_iack(struct mbuf *m, int offset, int count, int flags, int action) * message so we can skip it. */ - return (count * sizeof(struct pfsync_ins_ack)); + return (len); } static int @@ -1336,10 +1327,9 @@ static int pfsync_in_upd(struct mbuf *m, int offset, int count, int flags, int action) { struct pfsync_softc *sc = V_pfsyncif; - union pfsync_state_union *sa, *sp; + union pfsync_state_union sp; struct pf_kstate *st; - struct mbuf *mp; - int sync, offp, i, total_len, msg_len, msg_version; + int sync, i, total_len, msg_len, msg_version; u_int8_t timeout; switch (action) { @@ -1362,30 +1352,28 @@ pfsync_in_upd(struct mbuf *m, int offset, int count, int flags, int action) total_len = msg_len * count; - mp = m_pulldown(m, offset, total_len, &offp); - if (mp == NULL) { + if (m->m_pkthdr.len < (offset + total_len)) { V_pfsyncstats.pfsyncs_badlen++; return (-1); } - sa = (union pfsync_state_union *)(mp->m_data + offp); for (i = 0; i < count; i++) { - sp = (union pfsync_state_union *)((char *)sa + msg_len * i); + m_copydata(m, offset + msg_len * i, msg_len, &sp); switch (msg_version) { case PFSYNC_MSG_VERSION_1301: case PFSYNC_MSG_VERSION_1400: - timeout = sp->pfs_1301.timeout; + timeout = sp.pfs_1301.timeout; break; case PFSYNC_MSG_VERSION_1500: - timeout = sp->pfs_1500.timeout; + timeout = sp.pfs_1500.timeout; break; } /* check for invalid values */ if (timeout >= PFTM_MAX || - sp->pfs_1301.src.state > PF_TCPS_PROXY_DST || - sp->pfs_1301.dst.state > PF_TCPS_PROXY_DST) { + sp.pfs_1301.src.state > PF_TCPS_PROXY_DST || + sp.pfs_1301.dst.state > PF_TCPS_PROXY_DST) { if (V_pf_status.debug >= PF_DEBUG_MISC) { printf("pfsync_input: PFSYNC_ACT_UPD: " "invalid value\n"); @@ -1394,10 +1382,10 @@ pfsync_in_upd(struct mbuf *m, int offset, int count, int flags, int action) continue; } - st = pf_find_state_byid(sp->pfs_1301.id, sp->pfs_1301.creatorid); + st = pf_find_state_byid(sp.pfs_1301.id, sp.pfs_1301.creatorid); if (st == NULL) { /* insert the update */ - if (pfsync_state_import(sp, flags, msg_version)) + if (pfsync_state_import(&sp, flags, msg_version)) V_pfsyncstats.pfsyncs_badstate++; continue; } @@ -1407,7 +1395,7 @@ pfsync_in_upd(struct mbuf *m, int offset, int count, int flags, int action) } if (st->key[PF_SK_WIRE]->proto == IPPROTO_TCP) - sync = pfsync_upd_tcp(st, &sp->pfs_1301.src, &sp->pfs_1301.dst); + sync = pfsync_upd_tcp(st, &sp.pfs_1301.src, &sp.pfs_1301.dst); else { sync = 0; @@ -1415,18 +1403,18 @@ pfsync_in_upd(struct mbuf *m, int offset, int count, int flags, int action) * Non-TCP protocol state machine always go * forwards */ - if (st->src.state > sp->pfs_1301.src.state) + if (st->src.state > sp.pfs_1301.src.state) sync++; else - pf_state_peer_ntoh(&sp->pfs_1301.src, &st->src); - if (st->dst.state > sp->pfs_1301.dst.state) + pf_state_peer_ntoh(&sp.pfs_1301.src, &st->src); + if (st->dst.state > sp.pfs_1301.dst.state) sync++; else - pf_state_peer_ntoh(&sp->pfs_1301.dst, &st->dst); + pf_state_peer_ntoh(&sp.pfs_1301.dst, &st->dst); } if (sync < 2) { - pfsync_alloc_scrub_memory(&sp->pfs_1301.dst, &st->dst); - pf_state_peer_ntoh(&sp->pfs_1301.dst, &st->dst); + pfsync_alloc_scrub_memory(&sp.pfs_1301.dst, &st->dst); + pf_state_peer_ntoh(&sp.pfs_1301.dst, &st->dst); st->expire = pf_get_uptime(); st->timeout = timeout; } @@ -1450,27 +1438,24 @@ static int pfsync_in_upd_c(struct mbuf *m, int offset, int count, int flags, int action) { struct pfsync_softc *sc = V_pfsyncif; - struct pfsync_upd_c *ua, *up; + struct pfsync_upd_c up; struct pf_kstate *st; - int len = count * sizeof(*up); + int len = count * sizeof(up); int sync; - struct mbuf *mp; - int offp, i; + int i; - mp = m_pulldown(m, offset, len, &offp); - if (mp == NULL) { + if (m->m_pkthdr.len < (offset + len)) { V_pfsyncstats.pfsyncs_badlen++; return (-1); } - ua = (struct pfsync_upd_c *)(mp->m_data + offp); for (i = 0; i < count; i++) { - up = &ua[i]; + m_copydata(m, offset + sizeof(up) * i, sizeof(up), &up); /* check for invalid values */ - if (up->timeout >= PFTM_MAX || - up->src.state > PF_TCPS_PROXY_DST || - up->dst.state > PF_TCPS_PROXY_DST) { + if (up.timeout >= PFTM_MAX || + up.src.state > PF_TCPS_PROXY_DST || + up.dst.state > PF_TCPS_PROXY_DST) { if (V_pf_status.debug >= PF_DEBUG_MISC) { printf("pfsync_input: " "PFSYNC_ACT_UPD_C: " @@ -1480,11 +1465,11 @@ pfsync_in_upd_c(struct mbuf *m, int offset, int count, int flags, int action) continue; } - st = pf_find_state_byid(up->id, up->creatorid); + st = pf_find_state_byid(up.id, up.creatorid); if (st == NULL) { /* We don't have this state. Ask for it. */ PFSYNC_BUCKET_LOCK(&sc->sc_buckets[0]); - pfsync_request_update(up->creatorid, up->id); + pfsync_request_update(up.creatorid, up.id); PFSYNC_BUCKET_UNLOCK(&sc->sc_buckets[0]); continue; } @@ -1494,7 +1479,7 @@ pfsync_in_upd_c(struct mbuf *m, int offset, int count, int flags, int action) } if (st->key[PF_SK_WIRE]->proto == IPPROTO_TCP) - sync = pfsync_upd_tcp(st, &up->src, &up->dst); + sync = pfsync_upd_tcp(st, &up.src, &up.dst); else { sync = 0; @@ -1502,20 +1487,20 @@ pfsync_in_upd_c(struct mbuf *m, int offset, int count, int flags, int action) * Non-TCP protocol state machine always go * forwards */ - if (st->src.state > up->src.state) + if (st->src.state > up.src.state) sync++; else - pf_state_peer_ntoh(&up->src, &st->src); - if (st->dst.state > up->dst.state) + pf_state_peer_ntoh(&up.src, &st->src); + if (st->dst.state > up.dst.state) sync++; else - pf_state_peer_ntoh(&up->dst, &st->dst); + pf_state_peer_ntoh(&up.dst, &st->dst); } if (sync < 2) { - pfsync_alloc_scrub_memory(&up->dst, &st->dst); - pf_state_peer_ntoh(&up->dst, &st->dst); + pfsync_alloc_scrub_memory(&up.dst, &st->dst); + pf_state_peer_ntoh(&up.dst, &st->dst); st->expire = pf_get_uptime(); - st->timeout = up->timeout; + st->timeout = up.timeout; } st->pfsync_time = time_uptime; @@ -1536,27 +1521,24 @@ pfsync_in_upd_c(struct mbuf *m, int offset, int count, int flags, int action) static int pfsync_in_ureq(struct mbuf *m, int offset, int count, int flags, int action) { - struct pfsync_upd_req *ur, *ura; - struct mbuf *mp; - int len = count * sizeof(*ur); - int i, offp; + struct pfsync_upd_req ur; + int len = count * sizeof(ur); + int i; struct pf_kstate *st; - mp = m_pulldown(m, offset, len, &offp); - if (mp == NULL) { + if (m->m_pkthdr.len < (offset + len)) { V_pfsyncstats.pfsyncs_badlen++; return (-1); } - ura = (struct pfsync_upd_req *)(mp->m_data + offp); for (i = 0; i < count; i++) { - ur = &ura[i]; + m_copydata(m, offset + sizeof(ur) * i, sizeof(ur), &ur); - if (ur->id == 0 && ur->creatorid == 0) + if (ur.id == 0 && ur.creatorid == 0) pfsync_bulk_start(); else { - st = pf_find_state_byid(ur->id, ur->creatorid); + st = pf_find_state_byid(ur.id, ur.creatorid); if (st == NULL) { V_pfsyncstats.pfsyncs_badstate++; continue; @@ -1577,23 +1559,20 @@ pfsync_in_ureq(struct mbuf *m, int offset, int count, int flags, int action) static int pfsync_in_del_c(struct mbuf *m, int offset, int count, int flags, int action) { - struct mbuf *mp; - struct pfsync_del_c *sa, *sp; + struct pfsync_del_c sp; struct pf_kstate *st; - int len = count * sizeof(*sp); - int offp, i; + int len = count * sizeof(sp); + int i; - mp = m_pulldown(m, offset, len, &offp); - if (mp == NULL) { + if (m->m_pkthdr.len < (offset + len)) { V_pfsyncstats.pfsyncs_badlen++; return (-1); } - sa = (struct pfsync_del_c *)(mp->m_data + offp); for (i = 0; i < count; i++) { - sp = &sa[i]; + m_copydata(m, offset + sizeof(sp) * i, sizeof(sp), &sp); - st = pf_find_state_byid(sp->id, sp->creatorid); + st = pf_find_state_byid(sp.id, sp.creatorid); if (st == NULL) { V_pfsyncstats.pfsyncs_badstate++; continue; @@ -1610,10 +1589,8 @@ static int pfsync_in_bus(struct mbuf *m, int offset, int count, int flags, int action) { struct pfsync_softc *sc = V_pfsyncif; - struct pfsync_bus *bus; - struct mbuf *mp; - int len = count * sizeof(*bus); - int offp; + struct pfsync_bus bus; + int len = count * sizeof(bus); PFSYNC_BLOCK(sc); @@ -1623,15 +1600,14 @@ pfsync_in_bus(struct mbuf *m, int offset, int count, int flags, int action) return (len); } - mp = m_pulldown(m, offset, len, &offp); - if (mp == NULL) { - PFSYNC_BUNLOCK(sc); + if (m->m_pkthdr.len < (offset + len)) { V_pfsyncstats.pfsyncs_badlen++; return (-1); } - bus = (struct pfsync_bus *)(mp->m_data + offp); - switch (bus->status) { + m_copydata(m, offset, sizeof(bus), &bus); + + switch (bus.status) { case PFSYNC_BUS_START: callout_reset(&sc->sc_bulkfail_tmo, 4 * hz + V_pf_limits[PF_LIMIT_STATES].limit / @@ -1643,7 +1619,7 @@ pfsync_in_bus(struct mbuf *m, int offset, int count, int flags, int action) break; case PFSYNC_BUS_END: - if (time_uptime - ntohl(bus->endtime) >= + if (time_uptime - ntohl(bus.endtime) >= sc->sc_ureq_sent) { /* that's it, we're happy */ sc->sc_ureq_sent = 0; @@ -1673,22 +1649,20 @@ pfsync_in_tdb(struct mbuf *m, int offset, int count, int flags, int action) { int len = count * sizeof(struct pfsync_tdb); -#if defined(IPSEC) - struct pfsync_tdb *tp; - struct mbuf *mp; - int offp; - int i; - int s; - - mp = m_pulldown(m, offset, len, &offp); - if (mp == NULL) { + if (m->m_pkthdr.len < (offset + len)) { V_pfsyncstats.pfsyncs_badlen++; return (-1); } - tp = (struct pfsync_tdb *)(mp->m_data + offp); - for (i = 0; i < count; i++) - pfsync_update_net_tdb(&tp[i]); +#if defined(IPSEC) + struct pfsync_tdb tp; + int i; + + for (i = 0; i < count; i++) { + m_copydata(m, offset + sizeof(struct pfsync_tdb) * i, + sizeof(pfsync_tdb), &tp); + pfsync_update_net_tdb(tp); + } #endif return (len);