Skip to content

Commit

Permalink
ovn-controller: Refactor conntrack zone allocation.
Browse files Browse the repository at this point in the history 
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 <[email protected]>
Acked-by: Ben Pfaff <[email protected]>
  • Loading branch information
@shettyg
shettyg committed Jun 3, 2016
1 parent c164500 commit a478c4e
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 59 deletions.
61 changes: 6 additions & 55 deletions ovn/controller/binding.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,51 +76,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)
Expand Down Expand Up @@ -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;
Expand All @@ -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
Expand All @@ -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) {
Expand Down Expand Up @@ -213,18 +167,15 @@ 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);
}
}

SHASH_FOR_EACH (node, &lports) {
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
Expand Down
5 changes: 3 additions & 2 deletions ovn/controller/binding.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
54 changes: 52 additions & 2 deletions ovn/controller/ovn-controller.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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[])
{
Expand Down Expand Up @@ -353,15 +399,16 @@ 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);

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) {
Expand All @@ -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,
Expand All @@ -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);
Expand Down

0 comments on commit a478c4e

Please sign in to comment.