From owner-freebsd-net@FreeBSD.ORG Fri Oct 8 15:15:52 2004 Return-Path: Delivered-To: freebsd-net@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id C09F516A4CE for ; Fri, 8 Oct 2004 15:15:52 +0000 (GMT) Received: from kane.otenet.gr (kane.otenet.gr [195.170.0.27]) by mx1.FreeBSD.org (Postfix) with ESMTP id C430C43D31 for ; Fri, 8 Oct 2004 15:15:49 +0000 (GMT) (envelope-from keramida@freebsd.org) Received: from orion.daedalusnetworks.priv (host5.bedc.ondsl.gr [62.103.39.229])i98FFRos011715 for ; Fri, 8 Oct 2004 18:15:41 +0300 Received: from orion.daedalusnetworks.priv (orion [127.0.0.1]) i98FFFif004901 for ; Fri, 8 Oct 2004 18:15:15 +0300 (EEST) (envelope-from keramida@freebsd.org) Received: (from keramida@localhost)i98FFFR2004900 for freebsd-net@freebsd.org; Fri, 8 Oct 2004 18:15:15 +0300 (EEST) (envelope-from keramida@freebsd.org) Date: Fri, 8 Oct 2004 18:15:15 +0300 From: Giorgos Keramidas To: freebsd-net@freebsd.org Message-ID: <20041008151515.GA3136@orion.daedalusnetworks.priv> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="CE+1k2dSO48ffgeK" Content-Disposition: inline Subject: Calling m_pullup in ether_input X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 08 Oct 2004 15:15:53 -0000 --CE+1k2dSO48ffgeK Content-Type: text/plain; charset=us-ascii Content-Disposition: inline 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 --CE+1k2dSO48ffgeK Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="ether_input.pullup.patch" 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, + ðer_smallframes,0,"Number of too small input frames"); +SYSCTL_INT(_net_link_ether, OID_AUTO, pfailed, CTLFLAG_RD, + ðer_pfailed,0,"Number of pkt pullup attempts that failed"); +#if defined(INET) || defined(INET6) +SYSCTL_INT(_net_link_ether, OID_AUTO, ipfw, CTLFLAG_RW, + ðer_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, - ðer_ipfw,0,"Pass ether pkts through firewall"); -#endif - #if 0 /* * This is for reference. We have a table-driven version --CE+1k2dSO48ffgeK--