[PATCH| lldpd: use bond slave real mac with locally administered bit set as sour

ce man in lldp frames sent on bond slaves

By default lldpd sent src mac of zero for all lldp frames sent on bond
slaves. See https://tickets.cumulusnetworks.com/browse/CM-1283

This patch provides a few options to select the source address in lldp
trames sent on bonded slaves.

The problem with using the bond slave real mac in lldp frames
is described in https://github.com/vincentbernat/lldpd/issues/26

This patch provides 3 choices to src mac address selection on bond slaves:

# lldpcli help configure system bond-slave-src-mac-type

-- Set LLDP bond slave src mac type
      real  Real mac
      zero  All zero mac
     local  Real Mac with locally administered bit set

Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
Signed-off-by: Vincent Bernat <bernat@luffy.cx>

diff --git a/src/client/conf-system.c b/src/client/conf-system.c
index 442a60d7cb1252dd3dae30d30e918ea8f4d4796b..26b6b049f6be90edc6c6a496046f26ec75bf6326 100644 (file)
--- a/src/client/conf-system.c
+++ b/src/client/conf-system.c
@@ -101,6 +101,52 @@ cmd_update_descriptions(struct lldpctl_conn_t *conn, struct writer *w,
        return 1;
 }
 
+static int
+cmd_bondslave_srcmac_type(struct lldpctl_conn_t *conn, struct writer *w,
+    struct cmd_env *env, void *arg)
+{
+       char *value_str;
+       int m = 0, value = -1;
+
+       log_debug("lldpctl", "bond slave src mac");
+
+       lldpctl_atom_t *config = lldpctl_get_configuration(conn);
+       if (config == NULL) {
+               log_warnx("lldpctl",
+                       "unable to get configuration from lldpd. %s",
+                       lldpctl_last_strerror(conn));
+               return 0;
+       }
+
+       value_str = arg;
+       for (lldpctl_map_t *b_map =
+               lldpctl_key_get_map(lldpctl_k_config_bond_slave_src_mac_type);
+               b_map->string; b_map++) {
+               if (!strcmp(b_map->string, value_str)) {
+                       value = b_map->value;
+                       break;
+               }
+       }
+
+       if (value == -1) {
+               log_warnx("lldpctl", "invalid value");
+               return 0;
+       }
+
+       if (lldpctl_atom_set_int(config,
+               lldpctl_k_config_bond_slave_src_mac_type, value) == NULL) {
+               log_warnx("lldpctl", "unable to set bond slave src mac type."
+                       " %s", lldpctl_last_strerror(conn));
+               lldpctl_atom_dec_ref(config);
+               return 0;
+       }
+
+       log_info("lldpctl", "bond slave src mac set to new value : %s", arg);
+       lldpctl_atom_dec_ref(config);
+
+       return 1;
+}
+
 /**
  * Register `configure system` commands.
  *
@@ -157,6 +203,46 @@ register_commands_configure_system(struct cmd_node *configure,
                NEWLINE, "Set active interface pattern",
                NULL, cmd_iface_pattern, NULL);
 
+       struct cmd_node *bond_slave_src_mac_type =
+               commands_new(configure_system,
+                       "bond-slave-src-mac-type",
+                       "Set LLDP bond slave src mac type",
+                       NULL, NULL, NULL);
+
+       for (lldpctl_map_t *b_map =
+               lldpctl_key_get_map(lldpctl_k_config_bond_slave_src_mac_type);
+               b_map->string; b_map++) {
+               /* XXX: Would have been nice to encode the "Help" string in
+                * the map. Need to enhance the map struct for that
+                */
+               if (!strcmp(b_map->string, "real")) {
+                       commands_new(
+                               commands_new(bond_slave_src_mac_type,
+                                       b_map->string, "Real mac",
+                                       NULL, NULL, NULL),
+                                       NEWLINE, NULL,
+                                       NULL, cmd_bondslave_srcmac_type,
+                                       b_map->string);
+               } else if (!strcmp(b_map->string, "zero")) {
+                       commands_new(
+                               commands_new(bond_slave_src_mac_type,
+                                       b_map->string, "All zero mac",
+                                       NULL, NULL, NULL),
+                                       NEWLINE, NULL,
+                                       NULL, cmd_bondslave_srcmac_type,
+                                       b_map->string);
+               } else if (!strcmp(b_map->string, "local")) {
+                       commands_new(
+                               commands_new(bond_slave_src_mac_type,
+                                       b_map->string, "Real Mac with locally "
+                                       "administered bit set",
+                                       NULL, NULL, NULL),
+                                       NEWLINE, NULL,
+                                       NULL, cmd_bondslave_srcmac_type,
+                                       b_map->string);
+               }
+       }
+
        commands_new(
                commands_new(configure_interface,
                    "description", "Update interface descriptions with neighbor name",

diff --git a/src/client/display.c b/src/client/display.c
index 7e83d615b6a81ca6f88e0d902120b90c417727f0..3dcc23f43de52520fb32fdd5973d4d8c66d56223 100644 (file)
--- a/src/client/display.c
+++ b/src/client/display.c
@@ -737,6 +737,10 @@ display_configuration(lldpctl_conn_t *conn, struct writer *w)
            "no":"yes");
        tag_datatag(w, "lldpmed-faststart-interval", "LLDP-MED fast start interval",
            N(lldpctl_atom_get_str(configuration, lldpctl_k_config_fast_start_interval)));
+       tag_datatag(w, "bond-slave-src-mac-type",
+               "Src mac for lldp frames on bond slaves",
+               lldpctl_atom_get_str(configuration,
+                       lldpctl_k_config_bond_slave_src_mac_type));
 
        tag_end(w);
        tag_end(w);

diff --git a/src/client/lldpcli.8 b/src/client/lldpcli.8
index 5d419701bf5e339cb1d97deddfa6e4f2a885c0f3..b22c772def1dae54e62c8af645e703a6775ec94c 100644 (file)
--- a/src/client/lldpcli.8
+++ b/src/client/lldpcli.8
@@ -227,6 +227,28 @@ value and of the transmit delay. The default value is 4 and therefore
 the default TTL is 120 seconds.
 .Ed
 
+.Cd configure
+.Cd system bond-slave-src-mac-type Ar value
+.Bd -ragged -offset XXXXXX
+Set the type of src mac in lldp frames sent on bond slaves
+
+Valid types are:
+.Bl -tag -width "XXX." -compact -offset XX
+.It Sy real
+Slave real mac
+.It Sy zero
+All zero mac
+.It Sy local
+Real mac with locally administered bit set. If the real mac already
+has the locally administered bit set, default to zero mac.
+.El
+.Pp
+Default value for
+.Nm bond-slave-src-mac-type
+is
+.Nm local
+.Ed
+
 .Cd configure med fast-start
 .Cd enable | tx-interval Ar interval
 .Bd -ragged -offset XXXXXX

diff --git a/src/daemon/client.c b/src/daemon/client.c
index 325104eec99f56c7134ea0e7c232f1520a3d6988..0c2c4c0fb7428ef12145a02e9fd6135aa18b7e87 100644 (file)
--- a/src/daemon/client.c
+++ b/src/daemon/client.c
@@ -117,6 +117,18 @@ client_handle_set_configuration(struct lldpd *cfg, enum hmsg_type *type,
                cfg->g_config.c_set_ifdescr = config->c_set_ifdescr;
                levent_update_now(cfg);
        }
+       if (config->c_bond_slave_src_mac_type >
+               LLDP_BOND_SLAVE_SRC_MAC_TYPE_UNKNOWN &&
+               config->c_bond_slave_src_mac_type <=
+               LLDP_BOND_SLAVE_SRC_MAC_TYPE_MAX) {
+               log_debug("rpc", "change bond src mac type to %d",
+                               config->c_bond_slave_src_mac_type);
+               cfg->g_config.c_bond_slave_src_mac_type =
+                               config->c_bond_slave_src_mac_type;
+       } else {
+               log_info("rpc", "Invalid bond slave src mac type: %d\n",
+                               config->c_bond_slave_src_mac_type);
+       }
 
        lldpd_config_cleanup(config);
        free(config);

diff --git a/src/daemon/interfaces-linux.c b/src/daemon/interfaces-linux.c
index ebd374eba9150762eda7b20e1fdb7e75faf36753..10c04c82f190e3a96997deb35776084d45bdc703 100644 (file)
--- a/src/daemon/interfaces-linux.c
+++ b/src/daemon/interfaces-linux.c
@@ -474,8 +474,29 @@ iface_bond_init(struct lldpd *cfg, struct lldpd_hardware *hardware)
  * already set to 1, use an unused MAC address instead.
  */
 static void
-iface_mangle_mac(char *buffer)
+iface_mangle_mac(struct lldpd *cfg, char *src_mac)
 {
+#define MAC_UL_ADMINISTERED_BIT_MASK 0x02
+
+       if (cfg->g_config.c_bond_slave_src_mac_type ==
+               LLDP_BOND_SLAVE_SRC_MAC_TYPE_LOCALLY_ADMINISTERED) {
+               if (*src_mac & MAC_UL_ADMINISTERED_BIT_MASK)
+                       /* If locally administered bit already set,
+                        * use zero mac
+                        */
+                       memset(src_mac, 0, ETHER_ADDR_LEN);
+               else
+                       *src_mac |= MAC_UL_ADMINISTERED_BIT_MASK;
+       } else if (cfg->g_config.c_bond_slave_src_mac_type ==
+                       LLDP_BOND_SLAVE_SRC_MAC_TYPE_ZERO) {
+               memset(src_mac, 0, ETHER_ADDR_LEN);
+       }
+       /* If cfg->g_config.c_bond_slave_src_mac_type is
+        * LLDP_BOND_SLAVE_SRC_MAC_TYPE_UNKNOWN or
+        * LLDP_BOND_SLAVE_SRC_MAC_TYPE_REAL, dont change mac
+        */
+
+#if 0
        if (buffer[0] & 2) {
                /* Already a locally administered MAC address, use a fixed MAC
                 * address (an old 3c905 MAC address of a card that I own). */
@@ -484,6 +505,7 @@ iface_mangle_mac(char *buffer)
                return;
        }
        buffer[0] |= 2;
+#endif
 }
 
 static int
@@ -498,7 +520,7 @@ iface_bond_send(struct lldpd *cfg, struct lldpd_hardware *hardware,
                    hardware->h_ifname);
                return 0;
        }
