From patchwork Sun Jul 3 15:35:27 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ryan Moats X-Patchwork-Id: 643756 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 3rjDlD1DFdz9s9n for ; Mon, 4 Jul 2016 01:36:24 +1000 (AEST) Received: from archives.nicira.com (localhost [127.0.0.1]) by archives.nicira.com (Postfix) with ESMTP id 6F20D10B63; Sun, 3 Jul 2016 08:36:23 -0700 (PDT) X-Original-To: dev@openvswitch.org Delivered-To: dev@openvswitch.org Received: from mx3v3.cudamail.com (mx3.cudamail.com [64.34.241.5]) by archives.nicira.com (Postfix) with ESMTPS id A948310B45 for ; Sun, 3 Jul 2016 08:36:21 -0700 (PDT) Received: from bar6.cudamail.com (localhost [127.0.0.1]) by mx3v3.cudamail.com (Postfix) with ESMTPS id 408B916319A for ; Sun, 3 Jul 2016 09:36:21 -0600 (MDT) X-ASG-Debug-ID: 1467560179-0b32373f7d1f21c0001-byXFYA Received: from mx3-pf3.cudamail.com ([192.168.14.3]) by bar6.cudamail.com with ESMTP id Gvab4dymy09rZgCH (version=TLSv1 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO) for ; Sun, 03 Jul 2016 09:36:19 -0600 (MDT) X-Barracuda-Envelope-From: rmoats@oc7146733065.ibm.com X-Barracuda-RBL-Trusted-Forwarder: 192.168.14.3 Received: from unknown (HELO fed1rmfepo103.cox.net) (68.230.241.145) by mx3-pf3.cudamail.com with SMTP; 3 Jul 2016 15:36:18 -0000 Received-SPF: none (mx3-pf3.cudamail.com: domain at oc7146733065.ibm.com does not designate permitted sender hosts) X-Barracuda-Apparent-Source-IP: 68.230.241.145 X-Barracuda-RBL-IP: 68.230.241.145 Received: from fed1rmimpo209.cox.net ([68.230.241.160]) by fed1rmfepo103.cox.net (InterMail vM.8.01.05.28 201-2260-151-171-20160122) with ESMTP id <20160703153617.OYKC16575.fed1rmfepo103.cox.net@fed1rmimpo209.cox.net> for ; Sun, 3 Jul 2016 11:36:17 -0400 Received: from oc7146733065.ibm.com ([68.13.99.247]) by fed1rmimpo209.cox.net with cox id EFcG1t0075LF6cs01FcGg7; Sun, 03 Jul 2016 11:36:17 -0400 X-CT-Class: Clean X-CT-Score: 0.00 X-CT-RefID: str=0001.0A020206.577930F1.004D, ss=1, re=0.000, recu=0.000, reip=0.000, cl=1, cld=1, fgs=0 X-CT-Spam: 0 X-Authority-Analysis: v=2.1 cv=H/J7u7si c=1 sm=1 tr=0 a=Jmqd6mthTashISSy/JkQqg==:117 a=Jmqd6mthTashISSy/JkQqg==:17 a=L9H7d07YOLsA:10 a=9cW_t1CCXrUA:10 a=s5jvgZ67dGcA:10 a=cAmyUtKerLwA:10 a=VnNF1IyMAAAA:8 a=ZkFq_xkifhMn2iljTmMA:9 a=3kUGyA3KYsvLaAC8:21 a=voIwvsas5bLhIPcl:21 a=skCgnbhlp52w9zbo2JeP:22 X-CM-Score: 0.00 Authentication-Results: cox.net; none Received: by oc7146733065.ibm.com (Postfix, from userid 500) id B72561880388; Sun, 3 Jul 2016 10:36:15 -0500 (CDT) X-CudaMail-Envelope-Sender: rmoats@oc7146733065.ibm.com From: Ryan Moats To: dev@openvswitch.org X-CudaMail-MID: CM-V3-702007342 X-CudaMail-DTE: 070316 X-CudaMail-Originating-IP: 68.230.241.145 Date: Sun, 3 Jul 2016 10:35:27 -0500 X-ASG-Orig-Subj: [##CM-V3-702007342##][PATCH v21 2/8] Persist ovn flow tables Message-Id: <1467560133-5700-3-git-send-email-rmoats@us.ibm.com> X-Mailer: git-send-email 1.7.1 In-Reply-To: <1467560133-5700-1-git-send-email-rmoats@us.ibm.com> References: <1467560133-5700-1-git-send-email-rmoats@us.ibm.com> X-GBUdb-Analysis: 0, 68.230.241.145, Ugly c=0.071429 p=0 Source Normal X-MessageSniffer-Rules: 0-0-0-32767-c X-Barracuda-Connect: UNKNOWN[192.168.14.3] X-Barracuda-Start-Time: 1467560179 X-Barracuda-Encrypted: DHE-RSA-AES256-SHA X-Barracuda-URL: https://web.cudamail.com:443/cgi-mod/mark.cgi X-Virus-Scanned: by bsmtpd at cudamail.com X-Barracuda-BRTS-Status: 1 X-Barracuda-Spam-Score: 0.60 X-Barracuda-Spam-Status: No, SCORE=0.60 using global scores of TAG_LEVEL=3.5 QUARANTINE_LEVEL=1000.0 KILL_LEVEL=4.0 tests=BSF_SC5_MJ1963, RDNS_NONE X-Barracuda-Spam-Report: Code version 3.2, rules version 3.2.3.30979 Rule breakdown below pts rule name description ---- ---------------------- -------------------------------------------------- 0.10 RDNS_NONE Delivered to trusted network by a host with no rDNS 0.50 BSF_SC5_MJ1963 Custom Rule MJ1963 Subject: [ovs-dev] [PATCH v21 2/8] Persist ovn flow tables 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: , MIME-Version: 1.0 Errors-To: dev-bounces@openvswitch.org Sender: "dev" Ensure that ovn flow tables are persisted so that changes to them chan be applied incrementally - this is a prereq for making lflow_run and physical_run incremental. Signed-off-by: Ryan Moats --- ovn/controller/lflow.c | 26 ++-- ovn/controller/lflow.h | 3 +- ovn/controller/ofctrl.c | 271 +++++++++++++++++++++++++++++----------- ovn/controller/ofctrl.h | 18 ++- ovn/controller/ovn-controller.c | 9 +- ovn/controller/physical.c | 59 +++++---- ovn/controller/physical.h | 2 +- 7 files changed, 264 insertions(+), 124 deletions(-) diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c index 7eebac8..6e2d5e3 100644 --- a/ovn/controller/lflow.c +++ b/ovn/controller/lflow.c @@ -343,13 +343,13 @@ is_switch(const struct sbrec_datapath_binding *ldp) } -/* Adds the logical flows from the Logical_Flow table to 'flow_table'. */ +/* Adds the logical flows from the Logical_Flow table to flow tables. */ static void add_logical_flows(struct controller_ctx *ctx, const struct lport_index *lports, const struct mcgroup_index *mcgroups, const struct hmap *local_datapaths, const struct hmap *patched_datapaths, - const struct simap *ct_zones, struct hmap *flow_table) + const struct simap *ct_zones) { uint32_t conj_id_ofs = 1; @@ -489,8 +489,8 @@ add_logical_flows(struct controller_ctx *ctx, const struct lport_index *lports, m->match.flow.conj_id += conj_id_ofs; } if (!m->n) { - ofctrl_add_flow(flow_table, ptable, lflow->priority, - &m->match, &ofpacts); + ofctrl_add_flow(ptable, lflow->priority, &m->match, &ofpacts, + &lflow->header_.uuid, true); } else { uint64_t conj_stubs[64 / 8]; struct ofpbuf conj; @@ -505,8 +505,8 @@ add_logical_flows(struct controller_ctx *ctx, const struct lport_index *lports, dst->clause = src->clause; dst->n_clauses = src->n_clauses; } - ofctrl_add_flow(flow_table, ptable, lflow->priority, - &m->match, &conj); + ofctrl_add_flow(ptable, lflow->priority, &m->match, &conj, + &lflow->header_.uuid, true); ofpbuf_uninit(&conj); } } @@ -533,12 +533,12 @@ put_load(const uint8_t *data, size_t len, bitwise_one(&sf->mask, sf->field->n_bytes, ofs, n_bits); } -/* Adds an OpenFlow flow to 'flow_table' for each MAC binding in the OVN +/* Adds an OpenFlow flow to flow tables for each MAC binding in the OVN * southbound database, using 'lports' to resolve logical port names to * numbers. */ static void add_neighbor_flows(struct controller_ctx *ctx, - const struct lport_index *lports, struct hmap *flow_table) + const struct lport_index *lports) { struct ofpbuf ofpacts; struct match match; @@ -574,8 +574,8 @@ add_neighbor_flows(struct controller_ctx *ctx, ofpbuf_clear(&ofpacts); put_load(mac.ea, sizeof mac.ea, MFF_ETH_DST, 0, 48, &ofpacts); - ofctrl_add_flow(flow_table, OFTABLE_MAC_BINDING, 100, - &match, &ofpacts); + ofctrl_add_flow(OFTABLE_MAC_BINDING, 100, &match, &ofpacts, + &b->header_.uuid, true); } ofpbuf_uninit(&ofpacts); } @@ -587,12 +587,12 @@ lflow_run(struct controller_ctx *ctx, const struct lport_index *lports, const struct mcgroup_index *mcgroups, const struct hmap *local_datapaths, const struct hmap *patched_datapaths, - const struct simap *ct_zones, struct hmap *flow_table) + const struct simap *ct_zones) { update_address_sets(ctx); add_logical_flows(ctx, lports, mcgroups, local_datapaths, - patched_datapaths, ct_zones, flow_table); - add_neighbor_flows(ctx, lports, flow_table); + patched_datapaths, ct_zones); + add_neighbor_flows(ctx, lports); } void diff --git a/ovn/controller/lflow.h b/ovn/controller/lflow.h index a3fc50c..8f8f81a 100644 --- a/ovn/controller/lflow.h +++ b/ovn/controller/lflow.h @@ -63,8 +63,7 @@ void lflow_run(struct controller_ctx *, const struct lport_index *, const struct mcgroup_index *, const struct hmap *local_datapaths, const struct hmap *patched_datapaths, - const struct simap *ct_zones, - struct hmap *flow_table); + const struct simap *ct_zones); void lflow_destroy(void); #endif /* ovn/lflow.h */ diff --git a/ovn/controller/ofctrl.c b/ovn/controller/ofctrl.c index f537bc0..06d118c 100644 --- a/ovn/controller/ofctrl.c +++ b/ovn/controller/ofctrl.c @@ -16,7 +16,9 @@ #include #include "byte-order.h" #include "dirs.h" +#include "flow.h" #include "hash.h" +#include "hindex.h" #include "hmap.h" #include "ofctrl.h" #include "openflow/openflow.h" @@ -40,19 +42,23 @@ VLOG_DEFINE_THIS_MODULE(ofctrl); /* An OpenFlow flow. */ struct ovn_flow { /* Key. */ - struct hmap_node hmap_node; + struct hmap_node match_hmap_node; /* For match based hashing. */ + struct hindex_node uuid_hmap_node; /* For uuid based hashing. */ uint8_t table_id; uint16_t priority; - struct match match; + struct uuid uuid; /* Data. */ + struct match match; struct ofpact *ofpacts; size_t ofpacts_len; }; -static uint32_t ovn_flow_hash(const struct ovn_flow *); -static struct ovn_flow *ovn_flow_lookup(struct hmap *flow_table, - const struct ovn_flow *target); +static uint32_t ovn_flow_match_hash(const struct ovn_flow *); +static struct ovn_flow *ovn_flow_lookup_by_uuid(struct hindex *, + const struct ovn_flow *target); +static struct ovn_flow *ovn_flow_lookup_by_match(struct hmap *, + const struct ovn_flow *target); static char *ovn_flow_to_string(const struct ovn_flow *); static void ovn_flow_log(const struct ovn_flow *, const char *action); static void ovn_flow_destroy(struct ovn_flow *); @@ -100,11 +106,13 @@ static struct hmap installed_flows; * S_CLEAR_FLOWS or S_UPDATE_FLOWS, this is really the option we have. */ static enum mf_field_id mff_ovn_geneve; -static void ovn_flow_table_clear(struct hmap *flow_table); -static void ovn_flow_table_destroy(struct hmap *flow_table); +static void ovn_flow_table_destroy(void); static void ofctrl_recv(const struct ofp_header *, enum ofptype); +static struct hmap match_flow_table = HMAP_INITIALIZER(&match_flow_table); +static struct hindex uuid_flow_table = HINDEX_INITIALIZER(&uuid_flow_table); + void ofctrl_init(void) { @@ -313,7 +321,7 @@ run_S_CLEAR_FLOWS(void) VLOG_DBG("clearing all flows"); /* Clear installed_flows, to match the state of the switch. */ - ovn_flow_table_clear(&installed_flows); + ovn_flow_table_clear(); state = S_UPDATE_FLOWS; } @@ -431,7 +439,7 @@ void ofctrl_destroy(void) { rconn_destroy(swconn); - ovn_flow_table_destroy(&installed_flows); + ovn_flow_table_destroy(); rconn_packet_counter_destroy(tx_counter); } @@ -464,63 +472,132 @@ ofctrl_recv(const struct ofp_header *oh, enum ofptype type) } } -/* Flow table interface to the rest of ovn-controller. */ +/* Flow table interfaces to the rest of ovn-controller. */ -/* Adds a flow to 'desired_flows' with the specified 'match' and 'actions' to +/* Adds a flow to flow tables with the specified 'match' and 'actions' to * the OpenFlow table numbered 'table_id' with the given 'priority'. The * caller retains ownership of 'match' and 'actions'. * - * This just assembles the desired flow table in memory. Nothing is actually + * Because it is possible for both actions and matches to change on a rule, + * and because the hmap struct only supports a single hash, this method + * uses two hashing mechanisms - a map that uses table_id+priority+matches + * for its hash and an index that uses the appropriate ovsdb row UUID + * alone. + * + * This just assembles the desired flow tables in memory. Nothing is actually * sent to the switch until a later call to ofctrl_run(). * - * The caller should initialize its own hmap to hold the flows. */ + * The is_new boolean is used when tracking changes to indicate whether + * or not the logical flow driving the flow to be added is a new one or + * previously existed and has been modified. */ void -ofctrl_add_flow(struct hmap *desired_flows, - uint8_t table_id, uint16_t priority, - const struct match *match, const struct ofpbuf *actions) +ofctrl_add_flow(uint8_t table_id, uint16_t priority, + const struct match *match, const struct ofpbuf *actions, + const struct uuid *uuid, bool is_new) { + /* Structure that uses table_id+priority+various things as hashes. */ struct ovn_flow *f = xmalloc(sizeof *f); f->table_id = table_id; f->priority = priority; f->match = *match; f->ofpacts = xmemdup(actions->data, actions->size); f->ofpacts_len = actions->size; - f->hmap_node.hash = ovn_flow_hash(f); + memcpy(&f->uuid, uuid, sizeof f->uuid); + f->match_hmap_node.hash = ovn_flow_match_hash(f); + f->uuid_hmap_node.hash = uuid_hash(&f->uuid); - if (ovn_flow_lookup(desired_flows, f)) { - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 5); - if (!VLOG_DROP_INFO(&rl)) { - char *s = ovn_flow_to_string(f); - VLOG_INFO("dropping duplicate flow: %s", s); - free(s); + if (!is_new) { + struct ovn_flow *d = ovn_flow_lookup_by_match(&match_flow_table, f); + if (!d) { + d = ovn_flow_lookup_by_uuid(&uuid_flow_table, f); } - ovn_flow_destroy(f); - return; + if (d) { + hmap_remove(&match_flow_table, &d->match_hmap_node); + hindex_remove(&uuid_flow_table, &d->uuid_hmap_node); + ovn_flow_destroy(d); + } + } else { + /* This is an insert operation, so check to see if this + * is a duplicate via the match hash. If so, then + * check if the actions have changed. If it is a complete + * duplicate (i.e. the actions are the same) drop the new + * flow. If not, then drop the old flow as superseded. + * If the new rule is not a duplicate, check the action + * hash to see if this flow is superseding a previous + * flow and if so, drop the old flow and insert the + * new one. */ + + struct ovn_flow *d = ovn_flow_lookup_by_match(&match_flow_table, f); + + if (d) { + if (ofpacts_equal(f->ofpacts, f->ofpacts_len, + d->ofpacts, d->ofpacts_len)) { + ovn_flow_destroy(f); + return; + } + hmap_remove(&match_flow_table, &d->match_hmap_node); + hindex_remove(&uuid_flow_table, &d->uuid_hmap_node); + ovn_flow_destroy(d); + } } + hmap_insert(&match_flow_table, &f->match_hmap_node, + f->match_hmap_node.hash); + hindex_insert(&uuid_flow_table, &f->uuid_hmap_node, + f->uuid_hmap_node.hash); +} - hmap_insert(desired_flows, &f->hmap_node, f->hmap_node.hash); +/* Removes a bundles of flows from the flow table. */ + +void +ofctrl_remove_flows(const struct uuid *uuid) +{ + struct ovn_flow *f, *next; + HMAP_FOR_EACH_SAFE (f, next, match_hmap_node, &match_flow_table) { + if (uuid_equals(&f->uuid, uuid)) { + hmap_remove(&match_flow_table, &f->match_hmap_node); + hindex_remove(&uuid_flow_table, &f->uuid_hmap_node); + ovn_flow_destroy(f); + } + } } + /* ovn_flow. */ -/* Returns a hash of the key in 'f'. */ +/* Duplicate an ovn_flow structure. */ +struct ovn_flow * +ofctrl_dup_flow(struct ovn_flow *source) +{ + struct ovn_flow *answer = xmalloc(sizeof *answer); + answer->table_id = source->table_id; + answer->priority = source->priority; + answer->match = source->match; + answer->ofpacts = xmemdup(source->ofpacts, source->ofpacts_len); + answer->ofpacts_len = source->ofpacts_len; + answer->uuid = source->uuid; + answer->match_hmap_node.hash = ovn_flow_match_hash(answer); + answer->uuid_hmap_node.hash = uuid_hash(&source->uuid); + return answer; +} + +/* Returns a hash of the match key in 'f'. */ static uint32_t -ovn_flow_hash(const struct ovn_flow *f) +ovn_flow_match_hash(const struct ovn_flow *f) { return hash_2words((f->table_id << 16) | f->priority, match_hash(&f->match, 0)); - } /* Finds and returns an ovn_flow in 'flow_table' whose key is identical to - * 'target''s key, or NULL if there is none. */ + * 'target''s key, or NULL if there is none, using the match hashmap. */ static struct ovn_flow * -ovn_flow_lookup(struct hmap *flow_table, const struct ovn_flow *target) +ovn_flow_lookup_by_match(struct hmap* flow_table, + const struct ovn_flow *target) { struct ovn_flow *f; - HMAP_FOR_EACH_WITH_HASH (f, hmap_node, target->hmap_node.hash, + HMAP_FOR_EACH_WITH_HASH (f, match_hmap_node, target->match_hmap_node.hash, flow_table) { if (f->table_id == target->table_id && f->priority == target->priority @@ -531,6 +608,50 @@ ovn_flow_lookup(struct hmap *flow_table, const struct ovn_flow *target) return NULL; } +/* Find the first flow that the target flow can replace. The target flow + * can replace a flow in the flow table that: + * 1) has the same uuid as the target flow (i.e. comes from the same + * ovsdb table row), + * 2) has the same table_id and priority as the target flow, + * 3) either has the same match criteria as the target flow, or + has a match criteria that is either a proper subset or proper + superset of the target flow. + * + * The reason for the proper set and proper subet part of condition 3 + * is that adding/removing logical flow criteria will lead to flows + * being either more or less specific and we want to find and replace + * the right flow to maintain correct behavior. */ +static struct ovn_flow * +ovn_flow_lookup_by_uuid(struct hindex* flow_table, + const struct ovn_flow *target) +{ + struct ovn_flow *f; + + HINDEX_FOR_EACH_WITH_HASH (f, uuid_hmap_node, + target->uuid_hmap_node.hash, flow_table) { + if (f->table_id == target->table_id + && f->priority == target->priority + && (match_equal(&f->match, &target->match) + || ((flow_wildcards_has_extra(&f->match.wc, + &target->match.wc) + && flow_equal_except(&f->match.flow, + &target->match.flow, + &f->match.wc) + && !flow_wildcards_has_extra(&target->match.wc, + &f->match.wc)) + || (flow_wildcards_has_extra(&target->match.wc, + &f->match.wc) + && flow_equal_except(&target->match.flow, + &f->match.flow, + &target->match.wc) + && !flow_wildcards_has_extra(&f->match.wc, + &target->match.wc))))) { + return f; + } + } + return NULL; +} + static char * ovn_flow_to_string(const struct ovn_flow *f) { @@ -558,26 +679,29 @@ ovn_flow_destroy(struct ovn_flow *f) { if (f) { free(f->ofpacts); - free(f); } + free(f); } /* Flow tables of struct ovn_flow. */ -static void -ovn_flow_table_clear(struct hmap *flow_table) +void +ovn_flow_table_clear(void) { - struct ovn_flow *f; - HMAP_FOR_EACH_POP (f, hmap_node, flow_table) { + struct ovn_flow *f, *next; + HMAP_FOR_EACH_SAFE (f, next, match_hmap_node, &match_flow_table) { + hmap_remove(&match_flow_table, &f->match_hmap_node); + hindex_remove(&uuid_flow_table, &f->uuid_hmap_node); ovn_flow_destroy(f); } } static void -ovn_flow_table_destroy(struct hmap *flow_table) +ovn_flow_table_destroy(void) { - ovn_flow_table_clear(flow_table); - hmap_destroy(flow_table); + ovn_flow_table_clear(); + hmap_destroy(&match_flow_table); + hindex_destroy(&uuid_flow_table); } /* Flow table update. */ @@ -597,19 +721,16 @@ queue_flow_mod(struct ofputil_flow_mod *fm) * flows from 'flow_table' and frees them. (The hmap itself isn't * destroyed.) * - * This called be called be ofctrl_run() within the main loop. */ + * This can be called by ofctrl_run() within the main loop. */ void -ofctrl_put(struct hmap *flow_table) +ofctrl_put(void) { /* The flow table can be updated if the connection to the switch is up and * in the correct state and not backlogged with existing flow_mods. (Our * criteria for being backlogged appear very conservative, but the socket - * between ovn-controller and OVS provides some buffering.) Otherwise, - * discard the flows. A solution to either of those problems will cause us - * to wake up and retry. */ + * between ovn-controller and OVS provides some buffering.) */ if (state != S_UPDATE_FLOWS || rconn_packet_counter_n_packets(tx_counter)) { - ovn_flow_table_clear(flow_table); return; } @@ -617,8 +738,8 @@ ofctrl_put(struct hmap *flow_table) * longer desired, delete them; if any of them should have different * actions, update them. */ struct ovn_flow *i, *next; - HMAP_FOR_EACH_SAFE (i, next, hmap_node, &installed_flows) { - struct ovn_flow *d = ovn_flow_lookup(flow_table, i); + HMAP_FOR_EACH_SAFE (i, next, match_hmap_node, &installed_flows) { + struct ovn_flow *d = ovn_flow_lookup_by_match(&match_flow_table, i); if (!d) { /* Installed flow is no longer desirable. Delete it from the * switch and from installed_flows. */ @@ -629,9 +750,9 @@ ofctrl_put(struct hmap *flow_table) .command = OFPFC_DELETE_STRICT, }; queue_flow_mod(&fm); - ovn_flow_log(i, "removing"); + ovn_flow_log(i, "removing installed"); - hmap_remove(&installed_flows, &i->hmap_node); + hmap_remove(&installed_flows, &i->match_hmap_node); ovn_flow_destroy(i); } else { if (!ofpacts_equal(i->ofpacts, i->ofpacts_len, @@ -646,40 +767,38 @@ ofctrl_put(struct hmap *flow_table) .command = OFPFC_MODIFY_STRICT, }; queue_flow_mod(&fm); - ovn_flow_log(i, "updating"); + ovn_flow_log(i, "updating installed"); /* Replace 'i''s actions by 'd''s. */ free(i->ofpacts); - i->ofpacts = d->ofpacts; + i->ofpacts = xmemdup(d->ofpacts, d->ofpacts_len); i->ofpacts_len = d->ofpacts_len; - d->ofpacts = NULL; - d->ofpacts_len = 0; } - - hmap_remove(flow_table, &d->hmap_node); - ovn_flow_destroy(d); } } - /* The previous loop removed from 'flow_table' all of the flows that are - * already installed. Thus, any flows remaining in 'flow_table' need to - * be added to the flow table. */ + /* Iterate through the new flows and add those that aren't found + * in the installed flow table. */ struct ovn_flow *d; - HMAP_FOR_EACH_SAFE (d, next, hmap_node, flow_table) { - /* Send flow_mod to add flow. */ - struct ofputil_flow_mod fm = { - .match = d->match, - .priority = d->priority, - .table_id = d->table_id, - .ofpacts = d->ofpacts, - .ofpacts_len = d->ofpacts_len, - .command = OFPFC_ADD, - }; - queue_flow_mod(&fm); - ovn_flow_log(d, "adding"); - - /* Move 'd' from 'flow_table' to installed_flows. */ - hmap_remove(flow_table, &d->hmap_node); - hmap_insert(&installed_flows, &d->hmap_node, d->hmap_node.hash); + HMAP_FOR_EACH_SAFE (d, next, match_hmap_node, &match_flow_table) { + struct ovn_flow *i = ovn_flow_lookup_by_match(&installed_flows, d); + if (!i) { + /* Send flow_mod to add flow. */ + struct ofputil_flow_mod fm = { + .match = d->match, + .priority = d->priority, + .table_id = d->table_id, + .ofpacts = d->ofpacts, + .ofpacts_len = d->ofpacts_len, + .command = OFPFC_ADD, + }; + queue_flow_mod(&fm); + ovn_flow_log(d, "adding installed"); + + /* Copy 'd' from 'flow_table' to installed_flows. */ + struct ovn_flow *new_node = ofctrl_dup_flow(d); + hmap_insert(&installed_flows, &new_node->match_hmap_node, + new_node->match_hmap_node.hash); + } } } diff --git a/ovn/controller/ofctrl.h b/ovn/controller/ofctrl.h index bc9cfba..d594be7 100644 --- a/ovn/controller/ofctrl.h +++ b/ovn/controller/ofctrl.h @@ -20,6 +20,7 @@ #include #include "openvswitch/meta-flow.h" +#include "ovsdb-idl.h" struct controller_ctx; struct hmap; @@ -30,12 +31,21 @@ struct ovsrec_bridge; /* Interface for OVN main loop. */ void ofctrl_init(void); enum mf_field_id ofctrl_run(const struct ovsrec_bridge *br_int); -void ofctrl_put(struct hmap *flows); +void ofctrl_put(void); void ofctrl_wait(void); void ofctrl_destroy(void); -/* Flow table interface to the rest of ovn-controller. */ -void ofctrl_add_flow(struct hmap *flows, uint8_t table_id, uint16_t priority, - const struct match *, const struct ofpbuf *ofpacts); +struct ovn_flow *ofctrl_dup_flow(struct ovn_flow *source); + +/* Flow table interfaces to the rest of ovn-controller. */ +void ofctrl_add_flow(uint8_t table_id, uint16_t priority, + const struct match *, const struct ofpbuf *ofpacts, + const struct uuid *uuid, bool is_new); + +void ofctrl_remove_flows(const struct uuid *uuid); + +void ofctrl_flow_table_clear(void); + +void ovn_flow_table_clear(void); #endif /* ovn/ofctrl.h */ diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c index 3257eab..6a8ff9b 100644 --- a/ovn/controller/ovn-controller.c +++ b/ovn/controller/ovn-controller.c @@ -433,16 +433,15 @@ main(int argc, char *argv[]) update_ct_zones(&all_lports, &patched_datapaths, &ct_zones, ct_zone_bitmap); - struct hmap flow_table = HMAP_INITIALIZER(&flow_table); + ovn_flow_table_clear(); lflow_run(&ctx, &lports, &mcgroups, &local_datapaths, - &patched_datapaths, &ct_zones, &flow_table); + &patched_datapaths, &ct_zones); if (chassis_id) { physical_run(&ctx, mff_ovn_geneve, - br_int, chassis_id, &ct_zones, &flow_table, + br_int, chassis_id, &ct_zones, &local_datapaths, &patched_datapaths); } - ofctrl_put(&flow_table); - hmap_destroy(&flow_table); + ofctrl_put(); } sset_destroy(&all_lports); diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c index d7637f2..2b5ef0a 100644 --- a/ovn/controller/physical.c +++ b/ovn/controller/physical.c @@ -51,6 +51,9 @@ physical_register_ovs_idl(struct ovsdb_idl *ovs_idl) ovsdb_idl_add_column(ovs_idl, &ovsrec_interface_col_external_ids); } +/* UUID to identify OF flows not associated with ovsdb rows. */ +static struct uuid *hc_uuid = NULL; + /* Maps from a chassis to the OpenFlow port number of the tunnel that can be * used to reach that chassis. */ struct chassis_tunnel { @@ -149,11 +152,15 @@ get_localnet_port(struct hmap *local_datapaths, int64_t tunnel_key) void physical_run(struct controller_ctx *ctx, enum mf_field_id mff_ovn_geneve, const struct ovsrec_bridge *br_int, const char *this_chassis_id, - const struct simap *ct_zones, struct hmap *flow_table, + const struct simap *ct_zones, struct hmap *local_datapaths, struct hmap *patched_datapaths) { struct simap localvif_to_ofport = SIMAP_INITIALIZER(&localvif_to_ofport); struct hmap tunnels = HMAP_INITIALIZER(&tunnels); + if (!hc_uuid) { + hc_uuid = xmalloc(sizeof(struct uuid)); + uuid_generate(hc_uuid); + } for (int i = 0; i < br_int->n_ports; i++) { const struct ovsrec_port *port_rec = br_int->ports[i]; @@ -398,8 +405,9 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id mff_ovn_geneve, /* Resubmit to first logical ingress pipeline table. */ put_resubmit(OFTABLE_LOG_INGRESS_PIPELINE, &ofpacts); - ofctrl_add_flow(flow_table, OFTABLE_PHY_TO_LOG, - tag ? 150 : 100, &match, &ofpacts); + ofctrl_add_flow(OFTABLE_PHY_TO_LOG, + tag ? 150 : 100, &match, &ofpacts, + &binding->header_.uuid, true); if (!tag && (!strcmp(binding->type, "localnet") || !strcmp(binding->type, "l2gateway"))) { @@ -408,7 +416,8 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id mff_ovn_geneve, * action. */ ofpbuf_pull(&ofpacts, ofpacts_orig_size); match_set_dl_tci_masked(&match, 0, htons(VLAN_CFI)); - ofctrl_add_flow(flow_table, 0, 100, &match, &ofpacts); + ofctrl_add_flow(0, 100, &match, &ofpacts, + &binding->header_.uuid, true); } /* Table 33, priority 100. @@ -438,8 +447,8 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id mff_ovn_geneve, /* Resubmit to table 34. */ put_resubmit(OFTABLE_DROP_LOOPBACK, &ofpacts); - ofctrl_add_flow(flow_table, OFTABLE_LOCAL_OUTPUT, 100, &match, - &ofpacts); + ofctrl_add_flow(OFTABLE_LOCAL_OUTPUT, 100, &match, &ofpacts, + &binding->header_.uuid, true); /* Table 34, Priority 100. * ======================= @@ -450,8 +459,8 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id mff_ovn_geneve, match_set_metadata(&match, htonll(dp_key)); match_set_reg(&match, MFF_LOG_INPORT - MFF_REG0, port_key); match_set_reg(&match, MFF_LOG_OUTPORT - MFF_REG0, port_key); - ofctrl_add_flow(flow_table, OFTABLE_DROP_LOOPBACK, 100, - &match, &ofpacts); + ofctrl_add_flow(OFTABLE_DROP_LOOPBACK, 100, &match, &ofpacts, + &binding->header_.uuid, true); /* Table 64, Priority 100. * ======================= @@ -485,8 +494,8 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id mff_ovn_geneve, ofpact_put_STRIP_VLAN(&ofpacts); put_stack(MFF_IN_PORT, ofpact_put_STACK_POP(&ofpacts)); } - ofctrl_add_flow(flow_table, OFTABLE_LOG_TO_PHY, 100, - &match, &ofpacts); + ofctrl_add_flow(OFTABLE_LOG_TO_PHY, 100, &match, &ofpacts, + &binding->header_.uuid, true); } else if (!tun) { /* Remote port connected by localnet port */ /* Table 33, priority 100. @@ -508,8 +517,8 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id mff_ovn_geneve, /* Resubmit to table 33. */ put_resubmit(OFTABLE_LOCAL_OUTPUT, &ofpacts); - ofctrl_add_flow(flow_table, OFTABLE_LOCAL_OUTPUT, 100, &match, - &ofpacts); + ofctrl_add_flow(OFTABLE_LOCAL_OUTPUT, 100, &match, &ofpacts, + &binding->header_.uuid, true); } else { /* Remote port connected by tunnel */ /* Table 32, priority 100. @@ -532,8 +541,8 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id mff_ovn_geneve, /* Output to tunnel. */ ofpact_put_OUTPUT(&ofpacts)->port = ofport; - ofctrl_add_flow(flow_table, OFTABLE_REMOTE_OUTPUT, 100, - &match, &ofpacts); + ofctrl_add_flow(OFTABLE_REMOTE_OUTPUT, 100, &match, &ofpacts, + &binding->header_.uuid, true); } } @@ -608,8 +617,8 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id mff_ovn_geneve, * group as the logical output port. */ put_load(mc->tunnel_key, MFF_LOG_OUTPORT, 0, 32, &ofpacts); - ofctrl_add_flow(flow_table, OFTABLE_LOCAL_OUTPUT, 100, - &match, &ofpacts); + ofctrl_add_flow(OFTABLE_LOCAL_OUTPUT, 100, &match, &ofpacts, + &mc->header_.uuid, true); } /* Table 32, priority 100. @@ -646,8 +655,9 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id mff_ovn_geneve, if (local_ports) { put_resubmit(OFTABLE_LOCAL_OUTPUT, &remote_ofpacts); } - ofctrl_add_flow(flow_table, OFTABLE_REMOTE_OUTPUT, 100, - &match, &remote_ofpacts); + ofctrl_add_flow(OFTABLE_REMOTE_OUTPUT, 100, + &match, &remote_ofpacts, + &mc->header_.uuid, true); } } sset_destroy(&remote_chassis); @@ -690,7 +700,8 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id mff_ovn_geneve, put_resubmit(OFTABLE_LOCAL_OUTPUT, &ofpacts); - ofctrl_add_flow(flow_table, OFTABLE_PHY_TO_LOG, 100, &match, &ofpacts); + ofctrl_add_flow(OFTABLE_PHY_TO_LOG, 100, &match, &ofpacts, + hc_uuid, true); } /* Add flows for VXLAN encapsulations. Due to the limited amount of @@ -723,8 +734,8 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id mff_ovn_geneve, put_load(binding->tunnel_key, MFF_LOG_INPORT, 0, 15, &ofpacts); put_resubmit(OFTABLE_LOG_INGRESS_PIPELINE, &ofpacts); - ofctrl_add_flow(flow_table, OFTABLE_PHY_TO_LOG, 100, &match, - &ofpacts); + ofctrl_add_flow(OFTABLE_PHY_TO_LOG, 100, &match, &ofpacts, + hc_uuid, true); } } @@ -737,7 +748,8 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id mff_ovn_geneve, match_init_catchall(&match); ofpbuf_clear(&ofpacts); put_resubmit(OFTABLE_LOCAL_OUTPUT, &ofpacts); - ofctrl_add_flow(flow_table, OFTABLE_REMOTE_OUTPUT, 0, &match, &ofpacts); + ofctrl_add_flow(OFTABLE_REMOTE_OUTPUT, 0, &match, &ofpacts, + hc_uuid, true); /* Table 34, Priority 0. * ======================= @@ -751,7 +763,8 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id mff_ovn_geneve, MFF_LOG_REGS; #undef MFF_LOG_REGS put_resubmit(OFTABLE_LOG_EGRESS_PIPELINE, &ofpacts); - ofctrl_add_flow(flow_table, OFTABLE_DROP_LOOPBACK, 0, &match, &ofpacts); + ofctrl_add_flow(OFTABLE_DROP_LOOPBACK, 0, &match, &ofpacts, + hc_uuid, true); ofpbuf_uninit(&ofpacts); simap_destroy(&localvif_to_ofport); diff --git a/ovn/controller/physical.h b/ovn/controller/physical.h index 2f8b58a..1f98f71 100644 --- a/ovn/controller/physical.h +++ b/ovn/controller/physical.h @@ -43,7 +43,7 @@ struct simap; void physical_register_ovs_idl(struct ovsdb_idl *); void physical_run(struct controller_ctx *, enum mf_field_id mff_ovn_geneve, const struct ovsrec_bridge *br_int, const char *chassis_id, - const struct simap *ct_zones, struct hmap *flow_table, + const struct simap *ct_zones, struct hmap *local_datapaths, struct hmap *patched_datapaths); #endif /* ovn/physical.h */