Skip to content

Commit

Permalink
ovn: Use C strings instead of ds for extended tables.
Browse files Browse the repository at this point in the history
Dynamic strings are not needed for the most part and are introduing
additional conversions back and forth with C strings.

Signed-off-by: Justin Pettit <[email protected]>
Acked-by: Ben Pfaff <[email protected]>
  • Loading branch information
justinpettit committed Jul 31, 2018
1 parent 4cb3476 commit 2df707a
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 27 deletions.
4 changes: 2 additions & 2 deletions ovn/controller/ofctrl.c
Original file line number Diff line number Diff line change
Expand Up @@ -874,7 +874,7 @@ ofctrl_put(struct hmap *flow_table, struct shash *pending_ct_zones,
enum ofputil_protocol usable_protocols;
char *group_string = xasprintf("group_id=%"PRIu32",%s",
desired->table_id,
ds_cstr(&desired->info));
desired->name);
char *error = parse_ofp_group_mod_str(&gm, OFPGC11_ADD, group_string,
NULL, NULL, &usable_protocols);
if (!error) {
Expand All @@ -897,7 +897,7 @@ ofctrl_put(struct hmap *flow_table, struct shash *pending_ct_zones,
enum ofputil_protocol usable_protocols;
char *meter_string = xasprintf("meter=%"PRIu32",%s",
m_desired->table_id,
ds_cstr(&m_desired->info));
m_desired->name);
char *error = parse_ofp_meter_mod_str(&mm, meter_string, OFPMC13_ADD,
&usable_protocols);
if (!error) {
Expand Down
20 changes: 8 additions & 12 deletions ovn/lib/actions.c
Original file line number Diff line number Diff line change
Expand Up @@ -1049,7 +1049,7 @@ encode_CT_LB(const struct ovnact_ct_lb *cl,
recirc_table, zone_reg);
}

table_id = ovn_extend_table_assign_id(ep->group_table, &ds);
table_id = ovn_extend_table_assign_id(ep->group_table, ds_cstr(&ds));
ds_destroy(&ds);
if (table_id == EXT_TABLE_ID_INVALID) {
return;
Expand Down Expand Up @@ -2217,25 +2217,21 @@ encode_SET_METER(const struct ovnact_set_meter *cl,
uint32_t table_id;
struct ofpact_meter *om;

struct ds ds = DS_EMPTY_INITIALIZER;
char *name;
if (cl->burst) {
ds_put_format(&ds,
"kbps burst stats bands=type=drop rate=%"PRId64" "
"burst_size=%"PRId64"",
cl->rate, cl->burst);
name = xasprintf("kbps burst stats bands=type=drop rate=%"PRId64" "
"burst_size=%"PRId64"", cl->rate, cl->burst);
} else {
ds_put_format(&ds, "kbps stats bands=type=drop rate=%"PRId64"",
cl->rate);
name = xasprintf("kbps stats bands=type=drop rate=%"PRId64"",
cl->rate);
}

table_id = ovn_extend_table_assign_id(ep->meter_table, &ds);
table_id = ovn_extend_table_assign_id(ep->meter_table, name);
free(name);
if (table_id == EXT_TABLE_ID_INVALID) {
ds_destroy(&ds);
return;
}

ds_destroy(&ds);

/* Create an action to set the meter. */
om = ofpact_put_METER(ofpacts);
om->meter_id = table_id;
Expand Down
22 changes: 12 additions & 10 deletions ovn/lib/extend-table.c
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
*/

#include <config.h>
#include <string.h>

#include "bitmap.h"
#include "hash.h"
#include "openvswitch/vlog.h"
Expand All @@ -39,15 +41,15 @@ ovn_extend_table_destroy(struct ovn_extend_table *table)
struct ovn_extend_table_info *desired, *d_next;
HMAP_FOR_EACH_SAFE (desired, d_next, hmap_node, &table->existing) {
hmap_remove(&table->existing, &desired->hmap_node);
ds_destroy(&desired->info);
free(desired->name);
free(desired);
}
hmap_destroy(&table->desired);

struct ovn_extend_table_info *existing, *e_next;
HMAP_FOR_EACH_SAFE (existing, e_next, hmap_node, &table->existing) {
hmap_remove(&table->existing, &existing->hmap_node);
ds_destroy(&existing->info);
free(existing->name);
free(existing);
}
hmap_destroy(&table->existing);
Expand Down Expand Up @@ -84,7 +86,7 @@ ovn_extend_table_clear(struct ovn_extend_table *table, bool existing)
if (existing || g->new_table_id) {
bitmap_set0(table->table_ids, g->table_id);
}
ds_destroy(&g->info);
free(g->name);
free(g);
}
}
Expand All @@ -95,7 +97,7 @@ ovn_extend_table_remove(struct ovn_extend_table *table,
{
/* Remove 'existing' from 'groups->existing' */
hmap_remove(&table->existing, &existing->hmap_node);
ds_destroy(&existing->info);
free(existing->name);

/* Dealloc group_id. */
bitmap_set0(table->table_ids, existing->table_id);
Expand All @@ -115,7 +117,7 @@ ovn_extend_table_move(struct ovn_extend_table *table)
hmap_insert(&table->existing, &desired->hmap_node,
desired->hmap_node.hash);
} else {
ds_destroy(&desired->info);
free(desired->name);
free(desired);
}
}
Expand All @@ -124,24 +126,24 @@ ovn_extend_table_move(struct ovn_extend_table *table)
/* Assign a new table ID for the table information from the bitmap.
* If it already exists, return the old ID. */
uint32_t
ovn_extend_table_assign_id(struct ovn_extend_table *table, struct ds *ds)
ovn_extend_table_assign_id(struct ovn_extend_table *table, const char *name)
{
uint32_t table_id = 0, hash;
struct ovn_extend_table_info *table_info;

hash = hash_string(ds_cstr(ds), 0);
hash = hash_string(name, 0);

/* Check whether we have non installed but allocated group_id. */
HMAP_FOR_EACH_WITH_HASH (table_info, hmap_node, hash, &table->desired) {
if (!strcmp(ds_cstr(&table_info->info), ds_cstr(ds))) {
if (!strcmp(table_info->name, name)) {
return table_info->table_id;
}
}

/* Check whether we already have an installed entry for this
* combination. */
HMAP_FOR_EACH_WITH_HASH (table_info, hmap_node, hash, &table->existing) {
if (!strcmp(ds_cstr(&table_info->info), ds_cstr(ds))) {
if (!strcmp(table_info->name, name)) {
table_id = table_info->table_id;
}
}
Expand All @@ -161,7 +163,7 @@ ovn_extend_table_assign_id(struct ovn_extend_table *table, struct ds *ds)
bitmap_set1(table->table_ids, table_id);

table_info = xmalloc(sizeof *table_info);
ds_clone(&table_info->info, ds);
table_info->name = xstrdup(name);
table_info->table_id = table_id;
table_info->hmap_node.hash = hash;
table_info->new_table_id = new_table_id;
Expand Down
5 changes: 2 additions & 3 deletions ovn/lib/extend-table.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
#define MAX_EXT_TABLE_ID 65535
#define EXT_TABLE_ID_INVALID 0

#include "openvswitch/dynamic-string.h"
#include "openvswitch/hmap.h"
#include "openvswitch/list.h"

Expand All @@ -36,7 +35,7 @@ struct ovn_extend_table {

struct ovn_extend_table_info {
struct hmap_node hmap_node;
struct ds info; /* Details string for the table entity. */
char *name; /* Name for the table entity. */
uint32_t table_id;
bool new_table_id; /* 'True' if 'table_id' was reserved from
* ovn_extend_table's 'table_ids' bitmap. */
Expand All @@ -58,7 +57,7 @@ void ovn_extend_table_remove(struct ovn_extend_table *,
void ovn_extend_table_move(struct ovn_extend_table *);

uint32_t ovn_extend_table_assign_id(struct ovn_extend_table *,
struct ds *);
const char *name);

/* Iterates 'DESIRED' through all of the 'ovn_extend_table_info's in
* 'TABLE'->desired that are not in 'TABLE'->existing. (The loop body
Expand Down

0 comments on commit 2df707a

Please sign in to comment.