Preserve static neighbors during interface reload

dataplane/neighbor.cpp

@@ -50,7 +50,7 @@ eResult module::init(
 			                                                      ht_size);
 		}
 	});
-	DumpOSNeighbors();
+	DumpOSNeighbors(nullptr);
 	StartNetlinkMonitor();
 	StartResolveJob();
 
@@ -241,7 +241,7 @@ eResult module::neighbor_remove(const common::idp::neighbor_remove::request& req
 
 eResult module::neighbor_clear()
 {
-	return DumpOSNeighbors();
+	return DumpOSNeighbors(nullptr);
 }
 
 eResult module::neighbor_flush()
@@ -280,17 +280,18 @@ void module::StopNetlinkMonitor()
 	YANET_LOG_INFO("Netlink monitor stopped\n");
 }
 
-eResult module::DumpOSNeighbors()
+eResult module::DumpOSNeighbors(const dataplane::neighbor::generation_interface* static_entry_interfaces)
 {
 	std::vector<netlink::Entry> dump;
 	std::vector<std::pair<dataplane::neighbor::key, dataplane::neighbor::value>> static_entries;
 	{
 		auto interfaces_guard = generation_interface.current_lock_guard();
 		auto& new_interfaces = generation_interface.current();
-		auto& old_interfaces = generation_interface.next();
 		dump = neighbor_provider->GetHostDump(rcvbuf_size_, new_interfaces.interface_name_to_id);
 
+		if (static_entry_interfaces)
 		{
+			const auto& old_interfaces = *static_entry_interfaces;
 			auto lock = generation_hashtable.current_lock_guard();
 			for (auto it : generation_hashtable.current().hashtable_updater.begin()->second.range())
 			{
@@ -322,9 +323,9 @@ eResult module::DumpOSNeighbors()
 	}
 
 	eResult res = generation_hashtable.update(
-	        [&dump,
+	        [dump = std::move(dump),
 	         now = current_time_provider_(),
-	         &static_entries,
+	         static_entries = std::move(static_entries),
 	         this](
 	                neighbor::generation_hashtable& hashtable) {
 		        for (auto& [socket_id, hashtable_updater] : hashtable.hashtable_updater)
@@ -365,6 +366,15 @@ eResult module::DumpOSNeighbors()
 eResult module::neighbor_update_interfaces(const common::idp::neighbor_update_interfaces::request& request)
 {
 	generation_interface.next_lock();
+
+	/// static entries stored in the hashtables are keyed with the interface ids of the
+	/// generation being replaced, keep it to remap them onto the new one
+	dataplane::neighbor::generation_interface previous_interfaces;
+	{
+		auto interfaces_guard = generation_interface.current_lock_guard();
+		previous_interfaces = generation_interface.current();
+	}
+
 	auto& generation = generation_interface.next();
 	generation.interface_name_to_id.clear();
 	generation.interface_id_to_name.clear();
@@ -380,9 +390,9 @@ eResult module::neighbor_update_interfaces(const common::idp::neighbor_update_in
 	generation_interface.next_unlock();
 	std::lock_guard<std::mutex> guard(mutex_restart_monitor_);
 	StopNetlinkMonitor();
-	DumpOSNeighbors();
+	auto result = DumpOSNeighbors(&previous_interfaces);
 	StartNetlinkMonitor();
-	return eResult::success;
+	return result;
 }
 
 common::idp::neighbor_stats::response module::neighbor_stats() const
@@ -620,7 +630,13 @@ void module::NeighborThreadAction(uint32_t current_time)
 	{
 		std::lock_guard<std::mutex> guard(mutex_restart_monitor_);
 		StopNetlinkMonitor();
-		DumpOSNeighbors();
+
+		dataplane::neighbor::generation_interface current_interfaces;
+		{
+			auto interfaces_guard = generation_interface.current_lock_guard();
+			current_interfaces = generation_interface.current();
+		}
+		DumpOSNeighbors(&current_interfaces);
 		StartNetlinkMonitor();
 	}
 
@@ -690,4 +706,4 @@ void module::NeighborThreadAction(uint32_t current_time)
 	}
 }
 
-} // namespace dataplane::neighbor
+} // namespace dataplane::neighbor

dataplane/neighbor.h

@@ -129,7 +129,11 @@ class module
 	void StartResolveJob();
 	void StartNetlinkMonitor();
 	void StopNetlinkMonitor();
-	eResult DumpOSNeighbors();
+	/// Rebuilds the neighbor tables from the OS dump, remapping the stored static
+	/// entries onto the current interface generation.
+	/// @param static_entry_interfaces interface generation the stored static entries
+	///        are keyed with, or nullptr to omit static entries from the rebuild.
+	eResult DumpOSNeighbors(const dataplane::neighbor::generation_interface* static_entry_interfaces);
 
 	bool resolve(const dataplane::neighbor::key& key);
 

dataplane/unittest/neighbor.cpp

@@ -252,4 +252,41 @@ TEST(NeighborTest, Provider)
 	EXPECT_TRUE(equal(dut.neighbor_show(), expected));
 }
 
-} // namespace
+TEST(NeighborTest, StaticNeighborSurvivesInterfaceUpdate)
+{
+	auto mock = new MockProvider;
+	dataplane::neighbor::module dut(mock);
+	auto now = 1;
+	dut.init(
+	        {1},
+	        64 * 1024,
+	        0,
+	        YANET_CONFIG_NEIGHBOR_CHECK_INTERVAL,
+	        YANET_CONFIG_NEIGHBOR_REMOVE_TIMEOUT,
+	        YANET_CONFIG_RESOLVE_REMOVED,
+	        [](tSocketId) {
+		        auto size = dataplane::neighbor::hashtable::calculate_sizeof(64 * 1024);
+		        void* ptr = new char[size];
+		        dataplane::neighbor::hashtable* ht = new (ptr) dataplane::neighbor::hashtable;
+		        return ht;
+	        },
+	        [&]() { return now; },
+	        []() {},
+	        []() { return std::vector<dataplane::neighbor::key>{}; });
+
+	dut.neighbor_update_interfaces({{1, "route0", "kni1"}});
+	dut.neighbor_insert({"route0", "kni1", Common4FromString("192.168.1.1"), {"DE:AD:BE:EF:01:02"}});
+	dut.neighbor_flush();
+
+	common::idp::neighbor_show::response expected = {
+	        {"route0", "kni1", Common4FromString("192.168.1.1"), {"DE:AD:BE:EF:01:02"}, {}, {}}};
+	EXPECT_TRUE(equal(dut.neighbor_show(), expected));
+
+	dut.neighbor_update_interfaces({{2, "route0", "kni1"}});
+	EXPECT_TRUE(equal(dut.neighbor_show(), expected));
+
+	dut.neighbor_clear();
+	EXPECT_TRUE(equal(dut.neighbor_show(), {}));
+}
+
+} // namespace