Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ethtool: add support for private flags #22

Merged
merged 1 commit into from
Nov 24, 2023

Conversation

lorenz
Copy link
Contributor

@lorenz lorenz commented Nov 22, 2023

This adds support for geting and setting private flags which are driver-specific (and sometimes even model-specific) controls not part of any generic network settings API.

They are used to toggle low-level features in various NICs.

There are currently no tests in this, considering the driver-specific nature of this I am not sure how to best add some. This has been experimentally tested on Linux 6.1 LTS and seems to work fine.

Copy link
Collaborator

@SuperQ SuperQ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems reasonable, are there any example known flags that are common that we could document here?

@SuperQ SuperQ requested a review from mdlayher November 22, 2023 07:34
@mdlayher
Copy link
Owner

client_linux_test.go has some examples of tests that pack and unpack datastructures which may be good to have for these.

We also want to make sure this builds on non-Linux, see the mac build failures. Otherwise seems good so far.

@lorenz
Copy link
Contributor Author

lorenz commented Nov 22, 2023

@mdlayher Added some tests based on ethtool reference values and fixed the MacOS build.

Copy link
Owner

@mdlayher mdlayher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, well done.

@mdlayher
Copy link
Owner

Ah, looks like an easy fix:

Error: ./client.go:322:26: too many arguments in call to c.c.PrivateFlags
	have (Interface)
	want ()

This adds support for geting and setting private flags which are
driver-specific (and sometimes even model-specific) controls not part of
any generic network settings API.

They are used to toggle low-level features in various NICs.
@lorenz
Copy link
Contributor Author

lorenz commented Nov 22, 2023

@mdlayher Fixed, now actually cross-build-tested and passing.

@lorenz lorenz requested a review from mdlayher November 23, 2023 18:19
@mdlayher mdlayher merged commit 6bb0780 into mdlayher:main Nov 24, 2023
6 checks passed
@lorenz lorenz deleted the add-privflags branch November 24, 2023 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants