From b5eb3b185fb04bfed3ba6c89323cd82e7d666bae Mon Sep 17 00:00:00 2001
From: Mohsin Kazmi <sykazmi@cisco.com>
Date: Mon, 26 Feb 2018 18:36:17 +0100
Subject: VOM: Fix state reconciliation

This commit also fixes the acl and arp handle for
inspector to view internal state of VOM.

Change-Id: Ibc8ff6cb51d2a77b4c04993ac7212564b8892337
Signed-off-by: Mohsin Kazmi <sykazmi@cisco.com>
---
 src/vpp-api/vom/acl_binding.cpp       | 14 ++++++++
 src/vpp-api/vom/acl_binding.hpp       | 18 +++++++----
 src/vpp-api/vom/acl_binding_cmds.cpp  | 60 +++++++++++++++++++++++++++++++++--
 src/vpp-api/vom/acl_binding_cmds.hpp  | 20 ++----------
 src/vpp-api/vom/acl_list.cpp          | 19 +++++++++--
 src/vpp-api/vom/acl_list.hpp          | 40 ++++++++++++++---------
 src/vpp-api/vom/acl_list_cmds.cpp     |  9 +++++-
 src/vpp-api/vom/acl_list_cmds.hpp     | 43 ++++++++++++++-----------
 src/vpp-api/vom/arp_proxy_binding.cpp |  3 +-
 src/vpp-api/vom/arp_proxy_config.cpp  |  3 +-
 src/vpp-api/vom/om.cpp                |  3 +-
 11 files changed, 165 insertions(+), 67 deletions(-)

(limited to 'src/vpp-api')

diff --git a/src/vpp-api/vom/acl_binding.cpp b/src/vpp-api/vom/acl_binding.cpp
index b87cfbaa2e4..73f015df377 100644
--- a/src/vpp-api/vom/acl_binding.cpp
+++ b/src/vpp-api/vom/acl_binding.cpp
@@ -18,6 +18,13 @@
 
 namespace VOM {
 namespace ACL {
+template <>
+l2_binding::event_handler::event_handler()
+{
+  OM::register_listener(this);
+  inspect::register_handler({ "l2-acl-binding" }, "L2 ACL bindings", this);
+}
+
 template <>
 void
 l2_binding::event_handler::handle_populate(const client_db::key_t& key)
@@ -46,6 +53,13 @@ l2_binding::event_handler::handle_populate(const client_db::key_t& key)
   }
 }
 
+template <>
+l3_binding::event_handler::event_handler()
+{
+  OM::register_listener(this);
+  inspect::register_handler({ "l3-acl-binding" }, "L3 ACL bindings", this);
+}
+
 template <>
 void
 l3_binding::event_handler::handle_populate(const client_db::key_t& key)
diff --git a/src/vpp-api/vom/acl_binding.hpp b/src/vpp-api/vom/acl_binding.hpp
index 3687503ef06..6e994e4d2a5 100644
--- a/src/vpp-api/vom/acl_binding.hpp
+++ b/src/vpp-api/vom/acl_binding.hpp
@@ -49,7 +49,7 @@ public:
     : m_direction(direction)
     , m_itf(itf.singular())
     , m_acl(acl.singular())
-    , m_binding(0)
+    , m_binding(false)
   {
     m_evh.order();
   }
@@ -61,7 +61,7 @@ public:
     : m_direction(o.m_direction)
     , m_itf(o.m_itf)
     , m_acl(o.m_acl)
-    , m_binding(0)
+    , m_binding(o.m_binding)
   {
   }
 
@@ -97,6 +97,8 @@ public:
    */
   static void dump(std::ostream& os) { m_db.dump(os); }
 
