From patchwork Thu Jul 28 20:29:42 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Gurucharan Shetty X-Patchwork-Id: 653872 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from archives.nicira.com (archives.nicira.com [96.126.127.54]) by ozlabs.org (Postfix) with ESMTP id 3s0k4Q4kDTz9t1P for ; Fri, 29 Jul 2016 06:29:57 +1000 (AEST) Received: from archives.nicira.com (localhost [127.0.0.1]) by archives.nicira.com (Postfix) with ESMTP id 512C2112F6; Thu, 28 Jul 2016 13:29:54 -0700 (PDT) X-Original-To: dev@openvswitch.org Delivered-To: dev@openvswitch.org Received: from mx1e3.cudamail.com (mx1.cudamail.com [69.90.118.67]) by archives.nicira.com (Postfix) with ESMTPS id 612E3112F5 for ; Thu, 28 Jul 2016 13:29:52 -0700 (PDT) Received: from bar5.cudamail.com (localhost [127.0.0.1]) by mx1e3.cudamail.com (Postfix) with ESMTPS id D0BEC420407 for ; Thu, 28 Jul 2016 14:29:51 -0600 (MDT) X-ASG-Debug-ID: 1469737789-09eadd7aea30cf10001-byXFYA Received: from mx3-pf2.cudamail.com ([192.168.14.1]) by bar5.cudamail.com with ESMTP id 49xoH6P8rxNKM8SA (version=TLSv1 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO) for ; Thu, 28 Jul 2016 14:29:49 -0600 (MDT) X-Barracuda-Envelope-From: guru@ovn.org X-Barracuda-RBL-Trusted-Forwarder: 192.168.14.1 Received: from unknown (HELO relay8-d.mail.gandi.net) (217.70.183.201) by mx3-pf2.cudamail.com with ESMTPS (DHE-RSA-AES256-SHA encrypted); 28 Jul 2016 20:29:48 -0000 Received-SPF: pass (mx3-pf2.cudamail.com: SPF record at ovn.org designates 217.70.183.201 as permitted sender) X-Barracuda-Apparent-Source-IP: 217.70.183.201 X-Barracuda-RBL-IP: 217.70.183.201 Received: from mfilter16-d.gandi.net (mfilter16-d.gandi.net [217.70.178.144]) by relay8-d.mail.gandi.net (Postfix) with ESMTP id 2FBC6405A2 for ; Thu, 28 Jul 2016 22:29:46 +0200 (CEST) X-Virus-Scanned: Debian amavisd-new at mfilter16-d.gandi.net Received: from relay8-d.mail.gandi.net ([IPv6:::ffff:217.70.183.201]) by mfilter16-d.gandi.net (mfilter16-d.gandi.net [::ffff:10.0.15.180]) (amavisd-new, port 10024) with ESMTP id kXWklnwysooO for ; Thu, 28 Jul 2016 22:29:44 +0200 (CEST) X-Originating-IP: 209.85.215.42 Received: from mail-lf0-f42.google.com (mail-lf0-f42.google.com [209.85.215.42]) (Authenticated sender: guru@ovn.org) by relay8-d.mail.gandi.net (Postfix) with ESMTPSA id 1153040496 for ; Thu, 28 Jul 2016 22:29:43 +0200 (CEST) Received: by mail-lf0-f42.google.com with SMTP id l69so57514472lfg.1 for ; Thu, 28 Jul 2016 13:29:43 -0700 (PDT) X-Gm-Message-State: AEkoouvLaIsPy/j2q7Lf9tfyLVdjwE//ZegTEq9OuW54Vi4C9E2RhMcF5QdIb4Q1DZkXE5G9K+DQcBriCdgo+Q== X-Received: by 10.25.147.193 with SMTP id v184mr13929109lfd.43.1469737783422; Thu, 28 Jul 2016 13:29:43 -0700 (PDT) MIME-Version: 1.0 Received: by 10.114.182.75 with HTTP; Thu, 28 Jul 2016 13:29:42 -0700 (PDT) In-Reply-To: <20160728183725.GW22847@ovn.org> References: <1469644104-46077-1-git-send-email-nimaydesai1@gmail.com> <20160728183725.GW22847@ovn.org> X-CudaMail-Envelope-Sender: guru@ovn.org From: Guru Shetty Date: Thu, 28 Jul 2016 13:29:42 -0700 X-Gmail-Original-Message-ID: Message-ID: X-CudaMail-Whitelist-To: dev@openvswitch.org X-CudaMail-MID: CM-V2-727045459 X-CudaMail-DTE: 072816 X-CudaMail-Originating-IP: 217.70.183.201 To: Ben Pfaff X-ASG-Orig-Subj: [##CM-V2-727045459##]Re: [ovs-dev] [PATCH v4] ovn-northd, tests: Adding IPAM to ovn-northd. X-Barracuda-Connect: UNKNOWN[192.168.14.1] X-Barracuda-Start-Time: 1469737789 X-Barracuda-Encrypted: DHE-RSA-AES256-SHA X-Barracuda-URL: https://web.cudamail.com:443/cgi-mod/mark.cgi X-ASG-Whitelist: Header =?UTF-8?B?eFwtY3VkYW1haWxcLXdoaXRlbGlzdFwtdG8=?= X-Virus-Scanned: by bsmtpd at cudamail.com X-Barracuda-BRTS-Status: 1 X-Content-Filtered-By: Mailman/MimeDel 2.1.16 Cc: ovs dev , Nimay Desai Subject: Re: [ovs-dev] [PATCH v4] ovn-northd, tests: Adding IPAM to ovn-northd. X-BeenThere: dev@openvswitch.org X-Mailman-Version: 2.1.16 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@openvswitch.org Sender: "dev" On 28 July 2016 at 11:37, Ben Pfaff wrote: > 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 > > 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. > Do you think this incremental is helpful? /* Pipeline stages. */ @@ -879,6 +879,12 @@ static void build_ipam(struct northd_context *ctx, struct hmap *datapaths, struct hmap *ports) { + /* IPAM generally stands for IP address management. In non-virtualized + * world, MAC addresses come with the hardware. But, with virtualized + * workloads, they need to be assigned and managed. This function + * does both IP address management (ipam) and MAC address management + * (macam). */ + if (!ctx->ovnnb_txn) { return; } > > 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 > > --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 @@ >

