| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2023-11-02 | |||
| 14:00:31 | sean-k-mooney | and we would just not use aviable_isntance at all for the placment total | |
| 14:00:42 | sean-k-mooney | bauzas: it really should | |
| 14:00:49 | dansmith | suuuurely it does | |
| 14:00:55 | sean-k-mooney | that might be missing in libvirt but it will be in sysfs | |
| 14:00:55 | bauzas | sean-k-mooney: if you don't use available_instances, then you're asking people to precreate the mdevs | |
| 14:01:04 | sean-k-mooney | bauzas: no im not | |
| 14:01:11 | sean-k-mooney | im asking them to read the nvidia docs | |
| 14:01:14 | dansmith | I have to do another meeting for a bit | |
| 14:01:19 | sean-k-mooney | and tell us how many it can allcoate | |
| 14:01:44 | dansmith | surely we could also have a generic listing for the common types | |
| 14:01:53 | bauzas | (15:00:31) sean-k-mooney: and we would just not use aviable_isntance at all for the placment total | |
| 14:01:53 | bauzas | sean-k-mooney: I don't get then what you said | |
| 14:02:00 | bauzas | dansmith: that's like boiling the ocean | |
| 14:02:13 | bauzas | types are hardware-dependent | |
| 14:02:27 | sean-k-mooney | bauzas: im saying for the placment inventory we read total form the config option not form aviable_instnace | |
| 14:02:31 | dansmith | nvidia-472 differs in size on different cards? | |
| 14:02:37 | bauzas | https://docs.nvidia.com/grid/latest/grid-vgpu-user-guide/index.html#supported-gpus-grid-vgpu | |
| 14:02:49 | sean-k-mooney | dansmith: nvidia are creating a mdev type per card | |
| 14:02:59 | bauzas | dansmith: count every line in each tab, you'll get the number of mdev types | |
| 14:03:36 | dansmith | ack I understand | |
| 14:03:49 | bauzas | dansmith: no, the other way, nvidia-472 only stands for A100-20C which is only a type supported by A100 cards | |
| 14:03:59 | dansmith | yup, I got it | |
| 14:04:26 | sean-k-mooney | bauzas: lets park this for a bit and create an etherpad or something to collaberate on | |
| 14:04:51 | bauzas | formerly, before the magic world of SRIOV GPUs, available_instances was giving us the number of VGPUs a PCI device was able to create | |
| 14:04:54 | sean-k-mooney | its simpler to discuss when we have some of the constriat written down liek a spec | |
| 14:05:20 | bauzas | sean-k-mooney: there is already the bug report | |
| 14:05:28 | sean-k-mooney | i would like somethign we can edit | |
| 14:05:39 | bauzas | I'm not a big fan of writing a spec for fixing this | |
| 14:05:50 | sean-k-mooney | bauzas: i think you have too | |
| 14:05:55 | sean-k-mooney | depending on the fix | |
| 14:06:05 | sean-k-mooney | there are ways to do the minimal fix as a bug | |
| 14:06:20 | dansmith | I think it's clear we need to document this more than just some sysfs dumps in a bug :) | |
| 14:06:21 | bauzas | sean-k-mooney: have you seen my patch already ? | |
| 14:06:24 | sean-k-mooney | but if we want to fix this properly we need to change how we are doing mdev managment | |
| 14:06:42 | bauzas | as a reminder, mdev inventory isn't a problem with non-SRIOV GPUs | |
| 14:06:52 | bauzas | so whatever we decide will necessarly be opt-in | |
| 14:06:56 | sean-k-mooney | its not a problem for all sriov devices | |
| 14:07:06 | bauzas | and keeping the legacy behaviour too | |
| 14:07:08 | sean-k-mooney | its litrally aproblem just for how nvidia did it | |
| 14:07:34 | bauzas | "its litrally aproblem just for how nvidia did it" I should quote it, print it and put it on my front wall | |
| 14:08:17 | bauzas | the way that available_instances isn't a static number is also a terrible driver implementation, if you want MHO | |
| 14:08:18 | sean-k-mooney | before i left intel, i used a protoytp intel QAT impelemtnation wtch had sriov VF for each enging type and they supprot mdevs per VF with indepenent aviableity | |
| 14:08:31 | dansmith | is it really a problem though? I mean it kinda seems like this is the point of mdevs, no? to be allocatable in more dynamic ways than just static slices of a card? | |
| 14:09:09 | bauzas | true | |
| 14:09:10 | sean-k-mooney | dansmith: mevs from diffent pci endpoitn are intented to not affect each other | |
| 14:09:34 | sean-k-mooney | that is what nvida started breaking | |
| 14:09:50 | sean-k-mooney | the framework allows them to be dynmaic | |
| 14:09:58 | dansmith | there are multiple pci devices per card? or you mean the vfs? | |
| 14:10:14 | bauzas | oh wait | |
| 14:10:16 | bauzas | [stack@lenovo-sr655-01 ~]$ cat /sys/class/mdev_bus/0000\:41\:01.0/mdev_supported_types/nvidia-472/description | |
| 14:10:19 | bauzas | num_heads=1, frl_config=60, framebuffer=20480M, max_resolution=4096x2400, max_instance=2 | |
| 14:10:34 | bauzas | we can get the number of VGPUs in the description | |
| 14:10:38 | sean-k-mooney | bauzas: the desciptoin is not a standard format | |
| 14:10:43 | bauzas | this is horribly driver-specific | |
| 14:11:03 | sean-k-mooney | yes it is its litrally just a text filed | |
| 14:11:03 | bauzas | yeah | |
| 14:11:32 | dansmith | but it's something | |
| 14:11:43 | sean-k-mooney | its not somehting nova should read | |
| 14:12:00 | dansmith | it's something an operator could read in order to put the math in the config :) | |
| 14:12:24 | sean-k-mooney | grep . /sys/class/mdev_bus/0000\:41\:01.0/mdev_supported_types/nvidia-472/* | |
| 14:12:27 | bauzas | operators aldray read : | |
| 14:12:28 | bauzas | [stack@lenovo-sr655-01 ~]$ cat /sys/class/mdev_bus/0000\:41\:01.0/mdev_supported_types/nvidia-472/name | |
| 14:12:31 | bauzas | GRID A100-20C | |
| 14:12:32 | sean-k-mooney | bauzas: can you run ^ | |
| 14:12:41 | dansmith | I also wouldn't say nova *shouldn't* read it.. I think we could read and parse it as long as we don't fail if it has changed | |
| 14:12:42 | bauzas | and then Ctrl-F that string into https://docs.nvidia.com/grid/latest/grid-vgpu-user-guide/index.html#supported-gpus-grid-vgpu | |
| 14:13:22 | sean-k-mooney | dansmith: yes the operator can read it | |
| 14:13:39 | sean-k-mooney | dansmith: so would you be ok with adding max_instances to the mdev_type config section | |
| 14:14:01 | sean-k-mooney | or is that still too static | |
| 14:14:02 | bauzas | sean-k-mooney: if we go this way, this wouldn't be backportable, right? | |
| 14:14:10 | dansmith | well, I'm not sure that's a solution entirely, but I think making the operator tell us some of the math so we can do it better on the backend makes sense | |
| 14:14:30 | bauzas | I really tried to propose https://review.opendev.org/c/openstack/nova/+/899625 as a backportable mechanism | |
| 14:14:31 | dansmith | I'd need to hear the whole story before "being okay" with it, but that seems like a good direction to me | |
| 14:15:03 | bauzas | dansmith: finish your call and then we could do a gmeet | |
| 14:15:09 | dansmith | sure | |
| 14:15:11 | sean-k-mooney | bauzas: i dont really like that because it implies your still doing the math incorrect | |
| 14:15:14 | bauzas | I have the PTG summary to write | |
| 14:15:36 | dansmith | I don't love the "quick, delete the provider" approach either, FWIW | |
| 14:15:45 | bauzas | sean-k-mooney: it implies people were lazy enough to not explicitely tell which PCI devices to allocate | |
| 14:16:21 | bauzas | dansmith: that's understandable, but then you're pro-statically defining a list of VFs or an integer counting allocatable VFs | |
| 14:16:26 | sean-k-mooney | bauzas: and we are not internally trackign which device the mdev is allcoated form meaning we are not mapping the allcoations correctly in placment | |
| 14:16:30 | bauzas | (just clarifying the options) | |
| 14:17:17 | sean-k-mooney | removing the rp works if and only if we gurentte that the mdevs are created by packing the RPs | |
| 14:17:20 | bauzas | sean-k-mooney: if you want to have my frank opinion, I'm not a big fan of changing all the allocations we make | |
| 14:17:47 | dansmith | bauzas: I'm for doing better math, and it sounds like some hints for that math (in config) are necessary | |
| 14:17:48 | bauzas | sean-k-mooney: I very humbly think you're wrong on the fact we miscount | |
| 14:18:10 | bauzas | we very correcly count which mdevs against RPs | |
| 14:18:18 | dansmith | still would need to understand the whole picture *and* how we migrate to it, impact to existing people, and whether or not it's worth it if mdevs are going away | |
| 14:18:52 | bauzas | the latter conditional is hard to guess | |
| 14:19:03 | bauzas | but that's fair assumptions | |
| 14:19:37 | bauzas | and I'm not particularly attached to the bugfix itself | |
| 14:20:00 | bauzas | but again, I have my wills : a simple approach not redesigning our placement tracking which would be backportable | |
| 14:20:06 | sean-k-mooney | bauzas: in sriov mode is aviable_instances alway 1 | |
| 14:20:26 | sean-k-mooney | becasue for intels implemation (which they never released) it was not | |
| 14:20:30 | bauzas | see the bug report, yah until all mdevs are created | |
| 14:20:48 | bauzas | I literally wrote that in the bug description | |
| 14:21:10 | sean-k-mooney | what i dont like about that is that is a vendor specific detail | |
| 14:21:32 | bauzas | hell yeah, hence why I wanted to make the fix agnostic | |
| 14:22:09 | bauzas | we're just doing some math in some method that say 'oh look, that inventory is now 0, but meh, I don't care updating it' | |
| 14:22:38 | sean-k-mooney | well your solution does not work if the VF have aviable_instance>1 initally | |