+  static dependency_t order() { return m_evh.order(); }
+
 private:
   /**
    * Class definition for listeners to OM events
@@ -104,11 +106,8 @@ private:
   class event_handler : public OM::listener, public inspect::command_handler
   {
   public:
-    event_handler()
-    {
-      OM::register_listener(this);
-      inspect::register_handler({ "acl-binding" }, "ACL bindings", this);
-    }
+    event_handler();
+
     virtual ~event_handler() = default;
 
     /**
@@ -222,6 +221,11 @@ singular_db<typename ACL::binding<LIST>::key_t, ACL::binding<LIST>>
 
 template <typename LIST>
 typename ACL::binding<LIST>::event_handler binding<LIST>::m_evh;
+
+namespace {
+const static dependency_t __attribute__((unused)) l2o = l2_binding::order();
+const static dependency_t __attribute__((unused)) l3o = l3_binding::order();
+};
 };
 
 std::ostream& operator<<(std::ostream& os,
diff --git a/src/vpp-api/vom/acl_binding_cmds.cpp b/src/vpp-api/vom/acl_binding_cmds.cpp
index 534f7868a08..8c33cd47b79 100644
--- a/src/vpp-api/vom/acl_binding_cmds.cpp
+++ b/src/vpp-api/vom/acl_binding_cmds.cpp
@@ -39,6 +39,17 @@ l3_bind_cmd::issue(connection& con)
   return rc_t::OK;
 }
 
+template <>
+std::string
+l3_bind_cmd::to_string() const
+{
+  std::ostringstream s;
+  s << "l3-acl-bind:[" << m_direction.to_string()
+    << " itf:" << m_itf.to_string() << " acl:" << m_acl.to_string() << "]";
+
+  return (s.str());
+}
+
 template <>
 rc_t
 l3_unbind_cmd::issue(connection& con)
@@ -58,6 +69,17 @@ l3_unbind_cmd::issue(connection& con)
   return rc_t::OK;
 }
 
+template <>
+std::string
+l3_unbind_cmd::to_string() const
+{
+  std::ostringstream s;
+  s << "l3-acl-unbind:[" << m_direction.to_string()
+    << " itf:" << m_itf.to_string() << " acl:" << m_acl.to_string() << "]";
+
+  return (s.str());
+}
+
 template <>
 rc_t
 l3_dump_cmd::issue(connection& con)
@@ -74,6 +96,13 @@ l3_dump_cmd::issue(connection& con)
   return rc_t::OK;
 }
 
+template <>
+std::string
+l3_dump_cmd::to_string() const
+{
+  return ("l3-acl-bind-dump");
+}
+
 template <>
 rc_t
 l2_bind_cmd::issue(connection& con)
@@ -83,7 +112,6 @@ l2_bind_cmd::issue(connection& con)
   auto& payload = req.get_request().get_payload();
   payload.sw_if_index = m_itf.value();
   payload.is_add = 1;
-  // payload.is_input = (m_direction == direction_t::INPUT ? 1 : 0);
   payload.acl_index = m_acl.value();
 
   VAPI_CALL(req.execute());
@@ -93,6 +121,17 @@ l2_bind_cmd::issue(connection& con)
   return rc_t::OK;
 }
 
+template <>
+std::string
+l2_bind_cmd::to_string() const
+{
+  std::ostringstream s;
+  s << "l2-acl-bind:[" << m_direction.to_string()
+    << " itf:" << m_itf.to_string() << " acl:" << m_acl.to_string() << "]";
+
+  return (s.str());
+}
+
 template <>
 rc_t
 l2_unbind_cmd::issue(connection& con)
@@ -102,7 +141,6 @@ l2_unbind_cmd::issue(connection& con)
   auto& payload = req.get_request().get_payload();
   payload.sw_if_index = m_itf.value();
   payload.is_add = 0;
-  // payload.is_input = (m_direction == direction_t::INPUT ? 1 : 0);
   payload.acl_index = m_acl.value();
 
   VAPI_CALL(req.execute());
@@ -112,6 +150,17 @@ l2_unbind_cmd::issue(connection& con)
   return rc_t::OK;
 }
 
+template <>
+std::string
+l2_unbind_cmd::to_string() const
+{
+  std::ostringstream s;
+  s << "l2-acl-unbind:[" << m_direction.to_string()
+    << " itf:" << m_itf.to_string() << " acl:" << m_acl.to_string() << "]";
+
+  return (s.str());
+}
+
 template <>
 rc_t
 l2_dump_cmd::issue(connection& con)
@@ -128,6 +177,13 @@ l2_dump_cmd::issue(connection& con)
   return rc_t::OK;
 }
 
+template <>
+std::string
+l2_dump_cmd::to_string() const
+{
+  return ("l2-acl-bind-dump");
+}
+
 }; // namespace binding_cmds
 }; // namespace ACL
 }; // namespace VOM
diff --git a/src/vpp-api/vom/acl_binding_cmds.hpp b/src/vpp-api/vom/acl_binding_cmds.hpp
index 4e8895acb3c..b9af66e70c8 100644
--- a/src/vpp-api/vom/acl_binding_cmds.hpp
+++ b/src/vpp-api/vom/acl_binding_cmds.hpp
@@ -54,14 +54,7 @@ public:
   /**
    * convert to string format for debug purposes
    */
