From owner-freebsd-net@FreeBSD.ORG Mon May 12 09:54:52 2014 Return-Path: Delivered-To: freebsd-net@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 12120685; Mon, 12 May 2014 09:54:52 +0000 (UTC) Received: from mail-n.franken.de (drew.ipv6.franken.de [IPv6:2001:638:a02:a001:20e:cff:fe4a:feaa]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client CN "mail-n.franken.de", Issuer "Thawte DV SSL CA" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 967452092; Mon, 12 May 2014 09:54:51 +0000 (UTC) Received: from [10.225.7.42] (unknown [194.95.73.101]) (Authenticated sender: macmic) by mail-n.franken.de (Postfix) with ESMTP id 61F4E1C10491D; Mon, 12 May 2014 11:54:49 +0200 (CEST) Content-Type: text/plain; charset=us-ascii Mime-Version: 1.0 (Mac OS X Mail 7.2 \(1874\)) Subject: Re: RX checksum offloading problem From: Michael Tuexen In-Reply-To: <20140512014502.GB4085@michelle.cdnetworks.com> Date: Mon, 12 May 2014 11:54:47 +0200 Content-Transfer-Encoding: quoted-printable Message-Id: References: <0EB8F4F6-65C2-4B90-8101-FCC53A15C6F9@lurchi.franken.de> <20140507075612.GA1376@michelle.cdnetworks.com> <36469814-FAC8-4172-A792-487E2AB8ECB9@lurchi.franken.de> <20140507083751.GB1376@michelle.cdnetworks.com> <415C1CB5-3AF9-44E4-943A-74116037980E@lurchi.franken.de> <20140509013556.GA3014@michelle.cdnetworks.com> <20140512014502.GB4085@michelle.cdnetworks.com> To: pyunyh@gmail.com X-Mailer: Apple Mail (2.1874) Cc: FreeBSD Net , "Bjoern A. Zeeb" X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 12 May 2014 09:54:52 -0000 On 12 May 2014, at 03:45, Yonghyeon PYUN wrote: > On Fri, May 09, 2014 at 04:22:36PM +0200, Michael Tuexen wrote: >>=20 >> On 09 May 2014, at 12:46, Michael Tuexen = wrote: >>=20 >>> On 09 May 2014, at 03:35, Yonghyeon PYUN wrote: >>>=20 >>>> On Thu, May 08, 2014 at 08:40:22PM +0200, Michael Tuexen wrote: >>>>> On 07 May 2014, at 10:37, Yonghyeon PYUN wrote: >>>>>=20 >>>>>> On Wed, May 07, 2014 at 10:07:09AM +0200, Michael Tuexen wrote: >>>>>>> On 07 May 2014, at 09:56, Yonghyeon PYUN = wrote: >>>>>>>=20 >>>>>>>> On Sat, May 03, 2014 at 11:52:47AM +0200, Michael Tuexen wrote: >>>>>>>>> On 02 May 2014, at 16:02, Bjoern A. Zeeb = wrote: >>>>>>>>>=20 >>>>>>>>>>=20 >>>>>>>>>> On 02 May 2014, at 10:22 , Michael Tuexen = wrote: >>>>>>>>>>=20 >>>>>>>>>>> Dear all, >>>>>>>>>>>=20 >>>>>>>>>>> during testing I found that FreeBSD head (on a raspberry pi) = accepts SCTP packet >>>>>>>>>>> with bad checksums. After debugging this I figured out that = this is a problem with >>>>>>>>>>> the csum_flags defined in mbuf.h. >>>>>>>>>>>=20 >>>>>>>>>>> The SCTP code on its input path checks for CSUM_SCTP_VALID, = which is defined in mbuf.h: >>>>>>>>>>> #define CSUM_SCTP_VALID CSUM_L4_VALID >>>>>>>>>>> This makes sense: If CSUM_SCTP_VALID is set in csum_flags, = the packet is considered >>>>>>>>>>> to have a correct checksum. >>>>>>>>>>>=20 >>>>>>>>>>> For UDP and TCP some drivers calculate the UDP/TCP checksum = and set CSUM_DATA_VALID in >>>>>>>>>>> csum_flags to indicate that the UDP/TCP should consider = csum_data to figure out if >>>>>>>>>>> the packet has a correct checksum. The problem is that = CSUM_DATA_VALID is defined as >>>>>>>>>>> #define CSUM_DATA_VALID CSUM_L4_VALID >>>>>>>>>>> In this case the semantic is not that the packet has a valid = checksum, but the csum_data >>>>>>>>>>> field contains information. >>>>>>>>>>>=20 >>>>>>>>>>> Now the following happens (on the raspberry pi the driver = used is >>>>>>>>>>> dev/usb/net/if_smsc.c >>>>>>>>>>>=20 >>>>>>>>>>> 1. A packet is received and if it is not too short, the = checksum computed >>>>>>>>>>> is stored in csum_data and the flag CSUM_DATA_VALID is set. = This happens >>>>>>>>>>> for all IP packets, not only for UDP and TCP packets. >>>>>>>>>>> 2. In case of SCTP packets, the SCTP interprets = CSUM_DATA_VALID as CSUM_SCTP_VALID >>>>>>>>>>> and accepts the packet. So no SCTP checksum check ever = happened. >>>>>>>>>>>=20 >>>>>>>>>>> Alternatives to fix this: >>>>>>>>>>>=20 >>>>>>>>>>> 1. Change all drivers to set CSUM_DATA_VALID only in case of = UDP or TCP packets, since >>>>>>>>>>> it only makes sense in these cases. >>>>>>>>>>=20 >>>>>>>>>> Wait, or for SCTP in cad the crc32 (I think it was) was = actually checked but not otherwise. This is how it should be imho. It = seems like a driver bug. >>>>>>>>> I went through the list of drivers and you are right, it seems = to be a bug >>>>>>>>> in if_smsc.c. Most of the other drivers check for UDP/TCP, a = small set I can't tell. >>>>>>>>>=20 >>>>>>>>=20 >>>>>>>> I'm not sure how the controller computes TCP/UDP checksum = values. >>>>>>>> It seems the publicly available data sheet was highly sanitized = so >>>>>>>> it was useless to me. The comment in the driver says that the >>>>>>> Same for me... >>>>>>>> controller computes RX checksum after the IPv4 header to the = end of >>>>>>>> ethernet frame. After seeing that comment, three questions = popped >>>>>>>> up: >>>>>>>>=20 >>>>> OK, I did some testing. It looks like the card is just computing = the >>>>> checksum over the IP payload taking the correct IP header length = into account. >>>>>>>> 1. Is the controller smart enough to skip IP options header in >>>>>>>> TCP/UDP checksum offloading? >>>>> Yes, I can send fragmented and un-fragmented UDP packets with IP = options >>>>> and they are handled correctly. Even if the last fragment is too = short. >>>>=20 >>>> I'm assuming you're taking about receiving fragmented UDP packets >>>> with RX checksum offloading, right? >>> Correct. >>>>=20 >>>>>>>> 2. How controller handles UDP checksum value 0x0000(i.e. sender >>>>>>>> didn't compute UDP checksum)? >>>>> This case isn't handled. However, udp_input() looks first for zero = checksums >>>>> and only after that in the csum_flags. So it doesn't result in any = problems. >>>>> Would you prefer not to set CSUM_DATA_VALID in this case? >>>>=20 >>>> At least, it correctly updates UDP stats of netstat(1). >>> Let me double check that... >> I double checked it. The statistic counters are incremented. >> Please note that we had a bug in the sending code of head, which >> made it impossible to send UDP packets with 0 checksum. That is >> fixed in >> http://svnweb.freebsd.org/base?view=3Drevision&revision=3D265776 >=20 > Thanks. >=20 >>=20 >> So any preference whether to report CSUM_DATA_VALID if a UDP packet >> with checksum 0 is received or not? I'm pretty open, since it does >> not have any effect right now... >>=20 >=20 > Because upper stack correctly counts for these packets, your change > (report CSUM_DATA_VALID for UDP packet with checksum value 0) looks > fine. I don't remember how pf/ipf handles that case though but we > can easily fix pf/ipf once we see breakage. OK. Best regards Michael >=20 >> Best regards >> Michael >=20