protocols: refactor code to eliminate mutable global variables

Using global variables for anything other than read-only data is
problematic, as, at some point in the future, we might want to run
multiple distinct router instances in the same process, primarily for
testing purposes (e.g., WebAssembly virtual topologies).

This commit removes global variables responsible for storing keychains
and the global SR configuration. Instead, each protocol instance now
maintains a distinct copy of reference-counted shared configuration
objects, exchanged using the internal bus and stored within the
protocol instance structs.  Comprehensive documentation about shared
configuration objects will be added to the wiki.

Signed-off-by: Renato Westphal <[email protected]>

holo-bfd/src/master.rs

@@ -8,15 +8,17 @@ use std::collections::HashMap;
 
 use async_trait::async_trait;
 use derive_new::new;
-use holo_protocol::{InstanceChannelsTx, MessageReceiver, ProtocolInstance};
+use holo_protocol::{
+    InstanceChannelsTx, InstanceShared, MessageReceiver, ProtocolInstance,
+};
 use holo_southbound::rx::SouthboundRx;
 use holo_southbound::tx::SouthboundTx;
 use holo_utils::bfd::PathType;
 use holo_utils::ibus::IbusMsg;
 use holo_utils::ip::AddressFamily;
 use holo_utils::protocol::Protocol;
 use holo_utils::task::Task;
-use holo_utils::{Database, Receiver, Sender};
+use holo_utils::{Receiver, Sender};
 use tokio::sync::mpsc;
 
 use crate::error::{Error, IoError};
