cpu: support for detecting nx-gzip coprocessor feature

[Chandan-Abhyankar]

Signed-off-by: Chandan Abhyankar [email protected]
Adding nx-gzip label support

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: Chandan-Abhyankar / name: Chandan Abhyankar (a3e72f5)

[netlify]

✅ Deploy Preview for kubernetes-sigs-nfd ready!

Name	Link
🔨 Latest commit	d66096a
🔍 Latest deploy log	https://app.netlify.com/sites/kubernetes-sigs-nfd/deploys/63c79d863e17940009083723
😎 Deploy Preview	https://deploy-preview-956--kubernetes-sigs-nfd.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site settings.

[k8s-ci-robot]

Hi @Chandan-Abhyankar. 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.

[Chandan-Abhyankar]

/easycla

[prb112]

Hi @adrianchiris @Ethyling
I hope all is well.
Are there any steps we can take to address your comments/concerns?
Many thanks
Paul

[jjacobelli]

Hey @Chandan-Abhyankar @prb112. Is this feature available for every archs or should we move this to an arch dependent file with a stub file for archs that don't support it? Something similar to rdt_amd64.go and rdt_stub.go. I don't have a lot of knowledge about CPU attributes. Otherwise the code looks good to me, but I don't have the hardware to test it

[prb112]

Hey @Ethyling it is architecture specific - it'd be similar to the intel flags such as SstFeature. We could move it to a separate file. Do you want us to take that step? Many thanks, Paul

[jjacobelli]

@prb112 Yes, I think we should move this to a separate file so we don't include it for archs that don't have this feature. Thank you

[prb112]

@prb112 Yes, I think we should move this to a separate file so we don't include it for archs that don't have this feature. Thank you

We'll take care of it. Thanks @Ethyling

[Chandan-Abhyankar]

Hi @Ethyling As changes are architecture specific [Power], I have moved code to cpuid_ppc64le.go.
Kindly have a re-look and let me know if you have any comments.
Thank you

[jjacobelli]

Hey @Chandan-Abhyankar, I don't this this feature is related to cpuid so I thinkg we should move it to its own file (something like nx_ppc64le.go). We should also provide a stub file (i.e. nx_stub.go) for the other archs

[prb112]

Hi @Ethyling

nx-gzip is an on-chip accelerator to facilitate compression/decompression on PowerPC. PR from centos elaborates in more detail.

Do you still want us to split this out?

Thank you for the clarification,

Paul

[jjacobelli]

Hi @prb112, yes I still think we should split it in its own file. Let's ask inputs from @marquiz to confirm this

[prb112]

Hi @prb112, yes I still think we should split it in its own file. Let's ask inputs from @marquiz to confirm this

Thanks, I'll wait on the confirmation, take care, Paul

[marquiz]

Thank your for the review @Ethyling. I'm not an expert on power arch so I'm a bit unknowledgeable here 😊 If it's a cpu hw capability I think we could (or even should) put it under cpuid, if the better educated people here think that way.

Talking about rdt those are some very old legacy features that should probably be deprecated and moved under cpuid 🤔

ping @yselkowitz any thoughts from you?

/ok-to-test

[yselkowitz]

CPUID should be just that, which this isn't. It should be split out and handled separately (but still under source/cpu), like pstate or security.

Also, be sure to use hostpath.SysfsDir.Path as elsewhere in NFD.

[prb112]

Thank you for the feedback @yselkowitz @marquiz we've taken the comments into account and are working on a revision.

[Chandan-Abhyankar]

@marquiz , Thanks a lot for your pointers, help and support. I have now integrated all changes in one commit. Kindly have a re-look.

[marquiz]

Thanks a lot for your pointers, help and support. I have now integrated all changes in one commit. Kindly have a re-look.

Thanks @Chandan-Abhyankar. Still one nit about the commit message body 🙈 Use full sentences, starting with a capital letter and end with a period, e.g. Nest accelerator gzip support for IBM Power systems.

You could also remove the merge commit by rebasing on top of latest master.

[Chandan-Abhyankar]

@marquiz , Kindly review changes. I was facing issues with rebasing on top of latest master, hence followed my usual steps to check-in code.
For features.md and customization-guide.md (inside docs/usage folder), I took latest code from master branch and appended my changes. Hope all is good now. Thanks

[marquiz]

Thanks @Chandan-Abhyankar for the update. The commit looks a bit strange as it touches unrelated files, in docs for example. Could you do git fetch origin && git rebase origin/master and then re-push to base this on the latest master? Other than that, looks good now

[marquiz]

Thanks @Chandan-Abhyankar, looks good to go to me 👍

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Chandan-Abhyankar, marquiz

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

[marquiz]

/retitle cpu: support for detecting nx-gzip coprocessor feature

[Chandan-Abhyankar]

@marquiz , Thanks a lot for your help and support.

[jjacobelli]

/lgtm

[k8s-ci-robot]

LGTM label has been added.

Git tree hash: 609eb74ff94a3f9bc24014a4d513f0b3d50371ec

[prb112]

Thank you all - we appreciate all your review and effort in supporting this feature.