On Sun, Jul 03, 2016 at 11:16:05AM -0700, Ben Pfaff wrote: > On Wed, Jun 29, 2016 at 01:53:24PM -0700, Nimay Desai wrote: > > Added an IPv4 and MAC addresses management system to ovn-northd. When a > > logical > > switch's options:subnet field is set, logical ports attached to that switch > > that do not have a MAC/IPv4 address will automatically be allocated a > > globally > > unique MAC address/unused IPv4 address within the provided subnet. This > > can be useful for a user who wants to deploy many VM's or containers with > > networking capabilities, but does not care about the specific MAC/IPv4 > > addresses that are assigned. > > > > Added tests in ovn.at for ipam. > > > > Signed-off-by: Nimay Desai <nimaydes...@gmail.com> > > Thanks for working on this! > > "sparse" reports that MAC_ADDR_PREFIX is "long long" even though it's > suffixed with plain "L". I'd use a "ULL" suffix instead. > > There seems to be multiple instances of open-coded > HMAP_FOR_EACH_WITH_HASH searches for IP and MAC addresses. I recommend > adding helper functions. > > It seems inefficient in ipam_get_unused_mac() and ipam_get_unused_ip() > to start the search from the beginning of the space instead of from just > past the last-assigned address, or in random order. I don't know > whether it matters though. > > Our usual pattern, I think, is that a column is named "options" if the > meaning of its key-value pairs depend on the type of an entity, and > "other_config" if the meaning does not depend on a type. I think that > this "subnet" configuration is independent of type (there is only one > type of Logical_Switch), so I would put it in an other_config column. > > Everything following is about a few things that concern me a little. > None of these are necessarily big problems, but I want to bring them up > so that they can be thought through if necessary. > > Usually we do not design OVSDB schemas so that daemons with different > purposes modify the same column, because it can be a recipe for > confusion. For example, we use separate columns in the Open_vSwitch > schema to report the MAC address for a port and to request that a port > be assigned a specific MAC address, because with a single column it's > difficult to distinguish whether its value is meant for one purpose or > the other. I am not sure that this is exactly the same case, but if we > were going to follow this principle here, too, I would add a new column > to Logical_Switch_Port for northd-assigned addresses. It could be > called "dynamic_addresses", for example. > > I am not sure that I am comfortable with the idea of assigning a dynamic > address automatically when there is nothing in the addresses column. It > seems somewhat surprising. I would consider adding a new "addresses" > syntax to request a dynamic MAC/IP; for example, having it requested by > specifying "dynamic" in addresses. > > I suspect that many deployments would want to enable port security for > dynamic addresses, but I don't see a way to do that with the current > design, except in a race-prone way where the CMS copies "addresses" into > "port_security" once it's populated.
One more thought comes to mind. With this change, I believe that the OVN_Northbound database starts requiring persistence from OVN's point of view, because OVN is putting information into the database that cannot be reconstructed from the CMS. If the database is lost and reconstructed, the dynamic IP bindings will be lost (and likely reused quickly because the algorithm always starts from 1). This is probably worth documenting somehow. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
