feat: avoid duplicate autoconnection attempt

DEFAULT_CONFIG.json5

@@ -138,6 +138,8 @@
       /// or different values for router, peer or client mode (e.g. autoconnect: { router: [], peer: ["router", "peer"] }).
       /// Each value is a list of: "peer", "router" and/or "client".
       autoconnect: { router: [], peer: ["router", "peer"], client: ["router"] },
+      /// Strategy for autoconnection, mainly to avoid nodes connecting to each other redundantly.
+      // autoconnect_strategy: "greater_zid",
       /// Whether or not to listen for scout messages on UDP multicast and reply to them.
       listen: true,
     },
@@ -161,6 +163,8 @@
       /// or different values for router or peer mode (e.g. autoconnect: { router: [], peer: ["router", "peer"] }).
       /// Each value is a list of: "peer" and/or "router".
       autoconnect: { router: [], peer: ["router", "peer"] },
+      /// Strategy for autoconnection, mainly to avoid nodes connecting to each other redundantly.
+      // autoconnect_strategy: "greater_zid",
     },
   },
 

commons/zenoh-config/src/defaults.rs

@@ -85,6 +85,7 @@ pub mod scouting {
                 &crate::WhatAmIMatcher::empty().router();
             mode_accessor!(crate::WhatAmIMatcher);
         }
+        pub const autoconnect_strategy: Option<crate::AutoConnectStrategy> = None;
         pub mod listen {
             pub const router: &bool = &true;
             pub const peer: &bool = &true;
@@ -113,6 +114,7 @@ pub mod scouting {
                 &crate::WhatAmIMatcher::empty();
             mode_accessor!(crate::WhatAmIMatcher);
         }
+        pub const autoconnect_strategy: Option<crate::AutoConnectStrategy> = None;
     }
 }
 

commons/zenoh-config/src/lib.rs

@@ -25,7 +25,8 @@ pub mod qos;
 pub mod wrappers;
 
 #[allow(unused_imports)]
-use std::convert::TryFrom; // This is a false positive from the rust analyser
+use std::convert::TryFrom;
+// This is a false positive from the rust analyser
 use std::{
     any::Any, collections::HashSet, fmt, io::Read, net::SocketAddr, ops, path::Path, sync::Weak,
 };
@@ -184,6 +185,17 @@ pub enum Permission {
     Deny,
 }
 
+/// Strategy for autoconnection, mainly to avoid nodes connecting to each other redundantly.
+#[derive(Clone, Copy, Debug, Serialize, Deserialize, Eq, Hash, PartialEq)]
+#[serde(rename_all = "kebab-case")]
+pub enum AutoConnectStrategy {
+    /// A node will attempt to connect to another one only if its own zid is greater than the
+    /// other one. If both nodes use this strategy, only one will attempt the connection.
+    /// This strategy may not be suited if one of the node is not reachable by the other one,
+    /// for example because of a private IP.
+    GreaterZid,
+}
+
 pub trait ConfigValidator: Send + Sync {
     fn check_config(
         &self,
@@ -301,6 +313,8 @@ validated_struct::validator! {
                 pub ttl: Option<u32>,
                 /// Which type of Zenoh instances to automatically establish sessions with upon discovery through UDP multicast.
                 autoconnect: Option<ModeDependentValue<WhatAmIMatcher>>,
+                /// Strategy for autoconnection, mainly to avoid nodes connecting to each other redundantly.
+                autoconnect_strategy: Option<AutoConnectStrategy>,
                 /// Whether or not to listen for scout messages on UDP multicast and reply to them.
                 listen: Option<ModeDependentValue<bool>>,
             },
@@ -319,6 +333,8 @@ validated_struct::validator! {
                 target: Option<ModeDependentValue<WhatAmIMatcher>>,
                 /// Which type of Zenoh instances to automatically establish sessions with upon discovery through gossip.
                 autoconnect: Option<ModeDependentValue<WhatAmIMatcher>>,
+                /// Strategy for autoconnection, mainly to avoid nodes connecting to each other redundantly.
+                autoconnect_strategy: Option<AutoConnectStrategy>,
             },
         },
 

zenoh/src/net/common.rs

@@ -0,0 +1,17 @@
+use zenoh_config::AutoConnectStrategy;
+use zenoh_protocol::core::ZenohIdProto;
+
+/// Returns if the node should autoconnect to the other one according to the give strategy.
+///
+/// The goal is to avoid both node to attempt connecting to each other, as it would result into
+/// a waste of resource.
+pub(crate) fn should_autoconnect(
+    strategy: Option<AutoConnectStrategy>,
+    self_zid: ZenohIdProto,
+    other_zid: ZenohIdProto,
+) -> bool {
+    match strategy {
+        Some(AutoConnectStrategy::GreaterZid) => self_zid > other_zid,
+        None => true,
+    }
+}

zenoh/src/net/mod.rs

@@ -17,13 +17,10 @@
 //! This module is intended for Zenoh's internal use.
 //!
 //! [Click here for Zenoh's documentation](https://docs.rs/zenoh/latest/zenoh)
-#[doc(hidden)]
 pub(crate) mod codec;
-#[doc(hidden)]
+mod common;
 pub(crate) mod primitives;
-#[doc(hidden)]
 pub(crate) mod protocol;
-#[doc(hidden)]
 pub(crate) mod routing;
 #[doc(hidden)]
 pub mod runtime;

zenoh/src/net/routing/hat/linkstate_peer/mod.rs

@@ -193,6 +193,7 @@ impl HatBaseTrait for HatCode {
         } else {
             WhatAmIMatcher::empty()
         };
+        let autoconnect_strategy = *config.scouting().gossip().autoconnect_strategy();
 
         let peer_full_linkstate =
             unwrap_or_default!(config.routing().peer().mode()) == *"linkstate";
@@ -210,6 +211,7 @@ impl HatBaseTrait for HatCode {
             gossip_multihop,
             gossip_target,
             autoconnect,
+            autoconnect_strategy,
         ));
         Ok(())
     }

