Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2022-07-15
11:42:27 gibi it is used by the instance so we cannot find it afaik
11:42:58 gibi or more precisely
11:43:02 gibi /sys/bus/pci/devices/0000:19:0a.7/net is not exists
11:43:22 gibi as the pci device is attached to the instance there is no netdev on the host
11:45:42 gibi after the vnic_type changed to macvtap the vif plug code wants to call set_vf_interface_vlan on the pci device
11:45:55 artom So wait, shouldn't that also happen if you just restart nova-compute with any macvtap device?
11:46:23 gibi hm
11:46:30 gibi that is a good question
11:46:33 gibi wait
11:46:38 gibi if it is consumed as a macvtap
11:46:44 gibi then the VF is not consumed
11:46:49 gibi and probably the netdev exists
11:47:02 artom Ah, right.
11:47:06 artom Sorry, creating confusion.
11:47:07 gibi the problem is that we consumed the VF and then looking up the VF's netdev
11:47:27 artom Bingo.
11:47:35 artom So the functional test should probably be asserting *that*
11:49:42 gibi so probably mocking get_ifname_by_pci_address is too much and I should inject the fault at set_vf_interface_vlan as that is the last specific call
11:49:55 gibi for the macvtap plug
11:49:56 artom Also, this is in the plug logic, right?
11:50:03 artom So should also happen during instance hard reboot?
11:50:18 artom We don't do any PCI accounting update during reboot...
11:50:22 gibi yes
11:50:30 gibi probably the plug fails at hard reboot too
11:50:45 gibi I can try as I have real sriov hw right now
11:52:25 gibi still the test will not be perfect as with mocking I can only simulate that the VF is consumed
11:52:40 gibi so if you move the port set before boot but keep the mock as is then it will still fail
11:52:57 gibi as it will still simulate that the VF is cosumed
11:53:04 gibi even if in reality it would not be
11:53:48 artom Right, there's no way around the mocking
11:54:00 artom But... I think I'd an asserting what what device we tried to look for
11:54:04 artom *assertion
11:54:14 artom "<gibi> the problem is that we consumed the VF and then looking up the VF's netdev"
11:54:15 artom :)
11:55:13 gibi even always try to look for the VF of the macvtap in both cases (a) when booting with a proper macvtap, b) when change the port to macvtap and then rebooting)
11:55:36 gibi s/even//
11:55:58 artom So the first case is legit, obviously
11:56:02 gibi yes
11:56:06 gibi second shoudl fail
11:56:17 gibi but I cannot distinguish on the mock level
11:56:25 artom The breakage in the second case from the fact that we didn't free up the device when the port type changed
11:56:58 gibi in the func env we never consume VF the pci stuff is mocked out globally
11:57:11 artom No? We can't do something like assert_called_with(<netdev_path>)?
11:57:40 gibi we can do both in both case the path would be the same
11:57:52 gibi *but
11:58:16 gibi the difference is that the boot consumed someting in b) but not in a)
11:58:22 gibi but we does not track consumption
11:58:27 gibi as that is on the host OS level
11:58:58 artom Like, I'm not saying "stop asserting the service start failure"
11:59:12 artom Oh, I think I get it
11:59:46 artom There's no assertion we can make that would be different between "start with legit macvtap device" and "start with vnic_type changed macvtap"
11:59:52 gibi yes
11:59:58 gibi as the path and the logic is the same
12:00:03 gibi the diff happens during boot
12:00:30 gibi this is like extrnal state that we don't modell in test
12:00:50 gibi the boot changes the external state (the host OS) and the reboot will depend on that state
12:00:57 gibi but we don't carry that state in the test env
12:01:16 gibi we could, but we don't today
12:01:35 gibi we could create a proper stub for the pci module and track pci devices
12:03:11 artom That smells like a lot of work... :)
12:03:38 artom But... you understand why I find the test weird, right? Like, we go through all these change vnic_type steps, but they're all moot because of the mock
12:04:00 gibi yes, that would be a piece of work :)
12:04:15 gibi and yes, I got you, I try to figure out something better...
12:05:00 artom Sorry for not bringing any better solution :P
12:05:11 gibi basically the mock and the vnic_type change need to be coupled somehow...
12:05:43 gibi artom: no worries, you have a valid point, and it was a good excersise to talk it through
12:20:20 gibi artom: interestingly the hard reboot did not fail
12:20:57 gibi it created a proper macvtap interface and passed to the instance
12:21:28 gibi it is probably works because hard reboot unplugs the vif -> the VF will be freed the plug can lookup the netdev
12:22:10 gibi and the accounting in nova side is actually correct as the macvtap dev needs to keep the parent VF allocated to the instance (and that does not change during the reboot)
12:22:21 gibi so probably a direct -> direct-phyical change would not work
12:29:31 artom Ah, right, because it's the host OS accounting
12:29:36 artom So as long as we unplug first, we're fine
12:31:14 gibi in direct -> macvtap yes we are fine both on the OS level an in the nova PCI tracking
12:31:36 gibi in case of direct -> direct-phyical the PCI tracking in nova will be inconsistent
12:31:59 gibi as the instance will have a VF allocated but consuming a PF instead (probably)
12:58:33 gibi artom: I think I see a way to make the test more logical. We can check in the get_ifname_by_pci_address mock if the device being looked up is actaully consumed as a VF by the instance
13:00:58 opendevreview Balazs Gibizer proposed openstack/nova master: Reproduce bug 1981813 in func env https://review.opendev.org/c/openstack/nova/+/849985
13:01:02 gibi artom: ^^
13:17:30 artom Oooo, nice
13:18:50 gibi now if you move the port update before the boot then the mock will not raise the exception and the test will fail
13:19:23 gibi we have the external state but it is stored the domain in the fake libvirt connectioin
13:19:34 gibi * stored in the domain
13:23:44 opendevreview Balazs Gibizer proposed openstack/nova master: Gracefully ERROR in _init_instance vnic_type changed https://review.opendev.org/c/openstack/nova/+/850003
13:23:58 gibi and here is the "fix" (two error logs and a skip)
13:24:26 gibi I have to leave early today so I will add unit test and reno to the patch on monday
13:29:07 artom Sure
13:43:16 sean-k-mooney gibi: ack ill review that shortly i just got back form doctors appoinent they were runing 90 mins behind so took longer then planned
13:44:54 gibi thanks
13:45:04 gibi no rush, I will probably look at it only on Monday
13:45:44 sean-k-mooney ack
13:56:31 sean-k-mooney gibi: i have set the public security flag on https://bugs.launchpad.net/nova/+bug/1981813 by the way just to highlight it to the security team
13:57:15 sean-k-mooney it was a public bug downstream so that ship had already sailed when we triaged it
14:12:21 opendevreview Stephen Finucane proposed openstack/nova master: Remove unused requirement https://review.opendev.org/c/openstack/nova/+/850006
14:12:21 opendevreview Stephen Finucane proposed openstack/nova master: Fix compatibility with jsonschema 4.x https://review.opendev.org/c/openstack/nova/+/849867
14:12:37 stephenfin sean-k-mooney: Sorry, I missed your ping earlier. Respun with an arbitrary version now ^
14:14:30 sean-k-mooney cool
14:14:39 sean-k-mooney will look soon
15:23:45 opendevreview Stephen Finucane proposed openstack/nova master: Bump jsonschema minimum to 4.0.0 https://review.opendev.org/c/openstack/nova/+/850021
15:55:41 sean-k-mooney artom: you are aware that what you sugesed in https://review.opendev.org/c/openstack/nova/+/849104 is not allowed or what you are ment to do right

Earlier   Later