Handle display name change from empty to non-empty

Issue:
------
In certain configuration update scenarios we may receive
Physical Device or Physical Interface entry without display
name, however while translating this information to OVSDB
we need to mandatory have display name available, not
having display name available was not handled.

Fix:
----
Ignore logical interface object till display name for both
Physical Device and Interface is available, added change
to trigger updates to logical interface on display name
change using dependency tracker.

Fix propagation of Physical Device display name to Physical
Device VN as well.

Fixed test case to check for vxlan-id of ovs-logical-switch
validation

Added test case to simulate order in which display name for
Physical Device and Physical Interface is not available
during creation time.

Partial-Bug: 1471201
Change-Id: I958792a7c28da0e5019c5342dd1e7bdc473aae9d

src/vnsw/agent/oper/agent.sandesh

@@ -94,6 +94,8 @@ struct ItfSandeshData {
    44: string transport;
    45: string logical_interface_uuid;
    46: optional bool flood_unknown_unicast;
+   47: string physical_device;
+   48: string physical_interface;
 }
 
 struct VnIpamData {
@@ -1079,6 +1081,10 @@ trace sandesh MulticastLog {
     4: i32 label;   
 }
 
+trace sandesh OperTrace {
+    1: string message;
+}
+
 trace sandesh OperVrf {
     1: string message;
     2: string vrf_name;

src/vnsw/agent/oper/interface.cc

@@ -60,10 +60,15 @@ void InterfaceTable::RegisterDBClients(IFMapDependencyManager *dep) {
     typedef IFMapDependencyTracker::ReactionMap ReactionMap;
 
     ReactionMap physical_port_react = map_list_of<std::string, PropagateList>
-        ("self", list_of("self") ("physical-router-physical-interface"))
+        ("self",
+         list_of("self")
+                ("physical-router-physical-interface")
+                ("physical-interface-logical-interface"))
         ("physical-interface-logical-interface",
          list_of("physical-router-physical-interface"))
-        ("physical-router-physical-interface", list_of("self"));
+        ("physical-router-physical-interface",
+         list_of("self")
+                ("physical-interface-logical-interface"));
     dep->RegisterReactionMap("physical-interface", physical_port_react);
     dep->Register("physical-interface",
                   boost::bind(&AgentOperDBTable::ConfigEventHandler, this, _1));
@@ -73,7 +78,8 @@ void InterfaceTable::RegisterDBClients(IFMapDependencyManager *dep) {
     ReactionMap logical_port_react = map_list_of<std::string, PropagateList>
         ("self", list_of("self") ("physical-interface-logical-interface"))
         ("physical-interface-logical-interface",
-         list_of("physical-router-physical-interface"))
+         list_of("self")
+                ("physical-router-physical-interface"))
         ("logical-interface-virtual-machine-interface",
          list_of("physical-interface-logical-interface") ("self"));
     dep->RegisterReactionMap("logical-interface", logical_port_react);
@@ -729,8 +735,14 @@ void Interface::SetItfSandeshData(ItfSandeshData &data) const {
         break;
 
     case Interface::LOGICAL:
-        data.set_type("logical-port");
-        data.set_vrf_name("--NA--");
+        {
+            const LogicalInterface *lintf =
+                static_cast<const LogicalInterface*>(this);
+            data.set_type("logical-port");
+            data.set_vrf_name("--NA--");
+            data.set_physical_device(lintf->phy_dev_display_name());
+            data.set_physical_interface(lintf->phy_intf_display_name());
+        }
         break;
 
     case Interface::VM_INTERFACE: {

src/vnsw/agent/oper/logical_interface.cc

@@ -30,7 +30,8 @@ using boost::uuids::uuid;
 LogicalInterface::LogicalInterface(const boost::uuids::uuid &uuid,
                                    const std::string &name) :
     Interface(Interface::LOGICAL, uuid, name, NULL), display_name_(),
-    physical_interface_(), vm_interface_(), physical_device_(NULL) {
+    physical_interface_(), vm_interface_(), physical_device_(NULL),
+    phy_dev_display_name_(), phy_intf_display_name_() {
 }
 
 LogicalInterface::~LogicalInterface() {
@@ -72,6 +73,14 @@ bool LogicalInterface::OnChange(const InterfaceTable *table,
         ret = true;
     }
 
+    if (phy_intf_display_name_ != data->phy_intf_display_name_) {
+        OPER_TRACE(Trace, "Changing Physical Interface display name from "
+                   + phy_intf_display_name_ + " to " +
+                   data->phy_intf_display_name_);
+        phy_intf_display_name_ = data->phy_intf_display_name_;
+        ret = true;
+    }
+
     VmInterfaceKey vmi_key(AgentKey::ADD_DEL_CHANGE, data->vm_interface_, "");
     Interface *interface = static_cast<Interface *>
         (table->agent()->interface_table()->FindActiveEntry(&vmi_key));
@@ -88,6 +97,14 @@ bool LogicalInterface::OnChange(const InterfaceTable *table,
         ret = true;
     }
 
+    if (phy_dev_display_name_ != data->phy_dev_display_name_) {
+        OPER_TRACE(Trace, "Changing Physical Device display name from "
+                   + phy_dev_display_name_ + " to " +
+                   data->phy_dev_display_name_);
+        phy_dev_display_name_ = data->phy_dev_display_name_;
+        ret = true;
+    }
+
     return ret;
 }
 
@@ -129,10 +146,14 @@ LogicalInterfaceData::LogicalInterfaceData(Agent *agent, IFMapNode *node,
                                            const std::string &display_name,
                                            const std::string &port,
                                            const boost::uuids::uuid &vif,
-                                           const uuid &device_uuid) :
+                                           const uuid &device_uuid,
+                                           const std::string &phy_dev_display_name,
+                                           const std::string &phy_intf_display_name) :
     InterfaceData(agent, node, Interface::TRANSPORT_INVALID),
     display_name_(display_name),
-    physical_interface_(port), vm_interface_(vif), device_uuid_(device_uuid) {
+    physical_interface_(port), vm_interface_(vif), device_uuid_(device_uuid),
+    phy_dev_display_name_(phy_dev_display_name),
+    phy_intf_display_name_(phy_intf_display_name) {
 }
 
 LogicalInterfaceData::~LogicalInterfaceData() {
@@ -191,8 +212,11 @@ InterfaceKey *VlanLogicalInterfaceKey::Clone() const {
 VlanLogicalInterfaceData::VlanLogicalInterfaceData
 (Agent *agent, IFMapNode *node, const std::string &display_name,
  const std::string &physical_interface,
- const boost::uuids::uuid &vif, const boost::uuids::uuid &u, uint16_t vlan) :
-    LogicalInterfaceData(agent, node, display_name, physical_interface, vif, u),
+ const boost::uuids::uuid &vif, const boost::uuids::uuid &u,
+ const std::string &phy_dev_display_name,
+ const std::string &phy_intf_display_name, uint16_t vlan) :
+    LogicalInterfaceData(agent, node, display_name, physical_interface, vif, u,
+                         phy_dev_display_name, phy_intf_display_name),
     vlan_(vlan) {
 }
 
@@ -217,17 +241,24 @@ static LogicalInterfaceData *BuildData(Agent *agent, IFMapNode *node,
                                        const autogen::LogicalInterface *port) {
     // Find link with physical-interface adjacency
     string physical_interface;
+    string phy_dev_display_name;
+    string phy_intf_display_name;
     IFMapNode *adj_node = NULL;
     boost::uuids::uuid dev_uuid = nil_uuid();
     adj_node = agent->cfg_listener()->FindAdjacentIFMapNode
         (agent, node, "physical-interface");
     if (adj_node) {
         physical_interface = adj_node->name();
+        autogen::PhysicalInterface *port =
+            static_cast <autogen::PhysicalInterface *>(adj_node->GetObject());
+        assert(port);
+        phy_intf_display_name = port->display_name();
         IFMapNode *prouter_node = agent->cfg_listener()->FindAdjacentIFMapNode
             (agent, adj_node, "physical-router");
         if (prouter_node) {
             autogen::PhysicalRouter *router =
                 static_cast<autogen::PhysicalRouter *>(prouter_node->GetObject());
+            phy_dev_display_name = router->display_name();
             autogen::IdPermsType id_perms = router->id_perms();
             CfgUuidSet(id_perms.uuid.uuid_mslong, id_perms.uuid.uuid_lslong,
                        dev_uuid);
@@ -280,6 +311,8 @@ static LogicalInterfaceData *BuildData(Agent *agent, IFMapNode *node,
 
     return new VlanLogicalInterfaceData(agent, node, port->display_name(),
                                         physical_interface, vmi_uuid, dev_uuid,
+                                        phy_dev_display_name,
+                                        phy_intf_display_name,
                                         port->vlan_tag());
 }
 

src/vnsw/agent/oper/logical_interface.h

@@ -34,6 +34,8 @@ class LogicalInterface : public Interface {
                           const LogicalInterfaceData *data);
 
     const std::string &display_name() const { return display_name_; }
+    const std::string &phy_dev_display_name() const { return phy_dev_display_name_; }
+    const std::string &phy_intf_display_name() const { return phy_intf_display_name_; }
     Interface *physical_interface() const;
     VmInterface *vm_interface() const;
     PhysicalDevice *physical_device() const;
@@ -45,6 +47,8 @@ class LogicalInterface : public Interface {
     InterfaceRef vm_interface_;
     boost::uuids::uuid vm_uuid_;
     PhysicalDeviceRef physical_device_;
+    std::string phy_dev_display_name_;
+    std::string phy_intf_display_name_;
     DISALLOW_COPY_AND_ASSIGN(LogicalInterface);
 };
 
@@ -59,14 +63,18 @@ struct LogicalInterfaceData : public InterfaceData {
     LogicalInterfaceData(Agent *agent, IFMapNode *node,
                          const std::string &display_name,
                          const std::string &physical_interface,
-                        const boost::uuids::uuid &vif,
-                        const boost::uuids::uuid &device_uuid);
+                         const boost::uuids::uuid &vif,
+                         const boost::uuids::uuid &device_uuid,
+                         const std::string &phy_dev_display_name,
+                         const std::string &phy_intf_display_name);
     virtual ~LogicalInterfaceData();
 
     std::string display_name_;
     std::string physical_interface_;
     boost::uuids::uuid vm_interface_;
     boost::uuids::uuid device_uuid_;
+    std::string phy_dev_display_name_;
+    std::string phy_intf_display_name_;
 };
 
 struct VlanLogicalInterfaceKey : public LogicalInterfaceKey {
@@ -86,6 +94,8 @@ struct VlanLogicalInterfaceData : public LogicalInterfaceData {
                              const std::string &physical_interface,
                              const boost::uuids::uuid &vif,
                              const boost::uuids::uuid &device_uuid,
+                             const std::string &phy_dev_display_name,
+                             const std::string &phy_intf_display_name,
                              uint16_t vlan);
     virtual ~VlanLogicalInterfaceData();
 

src/vnsw/agent/oper/physical_device_vn.cc

@@ -65,6 +65,11 @@ bool PhysicalDeviceVn::Copy(PhysicalDeviceVnTable *table,
         ret = true;
     }
 
+    if (dev && (dev->name() != device_display_name_)) {
+        device_display_name_ = dev->name();
+        ret = true;
+    }
+
     VnEntry *vn = table->agent()->vn_table()->Find(vn_uuid_);
     if (vn != vn_.get()) {
         vn_.reset(vn);

src/vnsw/agent/oper/physical_device_vn.h

@@ -66,7 +66,7 @@ class PhysicalDeviceVn : AgentRefCount<PhysicalDeviceVn>,
     PhysicalDeviceVn(const boost::uuids::uuid &device_uuid,
                           const boost::uuids::uuid &vn_uuid) :
         device_uuid_(device_uuid), vn_uuid_(vn_uuid), device_(), vn_(),
-        vxlan_id_(0), tor_ip_(Ip4Address(0)) { }
+        vxlan_id_(0), tor_ip_(Ip4Address(0)), device_display_name_() { }
     virtual ~PhysicalDeviceVn() { }
 
     virtual bool IsLess(const DBEntry &rhs) const;
@@ -84,6 +84,7 @@ class PhysicalDeviceVn : AgentRefCount<PhysicalDeviceVn>,
     VnEntry *vn() const { return vn_.get(); }
     int vxlan_id() const { return vxlan_id_; }
     const IpAddress &tor_ip() const {return tor_ip_;}
+    const std::string &device_display_name() const {return device_display_name_;}
 
     bool Copy(PhysicalDeviceVnTable *table, const PhysicalDeviceVnData *data);
     void SendObjectLog(AgentLogEvent::type event) const;
@@ -98,6 +99,7 @@ class PhysicalDeviceVn : AgentRefCount<PhysicalDeviceVn>,
     VnEntryRef vn_;
     int vxlan_id_;
     IpAddress tor_ip_;
+    std::string device_display_name_;
     DISALLOW_COPY_AND_ASSIGN(PhysicalDeviceVn);
 };
 

src/vnsw/agent/oper/test/test_xml_physical_device.cc

@@ -95,7 +95,12 @@ bool AgentUtXmlPhysicalDevice::ReadXml() {
 bool AgentUtXmlPhysicalDevice::ToXml(xml_node *parent) {
     xml_node n = AddXmlNodeWithAttr(parent, NodeType().c_str());
     AddXmlNodeWithValue(&n, "name", name());
-    AddXmlNodeWithValue(&n, "display-name", name());
+    string display_name;
+    if (GetStringAttribute(node(), "display", &display_name)) {
+        AddXmlNodeWithValue(&n, "display-name", display_name);
+    } else {
+        AddXmlNodeWithValue(&n, "display-name", name());
+    }
     AddXmlNodeWithValue(&n, "physical-router-dataplane-ip", "111.111.111.111");
     AddIdPerms(&n);
     return true;
@@ -139,8 +144,8 @@ bool AgentUtXmlPhysicalInterface::ToXml(xml_node *parent) {
     AddIdPerms(&n);
 
     if (device_name_ != "") {
-        LinkXmlNode(parent, NodeType(), name(), "physical-router",
-                    device_name_);
+        LinkXmlNode(parent, "physical-router", device_name_,
+                    NodeType(), name());
     }
 
     return true;
@@ -187,12 +192,17 @@ bool AgentUtXmlRemotePhysicalInterface::ToXml(xml_node *parent) {
         fqdn = "dummy:" + agent->agent_name() + ":" + name();
     }
     AddXmlNodeWithValue(&n, "name", fqdn);
-    AddXmlNodeWithValue(&n, "display-name", name());
+    string display_name;
+    if (GetStringAttribute(node(), "display", &display_name)) {
+        AddXmlNodeWithValue(&n, "display-name", display_name);
+    } else {
+        AddXmlNodeWithValue(&n, "display-name", name());
+    }
     AddIdPerms(&n);
 
     if (device_name_ != "") {
-        LinkXmlNode(parent, NodeType(), fqdn, "physical-router",
-                    device_name_);
+        LinkXmlNode(parent, "physical-router", device_name_,
+                    NodeType(), fqdn);
     }
 
     return true;
@@ -265,13 +275,16 @@ string AgentUtXmlLogicalInterface::NodeType() {
 /////////////////////////////////////////////////////////////////////////////
 AgentUtXmlPhysicalDeviceValidate::AgentUtXmlPhysicalDeviceValidate
 (const string &name, const uuid &id, const xml_node &node) :
-    AgentUtXmlValidationNode(name, node), id_(id) {
+    AgentUtXmlValidationNode(name, node), id_(id), match_display_name_(false) {
 }
 
 AgentUtXmlPhysicalDeviceValidate::~AgentUtXmlPhysicalDeviceValidate() {
 }
 
 bool AgentUtXmlPhysicalDeviceValidate::ReadXml() {
+    if (AgentUtXmlValidationNode::ReadXml() == false)
+        return false;
+    match_display_name_ = GetStringAttribute(node(), "display", &display_name_);
     return true;
 }
 
@@ -281,7 +294,13 @@ bool AgentUtXmlPhysicalDeviceValidate::Validate() {
     dev = static_cast<PhysicalDevice *>
         (Agent::GetInstance()->physical_device_table()->FindActiveEntry(&key));
     if (present()) {
-        return dev != NULL;
+        if (dev != NULL) {
+            if (match_display_name_ && display_name_ != dev->name()) {
+                return false;
+            }
+            return true;
+        }
+        return false;
     } else {
         return dev == NULL;
     }

src/vnsw/agent/oper/test/test_xml_physical_device.h

@@ -94,6 +94,8 @@ class AgentUtXmlPhysicalDeviceValidate : public AgentUtXmlValidationNode {
     virtual const std::string ToString();
 private:
     const boost::uuids::uuid id_;
+    std::string display_name_;
+    bool match_display_name_;
 };
 
 class AgentUtXmlPhysicalInterfaceValidate : public AgentUtXmlValidationNode {

src/vnsw/agent/ovs_tor_agent/ovsdb_client/logical_switch_ovsdb.cc

@@ -16,7 +16,6 @@ extern "C" {
 
 #include <oper/vn.h>
 #include <oper/vrf.h>
-#include <oper/physical_device.h>
 #include <oper/physical_device_vn.h>
 #include <ovsdb_sandesh.h>
 #include <ovsdb_types.h>
@@ -35,8 +34,8 @@ LogicalSwitchEntry::LogicalSwitchEntry(OvsdbDBObject *table,
     name_(UuidToString(entry->vn()->GetUuid())), mcast_local_row_(NULL),
     mcast_remote_row_(NULL), mc_flood_entry_(NULL) {
     vxlan_id_ = entry->vxlan_id();
-    device_name_ = entry->device()->name();
-    tor_ip_ = entry->device()->ip();
+    device_name_ = entry->device_display_name();
+    tor_ip_ = entry->tor_ip();
 }
 
 LogicalSwitchEntry::LogicalSwitchEntry(OvsdbDBObject *table,
@@ -180,8 +179,8 @@ bool LogicalSwitchEntry::Sync(DBEntry *db_entry) {
         vxlan_id_ = entry->vxlan_id();
         change = true;
     }
-    if (device_name_ != entry->device()->name()) {
-        device_name_ = entry->device()->name();
+    if (device_name_ != entry->device_display_name()) {
+        device_name_ = entry->device_display_name();
         change = true;
     }
     if (tor_ip_ != entry->tor_ip()) {
@@ -438,18 +437,6 @@ KSyncDBObject::DBFilterResp LogicalSwitchTable::OvsdbDBEntryFilter(
     const PhysicalDeviceVn *entry =
         static_cast<const PhysicalDeviceVn *>(db_entry);
 
-    // Physical Device missing, trigger delete
-    if (entry->device() == NULL) {
-        if (ovsdb_entry != NULL) {
-            OVSDB_TRACE(Trace, "Missing Physical Device info, triggering delete"
-                               " of logical switch");
-        } else {
-            OVSDB_TRACE(Trace, "Missing Physical Device info, ignoring"
-                               " logical switch");
-        }
-        return DBFilterDelete;
-    }
-
     // Delete the entry which has invalid VxLAN id associated.
     if (entry->vxlan_id() == 0) {
         return DBFilterDelete;