| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-05-14 | |||
| 14:34:19 | sean-k-mooney | kashyap: sure but why do this need to be in placement | |
| 14:34:25 | sean-k-mooney | as a trait | |
| 14:34:26 | stephenfin | (again, just so I know. Genuine question) | |
| 14:35:25 | sean-k-mooney | kashyap: my concern is that you appear to be trying to use tratis to carry vm configuration in fomation by making the vendor specific when we should be able to provide an abstration here | |
| 14:35:59 | efried | stephenfin: Apparently there's "has a flag you can turn on to mitigate X vulnerability" with variants of "do it on the CPU" and "do it in virt" and then there's "is manufactured without the vulnerability in the first place so no flags needed". And different variants of those exist depending on amd vs intel. | |
| 14:36:26 | kashyap | sean-k-mooney: No, you're assuming far too much. This whole discussion "exploded" when I simply noticed a few weeks ago some missing CPU traits. | |
| 14:36:39 | efried | stephenfin: I had already questioned whether we really cared about those distinctions, as opposed to "vulnerable or not". | |
| 14:36:45 | efried | apparently we do. | |
| 14:36:54 | sean-k-mooney | efried: right but the request to turn on the flag shoudl either be in the nova.conf or as a flavor extra spec or as an image proerty | |
| 14:36:56 | kashyap | And digigng more, I then thought: "If you have generic CPU flags listed, what about those that provide mitigation for security flaws.) | |
| 14:37:17 | sean-k-mooney | that flag should not be enabled by addign a required tratit to the flavor or image | |
| 14:37:24 | efried | sean-k-mooney: Right, but how do you schedule (or avoid scheduling to) a system that is or is not able to enable those flags? | |
| 14:37:46 | stephenfin | efried: OK, I wasn't aware of that. Thanks | |
| 14:37:56 | sean-k-mooney | we normalise the vendor diference in the virt dirver to a standard trait that is vendor independed | |
| 14:38:29 | kashyap | stephenfin: Your earlier question is answered in the commit message: "Notes on the "SSB"/"SSBD" confusion" | |
| 14:38:43 | kashyap | (Here: https://review.opendev.org/#/c/655193/) | |
| 14:39:10 | kashyap | (It notes how Intel and AMD addressed the same issue differently.) | |
| 14:39:19 | dansmith | kashyap: are you suggesting traits for flaws, not just traits for cpu flags? | |
| 14:40:26 | kashyap | dansmith: On "traits for flaws", I'm not sure if it makes sense. For now, all I care about is: all the required CPU flags are captured as traits | |
| 14:40:48 | kashyap | Err, s/required/missing/ | |
| 14:41:26 | sean-k-mooney | kashyap: efried suggested generic HW_CPU_HAS_MELTDOWN_CURE, HW_CPU_HAS_SPECTRE_CURE style traits in teh review | |
| 14:41:46 | kashyap | sean-k-mooney: And did you see my response that I already pasted here? | |
| 14:41:48 | sean-k-mooney | personally i think that is a better solution then what you are suggesting | |
| 14:41:48 | dansmith | :( | |
| 14:41:51 | artom | MELTDOWN_CURE sounds like a thing all parents of toddlers need | |
| 14:42:31 | kashyap | sean-k-mooney: Huh, you haven't read the response, that I posted, have you: | |
| 14:42:34 | kashyap | * The benefit of HW_CPU_HAS_SPECTRE_CURE is that it glosses over the | |
| 14:42:37 | kashyap | difference between the AMD and Intel flag names for the fix ('IBRS' vs | |
| 14:42:40 | kashyap | 'IBPB'). However, we likely going to want feature-based traits for | |
| 14:42:40 | sean-k-mooney | althoght i woudl personally go with hw_CPU_VULNERABLITY_SPECTER | |
| 14:42:42 | kashyap | Given the above it doesn't make sense the "generic roll-up traits". | |
| 14:42:42 | kashyap | other things: AES, PCID, etc. | |
| 14:42:45 | kashyap | * The HW_CPU_HAS_SPECTRE_CURE is ill-defined :-( since there are many | |
| 14:42:48 | kashyap | Spectre-related bugs. | |
| 14:43:01 | kashyap | s/"sense the"/"sense to have"/ | |
| 14:43:24 | sean-k-mooney | yes i did see that comment | |
| 14:43:34 | kashyap | sean-k-mooney: If you have a _clear_ proposal, please write it as a (not too long) comment in the change, I'd appreciate it. | |
| 14:43:51 | kashyap | For now, I'll go with the structure I discussed with efried earlier. | |
| 14:44:57 | sean-k-mooney | ok i will. but to be clear adding a required trait should have 0 impact on the xml generation and should not enable a cpu feature flag in the libvirt xml for that cpu feature | |
| 14:45:53 | sean-k-mooney | if the host is configed in the nova.conf to enable that feature that is fine but reporting/requireing a trait should not change the behavior of the vm form another that was schduled to that host without requiring the traits | |
| 14:46:12 | dansmith | fwiw, I *hate* the "has the cure" trait | |
| 14:46:23 | kashyap | dansmith: Hehe, that's not going to happen :-) | |
| 14:46:57 | sean-k-mooney | dansmith: well what about the other fomulation of hw_cpu_vulnerablity_XYZ | |
| 14:47:01 | kashyap | dansmith: It was a thinking-out-loud point from Eric. | |
| 14:47:13 | edleafe | dansmith: it's as bad as "doesn't have the vulnerability" trait | |
| 14:47:38 | dansmith | sean-k-mooney: I don't like that either | |
| 14:47:42 | sean-k-mooney | which the cpu feature flags are in another vail | |
| 14:47:54 | dansmith | "is broken" or "was broken" or "has a software fix" are terrible traits to me | |
| 14:48:02 | sean-k-mooney | dansmith: ya i dont like tracking vulnerablity in placement period | |
| 14:48:07 | dansmith | sean-k-mooney: agreen | |
| 14:48:09 | dansmith | *agreed | |
| 14:48:30 | dansmith | I also don't think that tracking cpu flags in placement for non-feature flags makes sense | |
| 14:48:57 | dansmith | AVX2, yes... flags that just imply that a microcode fix has been applied, not so much | |
| 14:49:01 | sean-k-mooney | i stil am concerned that adding any of the security realted flag give me a vector to upload an image with a require/forbidn trait and target a vulnerable host | |
| 14:49:16 | dansmith | sean-k-mooney: for serious | |
| 14:49:17 | kashyap | dansmith: How about this idea (that I discussed here on PS:4 as "Another Idea" - https://review.opendev.org/#/c/655193/): | |
| 14:49:20 | kashyap | Make Nova check the 'sysfs' (/sys/devices/system/cpu/vulnerabilities) directory for vulnerabilities. And e.g. if it reports "Vulnerable" (instead of "Mitigation") for Meltdown (or other flaws), print a warning that the host is vulnerable, and on next release refuse to start the VMs | |
| 14:49:46 | sean-k-mooney | dansmith: :) | |
| 14:50:16 | dansmith | kashyap: I have a hard time understanding why this is a nova thing at all.. configs to turn on compatibility as needed makes sense, since we are controlling the definition of the VMs, | |
| 14:50:20 | sean-k-mooney | kashyap: i dont think its novas place to do that | |
| 14:50:28 | dansmith | but the management of the host-level patching is not our deal, IMHO | |
| 14:50:33 | dansmith | what sean-k-mooney said | |
| 14:51:01 | kashyap | dansmith: Nah, we don't _patch_ it. But isn't it fair game for Nova to tell that: "hey, your host is vulnerable to critical flaws, launching VMs there is dangerous?" | |
| 14:51:07 | dansmith | same reason we don't refuse to boot instances because we're concerned that our cpu fans are spinning too slowly | |
| 14:51:10 | dansmith | kashyap: not IMHO | |
| 14:51:13 | kashyap | I see. | |
| 14:51:25 | kashyap | Just wondering out loud. My umbilical cord isn't tied to that idea :-) | |
| 14:52:47 | sean-k-mooney | kashyap: depending on your enviornment (a air gapped secure datacenter) you ligitamately may want to have a vulnerable system for the performace improvments. i agree with dansmith that it should not be novas role to dictate your securety and threat model | |
| 14:53:00 | kashyap | One would think it's reasonable for Nova to prompt admins to secure their hypervisors. | |
| 14:53:11 | dansmith | sean-k-mooney: yeah, definitely agree.. I have unpatched systems for reasons :) | |
| 14:53:18 | kashyap | sean-k-mooney: Yeah, I've considered that point -- on someone _intentionally_ wanting to run unpatched | |
| 14:53:24 | kashyap | ... precisely for the perf reasons. | |
| 14:53:47 | sean-k-mooney | kashyap: you could make the same argument for libvirt if you go that route or qemu | |
| 14:54:10 | dansmith | or fan speed, or cpu temperature, or firmware patch levels, or os patch levels, or .... | |
| 14:54:21 | kashyap | Well, that doesn't make sense in the _common_ case for those components. Not everyone is sitting there with "air-gapped secure datacenters". | |
| 14:54:26 | sean-k-mooney | the kernel warns the user and i think that is where the warning should be | |
| 14:56:28 | kashyap | sean-k-mooney: Are you referring to the 'sysfs' directory as "kernel warns"? | |
| 14:57:01 | sean-k-mooney | kashyap: well i ment the message in dmesg form the early boot but sysfs can be used later at runtime | |
| 14:57:23 | sean-k-mooney | its also show in thing like lscpu | |
| 15:01:30 | sean-k-mooney | actully its not in lscpu but is in /proc/cpuinfo in the bugs line "bugs : cpu_meltdown spectre_v1 spectre_v2 spec_store_bypass l1tf" | |
| 15:01:56 | kashyap | sean-k-mooney: Yeah, it's _not_ in `lscpu` | |
| 15:02:14 | sean-k-mooney | ya it would be a nice enhancement to add it | |
| 15:02:22 | kashyap | Right, it's in /proc/cpuinfo. And of course, 'sysfs' is a better place. | |
| 15:04:33 | sean-k-mooney | yep and you allso get entires in dmesg as the are detected on kenel boot with the mitigations that are applied | |
| 15:04:36 | sean-k-mooney | like this | |
| 15:04:39 | sean-k-mooney | 0.033797] Spectre V2 : Mitigation: Full generic retpoline | |
| 15:04:41 | sean-k-mooney | [ 0.033797] Spectre V2 : Spectre v2 / SpectreRSB mitigation: Filling RSB on context switch | |
| 15:04:43 | sean-k-mooney | [ 0.033798] Spectre V2 : Spectre v2 mitigation: Enabling Indirect Branch Prediction Barrier | |
| 15:04:45 | sean-k-mooney | [ 0.033798] Spectre V2 : Enabling Restricted Speculation for firmware calls | |
| 15:04:47 | sean-k-mooney | [ 0.033799] Spectre V2 : Spectre v2 cross-process SMT mitigation: Enabling STIBP | |
| 15:04:49 | sean-k-mooney | [ 0.033800] Speculative Store Bypass: Mitigation: Speculative Store Bypass disabled via prctl and seccomp | |
| 15:05:19 | sean-k-mooney | which is the same info that gets stroed in cat /sys/devices/system/cpu/vulnerabilities/* | |
| 15:05:55 | openstackgerrit | Gorka Eguileor proposed openstack/nova master: Use os-brick locking for volume attach and detach https://review.opendev.org/614190 | |
| 15:13:26 | mriedem | bauzas: dansmith: can one of you hit this stable/stein test-only backport to keep things moving? https://review.opendev.org/#/c/658929/ | |
| 15:20:25 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Stop handling 'update_cells' on 'BandwidthUsage.create' https://review.opendev.org/651303 | |
| 15:20:25 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Remove 'instance_update_from_api' https://review.opendev.org/651302 | |
| 15:20:26 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Remove cells code https://review.opendev.org/651306 | |
| 15:20:26 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Stop handling cells v1 for instance naming https://review.opendev.org/651304 | |
| 15:20:27 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Remove conductor_api and _last_host_check from manager.py https://review.opendev.org/651059 | |
| 15:30:42 | openstackgerrit | Adrian Chiris proposed openstack/nova master: [FUP] Follow-up patch for SR-IOV live migration https://review.opendev.org/659101 | |