Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-07-22
15:25:08 stephenfin Alphazero_: You probably want #openstack or, at a stretch, #openstack-keystone
15:25:47 Alphazero_ whoops sorry ^^ just noticed the message at the top - will shift over...
15:28:45 artom Hrmm, maybe wait for sean-k-mooney to weight, seems like he has doubts as to the completeness of the fix
15:29:27 sean-k-mooney i think you are still relying on the pf being bound to a networking driver and having a netdev
15:29:39 sean-k-mooney if the PF is bound to vfio-pci
15:29:44 sean-k-mooney it will not have a net dev
15:30:04 sean-k-mooney in which case im not sure pci_utils.get_mac_by_pci_address will work
15:30:05 dansmith artom: sorry meant to say "ack" above, but...ack.
15:30:20 sean-k-mooney im looking at that code now
15:30:23 artom dansmith, no worries, I saw the +W go through, and am thankful (in silence)
15:30:33 dansmith artom: ack
15:31:45 sean-k-mooney artom: so ya https://github.com/openstack/nova/blob/master/nova/pci/utils.py#L162-L180 relise on the PF being bound to a networking driver not vfio-pci
15:32:35 sean-k-mooney so in that case we will still skip exposing the metadata for that PF
15:32:53 artom sean-k-mooney, so, I did test this (unlike the original patch :( ), what would cause the PF to use networking driver vs vfio-pci?
15:33:08 artom Because in the env I had it was using the former, going by what you're saying
15:33:13 sean-k-mooney udev
15:33:22 sean-k-mooney or the kernel configuration in general
15:33:43 sean-k-mooney try manually binding the PF driver to vfio-pci
15:33:52 artom Env is gone :(
15:34:03 sean-k-mooney it will still be usable by kvm but the metadata will be empty
15:34:35 sean-k-mooney you change wont break anythin it just will end up in the except block and retrun none
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

Earlier   Later