[ovs-dev,v2,1/3] OVN: introduce Controller_Event table
diff mbox series

Message ID 5b98c53626870def25bf96042f8e632d6e749d64.1562858727.git.lorenzo.bianconi@redhat.com
State New
Headers show
Series
  • OVN: add Controller Events
Related show

Commit Message

Lorenzo Bianconi July 11, 2019, 3:48 p.m. UTC
Add Controller_Event table to OVN SBDB in order to
report CMS related event.
Introduce event_table hashmap array and controller_event related
structures to ovn-controller in order to track pending events
forwarded by ovs-vswitchd. Moreover integrate event_table hashmap
array with event_table ovn-sbdb table

Signed-off-by: Mark Michelson <mmichels@redhat.com>
Co-authored-by: Mark Michelson <mmichels@redhat.com>
Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
---
 include/ovn/logical-fields.h    |   7 ++
 ovn/controller/ovn-controller.c |  10 +++
 ovn/controller/pinctrl.c        | 151 ++++++++++++++++++++++++++++++++
 ovn/controller/pinctrl.h        |   2 +
 ovn/lib/logical-fields.c        |  21 +++++
 ovn/ovn-sb.ovsschema            |  20 ++++-
 ovn/ovn-sb.xml                  |  40 +++++++++
 7 files changed, 248 insertions(+), 3 deletions(-)

Comments

0-day Robot July 11, 2019, 4:59 p.m. UTC | #1
Bleep bloop.  Greetings Lorenzo Bianconi, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


