From owner-freebsd-bugs@FreeBSD.ORG Tue Jan 17 23:00:21 2012 Return-Path: Delivered-To: freebsd-bugs@hub.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id C6D78106568D for ; Tue, 17 Jan 2012 23:00:21 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id 5209C8FC18 for ; Tue, 17 Jan 2012 23:00:19 +0000 (UTC) Received: from freefall.freebsd.org (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.5/8.14.5) with ESMTP id q0HN0JHp086230 for ; Tue, 17 Jan 2012 23:00:19 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.5/8.14.5/Submit) id q0HN0Jcr086229; Tue, 17 Jan 2012 23:00:19 GMT (envelope-from gnats) Resent-Date: Tue, 17 Jan 2012 23:00:19 GMT Resent-Message-Id: <201201172300.q0HN0Jcr086229@freefall.freebsd.org> Resent-From: FreeBSD-gnats-submit@FreeBSD.org (GNATS Filer) Resent-To: freebsd-bugs@FreeBSD.org Resent-Reply-To: FreeBSD-gnats-submit@FreeBSD.org, Dilip Chhetri Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 88827106566B for ; Tue, 17 Jan 2012 22:59:58 +0000 (UTC) (envelope-from nobody@FreeBSD.org) Received: from red.freebsd.org (red.freebsd.org [IPv6:2001:4f8:fff6::22]) by mx1.freebsd.org (Postfix) with ESMTP id 3FAA68FC17 for ; Tue, 17 Jan 2012 22:59:58 +0000 (UTC) Received: from red.freebsd.org (localhost [127.0.0.1]) by red.freebsd.org (8.14.4/8.14.4) with ESMTP id q0HMxvH8075114 for ; Tue, 17 Jan 2012 22:59:57 GMT (envelope-from nobody@red.freebsd.org) Received: (from nobody@localhost) by red.freebsd.org (8.14.4/8.14.4/Submit) id q0HMxvg4075113; Tue, 17 Jan 2012 22:59:57 GMT (envelope-from nobody) Message-Id: <201201172259.q0HMxvg4075113@red.freebsd.org> Date: Tue, 17 Jan 2012 22:59:57 GMT From: Dilip Chhetri To: freebsd-gnats-submit@FreeBSD.org X-Send-Pr-Version: www-3.1 Cc: Subject: kern/164265: tcp_lro_rx computes wrong checksum if 'csum' variable is 0 X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 17 Jan 2012 23:00:22 -0000 >Number: 164265 >Category: kern >Synopsis: tcp_lro_rx computes wrong checksum if 'csum' variable is 0 >Confidential: no >Severity: non-critical >Priority: medium >Responsible: freebsd-bugs >State: open >Quarter: >Keywords: >Date-Required: >Class: sw-bug >Submitter-Id: current-users >Arrival-Date: Tue Jan 17 23:00:19 UTC 2012 >Closed-Date: >Last-Modified: >Originator: Dilip Chhetri >Release: 7.2 >Organization: Panasas Inc >Environment: FreeBSD b81e46213f0590 7.2-RELEASE FreeBSD 7.2-RELEASE #17: Tue Jan 17 10:57:08 PST 2012 >Description: While modifying "bge" driver to use software LRO API provided by tcp_lro.c. I saw this piece of code, 224 /* Get the TCP checksum if we dont have it */ 225 if (!csum) 226 csum = tcp->th_sum; <=== this is wrong reading code further down in this same function, looks like "csum" is supposed to contain checksum of "TCP Header (with modified th_sum) + TCP Payload", therefore "csum = tcp->th_sum" assignment is WRONG. Since tcp_lro_flush() has this logic, 136 lro->m_head->m_pkthdr.csum_flags = CSUM_IP_CHECKED | 137 CSUM_IP_VALID | CSUM_DATA_VALID | CSUM_PSEUDO_HDR; 138 lro->m_head->m_pkthdr.csum_data = 0xffff; 139 lro->m_head->m_pkthdr.len = lro->len; tcp_input never looks at what is is in 'th_sum' field and things work fine (assuming tcp_lro_rx was called ONLY for mbuf's validated/good checksum). However, I don't know what else may broke due to wrong 'th_sum' filled in by tcp_lro_flush(). Even FreeBSD-9 has this same piece of logic. >How-To-Repeat: Setup, [client]<---->[FreeBSD server] FreeBSD server is using NIC with software LRO. use netperf to send packet from client to "freebsd server" and if you capture tcpdump on server side, you will see TCP packets with wrong checksum (wrong value of 'th_sum'), even though the packets are not corrupted. >Fix: refer patch file Patch attached with submission follows: --- sys/netinet/tcp_lro.c 2009-04-14 20:14:26.000000000 -0700 +++ /tmp/tcp_lro.c 2012-01-17 14:43:26.108463043 -0800 @@ -223,7 +223,23 @@ /* Get the TCP checksum if we dont have it */ if (!csum) - csum = tcp->th_sum; + /* Assuming, + * + * P = TCP Pseudo Header checksum + * H = TCP Header checksum (with th_sum 0) + * Hx= TCP Header checksum (with th_sum initialized) + * D = TCP Data checksum + * + * We are looking for checksum of (Hx + D). Since, + * tcp->th_sum = ~(P + H + D) + * (Hx + D) = (~th_sum + ~0x0 + th_sum + ~P) + * = 0xffff + (P ^ 0xffff) + */ + csum = in_pseudo(ip->ip_src.s_addr, ip->ip_dst.s_addr, + htonl(ntohs(ip->ip_len) - sizeof (*ip) + IPPROTO_TCP)); + csum = 0xffff + (csum ^ 0xffff); + csum = (csum >> 16) + (csum & 0xffff); + csum = (csum >> 16) + (csum & 0xffff); /* ensure no bits set besides ack or psh */ if ((tcp->th_flags & ~(TH_ACK | TH_PUSH)) != 0) >Release-Note: >Audit-Trail: >Unformatted: