| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-07-22 | |||
| 15:34:49 | artom | So, strictly speaking, better than what we have now? ;) | |
| 15:34:50 | sean-k-mooney | so your fix is valid if the PF is boud to say i40e | |
| 15:34:56 | sean-k-mooney | yep | |
| 15:35:11 | sean-k-mooney | as i said i think its incomplte not wrong. | |
| 15:35:18 | artom | Some of the PFs our there will start working, the others will remain device-tag-less | |
| 15:35:19 | sean-k-mooney | so maybe partial-bug? | |
| 15:35:32 | artom | That'd wfm | |
| 15:35:46 | sean-k-mooney | instead of close-bug and i'd be +1 on it | |
| 15:35:58 | artom | Ack, leave a review and I'll update the commit message | |
| 15:36:24 | sean-k-mooney | sure thing. ill do that in a few minutes. just grabbing something to drink brb | |
| 15:36:41 | artom | And if I ever get an SRIOV env again, I'll come up with a patch to address vfio-pci-bound PFs, presumable with some new method to find the MAC address from the PCI address. | |
| 15:41:20 | artom | mriedem, thanks for looking, replied | |
| 15:41:41 | artom | And welcome back, btw. No way 1 week was enough, but we're still happy to see you :) | |
| 15:44:55 | jangutter | artom: I think if the PF is bound to vfio-pci then the kernel knows almost nothing about the device. Only way to get info is via some kind of backdoor (like another PCI device that happens to "manage" the PF). | |
| 15:45:47 | artom | jangutter, so in those cases we should just give up on getting its MAC address? | |
| 15:46:01 | jangutter | artom: the really ugly way is to rebind the PCI device to a kernel driver, let it probe, check what the MAC is, rebind it to vfio-pci. | |
| 15:46:21 | artom | jangutter, I feel like that's not Nova's job... | |
| 15:46:25 | jangutter | artom: I concur. | |
| 15:47:28 | artom | Would the device's PCI address be the same in the guest as on the host? If so it'd be enough to just expose tag + PCI address in the metadata | |
| 15:47:36 | artom | If not, exposing just a device tag makes no sense | |
| 15:47:48 | sean-k-mooney | jangutter: well for neutron sriov passthouhg of PF we are ment to be discovering the PF mac and setting the neutron port to it | |
| 15:48:02 | sean-k-mooney | jangutter: im not sure exactly how we do that however | |
| 15:48:15 | sean-k-mooney | im hoping it more robost then we do for tagging | |
| 15:49:00 | sean-k-mooney | jangutter: but yes when its boud to vfio-pci the kernel only know what is reported in the pci config space | |
| 15:49:06 | sean-k-mooney | so what is reported by lspci | |
| 15:49:16 | sean-k-mooney | but i don think the mac is part of that | |
| 15:49:39 | jangutter | yeah, the problem is that vfio-pci is _not_ a driver. It's technically just "hey, this is a raw pci device and I'm not going to interpret anything about it." | |
| 15:50:34 | jangutter | there's code in libvirt that does the whole "driver rebinding" thing for the MLX-3 series for VF's, because it needs info from the driver to figure out which port the VF is connected to. | |
| 15:50:49 | cdent | efried: am I right that the shared disk spec is effectively dead: https://review.opendev.org/650188 (trying to trim my attention) | |
| 15:51:01 | jangutter | It would not be surprising to me if that's also done for PF's. | |
| 15:51:34 | artom | My stomach is angry, so I'm go going to get phood, but I'll read the scrollback when I get back | |
| 15:52:33 | jangutter | could this be exposed via devlink? My spider-sense says 'unlikely'. | |
| 16:09:07 | openstackgerrit | Andreas Jaeger proposed openstack/nova master: Update api-ref location https://review.opendev.org/672077 | |
| 16:20:27 | sean-k-mooney | artom: commented. i gave it a tentitive +1 rather then -1 given it looks like the vfio-pci case was never supported for metadata generation | |
| 16:21:57 | artom | sean-k-mooney, I'm all for respinning with Partial-bug | |
| 16:22:35 | artom | I'll also improve the LOG message | |
| 16:23:33 | artom | sean-k-mooney, what does the passed through PF look like from the guest? I'm assuming its guest PCI address won't be the same as the host | |
| 16:24:44 | artom | My train of thought is - if we can't find its MAC, and we don't know what its guest PCI address it, there's no point in exposing just a tag, because the guest will have no way of associating that tag with a device | |
| 16:25:09 | sean-k-mooney | it wont but the guest pci address is stored in the target element of the host dev element and the host pci address is in the source element | |
| 16:25:47 | artom | Ah, so we ignore the MAC entirely, and just expose that along with the tag | |
| 16:25:53 | sean-k-mooney | yep | |
| 16:26:01 | artom | I could do that in this patch as well, however I have no way if testing that | |
| 16:26:05 | artom | *of | |
| 16:26:44 | sean-k-mooney | i could proably test it on my server that i use for sriov dev but im not in shannon at the moment | |
| 16:28:02 | openstackgerrit | Andreas Jaeger proposed openstack/nova master: Update api-ref location https://review.opendev.org/672077 | |
| 16:28:53 | artom | It'd be a beefier change though | |
| 16:29:04 | artom | Currently LibvirtConfigGuestHostdevPCI doens't even understand <target> | |
| 16:29:34 | artom | Which actually means we're putting the wrong PCI address into the metadata, as we're using the <source> | |
| 16:29:37 | artom | *facepalm* | |
| 16:31:50 | artom | ... but we have no concept of PCI address in the VIF data structure that we use to store the tags | |
| 16:53:49 | openstackgerrit | Stephen Finucane proposed openstack/nova master: trivial: Rename exception argument https://review.opendev.org/671795 | |
| 16:53:50 | openstackgerrit | Stephen Finucane proposed openstack/nova master: trivial: Remove unused function parameter https://review.opendev.org/671796 | |
| 16:53:51 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Remove 'hardware.get_host_numa_usage_from_instance' https://review.opendev.org/671797 | |
| 16:53:51 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Remove 'hardware.host_topology_and_format_from_host' https://review.opendev.org/671798 | |
| 16:53:51 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Remove 'hardware.instance_topology_from_instance' https://review.opendev.org/671799 | |
| 16:53:52 | openstackgerrit | Stephen Finucane proposed openstack/nova master: WIP: hardware: Differentiate between shared and dedicated CPUs https://review.opendev.org/671800 | |
| 16:53:53 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Add support translating CPU policy extra specs, image meta https://review.opendev.org/671801 | |
| 16:53:53 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Remove deprecated CPU, RAM, disk claiming in resource tracker https://review.opendev.org/551026 | |
| 16:53:54 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Remove 'nova.virt.driver.ComputeDriver.estimate_instance_overhead' https://review.opendev.org/672106 | |
| 16:55:19 | stephenfin | dansmith: ^ I think the reno I've added to https://review.opendev.org/551026 should cover how to handle overhead sufficiently, when combined with https://review.opendev.org/671801 | |
| 16:55:31 | stephenfin | (Just FYI. The series is still WIP) | |
| 16:58:53 | mriedem | is it just me or are the zuul comments now always showing up regardless of the toggle CI button? | |
| 16:58:55 | dansmith | stephenfin: commented on something you said in there | |
| 17:01:59 | artom | sean-k-mooney, still around? Can I pick your brain for a thing? | |
| 17:02:09 | artom | (Related to that device tagging patch) | |
| 17:02:28 | stephenfin | mriedem: The zuul comments, yes. Third party CI are hidden | |
| 17:02:47 | stephenfin | I suspect that was intentional, since the button now reads "Toggle Extra CI" | |
| 17:07:28 | stephenfin | dansmith: Think you might be right, in which case this is much ado about nothing. I'll respond once I've made sure | |
| 17:07:36 | efried | artom: looks like mriedem is engaged with https://review.opendev.org/#/c/670593/ ? | |
| 17:07:53 | artom | efried, yep, thanks for looking | |
| 17:08:04 | dansmith | stephenfin: I didn't realize you were worried about something acutely happening now when we discussed earlier, I thought you were just generally wondering about the impact of moving away from those filters | |
| 17:08:09 | artom | and I think we're going back to drawing board for that one anyways | |
| 17:08:31 | efried | cdent: re shared disk spec, if it doesn't get some comeback from the author (or get picked up by someone else) then I guess it's dead, yeah. | |
| 17:08:36 | artom | The review process (sean-k-mooney mostly) unearthed other problems in that code | |
| 17:08:42 | stephenfin | No, it was pretty much the 'hw:emulator_threads_policy=isolate' case that I was worried about breaking | |
| 17:15:34 | sean-k-mooney | artom: my mother just arrived home but yes im still around | |
| 17:16:09 | openstackgerrit | Merged openstack/nova master: libvirt: move checking CONF.my_ip to init_host() https://review.opendev.org/671471 | |
| 17:16:28 | artom | sean-k-mooney, I thought I had a way of solving the tag <-> XML device problem for VFIO-PCI, but when writing it out in the review, turns out I didn't | |
| 17:16:45 | artom | I was thinking of using instance.pci_devices, but they don't have any MAC either | |
| 17:17:07 | artom | Unless we do a DB migration and start shoving them in there | |
| 17:17:35 | sean-k-mooney | you can use the vif['binding_profile']['pci_slot'] | |
| 17:17:48 | openstackgerrit | Andreas Jaeger proposed openstack/nova master: Update api-ref location https://review.opendev.org/672077 | |
| 17:18:11 | artom | sean-k-mooney, that won't solve the problem | |
| 17:18:12 | sean-k-mooney | that will be the host pci address for that port | |
| 17:21:47 | sean-k-mooney | artom: we can construct the NetworkInterfaceMetadata without setting the mac filed | |
| 17:21:55 | sean-k-mooney | or rather setting it condionally | |
| 17:22:19 | artom | sean-k-mooney, I know, but we need to correlate the tag in nova/objects/network_request.py to the correct device in the instance XML | |
| 17:22:36 | artom | Currently that's done by MAC address, because it's the one piece of info that's common (sometimes) to both | |
| 17:23:01 | sean-k-mooney | artom: but its not the "one" pices of info that is common | |
| 17:23:18 | sean-k-mooney | the pci address should be common too i belive | |
| 17:23:40 | artom | sean-k-mooney, right, but all of the existing code assumes the MAC will be used | |
| 17:24:11 | artom | So we write the tag in a VirtualInterface object, which has the MAC and nothing else. | |
| 17:24:46 | sean-k-mooney | well that is entirly broken | |
| 17:25:05 | artom | Now you tell me :P | |
| 17:25:06 | sean-k-mooney | you can have two ports with the same mac on different networks and attach them both to the same instance | |
| 17:25:36 | artom | Where were you 3 years ago or whenever Newton happened | |
| 17:25:38 | artom | ;) | |
| 17:26:14 | artom | See, I though Neutron placed a MAC uniqueness constraint per instance as well, not just per-Network | |
| 17:26:32 | sean-k-mooney | i proably pointed that out 3 years ago if i reviewd it :) | |
| 17:26:33 | sean-k-mooney | no | |