Re: git: 54c62e3e5d8c - main - pf: work around icmp6 packet-too-big not being sent when binat-ing

From: Kristof Provost <kp_at_FreeBSD.org>
Date: Wed, 24 Jan 2024 13:27:19 UTC
On 24 Jan 2024, at 11:06, Herbert J. Skuhra wrote:
> On Tue, 23 Jan 2024 19:42:10 +0100, Kristof Provost wrote:
>> On 23 Jan 2024, at 19:32, Herbert J. Skuhra wrote:
>>> On Mon, 22 Jan 2024 13:52:51 +0100, Kristof Provost wrote:
>>>>
>>>> The branch main has been updated by kp:
>>>>
>>>> URL: 
>>>> https://cgit.FreeBSD.org/src/commit/?id=54c62e3e5d8cd90c5571a1d4c8c5f062d580480e
>>>>
>>>> commit 54c62e3e5d8cd90c5571a1d4c8c5f062d580480e
>>>> Author:     Kristof Provost <kp@FreeBSD.org>
>>>> AuthorDate: 2024-01-17 17:11:27 +0000
>>>> Commit:     Kristof Provost <kp@FreeBSD.org>
>>>> CommitDate: 2024-01-22 11:52:14 +0000
>>>>
>>>>     pf: work around icmp6 packet-too-big not being sent when 
>>>> binat-ing
>>>>
>>>>     If we're applying NPTv6 we pass a packet with a modified source 
>>>> and/or
>>>>     destination address to the network stack.
>>>>
>>>>     If that packet then turns out to be larger than the MTU of the 
>>>> sending
>>>>     interface the stack will attempt to generate an icmp6 
>>>> packet-too-big
>>>>     error, but may fail to look up the appropriate source address 
>>>> for that
>>>>     error message. Even if it does, pf would still have to undo the 
>>>> binat
>>>>     operation inside the icmp6 packet so the sending host can make 
>>>> sense of
>>>>     the error.
>>>>
>>>>     We can avoid both problems entirely by having pf also perform 
>>>> the MTU
>>>>     check (taking the potential refragmentation into account), and
>>>>     generating the icmp6 error directly in pf.
>>>>
>>>>     See also:       https://redmine.pfsense.org/issues/14290
>>>>     Sponsored by:   Rubicon Communications, LLC ("Netgate")
>>>>     Differential Revision:  https://reviews.freebsd.org/D43499
>>>> ---
>>>>  sys/net/pfvar.h          |  1 +
>>>>  sys/netpfil/pf/pf.c      | 12 ++++++++++++
>>>>  sys/netpfil/pf/pf_norm.c | 15 +++++++++++++++
>>>>  3 files changed, 28 insertions(+)
>>>
>>> Does this change cause problems for others too?
>>>
>>> - ssh over IPv6 permanently disconnecting
>>> (client_loop: send disconnect: Broken pipe)
>>> - ssh connections over IPv6 hanging
>>> - git pull not working
>>> Fssh_ssh_dispatch_run_fatal: Connection to 
>>> 2604:1380:4091:a001::24ca:1 port 22: Permission denied
>>> fatal: Could not read from remote repository.
>>>
>> Can you include your pf.conf and a packet capture demonstrating one 
>> of these issues?
>
> So I assume this issue affects only me or this server (igb nic).
> Disabling tso6 seems to resolve the issue.
>
Ah. A Clue(tm)!

Try this:

	diff --git a/sys/netpfil/pf/pf.c b/sys/netpfil/pf/pf.c
	index 38a5a45d7991..2dc6d02d330a 100644
	--- a/sys/netpfil/pf/pf.c
	+++ b/sys/netpfil/pf/pf.c
	@@ -8515,7 +8515,7 @@ pf_test6(int dir, int pflags, struct ifnet *ifp, 
struct mbuf **m0, struct inpcb
	         * confused and fail to send the icmp6 packet too big error. 
Just send
	         * it here, before we do any NAT.
	         */
	-       if (dir == PF_OUT && IN6_LINKMTU(ifp) < pf_max_frag_size(m)) {
	+       if (dir == PF_OUT && pflags & PFIL_FWD && IN6_LINKMTU(ifp) < 
pf_max_frag_size(m)) {
	                PF_RULES_RUNLOCK();
	                *m0 = NULL;
	                icmp6_error(m, ICMP6_PACKET_TOO_BIG, 0, 
IN6_LINKMTU(ifp));

Best regards,
Kristof