Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-06-10
15:45:07 artom (That's from both emotional and physical pain)
15:48:24 aspiers stephenfin: ok to add you to reviewers for a couple of SEV patches which need more core attention? they're not particularly big or complex
15:48:31 stephenfin sure
15:48:35 aspiers thanks!
16:00:48 mriedem artom: sean-k-mooney: mdbooth: dansmith: comments in https://review.opendev.org/#/c/644881/
16:01:18 artom Whoa, you weren't kidding about tearing up
16:01:37 mdbooth Whoa!
16:01:43 mdbooth mriedem: Thanks, btw
16:02:33 artom sean-k-mooney, correct me if I'm wrong, but I don't believe "The bigger issue I have is the concern that we could have a server with both ovs hybrid plugged ports and ports without that" is possible
16:03:16 sean-k-mooney it is only posible if you have ovs +sriov ports
16:03:43 sean-k-mooney e.g. if they are all ovs ports then it would all be hybrid_plug true or false
16:03:59 artom sean-k-mooney, ah, so ports on which the firewall driver that causes hybrid plug to be used doens't apply
16:04:08 sean-k-mooney but if you have some vswitch prot for say managmenet and some sriov ports for the data plane then you could have a mixx
16:04:10 artom (https://review.opendev.org/#/c/660782/6/playbooks/legacy/nova-multinode/run.yaml@55)
16:06:16 sean-k-mooney right so if you are using sriov there is no firewall
16:06:18 artom So... do we need to handle that? Ie, wait for hybrid plugged ports in the compute manager, and for the sriov ones in the virt driver?
16:06:43 artom Or is there no vif to plug for the latter case, so it's irrelevant?
16:07:19 mriedem or will you get an event regardless b/c of the port binding change, in which case you don't need to filter on ovs hybrid plug in the compute manager
16:07:20 sean-k-mooney that is a good question there should be a vif plugged in that case
16:07:24 sean-k-mooney so i guess we would
16:07:54 mriedem which would take this change back to when artom, dansmith and i talked about it on hangouts 2 months ago
16:09:54 sean-k-mooney we would get an event for the sriov port
16:09:58 artom *sigh*
16:10:10 artom I suppose we better test it
16:10:20 sean-k-mooney but im trying to remeber when
16:10:49 sean-k-mooney plug is a noop for sriov. it is handeled by libvirt.
16:11:10 artom Actually, wait
16:11:27 artom Is resizing an instance with SRIOV supported?
16:11:30 sean-k-mooney yes
16:11:41 sean-k-mooney its kind of buggy but yes
16:12:45 sean-k-mooney althogh https://review.opendev.org/#/q/topic:bug/1809095+(status:open+OR+status:merged) will fix the most recent one
16:16:08 sean-k-mooney this is concerning https://github.com/openstack/neutron/blob/master/neutron/plugins/ml2/drivers/mech_sriov/mech_driver/mech_driver.py#L131-L143
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

Earlier   Later