Page MenuHomeFreeBSD
Differential D49427

gve: Add feature to adjust RX/TX queue counts
ClosedPublic
Actions

Authored by veethebee_google.com on Mar 20 2025, 8:36 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Aug 21, 10:32 AM
Unknown Object (File)
Thu, Aug 14, 11:48 PM
Unknown Object (File)
Mon, Aug 11, 6:08 PM
Unknown Object (File)
Thu, Aug 7, 2:12 AM
Unknown Object (File)
Tue, Aug 5, 2:24 AM
Unknown Object (File)
Mon, Aug 4, 3:53 AM
Unknown Object (File)
Fri, Aug 1, 10:34 PM
Unknown Object (File)
Wed, Jul 30, 1:18 AM
View All 45 Files
Subscribers
imp
ziaee

Details

Reviewers
markj
delphij
kibab
shailend_google.com
Group Reviewers
network
Commits
rGecc250c60049: gve: Add feature to adjust RX/TX queue counts
rGe0464f74d557: gve: Add feature to adjust RX/TX queue counts
Summary

This change introduces new sysctl handlers that allow the user to change
RX/TX queue counts. As before, the default queue counts will be the max
value the device can support. When chaning queue counts, the interface turns
down momentarily while allocating/freeing resources as necessary.

Signed-off-by: Vee Agarwal <veethebee@google.com>

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 63318
Build 60202: arc lint + arc unit

Event Timeline

veethebee_google.com created this revision.Mar 20 2025, 8:36 PM
Herald added subscribers: ziaee, imp. · View Herald TranscriptMar 20 2025, 8:36 PM
veethebee_google.com requested review of this revision.Mar 20 2025, 8:36 PM
Harbormaster completed remote builds in B63008: Diff 152477.Mar 20 2025, 8:36 PM
veethebee_google.com added a parent revision: D49426: gve: Allocate qpl per ring at ring allocation time.Mar 20 2025, 8:36 PM
veethebee_google.com added a child revision: D49428: gve: Add feature to change TX/RX ring size.
veethebee_google.com updated this revision to Diff 152481.Mar 20 2025, 10:52 PM

Update man page

Harbormaster completed remote builds in B63012: Diff 152481.Mar 20 2025, 10:52 PM
veethebee_google.com updated this revision to Diff 152939.Mar 31 2025, 11:24 PM

Set priv rx/tx fields to NULL after free

Harbormaster completed remote builds in B63235: Diff 152939.Mar 31 2025, 11:24 PM
shailend_google.com accepted this revision.Apr 1 2025, 12:06 AM
This revision is now accepted and ready to land.Apr 1 2025, 12:06 AM
markj added inline comments.Apr 2 2025, 12:54 PM
share/man/man4/gve.4
241

It'd be nicer IMO to document the two sysctls together rather than effectively duplicating the description.

243
sys/dev/gve/gve_main.c
214

Please use C-style /* */ comments in the kernel.

568

Why do we alloc resources for max_queues queues instead of num_queues queues?

sys/dev/gve/gve_sysctl.c
288
369

I would suggest using a plain CTLTYPE_UINT instead of constraining the queue count to 16. It feels a bit strange to encode the type of max_queues in the sysctl ABI. For command-line usage it doesn't matter, but for consistency with other NIC drivers it might be nicer to use a more generic type when accessing the sysctl from C.

370
374
markj mentioned this in D49428: gve: Add feature to change TX/RX ring size.Apr 2 2025, 1:04 PM
veethebee_google.com updated this revision to Diff 153140.Apr 3 2025, 8:08 PM

Fix from comments

This revision now requires review to proceed.Apr 3 2025, 8:08 PM
Harbormaster completed remote builds in B63318: Diff 153140.Apr 3 2025, 8:08 PM
veethebee_google.com marked 8 inline comments as done.Apr 3 2025, 8:14 PM
markj accepted this revision as: markj.Apr 4 2025, 12:23 PM

Seems ok to me modulo the comments.

sys/dev/gve/gve_sysctl.c
329

IMO printing a message in this case is overkill and not typical for sysctl handlers. The sysctl handler will return the current number of queues currently configured, so code which modifies the queue count can see how many queues were registered before.

359

Same comment here about the print.

This revision is now accepted and ready to land.Apr 4 2025, 12:23 PM
veethebee_google.com updated this revision to Diff 153163.Apr 4 2025, 8:22 PM

Don't log if queue count doesn't change

This revision now requires review to proceed.Apr 4 2025, 8:22 PM
Harbormaster completed remote builds in B63329: Diff 153163.Apr 4 2025, 8:22 PM
veethebee_google.com marked 2 inline comments as done.Apr 4 2025, 8:23 PM
This revision was not accepted when it landed; it landed in state Needs Review.Apr 4 2025, 11:27 PM
Closed by commit rGe0464f74d557: gve: Add feature to adjust RX/TX queue counts (authored by veethebee_google.com, committed by markj). · Explain Why
This revision was automatically updated to reflect the committed changes.
markj added a commit: rGe0464f74d557: gve: Add feature to adjust RX/TX queue counts.
markj added a commit: rGecc250c60049: gve: Add feature to adjust RX/TX queue counts.Apr 18 2025, 2:01 PM

Revision Contents
Changeset List

PathSize
share/
man/
man4/
19 lines
sys/
dev/
gve/
10 lines
88 lines
20 lines
91 lines
21 lines
6 lines

Diff 153140

View Options

share/man/man4/gve.4

Loading...
View Options

sys/dev/gve/gve.h

Loading...
View Options

sys/dev/gve/gve_main.c

Loading...
View Options

sys/dev/gve/gve_rx.c

Loading...
View Options

sys/dev/gve/gve_sysctl.c

Loading...
View Options

sys/dev/gve/gve_tx.c

Loading...
View Options

sys/dev/gve/gve_utils.c

Loading...