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


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

_______________________________________________
[EMAIL PROTECTED] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "[EMAIL PROTECTED]"