From owner-freebsd-net@FreeBSD.ORG Thu Nov 8 02:39:12 2012 Return-Path: Delivered-To: freebsd-net@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id B571EC07; Thu, 8 Nov 2012 02:39:12 +0000 (UTC) (envelope-from pyunyh@gmail.com) Received: from mail-da0-f54.google.com (mail-da0-f54.google.com [209.85.210.54]) by mx1.freebsd.org (Postfix) with ESMTP id 7002B8FC08; Thu, 8 Nov 2012 02:39:12 +0000 (UTC) Received: by mail-da0-f54.google.com with SMTP id z9so1026259dad.13 for ; Wed, 07 Nov 2012 18:39:12 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=from:date:to:cc:subject:message-id:reply-to:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; bh=OsP6Dc1/UrKwNwmjmNMKXoYTShtfJoECeOW8Zc36O+o=; b=jpmGH5X5A6T0n4cll05PEgi7xgCcTGY9/0AvL/xoVAJRkG/yDeIC7lBAM/yGZtF0u/ tP+E7jAIhJn6tXndWdHcO5yyyzB0b1xTA5ybi7MF/xhLQttb2zn6fgn6lXOZxjTfhmCI OMW9mh4wABSjhO4a/UD3ixuuaPcsENwxngYUObN7lI1j8ZicvFTWLhHcFN3xAKeGnM3k r3A527kiPgBYaIBtg+VC8u7W7l5IMmH1VnEP8gb7PiMdLUfLl2Ma2Gm3buFUGmSrFLlB SVCz+hukfiGesxM8ZX61tRzH0sWPUECN8wCCalnaTx8HSo3t8sClEZG3X2ZNOGjH2OKz buUw== Received: by 10.66.88.4 with SMTP id bc4mr17954266pab.42.1352342351905; Wed, 07 Nov 2012 18:39:11 -0800 (PST) Received: from pyunyh@gmail.com (lpe4.p59-icn.cdngp.net. [114.111.62.249]) by mx.google.com with ESMTPS id o5sm15248795paz.32.2012.11.07.18.39.08 (version=TLSv1/SSLv3 cipher=OTHER); Wed, 07 Nov 2012 18:39:10 -0800 (PST) Received: by pyunyh@gmail.com (sSMTP sendmail emulation); Thu, 08 Nov 2012 11:38:58 +0900 From: YongHyeon PYUN Date: Thu, 8 Nov 2012 11:38:58 +0900 To: Adrian Chadd Subject: Re: svn commit: r242739 - stable/9/sys/dev/ti Message-ID: <20121108023858.GA3127@michelle.cdnetworks.com> References: <201211080206.qA826RiN054539@svn.freebsd.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.4.2.3i Cc: FreeBSD Net , Pyun YongHyeon X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list Reply-To: pyunyh@gmail.com List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 08 Nov 2012 02:39:13 -0000 On Wed, Nov 07, 2012 at 06:15:30PM -0800, Adrian Chadd wrote: > So I am curious - did this give a real benefit? In 3.x/4.x days it surely have had helped a lot, I guess mainly because the CPU was not fast enough to saturate the link with software checksum(i.e. NFS over UDP). Generally I prefer correctness to performance and it seems there is no easy way to get full advantage of TCP/UDP checksum offloading of controller on fragmented IP packets on FreeBSD 8+. So I disabled it to reduce the chance of generating corrupted packets. > > If so, may I suggest we perhaps accelerate discussing if_transmit() of > multiple frames per call? Hmm, actually I'm still not a fan of if_transmit() at this moment. Honestly I don't have good queuing code in driver to handle queue full condition. Interactions with altq(9) is also one of my concern as well as packet reordering issue of drbr(9) interface. > That would allow features like this to be re-enabled. > > > > Adrian > > On 7 November 2012 18:06, Pyun YongHyeon wrote: > > Author: yongari > > Date: Thu Nov 8 02:06:27 2012 > > New Revision: 242739 > > URL: http://svnweb.freebsd.org/changeset/base/242739 > > > > Log: > > MFC r242425: > > Remove TCP/UDP checksum offloading feature for IP fragmented > > datagrams. Traditionally upper stack fragmented packets without > > computing TCP/UDP checksum and these datagrams were passed to > > driver. But there are chances that other packets slip into the > > interface queue in SMP world. If this happens firmware running on > > MIPS 4000 processor in the controller would see mixed packets and > > it shall send out corrupted packets. > > While I'm here simplify checksum offloading setup. > > > > Modified: > > stable/9/sys/dev/ti/if_ti.c > > Directory Properties: > > stable/9/sys/ (props changed) > > stable/9/sys/dev/ (props changed) > > > > Modified: stable/9/sys/dev/ti/if_ti.c > > ============================================================================== > > --- stable/9/sys/dev/ti/if_ti.c Thu Nov 8 02:01:04 2012 (r242738) > > +++ stable/9/sys/dev/ti/if_ti.c Thu Nov 8 02:06:27 2012 (r242739) > > @@ -127,7 +127,7 @@ __FBSDID("$FreeBSD$"); > > > > #include > > > > -#define TI_CSUM_FEATURES (CSUM_IP | CSUM_TCP | CSUM_UDP | CSUM_IP_FRAGS) > > +#define TI_CSUM_FEATURES (CSUM_IP | CSUM_TCP | CSUM_UDP) > > /* > > * We can only turn on header splitting if we're using extended receive > > * BDs. > > @@ -3083,16 +3083,10 @@ ti_encap(struct ti_softc *sc, struct mbu > > > > m = *m_head; > > csum_flags = 0; > > - if (m->m_pkthdr.csum_flags) { > > - if (m->m_pkthdr.csum_flags & CSUM_IP) > > - csum_flags |= TI_BDFLAG_IP_CKSUM; > > - if (m->m_pkthdr.csum_flags & (CSUM_TCP | CSUM_UDP)) > > - csum_flags |= TI_BDFLAG_TCP_UDP_CKSUM; > > - if (m->m_flags & M_LASTFRAG) > > - csum_flags |= TI_BDFLAG_IP_FRAG_END; > > - else if (m->m_flags & M_FRAG) > > - csum_flags |= TI_BDFLAG_IP_FRAG; > > - } > > + if (m->m_pkthdr.csum_flags & CSUM_IP) > > + csum_flags |= TI_BDFLAG_IP_CKSUM; > > + if (m->m_pkthdr.csum_flags & (CSUM_TCP | CSUM_UDP)) > > + csum_flags |= TI_BDFLAG_TCP_UDP_CKSUM; > > > > frag = sc->ti_tx_saved_prodidx; > > for (i = 0; i < nseg; i++) {