Page MenuHomeFreeBSD

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

Authored by nick_spun.io on Fri, Jul 10, 3:55 AM.
Referenced Files
F162639671: D58136.id181648.diff
Wed, Jul 15, 7:43 AM
Unknown Object (File)
Tue, Jul 14, 8:22 AM
Unknown Object (File)
Tue, Jul 14, 3:24 AM
Unknown Object (File)
Mon, Jul 13, 12:42 AM
Unknown Object (File)
Sun, Jul 12, 8:14 PM
Unknown Object (File)
Sat, Jul 11, 6:44 PM
Unknown Object (File)
Sat, Jul 11, 5:08 PM
Unknown Object (File)
Fri, Jul 10, 5:51 PM
Subscribers

Details

Reviewers
adrian
Group Reviewers
drivers
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 Skipped
Unit
Tests Skipped
Build Status
Buildable 74678
Build 71561: arc lint + arc unit

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