From owner-freebsd-stable@FreeBSD.ORG Sun Feb 8 14:33:33 2009 Return-Path: Delivered-To: freebsd-stable@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id EEC4B106568E for ; Sun, 8 Feb 2009 14:33:33 +0000 (UTC) (envelope-from rwatson@FreeBSD.org) Received: from cyrus.watson.org (cyrus.watson.org [65.122.17.42]) by mx1.freebsd.org (Postfix) with ESMTP id AE8FD8FC22 for ; Sun, 8 Feb 2009 14:33:33 +0000 (UTC) (envelope-from rwatson@FreeBSD.org) Received: from fledge.watson.org (fledge.watson.org [65.122.17.41]) by cyrus.watson.org (Postfix) with ESMTPS id 5CF9946B17; Sun, 8 Feb 2009 09:33:33 -0500 (EST) Date: Sun, 8 Feb 2009 14:33:33 +0000 (GMT) From: Robert Watson X-X-Sender: robert@fledge.watson.org To: Danny Braniss In-Reply-To: Message-ID: References: <20090208091656.GA31876@test71.vk2pj.dyndns.org> <20090208104253.GB31876@test71.vk2pj.dyndns.org> User-Agent: Alpine 2.00 (BSF 1167 2008-08-23) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: Peter Jeremy , freebsd-stable@freebsd.org Subject: Re: impossible packet length ... X-BeenThere: freebsd-stable@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Production branch of FreeBSD source code List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 08 Feb 2009 14:33:34 -0000 On Sun, 8 Feb 2009, Danny Braniss wrote: >> On Sun, 8 Feb 2009, Danny Braniss wrote: >> >>> looking at the bce source, it's not clear (to me :-). If errors are >>> detected in bce_rx_intr(), the packet gets dropped, which I would expect >>> to be the treatment of an offloded chekcum error, but it seems that is not >>> the case. >> >> I think we're thinking of different checksums -- devices/device drivers >> drop frames with bad ethernet checksums, but not IP and above layer >> checksums. > > I know I'm stepping on thin ice hear - haven't touched Stevens for a while, > (and I doubt it mentions offloading), but if the offload checksum is bad, > why not just drop the packet? > > The way I read the driver, if the offload checksum is on, and if no errors > where detected, then it's marked as ok. There are a few good reasons I can think of, but this is hardly a comprehensive list: (1) If there are bad higher level checksums on the wire, you want to see them in tcpdump, so allow them to get up to a higher layer if network layer checksums aren't good. (2) It's a matter of local policy as to whether UDP checksums (for example) are observed or not. (3) If you're forwarding or bridging packets, it should be up to the end nodes how they deal with bad UDP checksums on packets to them, not the routers. Looking at if_bce.c, the following seems to be reasonable logic; first, ethernet-layer checksums: 5902 /* Check the received frame for errors. */ 5903 if (status & (L2_FHDR_ERRORS_BAD_CRC | 5904 L2_FHDR_ERRORS_PHY_DECODE | L2_FHDR_ERRORS_ALIGNMENT | 5905 L2_FHDR_ERRORS_TOO_SHORT | L2_FHDR_ERRORS_GIANT_FRAME)) { 5906 5907 /* Log the error and release the mbuf. */ 5908 ifp->if_ierrors++; 5909 DBRUN(sc->l2fhdr_status_errors++); 5910 5911 m_freem(m0); 5912 m0 = NULL; 5913 goto bce_rx_int_next_rx; 5914 } I.e., if there are ethernet-level CRC failures, drop the packet. 5922 /* Validate the checksum if offload enabled. */ 5923 if (ifp->if_capenable & IFCAP_RXCSUM) { 5924 5925 /* Check for an IP datagram. */ 5926 if (!(status & L2_FHDR_STATUS_SPLIT) && 5927 (status & L2_FHDR_STATUS_IP_DATAGRAM)) { 5928 m0->m_pkthdr.csum_flags |= CSUM_IP_CHECKED; 5929 5930 /* Check if the IP checksum is valid. */ 5931 if ((l2fhdr->l2_fhdr_ip_xsum ^ 0xffff) == 0) 5932 m0->m_pkthdr.csum_flags |= CSUM_IP_VALID; 5933 } 5934 5935 /* Check for a valid TCP/UDP frame. */ 5936 if (status & (L2_FHDR_STATUS_TCP_SEGMENT | 5937 L2_FHDR_STATUS_UDP_DATAGRAM)) { 5938 5939 /* Check for a good TCP/UDP checksum. */ 5940 if ((status & (L2_FHDR_ERRORS_TCP_XSUM | 5941 L2_FHDR_ERRORS_UDP_XSUM)) == 0) { 5942 m0->m_pkthdr.csum_data = 5943 l2fhdr->l2_fhdr_tcp_udp_xsum; 5944 m0->m_pkthdr.csum_flags |= (CSUM_DATA_VALID 5945 | CSUM_PSEUDO_HDR); 5946 } 5947 } 5948 } Only look at higher level checksums if policy enables it on the interface; then, only if the hardware has a view on the IP-layer checksums, propagte that information to the mbuf flags from the descriptor ring entry flags, both whether or not the checksum was verified, and whether or not it was good. If policy disables it, or the hardware expresses no view, we don't set flags, which simply defers checksumming to a higher layer (if required -- for forwarded packets, we won't test UDP-layer checksums at all). Robert N M Watson Computer Laboratory University of Cambridge