-  std::string to_string() const
-  {
-    std::ostringstream s;
-    s << "acl-bind:[" << m_direction.to_string() << " itf:" << m_itf.to_string()
-      << " acl:" << m_acl.to_string() << "]";
-
-    return (s.str());
-  }
+  std::string to_string() const;
 
   /**
    * Comparison operator - only used for UT
@@ -117,14 +110,7 @@ public:
   /**
    * convert to string format for debug purposes
    */
-  std::string to_string() const
-  {
-    std::ostringstream s;
-    s << "acl-unbind:[" << m_direction.to_string()
-      << " itf:" << m_itf.to_string() << " acl:" << m_acl.to_string() << "]";
-
-    return (s.str());
-  }
+  std::string to_string() const;
 
   /**
    * Comparison operator - only used for UT
@@ -171,7 +157,7 @@ public:
   /**
    * convert to string format for debug purposes
    */
-  std::string to_string() const { return ("acl-bind-dump"); }
+  std::string to_string() const;
 
 private:
   /**
diff --git a/src/vpp-api/vom/acl_list.cpp b/src/vpp-api/vom/acl_list.cpp
index 557de515714..5b03f5db021 100644
--- a/src/vpp-api/vom/acl_list.cpp
+++ b/src/vpp-api/vom/acl_list.cpp
@@ -19,6 +19,14 @@
 
 namespace VOM {
 namespace ACL {
+
+template <>
+l2_list::event_handler::event_handler()
+{
+  OM::register_listener(this);
+  inspect::register_handler({ "l2-acl-list" }, "L2 ACL lists", this);
+}
+
 template <>
 void
 l2_list::event_handler::handle_populate(const client_db::key_t& key)
@@ -61,6 +69,13 @@ l2_list::event_handler::handle_populate(const client_db::key_t& key)
   }
 }
 
+template <>
+l3_list::event_handler::event_handler()
+{
+  OM::register_listener(this);
+  inspect::register_handler({ "l3-acl-list" }, "L3 ACL lists", this);
+}
+
 template <>
 void
 l3_list::event_handler::handle_populate(const client_db::key_t& key)
@@ -120,7 +135,7 @@ l3_list::update(const l3_list& obj)
   /*
    * always update the instance with the latest rule set
    */
