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

config-linux: Add Intel RDT CMT and MBM Linux support #1076

Merged
merged 1 commit into from Sep 10, 2021

Conversation

Creatone
Copy link
Contributor

@Creatone Creatone commented Nov 6, 2020

Add support for Intel Resource Director Technology (RDT) /
Cache Monitoring Technology (CMT) and Memory Bandwidth Monitoring (MBM).

Example:

"linux": {
    "intelRdt": {
        "enableCMT": true,
        "enableMBM": true
    }
}

This is the prerequisite of this runc proposal:
opencontainers/runc#2519

For more information about Intel RDT CMT and MBM, please refer to:
opencontainers/runc#2519

Signed-off-by: Paweł Szulik pawel.szulik@intel.com

@Creatone
Copy link
Contributor Author

Creatone commented Nov 6, 2020

@xiaochenshen

config-linux.md Outdated Show resolved Hide resolved
specs-go/config.go Outdated Show resolved Hide resolved
specs-go/config.go Outdated Show resolved Hide resolved
specs-go/config.go Outdated Show resolved Hide resolved
@xiaochenshen
Copy link
Contributor

"linux": {
    "intelRdt": {
        "monitoring": true
    }
}

@Creatone
Thank you for adding this to support Intel RDT monitoring features for OCI runtime-spec and runc.
Apart from the nitpicking, this change LGTM.

Copy link
Contributor

@xiaochenshen xiaochenshen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@caniszczyk
Copy link
Contributor

RFC @opencontainers/runtime-spec-maintainers

Creatone pushed a commit to Creatone/runc that referenced this pull request Nov 17, 2020
…l RDT CMT and MBM Linux support

Signed-off-by: Paweł Szulik <pawel.szulik@intel.com>
Creatone pushed a commit to Creatone/runc that referenced this pull request Nov 17, 2020
…l RDT CMT and MBM Linux support

Signed-off-by: Paweł Szulik <pawel.szulik@intel.com>
config-linux.md Outdated
@@ -549,6 +549,7 @@ The following parameters can be specified for the container:

