| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2023-11-02 | |||
| 14:53:40 | dansmith | okay I'm confused | |
| 14:53:40 | sean-k-mooney | artom: https://github.com/openstack/neutron-lib/blob/master/neutron_lib/constants.py#L676-L681 | |
| 14:53:49 | dansmith | I thought ya'll said the opposite earlier | |
| 14:53:52 | bauzas | changing the value once you create mdevs is a breaking change that we don't support | |
| 14:53:57 | sean-k-mooney | artom: so the api will not accpet it and we are missing the osc support too | |
| 14:54:01 | dansmith | right I understand that part | |
| 14:54:18 | dansmith | are we going to gmeet? | |
| 14:54:26 | bauzas | sure I can fire one | |
| 14:54:33 | dansmith | just saying, since artom showed up we | |
| 14:54:37 | dansmith | are all muddy here :) | |
| 14:55:03 | artom | Yep, I barged in like a rhino on a sugar rush, apologies | |
| 14:55:13 | bauzas | https://meet.google.com/hqy-bgjb-muk for people fancy discussing nvidia nightmare | |
| 14:55:24 | sean-k-mooney | dansmith: for what its worth the patch bauzas proposed should work given the hardware that is aviabel to buy today | |
| 14:55:53 | sean-k-mooney | its jsut depening on some asusmintions we have in nova and in how the current hardware works | |
| 16:49:10 | opendevreview | Will Szumski proposed openstack/nova stable/yoga: Ironic nodes with instance reserved in placement https://review.opendev.org/c/openstack/nova/+/867912 | |
| 17:41:27 | jovial | Is the nova-emulation job busted on stable/yoga? Was interested in progressing this patch: https://review.opendev.org/c/openstack/nova/+/898554 | |
| 17:42:31 | bauzas | sean-k-mooney: dansmith: so, if you don't mind, I'm gonna work on the VF filtering case with max_instances config option | |
| 17:42:45 | bauzas | and then we could nitpick on what we want | |
| 17:44:23 | dansmith | yep | |
| 17:44:46 | bauzas | dansmith: fwiw, there is also https://review.opendev.org/c/openstack/nova/+/899406 | |
| 17:45:07 | bauzas | with that patch, pas-ha[m] is forcing operators to explicitely define all the device_addresses | |
| 17:45:35 | bauzas | but I personnally think that adding a max_instances config option is painless for ops | |
| 17:45:41 | bauzas | so we could mix both | |
| 17:46:15 | bauzas | meaning that we would force ops to either explicitelyt specify the device addresses *or* give the number of VFs that nova needs to create | |
| 17:46:22 | bauzas | this sounds very consistent to me | |
| 17:50:21 | dansmith | max_instances seems much more useful to me so people don't have to encode pci device addresses in their config *just* to avoid the broken behavior | |
| 17:51:33 | bauzas | yeah so we will keep the existing behaviour but add another option for automatic discovery | |
| 17:51:45 | bauzas | dansmith: I definitely appreciate your help on that one | |
| 17:52:00 | bauzas | and I really want to give you review credits back on your own series | |
| 17:57:51 | sean-k-mooney | dansmith: i have some concerns with allow config with out pci adresses | |
| 17:58:11 | sean-k-mooney | i.e. if you allow some of your gpus to be used for pci passthogu and other for vgpu | |
| 17:58:36 | dansmith | right, that's a good reason *to* do it, but not a good reason to *require* it, IMHO | |
| 17:58:59 | sean-k-mooney | if you dont use device_address in that case we coudl passthough a device (that was allowed in the pci devspec) but not disalloed in the mdev config section | |
| 17:59:37 | sean-k-mooney | dansmith: i dont know i always felt that its a psudo security isseu to not explcitly list devices we can expose to a guest | |
| 17:59:45 | sean-k-mooney | i get that it sucks to need to be explict | |
| 17:59:54 | sean-k-mooney | but that the trade off | |
| 18:00:40 | dansmith | but in the mdev case you're not just blindly exposing them, | |
| 18:00:42 | sean-k-mooney | to be clear we have this tradeoff in the pci tracker too | |
| 18:00:50 | dansmith | you've already opted into "all devices with this mdev type" right? | |
| 18:01:12 | sean-k-mooney | yep so its the same as just putting vendor_id and product_id in the pci dev_spec | |
| 18:01:22 | sean-k-mooney | aka the pci whitelist | |
| 18:01:28 | dansmith | I don't think that's the same really | |
| 18:01:45 | opendevreview | Stephen Finucane proposed openstack/nova master: docs: Add documentation on server groups https://review.opendev.org/c/openstack/nova/+/899979 | |
| 18:01:46 | dansmith | well, if you do both vendor *and* product I guess it is | |
| 18:01:55 | sean-k-mooney | to me there is no diffence betwen mdev type and *vendor_id and product_id) | |
| 18:01:56 | dansmith | but not like worrying that 8086 is going to allow your root bridge to go through or something | |
| 18:02:16 | dansmith | but I really don't see the problem with that level of specificity | |
| 18:02:34 | stephenfin | bauzas: melwitt: https://review.opendev.org/c/openstack/nova/+/899979 came up as a result of an OpenShift question. I figure you'd both be interested in that addition to our docs | |
| 18:03:03 | sean-k-mooney | dansmith: i think its more an issue with how the config options are arranged more then anything esle | |
| 18:03:08 | stephenfin | I've also proposed helper alias for OSC to make requesting server groups slightly nicer https://review.opendev.org/c/openstack/python-openstackclient/+/899975 | |
| 18:03:12 | stephenfin | *a helper | |
| 18:03:36 | sean-k-mooney | dansmith: we split the mdevs into multipel dynmaic config sections to influcne this | |
| 18:03:45 | bauzas | sean-k-mooney: if we can ease your pain, we can document the fact that max_instances automatically passes *all* GPUs to nova | |
| 18:03:48 | sean-k-mooney | which to me make it harder to know if a device will be allowed or not | |
| 18:04:31 | bauzas | sean-k-mooney: and we could mention to use device_addresses as an alternative for people afraid of passing the whole GPUs | |
| 18:04:37 | sean-k-mooney | bauzas: just make -1 or None the default and use it as a sentanical to mean all. | |
| 18:04:55 | bauzas | lemme provide the patch first | |
| 18:05:08 | sean-k-mooney | bauzas: well i thing we shoudl be recommending device_address as the way peopel shoudl do it in general | |
| 18:05:24 | dansmith | sean-k-mooney: that doesn't work right? that gives us the wrong behavior we have now | |
| 18:05:56 | sean-k-mooney | dansmith: it doesn but i think bauzas was going to have it be wrong out of the box too | |
| 18:06:35 | sean-k-mooney | unless i missunderstood | |
| 18:06:38 | bauzas | tbc I was documenting in the config doc helper that max_instances should be set to the number of mediated devices a device can create | |
| 18:07:24 | sean-k-mooney | but where you going to recommend usign device_addresses as what peopel shoudl do by default? | |
| 18:07:53 | bauzas | we could | |
| 18:08:23 | sean-k-mooney | personally i woudl prefer reuqiring you do do "device_adddress=*" to have use any device with this mdev type behavior by the way | |
| 18:08:42 | sean-k-mooney | or saying you must set | |
| 18:08:51 | sean-k-mooney | either device_addres or max_instances | |
| 18:10:39 | bauzas | yeah those options are conflicting, for sre | |
| 18:10:58 | bauzas | I can mark those this way with oslo.config | |
| 18:11:19 | sean-k-mooney | but can you also make the mutally exclive group required | |
| 18:11:29 | sean-k-mooney | and the dynmic config section requried | |
| 18:12:01 | bauzas | we're on same page | |
| 18:12:32 | sean-k-mooney | well there is one thing im not sure you and i agree on | |
| 18:12:33 | bauzas | [mdev_nvidia-XXX] would become mandatory if we merge https://review.opendev.org/c/openstack/nova/+/899406 | |
| 18:12:40 | sean-k-mooney | ah | |
| 18:12:42 | sean-k-mooney | ya that | |
| 18:12:54 | bauzas | what's inside this section is mutually exclusive | |
| 18:13:04 | bauzas | either device_addresses or max_instances | |
| 18:13:07 | bauzas | wfy ? | |
| 18:13:17 | sean-k-mooney | so your ok with requiring that if an mdev type is in enabled_mdev_types | |
| 18:13:22 | sean-k-mooney | we must have the config section | |
| 18:13:32 | bauzas | that's what we agreed at the PTG | |
| 18:13:34 | sean-k-mooney | [mdev_<mdev name>] | |
| 18:13:48 | sean-k-mooney | ok then ya with that condition i think im good with your propsoal | |
| 18:13:57 | bauzas | in order to ease people's lifes, I'm just gonna add max_instances as a mutually exclusive option | |
| 18:14:29 | bauzas | if you're SR-IOV based, we'll pick N VFs for you | |
| 18:14:40 | bauzas | if you're not SRIOV, we'll just pick N devices for you | |
| 18:15:09 | melwitt | stephenfin: cool, thanks. I'll check it out | |
| 18:15:53 | sean-k-mooney | bauzas: we could just make max_instances apply to sriov case | |
| 18:16:04 | sean-k-mooney | since the non sriov case just works | |
| 18:16:35 | bauzas | with the non-sriov gpus, you need to specify all the cards | |
| 18:16:49 | sean-k-mooney | in the device list | |
| 18:16:52 | bauzas | giving them a way to say "I don't care" | |
| 18:16:58 | bauzas | seems good to me | |
| 18:17:07 | sean-k-mooney | ya so just add * to device_addreses | |
| 18:17:34 | sean-k-mooney | well im flexible on how we do that | |
| 18:18:09 | sean-k-mooney | im ok to have a away to say "expose all devices of this type" i just woudl like it to be a little more explict then it is today | |
| 18:18:09 | bauzas | we need implementation patches and then we could do anything we want | |
| 18:18:28 | bauzas | btw. I'm on PTO tomorrow | |
| 18:18:29 | sean-k-mooney | right os if we want to defer the details of how we do that for now | |