| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2021-03-10 | |||
| 19:31:18 | openstackgerrit | Stephen Finucane proposed openstack/nova master: libvirt: Move PCI host device parsing to 'host' https://review.opendev.org/c/openstack/nova/+/779851 | |
| 19:31:18 | openstackgerrit | Stephen Finucane proposed openstack/nova master: objects: Add 'VDPA' to 'PciDeviceType' https://review.opendev.org/c/openstack/nova/+/777481 | |
| 19:31:19 | openstackgerrit | Stephen Finucane proposed openstack/nova master: libvirt: Add vDPA nodedev parsing https://review.opendev.org/c/openstack/nova/+/770533 | |
| 19:31:20 | openstackgerrit | Stephen Finucane proposed openstack/nova master: libvirt: Add guest generation for vDPA https://review.opendev.org/c/openstack/nova/+/770532 | |
| 19:31:21 | openstackgerrit | Stephen Finucane proposed openstack/nova master: add hw:mlock extra spec https://review.opendev.org/c/openstack/nova/+/778347 | |
| 19:31:21 | openstackgerrit | Stephen Finucane proposed openstack/nova master: pci: Add vDPA vnic to PCI request mapping and filtering https://review.opendev.org/c/openstack/nova/+/778350 | |
| 20:12:54 | openstackgerrit | Merged openstack/nova master: libvirt: Stop passing around virt_type, caps https://review.opendev.org/c/openstack/nova/+/775689 | |
| 20:13:31 | openstackgerrit | Merged openstack/nova master: libvirt: Add missing type hints https://review.opendev.org/c/openstack/nova/+/775688 | |
| 20:23:14 | openstackgerrit | sean mooney proposed openstack/nova master: Support per port numa policies with SR-IOV https://review.opendev.org/c/openstack/nova/+/773792 | |
| 20:24:45 | sean-k-mooney | artom: ^ im having troble getting the socket test i addded here working https://review.opendev.org/c/openstack/nova/+/773792/10/nova/tests/functional/libvirt/test_pci_sriov_servers.py#1254 | |
| 20:31:28 | artom | sean-k-mooney, I wonder if it's because I set the 'socket' attribute on the pools in my filter method | |
| 20:31:45 | artom | And the pools that get passed to remove_device() don't have it | |
| 20:31:54 | artom | So it fails the equality test | |
| 20:32:23 | sean-k-mooney | its there in the host state object {"physnets": [], "tunneled": false}, "nova_object.changes": ["physnets", "tunneled"]}, "socket": 1}, | |
| 20:32:58 | sean-k-mooney | actully ill copy this to pastbin that not waht i wanted | |
| 20:34:04 | sean-k-mooney | http://paste.openstack.org/show/803443/ | |
| 20:34:09 | artom | del pool_keys['socket'] | |
| 20:34:09 | artom | if 'socket' in pool_keys: | |
| 20:34:09 | artom | del pool_keys['devices'] | |
| 20:34:09 | artom | Hrmm, yeah, if I add: | |
| 20:34:22 | artom | In _find_pool() | |
| 20:34:25 | artom | It makes the tests pass | |
| 20:34:28 | artom | OK, I have to run | |
| 20:34:37 | artom | Hopefully that helped a little bit, talk in ~1 hour | |
| 20:35:19 | sean-k-mooney | ok ya that might be the issue | |
| 20:36:17 | sean-k-mooney | but if it is your current patch might break neutron sriov ports? ill do some testing locally and try and see if its just a functest issue or a real one | |
| 20:37:34 | sean-k-mooney | the other polices do work however so i dont know it might just be an edgecase i need to fix to make both work correctly | |
| 20:38:07 | artom | I suspect it's a logic error in my patches | |
| 20:38:17 | artom | Though I do wonder how come my own functional tests passed...? | |
| 21:15:44 | sean-k-mooney | artom: i get the error on real hardware too. | |
| 21:31:08 | artom | sean-k-mooney, I wonder if if has something to do with VFs and PFs? | |
| 21:34:01 | sean-k-mooney | im not sure | |
| 21:34:12 | sean-k-mooney | did you test your code on real hardware | |
| 21:35:47 | sean-k-mooney | hum if i do some hacking in /sys to put all my nics on numa 0 i can boot again | |
| 21:36:20 | sean-k-mooney | basiclly echo 0 | sudo tee /sys/class/net/eth*/device/numa_node | |
| 21:36:34 | sean-k-mooney | then stop nova compute and libvirt fully and start them again | |
| 21:36:39 | artom | I didn't | |
| 21:37:06 | sean-k-mooney | when that was -1 it was failing i hadd to add pool_keys.pop('socket', None) | |
| 21:37:16 | sean-k-mooney | to find_pool | |
| 21:37:27 | sean-k-mooney | ill try removing that again | |
| 21:39:23 | sean-k-mooney | i can test it with pci alis too i guess i just need to spend some time to do it | |
| 21:40:51 | artom | That's the thing, I wasn't touching /sys, or indeed the PCI devices | |
| 21:41:13 | artom | Besides calculating the socket and saving it in pool['socket'] | |
| 21:41:29 | sean-k-mooney | ya well this is what is printign for the final resouce view | |
| 21:41:31 | sean-k-mooney | PciDevicePool(count=1,numa_node=0,product_id='10c9',tags={dev_type='type-PF',physical_network='public'},vendor_id='8086'), PciDevicePool(count=7,numa_node=0,product_id='10ca',tags={dev_type='type-VF',parent_ifname='eth1',physical_network='public'},vendor_id='8086')] | |
| 21:41:50 | sean-k-mooney | so the socket is not being added there incorerctly | |
| 21:43:02 | sean-k-mooney | a pic dev look more or less normal too | |
| 21:43:24 | sean-k-mooney | {"dev_id": "pci_0000_01_11_3", "address": "0000:01:11.3", "product_id": "10ca", "vendor_id": "8086", "numa_node": 0, "label": "label_8086_10ca", "dev_type": "type-VF", "parent_addr": "0000:01:00.1", "parent_ifname": "eth1", "capabilities": {"network": ["rx", "tx", "sg", "tso", "gso", "gro", "rxvlan", "txvlan", "txudptnl"]}} | |
| 21:45:02 | sean-k-mooney | strange it seam to be workign without the pop(socket ,none) | |
| 21:45:04 | artom | sean-k-mooney, hrmm, I really think it's the parent PF issue | |
| 21:45:10 | artom | It doesn't have the socket | |
| 21:45:40 | sean-k-mooney | ya maybe althou it shoudl | |
| 21:45:55 | sean-k-mooney | well in the same way the VF will | |
| 21:45:59 | artom | The logging I added to remove_device() and _find_pool() logs what I expect with your tests | |
| 21:46:05 | artom | But not with my functional tests | |
| 21:46:22 | artom | Meaning - it's never called, because in my func tests it only tests PFs (or "standard" PCI) | |
| 21:46:26 | artom | (Not sure which) | |
| 21:46:38 | artom | By testing with VFs and their parent PFs, you've uncovered an issue | |
| 21:46:52 | sean-k-mooney | standard is not a PF | |
| 21:47:03 | artom | But it has no parent, is my point | |
| 21:47:03 | sean-k-mooney | standard is a device that does not supprot sriov | |
| 21:47:09 | sean-k-mooney | a PF is type-PF | |
| 21:47:16 | sean-k-mooney | standard is type-PCI | |
| 21:47:36 | artom | The point is not the type, it's the parent device (or lack thereof) | |
| 21:48:26 | sean-k-mooney | correct you do not have to mark the parent as unavaible | |
| 21:48:30 | sean-k-mooney | as we do with VFs | |
| 21:48:49 | sean-k-mooney | or the child as unaviable with PFs | |
| 21:49:36 | sean-k-mooney | so you are not calling _handle_device_dependents | |
| 21:49:39 | sean-k-mooney | in your tests | |
| 21:49:48 | artom | Nope :) | |
| 21:49:56 | sean-k-mooney | well you might be but its a noop for you | |
| 21:49:56 | artom | Not that it as intentional | |
| 21:50:00 | artom | Or that I was aware it existed | |
| 21:50:26 | sean-k-mooney | you are calling it | |
| 21:50:51 | sean-k-mooney | but i does nothing for STANDARD/type-PCI | |
| 21:51:56 | artom | del pool['socket'] | |
| 21:51:56 | artom | if 'socket' in pool: | |
| 21:51:56 | artom | Yeah, if I do for pool in pools: | |
| 21:52:01 | artom | At the end of my new filter | |
| 21:52:05 | artom | It fixes your func tests | |
| 21:52:39 | artom | I guess you've made your point about being purely functional and not having side effects ;) | |
| 21:53:49 | sean-k-mooney | making it it a pure fucntion may or may not have caught it depending on if we wrote the test right | |
| 21:54:43 | artom | I think the "best" solution would be to not set pool['socket'] altogether, and come at this from another angle: figure out which NUMA nodes are allowed, and filter on those | |
| 21:54:53 | artom | No side effects | |
| 21:55:09 | sean-k-mooney | well you should not be setting pool sockets | |
| 21:55:25 | sean-k-mooney | and you dont need too if you did what was in my last comment | |
| 21:56:01 | sean-k-mooney | https://review.opendev.org/c/openstack/nova/+/772779/17#message-744974dfe27df3782bd2e6c066aa1b241fc7136a | |
| 21:57:01 | sean-k-mooney | the body of your filter could be basically this right | |
| 21:57:03 | sean-k-mooney | vm_sockets = [mappings[cell.id] for cell in numa_cells] | |
| 21:57:05 | sean-k-mooney | pools_to_remove = [] | |
| 21:57:07 | sean-k-mooney | for pool in pools: | |
| 21:57:09 | sean-k-mooney | pool_socket = mappings.get(pool['numa_node']) | |
| 21:57:11 | sean-k-mooney | if not (pci_socket and pool_socket in vm_sockets): | |
| 21:57:13 | sean-k-mooney | pools_to_remove += pool | |
| 21:57:15 | sean-k-mooney | return [pool for pool in pools if pool not in pools_to_remove] | |
| 21:57:49 | artom | There are some unbound variables in there, so I can't quite grok it | |
| 21:57:56 | artom | But I can come up with something equivalent | |
| 21:58:16 | sean-k-mooney | where mappings is basically {numa_node:[socket_id,socket_id] | |
| 21:58:32 | sean-k-mooney | artom: mappings is the only one right | |
| 21:58:42 | artom | pci_socket as well | |