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

[Adrian Chadd]

On 13 December 2013 10:26, John Baldwin <jhb at freebsd.org> wrote:
> On Monday, December 09, 2013 2:37:08 pm Adrian Chadd wrote:
>> Jack / John - thoughts?
>
> Note that if_start has always worked the way if_transmit would work with the
> err = 0 change.  All the other drivers in the tree are already giving you an
> error if an earlier packet in the IFQ fails to transmit, and it's been that
> way for decades.  If you decide to make if_transmit precise (only return an
> error if the specific packet fails), the corresponding change for if_start()
> drivers would be to never return an error ever.

Right. If we want some kind of sane network behaviour moving forward
we .. well, we have to define it as being sane. :-)

IMHO if_start()'s API should've been:

* queue something to the ifnet queue - fail it we can't queue it
* call if_start() to start trying to transmit something - a failure is
not that the queued frame failed, but the driver is unable to dequeue
frames.

Anyone using if_start() failing as "the frame i just queued failed to
transmit" is broken and well, we should just replace it with
if_transmit().

> It's not clear to me what the impact of that would be.  However, the current
> patch to return 0 in the drbr_putback case would at least restore things to
> the previous status quo from the past few decades for both if_start() and
> if_transmit() drivers.

I believe the most correct return behaviour from if_transmit() is "the
frame was successfully queued to the driver; it's now up to the driver
to eventually send the frame."

If the driver does direct dispatch then it'll error out if it couldn't
queue the frame to the hardware TX descriptor ring/queue/blah.

If the driver does its own queuing then it's up to the driver itself
to fail on the hardware transmit function and then retry it when
appropriate. It shouldn't be dropping frames during normal transmit
unless there's some kind of policy in place to do so - and yes, this
is a separate discussion to have with andre and rrs.


-adrian