Earlier  
Posted Nick Remark
#openstack-nova - 2019-05-14
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
15:51:45 openstackgerrit Stephen Finucane proposed openstack/nova master: Add an online migration for PciDevice.uuid https://review.opendev.org/530905
15:51:45 openstackgerrit Stephen Finucane proposed openstack/nova master: Modify PciDevice.uuid generation code https://review.opendev.org/530487
15:52:53 stephenfin dansmith: If you have any free time today, could you take a look at those two patches again? ^ They're the sixth and seventh oldest ones I have, respectively :)
15:56:08 dansmith stephenfin: is there something that is going to need this?
15:56:19 dansmith not that it's a bad thing to have proper uuids, but... just wondering

Earlier   Later