Take interface nh reference after creating in VmInterface.

Previously say a db entry is created so it has ref count of 0. Now references
are being taken and released in multiple partition (currently in flow). The
refcount manipulation is atomic but operations after manipulations are not. In
absence of self reference refcount can go to 0 and come back to 1 and in turn
dbstate could be deleted. Entry is not marked for deletion and is valid. This
results in double call to free state and second call asserts as there was no
state.
Bit more explanation on why refcount is 1 and there is no state above.
Say there are two referrers a and b. a has taken reference and then releasing
it. So refcount goes to 1 and then back to 0. While it has modified refcount and
going through clearrefstate (refcount being 0), b runs in parallel and
increments refcount as well as tried adding state. Meanwhile a also proceeds and
then deletes state. Result is refcount is at 1 and there is no state which is
wrong.

Closes-bug: #1571584
Change-Id: I2237971d2f36aac00fa76b90770943bdddc86c19

src/vnsw/agent/oper/vm_interface.cc

@@ -78,6 +78,11 @@ VmInterface::VmInterface(const boost::uuids::uuid &uuid) :
     ipv4_active_ = false;
     ipv6_active_ = false;
     l2_active_ = false;
+    l3_interface_nh_policy_.reset();
+    l2_interface_nh_policy_.reset();
+    l3_interface_nh_no_policy_.reset();
+    l2_interface_nh_no_policy_.reset();
+    multicast_nh_.reset();
 }
 
 VmInterface::VmInterface(const boost::uuids::uuid &uuid,
@@ -1265,11 +1270,23 @@ bool VmInterface::Delete(const DBRequest *req) {
 void VmInterface::UpdateL3MetadataIp(VrfEntry *old_vrf, bool force_update,
                                      bool policy_change,
                                      bool old_metadata_ip_active) {
+    InterfaceTable *table = static_cast<InterfaceTable *>(get_table());
+    Agent *agent = table->agent();
     assert(metadata_ip_active_);
     if (!old_metadata_ip_active) {
         InterfaceNH::CreateL3VmInterfaceNH(GetUuid(),
                                            MacAddress::FromString(vm_mac_),
                                            vrf_->GetName());
+        InterfaceNHKey key1(new VmInterfaceKey(AgentKey::ADD_DEL_CHANGE,
+                                              GetUuid(), ""),
+                            true, InterfaceNHFlags::INET4);
+        l3_interface_nh_policy_ = static_cast<NextHop *>(agent->
+                      nexthop_table()->FindActiveEntry(&key1));
+        InterfaceNHKey key2(new VmInterfaceKey(AgentKey::ADD_DEL_CHANGE,
+                                              GetUuid(), ""),
+                            false, InterfaceNHFlags::INET4);
+        l3_interface_nh_no_policy_ = static_cast<NextHop *>(agent->
+                      nexthop_table()->FindActiveEntry(&key2));
     }
     UpdateL3TunnelId(force_update, policy_change);
     UpdateMetadataRoute(old_metadata_ip_active, old_vrf);
@@ -1282,6 +1299,8 @@ void VmInterface::DeleteL3MetadataIp(VrfEntry *old_vrf, bool force_update,
     assert(!metadata_ip_active_);
     if (old_metadata_ip_active) {
         InterfaceNH::DeleteL3InterfaceNH(GetUuid());
+        l3_interface_nh_policy_.reset();
+        l3_interface_nh_no_policy_.reset();
     }
     DeleteL3TunnelId();
     DeleteMetadataRoute(old_metadata_ip_active, old_vrf,
@@ -2609,6 +2628,14 @@ void VmInterface::UpdateMulticastNextHop(bool old_ipv4_active,
         InterfaceNH::CreateMulticastVmInterfaceNH(GetUuid(),
                                                   MacAddress::FromString(vm_mac_),
                                                   vrf_->GetName());
+        InterfaceTable *table = static_cast<InterfaceTable *>(get_table());
+        Agent *agent = table->agent();
+        InterfaceNHKey key(new VmInterfaceKey(AgentKey::ADD_DEL_CHANGE,
+                                              GetUuid(), ""),
+                          false, (InterfaceNHFlags::INET4 |
+                                  InterfaceNHFlags::MULTICAST));
+        multicast_nh_ = static_cast<NextHop *>(agent->
+                        nexthop_table()->FindActiveEntry(&key));
     }
 }
 
@@ -2643,10 +2670,22 @@ void VmInterface::UpdateMacVmBinding() {
 }
 
 void VmInterface::UpdateL2NextHop(bool old_l2_active) {
+    InterfaceTable *table = static_cast<InterfaceTable *>(get_table());
+    Agent *agent = table->agent();
     if (L2Activated(old_l2_active)) {
         InterfaceNH::CreateL2VmInterfaceNH(GetUuid(),
                                            MacAddress::FromString(vm_mac_),
                                            vrf_->GetName());
+        InterfaceNHKey key1(new VmInterfaceKey(AgentKey::ADD_DEL_CHANGE,
+                                              GetUuid(), ""),
+                            true, InterfaceNHFlags::BRIDGE);
+        l2_interface_nh_policy_ = static_cast<NextHop *>(agent->
+                      nexthop_table()->FindActiveEntry(&key1));
+        InterfaceNHKey key2(new VmInterfaceKey(AgentKey::ADD_DEL_CHANGE,
+                                              GetUuid(), ""),
+                            false, InterfaceNHFlags::BRIDGE);
+        l2_interface_nh_no_policy_ = static_cast<NextHop *>(agent->
+                      nexthop_table()->FindActiveEntry(&key2));
     }
 }
 
@@ -2667,11 +2706,14 @@ void VmInterface::DeleteMacVmBinding(const VrfEntry *old_vrf) {
 void VmInterface::DeleteL2NextHop(bool old_l2_active) {
     if (L2Deactivated(old_l2_active)) {
         InterfaceNH::DeleteL2InterfaceNH(GetUuid());
+        l2_interface_nh_policy_.reset();
+        l2_interface_nh_no_policy_.reset();
     }
 }
 
 void VmInterface::DeleteMulticastNextHop() {
     InterfaceNH::DeleteMulticastVmInterfaceNH(GetUuid());
+    multicast_nh_.reset();
 }
 
 void VmInterface::DeleteL2ReceiveRoute(const VrfEntry *old_vrf,

src/vnsw/agent/oper/vm_interface.h

@@ -863,6 +863,11 @@ class VmInterface : public Interface {
     MetaDataIpMap metadata_ip_map_;
     HealthCheckInstanceSet hc_instance_set_;
     VmiEcmpLoadBalance ecmp_load_balance_;
+    NextHopRef l3_interface_nh_policy_;
+    NextHopRef l2_interface_nh_policy_;
+    NextHopRef l3_interface_nh_no_policy_;
+    NextHopRef l2_interface_nh_no_policy_;
+    NextHopRef multicast_nh_;
     DISALLOW_COPY_AND_ASSIGN(VmInterface);
 };
 

src/vnsw/agent/test/SConscript

@@ -134,6 +134,9 @@ test_dpdk_mpls = AgentEnv.MakeTestCmd(env, 'test_dpdk_mpls', agent_suite)
 test_bgp_service_configuration = AgentEnv.MakeTestCmd(env,
                                                       'test_bgp_service_configuration',
                                                       agent_suite)
+test_agent_db_entry = AgentEnv.MakeTestCmd(env,
+                                           'test_agent_db_entry',
+                                           agent_suite)
 
 flaky_test = env.TestSuite('agent-flaky-test', flaky_agent_suite)
 env.Alias('controller/src/vnsw/agent:flaky_test', flaky_test)

src/vnsw/agent/test/test_agent_db_entry.cc

@@ -0,0 +1,46 @@
+/*
+ * Copyright (c) 2016 Juniper Networks, Inc. All rights reserved.
+ */
+
+#include "test_cmn_util.h"
+
+void RouterIdDepInit(Agent *agent) {
+}
+
+class AgentDbEntry : public ::testing::Test {
+public:
+    AgentDbEntry() {
+        agent_ = Agent::GetInstance();
+    }
+    virtual ~AgentDbEntry() { }
+    Agent *agent_;
+};
+
+TEST_F(AgentDbEntry, db_entry_self_reference) {
+   struct PortInfo input[] = {
+        {"vnet1", 1, "1.1.1.10", "00:00:01:01:01:10", 1, 1},
+    };
+
+    client->Reset();
+    CreateVmportEnv(input, 1);
+    client->WaitForIdle();
+    VmInterfaceKey *intf_key = new VmInterfaceKey(AgentKey::ADD_DEL_CHANGE,
+                                                  MakeUuid(1), "vnet1");
+    InterfaceNHKey intf_nh_key(intf_key, false,
+                               (InterfaceNHFlags::INET4) |
+                               (InterfaceNHFlags::MULTICAST));
+    NextHop *nh = GetNH(&intf_nh_key);
+    EXPECT_TRUE(nh->GetRefCount() != 0);
+    DeleteVmportEnv(input, 1, true);
+    client->WaitForIdle();
+}
+
+int main(int argc, char *argv[]) {
+    GETUSERARGS();
+    client = TestInit(init_file, ksync_init, true, true, false);
+
+    int ret = RUN_ALL_TESTS();
+    TestShutdown();
+    delete client;
+    return ret;
+}