client: fix configuration modification

Handling of configuration change was messy. Whatever configuration item
needed to be updated, a whole lldpd_config structure was sent. The
daemon part was trying to guess what changed by assuming non-0 fields
need to be updated. However, when flags were added, the implementation
became inconsistent. Some flags used 1/2 for true/false, some others
kept 0/1. So, some flags were detected as changed while they were not.

Since we require to provide a current configuration before making any
change, we just copy the whole structure, modify what we want and send
it to the daemon. The daemon will then compare the new structure with
the old one to detect changes. Flags that were using 1/2 logic are now
just using 0/1 like others.

diff --git a/src/daemon/client.c b/src/daemon/client.c
index ef16874b1a05fd74e149e403418b42a79eb670ca..8603ffd8a2a07d583a3716d9989f9ac6e1034536 100644 (file)
--- a/src/daemon/client.c
+++ b/src/daemon/client.c
@@ -55,35 +55,40 @@ client_handle_set_configuration(struct lldpd *cfg, enum hmsg_type *type,
                *type = NONE;
                return 0;
        }
-       
+
+#define CHANGED(w) (config->w != cfg->g_config.w)
+#define CHANGED_STR(w) (config->w && (!cfg->g_config.w || strcmp(config->w, cfg->g_config.w)))
+
        /* What needs to be done? Transmit delay? */
