Skip to content
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

Added more Intel RDT capability discovery: CMT,MBM,MBA #120

Merged
merged 1 commit into from May 24, 2018

Conversation

okartau
Copy link
Contributor

@okartau okartau commented Apr 11, 2018

Added more Intel RDT capability discovery: CMT,MBM,MBA

Added Memory Bandwith Allocation (MBA) capability discovery.
Refined RDT monitoring capability detection;
Cache Monitoring Technology (CMT) and
Memory Bandwidth Monitoring (MBM)
capabilities can be detected separately.

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Apr 11, 2018
@okartau
Copy link
Contributor Author

okartau commented Apr 12, 2018

as I don't currently have suitable HW, I cant myself verify the positive code-path here, i.e. when MBA capability is detected. The logic relies on intel-cmt-cat, so if that can be trusted, the code here should work. But we may want to wait with merging until the NFD code is verified as well, just to be sure.

@balajismaniam
Copy link
Contributor

@okartau Thanks for opening this PR. Like you mentioned, we should wait till we can fully test the changes e2e.

Added Memory Bandwith Allocation (MBA) capability discovery.
Refined RDT monitoring capability detection;
Cache Monitoring Technology (CMT) and
Memory Bandwidth Monitoring (MBM)
capabilities can be detected separately.
@k8s-ci-robot 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 25, 2018
@okartau okartau changed the title Added Intel RDT Memory Bandwith Allocation capability discovery Added more Intel RDT capability discovery: CMT,MBM,MBA Apr 25, 2018
@okartau
Copy link
Contributor Author

okartau commented Apr 25, 2018

I added more capabilities to same PR, hope thats OK. Note that I leave already existing RDTMON but I added new labels for CMT and MBM.
RDTCMT in most cases overlaps with RDTMON, right?
So we may want to skip adding it.
But as there are separate bits returned by lcpuid(0xf,1) encoding CMT and MBM,
expressing both capabilities separately is kind of complete solution, isnt it.
I have verified this code on E5 v3 and E5v4, and it shows correctly that v3 CPU
has CMT support but no MBM support

@marquiz
Copy link
Contributor

marquiz commented May 24, 2018

I tested this on a machine with an Intel® Xeon® Platinum 8170M processor, with the following results:

[mlehtone@sklep0 node-feature-discovery]$ docker run 5c05eef78c7b --oneshot --no-publish
...
E0524 14:18:24.268227       1 rdt.go:69] support for RDT L2 allocation was not detected: exit status 1
2018/05/24 14:18:24 node.alpha.kubernetes-incubator.io/nfd-rdt-RDTMON = true
2018/05/24 14:18:24 node.alpha.kubernetes-incubator.io/nfd-rdt-RDTCMT = true
2018/05/24 14:18:24 node.alpha.kubernetes-incubator.io/nfd-rdt-RDTMBM = true
2018/05/24 14:18:24 node.alpha.kubernetes-incubator.io/nfd-rdt-RDTL3CA = true
2018/05/24 14:18:24 node.alpha.kubernetes-incubator.io/nfd-rdt-RDTMBA = true
2018/05/24 14:18:24 node.alpha.kubernetes-incubator.io/nfd-pstate-turbo = true
2018/05/24 14:18:24 node.alpha.kubernetes-incubator.io/nfd-memory-numa = true
E0524 14:18:24.276432       1 network.go:48] SR-IOV not supported for network interface: eth0: open /sys/class/net/eth0/device/sriov_totalvfs: no such file or directory
2018/05/24 14:18:24 discovery failed for source [selinux]: Failed to detect the status of selinux, please check if the system supports selinux and make sure /sys on the host is mounted into the container: open /host-sys/fs/selinux/enforce: no such file or directory
2018/05/24 14:18:24 continuing ...

Looks good, let's merge this

@poussa
Copy link
Contributor

poussa commented May 24, 2018

+1

@balajismaniam balajismaniam merged commit e984154 into kubernetes-sigs:master May 24, 2018
@marquiz marquiz mentioned this pull request Jun 15, 2018
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants