-
Notifications
You must be signed in to change notification settings - Fork 13
nfd: Update node-feature-rules-openshift.yaml #112
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
@vbedida79 @mregmi @uMartinXu pls help review. |
|
class: {op: In, value: ["0b40"]} | ||
- feature: kernel.loadedmodule | ||
matchExpressions: | ||
intel_qat: {op: Exists} | ||
|
||
- name: "intel.sgx" | ||
- name: "intel.sgx" |
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.
a change: indentation error here
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.
Good eye! Thanks @vbedida79
7121853
to
bc91705
Compare
Deployed it to check sgx. The node is not labelled with |
1ecc2e1
to
a7effe8
Compare
Got it, updated commit to keep SGX NFD rules as is for now. |
What is the NFD version available? Do we need to submit a ticket to get the version updated? |
matchExpressions: | ||
dfl_pci: {op: Exists} | ||
|
||
matchAny: |
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.
I prefer not to add the FPGA and IAA into the NFD if we haven't started to support them or will enable them in the coming release.
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.
I think we can keep IAA since it will be enabled in 1.1.0 release. I agree, we can remove FPGA and DLB as well. Can we keep IAA or remove it as well?
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.
I suggest to remove IAA, and only add what we will support.
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.
Makes sense! :)
@@ -62,7 +81,7 @@ spec: | |||
- feature: pci.device | |||
matchExpressions: | |||
vendor: {op: In, value: ["8086"]} | |||
device: {op: In, value: ["37c8", "4940"]} |
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.
@mregmi is these the device ID for 4xxx QAT device? Can we add some HW detail about the 4xxx QAT device?
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.
These are OK. What details you had in mind?
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.
All the devices with 4xxx device ID should be the integrated QAT 2.0 HW on SPR or later xeon platforms. @mythi is that correct?
Yes OCP NFD 4.12.0-202307182142. No exact info in repo Based on some older releases its upstream v0.x.0 for ocp 4.x. So v0.12.0. This change is in v0.13.0 upstream. Submitted a ticket on RH portal for confirming. |
Thanks @vbedida79 |
@hershpa for this PR, please only work on changing the QAT device. And in the commit log, explain the reason why we made this change. After that, I am OK to merge it. Thanks! |
Sounds good, thanks @uMartinXu for the review. |
fbd52fa
to
c28432e
Compare
Updated PR @uMartinXu, @vbedida79 |
@@ -54,15 +15,15 @@ spec: | |||
matchExpressions: | |||
vendor: {op: In, value: ["8086"]} | |||
class: {op: In, value: ["0300", "0380"]} | |||
|
|||
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.
Clean up by remove the space from this line.
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.
The PR looks good to me.
Keep node feature rules only for SGX, QAT, and GPU. Keep only 4xxx device IDs for QAT as we support QAT only on SPR or later platforms. Signed-off-by: Hersh Pathak [email protected]
c21b2df
to
c4f2047
Compare
Confirmed from RH, this change will be in OCP 4.14. |
Thanks @vbedida79 |
Keep node feature rules only for SGX, QAT, and GPU.
Keep only 4xxx device IDs for QAT as we support QAT only on SPR or later platforms.
Signed-off-by: Hersh Pathak [email protected]