Bug 248934 - e1000: hw.em.sbp default value change
Summary: e1000: hw.em.sbp default value change
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: 12.1-RELEASE
Hardware: Any Any
: --- Affects Only Me
Assignee: Kevin Bowling
URL:
Keywords: IntelNetworking
Depends on:
Blocks:
 
Reported: 2020-08-26 18:00 UTC by Franco Fichtner
Modified: 2021-05-28 06:29 UTC (History)
4 users (show)

See Also:
kbowling: mfc-stable13+
kbowling: mfc-stable12+
kbowling: mfc-stable11-


Attachments
hw.em.sbp off by default (1.08 KB, patch)
2020-08-26 18:00 UTC, Franco Fichtner
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Franco Fichtner 2020-08-26 18:00:22 UTC
Created attachment 217546 [details]
hw.em.sbp off by default

Hi,

The iflib rewrite changed sysctl hw.em.sbp default to on but it is still documented as off by default. This coupled with #248869 caused error counters for the affected NICs to report lots of errors as compared to FreeBSD 11 and below.

This also affects CURRENT and is present since 12.0, but marking this as a 12.1-RELEASE issue.

Patch is attached to restore the previous behaviour, but maybe the documentation should be changed instead.


Cheers,
Franco
Comment 1 Eric Joyner freebsd_committer freebsd_triage 2020-09-15 21:13:14 UTC
I'm not sure about this one; I agree that either the code or the documentation should change, but I don't know if we should change the default back, for 13.0 or 12.2.

I also don't know why it changed from 11, either. We at Intel didn't do the 1G driver conversion to iflib. (and I'm not even supposed to be maintaining it!)
Comment 2 Franco Fichtner 2020-09-24 06:40:15 UTC
I am unsure too. The only issue I see is that some applications VLANs with Netmap only work in device promisc mode and having this enabled by default might decrease performance which is hard to track down to this "debug" sysctl.
Comment 3 Kevin Bowling freebsd_committer freebsd_triage 2021-04-15 06:47:54 UTC
(In reply to Franco Fichtner from comment #2)
This looks good to me, will you go ahead and commit it?
Comment 4 Franco Fichtner 2021-04-15 06:51:03 UTC
(In reply to Kevin Bowling from comment #3)
Not a committer. I'm hopeful somebody can spare a bit of time to pick this up.


Cheers,
Franco
Comment 5 Kevin Bowling freebsd_committer freebsd_triage 2021-04-15 07:01:30 UTC
https://reviews.freebsd.org/D29768
Comment 6 commit-hook freebsd_committer freebsd_triage 2021-04-15 16:53:04 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=548d8a131d536d5f8e0818ff8cff7ffd63dbedfe

commit 548d8a131d536d5f8e0818ff8cff7ffd63dbedfe
Author:     Kevin Bowling <kbowling@FreeBSD.org>
AuthorDate: 2021-04-15 16:48:41 +0000
Commit:     Kevin Bowling <kbowling@FreeBSD.org>
CommitDate: 2021-04-15 16:48:41 +0000

    e1000: disable hw.em.sbp debug setting

    This is a debugging tunable that shouldn't have retained this setting
    after the initial iflib conversion of the driver

    PR:             248934
    Reported by:    Franco Fichtner <franco@opnsense.org>
    Reviewed by:    markj
    MFC after:      1 month
    Differential Revision:  https://reviews.freebsd.org/D29768

 sys/dev/e1000/if_em.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Comment 7 Kevin Bowling freebsd_committer freebsd_triage 2021-04-15 16:54:05 UTC
Thanks for the report!  I will MFC this back to 13 and 12.
Comment 8 Franco Fichtner 2021-04-15 19:28:23 UTC
Thank you. :)
Comment 9 commit-hook freebsd_committer freebsd_triage 2021-05-18 07:47:27 UTC
A commit in branch stable/13 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=135a6b5652f24f9979ad4a74d45943887f78e898

commit 135a6b5652f24f9979ad4a74d45943887f78e898
Author:     Kevin Bowling <kbowling@FreeBSD.org>
AuthorDate: 2021-04-15 16:48:41 +0000
Commit:     Kevin Bowling <kbowling@FreeBSD.org>
CommitDate: 2021-05-18 07:42:59 +0000

    e1000: disable hw.em.sbp debug setting

    This is a debugging tunable that shouldn't have retained this setting
    after the initial iflib conversion of the driver

    PR:             248934
    Reported by:    Franco Fichtner <franco@opnsense.org>
    Reviewed by:    markj
    MFC after:      1 month
    Differential Revision:  https://reviews.freebsd.org/D29768

    (cherry picked from commit 548d8a131d536d5f8e0818ff8cff7ffd63dbedfe)

 sys/dev/e1000/if_em.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Comment 10 commit-hook freebsd_committer freebsd_triage 2021-05-18 07:49:30 UTC
A commit in branch stable/12 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=94c02a765cb7f68c80844acb5898be90dc4069c5

commit 94c02a765cb7f68c80844acb5898be90dc4069c5
Author:     Kevin Bowling <kbowling@FreeBSD.org>
AuthorDate: 2021-04-15 16:48:41 +0000
Commit:     Kevin Bowling <kbowling@FreeBSD.org>
CommitDate: 2021-05-18 07:48:04 +0000

    e1000: disable hw.em.sbp debug setting

    This is a debugging tunable that shouldn't have retained this setting
    after the initial iflib conversion of the driver

    PR:             248934
    Reported by:    Franco Fichtner <franco@opnsense.org>
    Reviewed by:    markj
    MFC after:      1 month
    Differential Revision:  https://reviews.freebsd.org/D29768

    (cherry picked from commit 548d8a131d536d5f8e0818ff8cff7ffd63dbedfe)

 sys/dev/e1000/if_em.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Comment 11 Kevin Bowling freebsd_committer freebsd_triage 2021-05-28 06:29:27 UTC
No MFC to stable11 it has a different driver.