Bug 229006 - ipfw+nat and ng_nat Silently Drop Packets over 4k
Summary: ipfw+nat and ng_nat Silently Drop Packets over 4k
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: 11.1-RELEASE
Hardware: Any Any
: --- Affects Some People
Assignee: Andrey V. Elsukov
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-06-14 00:02 UTC by Jeff Kletsky
Modified: 2018-06-22 23:57 UTC (History)
3 users (show)

See Also:


Attachments
Possible patch from suggestion of Andrey V. Elsukov (566 bytes, patch)
2018-06-14 00:07 UTC, Jeff Kletsky
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jeff Kletsky 2018-06-14 00:02:28 UTC
As discovered on 11.1-RELEASE-p9 and present on -p10, reassembled packets over 4k are silently dropped by in-kernel NAT.

Patch based on suggestion of Andrey V. Elsukov supplied.

Cause identified by Andrey V. Elsukov on the freebsd-net and freebsd-ipfw lists on 2018-06-13 as being due to buffer allocation limits in the in-kernel implementation of libalias.

"The kernel version of libalias uses m_megapullup() function to make
single contiguous buffer. m_megapullup() uses m_get2() function to
allocate mbuf of appropriate size. If size of packet greater than 4k it
will fail. So, if you use MTU greater than 4k or if after fragments
reassembly you get a packet with length greater than 4k, ipfw_nat()
function will drop this packet."

Additional communication on those lists by Andrey suggested a patch might resolve this issue. The following is his code, I take no credit for it. Tested and "works for me" on kernel sources from 11.1-RELEASE-p10 and GENERIC kernconf.

/usr/src/sys/netinet/libalias]$ diff -u alias.c.orig alias.c
8<
--- alias.c.orig	2017-07-20 16:42:02.000000000 -0700
+++ alias.c	2018-06-13 15:41:46.862121000 -0700
@@ -1758,7 +1758,14 @@
 	if (m->m_next == NULL && M_WRITABLE(m))
 		return (m);
 
-	mcl = m_get2(len, M_NOWAIT, MT_DATA, M_PKTHDR);
+	if (len <= MJUMPAGESIZE)
+		mcl = m_get2(len, M_NOWAIT, MT_DATA, M_PKTHDR);
+	else if (len <= MJUM9BYTES)
+		mcl = m_getjcl(M_NOWAIT, MT_DATA, M_PKTHDR, MJUM9BYTES);
+	else if (len <= MJUM16BYTES)
+		mcl = m_getjcl(M_NOWAIT, MT_DATA, M_PKTHDR, MJUM16BYTES);
+	else
+		goto bad;
 	if (mcl == NULL)
 		goto bad;
 	m_align(mcl, len);
>8

Additional details on the situation that highlighted this can be found at 
https://forums.freebsd.org/threads/in-kernel-nat-dropping-large-udp-return-packets.66262/
Comment 1 Jeff Kletsky 2018-06-14 00:03:48 UTC
Relates to documentation issue #229003
Comment 2 Jeff Kletsky 2018-06-14 00:07:03 UTC
Created attachment 194242 [details]
Possible patch from suggestion of Andrey V. Elsukov
Comment 3 commit-hook freebsd_committer freebsd_triage 2018-06-14 11:16:13 UTC
A commit references this bug:

Author: ae
Date: Thu Jun 14 11:15:40 UTC 2018
New revision: 335133
URL: https://svnweb.freebsd.org/changeset/base/335133

Log:
  In m_megapullup() use m_getjcl() to allocate 9k or 16k mbuf when requested.

  It is better to try allocate a big mbuf, than just silently drop a big
  packet. A better solution could be reworking of libalias modules to be
  able use m_copydata()/m_copyback() instead of requiring the single
  contiguous buffer.

  PR:		229006
  MFC after:	1 week

Changes:
  head/sys/netinet/libalias/alias.c
Comment 4 Gleb Smirnoff freebsd_committer freebsd_triage 2018-06-14 23:44:13 UTC
Some historical context:

https://lists.freebsd.org/pipermail/svn-src-head/2013-March/045721.html

Due to discussion thread after this commit:

https://lists.freebsd.org/pipermail/svn-src-head/2013-March/045643.html
Comment 5 commit-hook freebsd_committer freebsd_triage 2018-06-21 10:52:22 UTC
A commit references this bug:

Author: ae
Date: Thu Jun 21 10:51:25 UTC 2018
New revision: 335473
URL: https://svnweb.freebsd.org/changeset/base/335473

Log:
  MFC r335133:
    In m_megapullup() use m_getjcl() to allocate 9k or 16k mbuf when requested.

    It is better to try allocate a big mbuf, than just silently drop a big
    packet. A better solution could be reworking of libalias modules to be
    able use m_copydata()/m_copyback() instead of requiring the single
    contiguous buffer.

    PR:		229006

Changes:
_U  stable/11/
  stable/11/sys/netinet/libalias/alias.c
Comment 6 commit-hook freebsd_committer freebsd_triage 2018-06-21 11:24:52 UTC
A commit references this bug:

Author: ae
Date: Thu Jun 21 11:24:21 UTC 2018
New revision: 335474
URL: https://svnweb.freebsd.org/changeset/base/335474

Log:
  MFC r335133:
    In m_megapullup() use m_getjcl() to allocate 9k or 16k mbuf when requested.

    It is better to try allocate a big mbuf, than just silently drop a big
    packet. A better solution could be reworking of libalias modules to be
    able use m_copydata()/m_copyback() instead of requiring the single
    contiguous buffer.

    PR:		229006

Changes:
_U  stable/10/
  stable/10/sys/netinet/libalias/alias.c
Comment 7 Andrey V. Elsukov freebsd_committer freebsd_triage 2018-06-21 11:25:11 UTC
Fixed in head/, stable/11 and stable/10. Thanks!
Comment 8 David Fullard 2018-06-22 23:57:10 UTC
Tested on a patched kernel sourced from FreeBSD 11.1-p11. Can confirm there is a significant improvement in network speeds and no packets are dropped.

I'd like to request an Errata Notice so to include it in the next security update.