| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-05-14 | |||
| 14:30:41 | kashyap | Heh | |
| 14:30:54 | efried | we have to categorize the intel- and amd-specific ones anyway. The strings that come out in the end are already figured. It's a question of whether to have INTEL_FOO and AMD_BAR in x86.py or have separate namespace dirs. | |
| 14:30:59 | sean-k-mooney | kashyap: my full stance on this is we should not do it peiord but if we must add these i would prefer them to be generic not vender specific and only as a last resouce expose vendor specific tratits | |
| 14:31:04 | stephenfin | but now that I've seen this, I've decided my first impression made some sense at least | |
| 14:31:43 | stephenfin | sean-k-mooney: Aye, agreed on the latter part of that at least. Not sure about the basis for the former so can't comment | |
| 14:32:10 | stephenfin | efried: Do we? Why? | |
| 14:32:19 | stephenfin | (just for context) | |
| 14:32:21 | kashyap | sean-k-mooney: You seem to be too hung up on 'generic'. It _doesn't_ fully make sense in this case. | |
| 14:32:39 | efried | I'll go out on a limb and say that anything x86 that's supported by both AMD and Intel should go into the x86 namespace, and random manufacturer that owns 0.0000001% of the x86 market who doesn't support that feature can just not turn it on. | |
| 14:33:03 | sean-k-mooney | kashyap: i think this is a missue use of traits and im pushing back becasue i dont think you have mad the case of why we should be exposing this in a non generic way | |
| 14:33:18 | efried | stephenfin: Because some of them are mutually exclusive. | |
| 14:33:20 | kashyap | sean-k-mooney: The case is the vulnerabilities cannot be trivially mapped into 'generic' traits> | |
| 14:33:33 | sean-k-mooney | traits are ment to be an abstration over hardware and are not intened to enable specifric feature flags in the emulated cpu | |
| 14:33:52 | efried | well now I can get behind that ^ | |
| 14:33:54 | kashyap | sean-k-mooney: And some people may want to run _without_ "L1TF" (which is Intel-only) | |
| 14:34:12 | stephenfin | efried: OK. How would that manifest itself in a way we care about? | |
| 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 | |