-
Notifications
You must be signed in to change notification settings - Fork 208
qat: drop AppArmor annotations #1860
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
Conversation
84e99cb
to
d40efb3
Compare
TODO: @hj-johannes-lee is still checking whether the |
Unfortunately |
I took a closer look. |
5cabe2a
to
2989f53
Compare
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.
LGTM.
Tested both when vfio-pci.ids are enabled in cmdline and when not enabled.
QAT device plugin has some initialization functions that require special SecurityContext parameters (e.g., setting Apparmor policies on some OSes). It's better to move all of the initialization to the privileged init-container that is already taking care some parts of it. With this change, we default to vfio-pci "DpdkDrv". Signed-off-by: Mikko Ylinen <[email protected]>
setupDeviceIDs() is obsoleted and the preferred approach is driver_override already implemented in qat-init.sh initcontainer. The new_id mechanism was added way before we had the initcontainer support in place. Furthermore, at least for vfio-pci we don't need it at all if the driver uses ids=8086:<qat VF dev IDs>. Drop write attemps to new_id in favor of the initcontainer functionality. Signed-off-by: Mikko Ylinen <[email protected]>
"unconfined" annotation was needed to get writes to new_id / bind to succeed on AppArmor enabled OSes. However, many things have changed: * new_id should not be used anymore and it was dropped in the plugin. * QAT initcontainer has assumed the role of HW initialization. * vfio-pci is the preferred "dpdkDriver" and starting with QAT Gen4, it is the only available VF driver so unbind isn't necessary. * k8s AppArmor is "GA" since 1.30 and the annotation is deprecated. As of now, the initcontainer will take care of binding QAT VFs to vfio-pci so the plugin does not neeed to set AppArmor at all. Signed-off-by: Mikko Ylinen <[email protected]>
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.
lgtm 👍
It's been GA since k8s 1.30 so we can enable it unconditionally starting with our 0.32 release.
Fixes: #1818
Fixes: #1887