Re: [ovs-dev] [PATCH v6] ovn: Allow for automatic dynamic updates of IPAM
On 08/02/2018 08:48 AM, Jakub Sitnicki wrote:
Hi Mark,
On Thu, 2 Aug 2018 08:18:12 -0400
Mark Michelson wrote:
OVN offers a method of IP address management that allows for an IPv4
subnet or IPv6 prefix to be specified on a logical switch. Then by
specifying a switch port's address as "dynamic" or "
dynamic", OVN will automatically assign addresses to the switch port.
While this works great for initial assignment of addresses, addresses
do not automatically adjust when changes are made to the switch's
configuration. For instance:
* If the subnet, ipv6_prefix, or exclude_ips for a logical switch
changes, the affected switch ports are not updated.
* If a switch port with a static IP address is added to the switch,
and that address conflicts with a dynamically assigned IP address, the
dynamic address is not updated.
* If a MAC address switched from being statically assigned to
dynamically assigned, the MAC address would not be updated.
* If a statically assigned MAC address changed, then the IPv6 address
would not be updated.
This patch solves all of the above issues by changing the algorithm
for IPAM assignment. There are essentially three steps.
1) While joining logical ports, all statically-assigned addresses
(i.e. any ports without "dynamic" addresses) have their addresses
registered to IPAM. This gives them top priority.
2) All logical ports with dynamic addresses are inspected. Any changes
that must be made to the addresses are collected to be made later. Any
addresses that do not require change are registered to IPAM. This
allows for previously assigned dynamic addresses to be kept.
3) All gathered changes are enacted.
The change contains new tests that ensure that dynamic addresses are
updated when appropriate.
This patch also alters some existing IPAM tests. Those tests assumed
that dynamic addresses would not be updated automatically, so those
tests either had to be altered or removed.
Signed-off-by: Mark Michelson
Acked-by: Jakub Sitnicki
---
v5->v6:
* Rebased
v4->v5:
Cleanups suggested by Jakub Sitnicki + rebase
* Add some convenience pointers for shortened code.
* Separate checking of updates of dynamic addresses and registration
of unchanged addresses.
* Use OVS_NOT_REACHED() instead of ovs_assert(0)
v3->v4:
Print warning when multiple dynamic addresses are configured on a
switch port. Ensure that dynamic addresses beyond the first on a
switch port are ignored. Found by Ben Pfaff.
v2->v3:
Fixed a checkpatch problem (line too long)
v1->v2:
Rebased
---
Recent fix pushed to master, 4d0214a365ae ("ovn: Fix typos in "ovn --
Address Set generation... test."), will cause the following test to
fail with this patch applied:
2578: ovn -- Address Set generation from Port Groups (dynamic addressing)
To fix the test you need to revert the mentioned commit and squash the
diff with your changes.
Thanks,
Jakub
Fixed in v7. Thanks!
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v6] ovn: Allow for automatic dynamic updates of IPAM
Hi Mark,
On Thu, 2 Aug 2018 08:18:12 -0400
Mark Michelson wrote:
> OVN offers a method of IP address management that allows for an IPv4
> subnet or IPv6 prefix to be specified on a logical switch. Then by
> specifying a switch port's address as "dynamic" or "
> dynamic", OVN will automatically assign addresses to the switch port.
>
> While this works great for initial assignment of addresses, addresses
> do not automatically adjust when changes are made to the switch's
> configuration. For instance:
> * If the subnet, ipv6_prefix, or exclude_ips for a logical switch
> changes, the affected switch ports are not updated.
> * If a switch port with a static IP address is added to the switch,
> and that address conflicts with a dynamically assigned IP address, the
> dynamic address is not updated.
> * If a MAC address switched from being statically assigned to
> dynamically assigned, the MAC address would not be updated.
> * If a statically assigned MAC address changed, then the IPv6 address
> would not be updated.
>
> This patch solves all of the above issues by changing the algorithm
> for IPAM assignment. There are essentially three steps.
> 1) While joining logical ports, all statically-assigned addresses
> (i.e. any ports without "dynamic" addresses) have their addresses
> registered to IPAM. This gives them top priority.
> 2) All logical ports with dynamic addresses are inspected. Any changes
> that must be made to the addresses are collected to be made later. Any
> addresses that do not require change are registered to IPAM. This
> allows for previously assigned dynamic addresses to be kept.
> 3) All gathered changes are enacted.
>
> The change contains new tests that ensure that dynamic addresses are
> updated when appropriate.
>
> This patch also alters some existing IPAM tests. Those tests assumed
> that dynamic addresses would not be updated automatically, so those
> tests either had to be altered or removed.
>
> Signed-off-by: Mark Michelson
> Acked-by: Jakub Sitnicki
> ---
> v5->v6:
> * Rebased
>
> v4->v5:
> Cleanups suggested by Jakub Sitnicki + rebase
> * Add some convenience pointers for shortened code.
> * Separate checking of updates of dynamic addresses and registration
>of unchanged addresses.
> * Use OVS_NOT_REACHED() instead of ovs_assert(0)
>
> v3->v4:
> Print warning when multiple dynamic addresses are configured on a
> switch port. Ensure that dynamic addresses beyond the first on a
> switch port are ignored. Found by Ben Pfaff.
>
> v2->v3:
> Fixed a checkpatch problem (line too long)
>
> v1->v2:
> Rebased
> ---
Recent fix pushed to master, 4d0214a365ae ("ovn: Fix typos in "ovn --
Address Set generation... test."), will cause the following test to
fail with this patch applied:
2578: ovn -- Address Set generation from Port Groups (dynamic addressing)
To fix the test you need to revert the mentioned commit and squash the
diff with your changes.
Thanks,
Jakub
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v6] ovn: Allow for automatic dynamic updates of IPAM
OVN offers a method of IP address management that allows for an IPv4 subnet or
IPv6 prefix to be specified on a logical switch. Then by specifying a
switch port's address as "dynamic" or " dynamic", OVN will
automatically assign addresses to the switch port.
While this works great for initial assignment of addresses, addresses do
not automatically adjust when changes are made to the switch's
configuration. For instance:
* If the subnet, ipv6_prefix, or exclude_ips for a logical switch
changes, the affected switch ports are not updated.
* If a switch port with a static IP address is added to the switch, and
that address conflicts with a dynamically assigned IP address, the
dynamic address is not updated.
* If a MAC address switched from being statically assigned to
dynamically assigned, the MAC address would not be updated.
* If a statically assigned MAC address changed, then the IPv6 address
would not be updated.
This patch solves all of the above issues by changing the algorithm for
IPAM assignment. There are essentially three steps.
1) While joining logical ports, all statically-assigned addresses (i.e.
any ports without "dynamic" addresses) have their addresses registered
to IPAM. This gives them top priority.
2) All logical ports with dynamic addresses are inspected. Any changes
that must be made to the addresses are collected to be made later. Any
addresses that do not require change are registered to IPAM. This allows
for previously assigned dynamic addresses to be kept.
3) All gathered changes are enacted.
The change contains new tests that ensure that dynamic addresses are
updated when appropriate.
This patch also alters some existing IPAM tests. Those tests assumed
that dynamic addresses would not be updated automatically, so those
tests either had to be altered or removed.
Signed-off-by: Mark Michelson
Acked-by: Jakub Sitnicki
---
v5->v6:
* Rebased
v4->v5:
Cleanups suggested by Jakub Sitnicki + rebase
* Add some convenience pointers for shortened code.
* Separate checking of updates of dynamic addresses and registration
of unchanged addresses.
* Use OVS_NOT_REACHED() instead of ovs_assert(0)
v3->v4:
Print warning when multiple dynamic addresses are configured on a
switch port. Ensure that dynamic addresses beyond the first on a switch
port are ignored. Found by Ben Pfaff.
v2->v3:
Fixed a checkpatch problem (line too long)
v1->v2:
Rebased
---
ovn/northd/ovn-northd.c | 398 +++-
tests/ovn.at| 97 +---
2 files changed, 362 insertions(+), 133 deletions(-)
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 35baabcad..067d52d82 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -986,9 +986,6 @@ ipam_add_port_addresses(struct ovn_datapath *od, struct
ovn_port *op)
for (size_t i = 0; i < op->nbsp->n_addresses; i++) {
ipam_insert_lsp_addresses(od, op, op->nbsp->addresses[i]);
}
-if (op->nbsp->dynamic_addresses) {
-ipam_insert_lsp_addresses(od, op, op->nbsp->dynamic_addresses);
-}
} else if (op->nbrp) {
struct lport_addresses lrp_networks;
if (!extract_lrp_networks(op->nbrp, _networks)) {
@@ -1061,64 +1058,263 @@ ipam_get_unused_ip(struct ovn_datapath *od)
return od->ipam_info.start_ipv4 + new_ip_index;
}
-static bool
-ipam_allocate_addresses(struct ovn_datapath *od, struct ovn_port *op,
-const char *addrspec)
+enum dynamic_update_type {
+NONE,/* No change to the address */
+REMOVE, /* Address is no longer dynamic */
+STATIC, /* Use static address (MAC only) */
+DYNAMIC, /* Assign a new dynamic address */
+};
+
+struct dynamic_address_update {
+struct ovs_list node; /* In build_ipam()'s list of updates. */
+
+struct ovn_port *op;
+
+struct lport_addresses current_addresses;
+struct eth_addr static_mac;
+enum dynamic_update_type mac;
+enum dynamic_update_type ipv4;
+enum dynamic_update_type ipv6;
+};
+
+static enum dynamic_update_type
+dynamic_mac_changed(const char *lsp_addresses,
+struct dynamic_address_update *update)
+{
+ struct eth_addr ea;
+
+ if (ovs_scan(lsp_addresses, ETH_ADDR_SCAN_FMT, ETH_ADDR_SCAN_ARGS(ea))) {
+ if (eth_addr_equals(ea, update->current_addresses.ea)) {
+ return NONE;
+ } else {
+ /* MAC is still static, but it has changed */
+ update->static_mac = ea;
+ return STATIC;
+ }
+ }
+
+ uint64_t mac64 = eth_addr_to_uint64(update->current_addresses.ea);
+ if ((mac64 ^ MAC_ADDR_PREFIX) >> 24) {
+ return DYNAMIC;
+ } else {
+ return NONE;
+ }
+}
+
+static enum dynamic_update_type
+dynamic_ip4_changed(struct dynamic_address_update *update)
+{
+const struct ipam_info *ipam = >op->od->ipam_info;
+const struct lport_addresses *cur_addresses = >current_addresses;
+bool dynamic_ip4 =