zenoh/src/net/routing/hat/linkstate_peer/network.rs

@@ -24,6 +24,7 @@ use zenoh_buffers::{
     ZBuf,
 };
 use zenoh_codec::WCodec;
+use zenoh_config::AutoConnectStrategy;
 use zenoh_link::Locator;
 use zenoh_protocol::{
     common::ZExtBody,
@@ -34,6 +35,7 @@ use zenoh_transport::unicast::TransportUnicast;
 
 use crate::net::{
     codec::Zenoh080Routing,
+    common::should_autoconnect,
     protocol::linkstate::{LinkState, LinkStateList},
     routing::dispatcher::tables::NodeId,
     runtime::{Runtime, WeakRuntime},
@@ -121,6 +123,7 @@ pub(super) struct Network {
     pub(super) gossip_multihop: bool,
     pub(super) gossip_target: WhatAmIMatcher,
     pub(super) autoconnect: WhatAmIMatcher,
+    pub(super) autoconnect_strategy: Option<AutoConnectStrategy>,
     pub(super) idx: NodeIndex,
     pub(super) links: VecMap<Link>,
     pub(super) trees: Vec<Tree>,
@@ -141,6 +144,7 @@ impl Network {
         gossip_multihop: bool,
         gossip_target: WhatAmIMatcher,
         autoconnect: WhatAmIMatcher,
+        autoconnect_strategy: Option<AutoConnectStrategy>,
     ) -> Self {
         let mut graph = petgraph::stable_graph::StableGraph::default();
         tracing::debug!("{} Add node (self) {}", name, zid);
@@ -159,6 +163,7 @@ impl Network {
             gossip_multihop,
             gossip_target,
             autoconnect,
+            autoconnect_strategy,
             idx,
             links: VecMap::new(),
             trees: vec![Tree {
@@ -510,7 +515,14 @@ impl Network {
                             );
                         }
 
-                        if !self.autoconnect.is_empty() && self.autoconnect.matches(whatami) {
+                        if !self.autoconnect.is_empty()
+                            && self.autoconnect.matches(whatami)
+                            && should_autoconnect(
+                                self.autoconnect_strategy,
+                                self.graph[self.idx].zid,
+                                zid,
+                            )
+                        {
                             // Connect discovered peers
                             if let Some(locators) = locators {
                                 let runtime = strong_runtime.clone();

zenoh/src/net/routing/hat/p2p_peer/gossip.rs

@@ -20,6 +20,7 @@ use zenoh_buffers::{
     ZBuf,
 };
 use zenoh_codec::WCodec;
+use zenoh_config::AutoConnectStrategy;
 use zenoh_link::Locator;
 use zenoh_protocol::{
     common::ZExtBody,
@@ -30,6 +31,7 @@ use zenoh_transport::unicast::TransportUnicast;
 
 use crate::net::{
     codec::Zenoh080Routing,
+    common::should_autoconnect,
     protocol::linkstate::{LinkState, LinkStateList},
     runtime::{Runtime, WeakRuntime},
 };
@@ -98,6 +100,7 @@ pub(super) struct Network {
     pub(super) gossip_multihop: bool,
     pub(super) gossip_target: WhatAmIMatcher,
     pub(super) autoconnect: WhatAmIMatcher,
+    pub(super) autoconnect_strategy: Option<AutoConnectStrategy>,
     pub(super) wait_declares: bool,
     pub(super) idx: NodeIndex,
     pub(super) links: VecMap<Link>,
@@ -116,6 +119,7 @@ impl Network {
         gossip_multihop: bool,
         gossip_target: WhatAmIMatcher,
         autoconnect: WhatAmIMatcher,
+        autoconnect_strategy: Option<AutoConnectStrategy>,
         wait_declares: bool,
     ) -> Self {
         let mut graph = petgraph::stable_graph::StableGraph::default();
@@ -134,6 +138,7 @@ impl Network {
             gossip_multihop,
             gossip_target,
             autoconnect,
+            autoconnect_strategy,
             wait_declares,
             idx,
             links: VecMap::new(),
@@ -439,7 +444,14 @@ impl Network {
                         );
                     }
 
-                    if !self.autoconnect.is_empty() && self.autoconnect.matches(whatami) {
+                    if !self.autoconnect.is_empty()
+                        && self.autoconnect.matches(whatami)
+                        && should_autoconnect(
+                            self.autoconnect_strategy,
+                            self.graph[self.idx].zid,
+                            zid,
+                        )
+                    {
                         // Connect discovered peers
                         if let Some(locators) = locators {
                             let runtime = strong_runtime.clone();

zenoh/src/net/routing/hat/p2p_peer/mod.rs

@@ -123,6 +123,7 @@ impl HatBaseTrait for HatCode {
         } else {
             WhatAmIMatcher::empty()
         };
+        let autoconnect_strategy = *config.scouting().gossip().autoconnect_strategy();
         let wait_declares = unwrap_or_default!(config.open().return_conditions().declares());
         let router_peers_failover_brokering =
             unwrap_or_default!(config.routing().router().peers_failover_brokering());
@@ -138,6 +139,7 @@ impl HatBaseTrait for HatCode {
                 gossip_multihop,
                 gossip_target,
                 autoconnect,
+                autoconnect_strategy,
                 wait_declares,
             ));
         }

zenoh/src/net/routing/hat/router/mod.rs

@@ -320,6 +320,7 @@ impl HatBaseTrait for HatCode {
         } else {
             WhatAmIMatcher::empty()
         };
+        let autoconnect_strategy = *config.scouting().gossip().autoconnect_strategy();
 
         let router_full_linkstate = true;
         let peer_full_linkstate =
@@ -339,6 +340,7 @@ impl HatBaseTrait for HatCode {
                 gossip_multihop,
                 gossip_target,
                 autoconnect,
+                autoconnect_strategy,
             ));
         }
         if peer_full_linkstate | gossip {
@@ -352,6 +354,7 @@ impl HatBaseTrait for HatCode {
                 gossip_multihop,
                 gossip_target,
                 autoconnect,
+                autoconnect_strategy,
             ));
         }
         if router_full_linkstate && peer_full_linkstate {

zenoh/src/net/routing/hat/router/network.rs

@@ -24,6 +24,7 @@ use zenoh_buffers::{
     ZBuf,
 };
 use zenoh_codec::WCodec;
+use zenoh_config::AutoConnectStrategy;
 use zenoh_link::Locator;
 use zenoh_protocol::{
     common::ZExtBody,
@@ -34,6 +35,7 @@ use zenoh_transport::unicast::TransportUnicast;
 
 use crate::net::{
     codec::Zenoh080Routing,
+    common::should_autoconnect,
     protocol::linkstate::{LinkState, LinkStateList},
     routing::dispatcher::tables::NodeId,
     runtime::Runtime,
@@ -121,6 +123,7 @@ pub(super) struct Network {
     pub(super) gossip_multihop: bool,
     pub(super) gossip_target: WhatAmIMatcher,
     pub(super) autoconnect: WhatAmIMatcher,
+    pub(super) autoconnect_strategy: Option<AutoConnectStrategy>,
     pub(super) idx: NodeIndex,
     pub(super) links: VecMap<Link>,
     pub(super) trees: Vec<Tree>,
@@ -141,6 +144,7 @@ impl Network {
         gossip_multihop: bool,
         gossip_target: WhatAmIMatcher,
         autoconnect: WhatAmIMatcher,
+        autoconnect_strategy: Option<AutoConnectStrategy>,
     ) -> Self {
         let mut graph = petgraph::stable_graph::StableGraph::default();
         tracing::debug!("{} Add node (self) {}", name, zid);
@@ -159,6 +163,7 @@ impl Network {
             gossip_multihop,
             gossip_target,
             autoconnect,
+            autoconnect_strategy,
             idx,
             links: VecMap::new(),
             trees: vec![Tree {
@@ -514,7 +519,14 @@ impl Network {
                             );
                         }
 
-                        if !self.autoconnect.is_empty() && self.autoconnect.matches(whatami) {
+                        if !self.autoconnect.is_empty()
+                            && self.autoconnect.matches(whatami)
+                            && should_autoconnect(
+                                self.autoconnect_strategy,
+                                self.graph[self.idx].zid,
+                                zid,
+                            )
+                        {
                             // Connect discovered peers
                             if let Some(locators) = locators {
                                 let runtime = self.runtime.clone();