Advertise RDT L3 num_closid #1100

PiotrProkop · 2023-03-29T10:27:21Z

Enable the feature discovery of RDTL3CA_NUM_CLOSID as part of the cpu-rdt source.

This feature is intended to be used as extended resource when this PR is merged #1099

netlify · 2023-03-29T10:27:28Z

✅ Deploy Preview for kubernetes-sigs-nfd ready!

Name	Link
🔨 Latest commit	`0e78eba`
🔍 Latest deploy log	https://app.netlify.com/sites/kubernetes-sigs-nfd/deploys/642e8f7b7c57b20008e1078d
😎 Deploy Preview	https://deploy-preview-1100--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.

PiotrProkop · 2023-03-29T10:56:43Z

@marquiz @ArangoGutierrez PTAL

ArangoGutierrez · 2023-03-29T19:36:10Z

Hold until #1099
/hold

ArangoGutierrez

Could you also add the new features here docs/usage/features.md

PiotrProkop · 2023-03-30T07:08:33Z

Could you also add the new features here docs/usage/features.md

@ArangoGutierrez done.

ArangoGutierrez · 2023-03-30T08:39:41Z

Could you also add the new features here docs/usage/features.md

@ArangoGutierrez done.

Awesome, let's wait for 1099 to get in

marquiz

Thanks @PiotrProkop for the enhancement! We need to document the new feature in the "Available features" table in customization guide (docs/usage/customization-guide.md), need also to change the "feature type" from "flag" to "attribute"

source/cpu/rdt_amd64.go

marquiz · 2023-04-04T07:13:46Z

docs/usage/features.md

+| RDTL3CA            | Intel L3 Cache Allocation Technology
+| RDTl2CA            | Intel L2 Cache Allocation Technology
+| RDTMBA             | Intel Memory Bandwidth Allocation (MBA) Technology
+| RDTL3CA_NUM_CLOSID | The number or available CLOSid(Class of service ID) for Intel L3 Cache Allocation Technology


Do we really want/need to advertise this as a label? I'd suggest to only have it as a "raw feature" for NodeFeatureRules to consume.

As a sidenote and a separate issue. We might want to deprecate the RDT labels as I'm not sure they are of any use in their current form (for example, they don't indicate if resctrlfs is mounted)

We are using RDTL3CA for scheduling purposes(when we want to setL3 cache partitions for our workloads). But we make sure resctrlfs is mounted when we provision hosts.

I agree that RDTL3CA_NUM_CLOSID as a label is of little to no use 😄 will change the code to stop advertise it.

PiotrProkop · 2023-04-05T10:45:10Z

Thanks @PiotrProkop for the enhancement! We need to document the new feature in the "Available features" table in customization guide (docs/usage/customization-guide.md), need also to change the "feature type" from "flag" to "attribute"

Thanks for the review! I have updated customization-guide.

marquiz

Thanks @PiotrProkop, almost there :)

docs/usage/customization-guide.md

source/cpu/rdt_amd64.go

deployment/helm/values.yaml

docs/usage/customization-guide.md

Signed-off-by: PiotrProkop <[email protected]>

marquiz

Thx @PiotrProkop, looks fine for me now 👍
/assign @ArangoGutierrez

ArangoGutierrez

/lgtm

k8s-ci-robot · 2023-04-06T10:56:28Z

LGTM label has been added.

Git tree hash: 42692216707f1ebb89e86a7f31cb89135794044c

k8s-ci-robot · 2023-04-06T10:56:29Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ArangoGutierrez, marquiz, PiotrProkop

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 · 2023-04-06T14:59:30Z

@PiotrProkop I think we could unhold this? Unless you want to verify it first with #1099. I'm ok with both

PiotrProkop · 2023-04-07T07:32:16Z

/unhold

PiotrProkop · 2023-04-07T07:32:45Z

@PiotrProkop I think we could unhold this? Unless you want to verify it first with #1099. I'm ok with both

Yeah, we can merge this!

k8s-ci-robot requested review from ArangoGutierrez and fmuyassarov March 29, 2023 10:27

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Mar 29, 2023

k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Mar 29, 2023

PiotrProkop force-pushed the expose-L3-num-closid branch 2 times, most recently from 1fc829e to e7e187b Compare March 29, 2023 10:48

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 29, 2023

