16:01:57 <Sukhdev> #startmeeting networking_ml2
16:01:57 <openstack> Meeting started Wed Jul  6 16:01:57 2016 UTC and is due to finish in 60 minutes.  The chair is Sukhdev. Information about MeetBot at http://wiki.debian.org/MeetBot.
16:01:58 <openstack> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
16:02:02 <openstack> The meeting name has been set to 'networking_ml2'
16:02:12 <Sukhdev> Good morning and welcome to ML2 meeting
16:02:19 <Sukhdev> #topic: Agenda
16:02:36 <Sukhdev> #link: https://wiki.openstack.org/wiki/Meetings/ML2#Meeting_July_6.2C_2016
16:03:06 <Sukhdev> If anybody wants to add to the agenda, speak up as we go along and we will adjust it accordingly
16:03:15 <Sukhdev> #topic: Announcements
16:03:19 <Sukhdev> I have two -
16:03:37 <Sukhdev> N2 is July 12-14
16:03:44 <Sukhdev> i.e. next week
16:03:53 <Sukhdev> time is flying by -
16:04:18 <Sukhdev> Neutron Mid-Cycle is in the middle of August in Cork, Ireland
16:04:43 <Sukhdev> This is the first international mid-cycle
16:05:07 <Sukhdev> If any of you plan on attending - please make a note of the date
16:05:53 <Sukhdev> Anybody has any other announcement?
16:06:13 <rkukura> Sukhdev: Is there a specific agenda yet for this mid-cycle, or is that TBD?
16:06:54 <Sukhdev> rkukura : good question - I did not look for etherpad - I am sure there is one in the works
16:07:43 <Sukhdev> rkukura : BTW, IBM is hosting
16:08:06 <rkukura> #link https://etherpad.openstack.org/p/newton-neutron-midcycle
16:09:01 <Sukhdev> rkukura : thanks for the link
16:09:41 <rkukura> I like that travel, hotels, activities, socializing, and pubs are all listed before topics ;)
16:10:06 <Sukhdev> Good marketing !!
16:10:53 <Sukhdev> So, anybody want to represent ML2 there?
16:11:35 <andreas_s_> I won't be able to attend..
16:11:44 <Sukhdev> Armando asked me to go - but, it is too far....
16:12:42 <Sukhdev> anything else on the announcements?
16:13:06 <Sukhdev> moving along....
16:13:34 <Sukhdev> #topic: Unify ML2 Portbinding and DVR portbindings
16:13:53 <Sukhdev> andreas_s_ : you are up
16:13:57 <andreas_s_> #link https://review.openstack.org/309416
16:14:05 <andreas_s_> Armando commented on the spec
16:14:14 <andreas_s_> I would like to discuss his 3 major points...
16:14:33 <andreas_s_> #1
16:14:41 <andreas_s_> #1 Are we sure that vnic_type and profile are always port specific (and not binding specific)?
16:14:53 <rkukura> I’m not so sure
16:15:14 <Sukhdev> profile is kind of more generic
16:15:25 <Sukhdev> vnic_type is port specific
16:15:47 <andreas_s_> the plan is to split the attributes up into binidng specific and port specific attributes (which would result in 2 tables)
16:15:57 <rkukura> With SR-IOV, I think binding:profile is set by Nova to details on the SR-IOV allocation for that VM on that host
16:16:29 <andreas_s_> rkukura, right, that's the only use case I found as well
16:16:53 <Sukhdev> andreas_s_ : ha ha - let me tell you another one :-)
16:17:12 <andreas_s_> Sukhdev, please :)
16:17:16 <Sukhdev> Ironic sets binding:profile to specify the physical port connectivity
16:17:35 <Sukhdev> PCI driver uses this to specify physical connectivity as well
16:17:37 <rkukura> Sukhdev: Can baremetal hosts migrate?
16:17:53 <Sukhdev> rkukura : not really -
16:18:11 <Sukhdev> I should clarify - today no - future - may be
16:19:05 <andreas_s_> so the quesiton is, if splitting up those attributes into separate tables makes sense
16:19:06 <rkukura> I can imagine other uses for distributed baremetal ports
16:19:36 <andreas_s_> or, and that's #2 if merging the portbindngs table into the dvr portbindingstable wouldn't be the simpler approach
16:20:35 <Sukhdev> I think #2 may be OK - #1 might break too many things
16:20:54 <Sukhdev> we just identified three user of binding:profile - I am sure there may be more as well
16:20:59 <rkukura> andreas_s_: I kind of thought merging these tables was the goal, but with any adjustements needed - some stuff is port-specific, and other stuff is binding-specific, so we need to make sure its all correct
16:22:06 <Sukhdev> I assumed #2 was merging tables - I think that is good
16:22:36 <andreas_s_> Sukhdev, got you. But still the question is, if there are use cases where a each binding might use a different profile
16:22:42 <Sukhdev> but, splitting binding profile - we have to make sure, if we mess with it, to keep it backward compatible - for all use cases known and unknown
16:23:00 <andreas_s_> Sukhdev, I have parts of that refactoring already running
16:23:06 <andreas_s_> Sukhdev, but the DVR pieces are still missing
16:23:59 <andreas_s_> but maybe the other way round is the more easy appraoch - first merging and then splitting into port and binding specifics?
16:24:23 <Sukhdev> I like the latter approach, andreas_s_
16:24:42 <andreas_s_> Let me ask you another question
16:24:53 <rkukura> makes sense to me to first cleanup/refactor without changing APIs at all, then do that afterwards as needed
16:24:54 <andreas_s_> why have those separate tables been introduced at all?
16:25:31 <rkukura> andreas_s_: Do you mean separate normal and DVR tables?
16:25:39 <andreas_s_> rkukura, yup
16:26:14 <andreas_s_> rkukura, cause the reason for this might be an indicator for the problems that will occur ;)
16:26:32 <Sukhdev> rkukura : did not give too many -2 on some of the DVR patches - that is the reason - just kidding :-):-)
16:27:33 <rkukura> As I recall, the DVR implementers chose to add new tables in  order to reduce risk of breaking non-DVR behavior
16:27:47 <Sukhdev> andreas_s_ : I think for historical reasons and keeping the backward compatibility
16:28:02 <Sukhdev> rkukura :+1
16:28:49 <rkukura> I certainly raised the issue, but eventually conceded that this was a reasonable approach to get the DVR code merged, which impacted a lot more than just ML2’s port binding.
16:29:25 <andreas_s_> rkukura, Sukhdev ok. got it - so there was no technical problem
16:29:41 <andreas_s_> more a decission to not break existng things and speed up development
16:30:14 <andreas_s_> so I will lay down the work on the current patch and start a new one merging together dvr and normal port table. Do you agree?
16:30:22 <rkukura> Not that I’m aware of - we identified the cleanup as a folow-on effort, I started it, and andreas_s_ has picked it up
16:30:46 <rkukura> andreas_s_: How is that different than the current patch?
16:31:10 <andreas_s_> rkukura, the current patch first splits up port and binding specific attributes
16:31:26 <rkukura> At least my original patch was really just fixing the schema without changing the
16:31:30 <rkukura> API
16:31:39 <andreas_s_> rkukura, but there's still separate logic for dvr and normal
16:32:34 <andreas_s_> rkukura, that's right. However the API needs to change as now new object are flying around
16:32:56 <andreas_s_> my impression is, that the codes gets a little more complex than it was before
16:33:26 <rkukura> My original parxh was not trying to address migration, or even generalize distributed ports, just normalize the schema
16:33:35 <rkukura> s/parxh/patch/
16:34:01 <andreas_s_> rkukura, ack
16:34:35 <rkukura> the idea was to fix the schema first, then try to simpify the code to eliminate special casing DVR, then deal with generalizing distributed ports, which would require some API adjustements
16:35:52 <rkukura> I’d recommend going back to this step-by-step approach, and hopefully get at least the schema cleanup into Newton
16:36:36 <andreas_s_> ok
16:36:59 <Sukhdev> sounds like a sound plan
16:37:12 <andreas_s_> so the plan is still to make host,vif_type and vif_details binding specific, while vnic_type and profile stay port specific
16:38:18 <andreas_s_> rkukura, could you please comment on the spec, mentioning that this is the right way of doing it?
16:38:33 <rkukura> andreas_s_: I think we need to consider making binding:profile host-specific
16:38:46 <rkukura> andreas_s_: I will
16:39:10 <andreas_s_> that means that there's only the vnic_type, that is valid for all bindings
16:40:18 <andreas_s_> if that's the case, I really wonder if it wouldn't be more efficient to take the cost of data duplication there and start with merging dvr and normal binding table
16:40:29 <Sukhdev> rkukura : can you clarify what do you mean by host-specific?
16:40:32 <andreas_s_> right from the beginning
16:40:40 <Sukhdev> will it mean the physical connectivity of host as well?
16:41:42 <andreas_s_> I mean the overall goal change a bit - now it's to make multiple portbindings generic - meaning making a normal port a distributed port with just one binding
16:41:50 <rkukura> I don’t yet have a position on whether binding:vnic_type should be per-port or per-binding
16:43:14 <rkukura> Sukhdev: By host-specific, I mean per-binding if there are multiple bindings for the same port
16:43:56 <rkukura> The current API has no way to address individual bindings
16:44:01 <Sukhdev> just to make sure - Ironic uses binding:profile to list the physical connectivity of BM host - and there could be multiple hosts represented by one neutron port
16:44:20 <Sukhdev> i.e. one neutron port == multiple physical ports
16:44:33 <Sukhdev> all represented by a single binding:profile
16:45:05 <rkukura> Sukhdev: I think you are talking about the situation after we’ve introduced some general notion of distributed binding, but we don’t have that yet
16:45:53 <Sukhdev> so, here is the specific case I am describing:
16:46:18 <rkukura> My approach was to cleanup the schema first to make normal binding a special case of distributed binding, but not to try to move attriibutes from per-port to per-binding yet, beyond what was done for DVR
16:46:45 <Sukhdev> In an MLAG deployment scenario - if a host is connected to two TORs then binding profiles carries a list of physical ports that map to single neutron port (and network)
16:47:53 <Sukhdev> I think as long as you are merging the tables and schema, we are good
16:48:11 <andreas_s_> rkukura, but with introducing the binding_result table you already made the decision to split up binding and port specific attributes, didn't you?
16:48:32 <andreas_s_> rkukura, when we just merge the tables, we keep everything as is today (hopefully :)
16:49:12 <rkukura> andreas_s_: Just what was needed to preserve DVR functionality- the intent was to generilize later, and move attributes around as needed
16:49:27 <rkukura> I guess I’m not really clear on what is meant by “merge the tables”
16:50:09 <andreas_s_> rkukura, it's basically moving all the data form current port_bindings table into dvr_port_bindings table
16:50:10 <rkukura> RIght now the API has no way to set or retrieve more than one value for each of the port attributes
16:50:25 <andreas_s_> rkukura, right
16:50:55 <rkukura> So I don’t see how we could move binding:vnic_type or binding:profile to be binding-specific in the current API
16:51:07 <rkukura> So my plan was to keep them per-port for now
16:51:20 <andreas_s_> rkukura, I think the discussion is not about the ResT API
16:51:29 <andreas_s_> rkukura, it's about the database schema
16:51:53 <rkukura> but the binding:vif_type and binding:vif_details obtained by get_device_details RPC are per-binding so that DVR can work with the L2 agent
16:52:24 <andreas_s_> right and today dvr has mutliple entries in the dvr_portbindings table
16:52:45 <rkukura> right, the host_id is a primary ke
16:52:46 <rkukura> key
16:52:53 <andreas_s_> right
16:53:34 <rkukura> but, until the API allows setting these per-host, the binding:vnic_type and binding:profile should not be in a table where the host is part of the primary key
16:53:53 <Sukhdev> rkukura : I think host and port are primary keys
16:54:14 <andreas_s_> rkukura, and that's why you introduced the portbindingresult table, right?
16:54:33 <rkukura> Otherwise, an update to either of these attritues would need to update all rows in the table for that port_id, and a get would need to return one row’s values, assuming the others are all these same
16:55:13 <andreas_s_> exactly - but are we sure that binding profile and vnic type a really specific per host?
16:55:34 <rkukura> yes, the portbindingresult table was intended to be the per-host stuff. I think we are now realizing this will eventually include inputs to binding, not just results, so there may be a better name for this table
16:56:25 <rkukura> I am pretty sure binding:profile would need to be per-host in order for VM migration to work with SR-IOV
16:56:43 <andreas_s_> rkukura, ok, so there's only vnic_type left that is not per host specific
16:57:53 * Sukhdev time check 3 min left
16:58:25 <rkukura> If we are going to modify the API to allow binding:profile to be host-specific, I think we should also look at use other use cases, and whether they might also require binding:vnic_type to be host-specific. But I’m not seeing any good cases for that right now.
16:59:06 <andreas_s_> so my question is: why don't we just store vnic_type port specific as well for the beginning
16:59:13 <rkukura> andreas_s_: I see your point that maybe we long term will not need to have both per-port and per-host tables
16:59:31 <andreas_s_> rkukura, right
16:59:50 <rkukura> is it reasonable for port_update to update all rows for that port whenever vnic_type is modified?
16:59:51 <andreas_s_> Especially after you explained that profile is per-host as well
17:00:16 * Sukhdev time is up
17:00:44 <andreas_s_> Sukhdev, thanks
17:00:46 <Sukhdev> Folks, we need to wrap this up - lets pick this up in the neutron channel
17:00:53 <Sukhdev> Thanks for attending
17:00:57 <rkukura> thanks!
17:00:57 <Sukhdev> bye
17:00:58 <andreas_s_> thanks for your time guys!
17:00:59 <rkukura> bye
17:01:02 <Sukhdev> #endmeeting