[ovs-dev] OVN: add static IP support to IPAM
diff mbox series

Message ID fd35ecd414174c5cb0203a4e469cacdbea7ef823.1544005475.git.lorenzo.bianconi@redhat.com
State Changes Requested
Headers show
Series
  • [ovs-dev] OVN: add static IP support to IPAM
Related show

Commit Message

Lorenzo Bianconi Dec. 5, 2018, 10:34 a.m. UTC
Add the capability to IPAM/MACAM framework to specify a static ip address
and get the L2 one allocated dynamically using the following syntax:

$ovn-nbctl lsp-set-addresses <port> "<IP> dynamic"

The static ip address needs to belong to the subnet configured for the
logical switch

Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
---
 ovn/lib/ovn-util.c        |  8 ++++++++
 ovn/northd/ovn-northd.c   | 37 +++++++++++++++++++++++++++++++++----
 ovn/utilities/ovn-nbctl.c |  5 ++++-
 tests/ovn.at              | 12 ++++++++++++
 4 files changed, 57 insertions(+), 5 deletions(-)

Comments

Numan Siddique Dec. 5, 2018, 1:22 p.m. UTC | #1
On Wed, Dec 5, 2018 at 4:05 PM Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
wrote:

> Add the capability to IPAM/MACAM framework to specify a static ip address
> and get the L2 one allocated dynamically using the following syntax:
>
> $ovn-nbctl lsp-set-addresses <port> "<IP> dynamic"
>

Until now, the usage  of dynamic is
  - "dynamic"  -> Generate both MAC and IP
  - "xx:xx:xx:xx:xx:xx dynamic" -> Generate only IP

Since the addresses column expects MAC address first and then IP does
"dynamic <IP>" makes sense for this
use case ? It's just  a suggestion :).


Thanks
Numan



