add vfio based devices to DSA #2194
Conversation
mythi
force-pushed
the
PR-2026-001
branch
2 times, most recently
from
cbafc6c to
37e8041
Compare
|
Last commit touches also other things besides DSA, and looks like it could be its own PR? |
|
mythi
force-pushed
the
PR-2026-001
branch
2 times, most recently
from
2257a01 to
a76849b
Compare
Signed-off-by: Mikko Ylinen <[email protected]>
|
ready for review |
Signed-off-by: Mikko Ylinen <[email protected]>
Signed-off-by: Mikko Ylinen <[email protected]>
Signed-off-by: Mikko Ylinen <[email protected]>
eero-t
left a comment
eero-t
left a comment
There was a problem hiding this comment.
Skipped e2e (somebody else can check that), but the rest looks OK (on somewhat cursory review).
tkatila
left a comment
tkatila
left a comment
There was a problem hiding this comment.
Overall looks good. Had some general observations and one nitpick.
| ProvisioningConfig string `json:"provisioningConfig,omitempty"` | ||
|
|
||
| // Driver name used for the DSA devices. | ||
| // +kubebuilder:validation:Enum=idxd;vfio-pci |
There was a problem hiding this comment.
You could also have // +kubebuilder:default=idxd but it's not generally used in our CRDs.
| DEV="${DEVICE_TYPE:-dsa}" | ||
| DSA_DRIVER=${DSA_DRIVER:-idxd} | ||
| NODE_NAME="${NODE_NAME:-}" | ||
| DSA_PCI_IDS=${DSA_PCI_IDS:-0b25 11fb 1212} |
There was a problem hiding this comment.
This cannot be set via the operator. But QAT's device id listing is similar so it can be left as a future improvement idea.
There was a problem hiding this comment.
In fact, with QAT this is configurable (based on what user sets as the kernelvfdrivers).
There was a problem hiding this comment.
Kernel drivers are configurable: https://github.com/intel/intel-device-plugins-for-kubernetes/blob/main/pkg/apis/deviceplugin/v1/qatdeviceplugin_types.go#L55
But the Device IDs are not: https://github.com/intel/intel-device-plugins-for-kubernetes/blob/main/demo/qat-init.sh#L4
There was a problem hiding this comment.
this is (only) done through the operator:
intel-device-plugins-for-kubernetes/pkg/controllers/qat/controller.go
Lines 224 to 225 in 839fe47
| func readFile(fpath string) (string, error) { | ||
| data, err := os.ReadFile(fpath) | ||
| if err != nil { | ||
| return "", errors.WithStack(err) | ||
| } | ||
|
|
||
| return strings.TrimSpace(string(data)), nil | ||
| } |
There was a problem hiding this comment.
I guess this is a duplicate from the idxd side. Would be nice to have one in a common/util location. But since it's a small function, it's fine as it is.
There was a problem hiding this comment.
Right, dpdkdrv.go has also one for something very similar: getDeviceID()
| "0x11fb": {}, | ||
| "0x1212": {}, | ||
| } | ||
| plugin = vfio.NewDevicePlugin(pciDevicesDir, dsaDeviceIDs) |
There was a problem hiding this comment.
nitpick: as sharedDevNum is not supported with vfio-pci mode, plugin should warn/error if it's set. Maybe the same could be added to the validation webhook also.
3 participants
Fixes: #524
TODO:
pkg/vfio/plugin_test.go