svn commit: r242739 - stable/9/sys/dev/ti

[YongHyeon PYUN]

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 at 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++) {