-  if (!m_hdl || obj.m_rules != m_rules) {
+  if (rc_t::OK != m_hdl.rc() || obj.m_rules != m_rules) {
     HW::enqueue(new list_cmds::l3_update_cmd(m_hdl, m_key, m_rules));
   }
   /*
@@ -137,7 +152,7 @@ l2_list::update(const l2_list& obj)
   /*
    * always update the instance with the latest rule set
    */
-  if (!m_hdl || obj.m_rules != m_rules) {
+  if (rc_t::OK != m_hdl.rc() || obj.m_rules != m_rules) {
     HW::enqueue(new list_cmds::l2_update_cmd(m_hdl, m_key, m_rules));
   }
   /*
diff --git a/src/vpp-api/vom/acl_list.hpp b/src/vpp-api/vom/acl_list.hpp
index ff3eeeb21b2..bd84ce1ea43 100644
--- a/src/vpp-api/vom/acl_list.hpp
+++ b/src/vpp-api/vom/acl_list.hpp
@@ -53,7 +53,8 @@ public:
    * Construct a new object matching the desried state
    */
   list(const key_t& key)
-    : m_key(key)
+    : m_hdl(handle_t::INVALID)
+    , m_key(key)
   {
   }
 
@@ -64,10 +65,10 @@ public:
   }
 
   list(const key_t& key, const rules_t& rules)
-    : m_key(key)
+    : m_hdl(handle_t::INVALID)
+    , m_key(key)
     , m_rules(rules)
   {
-    m_evh.order();
   }
 
   /**
@@ -129,7 +130,7 @@ public:
   /**
    * Return the VPP assign handle
    */
-  const handle_t& handle() const { return m_hdl.data(); }
+  const handle_t& handle() const { return (singular()->handle_i()); }
 
   static std::shared_ptr<list> find(const handle_t& handle)
   {
@@ -141,12 +142,19 @@ public:
     return (m_db.find(key));
   }
 
-  static void add(const handle_t& handle, std::shared_ptr<list> sp)
+  static void add(const key_t& key, const HW::item<handle_t>& item)
   {
-    m_hdl_db[handle] = sp;
+    std::shared_ptr<list> sp = find(key);
+
+    if (sp && item) {
+      m_hdl_db[item.data()] = sp;
+    }
   }
 
-  static void remove(const handle_t& handle) { m_hdl_db.erase(handle); }
+  static void remove(const HW::item<handle_t>& item)
+  {
+    m_hdl_db.erase(item.data());
+  }
 
   const key_t& key() const { return m_key; }
 
@@ -167,11 +175,8 @@ private:
   class event_handler : public OM::listener, public inspect::command_handler
   {
   public:
-    event_handler()
-    {
-      OM::register_listener(this);
-      inspect::register_handler({ "acl" }, "ACL lists", this);
-    }
+    event_handler();
+
     virtual ~event_handler() = default;
 
     /**
@@ -215,9 +220,14 @@ private:
    */
   static std::shared_ptr<list> find_or_add(const list& temp)
   {
-    return (m_db.find_or_add(temp.m_key, temp));
+    return (m_db.find_or_add(temp.key(), temp));
   }
 
+  /**
+   * return the acl-list's handle in the singular instance
+   */
+  const handle_t& handle_i() const { return (m_hdl.data()); }
+
   /*
    * It's the VOM::OM class that updates call update
    */
@@ -246,7 +256,7 @@ private:
   /**
    * A map of all ACLs keyed against VPP's handle
    */
-  static std::map<const handle_t, std::weak_ptr<list>> m_hdl_db;
+  static std::map<handle_t, std::weak_ptr<list>> m_hdl_db;
 
   /**
    * The Key is a user defined identifer for this ACL
@@ -279,7 +289,7 @@ singular_db<typename ACL::list<RULE>::key_t, ACL::list<RULE>> list<RULE>::m_db;
  * Definition of the static per-handle DB for ACL Lists
  */
 template <typename RULE>
-std::map<const handle_t, std::weak_ptr<ACL::list<RULE>>> list<RULE>::m_hdl_db;
+std::map<handle_t, std::weak_ptr<ACL::list<RULE>>> list<RULE>::m_hdl_db;
 
 template <typename RULE>
 typename ACL::list<RULE>::event_handler list<RULE>::m_evh;
diff --git a/src/vpp-api/vom/acl_list_cmds.cpp b/src/vpp-api/vom/acl_list_cmds.cpp
index 80df04ef145..2e59763f4fe 100644
--- a/src/vpp-api/vom/acl_list_cmds.cpp
+++ b/src/vpp-api/vom/acl_list_cmds.cpp
@@ -75,7 +75,8 @@ l3_update_cmd::issue(connection& con)
   VAPI_CALL(req.execute());
 
   m_hw_item = wait();
-  complete();
+  if (m_hw_item.rc() == rc_t::OK)
+    insert_acl();
 
   return rc_t::OK;
 }
@@ -94,6 +95,8 @@ l3_delete_cmd::issue(connection& con)
   wait();
   m_hw_item.set(rc_t::NOOP);
 
+  remove_acl();
+
   return rc_t::OK;
 }
 
@@ -138,6 +141,8 @@ l2_update_cmd::issue(connection& con)
   VAPI_CALL(req.execute());
 
   m_hw_item = wait();
+  if (m_hw_item.rc() == rc_t::OK)
+    insert_acl();
 
   return rc_t::OK;
 }
@@ -156,6 +161,8 @@ l2_delete_cmd::issue(connection& con)
   wait();
   m_hw_item.set(rc_t::NOOP);
 
+  remove_acl();
+
   return rc_t::OK;
 }
 
diff --git a/src/vpp-api/vom/acl_list_cmds.hpp b/src/vpp-api/vom/acl_list_cmds.hpp
index 23d77c7e26c..d24e75229de 100644
--- a/src/vpp-api/vom/acl_list_cmds.hpp
+++ b/src/vpp-api/vom/acl_list_cmds.hpp
@@ -76,18 +76,10 @@ public:
     return ((m_key == other.m_key) && (m_rules == other.m_rules));
   }
 
-  void complete()
-  {
-    std::shared_ptr<list<RULE>> sp = list<RULE>::find(m_key);
-    if (sp && this->item()) {
-      list<RULE>::add(this->item().data(), sp);
-    }
-  }
-
   void succeeded()
   {
     rpc_cmd<HW::item<handle_t>, HW::item<handle_t>, UPDATE>::succeeded();
-    complete();
+    list<RULE>::add(m_key, this->item());
   }
 
   /**
@@ -98,9 +90,13 @@ public:
     int acl_index = reply.get_response().get_payload().acl_index;
     int retval = reply.get_response().get_payload().retval;
 
-    VOM_LOG(log_level_t::DEBUG) << this->to_string() << " " << retval;
+    VOM_LOG(log_level_t::DEBUG) << this->to_string() << " retval:" << retval
+                                << " acl_index:" << acl_index;
 
-    HW::item<handle_t> res(acl_index, rc_t::from_vpp_retval(retval));
+    rc_t rc = rc_t::from_vpp_retval(retval);
+    handle_t handle(acl_index);
+
+    HW::item<handle_t> res(handle, rc);
 
     this->fulfill(res);
 
@@ -108,6 +104,11 @@ public:
   }
 
 private:
+  /**
+   * add the created acl to the DB
+   */
+  void insert_acl() { list<RULE>::add(m_key, this->item()); }
+
   /**
    * The key.
    */