build:
mv -f $depbase.Tpo $depbase.Po
depbase=`echo ovn/controller/ha-chassis.o | sed 's|[^/]*$|.deps/&|;s|\.o$||'`;\
gcc -std=gnu99 -DHAVE_CONFIG_H -I.    -I ./include -I ./include -I ./lib -I ./lib    -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror   -g -O2 -MT ovn/controller/ha-chassis.o -MD -MP -MF $depbase.Tpo -c -o ovn/controller/ha-chassis.o ovn/controller/ha-chassis.c &&\
mv -f $depbase.Tpo $depbase.Po
depbase=`echo ovn/controller/lflow.o | sed 's|[^/]*$|.deps/&|;s|\.o$||'`;\
gcc -std=gnu99 -DHAVE_CONFIG_H -I.    -I ./include -I ./include -I ./lib -I ./lib    -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror   -g -O2 -MT ovn/controller/lflow.o -MD -MP -MF $depbase.Tpo -c -o ovn/controller/lflow.o ovn/controller/lflow.c &&\
mv -f $depbase.Tpo $depbase.Po
depbase=`echo ovn/controller/lport.o | sed 's|[^/]*$|.deps/&|;s|\.o$||'`;\
gcc -std=gnu99 -DHAVE_CONFIG_H -I.    -I ./include -I ./include -I ./lib -I ./lib    -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror   -g -O2 -MT ovn/controller/lport.o -MD -MP -MF $depbase.Tpo -c -o ovn/controller/lport.o ovn/controller/lport.c &&\
mv -f $depbase.Tpo $depbase.Po
depbase=`echo ovn/controller/ofctrl.o | sed 's|[^/]*$|.deps/&|;s|\.o$||'`;\
gcc -std=gnu99 -DHAVE_CONFIG_H -I.    -I ./include -I ./include -I ./lib -I ./lib    -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror   -g -O2 -MT ovn/controller/ofctrl.o -MD -MP -MF $depbase.Tpo -c -o ovn/controller/ofctrl.o ovn/controller/ofctrl.c &&\
mv -f $depbase.Tpo $depbase.Po
depbase=`echo ovn/controller/pinctrl.o | sed 's|[^/]*$|.deps/&|;s|\.o$||'`;\
gcc -std=gnu99 -DHAVE_CONFIG_H -I.    -I ./include -I ./include -I ./lib -I ./lib    -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror   -g -O2 -MT ovn/controller/pinctrl.o -MD -MP -MF $depbase.Tpo -c -o ovn/controller/pinctrl.o ovn/controller/pinctrl.c &&\
mv -f $depbase.Tpo $depbase.Po
ovn/controller/pinctrl.c:289:1: error: ‘pinctrl_find_empty_lb_backends_event’ defined but not used [-Werror=unused-function]
 pinctrl_find_empty_lb_backends_event(char *vip, char *protocol,
 ^
cc1: all warnings being treated as errors
make[2]: *** [ovn/controller/pinctrl.o] Error 1
make[2]: Leaving directory `/var/lib/jenkins/jobs/upstream_build_from_pw/workspace'
make[1]: *** [all-recursive] Error 1
make[1]: Leaving directory `/var/lib/jenkins/jobs/upstream_build_from_pw/workspace'
make: *** [all] Error 2


Please check this out.  If you feel there has been an error, please email aconole@bytheb.org

Thanks,
0-day Robot

Patch
diff mbox series

diff --git a/include/ovn/logical-fields.h b/include/ovn/logical-fields.h
index 164b338b5..9bac8e027 100644
--- a/include/ovn/logical-fields.h
+++ b/include/ovn/logical-fields.h
@@ -20,6 +20,11 @@ 
 
 struct shash;
 
+enum ovn_controller_event {
+    OVN_EVENT_EMPTY_LB_BACKENDS = 0,
+    OVN_EVENT_MAX,
+};
+
 /* Logical fields.
  *
  * These values are documented in ovn-architecture(7), please update the
@@ -118,6 +123,8 @@  ovn_field_from_id(enum ovn_field_id id)
     return &ovn_fields[id];
 }
 
+const char *event_to_string(enum ovn_controller_event event);
+int string_to_event(const char *s);
 const struct ovn_field *ovn_field_from_name(const char *name);
 void ovn_destroy_ovnfields(void);
 #endif /* ovn/lib/logical-fields.h */
diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
index c4883aa6d..1a90b702e 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -133,6 +133,8 @@  update_sb_monitors(struct ovsdb_idl *ovnsb_idl,
      * Monitor Logical_Flow, MAC_Binding, Multicast_Group, and DNS tables for
      * local datapaths.
      *
+     * Monitor Controller_Event rows for local chassis.
+     *
      * We always monitor patch ports because they allow us to see the linkages
      * between related logical datapaths.  That way, when we know that we have
      * a VIF on a particular logical switch, we immediately know to monitor all
@@ -142,6 +144,7 @@  update_sb_monitors(struct ovsdb_idl *ovnsb_idl,
     struct ovsdb_idl_condition mb = OVSDB_IDL_CONDITION_INIT(&mb);
     struct ovsdb_idl_condition mg = OVSDB_IDL_CONDITION_INIT(&mg);
     struct ovsdb_idl_condition dns = OVSDB_IDL_CONDITION_INIT(&dns);
+    struct ovsdb_idl_condition ce =  OVSDB_IDL_CONDITION_INIT(&ce);
     sbrec_port_binding_add_clause_type(&pb, OVSDB_F_EQ, "patch");
     /* XXX: We can optimize this, if we find a way to only monitor
      * ports that have a Gateway_Chassis that point's to our own
@@ -165,6 +168,9 @@  update_sb_monitors(struct ovsdb_idl *ovnsb_idl,
         sbrec_port_binding_add_clause_options(&pb, OVSDB_F_INCLUDES, &l2);
         const struct smap l3 = SMAP_CONST1(&l3, "l3gateway-chassis", id);
         sbrec_port_binding_add_clause_options(&pb, OVSDB_F_INCLUDES, &l3);
+
+        sbrec_controller_event_add_clause_chassis(&ce, OVSDB_F_EQ,
+                                                  &chassis->header_.uuid);
     }
     if (local_ifaces) {
         const char *name;
@@ -191,11 +197,13 @@  update_sb_monitors(struct ovsdb_idl *ovnsb_idl,
     sbrec_mac_binding_set_condition(ovnsb_idl, &mb);
     sbrec_multicast_group_set_condition(ovnsb_idl, &mg);
     sbrec_dns_set_condition(ovnsb_idl, &dns);
+    sbrec_controller_event_set_condition(ovnsb_idl, &ce);
     ovsdb_idl_condition_destroy(&pb);
     ovsdb_idl_condition_destroy(&lf);
     ovsdb_idl_condition_destroy(&mb);
     ovsdb_idl_condition_destroy(&mg);
     ovsdb_idl_condition_destroy(&dns);
+    ovsdb_idl_condition_destroy(&ce);
 }
 
 static const char *
@@ -1981,6 +1989,8 @@  main(int argc, char *argv[])
                                 sbrec_port_binding_by_name,
                                 sbrec_mac_binding_by_lport_ip,
                                 sbrec_dns_table_get(ovnsb_idl_loop.idl),
+                                sbrec_controller_event_table_get(
+                                    ovnsb_idl_loop.idl),
                                 br_int, chassis,
                                 &ed_runtime_data.local_datapaths,
                                 &ed_runtime_data.active_tunnels);
diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
index a442738a0..b8ed375fe 100644
--- a/ovn/controller/pinctrl.c
+++ b/ovn/controller/pinctrl.c
@@ -226,6 +226,153 @@  static bool may_inject_pkts(void);
 
 COVERAGE_DEFINE(pinctrl_drop_put_mac_binding);
 COVERAGE_DEFINE(pinctrl_drop_buffered_packets_map);
+COVERAGE_DEFINE(pinctrl_drop_controller_event);
+
+struct empty_lb_backends_event {
+    struct hmap_node hmap_node;
+    long long int timestamp;
+
+    char *vip;
+    char *protocol;
+    char *load_balancer;
+};
+
+static struct hmap event_table[OVN_EVENT_MAX];
+static int64_t event_seq_num;
+
+static void
+init_event_table(void)
+{
+    for (size_t i = 0; i < OVN_EVENT_MAX; i++) {
+        hmap_init(&event_table[i]);
+    }
+}
+
+#define EVENT_TIMEOUT   10000
+static void
+empty_lb_backends_event_gc(bool flush)
+{
+    struct empty_lb_backends_event *cur_ce, *next_ce;
+    long long int now = time_msec();
+
+    HMAP_FOR_EACH_SAFE (cur_ce, next_ce, hmap_node,
+                        &event_table[OVN_EVENT_EMPTY_LB_BACKENDS]) {
+        if ((now < cur_ce->timestamp + EVENT_TIMEOUT) && !flush) {
+            continue;
+        }
+
+        free(cur_ce->vip);
+        free(cur_ce->protocol);
+        free(cur_ce->load_balancer);
+        hmap_remove(&event_table[OVN_EVENT_EMPTY_LB_BACKENDS],
+                    &cur_ce->hmap_node);
+        free(cur_ce);
+    }
+}
+
+static void
+event_table_gc(bool flush)
+{
+    empty_lb_backends_event_gc(flush);
+}
+
+static void
+event_table_destroy(void)
+{
+    event_table_gc(true);
+    for (size_t i = 0; i < OVN_EVENT_MAX; i++) {
+        hmap_destroy(&event_table[i]);
+    }
+}
+
+static struct empty_lb_backends_event *
+pinctrl_find_empty_lb_backends_event(char *vip, char *protocol,
+                                     char *load_balancer, uint32_t hash)
+{
+    struct empty_lb_backends_event *ce;
+    HMAP_FOR_EACH_WITH_HASH (ce, hmap_node, hash,
+                             &event_table[OVN_EVENT_EMPTY_LB_BACKENDS]) {
+        if (!strcmp(ce->vip, vip) &&
+            !strcmp(ce->protocol, protocol) &&
+            !strcmp(ce->load_balancer, load_balancer)) {
+            return ce;
+        }
+    }
+    return NULL;
+}
+
+static const struct sbrec_controller_event *
+empty_lb_backends_lookup(struct empty_lb_backends_event *event,
+                         const struct sbrec_controller_event_table *ce_table,
+                         const struct sbrec_chassis *chassis)
+{
+    const struct sbrec_controller_event *sbrec_event;
+    const char *event_type = event_to_string(OVN_EVENT_EMPTY_LB_BACKENDS);
+    char ref_uuid[UUID_LEN + 1];
+    sprintf(ref_uuid, UUID_FMT, UUID_ARGS(&chassis->header_.uuid));
+
+    SBREC_CONTROLLER_EVENT_TABLE_FOR_EACH (sbrec_event, ce_table) {
+        if (strcmp(sbrec_event->event_type, event_type)) {
+            continue;
+        }
+
+        char chassis_uuid[UUID_LEN + 1];
+        sprintf(chassis_uuid, UUID_FMT,
+                UUID_ARGS(&sbrec_event->chassis->header_.uuid));
+        if (strcmp(ref_uuid, chassis_uuid)) {
+            continue;
+        }
+
+        const char *vip = smap_get(&sbrec_event->event_info, "vip");
+        const char *protocol = smap_get(&sbrec_event->event_info, "protocol");
+        const char *load_balancer = smap_get(&sbrec_event->event_info,
+                                             "load_balancer");
+
+        if (!strcmp(event->vip, vip) &&
+            !strcmp(event->protocol, protocol) &&
+            !strcmp(event->load_balancer, load_balancer)) {
+            return sbrec_event;
+        }
+    }
+
+    return NULL;
+}
+
+static void
+controller_event_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
+                     const struct sbrec_controller_event_table *ce_table,
+                     const struct sbrec_chassis *chassis)
+    OVS_REQUIRES(pinctrl_mutex)
+{
+    if (!ovnsb_idl_txn) {
+        goto out;
+    }
+
+    struct empty_lb_backends_event *empty_lbs;
+    HMAP_FOR_EACH (empty_lbs, hmap_node,
+                   &event_table[OVN_EVENT_EMPTY_LB_BACKENDS]) {
+        const struct sbrec_controller_event *event;
+
+        event = empty_lb_backends_lookup(empty_lbs, ce_table, chassis);
+        if (!event) {
+            struct smap event_info = SMAP_INITIALIZER(&event_info);
+
+            smap_add(&event_info, "vip", empty_lbs->vip);
+            smap_add(&event_info, "protocol", empty_lbs->protocol);
+            smap_add(&event_info, "load_balancer", empty_lbs->load_balancer);
+
+            event = sbrec_controller_event_insert(ovnsb_idl_txn);
+            sbrec_controller_event_set_event_type(event,
+                    event_to_string(OVN_EVENT_EMPTY_LB_BACKENDS));
+            sbrec_controller_event_set_seq_num(event, ++event_seq_num);
+            sbrec_controller_event_set_event_info(event, &event_info);
+            sbrec_controller_event_set_chassis(event, chassis);
+        }
+    }
+
+out:
+    event_table_gc(!!ovnsb_idl_txn);
+}
 
 void
 pinctrl_init(void)
@@ -234,6 +381,7 @@  pinctrl_init(void)
     init_send_garps();
     init_ipv6_ras();
     init_buffered_packets_map();
+    init_event_table();
     pinctrl.br_int_name = NULL;
     pinctrl_handler_seq = seq_create();
     pinctrl_main_seq = seq_create();
@@ -1897,6 +2045,7 @@  pinctrl_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
             struct ovsdb_idl_index *sbrec_port_binding_by_name,
             struct ovsdb_idl_index *sbrec_mac_binding_by_lport_ip,
             const struct sbrec_dns_table *dns_table,
+            const struct sbrec_controller_event_table *ce_table,
             const struct ovsrec_bridge *br_int,
             const struct sbrec_chassis *chassis,
             const struct hmap *local_datapaths,
@@ -1922,6 +2071,7 @@  pinctrl_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
     prepare_ipv6_ras(sbrec_port_binding_by_datapath,
                      sbrec_port_binding_by_name, local_datapaths);
     sync_dns_cache(dns_table);
+    controller_event_run(ovnsb_idl_txn, ce_table, chassis);
     run_buffered_binding(sbrec_port_binding_by_datapath,
                          sbrec_mac_binding_by_lport_ip,
                          local_datapaths);
@@ -2270,6 +2420,7 @@  pinctrl_destroy(void)
     destroy_send_garps();
     destroy_ipv6_ras();
     destroy_buffered_packets_map();
+    event_table_destroy();
     destroy_put_mac_bindings();
     destroy_dns_cache();
     seq_destroy(pinctrl_main_seq);
diff --git a/ovn/controller/pinctrl.h b/ovn/controller/pinctrl.h
index f61d7056e..fdef27a6d 100644
--- a/ovn/controller/pinctrl.h
+++ b/ovn/controller/pinctrl.h
@@ -29,6 +29,7 @@  struct ovsdb_idl_txn;
 struct ovsrec_bridge;
 struct sbrec_chassis;
 struct sbrec_dns_table;
+struct sbrec_controller_event_table;
 
 void pinctrl_init(void);
 void pinctrl_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
@@ -38,6 +39,7 @@  void pinctrl_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
                  struct ovsdb_idl_index *sbrec_port_binding_by_name,
                  struct ovsdb_idl_index *sbrec_mac_binding_by_lport_ip,
                  const struct sbrec_dns_table *,
+                 const struct sbrec_controller_event_table *,
                  const struct ovsrec_bridge *, const struct sbrec_chassis *,
                  const struct hmap *local_datapaths,
                  const struct sset *active_tunnels);
diff --git a/ovn/lib/logical-fields.c b/ovn/lib/logical-fields.c
index 579537d6b..adcbbe02d 100644
--- a/ovn/lib/logical-fields.c
+++ b/ovn/lib/logical-fields.c
@@ -225,6 +225,27 @@  ovn_init_symtab(struct shash *symtab)
     expr_symtab_add_ovn_field(symtab, "icmp4.frag_mtu", OVN_ICMP4_FRAG_MTU);
 }
 
+const char *
+event_to_string(enum ovn_controller_event event)
+{
+    switch (event) {
+    case OVN_EVENT_EMPTY_LB_BACKENDS:
+        return "empty_lb_backends";
+    case OVN_EVENT_MAX:
+    default:
+        return "";
+    }
+}
+
+int
+string_to_event(const char *s)
+{
+    if (!strcmp(s, "empty_lb_backends")) {
+        return OVN_EVENT_EMPTY_LB_BACKENDS;
+    }
+    return -1;
+}
+
 const struct ovn_field *
 ovn_field_from_name(const char *name)
 {
diff --git a/ovn/ovn-sb.ovsschema b/ovn/ovn-sb.ovsschema
index 2b543c6f5..eced7c4ed 100644
--- a/ovn/ovn-sb.ovsschema
+++ b/ovn/ovn-sb.ovsschema
@@ -1,7 +1,7 @@ 
 {
     "name": "OVN_Southbound",
-    "version": "2.3.0",
-    "cksum": "3092285199 17409",
+    "version": "2.4.0",
+    "cksum": "1795697952 18106",
     "tables": {
         "SB_Global": {
             "columns": {
@@ -349,4 +349,18 @@ 
                     "type": {"key": "string", "value": "string",
                              "min": 0, "max": "unlimited"}}},
             "indexes": [["name"]],
-            "isRoot": true}}}
+            "isRoot": true},
+        "Controller_Event": {
+            "columns": {
+                "event_type": {"type": {"key": {"type": "string",
+                               "enum": ["set", ["empty_lb_backends"]]}}},
+                "event_info": {"type": {"key": "string", "value": "string",
+                               "min": 0, "max": "unlimited"}},
+                "chassis": {"type": {"key": {"type": "uuid",
+                                             "refTable": "Chassis",
+                                             "refType": "weak"},
+                                     "min": 0, "max": 1}},
+                "seq_num": {"type": {"key": "integer"}}
+            },
+            "isRoot": true
+         }}}
diff --git a/ovn/ovn-sb.xml b/ovn/ovn-sb.xml
index c2faa2c5f..91f867ffb 100644
--- a/ovn/ovn-sb.xml
+++ b/ovn/ovn-sb.xml
@@ -3494,4 +3494,44 @@  tcp.flags = RST;
       </column>
     </group>
   </table>
+  <table name="Controller_Event" title="Controller Event table">
+    <p>
+      Database table used by <code>ovn-controller</code> to report CMS
+      related events. Please note there is no guarantee a given event is
+      written exactly once in the db. It is CMS responsibility to squash
+      duplicated lines or to filter out duplicated events
+    </p>
+    <column name="event_type">
+      Event type occurred
+    </column>
+    <column name="event_info">
+    <p>
+      Key-value pairs used to specify event info to the CMS.
+      Possible values are:
+    </p>
+      <ul>
+        <li>
+         <code>vip</code>: VIP reported for the <code>empty_lb_backends</code>
+         event
+        </li>
+        <li>
+          <code>protocol</code>: Transport protocol reported for the
+          <code>empty_lb_backends</code> event
+        </li>
+        <li>
+          <code>load_balancer</code>: UUID of the load balancer reported for
+          the <code>empty_lb_backends</code> event
+        </li>
+      </ul>
+    </column>
+    <column name="chassis">
+      This column is a <ref table="Chassis"/> record to identify the chassis
+      that has managed a given event.
+    </column>
+    <column name="seq_num">
+      Event sequence number. Global counter for controller generated events.
+      It can be used by the CMS to detect possible duplication of the same
+      event.
+    </column>
+  </table>
 </database>