[ovs-dev,ACL,Meters,1/7] ovn: Use C strings instead of ds for extended tables.
diff mbox series

Message ID 20180730064638.121021-1-jpettit@ovn.org
State Accepted
Headers show
Series
  • [ovs-dev,ACL,Meters,1/7] ovn: Use C strings instead of ds for extended tables.
Related show

Commit Message

Justin Pettit July 30, 2018, 6:46 a.m. UTC
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 <jpettit@ovn.org>
---
 ovn/controller/ofctrl.c |  4 ++--
 ovn/lib/actions.c       | 20 ++++++++------------
 ovn/lib/extend-table.c  | 22 ++++++++++++----------
 ovn/lib/extend-table.h  |  5 ++---
 4 files changed, 24 insertions(+), 27 deletions(-)

Comments

Ben Pfaff July 30, 2018, 7:55 p.m. UTC | #1
On Sun, Jul 29, 2018 at 11:46:32PM -0700, Justin Pettit wrote:
> 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 <jpettit@ovn.org>

For the series, assuming you think my comments are reasonable:

Acked-by: Ben Pfaff <blp@ovn.org>

Patch
diff mbox series

diff --git a/ovn/controller/ofctrl.c b/ovn/controller/ofctrl.c
index 349de3aaa284..c7f41612456d 100644
--- a/ovn/controller/ofctrl.c
+++ b/ovn/controller/ofctrl.c
@@ -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) {
@@ -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) {
diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c
index 5c559e37a479..430bb677bafb 100644
--- a/ovn/lib/actions.c
+++ b/ovn/lib/actions.c
@@ -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;
@@ -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;
diff --git a/ovn/lib/extend-table.c b/ovn/lib/extend-table.c
index e18713b9766c..511d1a84b0c0 100644
--- a/ovn/lib/extend-table.c
+++ b/ovn/lib/extend-table.c
@@ -15,6 +15,8 @@ 
  */
 
 #include <config.h>
+#include <string.h>
+
 #include "bitmap.h"
 #include "hash.h"
 #include "openvswitch/vlog.h"
@@ -39,7 +41,7 @@  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);
@@ -47,7 +49,7 @@  ovn_extend_table_destroy(struct ovn_extend_table *table)
     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);
@@ -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);
     }
 }
@@ -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);
@@ -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);
         }
     }
@@ -124,16 +126,16 @@  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;
         }
     }
@@ -141,7 +143,7 @@  ovn_extend_table_assign_id(struct ovn_extend_table *table, struct ds *ds)
     /* 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;
         }
     }
@@ -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;
diff --git a/ovn/lib/extend-table.h b/ovn/lib/extend-table.h
index d9ae549beaf5..e4501f254fb7 100644
--- a/ovn/lib/extend-table.h
+++ b/ovn/lib/extend-table.h
@@ -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"
 
@@ -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. */
@@ -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