Date: Tue, 17 Jan 2012 22:59:57 GMT From: Dilip Chhetri <dchhetri@panasas.com> To: freebsd-gnats-submit@FreeBSD.org Subject: kern/164265: tcp_lro_rx computes wrong checksum if 'csum' variable is 0 Message-ID: <201201172259.q0HMxvg4075113@red.freebsd.org> Resent-Message-ID: <201201172300.q0HN0Jcr086229@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
>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:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201201172259.q0HMxvg4075113>