[ovs-dev] [PATCH v3 3/5] ovn-controller: Refactor conntrack zone allocation.
Gurucharan Shetty
guru at ovn.org
Thu May 19 20:02:32 UTC 2016
We currently allocate conntrack zones in binding.c. It fits
in nicely there because we currently only allocate conntrack
zones to logical ports and binding.c is where we figure out
the local ones.
An upcoming commit needs conntrack zone allocation for routers
in a gateway. For that reason, this commit moves conntrack zone
allocation code to ovn-controller.c where it would be easily
accessible for router zone allocation too.
Signed-off-by: Gurucharan Shetty <guru at ovn.org>
---
ovn/controller/binding.c | 61 ++++-------------------------------------
ovn/controller/binding.h | 5 ++--
ovn/controller/ovn-controller.c | 54 ++++++++++++++++++++++++++++++++++--
3 files changed, 61 insertions(+), 59 deletions(-)
diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
index e5e55b1..a07c327 100644
--- a/ovn/controller/binding.c
+++ b/ovn/controller/binding.c
@@ -77,51 +77,6 @@ get_local_iface_ids(const struct ovsrec_bridge *br_int, struct shash *lports)
}
static void
-update_ct_zones(struct sset *lports, struct simap *ct_zones,
- unsigned long *ct_zone_bitmap)
-{
- struct simap_node *ct_zone, *ct_zone_next;
- const char *iface_id;
- int scan_start = 1;
-
- /* xxx This is wasteful to assign a zone to each port--even if no
- * xxx security policy is applied. */
-
- /* Delete any zones that are associated with removed ports. */
- SIMAP_FOR_EACH_SAFE(ct_zone, ct_zone_next, ct_zones) {
- if (!sset_contains(lports, ct_zone->name)) {
- bitmap_set0(ct_zone_bitmap, ct_zone->data);
- simap_delete(ct_zones, ct_zone);
- }
- }
-
- /* Assign a unique zone id for each logical port. */
- SSET_FOR_EACH(iface_id, lports) {
- size_t zone;
-
- if (simap_contains(ct_zones, iface_id)) {
- continue;
- }
-
- /* We assume that there are 64K zones and that we own them all. */
- zone = bitmap_scan(ct_zone_bitmap, 0, scan_start, MAX_CT_ZONES + 1);
- if (zone == MAX_CT_ZONES + 1) {
- static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
- VLOG_WARN_RL(&rl, "exhausted all ct zones");
- return;
- }
- scan_start = zone + 1;
-
- bitmap_set1(ct_zone_bitmap, zone);
- simap_put(ct_zones, iface_id, zone);
-
- /* xxx We should erase any old entries for this
- * xxx zone, but we need a generic interface to the conntrack
- * xxx table. */
- }
-}
-
-static void
add_local_datapath(struct hmap *local_datapaths,
const struct sbrec_port_binding *binding_rec)
{
@@ -148,8 +103,8 @@ update_qos(const struct ovsrec_interface *iface_rec,
void
binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int,
- const char *chassis_id, struct simap *ct_zones,
- unsigned long *ct_zone_bitmap, struct hmap *local_datapaths)
+ const char *chassis_id, struct sset *all_lports,
+ struct hmap *local_datapaths)
{
const struct sbrec_chassis *chassis_rec;
const struct sbrec_port_binding *binding_rec;
@@ -164,10 +119,9 @@ binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int,
* We'll remove our chassis from all port binding records below. */
}
- struct sset all_lports = SSET_INITIALIZER(&all_lports);
struct shash_node *node;
SHASH_FOR_EACH (node, &lports) {
- sset_add(&all_lports, node->name);
+ sset_add(all_lports, node->name);
}
/* Run through each binding record to see if it is resident on this
@@ -178,10 +132,10 @@ binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int,
= shash_find_and_delete(&lports, binding_rec->logical_port);
if (iface_rec
|| (binding_rec->parent_port && binding_rec->parent_port[0] &&
- sset_contains(&all_lports, binding_rec->parent_port))) {
+ sset_contains(all_lports, binding_rec->parent_port))) {
if (binding_rec->parent_port && binding_rec->parent_port[0]) {
/* Add child logical port to the set of all local ports. */
- sset_add(&all_lports, binding_rec->logical_port);
+ sset_add(all_lports, binding_rec->logical_port);
}
add_local_datapath(local_datapaths, binding_rec);
if (iface_rec && ctx->ovs_idl_txn) {
@@ -213,7 +167,7 @@ binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int,
* to list them in all_lports because we want to allocate
* a conntrack zone ID for each one, as we'll be creating
* a patch port for each one. */
- sset_add(&all_lports, binding_rec->logical_port);
+ sset_add(all_lports, binding_rec->logical_port);
}
}
@@ -221,10 +175,7 @@ binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int,
VLOG_DBG("No port binding record for lport %s", node->name);
}
- update_ct_zones(&all_lports, ct_zones, ct_zone_bitmap);
-
shash_destroy(&lports);
- sset_destroy(&all_lports);
}
/* Returns true if the database is all cleaned up, false if more work is
diff --git a/ovn/controller/binding.h b/ovn/controller/binding.h
index 6e19c10..25f8989 100644
--- a/ovn/controller/binding.h
+++ b/ovn/controller/binding.h
@@ -24,11 +24,12 @@ struct hmap;
struct ovsdb_idl;
struct ovsrec_bridge;
struct simap;
+struct sset;
void binding_register_ovs_idl(struct ovsdb_idl *);
void binding_run(struct controller_ctx *, const struct ovsrec_bridge *br_int,
- const char *chassis_id, struct simap *ct_zones,
- unsigned long *ct_zone_bitmap, struct hmap *local_datapaths);
+ const char *chassis_id, struct sset *all_lports,
+ struct hmap *local_datapaths);
bool binding_cleanup(struct controller_ctx *, const char *chassis_id);
#endif /* ovn/binding.h */
diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
index bc4c24f..a87937f 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -47,6 +47,7 @@
#include "lib/bitmap.h"
#include "lib/hash.h"
#include "smap.h"
+#include "sset.h"
#include "stream-ssl.h"
#include "stream.h"
#include "unixctl.h"
@@ -252,6 +253,51 @@ get_ovnsb_remote_probe_interval(struct ovsdb_idl *ovs_idl, int *value)
return false;
}
+static void
+update_ct_zones(struct sset *lports, struct simap *ct_zones,
+ unsigned long *ct_zone_bitmap)
+{
+ struct simap_node *ct_zone, *ct_zone_next;
+ const char *iface_id;
+ int scan_start = 1;
+
+ /* xxx This is wasteful to assign a zone to each port--even if no
+ * xxx security policy is applied. */
+
+ /* Delete any zones that are associated with removed ports. */
+ SIMAP_FOR_EACH_SAFE(ct_zone, ct_zone_next, ct_zones) {
+ if (!sset_contains(lports, ct_zone->name)) {
+ bitmap_set0(ct_zone_bitmap, ct_zone->data);
+ simap_delete(ct_zones, ct_zone);
+ }
+ }
+
+ /* Assign a unique zone id for each logical port. */
+ SSET_FOR_EACH(iface_id, lports) {
+ size_t zone;
+
+ if (simap_contains(ct_zones, iface_id)) {
+ continue;
+ }
+
+ /* We assume that there are 64K zones and that we own them all. */
+ zone = bitmap_scan(ct_zone_bitmap, 0, scan_start, MAX_CT_ZONES + 1);
+ if (zone == MAX_CT_ZONES + 1) {
+ static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
+ VLOG_WARN_RL(&rl, "exhausted all ct zones");
+ return;
+ }
+ scan_start = zone + 1;
+
+ bitmap_set1(ct_zone_bitmap, zone);
+ simap_put(ct_zones, iface_id, zone);
+
+ /* xxx We should erase any old entries for this
+ * xxx zone, but we need a generic interface to the conntrack
+ * xxx table. */
+ }
+}
+
int
main(int argc, char *argv[])
{
@@ -353,6 +399,7 @@ main(int argc, char *argv[])
struct hmap local_datapaths = HMAP_INITIALIZER(&local_datapaths);
struct hmap patched_datapaths = HMAP_INITIALIZER(&patched_datapaths);
+ struct sset all_lports = SSET_INITIALIZER(&all_lports);
const struct ovsrec_bridge *br_int = get_br_int(&ctx);
const char *chassis_id = get_chassis_id(ctx.ovs_idl);
@@ -360,8 +407,8 @@ main(int argc, char *argv[])
if (chassis_id) {
chassis_run(&ctx, chassis_id);
encaps_run(&ctx, br_int, chassis_id);
- binding_run(&ctx, br_int, chassis_id, &ct_zones, ct_zone_bitmap,
- &local_datapaths);
+ binding_run(&ctx, br_int, chassis_id, &all_lports,
+ &local_datapaths);
}
if (br_int && chassis_id) {
@@ -376,6 +423,7 @@ main(int argc, char *argv[])
enum mf_field_id mff_ovn_geneve = ofctrl_run(br_int);
pinctrl_run(&ctx, &lports, br_int, chassis_id, &local_datapaths);
+ update_ct_zones(&all_lports, &ct_zones, ct_zone_bitmap);
struct hmap flow_table = HMAP_INITIALIZER(&flow_table);
lflow_run(&ctx, &lports, &mcgroups, &local_datapaths,
@@ -391,6 +439,8 @@ main(int argc, char *argv[])
lport_index_destroy(&lports);
}
+ sset_destroy(&all_lports);
+
struct local_datapath *cur_node, *next_node;
HMAP_FOR_EACH_SAFE (cur_node, next_node, hmap_node, &local_datapaths) {
hmap_remove(&local_datapaths, &cur_node->hmap_node);
--
1.9.1
More information about the dev
mailing list