@@ -114,7 +116,7 @@ impl ProtocolInstance for Master {
 
     async fn new(
         _name: String,
-        _db: Option<Database>,
+        _shared: InstanceShared,
         tx: InstanceChannelsTx<Master>,
     ) -> Master {
         Master {

holo-keychain/src/northbound/configuration.rs

@@ -16,7 +16,7 @@ use holo_northbound::configuration::{
 use holo_northbound::paths::key_chains;
 use holo_utils::crypto::CryptoAlgo;
 use holo_utils::ibus::IbusMsg;
-use holo_utils::keychain::{Key, Keychain, KeychainKey, KEYCHAINS};
+use holo_utils::keychain::{Key, Keychain, KeychainKey};
 use holo_utils::yang::DataNodeRefExt;
 use holo_yang::TryFromYang;
 
@@ -39,6 +39,7 @@ pub enum Resource {}
 #[derive(Debug, Eq, Ord, PartialEq, PartialOrd)]
 pub enum Event {
     KeychainChange(String),
+    KeychainDelete(String),
 }
 
 // ===== callbacks =====
@@ -54,6 +55,9 @@ fn load_callbacks() -> Callbacks<Master> {
         .delete_apply(|master, args| {
             let name = args.list_entry.into_keychain().unwrap();
             master.keychains.remove(&name);
+
+            let event_queue = args.event_queue;
+            event_queue.insert(Event::KeychainDelete(name.clone()));
         })
         .lookup(|_master, _list_entry, dnode| {
             let name = dnode.get_string_relative("./name").unwrap();
@@ -434,14 +438,17 @@ impl Provider for Master {
                     .max()
                     .unwrap_or(0);
 
-                // Update the global list of keychains shared by all protocols.
-                KEYCHAINS
-                    .lock()
-                    .unwrap()
-                    .insert(name, Arc::new(keychain.clone()));
+                // Create a reference-counted copy of the keychain to be shared among all
+                // protocol instances.
+                let keychain = Arc::new(keychain.clone());
 
                 // Notify protocols that the keychain has been updated.
-                let msg = IbusMsg::KeychainUpd(keychain.name.clone());
+                let msg = IbusMsg::KeychainUpd(keychain.clone());
+                let _ = self.ibus_tx.send(msg);
+            }
+            Event::KeychainDelete(name) => {
+                // Notify protocols that the keychain has been deleted.
+                let msg = IbusMsg::KeychainDel(name);
                 let _ = self.ibus_tx.send(msg);
             }
         }

holo-ldp/src/instance.rs

@@ -13,15 +13,17 @@ use async_trait::async_trait;
 use derive_new::new;
 use enum_as_inner::EnumAsInner;
 use holo_northbound::paths::control_plane_protocol::mpls_ldp;
-use holo_protocol::{InstanceChannelsTx, MessageReceiver, ProtocolInstance};
+use holo_protocol::{
+    InstanceChannelsTx, InstanceShared, MessageReceiver, ProtocolInstance,
+};
 use holo_southbound::rx::SouthboundRx;
 use holo_southbound::tx::SouthboundTx;
 use holo_utils::ibus::IbusMsg;
 use holo_utils::mpls::Label;
 use holo_utils::protocol::Protocol;
 use holo_utils::socket::{TcpListener, UdpSocket};
 use holo_utils::task::Task;
-use holo_utils::{Database, Receiver, Sender};
+use holo_utils::{Receiver, Sender};
 use ipnetwork::{IpNetwork, Ipv4Network, Ipv6Network};
 use tokio::sync::mpsc;
 
@@ -297,7 +299,7 @@ impl ProtocolInstance for Instance {
 
     async fn new(
         name: String,
-        _db: Option<Database>,
+        _shared: InstanceShared,
         tx: InstanceChannelsTx<Instance>,
     ) -> Instance {
         Debug::InstanceCreate.log();

holo-ospf/src/events.rs

@@ -11,8 +11,7 @@ use std::sync::Arc;
 
 use chrono::Utc;
 use holo_utils::bfd;
-use holo_utils::ibus::SrCfgEventMsg;
-use holo_utils::sr::SrCfg;
+use holo_utils::ibus::SrCfgEvent;
 
 use crate::area::{Area, AreaType};
 use crate::collections::{
@@ -41,7 +40,7 @@ use crate::packet::{
     PacketType,
 };
 use crate::version::Version;
-use crate::{gr, output, spf, sr, tasks};
+use crate::{gr, output, spf, tasks};
 
 // ===== Interface FSM event =====
 
@@ -1411,15 +1410,11 @@ where
 
 pub(crate) fn process_sr_cfg_change<V>(
     instance: &mut Instance<V>,
-    sr_config: Arc<SrCfg>,
-    change: SrCfgEventMsg,
+    change: SrCfgEvent,
 ) -> Result<(), Error<V>>
 where
     V: Version,
 {
-    // Update local reference-counted copy of the SR configuration.
-    *sr::CONFIG.lock().unwrap() = sr_config;
-
     if let Some((instance, arenas)) = instance.as_up() {
         if instance.config.sr_enabled {
             // Check which LSAs need to be reoriginated or flushed.

holo-ospf/src/instance.rs

@@ -13,16 +13,16 @@ use std::time::Instant;
 use async_trait::async_trait;
 use chrono::{DateTime, Utc};
 use holo_northbound::paths::control_plane_protocol::ospf;
-use holo_protocol::{InstanceChannelsTx, MessageReceiver, ProtocolInstance};
+use holo_protocol::{
+    InstanceChannelsTx, InstanceShared, MessageReceiver, ProtocolInstance,
+};
 use holo_southbound::rx::SouthboundRx;
 use holo_southbound::tx::SouthboundTx;
 use holo_utils::ibus::IbusMsg;
 use holo_utils::ip::AddressFamily;
 use holo_utils::protocol::Protocol;
 use holo_utils::task::TimeoutTask;
-use holo_utils::{
-    Database, Receiver, Sender, UnboundedReceiver, UnboundedSender,
-};
+use holo_utils::{Receiver, Sender, UnboundedReceiver, UnboundedSender};
 use tokio::sync::mpsc;
 
 use crate::collections::{
@@ -63,8 +63,8 @@ pub struct Instance<V: Version> {
     pub arenas: InstanceArenas<V>,
     // Instance Tx channels.
     pub tx: InstanceChannelsTx<Instance<V>>,
-    // Non-volatile storage.
-    pub db: Option<Database>,
+    // Shared data.
+    pub shared: InstanceShared,
 }
 
 #[derive(Debug, Default)]
@@ -217,13 +217,13 @@ pub struct ProtocolInputChannelsRx<V: Version> {
     pub grace_period: Receiver<GracePeriodMsg>,
 }
 
-#[derive(Debug)]
 pub struct InstanceUpView<'a, V: Version> {
     pub name: &'a str,
     pub system: &'a InstanceSys,
     pub config: &'a InstanceCfg,
     pub state: &'a mut InstanceState<V>,
     pub tx: &'a InstanceChannelsTx<Instance<V>>,
+    pub shared: &'a InstanceShared,
 }
 
 // OSPF version-specific code.
@@ -389,7 +389,7 @@ where
     fn boot_count_get(&self) -> u32 {
         let mut boot_count = 0;
 
-        if let Some(db) = &self.db {
+        if let Some(db) = &self.shared.db {
             let db = db.lock().unwrap();
 
             let key = format!("{}-{}-boot-count", V::PROTOCOL, self.name);
@@ -403,7 +403,7 @@ where
 
     // Stores the updated boot count of the instance in non-volatile memory.
     fn boot_count_update(&mut self) {
-        if let Some(db) = &self.db {
+        if let Some(db) = &self.shared.db {
             let mut db = db.lock().unwrap();
             let mut boot_count = 0;
 
@@ -428,6 +428,7 @@ where
                 config: &self.config,
                 state,
                 tx: &self.tx,
+                shared: &self.shared,
             };
             Some((instance, &mut self.arenas))
         } else {
@@ -452,7 +453,7 @@ where
 
     async fn new(
         name: String,
-        db: Option<Database>,
+        shared: InstanceShared,
         tx: InstanceChannelsTx<Instance<V>>,
     ) -> Instance<V> {
         Debug::<V>::InstanceCreate.log();
@@ -464,7 +465,7 @@ where
             state: None,
             arenas: Default::default(),
             tx,
-            db,
+            shared,
         }
     }
 
@@ -855,18 +856,37 @@ where
     V: Version,
 {
     match msg {
-        // SR configuration change event.
-        IbusMsg::SrCfgEvent { event, sr_config } => {
-            events::process_sr_cfg_change(instance, sr_config, event)?
-        }
         // BFD peer state update event.
         IbusMsg::BfdStateUpd { sess_key, state } => {
             events::process_bfd_state_update(instance, sess_key, state)?
         }
         // Keychain update event.
-        IbusMsg::KeychainUpd(keychain_name) => {
+        IbusMsg::KeychainUpd(keychain) => {
+            // Update the local copy of the keychain.
+            instance
+                .shared
+                .keychains
+                .insert(keychain.name.clone(), keychain.clone());
+
+            // Update all interfaces using this keychain.
+            events::process_keychain_update(instance, &keychain.name)?
+        }
+        // Keychain delete event.
+        IbusMsg::KeychainDel(keychain_name) => {
+            // Remove the local copy of the keychain.
+            instance.shared.keychains.remove(&keychain_name);
+
+            // Update all interfaces using this keychain.
             events::process_keychain_update(instance, &keychain_name)?
         }
+        // SR configuration update.
+        IbusMsg::SrCfgUpd(sr_config) => {
+            instance.shared.sr_config = sr_config;
+        }
+        // SR configuration event.
+        IbusMsg::SrCfgEvent(event) => {
+            events::process_sr_cfg_change(instance, event)?
+        }
         // Ignore other events.
         _ => {}
     }

holo-ospf/src/interface.rs

@@ -15,7 +15,7 @@ use holo_northbound::paths::control_plane_protocol::ospf;
 use holo_protocol::InstanceChannelsTx;
 use holo_utils::crypto::CryptoAlgo;
 use holo_utils::ip::{AddressFamily, IpAddrKind, IpNetworkKind};
-use holo_utils::keychain::{Key, KEYCHAINS};
+use holo_utils::keychain::{Key, Keychains};
 use holo_utils::socket::{AsyncFd, Socket};
 use holo_utils::task::{IntervalTask, Task, TimeoutTask};
 use holo_utils::{bfd, UnboundedSender};
@@ -317,7 +317,7 @@ where
         Debug::<V>::InterfaceStart(&self.name).log();
 
         if !self.is_passive() {
-            self.state.auth = self.auth();
+            self.state.auth = self.auth(&instance.shared.keychains);
 
             // Start network Tx/Rx tasks.
             match InterfaceNet::new(
@@ -481,7 +481,7 @@ where
         )
     }
 
-    fn auth(&self) -> Option<AuthMethod> {
+    fn auth(&self, keychains: &Keychains) -> Option<AuthMethod> {
         if let (Some(key), Some(key_id), Some(algo)) = (
             &self.config.auth_key,
             self.config.auth_keyid,
@@ -493,7 +493,7 @@ where
         }
 
         if let Some(keychain) = &self.config.auth_keychain {
-            if let Some(keychain) = KEYCHAINS.lock().unwrap().get(keychain) {
+            if let Some(keychain) = keychains.get(keychain) {
                 return Some(AuthMethod::Keychain(keychain.clone()));
             }
         }
@@ -507,7 +507,7 @@ where
         instance: &InstanceUpView<'_, V>,
     ) {
         // Update authentication data.
-        self.state.auth = self.auth();
+        self.state.auth = self.auth(&instance.shared.keychains);
 
         if let Some(mut net) = self.state.net.take() {
             // Enable or disable checksum offloading.

holo-ospf/src/lsdb.rs

@@ -13,7 +13,7 @@ use std::time::Instant;
 use bitflags::bitflags;
 use chrono::Utc;
 use derive_new::new;
-use holo_utils::ibus::SrCfgEventMsg;
+use holo_utils::ibus::SrCfgEvent;
 use holo_utils::task::TimeoutTask;
 use holo_utils::UnboundedSender;
 use serde::{Deserialize, Serialize};
@@ -121,7 +121,7 @@ pub enum LsaOriginateEvent {
     StubRouterChange,
     SrEnableChange,
     SrCfgChange {
-        change: SrCfgEventMsg,
+        change: SrCfgEvent,
     },
     GrHelperChange,
     GrHelperExit {

holo-ospf/src/ospfv2/lsdb.rs

@@ -7,7 +7,7 @@
 use std::collections::BTreeMap;
 use std::net::Ipv4Addr;
 
-use holo_utils::ibus::SrCfgEventMsg;
+use holo_utils::ibus::SrCfgEvent;
 use holo_utils::ip::{AddressFamily, Ipv4NetworkExt};
 use holo_utils::mpls::Label;
 use holo_utils::sr::{IgpAlgoType, Sid, SidLastHopBehavior};
@@ -42,7 +42,6 @@ use crate::packet::tlv::{
     SrAlgoTlv, SrLocalBlockTlv,
 };
 use crate::route::{SummaryNet, SummaryRtr};
-use crate::sr;
 use crate::version::Ospfv2;
 
 // ===== impl Ospfv2 =====
@@ -224,13 +223,13 @@ impl LsdbVersion<Self> for Ospfv2 {
             }
             LsaOriginateEvent::SrCfgChange { change } => {
                 match change {
-                    SrCfgEventMsg::LabelRangeUpdate => {
+                    SrCfgEvent::LabelRangeUpdate => {
                         // Reoriginate Router Information LSA(s) in all areas.
                         for area in arenas.areas.iter() {
                             lsa_orig_router_info(area, instance);
                         }
                     }
-                    SrCfgEventMsg::PrefixSidUpdate(af) => {
+                    SrCfgEvent::PrefixSidUpdate(af) => {
                         if af == AddressFamily::Ipv4 {
                             // (Re)originate Extended Prefix Opaque LSA(s) in
                             // all areas.
@@ -552,7 +551,7 @@ fn lsa_orig_router_info(
     area: &Area<Ospfv2>,
     instance: &InstanceUpView<'_, Ospfv2>,
 ) {
-    let sr_config = sr::CONFIG.lock().unwrap();
+    let sr_config = &instance.shared.sr_config;
     let lsdb_id = LsdbId::Area(area.id);
 
     // LSA's header options.
@@ -611,7 +610,7 @@ fn lsa_orig_ext_prefix(
     instance: &InstanceUpView<'_, Ospfv2>,
     arenas: &InstanceArenas<Ospfv2>,
 ) {
-    let sr_config = sr::CONFIG.lock().unwrap();
+    let sr_config = &instance.shared.sr_config;
     let lsdb_id = LsdbId::Area(area.id);
 
     // LSA's header options.