On Wed, Jul 27, 2016 at 11:28:24AM -0700, Nimay Desai wrote:
> Added an IPv4 and MAC addresses management system to ovn-northd. When a
> logical
> switch's other_config:subnet field is set, logical ports attached to that
> switch that have the keyword "dynamic" in their addresses column will
> automatically be allocated a globally unique MAC address/unused IPv4 address
> within the provided subnet. The allocated address will populate the
> dynamic_addresses column. 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>
The code uses the abbreviations 'ipam' and 'macam' a lot. IPAM is a
fairly common term but I don't know what macam stands for. I think it
would be a good idea to explain both terms in comments.
Here are some suggested improvements. The code improvements are, I
hope, self-explanatory. The changes to the documentation are mainly to
make it clear that ovn-northd does the work. (I find it really
confusing when documentation says that something happens, but not what
does it, because then you have to assume or guess what does it and it's
easy to guess wrong.)
I'm done with review. I'll leave it to Guru to shepherd this in though
since he's been guiding the work.
Acked-by: Ben Pfaff <b...@ovn.org>
--8<--------------------------cut here-------------------------->8--
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 25af707..d5cbd37 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -867,21 +867,9 @@ ipam_allocate_addresses(struct ovn_datapath *od, struct
ovn_port *op,
ipam_insert_ip(od, ip, false);
ipam_insert_mac(&mac, false);
- /* Convert IP to string form. */
- struct ds ip_ds;
- ds_init(&ip_ds);
- ds_put_format(&ip_ds, IP_FMT, IP_ARGS(htonl(ip)));
-
- /* Convert MAC to string form. */
- struct ds mac_ds;
- ds_init(&mac_ds);
- ds_put_format(&mac_ds, ETH_ADDR_FMT, ETH_ADDR_ARGS(mac));
-
- char *new_addr = xasprintf("%s %s", mac_ds.string, ip_ds.string);
- nbrec_logical_switch_port_set_dynamic_addresses(op->nbsp,
- (const char*) new_addr);
- ds_destroy(&ip_ds);
- ds_destroy(&mac_ds);
+ char *new_addr = xasprintf(IP_FMT" "ETH_ADDR_FMT,
+ IP_ARGS(htonl(ip)), ETH_ADDR_ARGS(mac));
+ nbrec_logical_switch_port_set_dynamic_addresses(op->nbsp, new_addr);
free(new_addr);
return true;
diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml
index 249d3c5..85aa2d3 100644
--- a/ovn/ovn-nb.xml
+++ b/ovn/ovn-nb.xml
@@ -125,11 +125,12 @@
</p>
<column name="other_config" key="subnet">
- If set, logical ports that are attached to this switch that have the
- "dynamic" keyword in their addresses column will automatically be
- allocated a globally unique MAC address/unused IPv4 address within the
- provided IPv4 subnet. The allocated address will populate the
- <ref column="dynamic_addresses"/> column.
+ Set this to an IPv4 subnet, e.g. <code>192.168.0.0/24</code>, to enable
+ <code>ovn-northd</code> to automatically assign IP addresses within
+ that subnet. Use the <code>dynamic</code> keyword in the <ref
+ table="Logical_Switch_Port"/> table's <ref table="Logical_Switch_Port"
+ column="addresses"/> column to request dynamic address assignment for a
+ particular port.
</column>
</group>
@@ -439,22 +440,23 @@
<dt><code>dynamic</code></dt>
<dd>
- This indicates that the logical port should be automatically
- assigned a globally unique MAC address and an unused IPv4 address
- within the subnet that this logical port belongs to. The assigned
- addresses will populate the <ref column="dynamic_addresses"/>
- column. For this keyword to work properly, the other_config:subnet
- of the logical switch that this logical port is attached to must be
- set.
+ Use this keyword to make <code>ovn-northd</code> generate a
+ globally unique MAC address and choose an unused IPv4 address with
+ the logical port's subnet and store them in the port's <ref
+ column="dynamic_addresses"/> column. <code>ovn-northd</code> will
+ use the subnet specified in <ref table="Logical_Switch"
+ column="other_config" key="subnet"/> in the port's <ref
+ table="Logical_Switch"/>.
</dd>
</dl>
</column>
<column name="dynamic_addresses">
<p>
- Addresses assigned to the logical port by the IPAM. Addresses will
- be of the same format as those that populate the
- <ref column="addresses"/> column. Note that these addresses are
+ Addresses assigned to the logical port by <code>ovn-northd</code>, if
+ <code>dynamic</code> is specified in <ref column="addresses"/>.
+ Addresses will be of the same format as those that populate the <ref
+ column="addresses"/> column. Note that these addresses are
constructed and managed locally in ovn-northd, so they cannot be
reconstructed in the event that the database is lost.
</p>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