@@ -122,7 +123,7 @@ private:
 /**
  * A cmd class that Deletes an ACL
  */
-template <typename DELETE>
+template <typename RULE, typename DELETE>
 class delete_cmd : public rpc_cmd<HW::item<handle_t>, rc_t, DELETE>
 {
 public:
@@ -157,6 +158,12 @@ public:
   {
     return (this->item().data() == other.item().data());
   }
+
+private:
+  /**
+   * remove the acl from the DB
+   */
+  void remove_acl() { list<RULE>::remove(this->item()); }
 };
 
 /**
@@ -170,37 +177,35 @@ public:
    * Constructor
    */
   dump_cmd() = default;
-  dump_cmd(const dump_cmd& d) = default;
 
   /**
    * Issue the command to VPP/HW
    */
-  rc_t issue(connection& con) { return rc_t::INVALID; }
+  rc_t issue(connection& con);
 
   /**
    * convert to string format for debug purposes
    */
   std::string to_string() const { return ("acl-list-dump"); }
 
-private:
   /**
-   * HW reutrn code
+   * Comparison operator - only used for UT
    */
-  HW::item<bool> item;
+  bool operator==(const dump_cmd& i) const { return true; }
 };
 
 /**
  * Typedef the L3 ACL commands
  */
 typedef update_cmd<l3_rule, vapi::Acl_add_replace> l3_update_cmd;
-typedef delete_cmd<vapi::Acl_del> l3_delete_cmd;
+typedef delete_cmd<l3_rule, vapi::Acl_del> l3_delete_cmd;
 typedef dump_cmd<vapi::Acl_dump> l3_dump_cmd;
 
 /**
  * Typedef the L2 ACL commands
  */
 typedef update_cmd<l2_rule, vapi::Macip_acl_add> l2_update_cmd;
-typedef delete_cmd<vapi::Macip_acl_del> l2_delete_cmd;
+typedef delete_cmd<l2_rule, vapi::Macip_acl_del> l2_delete_cmd;
 typedef dump_cmd<vapi::Macip_acl_dump> l2_dump_cmd;
 
 }; // namespace list_cmds
diff --git a/src/vpp-api/vom/arp_proxy_binding.cpp b/src/vpp-api/vom/arp_proxy_binding.cpp
index 407436cfc8f..7053390b7c7 100644
--- a/src/vpp-api/vom/arp_proxy_binding.cpp
+++ b/src/vpp-api/vom/arp_proxy_binding.cpp
@@ -109,7 +109,8 @@ arp_proxy_binding::singular() const
 arp_proxy_binding::event_handler::event_handler()
 {
   OM::register_listener(this);
-  inspect::register_handler({ "arp-proxy" }, "ARP proxy bindings", this);
+  inspect::register_handler({ "arp-proxy-binding" }, "ARP proxy bindings",
+                            this);
 }
 
 void
diff --git a/src/vpp-api/vom/arp_proxy_config.cpp b/src/vpp-api/vom/arp_proxy_config.cpp
index 5130ba8d850..c72b5f2e8b0 100644
--- a/src/vpp-api/vom/arp_proxy_config.cpp
+++ b/src/vpp-api/vom/arp_proxy_config.cpp
@@ -101,7 +101,8 @@ arp_proxy_config::singular() const
 arp_proxy_config::event_handler::event_handler()
 {
   OM::register_listener(this);
-  inspect::register_handler({ "arp-proxy" }, "ARP Proxy configurations", this);
+  inspect::register_handler({ "arp-proxy-config" }, "ARP Proxy configurations",
+                            this);
 }
 
 void
diff --git a/src/vpp-api/vom/om.cpp b/src/vpp-api/vom/om.cpp
index 5f5c26e4eea..5ac7e9da44b 100644
--- a/src/vpp-api/vom/om.cpp
+++ b/src/vpp-api/vom/om.cpp
@@ -87,9 +87,8 @@ OM::replay()
    */
   for (listener* l : *m_listeners) {
     l->handle_replay();
+    HW::write();
   }
-
-  HW::write();
 }
 
 void
-- 
cgit 1.2.3-korg