14:00:59 <slaweq> #startmeeting neutron_drivers
14:00:59 <opendevmeet> Meeting started Fri Jul  2 14:00:59 2021 UTC and is due to finish in 60 minutes.  The chair is slaweq. Information about MeetBot at http://wiki.debian.org/MeetBot.
14:00:59 <opendevmeet> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
14:00:59 <opendevmeet> The meeting name has been set to 'neutron_drivers'
14:01:02 <mlavalle> o/
14:01:03 <opendevreview> Rodolfo Alonso proposed openstack/neutron stable/victoria: [OVN] Do not fail when processing SG rule deletion  https://review.opendev.org/c/openstack/neutron/+/799210
14:01:06 <slaweq> hi
14:01:07 <ralonsoh> hi
14:01:30 <obondarev> hi
14:01:39 <slaweq> let's wait few more minutes for people to join
14:01:50 <slaweq> I know that haleyb and njohnston are on PTO today
14:01:57 <seba> hi
14:02:01 <slaweq> but amotoki and yamamoto will maybe join
14:02:03 <amotoki> hi
14:02:15 <opendevreview> Pedro Henrique Pereira Martins proposed openstack/neutron master: Extend database to support portforwardings with port range  https://review.opendev.org/c/openstack/neutron/+/798961
14:02:43 <manub> hi
14:03:55 <slaweq> ok, let's start
14:04:06 <slaweq> agenda for today is at https://wiki.openstack.org/wiki/Meetings/NeutronDrivers
14:04:10 <mlavalle> do we have quorum?
14:04:18 <slaweq> mlavalle: I think so
14:04:25 <slaweq> there is You, ralonsoh amotoki and me
14:04:33 <mlavalle> ok
14:04:34 <slaweq> so minimum but quorum, right?
14:04:44 <amotoki> yeah, I think so
14:04:50 <slaweq> #topic RFEs
14:04:50 <ralonsoh> actually I'm presenting a RFE, so I should not vote
14:05:13 <slaweq> ralonsoh: sure, so with Your rfe we can wait for next meeting
14:05:19 <ralonsoh> perfect
14:05:38 <slaweq> we have then one rfe for today :)
14:05:40 <slaweq> https://bugs.launchpad.net/neutron/+bug/1930866
14:07:04 <ralonsoh> who is presenting it?
14:07:29 <mlavalle> doesn't matter
14:07:33 <mlavalle> we can discuss it
14:07:37 <ralonsoh> ok, perfect
14:07:44 <mlavalle> that's the usual approach
14:07:59 <slaweq> yeah, personally I think it is totally valid issue
14:08:11 <slaweq> I didn't know that nova have something like "lock server"
14:08:36 <mlavalle> it is. we should worry about the complete end user experience accross all projects, not only Neutron
14:08:58 <slaweq> mlavalle: yes, exactly :)
14:09:00 <mlavalle> end users don't use Neutron. They use OpenStack
14:09:54 <ralonsoh> because this is part of the Nova API, can we ask them to modify the VM ports? as obondarev suggested
14:10:23 <ralonsoh> or should be us responsible of checking this state?
14:10:27 <slaweq> ralonsoh: yes, but looking from the neutron PoV only, we should provide some way to "lock" port in such case
14:10:31 <amotoki> the bug is reported about locked instances. what I am not sure is whether we need to handle ports used by locked instances specially.
14:10:40 <mlavalle> similar to what we do with the dns_name attribute when NOva creates a port for an instance
14:10:41 <slaweq> then nova could "lock" port as part of server lock
14:11:31 <amotoki> potentially endusers can hit similar issues even for non-lcoked instances.
14:11:43 <mlavalle> yeap
14:12:07 <ralonsoh> I don't see in what scenario, sorry
14:12:10 <slaweq> amotoki: IMHO we should, I'm not sure if forbid to delete any port which is attached to the instance would be good idea as that would be pretty big change in API
14:12:20 <mlavalle> but in the case of locked instances we really deliver an awful end user experience, because OpenStack made a promise that gets broken
14:12:21 <jkulik> looking from Cinder perspective, if a Volume is attached, you cannot just delete it. would the same make sense for a port?
14:12:51 <obondarev> neutron already forbids deleting port of certain types, right?
14:13:25 <slaweq> if we will change neutron so it will forbid deletion all ports which are attached to vms, we will break nova I think
14:13:46 <slaweq> and nova will need to adjust own code to first detach port and then delete it
14:13:51 <amotoki> yes, we already forbit deleting ports used by router interfaces (and otherr maybe)
14:13:57 <slaweq> and that will make problems during e.g. upgrade
14:14:07 <slaweq> or am I missing something?
14:14:45 <ralonsoh> slaweq, right we need to mark those ports somehow
14:14:46 <mlavalle> no
14:14:53 <amotoki> slaweq: I haven't checked the whole procedure in server delete. It may affect nova precedures in deelting ports attached to instances.
14:15:01 <mlavalle> no, you are not missing anything
14:15:26 <mlavalle> maybe we want to discuss this with Nova folks
14:15:33 <mlavalle> is gibi around?
14:15:41 <gibi> mlavalle: hi
14:15:47 <slaweq> hi gibi :)
14:16:01 <jkulik> tbh, I always found it confusing that there are 2 interfaces to attach/detach a port/network to/from an instances - Nova and Neutron directly
14:17:00 <amotoki> jkulik: precisely speaking there is no two ways to attach ports. neutron port deletion is not visilbe to nova, so it confuses users.
14:17:11 <sean-k-mooney> for what its worth we have suggested bocking port deletion of in use port in the past
14:17:25 <sean-k-mooney> amotoki: actully it is
14:17:43 <sean-k-mooney> neutron send a network-vif-delete event to nova when the neutron prot is delete
14:18:16 <amotoki> sean-k-mooney: ah, good point. i totally forget it.
14:18:46 <sean-k-mooney> amotoki: form a nova point of vew we have never really support this usecause though we would really prefer if you detached it first then deleted it if you needed too
14:19:30 <ralonsoh> sorry but I don't think we should go this way, making this change in Neutron/Nova
14:19:34 <gibi> I agree with sean-k-mooney, while deleting a bound port is possible today and there is some level of support for it in nova, this is something that complicates things
14:19:48 <sean-k-mooney> regarding https://bugs.launchpad.net/neutron/+bug/1930866 is there an object to just blocking port delete while it has the device-owner and device id set
14:20:17 <slaweq> gibi: sean-k-mooney: but today nova, when e.g. vm is deleted will just call neutron once to delete port, right?
14:20:26 <slaweq> or will it first detach port and then delete it?
14:20:41 <sean-k-mooney> slaweq: that is a good question
14:21:03 <gibi> slaweq: nova will unbind the port during VM delete, and if the port was actually created by nova during the boot with a network, then nova will delete the port too
14:21:11 <sean-k-mooney> we proably dont do a port update and then a delete but we could
14:21:30 <sean-k-mooney> gibi: oh we do unbind i was just going to check that
14:21:46 <seba> disallowing port delete for ports with deviceowner/device_id set would mean that a user could not remove dangling ports anymore without having write access to those fields
14:22:28 <sean-k-mooney> seba: those feild i belive are writable by the user. and they still could by doing a nova server delete or port detach via nova
14:22:44 <amotoki> seba: no, what we discuss is just about port deletion.
14:23:03 <gibi> the above RFE talks about locked instances specifically. Nova could reject the network-vif-delete event if the instance is locked. It could be a solution
14:23:17 <slaweq> device_owner is writable by NET_OWNER and ADMINs https://github.com/openstack/neutron/blob/master/neutron/conf/policies/port.py#L380
14:23:28 <ralonsoh> gibi but at this point the port is already gone
14:23:34 <sean-k-mooney> gibi: i think neutron sends that asyc form the deletion of the port
14:23:35 <slaweq> so in typical use case, user will be able to clean it
14:23:46 <mlavalle> I agree with gibi ... let's reduce the scope of this to locked instances
14:23:47 <gibi> ralonsoh: ahh, then never mind, we cannot do that
14:23:52 <amotoki> gibi: but neutron still can delete a port though?
14:24:38 <sean-k-mooney> mlavalle: well neutron realy shoudl not care or know if an instance is locked
14:24:42 <amotoki> ralonsoh commented the same thing already :)
14:24:43 <gibi> my suggestion can only be implemented if nova can prevent the port deletion by rejecting the netwokr-vif-delete event
14:25:17 <sean-k-mooney> gibi: that would require the neutron server to send that event first and check the return before proceeding with the db deletion
14:25:29 <gibi> sean-k-mooney: yeah, I realize that
14:25:42 <sean-k-mooney> i dont think neutroc currently check the status code of those notificaitons
14:25:49 <slaweq> personally I like the idea of not allowing port deletion if it is attached to vm but to avoid problems during e.g. upgrades we could add temporary config knob to allow old behaviour
14:26:18 <slaweq> if we would forbid deletion of such ports it would be more consistent with what cinder do
14:26:31 <gibi> slaweq: if we go that way the I thin Octavia folks should be involved, I think they also depend on deleting a bound port
14:26:31 <slaweq> so IMHO more consisten UX in general :)
14:26:49 <ralonsoh> qibi do Octavia use Nova or Neutron API?
14:26:50 <slaweq> gibi: ouch, I didn't know that
14:27:02 <slaweq> so I wonder who else we may break :)
14:27:16 <gibi> I have a faint recollection from a PTG where they approached us with this port delete case as nova had some issue with it
14:27:25 <jkulik> https://github.com/sapcc/nova/blob/cd084aeeb8a2110759912c1b529917a9d3aac555/nova/network/neutron.py#L1683-L1686 looks like nova unbinds pre-existing ports, but directly deletes those it created without unbind. looks like an easy change though.
14:27:29 <gibi> I have to dig if I found a recording of it
14:27:48 <slaweq> jkulik: that's what I though :)
14:27:49 <gibi> jkulik: good reference, and I agree we can change that sequence
14:27:54 <slaweq> so nova would need changes too
14:29:12 <sean-k-mooney> yes it look like it would we coudl backport that however
14:29:29 <sean-k-mooney> maybe you could contol this tempoerally on the neutron side with a workaround config option
14:29:47 <sean-k-mooney> i hate config driven api behavior but since neutron does not use microverions
14:30:08 <sean-k-mooney> the only other way to do this would be with a new extsion but tha tis not backportable
14:30:12 <ralonsoh> but we have extensions
14:30:41 <jkulik> if it's not a bugfix, but a change to not allow deletion of ports with device-owner and -id, can this even be supported by a new API version? or would all old API versions also behave differently, then?
14:30:46 <slaweq> ralonsoh yes, but imagine upgrade case:
14:30:53 <slaweq> older nova, new neutron
14:30:54 <obondarev_> so what are downsides of: "nova sets 'locked' flag in port_binding dict for locked instances, neutron checks that flag on port delete"?
14:31:07 <slaweq> old nova wants to delete bound port and it fails
14:31:17 <slaweq> and old nova don't know about extension at all :)
14:31:17 <sean-k-mooney> slaweq: so the extion would hae to be configurable
14:31:40 <ralonsoh> slaweq, right. So let's make this configurable for the next release
14:31:44 <slaweq> sean-k-mooney: yes, that's what I wrote few minutes ago also :) I think that we could add temporary config option for that
14:32:12 <sean-k-mooney> slaweq: yep for xena and then make it mandaroy for Y
14:32:14 <slaweq> I know we did that already with some other things between nova and neutron
14:32:24 <sean-k-mooney> that would allow nova to always unbidn before deleting
14:32:24 <slaweq> sean-k-mooney++ for me would be ok
14:32:32 <jkulik> iirc, locked instances in Nova can still be changed by an admin
14:32:47 <jkulik> would Nova then be able to delete the locked port, too?
14:32:58 <sean-k-mooney> jkulik: well instances are locked not ports right
14:33:07 <slaweq> exactly
14:33:11 <jkulik> sean-k-mooney: if we would go for "nova locks the port in neutron"
14:33:40 <sean-k-mooney> ok so new neutron extion for locked ports
14:33:53 <sean-k-mooney> if nova detect it we lock them automiatcly if you lock the vm?
14:34:07 <mlavalle> that seems reasonable
14:34:25 <mlavalle> nova already detects other neutron extensions, like extended port binding
14:34:33 <sean-k-mooney> and then neutron just prevents updating locked ports
14:34:49 <sean-k-mooney> mlavalle: yep that shoudl not be hard to add on the nova side
14:34:54 <gibi> sean-k-mooney: does it mean nova needs an upgrade step where it updates the ports of already locked instances?
14:35:19 <gibi> like syncing this state for exising instances
14:35:24 <sean-k-mooney> good question
14:35:31 <sean-k-mooney> we could do that in init host i guess
14:36:03 <sean-k-mooney> our we could just not and document that you will need to lock them again
14:36:10 <gibi> I think locking / unlocking happens in the API so it would be strange to do the state synch in compute side
14:36:35 <gibi> anyhow, this needs a nova spec
14:36:49 <gibi> I dont want to solve all the open question on friday afternoon :D
14:36:49 <mlavalle> nad I suggest a Neutron spec as well
14:37:22 <sean-k-mooney> well i was going to say technially its not an api change on the nova side so it could be a specless blueprint but ya for the upgrade question a spec is needed
14:37:32 <slaweq> so do we want to add "lock port" extension to neutron or forbid deletion of ports in-use?
14:37:41 <slaweq> IIUC we have such 2 alternatives now, right?
14:37:45 <mlavalle> add lock port extension
14:37:45 <sean-k-mooney> yes
14:37:51 <amotoki> yes
14:38:16 <sean-k-mooney> either seam vaild but lock-port is proably a better mapping to the rfe
14:38:37 <jkulik> if I'm admin, I can delete a locked instance. Neutron needs to take this into account for the "locked" port
14:38:45 <sean-k-mooney> i would still personally like to forbid deliting in use ports
14:39:09 <sean-k-mooney> jkulik: well no nova can unlock them in that case
14:39:13 <amotoki> the bug was reported for locked instances, but I personally prefer to "block deletion of ports used by nova".
14:39:23 <slaweq> but what "locked port" would mean - it can't be deleted only? can't be updated at all?
14:39:37 <jkulik> sean-k-mooney: yeah, makes sense
14:39:52 <sean-k-mooney> slaweq: i would assume cant be updated at all but we could detail that in the spec
14:40:19 <slaweq> IMHO blocking deletion of ports in use is more straight forward solution, but from the other hand it may break more people so it's more risky :)
14:40:47 <slaweq> sean-k-mooney: yeah, we could try to align with nova's behaviour for locked instances
14:40:54 <amotoki> neutron already blocks direct port deletion of router interfaces. in this case we disallow to delete ports used as router interface, but we still allow to update device_owner/id of such ports. if users would like to delete such ports expliclity they first need to clear device_owner/id and then they can delete these ports.
14:40:55 <slaweq> and that can be clarified in the spec
14:41:13 <sean-k-mooney> i know that doing a delete this way used ot leak some resouces on the nova side in the past like sriov resouces
14:41:26 <sean-k-mooney> slaweq: unfrotunetly i dont knwo off hand what the nova behivor actully is
14:41:40 <sean-k-mooney> but yes it would be nice to keep them consitnet
14:41:44 <slaweq> sean-k-mooney: np, it can be discussed in the spec as You said :)
14:44:42 <slaweq> so, do we want to vote for prefered option?
14:45:10 <mlavalle> if we have to vote I lean towrds lock port extension
14:45:23 <obondarev> if use existing port's binding_profile dict - do we need a neutron API extension at all?
14:45:50 <slaweq> obondarev: I would say yes, to make discoverable new behaviour in neutron
14:45:52 <mlavalle> another key - value pair there?
14:45:54 <slaweq> it's still API change
14:46:07 <obondarev> ok, makes sense
14:46:09 <amotoki> slaweq: +1
14:46:10 <slaweq> so You need to somehow tell users that neutron supports that
14:46:24 <ralonsoh> IMO this conversation has been a but chaotic: we started with the "lock port" idea, then we moved to block a bounf port deletion and now we are voting for a "lock port" extension
14:46:37 <ralonsoh> I really don't understand what happened in the middle
14:46:52 <slaweq> ralonsoh: :)
14:46:57 <mlavalle> then let's not vote and decide the entire thing in the spec
14:47:03 <ralonsoh> we were going to implement this RFE by blocking the deletion of a bound port
14:47:17 <amotoki> yeah, we discussed two approaches
14:47:21 <ralonsoh> I know
14:47:26 <ralonsoh> but mixing both
14:47:50 <ralonsoh> so the point is to provide a transition knob from neutron to Nova
14:47:53 <ralonsoh> or extension
14:48:02 <ralonsoh> to know if this is actually supported in Neutron
14:48:10 <ralonsoh> and then implement the port deletion block
14:48:19 <ralonsoh> (that will also comply with the RFE)
14:48:42 <sean-k-mooney> obondarev: the content of the binding_profile is own by nova and is one way
14:48:54 <slaweq> so I propose that:
14:48:57 <sean-k-mooney> it provide info from nova to the neutron backedn
14:49:17 <slaweq> 1. we will approve rfe and will continue work on it in spec - I think we all agree that this if valid rfe
14:49:46 <slaweq> 2. I will summarize this discussion in the LP's comment and will describe both potential solutions
14:50:07 <mlavalle> +1, yes it's a valid rfe
14:50:16 <ralonsoh> +1, the RFE is legit
14:50:22 <slaweq> if there is anyone who wants to work on it and propose spec for it, that's great but if not, I will propose something
14:50:32 <slaweq> *by something I mean RFE :)
14:50:41 <slaweq> sorry, spec :)
14:50:45 <mlavalle> and I can work on it
14:50:53 <slaweq> mlavalle: great, thx
14:50:53 <ralonsoh> thanks
14:51:19 <amotoki> totally agree what is proposed.
14:51:41 <slaweq> thx, so I think we have agreement about next steps with that rfe :)
14:52:13 <slaweq> according to second rfe from ralonsoh we will discuss it on next meeting
14:52:20 <ralonsoh> thanks
14:52:25 <ralonsoh> I'll update the spec
14:52:44 <slaweq> #topic On Demand agenda
14:52:55 <slaweq> seba: You wanted to discuss about https://review.opendev.org/c/openstack/neutron/+/788714
14:53:01 <seba> yes!
14:53:04 <slaweq> so You have few minutes now :)
14:53:21 <seba> okay, so just so you understand where I come from: I maintain a neutron driver using hierarchical portbinding (HPB), which allocates second-level VLAN segments. If I end up with a (network, physnet) combination exists with different segmentation_ids my network breaks.
14:53:41 <seba> This can happen when using allocate_dynamic_segment(), so my goal would be to either get allocate_dynamic_segment() safe or find another way for safe segment allocation in neutron.
14:54:33 <seba> We discussed https://bugs.launchpad.net/neutron/+bug/1791233 on another driver meeting and the idea to solve this was to employ a constraint on the network segments table to make (network_type, network, physical_network) unique.
14:55:37 <ralonsoh> that could be an solution, but doesn't work for tunneled networks
14:55:55 <sean-k-mooney> well in that case physical network would be None
14:55:55 <ralonsoh> you'll have (vxlan, net_1, None)
14:56:11 <ralonsoh> yes and repeated several times
14:56:20 <seba> ralonsoh, that should not be a problem, two "None"s are never the same
14:56:24 <sean-k-mooney> we dont curently supprot have 2 vxlan secment for one neutron network
14:57:31 <seba> ralonsoh, jkulik wrote something about the NULL values in the bugreport and how they're not the same with most if not all major databases
14:57:53 <jkulik> so that use-case would not be hindered by the UniqueConstraint
14:57:54 <sean-k-mooney> there is no valid configuration wehre we can have 2 (vxlan, net_1, None) segment with different vxlan vids right
14:58:07 <seba> sean-k-mooney, I have one top-level vxlan segment and then a vlan segment below it for handoff to the next driver. I don't see though what would stop me from having a second level vxlan segment
14:58:41 <sean-k-mooney> well i was thinkign about what the segments exteion allows
14:58:58 <sean-k-mooney> when doing heracial port binding that is sligly idfferent
14:59:01 <seba> ah, so you're thinking about multiple vxlan segments without specifying a physnet?
14:59:22 <sean-k-mooney> seba: yes sicne tunnels do not have a physnet
14:59:48 <slaweq> we need to finish meeting now, but please continue discussion in the channel :) I have to leave now becuase I have another meeting. Have a great weekend!
14:59:52 <slaweq> #endmeeting