Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-07-22
17:33:52 sean-k-mooney also is this really the unique constarit https://github.com/openstack/nova/blob/master/nova/db/sqlalchemy/models.py#L860-L861
17:35:00 artom sean-k-mooney, IIRC there was something else higher up
17:35:05 artom (About the constraint)
17:35:15 sean-k-mooney well that constiarint is wrong
17:35:30 sean-k-mooney it would only allow 1 instnace of the mac in a cell
17:35:45 sean-k-mooney which is not required by neutorn
17:36:34 artom Makes device tagging mostly work though ;)
17:37:31 sean-k-mooney im not sure we want to fix all the issue here in one patch
17:37:50 sean-k-mooney we could but im not sure you want to have to write db migrations to fix this
17:39:34 sean-k-mooney the uniqui constratint should be (address,deleted,uuid) if uuid is the network uuid
17:40:02 artom sean-k-mooney, btw, just double checked, and the virtual_interface uuid does appear to be the interface UUID, not the port
17:40:38 sean-k-mooney the instance uuid should be in the instance_uuid field
17:40:48 sean-k-mooney not the uuid field
17:41:37 artom I mean the virtual_interface has its own UUID, much like an instance
17:41:45 artom Dunno why, but that's what it is
17:41:54 sean-k-mooney a ok
17:42:07 artom I checked on my devstack, the UUIDs don't match between ports and virtal_interfaces
17:42:21 sean-k-mooney in that case the constratin would be (address,deleted,network_id)
17:42:29 sean-k-mooney ok
17:42:53 artom I don't disagree, but it has the side effect of allowing device tagging to work, and it's kinda too late to impose it now
17:43:25 sean-k-mooney it a fairly sever regression
17:43:34 sean-k-mooney that was intoduec a long time agp
17:44:03 sean-k-mooney can you try and boot two vms with ports that have the same mac on different network and see what happens?
17:44:46 sean-k-mooney i need to run to the shop to grab food for dinner
17:45:14 artom Sure, thanks for the help so far
17:45:37 sean-k-mooney ill be back in an hour or so. for now i would suggest lets just fix the case we know is broken but should work
17:45:51 artom I suppose
17:45:54 sean-k-mooney we can try and figureout how to make vfio-pci work as a sperate patch
17:46:09 artom Yeah, incremental improvement and all that
17:46:45 sean-k-mooney yep. i also agree with mriedem it would be nice to add a functional test for this if we can.
17:47:17 sean-k-mooney anyway ill be back in about an hour and a half after dinner
17:47:19 artom That's always the problem with functional tests - unless you *know* all the possible things the hardware can do, they're mostly useless
17:47:32 mriedem i didn't necessary ask for a functional test,
17:47:41 mriedem anything in functional testing for that would have to be stubbed with fakelibvirt anyway
17:47:53 artom Otherwise you're essentially re-writing the same code, but shaped like a test
17:48:15 mriedem bbiab but i'll review https://review.opendev.org/#/c/667177/ when i get back
17:48:20 artom And clearly I *don't* know all the possible things the hardware can do, that's why we're here in the first place
17:55:09 sean-k-mooney ya i guess that is fair. i wont be back at my place in shannon until wednesday but i can look at recreating the vfio-pci case later this week or early next week.
17:56:16 sean-k-mooney i was assuming we would just extend https://github.com/openstack/nova/blob/master/nova/tests/functional/libvirt/test_pci_sriov_servers.py to check metadtaa was generated.
17:57:19 artom sean-k-mooney, for the record, 2 VMs with identical MACs on different networks (1 VM per network) work fine
17:59:10 sean-k-mooney cool the unique constraint musts be changed somewhere
17:59:24 artom ... and actually, 1 VM with 2 ports, same MAC on both but different networks, also works fine
17:59:37 artom Which is... worrying
18:01:50 artom Oh, the address is now being stored as MAC/port_uuid
18:01:54 artom This seems new...
18:07:44 sean-k-mooney oh right i rememebr that hack
18:07:48 sean-k-mooney ok that makes sense
18:08:43 sean-k-mooney we did it that way because nova networks required a unique mac
18:08:47 sean-k-mooney but neutron did not
18:08:49 artom So https://review.opendev.org/#/c/304511/, which depends on MAC address global uniqueness (or at least per-instance uniqueness), merged the same say as https://review.opendev.org/#/c/336069/, which removed said uniquenss
18:12:37 mriedem that's from newton
18:12:59 artom I know, I'm just going back in time and realizing that device tagging never fully worked
18:13:15 dansmith WAT
18:13:18 artom Because if you had a VM with 2 identially-MAC'ed NICs, device tags could get switched around
18:15:08 artom Most people probably have autogenerated MACs, so the likelihood of collisions is pretty low (made apparent by this not having been reported in 3 years), but still technically broken
18:16:49 dansmith ah well, that's less concerning to me I think
18:17:02 dansmith a single machine with two identical macs on different networks would be confusing to the guest OS as well
18:27:17 openstackgerrit Artom Lifshitz proposed openstack/nova master: WIP: Device tagging: expose target PCI address, not source https://review.opendev.org/672127
18:27:17 openstackgerrit Artom Lifshitz proposed openstack/nova master: Device tags: don't pass pf_interface=True to get_mac_by_pci_address https://review.opendev.org/670593
18:27:28 artom mriedem, sean-k-mooney ^^
18:28:55 artom (Thanks for hitting those backports!)
18:29:18 mriedem you didn't really need to rebase this... https://review.opendev.org/#/c/670593/1..2
18:34:07 sean-k-mooney dansmith: i think the usecase for identical mac on different networks had something to do with bonds/loadbalancing but its limited scope for when it actully makes sense
18:34:50 dansmith sean-k-mooney: that only makes sense if you've got upstream filtering by the network or something.. normally boding two nics in linux uses two macs and the bonding agent chooses one
18:35:28 sean-k-mooney i know that people used to use it with sriov and custom physnets to force VF to come from differnet phyiscal nic which they would bound in the guest
18:36:27 sean-k-mooney so they would create 2 netwroks with the same vlan on different physnets that were physically the same netwrok then create two ports with the same mac and bond them
18:37:06 dansmith presumably because neutron or ovs or whatever will not allow you to transmit as a different mac than the port expects right?
18:37:23 sean-k-mooney dansmith: right the correct way to do it would be two mac and then use teh allow_adress pair extention to allow the ohter mac
18:37:30 dansmith I understand the use as a workaround, but it's a nightmare for exactly this sort of confusion to nova and the guest inside
18:37:50 sean-k-mooney yes neutron mac anti spoofing rules would drop it if you didnt use the allowed adress pairs extention
18:38:00 sean-k-mooney at least for ovs
18:38:08 dansmith aye
18:38:34 sean-k-mooney for sriov i think it would similarly be drop by the nic unless you toll the nic specific ally to allow the other mac
18:38:43 sean-k-mooney im not sure if we support that or not
18:39:54 sean-k-mooney dansmith: but ya ist a pain in the ass and really just a hack
18:42:38 artom mriedem, failure to git review -R strikes again :(
18:45:04 efried in your .gitconfig and you'll never have to remember -R again.
18:45:04 efried rebase = false
18:45:04 efried [gitreview]
18:45:04 efried artom: I think you skipped out the other day when I mentioned this, but you can put
18:46:19 artom efried, cheers for that
18:47:04 artom (Though I think in this case I actually did a git rebase -i mindlessly when I realized I'd forgotten something in the patch below the new WIP one)
18:52:27 openstackgerrit Andreas Jaeger proposed openstack/python-novaclient master: Update api-ref location https://review.opendev.org/672135
19:06:12 openstackgerrit Matt Riedemann proposed openstack/nova master: Add FUP unit test for port heal allocations https://review.opendev.org/672142
19:07:40 sean-k-mooney git review does not automaticlaly rebase
19:08:28 sean-k-mooney it check if the patch can be rebased/merged but it doesnt auto rebase
19:08:43 sean-k-mooney man git-review
19:10:02 mriedem melwitt: i cherry picked this using the gerrit ui https://review.opendev.org/#/c/672038/
19:10:06 mriedem so i'm not sure what merge conflict you're seeing
19:11:22 melwitt mriedem: I know. just saying when I cherry-pick -x it, the test conflicts. unless I've done something wrong
19:13:46 mriedem i'm not sure why the gerrit ui wouldn't fail then
19:13:52 mriedem usually the ui is pickier
19:24:52 openstackgerrit Matt Riedemann proposed openstack/nova master: Correct project/user id descriptions for os-instance-actions https://review.opendev.org/670027
19:30:47 mriedem easy api-ref fix ^
19:44:26 openstackgerrit Artom Lifshitz proposed openstack/nova stable/stein: libvirt: move checking CONF.my_ip to init_host() https://review.opendev.org/672154
19:47:12 openstackgerrit Artom Lifshitz proposed openstack/nova stable/stein: libvirt: move checking CONF.my_ip to init_host() https://review.opendev.org/672154
19:47:44 openstackgerrit Artom Lifshitz proposed openstack/nova stable/rocky: libvirt: move checking CONF.my_ip to init_host() https://review.opendev.org/672155
19:48:21 openstackgerrit Artom Lifshitz proposed openstack/nova stable/stein: libvirt: move checking CONF.my_ip to init_host() https://review.opendev.org/672154
19:49:10 openstackgerrit Artom Lifshitz proposed openstack/nova stable/rocky: libvirt: move checking CONF.my_ip to init_host() https://review.opendev.org/672155
20:00:31 openstackgerrit Artom Lifshitz proposed openstack/nova stable/queens: libvirt: move checking CONF.my_ip to init_host() https://review.opendev.org/672161

Earlier   Later