Bug 262024 - em(4): iflib handles bad packets incorrectly
Summary: em(4): iflib handles bad packets incorrectly
Status: Open
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Some People
Assignee: freebsd-net (Nobody)
URL:
Keywords: IntelNetworking, needs-patch, needs-qa
Depends on:
Blocks:
 
Reported: 2022-02-17 21:54 UTC by Eric Joyner
Modified: 2022-02-18 01:21 UTC (History)
6 users (show)

See Also:
koobs: maintainer-feedback? (kbowling)
koobs: maintainer-feedback? (vmaffione)
koobs: maintainer-feedback? (sbruno)
koobs: maintainer-feedback? (hselasky)
koobs: mfc-stable13?
koobs: mfc-stable12?

Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Joyner freebsd_committer freebsd_triage 2022-02-17 21:54:22 UTC 
We saw an issue where the em(4) interface would flap repeatedly, and the root cause was that the SBP flag was enabled, causing the hardware to send bad packets to the driver.

It appears that if the driver's isc_rxd_pkt_get() handler (e.g. in em here: https://cgit.freebsd.org/src/tree/sys/dev/e1000/em_txrx.c#n692) returns EBADMSG (because the hardware said the packet was bad), the iflib function that calls that will immediately jump to resetting the hardware.

We don't think that this should be the case (especially if the driver is explicitly allowing bad packets) and think it should be changed.

A previous bug report/commit changed the em(4) driver's default SBP setting to false, so that future users probably won't encounter this behavior unless they manually set it back to true.

In this case we've seen it em(4) specifically, but it could affect other iflib-using drivers that allow hardware to forward bad packets to the driver.
Comment 1 Eric Joyner freebsd_committer freebsd_triage 2022-02-17 21:57:37 UTC 
(In reply to Eric Joyner from comment #0)

You can see the call to the driver's isc_rxd_pkt_get() from iflib here: https://cgit.freebsd.org/src/tree/sys/net/iflib.c#n2987

The error handling code that indicates to iflib that it should try resetting itself is here: https://cgit.freebsd.org/src/tree/sys/net/iflib.c#n3087
Comment 2 Kubilay Kocak freebsd_committer freebsd_triage 2022-02-17 22:53:33 UTC 
@Eric Can we tell what other drivers may be affected? Also mentioned was  "A previous bug report/commit changed the em(4)" Could you link to those?
Comment 3 Eric Joyner freebsd_committer freebsd_triage 2022-02-18 01:21:31 UTC 
(In reply to Kubilay Kocak from comment #2)

Here's the patch to em that changed the sbp default: https://reviews.freebsd.org/D29768

As for other drivers, I don't know. Possibly ixgbe(4) might expose an sbp tunable/sysctl.