The change I proposed is intended to be conservative, imposing limits on
padding if and only if it is the reason packet length goes above the
MTU, otherwise keeping the existing behavior.
Bjorn's change aims to address the broader MTU issue (whether or not is
caused by padding) more aggressively.
One observations about the new proposal:
- When `m->m_pkthdr.len` is already above `t->t_mtu` (the scenario Bjorn
seeks to address), the calculation of `padding_len` triggers unsigned
integer wrapping, potentially causing unintended effect on the `bzero()`
that follows.
And a few questions:
1. Why truncate not discard?
2. What are all the scenarios where `m_pkthdr.len` could go beyond
`t->t_mtu`?
3. Depending on the scenarios in 2. should ICMP need frag / ICMP6 too
large or a debug message be generated?
4. Are there any pathway in OpenBSD where the `if_mtu` member `struct
ifnet` can become less than or equal to zero?
Thanks.
On 8/7/26 17:45, Bjorn Ketelaars wrote:
On Fri 07/08/2026 07:17, Zixu Wu wrote:
Zero padding shouldn't cause the packet size to go beyond MTU.
diff --git a/sys/net/if_wg.c b/sys/net/if_wg.c
index 0641c4b5ba9..90ef5b808a0 100644
--- a/sys/net/if_wg.c
+++ b/sys/net/if_wg.c
@@ -1592,6 +1592,9 @@ wg_encap(struct wg_softc *sc, struct mbuf *m)
peer = t->t_peer;
plaintext_len = WG_PKT_WITH_PADDING(m->m_pkthdr.len);
+ if (m->m_pkthdr.len <= t->t_mtu && plaintext_len > t->t_mtu) {
+ plaintext_len = t->t_mtu;
+ }
padding_len = plaintext_len - m->m_pkthdr.len;
out_len = sizeof(struct wg_pkt_data) + plaintext_len +
NOISE_AUTHTAG_LEN;
I think the premise of Zixu's patch is valid: with t_mtu = if_mtu = 1420
(default, not a multiple of 16), inner packets of 1409–1420 round up to
1424 via WG_PKT_WITH_PADDING. Adding the 80-byte wg envelope pushes
total volume to 1504 bytes, exceeding the standard transport capacity
and forcing the network to split single packages into fragments.
Zixu's proposed change:
if (m->m_pkthdr.len <= t->t_mtu && plaintext_len > t->t_mtu) {
plaintext_len = t->t_mtu;
}
However:
- If an incoming package already exceeds the limit (m->m_pkthdr.len >
t->t_mtu), the guard condition evaluates to false. The restriction is
bypassed completely, allowing padding to inflate the payload even
further. The MTU ceiling must apply unconditionally.
- System keepalives initialize with zeroed values (t_mtu = 0, len = 0).
Applying padding to zero yields 16 bytes, triggering the clamp back to
0 only by coincidence. Explicitly checking (t->t_mtu != 0) makes the
logic transparent: keepalives carry no MTU constraint, whereas data
payloads must strictly observe it.
Using MIN() provides an explicit, elegant safeguard while maintaining
clean code conventions:
diff --git sys/net/if_wg.c sys/net/if_wg.c
index 0641c4b5ba9..1a00af9dc1d 100644
--- sys/net/if_wg.c
+++ sys/net/if_wg.c
@@ -1592,6 +1592,8 @@ wg_encap(struct wg_softc *sc, struct mbuf *m)
peer = t->t_peer;
plaintext_len = WG_PKT_WITH_PADDING(m->m_pkthdr.len);
+ if (t->t_mtu != 0)
+ plaintext_len = MIN(plaintext_len, t->t_mtu);
padding_len = plaintext_len - m->m_pkthdr.len;
out_len = sizeof(struct wg_pkt_data) + plaintext_len +
NOISE_AUTHTAG_LEN;
