A small fix for if_em.c, if_igb.c, if_ixgbe.c

[Yonghyeon PYUN]

On Mon, Dec 02, 2013 at 03:56:59PM -0500, Randall Stewart wrote:
> 
> On Dec 1, 2013, at 9:23 PM, Yonghyeon PYUN wrote:
> 
> > On Fri, Nov 29, 2013 at 06:24:12PM +0100, Michael Tuexen wrote:
> >> Dear all,
> >> 
> >> ifnet(9) says regarding if_transmit():
> >> 
> >> Transmit a packet on an interface or queue it if the interface is
> >> in use.  This function will return ENOBUFS if the devices software
> >> and hardware queues are both full.
> >> 
> >> The drivers for em, igb and ixgbe might also return an error even
> >> in the case the packet was enqueued. The attached patches fix this
> >> issue.
> > 
> > How do you know the packet is successfully enqueued but driver
> > returns an error?  Do non-buf-ring-aware drivers also show the same
> > behavior?
> > 
> All of the drivers have traditionally (from what I can tell
> and all the ones I have poked at) no matter if they are the new
> format (with ring-buf) or the old, would only return an error in
> the enqueue if we hit the limit.
> 
> The driver down the road can in theory drop the packet for other
> reasons (errors etc) and there is no communication back up to the
> upper layers that this occurred.
> 

Hmm, I was under the impression that buf_ring changed old behavior
we had in the past.  Before introduction of if_transmit, queuing
was done in upper layer so returning an error in driver's TX path
didn't affect upper layer.  With if_transmit, queuing and TX
processing would be done in driver.  In order to preserve old
behavior, buf-ring-aware drivers may have to return ENOBUFS as you
said.
The compatibility code introduced in if_transmit for legacy drivers
shall return ENOBUFS when there is no room in if_snd.  This is the
reason why I asked whether Michael sees the same behavior on
non-buf-ring aware drivers.

> 
> 
> >> 
> >> Any comments?
> > 
> > I'm afraid the patch you posted ignores any errors(i.e.
> > m_defrag(9), bus_dma(9) etc) happened during TX processing.
> 
> But that is always the case. Most of the time when you send
> down to if_transmit() the first time you are going to get
> your thread working on those things m_defrag() and bus_dma().. but if
> another thread awoke the driver ahead of you all you get is the return
> code of the queue into the buffers.. you can't know what is happening on
> the other thread that is actually putting the work out.
> 
> This has always been the case.
> 
> This patch I think is *very* much needed on all the ring buffer aware drivers except maybe
> Chelsio (since there's is so different it probably does not have this issue).
> 
> I will be applying this to all of Adara's code and I would *strongly* encourage Jack to get this
> in to the intel side.
> 
> I will also pull this patch (and fix all the other drivers) in the branch I will be creating
> shortly per Adrian's suggestion on the multi-Q qos stuff I was working on..
> 
> Jack? when can you get this in ??
> 
> R
> 
> 
> > 
> >> 
> >> Jack: What do you think? Would you prefer to commit the fix if
> >> you think it is acceptable?
> >> 
> >> Best regards
> >> Michael
> > _______________________________________________
> > 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"
> > 
> 
> ------------------------------
> Randall Stewart
> 803-317-4952 (cell)
> 
> _______________________________________________
> 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"