Page MenuHomeFreeBSD

aq(4): drop errored RX frames instead of resetting the interface
ClosedPublic

Authored by nick_spun.io on Fri, Jul 10, 3:55 AM.
Referenced Files
F164338273: D58136.id182230.diff
Thu, Jul 30, 11:04 PM
Unknown Object (File)
Thu, Jul 30, 8:57 PM
Unknown Object (File)
Thu, Jul 30, 7:43 AM
Unknown Object (File)
Thu, Jul 30, 3:13 AM
Unknown Object (File)
Wed, Jul 29, 2:14 PM
Unknown Object (File)
Tue, Jul 28, 12:33 PM
Unknown Object (File)
Mon, Jul 27, 4:15 PM
Unknown Object (File)
Mon, Jul 27, 3:55 PM
Subscribers

Details

Summary

aq_isc_rxd_pkt_get() returned EBADMSG when a receive descriptor's
MAC/receive-error bit (rx_stat bit 0) was set. iflib treats any error
from isc_rxd_pkt_get() as a fatal ring fault and answers with
IFC_DO_RESET -- a full interface reinitialization. A per-frame receive
error is not a ring fault: on a marginal link or cable the Atlantic
delivers errored frames continuously, so each one triggered another
reset and the interface reset-stormed itself into carrying no traffic
instead of merely dropping the bad frames.

The Atlantic delivers errored frames to the host by design (Linux drops
them in software via buff->is_error), and iflib offers no per-frame
error return that isn't a reset. Follow the vmxnet3 model: on a receive
error zero the fragment lengths and return success. iflib then discards
the packet (assemble_segments() excludes zero-length fragments) while
still recycling the descriptors through the refill path -- no reset.
Also drop frames flagged with an RX-DMA fault (rdm_err), not just the
MAC-error bit; and keep iri_len non-zero on that drop path, since iflib
asserts iri_len != 0.

The genuinely structural errors -- more segments than isc_rx_nsegments,
or a pkt_len inconsistent with the descriptor count -- still return
EBADMSG, since those indicate a confused ring where a reset is the right
recovery.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

adrian added a subscriber: drivers.

Good catch; I think we should add a TODO somewhere to extend iflib to handle temporary errors rather than the big hammer it has right now.

sys/dev/aq/aq_ring.c
393

What's this mean though? Is iflib treating this as a "too short" packet?

sys/dev/aq/aq_ring.c
393

Yeah I lifted this trick from vmxnet3 - iflib will drop zero-length segments but still handle reclaiming everything

This revision is now accepted and ready to land.Wed, Jul 15, 8:11 PM