>
> The static ip address needs to belong to the subnet configured for the
> logical switch
>
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
> ---
>  ovn/lib/ovn-util.c        |  8 ++++++++
>  ovn/northd/ovn-northd.c   | 37 +++++++++++++++++++++++++++++++++----
>  ovn/utilities/ovn-nbctl.c |  5 ++++-
>  tests/ovn.at              | 12 ++++++++++++
>  4 files changed, 57 insertions(+), 5 deletions(-)
>
> diff --git a/ovn/lib/ovn-util.c b/ovn/lib/ovn-util.c
> index e9464e926..3bd956ab0 100644
> --- a/ovn/lib/ovn-util.c
> +++ b/ovn/lib/ovn-util.c
> @@ -72,13 +72,21 @@ add_ipv6_netaddr(struct lport_addresses *laddrs,
> struct in6_addr addr,
>   *    "xx:xx:xx:xx:xx:xx dynamic":
>   *        Use specified MAC address, but allocate an IP address
>   *        dynamically.
> + *
> + *    "x.x.x.x dynamic":
> + *        Use specified IP address, but allocate a MAC address
> + *        dynamically.
>   */
>  bool
>  is_dynamic_lsp_address(const char *address)
>  {
>      struct eth_addr ea;
> +    ovs_be32 ip;
>      int n;
>      return (!strcmp(address, "dynamic")
> +            || (ovs_scan(address, IP_SCAN_FMT" dynamic%n",
> +                         IP_SCAN_ARGS(&ip), &n)
> +                         && address[n] == '\0')
>              || (ovs_scan(address, ETH_ADDR_SCAN_FMT" dynamic%n",
>                           ETH_ADDR_SCAN_ARGS(ea), &n) && address[n] ==
> '\0'));
>  }
> diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
> index 2de9fb38d..efb017b17 100644
> --- a/ovn/northd/ovn-northd.c
> +++ b/ovn/northd/ovn-northd.c
> @@ -1104,6 +1104,7 @@ struct dynamic_address_update {
>
>      struct lport_addresses current_addresses;
>      struct eth_addr static_mac;
> +    ovs_be32 static_ip;
>      enum dynamic_update_type mac;
>      enum dynamic_update_type ipv4;
>      enum dynamic_update_type ipv6;
> @@ -1142,7 +1143,8 @@ dynamic_mac_changed(const char *lsp_addresses,
>  }
>
>  static enum dynamic_update_type
> -dynamic_ip4_changed(struct dynamic_address_update *update)
> +dynamic_ip4_changed(const char *lsp_addrs,
> +                    struct dynamic_address_update *update)
>  {
>      const struct ipam_info *ipam = &update->op->od->ipam_info;
>      const struct lport_addresses *cur_addresses =
> &update->current_addresses;
> @@ -1175,6 +1177,25 @@ dynamic_ip4_changed(struct dynamic_address_update
> *update)
>           */
>          return DYNAMIC;
>      } else {
> +        ovs_be32 new_ip;
> +        int n = 0;
> +
> +        if (ovs_scan(lsp_addrs, IP_SCAN_FMT" dynamic%n",
> +                     IP_SCAN_ARGS(&new_ip), &n)
> +            && lsp_addrs[n] == '\0') {
> +
> +            index = ntohl(new_ip) - ipam->start_ipv4;
> +            if (ntohl(new_ip) < ipam->start_ipv4 ||
> +                index > ipam->total_ipv4s ||
> +                bitmap_is_set(ipam->allocated_ipv4s, index)) {
> +                /* new static ip is not valid */
> +                return DYNAMIC;
> +            } else if (cur_addresses->ipv4_addrs[0].addr != new_ip) {
> +                update->ipv4 = STATIC;
> +                update->static_ip = new_ip;
> +                return STATIC;
> +            }
> +        }
>          return NONE;
>      }
>  }
> @@ -1226,7 +1247,7 @@ dynamic_addresses_check_for_updates(const char
> *lsp_addrs,
>                                      struct dynamic_address_update *update)
>  {
>      update->mac = dynamic_mac_changed(lsp_addrs, update);
> -    update->ipv4 = dynamic_ip4_changed(update);
> +    update->ipv4 = dynamic_ip4_changed(lsp_addrs, update);
>      update->ipv6 = dynamic_ip6_changed(update);
>      if (update->mac == NONE &&
>          update->ipv4 == NONE &&
> @@ -1269,6 +1290,7 @@ set_dynamic_updates(const char *addrspec,
>                      struct dynamic_address_update *update)
>  {
>      struct eth_addr mac;
> +    ovs_be32 ip;
>      int n = 0;
>      if (ovs_scan(addrspec, ETH_ADDR_SCAN_FMT" dynamic%n",
>                   ETH_ADDR_SCAN_ARGS(mac), &n)
> @@ -1278,7 +1300,13 @@ set_dynamic_updates(const char *addrspec,
>      } else {
>          update->mac = DYNAMIC;
>      }
> -    if (update->op->od->ipam_info.allocated_ipv4s) {
> +
> +    if (ovs_scan(addrspec, IP_SCAN_FMT" dynamic%n",
> +                 IP_SCAN_ARGS(&ip), &n)
> +        && addrspec[n] == '\0') {
> +        update->ipv4 = STATIC;
> +        update->static_ip = ip;
> +    } else if (update->op->od->ipam_info.allocated_ipv4s) {
>          update->ipv4 = DYNAMIC;
>      } else {
>          update->ipv4 = NONE;
> @@ -1303,7 +1331,8 @@ update_dynamic_addresses(struct
> dynamic_address_update *update)
>      case REMOVE:
>          break;
>      case STATIC:
> -        OVS_NOT_REACHED();
> +        ip4 = update->static_ip;
> +        break;
>      case DYNAMIC:
>          ip4 = htonl(ipam_get_unused_ip(update->od));
>      }
> diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
> index 9d1b22089..f00d2291b 100644
> --- a/ovn/utilities/ovn-nbctl.c
> +++ b/ovn/utilities/ovn-nbctl.c
> @@ -1529,11 +1529,14 @@ nbctl_lsp_set_addresses(struct ctl_context *ctx)
>      int i;
>      for (i = 2; i < ctx->argc; i++) {
>          struct eth_addr ea;
> +        ovs_be32 ip;
>
>          if (strcmp(ctx->argv[i], "unknown") && strcmp(ctx->argv[i],
> "dynamic")
>              && strcmp(ctx->argv[i], "router")
>              && !ovs_scan(ctx->argv[i], ETH_ADDR_SCAN_FMT,
> -                         ETH_ADDR_SCAN_ARGS(ea))) {
> +                         ETH_ADDR_SCAN_ARGS(ea))
> +            && !ovs_scan(ctx->argv[i], IP_SCAN_FMT,
> +                         IP_SCAN_ARGS(&ip))) {
>              ctl_error(ctx, "%s: Invalid address format. See ovn-nb(5). "
>                        "Hint: An Ethernet address must be "
>                        "listed before an IP address, together as a single "
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 2db3f675a..8fd74a22e 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -5616,6 +5616,18 @@ AT_CHECK([ovn-nbctl get Logical-Switch-Port p41
> dynamic_addresses], [0],
>           ["f0:00:00:00:10:2b 192.168.1.3"
>  ])
>
> +# Add static ip address
> +ovn-nbctl --wait=sb lsp-set-addresses p41 "192.168.1.100 dynamic"
> +ovn-nbctl list Logical-Switch-Port p41
> +ovn-nbctl --wait=sb lsp-add sw5 p42 -- lsp-set-addresses p42 \
> +"192.168.1.101 dynamic"
> +AT_CHECK([ovn-nbctl get Logical-Switch-Port p41 dynamic_addresses], [0],
> +         ["0a:00:00:a8:01:65 192.168.1.100"
> +])
> +AT_CHECK([ovn-nbctl get Logical-Switch-Port p42 dynamic_addresses], [0],
> +         ["0a:00:00:a8:01:66 192.168.1.101"
> +])
> +
>  # define a mac address prefix
>  ovn-nbctl ls-add sw6
>  ovn-nbctl --wait=hv set NB_Global . options:mac_prefix="00:11:22:33:44:55"
> --
> 2.19.2
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
Lorenzo Bianconi Dec. 5, 2018, 2:08 p.m. UTC | #2
> On Wed, Dec 5, 2018 at 4:05 PM Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
> wrote:
> 
> > Add the capability to IPAM/MACAM framework to specify a static ip address
> > and get the L2 one allocated dynamically using the following syntax:
> >
> > $ovn-nbctl lsp-set-addresses <port> "<IP> dynamic"
> >
> 
> Until now, the usage  of dynamic is
>   - "dynamic"  -> Generate both MAC and IP
>   - "xx:xx:xx:xx:xx:xx dynamic" -> Generate only IP
> 
> Since the addresses column expects MAC address first and then IP does
> "dynamic <IP>" makes sense for this
> use case ? It's just  a suggestion :).

Hi Numan,

thx for the hint, I will fix it in v2.

Regards,
Lorenzo

Patch
diff mbox series

diff --git a/ovn/lib/ovn-util.c b/ovn/lib/ovn-util.c
index e9464e926..3bd956ab0 100644
--- a/ovn/lib/ovn-util.c
+++ b/ovn/lib/ovn-util.c
@@ -72,13 +72,21 @@  add_ipv6_netaddr(struct lport_addresses *laddrs, struct in6_addr addr,
  *    "xx:xx:xx:xx:xx:xx dynamic":
  *        Use specified MAC address, but allocate an IP address
  *        dynamically.
+ *
+ *    "x.x.x.x dynamic":
+ *        Use specified IP address, but allocate a MAC address
+ *        dynamically.
  */
 bool
 is_dynamic_lsp_address(const char *address)
 {
     struct eth_addr ea;
+    ovs_be32 ip;
     int n;
     return (!strcmp(address, "dynamic")
+            || (ovs_scan(address, IP_SCAN_FMT" dynamic%n",
+                         IP_SCAN_ARGS(&ip), &n)
+                         && address[n] == '\0')
             || (ovs_scan(address, ETH_ADDR_SCAN_FMT" dynamic%n",
                          ETH_ADDR_SCAN_ARGS(ea), &n) && address[n] == '\0'));
 }
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 2de9fb38d..efb017b17 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -1104,6 +1104,7 @@  struct dynamic_address_update {
 
     struct lport_addresses current_addresses;
     struct eth_addr static_mac;
+    ovs_be32 static_ip;
     enum dynamic_update_type mac;
     enum dynamic_update_type ipv4;
     enum dynamic_update_type ipv6;
@@ -1142,7 +1143,8 @@  dynamic_mac_changed(const char *lsp_addresses,
 }
 
 static enum dynamic_update_type
-dynamic_ip4_changed(struct dynamic_address_update *update)
+dynamic_ip4_changed(const char *lsp_addrs,
+                    struct dynamic_address_update *update)
 {
     const struct ipam_info *ipam = &update->op->od->ipam_info;
     const struct lport_addresses *cur_addresses = &update->current_addresses;
@@ -1175,6 +1177,25 @@  dynamic_ip4_changed(struct dynamic_address_update *update)
          */
         return DYNAMIC;
     } else {
+        ovs_be32 new_ip;
+        int n = 0;
+
+        if (ovs_scan(lsp_addrs, IP_SCAN_FMT" dynamic%n",
+                     IP_SCAN_ARGS(&new_ip), &n)
+            && lsp_addrs[n] == '\0') {
+
+            index = ntohl(new_ip) - ipam->start_ipv4;
+            if (ntohl(new_ip) < ipam->start_ipv4 ||
+                index > ipam->total_ipv4s ||
+                bitmap_is_set(ipam->allocated_ipv4s, index)) {
+                /* new static ip is not valid */
+                return DYNAMIC;
+            } else if (cur_addresses->ipv4_addrs[0].addr != new_ip) {
+                update->ipv4 = STATIC;
+                update->static_ip = new_ip;
+                return STATIC;
+            }
+        }
         return NONE;
     }
 }
@@ -1226,7 +1247,7 @@  dynamic_addresses_check_for_updates(const char *lsp_addrs,
                                     struct dynamic_address_update *update)
 {
     update->mac = dynamic_mac_changed(lsp_addrs, update);
-    update->ipv4 = dynamic_ip4_changed(update);
+    update->ipv4 = dynamic_ip4_changed(lsp_addrs, update);
     update->ipv6 = dynamic_ip6_changed(update);
     if (update->mac == NONE &&
         update->ipv4 == NONE &&
@@ -1269,6 +1290,7 @@  set_dynamic_updates(const char *addrspec,
                     struct dynamic_address_update *update)
 {
     struct eth_addr mac;
+    ovs_be32 ip;
     int n = 0;
     if (ovs_scan(addrspec, ETH_ADDR_SCAN_FMT" dynamic%n",
                  ETH_ADDR_SCAN_ARGS(mac), &n)
@@ -1278,7 +1300,13 @@  set_dynamic_updates(const char *addrspec,
     } else {
         update->mac = DYNAMIC;
     }
-    if (update->op->od->ipam_info.allocated_ipv4s) {
+
+    if (ovs_scan(addrspec, IP_SCAN_FMT" dynamic%n",
+                 IP_SCAN_ARGS(&ip), &n)
+        && addrspec[n] == '\0') {
+        update->ipv4 = STATIC;
+        update->static_ip = ip;
+    } else if (update->op->od->ipam_info.allocated_ipv4s) {
         update->ipv4 = DYNAMIC;
     } else {
         update->ipv4 = NONE;
@@ -1303,7 +1331,8 @@  update_dynamic_addresses(struct dynamic_address_update *update)
     case REMOVE:
         break;
     case STATIC:
-        OVS_NOT_REACHED();
+        ip4 = update->static_ip;
+        break;
     case DYNAMIC:
         ip4 = htonl(ipam_get_unused_ip(update->od));
     }
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index 9d1b22089..f00d2291b 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -1529,11 +1529,14 @@  nbctl_lsp_set_addresses(struct ctl_context *ctx)
     int i;
     for (i = 2; i < ctx->argc; i++) {
         struct eth_addr ea;
+        ovs_be32 ip;
 
         if (strcmp(ctx->argv[i], "unknown") && strcmp(ctx->argv[i], "dynamic")
             && strcmp(ctx->argv[i], "router")
             && !ovs_scan(ctx->argv[i], ETH_ADDR_SCAN_FMT,
-                         ETH_ADDR_SCAN_ARGS(ea))) {
+                         ETH_ADDR_SCAN_ARGS(ea))
+            && !ovs_scan(ctx->argv[i], IP_SCAN_FMT,
+                         IP_SCAN_ARGS(&ip))) {
             ctl_error(ctx, "%s: Invalid address format. See ovn-nb(5). "
                       "Hint: An Ethernet address must be "
                       "listed before an IP address, together as a single "
diff --git a/tests/ovn.at b/tests/ovn.at
index 2db3f675a..8fd74a22e 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -5616,6 +5616,18 @@  AT_CHECK([ovn-nbctl get Logical-Switch-Port p41 dynamic_addresses], [0],
          ["f0:00:00:00:10:2b 192.168.1.3"
 ])
 
+# Add static ip address
+ovn-nbctl --wait=sb lsp-set-addresses p41 "192.168.1.100 dynamic"
+ovn-nbctl list Logical-Switch-Port p41
+ovn-nbctl --wait=sb lsp-add sw5 p42 -- lsp-set-addresses p42 \
+"192.168.1.101 dynamic"
+AT_CHECK([ovn-nbctl get Logical-Switch-Port p41 dynamic_addresses], [0],
+         ["0a:00:00:a8:01:65 192.168.1.100"
+])
+AT_CHECK([ovn-nbctl get Logical-Switch-Port p42 dynamic_addresses], [0],
+         ["0a:00:00:a8:01:66 192.168.1.101"
+])
+
 # define a mac address prefix
 ovn-nbctl ls-add sw6
 ovn-nbctl --wait=hv set NB_Global . options:mac_prefix="00:11:22:33:44:55"