-
Notifications
You must be signed in to change notification settings - Fork 771
[WIP] : add ability to dynamically detect gdrcopy module #1550
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
base: main
Are you sure you want to change the base?
Conversation
internal/utils/utils.go
Outdated
| // isDriverLoaded checks if the specified driver module is loaded by reading the modules file. | ||
| // It first checks "/host/proc/modules" (for containerized environments) and falls back to "/proc/modules" if not found. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: let's keep a maximum column length for the docu-comments around 80.
internal/utils/utils.go
Outdated
| }) | ||
| } | ||
|
|
||
| // isDriverLoadedWithPath checks if the specified driver module is loaded by reading the specified modules file. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: let's keep a maximum column length for the docu-comments around 80.
| cdi.WithDeviceIDStrategy(*config.Flags.Plugin.DeviceIDStrategy), | ||
| cdi.WithVendor("k8s.device-plugin.nvidia.com"), | ||
| cdi.WithGdrcopyEnabled(*config.Flags.GDRCopyEnabled), | ||
| cdi.WithGdrcopyEnabled(*config.Flags.GDRCopyEnabled || utils.IsGdrdrvLoaded()), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question: Since these are effectively no-ops if the drivers are not available, is it a simpler change to change the default for these settings instead of adding logic to check whether the modules are loaded?
b8cbf91 to
eb38ccb
Compare
Changes include: 1. For cdi, always attempt discovery of devices 2. For non-cdi, dynamically detect and enable if driver is loaded Signed-off-by: Rahul Sharma <rahulsharm@nvidia.com>
eb38ccb to
7a2c95a
Compare
Description
This PR introduces the ability to dynamically detect loaded modules.
When CDI is enabled:
Since k8s-device-plugin is resilient to missing drivers and does not crash if something is missing, we can always attempt to discover the supported devices/drivers.
When CDI is disabled:
We try to dynamically detect if the driver is loaded or not and based on that, set the relevant env var.
Testing
Test details: