made IP_RECV_ERR configurable

Relates to: #652 For systems with less robust network, always enabling IP_RECV_ERR might result in unnecessary disconnects and a less robust BlueChi setup. Therefore, a configuration setting was introduced to enable or disable this option depending on the current needs. Signed-off-by: Michael Engel <[email protected]>
eclipse-bluechi · Dec 20, 2023 · 924e153 · 924e153
1 parent b572dad
commit 924e153
Show file tree

Hide file tree

Showing 13 changed files with 96 additions and 25 deletions.
diff --git a/config/agent/agent.conf b/config/agent/agent.conf
@@ -40,3 +40,8 @@
 #
 # If this flag is set to true, no logs are written by bluechi.
 #LogIsQuiet=false
+
+# Enables or disables extended, reliable error message passing for the peer connection with the
+# controller. For example, if set to true, the peer connection will be dropped instantly on
+# Host unreachable errors.
+#IPReceiveErrors=true
diff --git a/config/controller/controller.conf b/config/controller/controller.conf
@@ -27,3 +27,8 @@
 #
 # If this flag is set to true, no logs are written by bluechi.
 #LogIsQuiet=false
+
+# Enables or disables extended, reliable error message passing for the peer connection with
+# the agent. For example, if set to true, the peer connection will be dropped instantly on
+# Host unreachable errors.
+#IPReceiveErrors=true
diff --git a/doc/man/bluechi-agent.conf.5.md b/doc/man/bluechi-agent.conf.5.md
@@ -66,6 +66,14 @@ By default `journald` is used as the target.
 
 If this flag is set to `true`, no logs are written by bluechi. By default the flag is set to `false`.
 
+#### **IPReceiveErrors** (string)
+
+If this flag is set to `true`, it enables extended, reliable error message passing for
+the peer connection with the controller. This results in BlueChi receiving errors such as
+host unreachable ICMP packets instantly and possibly dropping the connection. This is
+useful to detect disconnects faster, but should be used with care as this might cause
+unnecessary disconnects in less robut networks. Default: true.
+
 ## Example
 
 Using `ManagerHost` and `ManagerPort` options:

diff --git a/doc/man/bluechi-controller.conf.5.md b/doc/man/bluechi-controller.conf.5.md
@@ -52,6 +52,15 @@ By default `journald` is used as the target.
 
 If this flag is set to `true`, no logs are written by bluechi. By default the flag is set to `false`.
 
+#### **IPReceiveErrors** (string)
+
+If this flag is set to `true`, it enables extended, reliable error message passing for
+the peer connection with all agents. This results in BlueChi receiving errors such as
+host unreachable ICMP packets instantly and possibly dropping the connection. This is
+useful to detect disconnects faster, but should be used with care as this might cause
+unnecessary disconnects in less robut networks. Default: true.
+
+
 ## Example
 
 A basic example of a configuration file for `bluechi`:

diff --git a/src/agent/agent.c b/src/agent/agent.c
@@ -408,6 +408,7 @@ Agent *agent_new(void) {
         agent->connection_retry_count = 0;
         agent->wildcard_subscription_active = false;
         agent->metrics_enabled = false;
+        agent->ip_receive_errors = false;
         agent->disconnect_timestamp = 0;
 
         return steal_pointer(&agent);
@@ -623,6 +624,8 @@ bool agent_parse_config(Agent *agent, const char *configfile) {
                 }
         }
 
+        agent->ip_receive_errors = cfg_get_bool_value(agent->config, CFG_IP_RECEIVE_ERRORS);
+
         _cleanup_free_ const char *dumped_cfg = cfg_dump(agent->config);
         bc_log_debug_with_data("Final configuration used", "\n%s", dumped_cfg);
 
