Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-06-10
16:16:32 artom I suppose I could hax _get_neutron_events_for_migration() to be smart about what vifs it returns...
16:16:52 artom That seems fragile as hell, event more than the patch in its current form
16:16:54 sean-k-mooney if the the prot is a PF https://github.com/openstack/neutron/blob/master/neutron/plugins/ml2/drivers/mech_sriov/mech_driver/mech_driver.py#L163-L168
16:17:03 sean-k-mooney we will send the event when we update the binding
16:17:16 sean-k-mooney if the port if a vf we will send the event when the agent wires it up
16:18:04 artom At point do we just say screw it and start queuing reveived events?
16:18:09 artom *At what
16:18:27 artom Or unload all those problems on Neutron and tell them to send the events consistently?
16:19:13 sean-k-mooney well https://review.opendev.org/#/c/645173/ is ment to help with this
16:19:22 artom Because assuming what sean-k-mooney is saying is True (and I have no reason to doubt it, but it def needs testing), we now have 3 different port "types" that we need to be aware of
16:19:40 sean-k-mooney but i have not been working on it because of other stuff like RDT/CAT
16:20:53 artom sean-k-mooney, yeah, but depending on that spec would make the Nova patch unbackportable, which I'm trying to avoid
16:21:03 artom (Never mind that it would take forever
16:21:05 artom )
16:21:53 sean-k-mooney right also i have not had time to work on it for a month and likely wont in the short term as i have nto figured out how to expresss when the event will be sent yet in an abstct way
16:22:44 artom I guess "do what we did before (wait in libvirt), except for known cases where it doesn't work and we have to do new thing (wait in compute manager)" would be a valid guiding principle
16:23:05 artom I'm gonna go get lunch, then try and see if I can code that in a palatable way
16:23:14 sean-k-mooney you would have to wait if hybrid_plug=true or vnic_type=direct-physical
16:23:20 sean-k-mooney in the manager
16:23:27 sean-k-mooney else waith in the driver
16:23:51 artom sean-k-mooney, so, that latter part hasn't actually come up (yet?), so I don't think I want to include it
16:24:24 artom But I'm going to try to genericize my _uses_hybrid_plug() method a bit to make it extensible
16:24:25 sean-k-mooney we could fix sriov in a sperate patch?
16:24:33 sean-k-mooney artom: mriedem thoughts ^
16:25:02 artom ... and also do as yet unclear things to allow the compute manager to communicate to the virt driver *which VIFs* it needs to listen to events for
16:25:11 artom Or some other way of achieving the same effect
16:25:17 sean-k-mooney artom: we would need to be able to express on a per vf bassis if we should wait which is what i want to do with that sepec anyway
16:25:38 artom sean-k-mooney, yeah, it's basically the same, just without the Neutron part
16:26:34 sean-k-mooney we would have to embed logcici into the vif object to encode "wait in dirver" or what ever instead of using a flag for all vifs yes
16:27:19 artom sean-k-mooney, that's not going to be backportable either, is it?
16:28:58 sean-k-mooney the vif "objects" we store in the networkin info cache are dicts so its not versioned
16:28:58 efried stephenfin: I've got a permalink in nova docs
16:28:58 efried https://opendev.org/openstack/nova/blame/commit/1316c1c2850d2f966f335b628f7f5fe88cef611c/doc/source/user/flavors.rst#L776
16:28:58 efried that I want to use from a reno. How do I do that?
16:29:10 sean-k-mooney but i would have to check the code to determin that
16:29:41 efried stephenfin: I don't think I can just use extra-specs-numbered-resource-groupings_ - because the renos and the docs are in separate repositories?
16:29:42 mriedem efried: https://docs.openstack.org/nova/latest/user/flavors.html#extra-specs-numbered-resource-groupings
16:29:59 mriedem unless nova-doc works in reno builds
16:30:10 stephenfin efried: https://docs.openstack.org/nova/latest/user/flavors.html#extra-specs-numbered-resource-groupings
16:30:13 stephenfin yeah
16:30:30 stephenfin it doesn't because release note builds aren't versioned
16:30:32 efried stephenfin: mriedem: Right, so I have to use the whole URL, okay. Thanks.
16:30:33 artom sean-k-mooney, oh, those VIFs. Well, we have what we need already, no? ovs_hybrid_plug and type
16:30:36 sean-k-mooney artom: ya the vif you are using in that code is an instance of nova.network.model.Vif https://github.com/openstack/nova/blob/master/nova/network/model.py#L380
16:30:48 sean-k-mooney yes we do
16:31:28 sean-k-mooney but if you wanted to add a flag so you did not check bot of those you could add a property to that class that return a vlaue base on if ether of the dict values were set
16:32:46 sean-k-mooney e.g. put the logic to wait into the Vif class and have a property "wait_location" that return "driver" or "manager" based on the vnic_type or hybrid_plug
16:32:59 sean-k-mooney artom: like https://github.com/openstack/nova/blob/master/nova/network/model.py#L461
16:33:14 sean-k-mooney i guess it could be just a fuction too
16:33:23 aspiers efried: according to the SEV spec, our current plan is to add hw:mem_encryption to flavor extra specs and image properties, but I've just noticed that while existing extra specs params use the "hw:" prefix, image properties all use the "hw_" prefix. Presumably I should remain consistent with those conventions?
16:33:51 artom sean-k-mooney, not sure about putting logic in the model itself, but yeah, it's an idea
16:34:07 mriedem sean-k-mooney: so as it stands, artom's patch will do:
16:34:19 sean-k-mooney artom: well we have is_hybrid_plug_enabled in the Vif object
16:34:19 mriedem 1. if all ports are ovs_hybrid_plug=True, wait for all in the manager and you're fine,
16:34:24 artom 0. drive artom to even moar excessive alcoholism
16:34:35 mriedem 2. if all ports are ovs_hybrid_plug=false, wait in the driver and you're fine (tested in the gate),
16:34:39 artom sean-k-mooney, yeah, but that's basically just a getter
16:35:07 mriedem 3. if some ports are ovs_hybrid_plug=True but not all, we'll wait for all events for all ports in the manager and some might not come (at least direct-physical)
16:35:24 sean-k-mooney mriedem: correctr
16:35:40 mriedem ok so based on #3,
16:36:04 mriedem what if we only wait for the known ones in the manager (ovs_hyrbid_plug=True),
16:36:26 sean-k-mooney direct-pysical will be the same as ovs_hyrbid_plug=True
16:36:35 mriedem and if there are others, we pass vifs_already_plugged=False to the driver? that would fail in the ovs_hybrid_plug=true ports b/c we already processed that event yeah?
16:36:58 sean-k-mooney vnic_type=direct and macvatap are teh same as ovs_hyrbid_plug=False and will arrive at the dirver
16:37:51 sean-k-mooney mriedem: yes it would fail for ovs_hyrbid_plug=True because we already processed it
16:37:57 mriedem i'm trying to get at some best effort kind of logic if we don't have good enough details from neutron and when we get the events from neutron is inconsistent per backend (which we know it is)
16:38:45 mriedem thinking like, compute manager waits for all that are ovs_hybrid_plug=True and if there are others (a mix), we pass vifs_already_plugged=True so the driver doesn't wait
16:38:47 sean-k-mooney if we get a mix we could just not wait at all or wait wait for the ones we expect in the manager and not wait in the driver
16:38:55 mriedem yup i just said the same
16:39:02 sean-k-mooney ya that would work
16:39:15 sean-k-mooney its technically racy but its better then what we have today
16:39:36 sean-k-mooney but i also think it would be simple to have the driver check if it should wait per vif
16:39:41 mriedem in the #2 case where none of the ports are ovs_hybrid_plug=True, we'd pass vifs_already_plugged=false to the driver and assume the driver will wait and get all of the events after it plugs
16:39:44 sean-k-mooney instead of needing to set teh flag
16:39:55 sean-k-mooney yep
16:40:23 sean-k-mooney 1 and 2 are easy
16:40:57 openstackgerrit Eric Fried proposed openstack/nova master: Defaults missing group_policy to 'none' https://review.opendev.org/657796
16:41:02 sean-k-mooney but we really shoudl be checking "ovs_hyrbid_plug=True or vnic_type=direct-physical"
16:41:16 mriedem how many different types of vifs are in the wild?
16:41:34 sean-k-mooney technically its a finite set defiend in neutron lib
16:41:49 sean-k-mooney but not all backend will treat them all the same
16:41:52 mriedem i just kind of vomit in my mouth a bit when we try encoding this in nova
16:41:59 efried aspiers: Yes
16:42:08 sean-k-mooney mriedem: ya i know it sucks
16:42:09 artom mriedem, we really shouldn't have to in theory
16:42:18 artom But reality and all that
16:42:22 efried aspiers: And if you're collecting a spec tweak fup patch, clarify in there.
16:42:55 sean-k-mooney for what its worth however i think we can encode it here in a new 3 line function https://github.com/openstack/nova/blob/master/nova/network/model.py#L461
16:43:21 artom sean-k-mooney, yeah, that part's not hard
16:43:43 artom It's how we use it that I'm fuzzier on
16:43:55 artom Ie, split events to be waited for between compute and libvirt
16:44:26 artom I'm making an executing decision: ph00d, then hax. A patch, even broken WIP, is better than endless discussion
16:44:28 mriedem i'm not terribly comfortable in backporting something that gets this complicated especially since we don't test any of these vif types in the gate
16:44:34 sean-k-mooney e.g. def wait_in_manager(self):
16:44:36 sean-k-mooney retrun self.is_hybrid_plug_enabled or self.get("vnic_type") == "direct-physical"
16:45:00 mriedem artom: hacking something up that you do'nt have agreement on from the people reviewing it isn't going to speed this up
16:45:11 sean-k-mooney then in the manager an driver we just do if vif.wait_in_manager():
16:45:13 artom mriedem, I thought we kinda did?
16:45:27 mriedem how about, 1. artom gets food, 2. sean comments on the change
16:45:30 mriedem 3. artom digests both

Earlier   Later