-       iface_mangle_mac(buffer + ETHER_ADDR_LEN);
+       iface_mangle_mac(cfg, buffer + ETHER_ADDR_LEN);
        return write(hardware->h_sendfd,
            buffer, size);
 }

diff --git a/src/daemon/lldpd.c b/src/daemon/lldpd.c
index 9ef39fb3b6e1b821d390063e6b7ef5ab561c87be..c74ca17750385b68ee54aff3b8b6a25118f358e6 100644 (file)
--- a/src/daemon/lldpd.c
+++ b/src/daemon/lldpd.c
@@ -1480,6 +1480,8 @@ lldpd_main(int argc, char *argv[], char *envp[])
        cfg->g_snmp = snmp;
        cfg->g_snmp_agentx = agentx;
 #endif /* USE_SNMP */
+       cfg->g_config.c_bond_slave_src_mac_type = \
+           LLDP_BOND_SLAVE_SRC_MAC_TYPE_LOCALLY_ADMINISTERED;
 
        /* Get ioctl socket */
        log_debug("main", "get an ioctl socket");

diff --git a/src/lib/atom-private.c b/src/lib/atom-private.c
index 0cbf1c0bfc0746c2573f7e1d6be9f0b3dd30f71d..06886c0fa2158b923421ccfe4b52926c199e219b 100644 (file)
--- a/src/lib/atom-private.c
+++ b/src/lib/atom-private.c
@@ -260,6 +260,13 @@ static lldpctl_map_t port_med_pow_source_map2[] = {
 static lldpctl_map_t *port_med_pow_priority_map = port_dot3_power_priority_map;
 #endif
 
+static lldpctl_map_t bond_slave_src_mac_map[] = {
+       { LLDP_BOND_SLAVE_SRC_MAC_TYPE_REAL,   "real"},
+       { LLDP_BOND_SLAVE_SRC_MAC_TYPE_ZERO,   "zero"},
+       { LLDP_BOND_SLAVE_SRC_MAC_TYPE_LOCALLY_ADMINISTERED, "local" },
+       { LLDP_BOND_SLAVE_SRC_MAC_TYPE_UNKNOWN, NULL},
+};
+
 static const char*
 map_lookup(lldpctl_map_t *list, int n)
 {
@@ -316,6 +323,8 @@ lldpctl_key_get_map(lldpctl_key_t key)
        case lldpctl_k_dot3_power_priority:
                return port_dot3_power_priority_map;
 #endif
+       case lldpctl_k_config_bond_slave_src_mac_type:
+               return bond_slave_src_mac_map;
        default: return empty_map;
        }
 }
@@ -357,6 +366,9 @@ _lldpctl_atom_get_str_config(lldpctl_atom_t *atom, lldpctl_key_t key)
                res = c->config->c_description; break;
        case lldpctl_k_config_platform:
                res = c->config->c_platform; break;
+       case lldpctl_k_config_bond_slave_src_mac_type:
+               return map_lookup(bond_slave_src_mac_map,
+                               c->config->c_bond_slave_src_mac_type);
        default:
                SET_ERROR(atom->conn, LLDPCTL_ERR_NOT_EXIST);
                return NULL;
@@ -484,6 +496,10 @@ _lldpctl_atom_set_int_config(lldpctl_atom_t *atom, lldpctl_key_t key,
                config.c_tx_hold = value;
                if (value > 0) c->config->c_tx_hold = value;
                break;
+       case lldpctl_k_config_bond_slave_src_mac_type:
+               config.c_bond_slave_src_mac_type = value;
+               c->config->c_bond_slave_src_mac_type = value;
+               break;
        default:
                SET_ERROR(atom->conn, LLDPCTL_ERR_NOT_EXIST);
                return NULL;

diff --git a/src/lib/lldpctl.h b/src/lib/lldpctl.h
index 171ac23eeabeaf149f95435574fef3d2e9bfeb97..a3f5bb751ff730a3b670047316ca420881f12925 100644 (file)
--- a/src/lib/lldpctl.h
+++ b/src/lib/lldpctl.h
@@ -708,6 +708,7 @@ typedef enum {
        lldpctl_k_insert_cnt,   /**< `(I)` insert cnt. Only works for a local port. */
        lldpctl_k_delete_cnt,   /**< `(I)` delete cnt. Only works for a local port. */
        lldpctl_k_config_tx_hold, /**< `(I,WO)` Transmit hold interval. */
+       lldpctl_k_config_bond_slave_src_mac_type, /**< `(I,WO)` bond slave src mac type. */
 } lldpctl_key_t;
 
 /**

diff --git a/src/lldp-const.h b/src/lldp-const.h
index b595365158d3e536e56d752f0d9a3c1a505493a4..1c5248941ddad594fac58e9aae94cb05c506f273 100644 (file)
--- a/src/lldp-const.h
+++ b/src/lldp-const.h
@@ -216,4 +216,11 @@
 #define LLDPD_MODE_MAX         LLDPD_MODE_FDP
 
 
+/* Bond slave src mac type constants */
+#define LLDP_BOND_SLAVE_SRC_MAC_TYPE_UNKNOWN   0
+#define LLDP_BOND_SLAVE_SRC_MAC_TYPE_REAL      1
+#define LLDP_BOND_SLAVE_SRC_MAC_TYPE_ZERO      2
+#define LLDP_BOND_SLAVE_SRC_MAC_TYPE_LOCALLY_ADMINISTERED 3
+#define LLDP_BOND_SLAVE_SRC_MAC_TYPE_MAX LLDP_BOND_SLAVE_SRC_MAC_TYPE_LOCALLY_ADMINISTERED
+
 #endif /* _LLDP_H */

diff --git a/src/lldpd-structs.h b/src/lldpd-structs.h
index 67c047f50802f4271dc7e0e5f54be0fc9cdec09c..396c2e1655133d1b3ac720101731f64e0320e98a 100644 (file)
--- a/src/lldpd-structs.h
+++ b/src/lldpd-structs.h
@@ -333,6 +333,8 @@ struct lldpd_config {
        int c_tx_fast_interval; /* Time intr between sends during fast start */
 #endif
        int c_tx_hold;          /* Transmit hold */
+       int c_bond_slave_src_mac_type; /* Src mac type in lldp frames over bond
+                                         slaves */
 };
 MARSHAL_BEGIN(lldpd_config)
 MARSHAL_STR(lldpd_config, c_mgmt_pattern)