14:00:34 <slaweq> #startmeeting neutron_drivers
14:00:35 <openstack> Meeting started Fri Jul 31 14:00:34 2020 UTC and is due to finish in 60 minutes.  The chair is slaweq. Information about MeetBot at http://wiki.debian.org/MeetBot.
14:00:36 <openstack> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
14:00:39 <openstack> The meeting name has been set to 'neutron_drivers'
14:00:48 <ralonsoh> hi
14:00:50 <slaweq> Hi
14:00:51 <rafaelweingartne> Hello
14:00:52 <yamamoto> hi
14:01:41 <amotoki> hi, but I had a virtual drinking with my team, so I might be optional today. I will try my best.
14:01:50 <njohnston> o/
14:02:03 <slaweq> I know that haleyb is on PTO this week and mlavalle will probably not be here today
14:02:26 <slaweq> but we have quorum so lets start
14:02:30 <slaweq> #topic RFEs
14:02:52 <slaweq> first one:
14:02:54 <slaweq> #link https://bugs.launchpad.net/neutron/+bug/1880532
14:02:55 <openstack> Launchpad bug 1880532 in neutron "[RFE]L3 Router should support ECMP" [Wishlist,New] - Assigned to XiaoYu Zhu (honglan0914)
14:03:13 <slaweq> we already talked about it few times
14:03:23 <mlavalle> o/
14:03:45 <slaweq> and reporter checked that it can be done with existing API only, using extraroutes attribute of the router
14:04:57 <slaweq> hi mlavalle :)
14:06:38 <slaweq> so for me this proposal looks ok now, and if we don't need to change API (or only do some very small changes, than it's fine for me)
14:07:08 <njohnston> I feel similarly
14:08:41 <yamamoto> fine for me
14:08:53 <mlavalle> Agree. Went thorugh it last week and came to the same conclusion
14:10:49 <amotoki> Generally looks good. I wonder whether we need to update the description of the API behavior, thought?
14:10:49 <ralonsoh> ok for me, I still need to review the uploaded patch
14:11:22 <slaweq> amotoki: yes, that's are small changes which I think will be needed
14:11:51 <slaweq> thx all for votes, I will mark it as approved, and please also review proposed spec :)
14:12:00 <amotoki> yeah, it is a reasonable improvement
14:12:25 <slaweq> ok, so next one
14:12:28 <slaweq> #link https://bugs.launchpad.net/neutron/+bug/1889431
14:12:29 <openstack> Launchpad bug 1889431 in neutron "[RFE] Add local-ip-prefix to Neutron metering label rules" [Wishlist,In progress] - Assigned to Rafael Weingartner (rafaelweingartner)
14:13:00 <slaweq> with that one I have small problem
14:13:37 <slaweq> it seems reasonable and valid change but from the other hand it will be backward incompatible
14:13:56 <slaweq> of course we may add new api extension to make it discoverable
14:15:39 <rafaelweingartne> to be fair, the commit cited there in [1] also broke it. And the documentation was not updated accordingly; this leads people to think that there is a bug with the current code. just when looking at the storyline, we see that it was changed on purpose.
14:16:02 <slaweq> yes, that's true
14:17:54 <rafaelweingartne> That is why we proposed a migration plan for people using the current code base. I agree with you though, we would be breaking API compatibility.
14:18:57 <ralonsoh> but what's the problem of adding an extension here?
14:19:03 <ralonsoh> to the current API
14:19:22 <slaweq> ralonsoh: no problem for me
14:19:27 <ralonsoh> ah ok
14:19:37 <rafaelweingartne> What do you guys mean by API extension?
14:19:42 <njohnston> There certainly does seem to be a legitimate case for this, and with discoverability I am totally OK with this.
14:19:45 <slaweq> just wanted to mention that we need api extension to make it discoverable
14:20:01 <rafaelweingartne> is it like microversioning in Cinder?
14:20:01 <ralonsoh> rafaelweingartne, something like this https://review.opendev.org/#/c/740058/
14:20:02 <patchbot> patch 740058 - neutron-lib - New api-def: port-numa-affinity-policy - 6 patch sets
14:20:25 <ralonsoh> when slaweq said "discoverable", that means you enable this extension and the API contains this new parameter
14:20:30 <ralonsoh> that won't break the current code
14:21:07 <amotoki> rafaelweingartne: yes, neutron does not introduce microversioning and a new feature needs to be discovered by an API extension
14:21:21 <rafaelweingartne> well, that is the problem
14:21:30 <rafaelweingartne> because we introduce a new parameter
14:21:42 <ralonsoh> and that's ok
14:21:43 <rafaelweingartne> but this new parameter will take the role/behavior of the old one
14:21:55 <ralonsoh> but referred to a new extension
14:22:08 <ralonsoh> we can discuss the technical issues in the n-lib patch
14:22:14 <amotoki> what I am not sure so far is whether how we should interprete "lcoal" now.
14:22:17 <slaweq> rafaelweingartne: this isn't a problem for sure, You will add this new parameter in new extension and that's fine
14:22:28 <rafaelweingartne> ok
14:22:43 <rafaelweingartne> I am not sure with these details in some of openstack components
14:22:48 <slaweq> rafaelweingartne: if you will need help with that, we can discuss it on neutron channel :)
14:22:55 <rafaelweingartne> sure
14:23:00 <amotoki> The problem description says that we changed the interpretation of "local"/"remote" at some time.
14:23:21 <slaweq> amotoki: that's good point
14:23:28 <rafaelweingartne> yes, here: https://github.com/openstack/neutron/commit/92db1d4a2c49b1f675b6a9552a8cc5a417973b64
14:23:41 <slaweq> wouldn't "source" and "destination" better names?
14:23:52 <rafaelweingartne> good point :)
14:23:59 <rafaelweingartne> internally, I proposed source
14:24:12 <rafaelweingartne> but the network engineers said that it would make more sense local...
14:24:18 <rafaelweingartne> I am not sure though, which one is better
14:25:05 <amotoki> what is the real problem? what is "local"?
14:25:28 <rafaelweingartne> local_ip_prefix would be the internal network behind the router
14:25:33 <slaweq> amotoki: in fact when I'm now thinking about it, it depends on "direction" really
14:25:47 <slaweq> no?
14:25:51 <rafaelweingartne> hmm
14:25:57 <rafaelweingartne> yes and no
14:26:17 <njohnston> source and dest are directional attributes, whereas local and remote are proximal attributes
14:26:27 <amotoki> IIRC the commit clarifies the meaining of "remote" was different from other context in neutron. "remote" is external, soI  think  the commit is right.
14:27:04 <rafaelweingartne> the commit in [1] is using the remote attribute as the source/local IP of the VMs inside openstack
14:27:09 <rafaelweingartne> this creates confusions
14:27:24 <rafaelweingartne> people were using the remote attribute as the remote (IP outside openstack)
14:29:17 <amotoki> sounds fair. I cannot remember the whole discussion at that time.
14:29:39 <rafaelweingartne> and also, with respect to inflow and outflow, we wrote a section describind that "Neutron Metering agent changes"
14:29:55 <slaweq> ok, so local/remote will be from the "vm" point of view - local is vm IP address and remote is something in outside world, right?
14:30:03 <rafaelweingartne> exactly
14:30:11 <slaweq> that sounds good for me
14:30:35 <rafaelweingartne> and by having these two attributes we can address the need described in [1], and many others
14:30:46 <slaweq> and now it's not like that as remote_ip_prefix is in fact IP which belongs to the OpenStack vm
14:30:48 <slaweq> right?
14:30:55 <rafaelweingartne> exactly
14:31:20 <slaweq> that's why You want to change this naming convention and not only add new parameter
14:31:21 <slaweq> ok
14:31:26 <slaweq> now it seems clear for me
14:32:33 <ralonsoh> that's perfect if this is also documented
14:32:46 <rafaelweingartne> yes, it will be
14:33:02 <rafaelweingartne> same standard as I did with the granular extension for the neutron metering
14:33:26 <amotoki> I agree with the confusion discussed so far. the question now looks like whether we need to change the current interpretation in addtion to a new proposed field.
14:33:59 <amotoki> Changing the existing behavior will introduce another confusion, so we might be conservative.
14:34:01 <slaweq> amotoki: I think so as currently "remote_ip_prefix" is in fact used as local IP (belongs to OpenStack vm)
14:34:36 <amotoki> slaweq: I think so too
14:34:37 <rafaelweingartne> I would say so, otherwise, confusions would still be created by using something called "remote_IP" to indicate an IP inside OpenStack where these rules are applied
14:36:55 <amotoki> I now wonder how we can achieve this without breaking existing users....
14:38:27 <njohnston> one way would be to create new names - let's say we use 'cloud_ip_prefix' and 'external_ip_prefix' - and deprecate remote_ip_prefix in the docs, while mapping it to cloud_ip_prefix internally
14:39:11 <slaweq> njohnston: good point, maybe it could also be "internal_ip_prefix" and "external_ip_prefix"
14:39:20 <slaweq> but will that be clear for people?
14:39:38 <rafaelweingartne> that would work, but the pseudo bug would still exist, the remote_ip_prefix would need proper documentation and deprecation
14:39:53 <amotoki> yeah, it will be the best way when we have a wrong/confusing naming.
14:39:54 <rafaelweingartne> cant this create more confusions? having external and remote attributes?
14:40:07 <rafaelweingartne> at the same time
14:40:43 <amotoki> rafaelweingartne: doesn't the RFE propose to change the meaning of 'remote'? am I wrong?
14:40:57 <rafaelweingartne> yes
14:41:35 <rafaelweingartne> I mean, it depends on the perspective. The remote would start to fulfill its claim (being the remote IP)
14:41:50 <rafaelweingartne> instead of the internal/loca/vmIP
14:42:04 <amotoki> what we are wonder is how we won't break existing API users when we change the API behavior. Of course we need to clarify what the new terms mean.
14:42:16 <rafaelweingartne> yes, and that is a good point
14:42:27 <rafaelweingartne> How was the change introduced via [1] handled? That also changed the way API was being used and broke compatibility.
14:42:56 <amotoki> I believe that was not a good example.
14:43:10 <rafaelweingartne> hmm
14:43:11 <rafaelweingartne> I see
14:44:12 <njohnston> I think that remote_ip is tainted by the fact that we have to maintain API compatibility with an implementation that has the opposite meaning of the name.  I think replacing and deprecating it - and making a special note in the API ref - is the best path.
14:44:51 <njohnston> Perhaps 'cloud_ip" for the local and "peer_ip" for the remote would work, as they don't frame the sides in terms of an assumed reference point of the user.
14:44:51 <rafaelweingartne> ok, so I will need to add that into my proposal then, right?
14:45:11 <amotoki> njohnston: agree (but I need double check after the meeting)
14:46:04 <slaweq> njohnston: amotoki I like that proposal
14:46:19 <slaweq> that way we will not break any existing users and their applications
14:46:30 <slaweq> and we will improve our API
14:46:33 <amotoki> rafaelweingartne: I think so. we are reverting the meaning of the field twice. It should be avoided in general, so it is better to be covered in the context.
14:46:49 <rafaelweingartne> ok, I will do that then
14:48:17 <slaweq> so I'm ok to approve this rfe with the note that it should add new attributes, and just deprecate the existing one but not remove or change it's behaviour
14:48:21 <mlavalle> seems reasonable to me
14:48:33 <slaweq> are others ok too?
14:48:36 <ralonsoh> +1
14:48:42 <amotoki> sounds good
14:48:53 <njohnston> +1
14:48:56 <rafaelweingartne> Thank you guys
14:49:18 <slaweq> yamamoto: any thoughts?
14:49:29 <amotoki> rafaelweingartne: thanks for taking care of this complicated one.
14:49:30 <yamamoto> i'd like to abstain as i'm just confused :-)
14:50:07 <slaweq> ok
14:50:30 <slaweq> but as we have most people +1 about it, I will mark it as approved
14:51:02 <slaweq> rafaelweingartne: please propose spec with detailed description of the current status and proposed changes (api especially)
14:51:14 <rafaelweingartne> sure, I will do
14:51:18 <slaweq> thx a lot
14:51:26 <slaweq> and thx for the new proposal :)
14:51:32 <slaweq> ok, lets move on
14:51:38 <slaweq> there is third one for today
14:51:42 <slaweq> https://bugs.launchpad.net/neutron/+bug/1888487
14:51:43 <openstack> Launchpad bug 1888487 in neutron "Change the default value of "propagate_uplink_status" to True" [Wishlist,New] - Assigned to Rodolfo Alonso (rodolfo-alonso-hernandez)
14:51:45 <slaweq> from ralonsoh
14:51:49 <ralonsoh> very quick
14:52:10 <ralonsoh> this extension, "propagate_uplink_status", allows to propagate the PF status to the VF, in SRIOV
14:52:16 <ralonsoh> (that's only for SRIOV)
14:52:30 <ralonsoh> by default, the VF status will be set to enabled/disables
14:52:48 <ralonsoh> with this extension and the paremeter to True, the link status will be auto
14:53:00 <ralonsoh> my proposal: when this extension is enabled
14:53:17 <slaweq> "propagate_uplink_status" is port's parameter in API, right?
14:53:17 <ralonsoh> set "propagate_uplink_status" to True by default --> set link status to auto
14:53:20 <ralonsoh> yes
14:53:28 <ralonsoh> by default now is False
14:53:34 <ralonsoh> I want to change that
14:53:43 <slaweq> and You want to basically change default value of this attribute, right?
14:53:47 <ralonsoh> yes
14:54:07 <ralonsoh> I know could sound trivial, but is a change in a default parameter
14:54:43 <ralonsoh> reason: customers enabling this extension want to have VF link status to auto
14:55:13 <ralonsoh> with this parameter set to True, they don't need to change nothing in the port creation
14:55:25 <ralonsoh> *change anything
14:56:00 <njohnston> I am +1 on this because I think it is very unusual for a user to actually desire the behavior that we currently have as the default
14:56:44 <slaweq> and by default this extension isn't enabled so users who don't want to use "auto" don't have this extension loaded probably
14:56:55 <ralonsoh> exactly
14:57:09 <slaweq> I'm ok to change that
14:57:21 <slaweq> of course with "big" release note about the change :)
14:57:27 <ralonsoh> sure!!
14:57:54 <mlavalle> +1
14:57:56 <amotoki> sounds reasonable to me too so far.
14:58:08 <ralonsoh> thank you all
14:58:17 <yamamoto> +1
14:58:33 <amotoki> do we introduce a new extension?
14:58:33 <slaweq> ok, sounds like we can approve it :)
14:59:12 <slaweq> amotoki: I'm not sure if we need that in this case TBH
14:59:32 <slaweq> amotoki: but we can discuss it in the review of the patch probably
14:59:38 <slaweq> ok for You?
14:59:49 <amotoki> slaweq: I am okay
14:59:57 <slaweq> thx
14:59:59 <amotoki> this is really a gray zone
15:00:03 <slaweq> we are running out of time now
15:00:08 <slaweq> thx for attending the meeting
15:00:10 <ralonsoh> bye
15:00:14 <njohnston> very productive meeting!  thanks all
15:00:16 <slaweq> have a great weekend
15:00:18 <yamamoto> good night
15:00:18 <slaweq> o/
15:00:20 <amotoki> o
15:00:21 <amotoki> o/
15:00:22 <slaweq> #endmeeting