Calling m_pullup in ether_input

[Giorgos Keramidas]

In ether_input() we unconditionaly discard the mbufs whose m_len is less than
ETHER_HDR_LEN.  A bit higher M_PKTHDR has been checked but the check made
before discarding the frame doesn't pay attention to the m->m_pkthdr.len (the
total packet length).

I am trying to find out how often this happens, by using the attached patch to
count the number of small frames received in ether_input() and the number of
failed m_pullup() attempts that result from that.

Does this change seem reasonable as an instrumentation  of the particular
problem or am I unknowingly breaking something in the way ether_input() is
supposed to work?

- Giorgos

-------------- next part --------------
Index: if_ethersubr.c
===================================================================
RCS file: /home/ncvs/src/sys/net/if_ethersubr.c,v
retrieving revision 1.177
diff -u -u -r1.177 if_ethersubr.c
--- if_ethersubr.c	27 Jul 2004 23:20:45 -0000	1.177
+++ if_ethersubr.c	8 Oct 2004 15:10:40 -0000
@@ -124,6 +124,20 @@
 static int ether_ipfw;
 #endif
 
+static int ether_smallframes;	/* Number of too small input packets. */
+static int ether_pfailed;	/* Failed m_pullup attempts. */
+
+SYSCTL_DECL(_net_link);
+SYSCTL_NODE(_net_link, IFT_ETHER, ether, CTLFLAG_RW, 0, "Ethernet");
+SYSCTL_INT(_net_link_ether, OID_AUTO, smallframes, CTLFLAG_RD,
+	    &ether_smallframes,0,"Number of too small input frames");
+SYSCTL_INT(_net_link_ether, OID_AUTO, pfailed, CTLFLAG_RD,
+	    &ether_pfailed,0,"Number of pkt pullup attempts that failed");
+#if defined(INET) || defined(INET6)
+SYSCTL_INT(_net_link_ether, OID_AUTO, ipfw, CTLFLAG_RW,
+	    &ether_ipfw,0,"Pass ether pkts through firewall");
+#endif
+
 /*
  * Ethernet output routine.
  * Encapsulate a packet of type family for the local net.
@@ -482,6 +496,7 @@
 ether_input(struct ifnet *ifp, struct mbuf *m)
 {
 	struct ether_header *eh;
+	int mlen, plen;
 	u_short etype;
 
 	/*
@@ -494,14 +509,26 @@
 		m_freem(m);
 		return;
 	}
+	/*
+	 * If the first mbuf of the chain doesn't have at least ETHER_HDR_LEN
+	 * bytes check the m->m_pkthdr.len to see if the entire packet holds
+	 * more than ETHER_HDR_LEN data and attempt a pullup if it does.
+	 */
+	mlen = m->m_len;
+	plen = m->m_pkthdr.len;
 	if (m->m_len < ETHER_HDR_LEN) {
-		/* XXX maybe should pullup? */
-		if_printf(ifp, "discard frame w/o leading ethernet "
-				"header (len %u pkt len %u)\n",
-				m->m_len, m->m_pkthdr.len);
-		ifp->if_ierrors++;
-		m_freem(m);
-		return;
+		ether_smallframes++;
+		if (m->m_pkthdr.len < ETHER_HDR_LEN ||
+		    ((m = m_pullup(m, ETHER_HDR_LEN)) == NULL)) {
+			if_printf(ifp, "discard frame w/o leading ethernet "
+			    "header (len %u pkt len %u)\n", mlen, plen);
+			ifp->if_ierrors++;
+			if (m)
+				m_freem(m);
+			else
+				ether_pfailed++;
+			return;
+		}
 	}
 	eh = mtod(m, struct ether_header *);
 	etype = ntohs(eh->ether_type);
@@ -906,13 +933,6 @@
 		bdgtakeifaces_ptr();
 }
 
-SYSCTL_DECL(_net_link);
-SYSCTL_NODE(_net_link, IFT_ETHER, ether, CTLFLAG_RW, 0, "Ethernet");
-#if defined(INET) || defined(INET6)
-SYSCTL_INT(_net_link_ether, OID_AUTO, ipfw, CTLFLAG_RW,
-	    &ether_ipfw,0,"Pass ether pkts through firewall");
-#endif
-
 #if 0
 /*
  * This is for reference.  We have a table-driven version