-       if (config->c_tx_interval > 0) {
+       if (CHANGED(c_tx_interval) && config->c_tx_interval > 0) {
                log_debug("rpc", "client change transmit interval to %d",
                        config->c_tx_interval);
                cfg->g_config.c_tx_interval = config->c_tx_interval;
                LOCAL_CHASSIS(cfg)->c_ttl = cfg->g_config.c_tx_interval *
                        cfg->g_config.c_tx_hold;
        }
-       if (config->c_tx_hold > 0) {
-               log_debug("rpc", "client change transmit hold to %d",
-                       config->c_tx_hold);
-               cfg->g_config.c_tx_hold = config->c_tx_hold;
-               LOCAL_CHASSIS(cfg)->c_ttl = cfg->g_config.c_tx_interval *
-                       cfg->g_config.c_tx_hold;
-       }
-       if (config->c_tx_interval < 0) {
-               log_debug("rpc", "client asked for immediate retransmission");
-               levent_send_now(cfg);
+       if (CHANGED(c_tx_hold) && config->c_tx_hold != 0) {
+               if (config->c_tx_interval < 0) {
+                       log_debug("rpc", "client asked for immediate retransmission");
+                       levent_send_now(cfg);
+               } else {
+                       log_debug("rpc", "client change transmit hold to %d",
+                           config->c_tx_hold);
+                       cfg->g_config.c_tx_hold = config->c_tx_hold;
+                       LOCAL_CHASSIS(cfg)->c_ttl = cfg->g_config.c_tx_interval *
+                           cfg->g_config.c_tx_hold;
+               }
        }
-       if (config->c_lldp_portid_type > LLDP_PORTID_SUBTYPE_UNKNOWN &&
-            config->c_lldp_portid_type <= LLDP_PORTID_SUBTYPE_MAX) {
-            log_debug("rpc", "change lldp portid tlv subtype to %d",
-                      config->c_lldp_portid_type);
-            cfg->g_config.c_lldp_portid_type = config->c_lldp_portid_type;
-           levent_update_now(cfg);
+       if (CHANGED(c_lldp_portid_type) &&
+           config->c_lldp_portid_type > LLDP_PORTID_SUBTYPE_UNKNOWN &&
+           config->c_lldp_portid_type <= LLDP_PORTID_SUBTYPE_MAX) {
+               log_debug("rpc", "change lldp portid tlv subtype to %d",
+                   config->c_lldp_portid_type);
+               cfg->g_config.c_lldp_portid_type = config->c_lldp_portid_type;
+               levent_update_now(cfg);
        }
        /* Pause/resume */
-       if (config->c_paused != cfg->g_config.c_paused) {
+       if (CHANGED(c_paused)) {
                log_debug("rpc", "client asked to %s lldpd",
                    config->c_paused?"pause":"resume");
                cfg->g_config.c_paused = config->c_paused;
@@ -91,59 +96,60 @@ client_handle_set_configuration(struct lldpd *cfg, enum hmsg_type *type,
        }
 
 #ifdef ENABLE_LLDPMED
-       if (config->c_enable_fast_start) {
-               cfg->g_config.c_enable_fast_start = (config->c_enable_fast_start == 1);
+       if (CHANGED(c_enable_fast_start)) {
+               cfg->g_config.c_enable_fast_start = config->c_enable_fast_start;
                log_debug("rpc", "client asked to %s fast start",
                    cfg->g_config.c_enable_fast_start?"enable":"disable");
        }
-       if (config->c_tx_fast_interval) {
+       if (CHANGED(c_tx_fast_interval) &&
+           config->c_tx_fast_interval > 0) {
                log_debug("rpc", "change fast interval to %d", config->c_tx_fast_interval);
                cfg->g_config.c_tx_fast_interval = config->c_tx_fast_interval;
        }
 #endif
-       if (config->c_iface_pattern) {
+       if (CHANGED_STR(c_iface_pattern)) {
                log_debug("rpc", "change interface pattern to %s", config->c_iface_pattern);
                free(cfg->g_config.c_iface_pattern);
                cfg->g_config.c_iface_pattern = strdup(config->c_iface_pattern);
                levent_update_now(cfg);
        }
-       if (config->c_mgmt_pattern) {
+       if (CHANGED_STR(c_mgmt_pattern)) {
                log_debug("rpc", "change management pattern to %s", config->c_mgmt_pattern);
                free(cfg->g_config.c_mgmt_pattern);
                cfg->g_config.c_mgmt_pattern = strdup(config->c_mgmt_pattern);
                levent_update_now(cfg);
        }
-       if (config->c_description) {
+       if (CHANGED_STR(c_description)) {
                log_debug("rpc", "change chassis description to %s", config->c_description);
                free(cfg->g_config.c_description);
                cfg->g_config.c_description = strdup(config->c_description);
                levent_update_now(cfg);
        }
-       if (config->c_platform) {
+       if (CHANGED_STR(c_platform)) {
                log_debug("rpc", "change platform description to %s", config->c_platform);
                free(cfg->g_config.c_platform);
                cfg->g_config.c_platform = strdup(config->c_platform);
                levent_update_now(cfg);
        }
-       if (config->c_hostname) {
+       if (CHANGED_STR(c_hostname)) {
                log_debug("rpc", "change system name to %s", config->c_hostname);
                free(cfg->g_config.c_hostname);
                cfg->g_config.c_hostname = strdup(config->c_hostname);
                levent_update_now(cfg);
        }
-       if (config->c_set_ifdescr != cfg->g_config.c_set_ifdescr) {
+       if (CHANGED(c_set_ifdescr)) {
                log_debug("rpc", "%s setting of interface description based on discovered neighbors",
                    config->c_set_ifdescr?"enable":"disable");
                cfg->g_config.c_set_ifdescr = config->c_set_ifdescr;
                levent_update_now(cfg);
        }
-       if (config->c_promisc != cfg->g_config.c_promisc) {
+       if (CHANGED(c_promisc)) {
                log_debug("rpc", "%s promiscuous mode on managed interfaces",
                    config->c_promisc?"enable":"disable");
                cfg->g_config.c_promisc = config->c_promisc;
                levent_update_now(cfg);
        }
-       if (config->c_bond_slave_src_mac_type != 0) {
+       if (CHANGED(c_bond_slave_src_mac_type)) {
                if (config->c_bond_slave_src_mac_type >
                    LLDP_BOND_SLAVE_SRC_MAC_TYPE_UNKNOWN &&
                    config->c_bond_slave_src_mac_type <=

diff --git a/src/lib/atom-private.c b/src/lib/atom-private.c
index c5dd0fb4b6f07aabc64fcabe1d7ca7981414b59a..de6e7ee20a0a7e43b44fc258c74b0edf98230c0e 100644 (file)
--- a/src/lib/atom-private.c
+++ b/src/lib/atom-private.c
@@ -398,7 +398,8 @@ _lldpctl_atom_set_str_config(lldpctl_atom_t *atom, lldpctl_key_t key,
 {
        struct _lldpctl_atom_config_t *c =
            (struct _lldpctl_atom_config_t *)atom;
-       struct lldpd_config config = {};
+       struct lldpd_config config;
+       memcpy(&config, c->config, sizeof(struct lldpd_config));
        char *iface_pattern = NULL, *mgmt_pattern = NULL;
        char *description = NULL;
        char *canary = NULL;
@@ -513,7 +514,8 @@ _lldpctl_atom_set_int_config(lldpctl_atom_t *atom, lldpctl_key_t key,
        char *canary = NULL;
        struct _lldpctl_atom_config_t *c =
            (struct _lldpctl_atom_config_t *)atom;
-       struct lldpd_config config = {};
+       struct lldpd_config config;
+       memcpy(&config, c->config, sizeof(struct lldpd_config));
 
        switch (key) {
        case lldpctl_k_config_paused:
@@ -531,8 +533,7 @@ _lldpctl_atom_set_int_config(lldpctl_atom_t *atom, lldpctl_key_t key,
                break;
 #ifdef ENABLE_LLDPMED
        case lldpctl_k_config_fast_start_enabled:
-               config.c_enable_fast_start =  value?1:2;
-               c->config->c_enable_fast_start = value;
+               config.c_enable_fast_start = c->config->c_enable_fast_start = value;
                break;
        case lldpctl_k_config_fast_start_interval:
                config.c_tx_fast_interval = c->config->c_tx_fast_interval = value;