Date: Thu, 8 Nov 2012 11:38:58 +0900 From: YongHyeon PYUN <pyunyh@gmail.com> To: Adrian Chadd <adrian@freebsd.org> Cc: FreeBSD Net <freebsd-net@freebsd.org>, Pyun YongHyeon <yongari@freebsd.org> Subject: Re: svn commit: r242739 - stable/9/sys/dev/ti Message-ID: <20121108023858.GA3127@michelle.cdnetworks.com> In-Reply-To: <CAJ-VmomEOPGbLwmOmL0EdenZA7QKbV5P-hAYsTRcwLao2LbAqg@mail.gmail.com> References: <201211080206.qA826RiN054539@svn.freebsd.org> <CAJ-VmomEOPGbLwmOmL0EdenZA7QKbV5P-hAYsTRcwLao2LbAqg@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
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 <yongari@freebsd.org> 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 <sys/sysctl.h> > > > > -#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++) {
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20121108023858.GA3127>