Earlier  
Posted Nick Remark
#openstack-nova - 2018-06-26
16:14:55 openstackgerrit Merged openstack/nova master: xenapi: drop deprecated vif_driver config option https://review.openstack.org/567877
16:15:17 stephenfin But I didn't want to reuse that in case I wanted to add specific properties or the likes in the future
16:15:51 mriedem InstanceNUMANetworkAffinity is meant to be the requested thing right?
16:15:57 mriedem NUMANetworkAffinity is the inventory on the host
16:16:03 mriedem s/host/NUMACell/
16:16:07 stephenfin Yup
16:16:10 dansmith I don't think so
16:16:17 mriedem that was my understanding from the spec
16:16:17 stephenfin well
16:16:27 stephenfin first one is metadata about the requested thing (networks)
16:16:28 dansmith we're populating the InstanceNUMAOne from the details of the network you asked for right?
16:16:30 dansmith right
16:16:41 dansmith it's all metadata about the network, regardless of why we're looking for it
16:17:04 dansmith seriously, why not call this NetworkDetails ?
16:17:34 stephenfin No reason not to, other than it sounding mighty generic
16:17:46 dansmith right, but none of this really has anything to do with NUMA itself,
16:17:57 dansmith we're just adding it whilst doing some NUMA stuff, which doesn't make it NUMA-related
16:18:06 mriedem if only we had a nova.objects.Network object...
16:18:19 mriedem i kid
16:18:25 stephenfin dansmith - a man after jaypipes' own heart
16:18:39 stephenfin a'ight, NetworkDetails it is
16:18:53 dansmith stephenfin: and only one please :)
16:19:03 stephenfin all those patches are going to fail anyway. I half-assed the rebase to get it out of the gate
16:19:19 dansmith stephenfin: does tunneled apply to non-physnet-related networks?
16:19:30 mriedem is that thing going to be defined in nova/objects/network.py? because right now you have those other objects in numa-specific modules
16:19:30 dansmith NetworkPhysicalDetails if tunneled is physnet-specific
16:19:57 stephenfin yes. A network is either L2 (physnet) or L3 (tunnelled)
16:20:08 dansmith um
16:20:31 dansmith so why are physnets and tunneled on the same object?
16:20:43 dansmith could you ever have tunneled=True with something non-empty in physnets/
16:21:07 stephenfin sean-k-mooney[m] tells me yes, I can
16:21:11 jaypipes dansmith: I asked the exact same question of stephenfin :)
16:21:17 stephenfin but in general, you'd have one or the other set
16:21:24 dansmith stephenfin: so why do we need both?
16:21:30 dansmith if physnets is empty, it's tunneled?
16:22:21 stephenfin We don't create one of these objects for ever requested network - we combine them for multiple networks
16:22:30 stephenfin So I could request networks a to d
16:22:34 mriedem that raises a concern / question i had in the patch that sets tunneled during the api request processing
16:22:45 stephenfin c could be a VXLAN network
16:22:53 stephenfin a and b could use physnet foo
16:22:59 stephenfin and d could use physnet c
16:23:00 dansmith stephenfin: I'm super confused now
16:23:01 stephenfin *d
16:23:09 mriedem https://review.openstack.org/#/c/564445/10/nova/network/neutronv2/api.py@1660
16:23:27 stephenfin OK, from the top
16:23:27 mriedem according to ^ if any requested network is tunneled, they are all going to be marked as tunneled
16:23:30 dansmith (note that this is why I like to avoid merging objects until we see how they're used)
16:23:35 mriedem even if one is vlan
16:24:58 stephenfin A L2 network can be mapped to one of many physical networks
16:25:11 stephenfin *A neutron L2 networks
16:25:28 stephenfin but neutron only supports a single tunnel endpoint
16:26:11 openstackgerrit Merged openstack/nova master: network: Rename '_get_phynet_info' https://review.openstack.org/577233
16:26:16 openstackgerrit Merged openstack/nova master: network: Rename 'create_pci_requests_for_sriov_ports' https://review.openstack.org/575488
16:26:34 stephenfin I need that object for two things
16:26:42 mriedem so you're building business logic into the InstanceNUMANetworkAffinity object
16:26:59 mriedem it seems what you really need is a simple object that contains the physnet name and tunneled value (based on type)
16:27:07 mriedem you get a list of those and put them in the request spec or whatever for scheduling
16:27:22 mriedem if at least 1 is tunneled, then you do your business logic for that later
16:27:29 tssurya mriedem: ack , I will backport it now
16:27:58 tssurya also the spec is updated, I am working on the POC for our final option
16:28:00 dansmith mriedem: I guess I'm not sure why you need the tunneled value .. I thought if physnets is set, it implies !tunneled
16:28:33 stephenfin dansmith: not if my host is connected to multiple networks
16:28:49 stephenfin ditto for my guest
16:29:24 stephenfin If network foo was a VLAN network, network bar a VXLAN network, and an instance was attached to both, I'd need to represent that
16:29:29 dansmith okay, but if you do multiple of those request objects...
16:29:55 dansmith but that's what I was asking above, if it ever makes sense to have physnets and tunneled be true
16:30:48 stephenfin Yeah, it wouldn't make sense to have both attributes if we split this up
16:31:17 dansmith I guess I'm also still a little confused now, because it seems like you're representing multiple networks here,
16:31:25 stephenfin Because on the individual objects (one per request, I guess?) if physnet_name was set, tunneled would always be False
16:31:28 stephenfin I am
16:31:31 dansmith but have no info on which network is connected to which physnet, or which numa node
16:31:42 dansmith although I guess you've got that in config on compute...
16:31:44 stephenfin I don't care about the former
16:32:00 stephenfin I just care that I am connected to these physnets
16:32:18 mriedem stephenfin: wouldn't it be 1 per nic in the request?
16:32:35 stephenfin per NIC?
16:32:42 mriedem per requested network
16:32:59 mriedem you said, "Because on the individual objects (one per request, I guess?)"
16:33:10 stephenfin Oh, sorry, network request
16:33:36 stephenfin But also, not really if, say, network foo and network bar had the same physnet attribute
16:33:42 mriedem right that's what i'm saying, because create_resource_requests loops over the requested networks
16:33:51 mriedem oh
16:34:39 stephenfin You could be attaching multiple ports that are all sharing the same network
16:34:48 mriedem yup
16:35:17 mriedem or none of the networks have provider:physical_network
16:35:29 stephenfin yeah
16:35:57 stephenfin I guess you could say I'm trying to distill what the user requested to: what physnets, if any, are these networks mapped to and are any of the networks tunneled
16:36:31 mriedem above you said, "if physnet_name was set, tunneled would always be False"
16:36:40 dansmith right, confused about the 'and' there
16:37:02 stephenfin that's if I was to not use this combined object as I'm doing
16:37:28 stephenfin it's possible that half of the requested networks could be tunneled and the other half could be L2 type with a physnet attribute set
16:37:43 mriedem so the other way you can do this is just store a list of requested networks that are physical networks, right? and then infer the tunneled value if that's not empty
16:37:50 dansmith so in that case, wouldn't physnets be non-empty and tunneled be True?
16:38:01 stephenfin for the combined object, yeah
16:38:14 dansmith mriedem: right, that's where I thought we were going with the multiples here
16:38:32 mriedem stephenfin: you seem to be contradicting what is possible regardless of how you plan on modeling it
16:38:46 mriedem if physnets: tunneled = False, unless you have requested tunneled networks
16:38:51 stephenfin lemme scroll up
16:38:54 stephenfin I thought you were suggesting that I have some kind of list object with multiple sub objects?
16:39:05 mriedem no

Earlier   Later