Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 13 Sep 2017 06:45:33 -0700
From:      Cy Schubert <Cy.Schubert@komquats.com>
To:        Warner Losh <imp@bsdimp.com>
Cc:        Cy Schubert <Cy.Schubert@komquats.com>, Ngie Cooper <yaneurabeya@gmail.com>, Stephen Hurd <shurd@freebsd.org>, src-committers <src-committers@freebsd.org>, "svn-src-all@freebsd.org" <svn-src-all@freebsd.org>, "svn-src-head@freebsd.org" <svn-src-head@freebsd.org>
Subject:   Re: svn commit: r323516 - in head/sys: dev/bnxt dev/e1000 kern net  sys
Message-ID:  <201709131345.v8DDjXH0033179@slippy.cwsent.com>
In-Reply-To: Message from Warner Losh <imp@bsdimp.com> of "Wed, 13 Sep 2017 07:35:46 -0600." <CANCZdfoDjhY0HW%2BYuErVDRJ0YC4SPXTL0GBYFrTJvpax0KcmTA@mail.gmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help
In message <CANCZdfoDjhY0HW+YuErVDRJ0YC4SPXTL0GBYFrTJvpax0KcmTA@mail.gmail.c
om>
, Warner Losh writes:
> --001a1141f15ac444250559123c56
> Content-Type: text/plain; charset="UTF-8"
> 
> On Wed, Sep 13, 2017 at 1:11 AM, Cy Schubert <Cy.Schubert@komquats.com>
> wrote:
> 
> > In message <4D5AE475-5A38-4429-8B71-DBECDFB0A1FF@gmail.com>, Ngie Cooper
> > writes
> > :
> > >
> > > > On Sep 12, 2017, at 18:18, Stephen Hurd <shurd@FreeBSD.org> wrote:
> > > >
> > > > Author: shurd
> > > > Date: Wed Sep 13 01:18:42 2017
> > > > New Revision: 323516
> > > > URL: https://svnweb.freebsd.org/changeset/base/323516
> > > >
> > > > Log:
> > > >  Roll up iflib commits from github.  This pulls in most of the work
> > done
> > > >  by Matt Macy as well as other changes which he has accepted via pull
> > > >  request to his github repo at https://github.com/mattmacy/networking/
> > > >
> > > >  This should bring -CURRENT and the github repo into close enough sync
> > to
> > > >  allow small feature branches rather than a large chain of
> > interdependant
> > > >  patches being developed out of tree.  The reset of the synchronization
> > > >  should be able to be completed on github by splitting the remaining
> > > >  changes that are not yet ready into short feature branches for later
> > > >  review as smaller commits.
> > > >
> > > >  Here is a summary of changes included in this patch:
> > > >
> > > >  1)  More checks when INVARIANTS are enabled for eariler problem
> > > >      detection
> > > >  2)  Group Task Queue cleanups
> > > >      - Fix use of duplicate shortdesc for gtaskqueue malloc type.
> > > >        Some interfaces such as memguard(9) use the short description to
> > > >        identify malloc types, so duplicates should be avoided.
> > > >  3)  Allow gtaskqueues to use ithreads in addition to taskqueues
> > > >      - In some cases, this can improve performance
> > > >  4)  Better logging when taskqgroup_attach*() fails to set interrupt
> > > >      affinity.
> > > >  5)  Do not start gtaskqueues until they're needed
> > > >  6)  Have mp_ring enqueue function enter the ABDICATED rather than BUSY
> > > >      state.  This moves the TX to the gtaskq and allows processing to
> > > >      continue faster as well as make TX batching more likely.
> > > >  7)  Add an ift_txd_errata function to struct if_txrx.  This allows
> > > >      drivers to inspect/modify mbufs before transmission.
> > > >  8)  Add a new IFLIB_NEED_ZERO_CSUM for drivers to indicate they need
> > > >      checksums zeroed for checksum offload to work.  This avoids
> > modifying
> > > >      packet data in the TX path when possible.
> > > >  9)  Use ithreads for iflib I/O instead of taskqueues
> > > >  10) Clean up ioctl and support async ioctl functions
> > > >  11) Prefetch two cachlines from each mbuf instead of one up to 128B.
> > We
> > > >      often need to parse packet header info beyond 64B.
> > > >  12) Fix potential memory corruption due to fence post error in
> > > >      bit_nclear() usage.
> > > >  13) Improved hang detection and handling
> > > >  14) If the packet is smaller than MTU, disable the TSO flags.
> > > >      This avoids extra packet parsing when not needed.
> > > >  15) Move TCP header parsing inside the IS_TSO?() test.
> > > >      This avoids extra packet parsing when not needed.
> > > >  16) Pass chains of mbufs that are not consumed by lro to if_input()
> > > >      rather call if_input() for each mbuf.
> > > >  17) Re-arrange packet header loads to get as much work as possible
> > done
> > > >      before a cache stall.
> > > >  18) Lock the context when calling IFDI_ATTACH_PRE()/IFDI_ATTACH_
> > POST()/
> > > >      IFDI_DETACH();
> > > >  19) Attempt to distribute RX/TX tasks across cores more sensibly,
> > > >      especially when RX and TX share an interrupt.  RX will attempt to
> > > >      take the first threads on a core, and TX will attempt to take
> > > >      successive threads.
> > > >  20) Allow iflib_softirq_alloc_generic() to request affinity to the
> > same
> > > >      cpus an interrupt has affinity with.  This allows TX queues to
> > > >      ensure they are serviced by the socket the device is on.
> > > >  21) Add new iflib sysctls to net.iflib:
> > > >      - timer_int - interval at which to run per-queue timers in ticks
> > > >      - force_busdma
> > > >  22) Add new per-device iflib sysctls to dev.X.Y.iflib
> > > >      - rx_budget allows tuning the batch size on the RX path
> > > >      - watchdog_events Count of watchdog events seen since load
> > > >  23) Fix error where netmap_rxq_init() could get called before
> > > >      IFDI_INIT()
> > > >  24) e1000: Fixed version of r323008: post-cold sleep instead of DELAY
> > > >      when waiting for firmware
> > > >      - After interrupts are enabled, convert all waits to sleeps
> > > >      - Eliminates e1000 software/firmware synchronization busy waits
> > after
> > > >        startup
> > > >  25) e1000: Remove special case for budget=1 in em_txrx.c
> > > >      - Premature optimization which may actually be incorrect with
> > > >        multi-segment packets
> > > >  26) e1000: Split out TX interrupt rather than share an interrupt for
> > > >      RX and TX.
> > > >      - Allows better performance by keeping RX and TX paths separate
> > > >  27) e1000: Separate igb from em code where suitable
> > > >      Much easier to understand separate functions and "if (is_igb)"
> > than
> > > >      previous tests like "if (reg_icr & (E1000_ICR_RXSEQ |
> > E1000_ICR_LSC))"
> > > >
> > > >  #blamebruno
> > > >
> > > >  Reviewed by:    sbruno
> > > >  Approved by:    sbruno (mentor)
> > > >  Sponsored by:    Limelight Networks
> > > >  Differential Revision:    https://reviews.freebsd.org/D12235
> > >
> > > *gasps at the LoC count and number of changed drivers*
> > >
> > > Could someone please better break this up in the future..?
> >
> > Agreed. Down the road parsing out individual commits in this jumbo commit
> > will be difficult to parse. IMO this may as well have simply been, import
> > from https://github.com/mattmacy/networking/, just like Mr. Torvalds does
> > at kernel.org. I expect to see a commit like this there but not here. Can
> > we break this down to its functional commits?
> 
> 
> I use git + svn for other things. It should be possible to use that to push
> the changes in 'machine gun style' (meaning 27 changes pushed so fast they
> show up as 27 consecutive r numbers). I do change curation, etc with it so
> I get good sized commits. That's why you'll often see 10 or 20 commits from
> me show up in a matter of a couple of minutes. git svn dcommit is to blame,
> but so is about two dozen git rebases and other git crazy along the way.
> Something like that would be useful here. I'd be quite happy to walk people
> through my setup if it helps to reduce this.

I'm interested.


-- 
Cheers,
Cy Schubert <Cy.Schubert@cschubert.com>
FreeBSD UNIX:  <cy@FreeBSD.org>   Web:  http://www.FreeBSD.org

	The need of the many outweighs the greed of the few.






Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201709131345.v8DDjXH0033179>