| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2022-05-12 | |||
| 17:37:12 | sean-k-mooney | so it proably makes sense to at least make that change | |
| 17:37:52 | sean-k-mooney | i was ment ot re review the viommu spec today too sorry ricolin | |
| 17:38:21 | sean-k-mooney | ill try and get back to it tomorrow but likely wont get to it today | |
| 18:15:07 | opendevreview | Artom Lifshitz proposed openstack/nova stable/wallaby: DNM: Testing stuff https://review.opendev.org/c/openstack/nova/+/841626 | |
| 23:29:13 | ricolin | sean-k-mooney: yeah, I can help with the mlock patch, and thanks for your review:) | |
| #openstack-nova - 2022-05-13 | |||
| 07:04:10 | opendevreview | Wenping Song proposed openstack/os-traits master: Update python testing as per zed cycle testing runtime https://review.opendev.org/c/openstack/os-traits/+/841682 | |
| 08:42:20 | opendevreview | Wenping Song proposed openstack/placement master: Update python testing as per zed cycle testing runtime https://review.opendev.org/c/openstack/placement/+/841690 | |
| 08:42:39 | opendevreview | Wenping Song proposed openstack/placement master: Update python testing as per zed cycle testing runtime https://review.opendev.org/c/openstack/placement/+/841690 | |
| 09:20:32 | opendevreview | Wenping Song proposed openstack/os-resource-classes master: Update python testing as per zed cycle testing runtime https://review.opendev.org/c/openstack/os-resource-classes/+/841700 | |
| 09:46:43 | opendevreview | Wenping Song proposed openstack/os-resource-classes master: Update python testing as per zed cycle testing runtime https://review.opendev.org/c/openstack/os-resource-classes/+/841700 | |
| 10:12:03 | sean-k-mooney | ricolin: +1 on the spec | |
| 10:12:23 | sean-k-mooney | ricolin: i have resotred the mlock patch too so feel free to updated it | |
| 10:12:29 | ricolin | thanks sean-k-mooney :) | |
| 10:15:28 | sean-k-mooney | ricolin: if you adress the extra spec name im baicly +2 ill read over it again when you respin but no rush | |
| 10:24:09 | opendevreview | Rico Lin proposed openstack/nova-specs master: Add vIOMMU device support for libvirt driver https://review.opendev.org/c/openstack/nova-specs/+/840310 | |
| 10:25:12 | ricolin | sean-k-mooney: done ^^^ :) | |
| 10:28:37 | sean-k-mooney | +2 :) am milestone 1 i think is thursday. assuming gibi or others approve the spec can you submit the patch to os-traits to add the traits | |
| 10:28:55 | sean-k-mooney | if we merge that before tursday then the traits can be included in the m1 relases of os-traits | |
| 10:30:37 | sean-k-mooney | nova's unit tests need the traits to be in a release version of os-traits to pass if its not merge by then its not really a big deal we will just do another release whenever they land and the nova code look good to merge | |
| 10:51:22 | stephenfin | sean-k-mooney: gibi: melwitt: Reviewed that PCI in placement spec. Looks pretty good, though I'm convinced you're making an unnecessary rod for your own back in trying to port that dynamic PF/VF logic to placement land ;-) | |
| 11:00:54 | sean-k-mooney | stephenfin: ya it is complicating things | |
| 11:01:15 | sean-k-mooney | stephenfin: that use case is really only for sriov nics | |
| 11:01:23 | sean-k-mooney | as in for neuton ports | |
| 11:01:31 | sean-k-mooney | i dont think it really exists for pci alias | |
| 11:02:09 | sean-k-mooney | so we can kind of punt. technialy it applise to the alias too but i dont think that is a common usecase there | |
| 11:02:45 | sean-k-mooney | stephenfin: you might want to look over ricolin's spec for the viommu support | |
| 11:02:51 | sean-k-mooney | its pretty short | |
| 11:03:17 | sean-k-mooney | https://review.opendev.org/c/openstack/nova-specs/+/840310 | |
| 11:04:00 | stephenfin | sure | |
| 11:06:36 | gibi | stephenfin: so you suggest that deciding to whitelist either the PF or its VFs are rarely used? I think the opposite. It is so easy to just whitelist everthing under 0000:81:* both PF and VF, without thingking to much, and then start using the compute for both direct and direct-physical ports, it just works, so I think there is a lot of deployment out there that might not need to whitelist both but | |
| 11:06:42 | gibi | they did | |
| 11:07:44 | gibi | if we start rejecting such config there will be a lot of pain figuring out a whitelist in these deployments that matches the current consumption | |
| 11:08:10 | sean-k-mooney | im not sure really. im tempted to say perhaps we sould only support that if you are not trackign devices in placment | |
| 11:08:45 | sean-k-mooney | and in the mvp implement supprot for only tracking deivces as PFs or VFs | |
| 11:09:03 | sean-k-mooney | and then we could add the mixed suport after if needed | |
| 11:09:04 | stephenfin | No, I'm suggesting people wanting to use both 'direct' and 'direct-physical' on a single host is likely rare. I think it's reasonable to insist that users that use wildcard device addresses (or vendor/device IDs) choose whether they want to consume the VFs or PFs | |
| 11:09:47 | sean-k-mooney | stephenfin: well your suggestiong they make that dissionc staticaly at deployment time | |
| 11:09:56 | stephenfin | At the moment, a user can dynamically choose whether they consume the PF or one or more of the VFs. I think moving to a static model makes sense now | |
| 11:09:59 | stephenfin | Yup | |
| 11:10:01 | sean-k-mooney | rather then dynmical as workloads are schulded | |
| 11:10:05 | stephenfin | exactly | |
| 11:10:32 | sean-k-mooney | i think pf passhtough is much less common then vf | |
| 11:10:39 | gibi | I have no problems forcing this to new deployments, but I still believe upgrade will be a pain | |
| 11:11:07 | stephenfin | *borderline non-existent (I say, with no actual evidence either way :) However, it seems like an odd thing to do, especially when we don't support nested virt) | |
| 11:11:10 | gibi | but yeah, we can push out the pain to the future by keepin the pci tracking in placement optional | |
| 11:11:57 | sean-k-mooney | stephenfin: well my evidence is that live migrtation, cold migration and unshelve have basicaly been broken since the feature was added until like 2 cylces ago | |
| 11:11:59 | stephenfin | I mean, it's been more than three years and people can still avoid tracking of pinned CPUs in placement | |
| 11:12:20 | stephenfin | so that can can be kicked endlessly down the road | |
| 11:12:44 | sean-k-mooney | so while i know we have some customer using PFs those custoemr also have static deployments | |
| 11:13:28 | sean-k-mooney | gibi: i guess its really up to you | |
| 11:13:36 | sean-k-mooney | if you want to include it in the mvp | |
| 11:13:40 | gibi | OK, I see an agreement forming. Let's implement PCI tracking with placement without the dynamic selection. Keep the everything working as today if the PCI tracking in placement is disabled | |
| 11:13:48 | sean-k-mooney | i think it could be a patch at the end of the seirse by the way | |
| 11:14:36 | sean-k-mooney | gibi: sound good to me that means the prefilter will not have an auto mode | |
| 11:14:42 | gibi | then when everythin (except the dynamic thing works with placemnet) deprecate the old way | |
| 11:14:46 | gibi | and wait for feedback | |
| 11:15:18 | gibi | yeah that means no auto mode | |
| 11:15:28 | sean-k-mooney | you will opt in on the compute with the new config option and opt in on schduler by enabling prefilter | |
| 11:15:29 | gibi | operator needs to first enable tracking in the compute config | |
| 11:15:36 | gibi | then enable prefiltering in the scheduler | |
| 11:15:41 | sean-k-mooney | yep | |
| 11:16:31 | gibi | and if somebody only opt in on a set of computes then enables the prefilter then we say sorry you lost the non enabled computes from the PCI scheduling | |
| 11:16:48 | stephenfin | I think lack of auto mode is a good thing. I called that out in the review as something weird (and I think melwitt had similar concerns) | |
| 11:16:54 | stephenfin | The less magic, the better | |
| 11:17:41 | sean-k-mooney | ack its nice form an opts perspectvie if an only if it means they dont have to do anything on upgrade | |
| 11:17:53 | stephenfin | sean-k-mooney: woah, that vIOMMU thing has got way bigger. I was proposing enabling by default on supported platforms with zero configurability. We now have...three knobs? :-O | |
| 11:18:40 | stephenfin | I've asked ricolin to clarify the need for each knob since it's not at all obvious from reading the spec. If they're really necessary, we'll need this info to document the extra specs. | |
| 11:18:46 | gibi | upgrading to Zed (if it lands) with default config will mean only the old PCI scheduling will be used by nova. So no real upgrade impact. Then operators needs to enable the new tracking | |
| 11:18:49 | sean-k-mooney | stephenfin: yep. the model, bit with which i dont like exposing but is required and locked memroy wich is technialy unrealted | |
| 11:19:23 | sean-k-mooney | but they need locked memroy for there specific hardware and you can only get that with realtim or sev today | |
| 11:19:42 | sean-k-mooney | gibi: yep with no config changes old behavior | |
| 11:19:58 | gibi | so we need to incentivise ops to enable the new behavior | |
| 11:20:00 | stephenfin | At risk of pre-empting discussion on the spec, why is the model necessary? Are these guest OS behavior implications or something? Is there not a sane default we can pick? | |
| 11:20:04 | sean-k-mooney | so no upgrade impact by defualt but we will ahve to document how to move form one to the other | |
| 11:20:39 | sean-k-mooney | stephenfin: the sane default would be virtio but it need a very new libvirt/qemu | |
| 11:21:03 | sean-k-mooney | we cant use intel because well there usecase is for aarch64 servers | |
| 11:21:33 | sean-k-mooney | we could choose dynmically i guess | |
| 11:21:47 | stephenfin | And I'm guessing we can't check what the libvirt version is and use 'virtio' if libvirt > 8.3.0 else 'intel'/'smmuv3' for Intel/ARM respectively? | |
| 11:21:49 | sean-k-mooney | based on the machine-type but i dont like the live migration implciations of that | |
| 11:22:01 | stephenfin | So long as we record the model used in system metadata, we should be fine | |
| 11:22:05 | sean-k-mooney | well we could but ya | |
| 11:22:09 | stephenfin | (i.e. so we don't change during rebuild) | |
| 11:22:14 | stephenfin | *migration | |
| 11:22:19 | sean-k-mooney | record in system_metadta or elsewhere for migration | |
| 11:22:23 | stephenfin | Yup | |
| 11:22:46 | sean-k-mooney | i was hoping we woudl only expose the model extra spec initally | |
| 11:22:51 | stephenfin | In case it's not obvious, I hate exposing knobs unless they're really useful. If people want all the knobs, use libvirt/oVirt :) | |
| 11:23:15 | sean-k-mooney | yep i know it had 6 i think at one point | |
| 11:23:25 | sean-k-mooney | i have been slowing getting it down to the minimal set | |
| 11:25:45 | stephenfin | what about aw_bits? | |
| 11:25:54 | sean-k-mooney | stephenfin: by the way im not sure if we can just turn the viommu on always or if there are performance overhead even if you dont explicty try to use it for say realtime guests | |
| 11:26:06 | sean-k-mooney | stephenfin: that i really did not want to expose | |
| 11:26:53 | sean-k-mooney | stephenfin: mnaser left a comment about why they needed it i dont knwo if we can always set it to the max support or if it need to be tuneable | |
| 11:29:23 | sean-k-mooney | actully it was on irc let me see if i can find it | |
| 11:31:12 | sean-k-mooney | stephenfin: i actully tought you exposed that in your patch by the way | |
| 11:31:16 | sean-k-mooney | the aw_bit | |
| 11:31:56 | stephenfin | Nope https://review.opendev.org/c/openstack/nova/+/830646/1/nova/virt/libvirt/config.py#3676 | |
| 11:32:27 | sean-k-mooney | oh so ricolin added it | |
| 11:33:07 | sean-k-mooney | https://review.opendev.org/c/openstack/nova/+/830646/5/nova/virt/libvirt/config.py#3700= | |
| 11:33:21 | sean-k-mooney | ah you just found it | |