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

On Wed, 24 Jan 2024 14:27:19 +0100, Kristof Provost wrote: 
> 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));

This patch fixes the issue. Thanks a lot.

--
Herbert