ArangoGutierrez requested changes Mar 29, 2023

View reviewed changes

PiotrProkop force-pushed the expose-L3-num-closid branch from e7e187b to d1ec296 Compare March 30, 2023 07:07

marquiz reviewed Apr 4, 2023

View reviewed changes

PiotrProkop force-pushed the expose-L3-num-closid branch from d1ec296 to 3fdd43c Compare April 5, 2023 10:43

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Apr 5, 2023

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 6, 2023

PiotrProkop force-pushed the expose-L3-num-closid branch from 3fdd43c to 98c6eeb Compare April 6, 2023 07:25

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Apr 6, 2023

marquiz reviewed Apr 6, 2023

View reviewed changes

docs/usage/customization-guide.md Outdated Show resolved Hide resolved

source/cpu/rdt_amd64.go Show resolved Hide resolved

deployment/helm/values.yaml Outdated Show resolved Hide resolved

PiotrProkop force-pushed the expose-L3-num-closid branch 3 times, most recently from 2ebafe2 to c3a42fb Compare April 6, 2023 08:04

marquiz reviewed Apr 6, 2023

View reviewed changes

docs/usage/customization-guide.md Outdated Show resolved Hide resolved

Advertise RDT L3 num_closid

0e78eba

Signed-off-by: PiotrProkop <[email protected]>

PiotrProkop force-pushed the expose-L3-num-closid branch from c3a42fb to 0e78eba Compare April 6, 2023 09:23

marquiz approved these changes Apr 6, 2023

View reviewed changes

k8s-ci-robot assigned ArangoGutierrez Apr 6, 2023

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 6, 2023

ArangoGutierrez approved these changes Apr 6, 2023

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 6, 2023

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 7, 2023

k8s-ci-robot merged commit 6740224 into kubernetes-sigs:master Apr 7, 2023

marquiz mentioned this pull request Apr 12, 2023

Release v0.13.0 #1068

Closed

24 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Advertise RDT L3 num_closid #1100

Advertise RDT L3 num_closid #1100

PiotrProkop commented Mar 29, 2023

netlify bot commented Mar 29, 2023 •

edited

Loading

PiotrProkop commented Mar 29, 2023

ArangoGutierrez commented Mar 29, 2023

ArangoGutierrez left a comment

PiotrProkop commented Mar 30, 2023

ArangoGutierrez commented Mar 30, 2023

marquiz left a comment

marquiz Apr 4, 2023

PiotrProkop Apr 4, 2023

PiotrProkop Apr 4, 2023

PiotrProkop Apr 5, 2023

PiotrProkop commented Apr 5, 2023

marquiz left a comment

marquiz left a comment

ArangoGutierrez left a comment

k8s-ci-robot commented Apr 6, 2023

k8s-ci-robot commented Apr 6, 2023

marquiz commented Apr 6, 2023

PiotrProkop commented Apr 7, 2023

PiotrProkop commented Apr 7, 2023

Advertise RDT L3 num_closid #1100

Advertise RDT L3 num_closid #1100

Conversation

PiotrProkop commented Mar 29, 2023

netlify bot commented Mar 29, 2023 • edited Loading

✅ Deploy Preview for kubernetes-sigs-nfd ready!

PiotrProkop commented Mar 29, 2023

ArangoGutierrez commented Mar 29, 2023

ArangoGutierrez left a comment

Choose a reason for hiding this comment

PiotrProkop commented Mar 30, 2023

ArangoGutierrez commented Mar 30, 2023

marquiz left a comment

Choose a reason for hiding this comment

marquiz Apr 4, 2023

Choose a reason for hiding this comment

PiotrProkop Apr 4, 2023

Choose a reason for hiding this comment

PiotrProkop Apr 4, 2023

Choose a reason for hiding this comment

PiotrProkop Apr 5, 2023

Choose a reason for hiding this comment

PiotrProkop commented Apr 5, 2023

marquiz left a comment

Choose a reason for hiding this comment

marquiz left a comment

Choose a reason for hiding this comment

ArangoGutierrez left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Apr 6, 2023

k8s-ci-robot commented Apr 6, 2023

marquiz commented Apr 6, 2023

PiotrProkop commented Apr 7, 2023

PiotrProkop commented Apr 7, 2023

netlify bot commented Mar 29, 2023 •

edited

Loading