igb and ALTQ in 9.1-rc3

[Barney Cordoba]

--- On Tue, 12/11/12, Karim Fodil-Lemelin <fodillemlinkarim at gmail.com> wrote:

> From: Karim Fodil-Lemelin <fodillemlinkarim at gmail.com>
> Subject: Re: igb and ALTQ in 9.1-rc3
> To: freebsd-net at freebsd.org
> Cc: nodens2099 at gmail.com
> Date: Tuesday, December 11, 2012, 9:56 AM
> On 11/12/2012 9:15 AM, Ermal Luçi
> wrote:
> > On Tue, Dec 11, 2012 at 2:05 PM, Barney Cordoba <barney_cordoba at yahoo.com>wrote:
> >
> >>
> >> --- On Tue, 12/11/12, Gleb Smirnoff <glebius at FreeBSD.org>
> wrote:
> >>
> >>> From: Gleb Smirnoff <glebius at FreeBSD.org>
> >>> Subject: Re: igb and ALTQ in 9.1-rc3
> >>> To: "Jack Vogel" <jfvogel at gmail.com>
> >>> Cc: "Clement Hermann (nodens)" <nodens2099 at gmail.com>,
> "Barney Cordoba"
> >> <barney_cordoba at yahoo.com>,
> freebsd-net at FreeBSD.org
> >>> Date: Tuesday, December 11, 2012, 2:58 AM
> >>> On Mon, Dec 10, 2012 at 03:31:19PM
> >>> -0800, Jack Vogel wrote:
> >>> J> UH, maybe asking the owner of the driver
> would help
> >>> :)
> >>> J>
> >>> J> ... and no, I've never been aware of
> doing anything to
> >>> stop supporting altq
> >>> J> so you wouldn't see any commits. If
> there's something
> >>> in the altq code or
> >>> J> support (which I have nothing to do with)
> that caused
> >>> this no-one informed
> >>> J> me.
> >>>
> >>> Switching from if_start to if_transmit
> effectively disables
> >>> ALTQ support.
> >>>
> >>> AFAIR, there is some magic implemented in other
> drivers that
> >>> makes them
> >>> modern (that means using if_transmit), but
> still capable to
> >>> switch to queueing
> >>> mode if SIOCADDALTQ was casted upon them.
> >>>
> >> It seems pretty difficult to say that something is
> compatible with
> >> something else if it hasn't been tested in a few
> years.
> >>
> >> It seems to me that ATLQ is the one that should
> handle if_transmit.
> >> although it's a good argument for having a raw
> "send" function in
> >> drivers. Ethernet drivers don't need more than a
> send() routing that
> >> loads a packet into the ring. The decision on what
> to do if you can't
> >> queue a packet should be in the  network
> layer, if we must still call
> >> things layers.
> >>
> >> "start" is a leftover from a day when you stuffed a
> buffer and waited
> >> for an interrupt to stuff in another. The whole
> idea is antiquated.
> >>
> >> Imagine drivers that pull packets off of a card and
> simply queue it;
> >> and that you simply submit a packet to be queued
> for transmit. Instead
> >> of trying to find 35 programmers that understand
> all of the lock BS,
> >> you only need to have a couple.
> >>
> >> I always disable all of the gobbledegook like
> checksum offloading. They
> >> just muddy the water and have very little effect on
> performance. A modern
> >> cpu can do a checksum as fast as you can manage the
> "capabilities" without
> >> disrupting the processing path.
> >>
> >> With FreeBSD, every driver is an experience. Some
> suck so bad that they
> >> should come with a warning. The MSK driver is
> completely useless, as
> >> an example.
> >>
> >>
> >> BC
> >> _______________________________________________
> >> freebsd-net at freebsd.org
> mailing list
> >> http://lists.freebsd.org/mailman/listinfo/freebsd-net
> >> To unsubscribe, send any mail to "freebsd-net-unsubscribe at freebsd.org"
> >>
> > During implementation of if_transmit altq was not
> considered at all.
> > The default if_transmit provides some compatibility but
> that is void since
> > altq has not been converted to call if_transmit after
> processing the mbuf.
> >
> > ALTQ can be adapted quite easily to if_transmit model
> it just wasn't done
> > at the time.
> > With if_transmit model it can even be modularized and
> not be a compile
> > kernel option since the queue of the iface is
> abstracted now.
> >
> > I have always wanted to do a diff but have not yet got
> to it.
> > The change is quite simple just provide an
> altq_transmit default method and
> > just hook into if_transmit model on the fly.
> > You surely need to handle some iface events and enable
> altq based on
> > request but its is not a hard to implement.
> >
> > I will always have this in my TODO but not sure when i
> can get to it.
> >
> The issue is not only that igb doesn't support if_transmit
> or if_start 
> method but that ALTQ isn't multiqueue ready and still uses
> the IFQ_LOCK 
> for all of its enqueue/dequeue operations. A simple drop in
> of 
> if_transmit is bound to cause race conditions on any
> multiqueue driver 
> with ALTQ.
> 
> I do have a patch set for this on igb but its ugly and needs
> more work 
> although it should get you going. Let me know if your
> interested I will 
> clean it up and send it over. For more information on ALTQ
> discussion 
> and igb please read this thread: 
> http://freebsd.1045724.n5.nabble.com/em-igb-if-transmit-drbr-and-ALTQ-td5760338.html
> 
> Best regards,
> 
> Karim.

At minimum, the drivers should make multiqueue an option, at least
until it works better than a single queue driver. Many MOBOs have igb
nics on board and such a mainstream NIC shouldn't be strapped using
experimental code that clearly isn't ready for prime time.

BC