* If `closID` is set, and neither of `l3CacheSchema` and `memBwSchema` are set, runtime MUST check if corresponding pre-configured directory `closID` is present in mounted `resctrl`. If such pre-configured directory `closID` exists, runtime MUST assign container to this `closID` and [generate an error](runtime.md#errors) if directory does not exist.

* **`monitoring`** *(boolean OPTIONAL)* - specifies if Intel RDT monitoring features (CMT and MBM) should be enabled
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Brief description about RDT, CMT, and MBM would be appreciated

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Description of IntelRDT is above. Added CMT and MBM information.

Creatone pushed a commit to Creatone/runc that referenced this pull request Nov 17, 2020
…l RDT CMT and MBM Linux support

Signed-off-by: Paweł Szulik <pawel.szulik@intel.com>
config-linux.md Outdated Show resolved Hide resolved
config-linux.md Outdated Show resolved Hide resolved
@xiaochenshen
Copy link
Contributor

LGTM (apart from the nitpicking).

@xiaochenshen
Copy link
Contributor

LGTM

@tianon
Copy link
Member

tianon commented Nov 24, 2020

Do you foresee any reason for this to need to be more than just a simple boolean in the future? As an example, can each of the monitoring features mentioned be enabled separately, and is there a use case for doing so?

@Creatone
Copy link
Contributor Author

@tianon Good point. It would be a good thing that the user can choose which features want to use.

@xiaochenshen
Copy link
Contributor

Do you foresee any reason for this to need to be more than just a simple boolean in the future? As an example, can each of the monitoring features mentioned be enabled separately, and is there a use case for doing so?

@tianon @Creatone
It makes sense to split Intel RDT monitoring features (CMT and MBM) in config because we could selectively enable/disable Intel RDT sub-features by kernel command line parameter rdt= even though hardware supports both monitoring features. See more details in option rdt= in https://www.kernel.org/doc/Documentation/admin-guide/kernel-parameters.txt

@vbatts
Copy link
Member

vbatts commented Nov 30, 2020

Generally speaking, is this a per container feature? reading the docs looks like it is a kernel global, so does this really only apply for kata-containers that will boot a container/pod into its own kernel that this could apply?

Also, it looks like commit 8f6cf10 has dangling whitespace issues.

@xiaochenshen
Copy link
Contributor

Generally speaking, is this a per container feature?

@vbatts Yes, this is a per container feature which depends on Intel RDT kernel support on host.
You could find more details about Intel RDT support for OCI/runc container from:
opencontainers/runc#433
opencontainers/runc#1596
opencontainers/runc#2292

reading the docs looks like it is a kernel global, so does this really only apply for kata-containers that will boot a container/pod into its own kernel that this could apply?

This is not targeted for VM or VM based container (e.g., Kata containers).

@vbatts
Copy link
Member

vbatts commented Dec 1, 2020

gotcha

LGTM

Approved with PullApprove

Copy link
Member

@tianon tianon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

(but I'd love to get at least one more @opencontainers/runtime-spec-maintainers member to take a look 🙏 ❤️)

Creatone pushed a commit to Creatone/runc that referenced this pull request Dec 8, 2020
…l RDT CMT and MBM Linux support

Signed-off-by: Paweł Szulik <pawel.szulik@intel.com>
Creatone pushed a commit to Creatone/runc that referenced this pull request Dec 28, 2020
…l RDT CMT and MBM Linux support

Signed-off-by: Paweł Szulik <pawel.szulik@intel.com>
specs-go/config.go Outdated Show resolved Hide resolved
config-linux.md Outdated Show resolved Hide resolved
config-linux.md Outdated Show resolved Hide resolved
Copy link
Contributor

@kolyshkin kolyshkin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The commits need to be squashed. It _usually) does not make sense to introduce something and then replace it with something else in the same PR.

Left some minor comments as well. PTAL @Creatone

@Creatone
Copy link
Contributor Author

@kolyshkin PTAL

config-linux.md Outdated Show resolved Hide resolved
@kolyshkin
Copy link
Contributor

@Creatone the example in the commit message no longer reflects reality -- please update.

config-linux.md Outdated Show resolved Hide resolved
Add support for Intel Resource Director Technology (RDT) /
Cache Monitoring Technology (CMT) and Memory Bandwidth Monitoring (MBM).

Example:

"linux": {
    "intelRdt": {
        "enableCMT": true,
        "enableMBM": true
    }
}

This is the prerequisite of this runc proposal:
opencontainers/runc#2519

For more information about Intel RDT CMT and MBM, please refer to:
opencontainers/runc#2519

Signed-off-by: Paweł Szulik <pawel.szulik@intel.com>
@Creatone
Copy link
Contributor Author

@kolyshkin PTAL

Creatone pushed a commit to Creatone/runc that referenced this pull request Jul 13, 2021
…l RDT CMT and MBM Linux support

Signed-off-by: Paweł Szulik <pawel.szulik@intel.com>
@Creatone
Copy link
Contributor Author

@caniszczyk @crosbymichael @cyphar @dqminh @giuseppe @hqhq @mrunalp @tianon @vbatts @vishh PTAL, it's waiting from Nov 2020

Copy link
Member

@giuseppe giuseppe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry for the delay.

LGTM

@vbatts vbatts merged commit 0d6cc58 into opencontainers:master Sep 10, 2021
@gifhupp

This comment has been minimized.

@gifhupp

This comment has been minimized.

@AkihiroSuda AkihiroSuda mentioned this pull request Jan 24, 2023
@AkihiroSuda AkihiroSuda added this to the v1.1.0 milestone Feb 1, 2023
@AkihiroSuda AkihiroSuda mentioned this pull request Jun 26, 2023
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants