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

Adrian Chadd adrian at freebsd.org
Thu Dec 5 18:29:52 UTC 2013


Hi,

Yes. Looking at the ixgbe code, ixgbe_mq_start_locked() returns an
error from ixgbe_xmit() but if it fails, it puts the buffer back. But
it's already successfully queued a frame to the driver, so in this
instance it shouldn't return the error from ixgbe_mq_start_locked().

The same deal in if_em.c and igb.c

Now, drbr_putback() used to fail and now it doesn't, as you've said.
So we should change the xxx_mq_start_locked() to set err=0 if we go
via the drbr_putback() routine, as it hasn't actually failed to
transmit.

Now the very dirty thing is this - the error from xxx_transmit() is
for the mbuf being queued at the end; but xxx_mq_start_locked()
failures are for transmitting from the front. If there's only packet
in the queue and that fails then they're the same thing and returning
the error from xxx_mq_start_locked() matches the current mbuf being
queued. But otherwise, they're referring to totally different packets.
For TCP this may hurt; the TCP stack treats ENOBUFS a certain way and
kicks off a timer to schedule a retransmit. I don't think we can fix
_this_ right now.

So Michael - can you redo your patch to set err=0 if drbr_putback() is
called, and retest?

Thanks!




-adrian


More information about the freebsd-net mailing list