iflib: Add sysctl to request extra MSIX vectors on driver load
ClosedPublic
Actions

Authored by erj on Aug 4 2023, 6:06 PM.

Details

Reviewers

Group Reviewers

Commits

rG1eb348e090fd: iflib: Add sysctl to request extra MSIX vectors on driver load
rGff28ca091142: iflib: Add sysctl to request extra MSIX vectors on driver load
rG3c7da27a473e: iflib: Add sysctl to request extra MSIX vectors on driver load

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 created this revision.Aug 4 2023, 6:06 PM

Herald added a reviewer: shurd. · View Herald TranscriptAug 4 2023, 6:06 PM

Herald added a reviewer: iflib. · View Herald Transcript

Herald added subscribers: glebius, melifaro, ae, imp. · View Herald Transcript

erj requested review of this revision.Aug 4 2023, 6:06 PM

Harbormaster completed remote builds in B52982: Diff 125597.Aug 4 2023, 6:06 PM

erj added a parent revision: D41325: iflib: Remove redundant variable.Aug 4 2023, 6:06 PM

erj added a child revision: D39930: iflib: Add subinterface interrupt allocation function.Aug 4 2023, 6:13 PM

marius added a subscriber: marius.Aug 4 2023, 7:09 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?

erj added inline comments.Aug 4 2023, 7:16 PM

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.

marius added inline comments.Aug 4 2023, 7:30 PM

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

Harbormaster completed remote builds in B53009: Diff 125685.Aug 7 2023, 10:58 PM

Rebase on latest main

Harbormaster completed remote builds in B56140: Diff 134776.Feb 21 2024, 5:20 AM

gallatin added a subscriber: gallatin.Feb 21 2024, 6:29 PM

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?

gallatin added inline comments.Feb 21 2024, 6:40 PM

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 :(