> > > - 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 > - column. > + Set this to an IPv4 subnet, e.g. 192.168.0.0/24, to > enable > + ovn-northd to automatically assign IP addresses > within > + that subnet. Use the dynamic keyword in the + table="Logical_Switch_Port"/> table's table="Logical_Switch_Port" > + column="addresses"/> column to request dynamic address assignment > for a > + particular port. > > > > @@ -439,22 +440,23 @@ > >
dynamic
>
> - 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 > - 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 ovn-northd 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 + column="dynamic_addresses"/> column. ovn-northd > will > + use the subnet specified in + column="other_config" key="subnet"/> in the port's + table="Logical_Switch"/>. >
> > > > >

> - Addresses assigned to the logical port by the IPAM. Addresses > will > - be of the same format as those that populate the > - column. Note that these addresses are > + Addresses assigned to the logical port by > ovn-northd, if > + dynamic is specified in . > + Addresses will be of the same format as those that populate the > + 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. >

> diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c index 02b26bb..18756e9 100644 --- a/ovn/northd/ovn-northd.c +++ b/ovn/northd/ovn-northd.c @@ -62,8 +62,8 @@ static const char *default_sb_db(void); #define MAC_ADDR_PREFIX 0x0A0000000000ULL #define MAC_ADDR_SPACE 0xffffff -/* MAC address table of "struct eth_addr"s, that holds the MAC addresses - * allocated by the ipam. */ +/* MAC address management (macam) table of "struct eth_addr"s, that holds the + * MAC addresses allocated by the OVN ipam module. */ static struct hmap macam = HMAP_INITIALIZER(&macam); ^L