git: 184291b0a56a - main - genet: pullup minimum header amount for IPv4

[Mike Karels]

The branch main has been updated by karels:

URL: https://cgit.FreeBSD.org/src/commit/?id=184291b0a56a68f2c0dc4388419b0f7d85a08cf4

commit 184291b0a56a68f2c0dc4388419b0f7d85a08cf4
Author:     Mike Karels <karels at FreeBSD.org>
AuthorDate: 2021-06-20 17:50:31 +0000
Commit:     Mike Karels <karels at FreeBSD.org>
CommitDate: 2021-06-26 16:04:02 +0000

    genet: pullup minimum header amount for IPv4
    
    The genet driver (RPi4 Ethernet) had code to pull headers into the
    first mbuf if there was only an Ethernet header there.  This was
    originally needed for ICMPv6 replies, then for forwarded IPv6/TCP.
    Now a situation has been found where it is needed for IPv4, when
    using NAT with IPFW.  Generalize to do this for all protocols.
    Rather than using an IPv6-related definition for the length, move
    the length to a variable that can be set with sysctl
    (hw.genet.tx_hdr_min).  Move an old tunable to a new RDTUN variable
    with a better name.
    
    PR:             25607
    MFC after:      3 days
    Reviewers:      emaste
    Differential Revision: https://reviews.freebsd.org/D30831
---
 sys/arm64/broadcom/genet/if_genet.c | 53 +++++++++++++++++++++----------------
 1 file changed, 30 insertions(+), 23 deletions(-)

diff --git a/sys/arm64/broadcom/genet/if_genet.c b/sys/arm64/broadcom/genet/if_genet.c
index 5d70c7075c3a..31fab0f9cf09 100644
--- a/sys/arm64/broadcom/genet/if_genet.c
+++ b/sys/arm64/broadcom/genet/if_genet.c
@@ -49,6 +49,7 @@ __FBSDID("$FreeBSD$");
 #include <sys/mbuf.h>
 #include <sys/socket.h>
 #include <sys/sockio.h>
+#include <sys/sysctl.h>
 #include <sys/module.h>
 #include <sys/taskqueue.h>
 #include <sys/gpio.h>
@@ -97,9 +98,27 @@ __FBSDID("$FreeBSD$");
 
 #define	TX_MAX_SEGS		20
 
-/* Maximum number of mbufs to send to if_input */
+static SYSCTL_NODE(_hw, OID_AUTO, genet, CTLFLAG_RD | CTLFLAG_MPSAFE, 0,
+    "genet driver parameters");
+
+/* Maximum number of mbufs to pass per call to if_input */
 static int gen_rx_batch = 16 /* RX_BATCH_DEFAULT */;
-TUNABLE_INT("hw.gen.rx_batch", &gen_rx_batch);
+SYSCTL_INT(_hw_genet, OID_AUTO, rx_batch, CTLFLAG_RDTUN,
+    &gen_rx_batch, 0, "max mbufs per call to if_input");
+
+TUNABLE_INT("hw.gen.rx_batch", &gen_rx_batch);	/* old name/interface */
+
+/*
+ * Transmitting packets with only an Ethernet header in the first mbuf
+ * fails.  Examples include reflected ICMPv6 packets, e.g. echo replies;
+ * forwarded IPv6/TCP packets; and forwarded IPv4/TCP packets that use NAT
+ * with IPFW.  Pulling up the sizes of ether_header + ip6_hdr + icmp6_hdr
+ * seems to work for both ICMPv6 and TCP over IPv6, as well as the IPv4/TCP
+ * case.
+ */
+static int gen_tx_hdr_min = 56;		/* ether_header + ip6_hdr + icmp6_hdr */
+SYSCTL_INT(_hw_genet, OID_AUTO, tx_hdr_min, CTLFLAG_RW,
+    &gen_tx_hdr_min, 0, "header to add to packets with ether header only");
 
 static struct ofw_compat_data compat_data[] = {
 	{ "brcm,genet-v1",		1 },
@@ -995,31 +1014,19 @@ gen_encap(struct gen_softc *sc, struct mbuf **mp)
 	m = *mp;
 
 	/*
-	 * Reflected ICMPv6 packets, e.g. echo replies, tend to get laid
-	 * out with only the Ethernet header in the first mbuf, and this
-	 * doesn't seem to work.  Forwarded TCP packets over IPv6 also
-	 * fail if laid out with only the Ethernet header in the first mbuf.
-	 * For now, pull up any IPv6 packet with that layout.  Maybe IPv4
-	 * needs it but we haven't run into it.  Pulling up the sizes of
-	 * ether_header + ip6_header + icmp6_hdr seems to work for both
-	 * ICMPv6 and TCP over IPv6.
+	 * Don't attempt to send packets with only an Ethernet header in
+	 * first mbuf; see comment above with gen_tx_hdr_min.
 	 */
-#define IP6_PULLUP_LEN (sizeof(struct ether_header) + \
-		        sizeof(struct ip6_hdr) + 8)
 	if (m->m_len == sizeof(struct ether_header)) {
-		int ether_type = mtod(m, struct ether_header *)->ether_type;
-		if (ntohs(ether_type) == ETHERTYPE_IPV6) {
-			m = m_pullup(m, MIN(m->m_pkthdr.len, IP6_PULLUP_LEN));
-			if (m == NULL) {
-				if (sc->ifp->if_flags & IFF_DEBUG)
-					device_printf(sc->dev,
-					    "IPV6 pullup fail\n");
-				*mp = NULL;
-				return (ENOMEM);
-			}
+		m = m_pullup(m, MIN(m->m_pkthdr.len, gen_tx_hdr_min));
+		if (m == NULL) {
+			if (sc->ifp->if_flags & IFF_DEBUG)
+				device_printf(sc->dev,
+				    "header pullup fail\n");
+			*mp = NULL;
+			return (ENOMEM);
 		}
 	}
-#undef IP6_PULLUP_LEN
 
 	if ((if_getcapenable(sc->ifp) & (IFCAP_TXCSUM | IFCAP_TXCSUM_IPV6)) !=
 	    0) {