Page MenuHomeFreeBSD

iflib: Add sysctl to request extra MSIX vectors on driver load
Needs ReviewPublic

Authored by erj on Aug 4 2023, 6:06 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Apr 6, 11:12 AM
Unknown Object (File)
Feb 29 2024, 7:11 AM
Unknown Object (File)
Feb 28 2024, 1:48 PM
Unknown Object (File)
Feb 28 2024, 12:00 PM
Unknown Object (File)
Feb 28 2024, 10:21 AM
F77652750: image.png
Feb 21 2024, 6:43 PM
F77652634: image.png
Feb 21 2024, 6:43 PM
Unknown Object (File)
Jan 4 2024, 8:41 AM

Details

Reviewers
shurd
Group Reviewers
iflib
Summary

Intended to be used with upcoming feature to add sub-interfaces, since
those new interfaces will be dynamically created and will need to have
spare MSI-X interrupts already allocated for them on driver load.

While here, fix some alignment of sysctl declarations as well.

Signed-off-by: Eric Joyner <erj@FreeBSD.org>

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 56140
Build 53028: arc lint + arc unit

Event Timeline

erj requested review of this revision.Aug 4 2023, 6:06 PM
marius added inline comments.
sys/net/iflib.c
197

Currently, there's a mix and match of different sizes for actually boolean SYSCTLs. 'ifc_sysctl_use_extra_msix_vectors' also sounds like it's intent is to be an on/off knob rather than a count of vectors. Is there a particular reason for it to be of type uint16_t?

sys/net/iflib.c
197

It is actually intended to be a count -- I think the name might not make it clear (especially since it comes after use_logical_cores which is supposed to act like a bool); I wasn't sure on what to use, so I went with this and thought about revisiting it later. Do you have a suggestion?

The variable doesn't get used in iflib proper because ice(4) handles its own MSI-X interrupt allocations (hence the public accessor function), but it's located here just in case other drivers would like to use it or whatever enhanced functionality ice(4) has gets pulled into iflib.

sys/net/iflib.c
197

Well, I'd just drop the 'use_' parts then and make the SYSCTL description something like 'attempted reservation count of extra MSI-X vectors for runtime interface creation'. The latter might be worded more precisley depending on what 'try' means in this context, i. e. is failure to reserve these vectors fatal or not, will their number be reduced to what can acutally be handled etc.

Update variable names and sysctl help string

gallatin added inline comments.
sys/net/iflib.c
6817

Why is this a tunable? It would seem to be calculated at driver load, when the vectors are allocated.

Speaking of that, maybe I'm being dense, but I don't see anything in this change that actually allocates extra vectors. What am I missing?

sys/net/iflib.c
6817

Why is this a tunable? It would seem to be calculated at driver load, when the vectors are allocated.

Speaking of that, maybe I'm being dense, but I don't see anything in this change that actually allocates extra vectors. What am I missing?

Oh, this is some kind of diff stack. I can't seem to figure how how to see related changes in the web interface :(

@gallatin Do you see the Stack tab?

image.png (337×682 px, 29 KB)

Weirdly, the later diffs in the stack have the label in red with the number of open diffs in parentheses, like this:

image.png (94×434 px, 4 KB)