@@ -2400,6 +2403,12 @@ static bool agent_connect(Agent *agent) {
         if (r < 0) {
                 bc_log_warn("Failed to set KEEPALIVE on socket");
         }
+        if (agent->ip_receive_errors) {
+                r = bus_socket_enable_recv_err(agent->peer_dbus);
+                if (r < 0) {
+                        bc_log_warnf("Failed to enable receiving errors on socket: %s", strerror(-r));
+                }
+        }
 
         r = sd_bus_add_object_vtable(
                         agent->peer_dbus,

diff --git a/src/agent/agent.h b/src/agent/agent.h
@@ -65,6 +65,8 @@ struct Agent {
 
         bool metrics_enabled;
 
+        bool ip_receive_errors;
+
         sd_event *event;
 
         sd_bus *api_bus;

diff --git a/src/libbluechi/bus/utils.c b/src/libbluechi/bus/utils.c
@@ -336,8 +336,21 @@ int bus_socket_set_keepalive(sd_bus *bus) {
                 return -errno;
         }
 
-        int enable = 1;
-        r = setsockopt(fd, IPPROTO_IP, IP_RECVERR, &enable, sizeof(int));
+        return 0;
+}
+
+int bus_socket_enable_recv_err(sd_bus *bus) {
+        int fd = sd_bus_get_fd(bus);
+        if (fd < 0) {
+                return fd;
+        }
+
+        if (!is_socket_tcp(fd)) {
+                return -EINVAL;
+        }
+
+        int flag = 1;
+        int r = setsockopt(fd, IPPROTO_IP, IP_RECVERR, &flag, sizeof(int));
         if (r < 0) {
                 return -errno;
         }

diff --git a/src/libbluechi/bus/utils.h b/src/libbluechi/bus/utils.h
@@ -42,6 +42,7 @@ int bus_parse_unit_on_node_info(sd_bus_message *message, UnitInfo *u);
 
 int bus_socket_set_no_delay(sd_bus *bus);
 int bus_socket_set_keepalive(sd_bus *bus);
+int bus_socket_enable_recv_err(sd_bus *bus);
 
 bool bus_id_is_valid(const char *name);
 

diff --git a/src/libbluechi/common/cfg.c b/src/libbluechi/common/cfg.c
@@ -398,39 +398,52 @@ const char *cfg_dump(struct config *config) {
         return cfg_info;
 }
 
-int cfg_agent_def_conf(struct config *config) {
-        int result = cfg_set_default_section(config, CFG_SECT_AGENT);
-        if (result != 0) {
+static int cfg_def_conf(struct config *config) {
+        int result = 0;
+
+        if ((result = cfg_set_value(config, CFG_LOG_LEVEL, log_level_to_string(LOG_LEVEL_INFO))) != 0) {
                 return result;
         }
 
-        _cleanup_free_ char *default_node_name = get_hostname();
-        if ((result = cfg_set_value(config, CFG_NODE_NAME, default_node_name)) != 0) {
+        if ((result = cfg_set_value(config, CFG_LOG_TARGET, BC_LOG_TARGET_JOURNALD)) != 0) {
                 return result;
         }
 
-        if ((result = cfg_set_value(config, CFG_MANAGER_HOST, BC_DEFAULT_HOST)) != 0) {
+        if ((result = cfg_set_value(config, CFG_LOG_IS_QUIET, NULL)) != 0) {
                 return result;
         }
 
-        if ((result = cfg_set_value(config, CFG_HEARTBEAT_INTERVAL, AGENT_HEARTBEAT_INTERVAL_MSEC)) != 0) {
+        if ((result = cfg_set_value(config, CFG_IP_RECEIVE_ERRORS, BC_DEFAULT_IP_RECEIVE_ERROR)) != 0) {
                 return result;
         }
 
-        if ((result = cfg_set_value(config, CFG_MANAGER_PORT, BC_DEFAULT_PORT)) != 0) {
+        return 0;
+}
+
+int cfg_agent_def_conf(struct config *config) {
+        int result = cfg_set_default_section(config, CFG_SECT_AGENT);
+        if (result != 0) {
                 return result;
         }
 
-        const char *LOG_LEVEL_INFO = "INFO";
-        if ((result = cfg_set_value(config, CFG_LOG_LEVEL, LOG_LEVEL_INFO)) != 0) {
+        if ((result = cfg_def_conf(config)) != 0) {
                 return result;
         }
 
-        if ((result = cfg_set_value(config, CFG_LOG_TARGET, BC_LOG_TARGET_JOURNALD)) != 0) {
+        _cleanup_free_ char *default_node_name = get_hostname();
+        if ((result = cfg_set_value(config, CFG_NODE_NAME, default_node_name)) != 0) {
                 return result;
         }
 
-        if ((result = cfg_set_value(config, CFG_LOG_IS_QUIET, NULL)) != 0) {
+        if ((result = cfg_set_value(config, CFG_MANAGER_HOST, BC_DEFAULT_HOST)) != 0) {
+                return result;
+        }
+
+        if ((result = cfg_set_value(config, CFG_HEARTBEAT_INTERVAL, AGENT_HEARTBEAT_INTERVAL_MSEC)) != 0) {
+                return result;
+        }
+
+        if ((result = cfg_set_value(config, CFG_MANAGER_PORT, BC_DEFAULT_PORT)) != 0) {
                 return result;
         }
 
@@ -445,23 +458,15 @@ int cfg_manager_def_conf(struct config *config) {
                 return result;
         }
 
-        if ((result = cfg_set_value(config, CFG_MANAGER_PORT, BC_DEFAULT_PORT)) != 0) {
-                return result;
-        }
-
-        if ((result = cfg_set_value(config, CFG_ALLOWED_NODE_NAMES, "")) != 0) {
+        if ((result = cfg_def_conf(config)) != 0) {
                 return result;
         }
 
-        if ((result = cfg_set_value(config, CFG_LOG_LEVEL, log_level_to_string(LOG_LEVEL_INFO))) != 0) {
-                return result;
-        }
-
-        if ((result = cfg_set_value(config, CFG_LOG_TARGET, BC_LOG_TARGET_JOURNALD)) != 0) {
+        if ((result = cfg_set_value(config, CFG_MANAGER_PORT, BC_DEFAULT_PORT)) != 0) {
                 return result;
         }
 
-        if ((result = cfg_set_value(config, CFG_LOG_IS_QUIET, NULL)) != 0) {
+        if ((result = cfg_set_value(config, CFG_ALLOWED_NODE_NAMES, "")) != 0) {
                 return result;
         }
 

diff --git a/src/libbluechi/common/cfg.h b/src/libbluechi/common/cfg.h
@@ -17,6 +17,7 @@
 #define CFG_NODE_NAME "NodeName"
 #define CFG_ALLOWED_NODE_NAMES "AllowedNodeNames"
 #define CFG_HEARTBEAT_INTERVAL "HeartbeatInterval"
+#define CFG_IP_RECEIVE_ERRORS "IPReceiveErrors"
 
 /*
  * Global section - this is used, when configuration options are specified in the configuration file

diff --git a/src/libbluechi/common/protocol.h b/src/libbluechi/common/protocol.h
@@ -5,6 +5,8 @@
 
 #define BC_DEFAULT_PORT "842"
 #define BC_DEFAULT_HOST "127.0.0.1"
+/* Enable extended reliable error message passing */
+#define BC_DEFAULT_IP_RECEIVE_ERROR "true"
 
 #define BC_DBUS_NAME "org.eclipse.bluechi"
 #define BC_AGENT_DBUS_NAME "org.eclipse.bluechi.Agent"

diff --git a/src/manager/manager.c b/src/manager/manager.c
@@ -41,6 +41,7 @@ Manager *manager_new(void) {
                 manager->api_bus_service_name = steal_pointer(&service_name);
                 manager->event = steal_pointer(&event);
                 manager->metrics_enabled = false;
+                manager->ip_receive_errors = false;
                 LIST_HEAD_INIT(manager->nodes);
                 LIST_HEAD_INIT(manager->anonymous_nodes);
                 LIST_HEAD_INIT(manager->jobs);
@@ -345,6 +346,8 @@ bool manager_parse_config(Manager *manager, const char *configfile) {
                 }
         }
 
+        manager->ip_receive_errors = cfg_get_bool_value(manager->config, CFG_IP_RECEIVE_ERRORS);
+
         _cleanup_free_ const char *dumped_cfg = cfg_dump(manager->config);
         bc_log_debug_with_data("Final configuration used", "\n%s", dumped_cfg);
 
@@ -377,6 +380,12 @@ static int manager_accept_node_connection(
         if (r < 0) {
                 bc_log_warnf("Failed to set KEEPALIVE on socket: %s", strerror(-r));
         }
+        if (manager->ip_receive_errors) {
+                r = bus_socket_enable_recv_err(dbus_server);
+                if (r < 0) {
+                        bc_log_warnf("Failed to enable receiving errors on socket: %s", strerror(-r));
+                }
+        }
 
         /* Add anonymous node */
         node = manager_add_node(manager, NULL);

diff --git a/src/manager/manager.h b/src/manager/manager.h
@@ -26,6 +26,8 @@ struct Manager {
 
         bool metrics_enabled;
 
+        bool ip_receive_errors;
+
         int n_nodes;
         LIST_HEAD(Node, nodes);
         LIST_HEAD(Node, anonymous_nodes);