source/cpu: detect Intel SGX

[mythi]

Fixes: #130, #638

[k8s-ci-robot]

Hi @mythi. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[marquiz]

Basically looks good and simple to me.

Would it be useful to add other SGX attributes from klauspost/cpuid as raw features, now that we add this, WDYT? I'm thinking about these max enclave size etc.

[marquiz]

Should we advertise epcSize as the raw feature, directly?

[mythi]

I wanted to start with something minimal/basic and enabled is consistent with the other cpusource features. The boolean value can be taken into nodeSelector directly which is my use case at least until #464 lands. Would there be any use for epcSize other than using it in, e.g., {op: Gt, value: ["0"]}?

I'm thinking about these max enclave size etc.

I'm not sure how useful they are. We could label, e.g., sgx2=true but to actually use the feature the kernel support needs to be there and we cannot detect that. max enclave size value is a bit value for 2^(<value>) computation.

[marquiz]

I wanted to start with something minimal/basic and enabled is consistent with the other cpusource features. The boolean value can be taken into nodeSelector directly which is my use case at least until #464 lands. Would there be any use for epcSize other than using it in, e.g., {op: Gt, value: ["0"]}?

Yeah, for the label enabled makes sense. I might be wrong but my intuition is that the better direction would be to expose "raw features" as raw as possible, avoiding any translations. More likely to avoid problems like the kernel module values (currently =y and =m now translate to true and we have no means of telling between y or m). As you said, in this case enabled would translate to {op: Gt, value: ["0"]} in the custom rules. That said, I don't know what use people could find for the numerical value (expose it as an extended resource?). Maybe we should expose both, enabled and epcSize?

I'm thinking about these max enclave size etc.

I'm not sure how useful they are. We could label, e.g., sgx2=true but to actually use the feature the kernel support needs to be there and we cannot detect that. max enclave size value is a bit value for 2^(<value>) computation.

K, can be added later, if needed

[mythi]

expose it as an extended resource?). Maybe we should expose both, enabled and epcSize?

We have an NFD source hook to read epcSize and that value is registered as sgx.intel.com/epc. I guess NFD could register the value too but why not then use the NFD source hook because it comes with the device plugin that is also needed.

[marquiz]

/ok-to-test

[mythi]

rebased to latest master and force pushed

[marquiz]

Thanks @mythi for this enhancement 👍 Let's merge this and expand functionality in future, if needed

/lgtm

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: marquiz, mythi

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [marquiz]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment