lldpad: fix link flap bug This patch enhances the entire link event processing mechanism in lldpad to improve the reliability of detecting and handling events correctly. Signed-off-by: Eric Multanen --- config.c | 62 +++++++++++--- dcb_protocol.c | 31 +++++-- event_iface.c | 210 +++++++++++++++++++++++-------------------------- include/drv_cfg.h | 2 include/event_iface.h | 1 include/lldp_util.h | 1 lldp/ports.c | 38 +++++++++ lldp/ports.h | 3 + lldp/rx.c | 1 lldp/tx.c | 2 lldp_dcbx.c | 4 - lldp_rtnl.c | 83 ++++++++++++++++++- lldp_util.c | 9 ++ 13 files changed, 301 insertions(+), 146 deletions(-) -- diff --git a/config.c b/config.c index 67998a0..72e4518 100644 --- a/config.c +++ b/config.c @@ -94,6 +94,11 @@ void scan_port(void *eloop_data, void *user_ctx) { struct port *port; struct if_nameindex *nameidx, *p; + struct lldp_module *np; + const struct lldp_mod_ops *ops; + int err; + + printf("%s: NLMSG dropped, scan ports.\n", __func__); nameidx = if_nameindex(); if (nameidx == NULL) { @@ -105,24 +110,50 @@ void scan_port(void *eloop_data, void *user_ctx) if (!init_cfg()) goto err_out; - while (p->if_index != 0) { - if (is_bond(p->if_name) == 0 && - is_vlan(p->if_name) == 0 && - check_link_status(p->if_name) > 0) { - port = porthead; - while (port != NULL) { - if (!strncmp(p->if_name, port->ifname, - MAX_DEVICE_NAME_LEN)) - /* If device is already in the port list - * don't need to add adapter - */ - goto check_next; - port = port->next; + /* Walk port list looking for devices that are not in if_nameindex. + * If the device is in the port list but not in the if_nameindex + * list then we missed a RTM_DELLINK event and the device is no + * longer available, possibly because the module has been unloaded. + * For this case lets remove the device from the ports list if it + * comes back online we should receive a RTM_NEWLINK event and can + * readd it there. + */ + port = porthead; + while (port != NULL) { + int found = 0; + struct port *del; + p = nameidx; + while (p->if_index != 0) { + if (!strncmp(p->if_name, port->ifname, + MAX_DEVICE_NAME_LEN)) { + /* Good device exists continue port walk */ + found = 1; + break; } + p++; + } + del = port; + port = port->next; + if (!found) + remove_adapter(del->ifname); + } - add_adapter(p->if_name); + /* Walk port list looking for devices that should have been added + * to our port list but have not most likely due to a dropped nlmsg. + * At this point we need to add the device and call ops ifup routines. + * Additional we call set_hw_all() for all devices already in the port + * list this should force the hw settings to get update if needed. + * This is required because we currently do not know if we missed + * IF_OPER_UP or IF_OPER_DORMANT. And an extra set_hw_all() + * shouldn't hurt anything. + */ + p = nameidx; + while (p->if_index != 0) { + if (is_valid_lldp_device(p->if_name) && + check_link_status(p->if_name) > 0) { + oper_add_device(p->if_name); + set_hw_all(p->if_name); } -check_next: p++; } @@ -368,6 +399,7 @@ void init_ports(void) LIST_FOREACH(np, &lldp_head, lldp) np->ops->lldp_mod_ifup(p->if_name); + set_lldp_port_enable_state(p->if_name, 1); } p++; } diff --git a/dcb_protocol.c b/dcb_protocol.c index 7305456..0407c27 100644 --- a/dcb_protocol.c +++ b/dcb_protocol.c @@ -35,6 +35,7 @@ #include "messages.h" #include "lldp.h" #include "tlv_dcbx.h" +#include static void handle_opermode_true(char *device_name); u8 gdcbx_subtype = dcbx_subtype2; @@ -1073,9 +1074,6 @@ int dcbx_add_adapter(char *device_name) } } - /* apply all feature setting to the driver:linux only */ - set_hw_all(device_name); - EventFlag = 0; DCB_SET_FLAGS(EventFlag, DCB_LOCAL_CHANGE_PG | DCB_LOCAL_CHANGE_PFC| DCB_LOCAL_CHANGE_APPTLV); @@ -1258,11 +1256,14 @@ boolean_t remove_adapter(char *device_name) not_default = memcmp(DEF_CFG_STORE, device_name, strlen(DEF_CFG_STORE)); - strncpy (devName, device_name, MAX_DEVICE_NAME_LEN); + strncpy(devName, device_name, MAX_DEVICE_NAME_LEN); - if (not_default && (remove_port(devName) < 0)) { - printf("remove_port: failed\n"); - retval = FALSE; + if (not_default) { + set_linkmode(device_name, 0); + if (remove_port(devName) < 0) { + printf("remove_port: failed\n"); + retval = FALSE; + } } return retval; @@ -1324,6 +1325,14 @@ int dcbx_remove_all(void) void remove_all_adapters() { char sTmp[MAX_DEVICE_NAME_LEN*2]; + struct port *port, *p; + + port = porthead; + while (port != NULL) { + p = port; + port = port->next; + remove_adapter(p->ifname); + } /* the default attributes object needs to be removed last */ if (remove_adapter(DEF_CFG_STORE) == FALSE) { @@ -4094,6 +4103,7 @@ dcb_result run_dcb_protocol(char *device_name, u32 EventFlag, u32 Subtype) dcb_result result = dcb_success; boolean_t LocalChange = FALSE; u32 i, SubTypeMin, SubTypeMax; + int oper; printf("running DCB protocol for %s, flags:%04x\n", device_name, EventFlag); @@ -4174,7 +4184,12 @@ dcb_result run_dcb_protocol(char *device_name, u32 EventFlag, u32 Subtype) } /* apply all feature setting to the driver: linux only */ - set_hw_all(device_name); + oper = get_operstate(device_name); + if (oper == IF_OPER_UP || oper == IF_OPER_UNKNOWN) { + printf("%s: %s: Managed DCB device coming online, program HW\n", + __func__, device_name); + set_hw_all(device_name); + } /* Run the control state machine. */ result = run_control_protocol(device_name, EventFlag); diff --git a/event_iface.c b/event_iface.c index 84d5c14..5a5fd70 100644 --- a/event_iface.c +++ b/event_iface.c @@ -45,44 +45,15 @@ #include "config.h" #include "lldp/l2_packet.h" #include "config.h" +#include "lldp/states.h" static void event_if_decode_rta(int type, struct rtattr *rta); #define MAX_PAYLOAD 4096 /* maximum payload size*/ -#define LINK_UP 1 -#define LINK_DOWN 2 -#define LINK_DORMANT 3 -#define DEVICE_REMOVE 4 -#define DEVICE_ADD 5 static char *device_name = NULL; static int link_status = 0; -static char *decode_oper_state(int operstate) -{ - switch(operstate) { - case IF_OPER_UNKNOWN: - return "IF_OPER_UNKNOWN"; - case IF_OPER_NOTPRESENT: - return "IF_OPER_NOTPRESENT"; - case IF_OPER_DOWN: - link_status = LINK_DOWN; - return "IF_OPER_DOWN"; - case IF_OPER_LOWERLAYERDOWN: - return "IF_OPER_LOWERLAYERDOWN"; - case IF_OPER_TESTING: - return "IF_OPER_TESTING"; - case IF_OPER_DORMANT: - link_status = LINK_DORMANT; - return "IF_OPER_DORMANT"; - case IF_OPER_UP: - link_status = LINK_UP; - return "IF_OPER_UP"; - default: - return "UNKNOWN"; - } -} - static void event_if_decode_rta(int type, struct rtattr *rta) { @@ -97,7 +68,7 @@ static void event_if_decode_rta(int type, struct rtattr *rta) break; case IFLA_OPERSTATE: TRACE1(" IFLA_OPERSTATE ", type); - decode_oper_state(*((int *)RTA_DATA(rta))); + link_status = (*((int *)RTA_DATA(rta))); break; case IFLA_LINKMODE: TRACE1(" IFLA_LINKMODE ", type) @@ -115,14 +86,55 @@ static void event_if_decode_rta(int type, struct rtattr *rta) } } +int oper_add_device(char *device_name) +{ + struct lldp_module *np; + const struct lldp_mod_ops *ops; + struct port *port; + int err; + + port = porthead; + while (port != NULL) { + if (!strncmp(device_name, port->ifname, MAX_DEVICE_NAME_LEN)) + break; + port = port->next; + } + + if (!port) { + if (is_bond(device_name)) + err = add_bond_port(device_name); + else + err = add_adapter(device_name); + + if (err) { + printf("%s: Error adding device %s\n", + __func__, device_name); + return err; + } else + printf("%s: Adding device %s\n", + __func__, device_name); + } else if (!port->portEnabled) + reinit_port(device_name); + + if (!port || !port->portEnabled) { + LIST_FOREACH(np, &lldp_head, lldp) { + ops = np->ops; + if (ops->lldp_mod_ifup) + ops->lldp_mod_ifup(device_name); + } + } + set_lldp_port_enable_state(device_name, 1); + return 0; +} + static void event_if_decode_nlmsg(int route_type, void *data, int len) { struct lldp_module *np; const struct lldp_mod_ops *ops; struct rtattr *rta; + struct port *port; int attrlen; - int val; - int err; + int val, err; switch (route_type) { case RTM_NEWLINK: @@ -152,95 +164,71 @@ static void event_if_decode_nlmsg(int route_type, void *data, int len) TRACE1("link status: ", link_status); TRACE2("device name: ", device_name); - if (link_status) { - switch (link_status) { - case LINK_DOWN: - printf("******* LINK DOWN: %s\n", device_name); - /* Don't need to check the dcb status */ - if (is_bond(device_name)) { - err = remove_bond_port(device_name); - if (err < 0) - syslog(LOG_ERR, "failed to " - "remove bond port %s", - device_name); - break; - } - if (is_vlan(device_name)) - break; - - val = get_port_hw_resetting(device_name); - - if (val != 1) { - if (!init_cfg()) - break; - - LIST_FOREACH(np, &lldp_head, lldp) { - ops = np->ops; - if (ops->lldp_mod_ifdown) - ops->lldp_mod_ifdown( - device_name); - } - - remove_adapter(device_name); - destroy_cfg(); - break; - } + switch (link_status) { + case IF_OPER_DOWN: + printf("******* LINK DOWN: %s\n", device_name); - printf("******* IGNORING: %s\n", device_name); - set_lldp_port_enable_state(device_name, 0); + err = is_valid_lldp_device(device_name); + if (!err) break; - case LINK_DORMANT: - printf("******* LINK DORMANT: %s\n", - device_name); - set_port_oper_delay(device_name); - case LINK_UP: - printf("******* LINK UP: %s\n", device_name); - - if (is_bond(device_name)) { - err = add_bond_port(device_name); - if (err < 0) - syslog(LOG_ERR, "failed to " - "register bond port %s", - device_name); - break; - } - if (is_vlan(device_name)) + port = porthead; + while (port != NULL) { + if (!strncmp(device_name, port->ifname, + MAX_DEVICE_NAME_LEN)) break; + port = port->next; + } - if (is_bridge(device_name)) - break; + if (!port) { + printf("%s: %s: Unknown device!\n", + __func__, device_name); - err = get_port_hw_resetting(device_name); - if (err == 1) { - printf("******* IGNORING: %s\n", - device_name); - set_port_hw_resetting( - device_name, 0); - set_lldp_port_enable_state( - device_name, 1); - break; - } + break; + } - err = add_adapter(device_name); + if (!init_cfg()) + break; + + LIST_FOREACH(np, &lldp_head, lldp) { + ops = np->ops; + if (ops->lldp_mod_ifdown) + ops->lldp_mod_ifdown(device_name); + } - LIST_FOREACH(np, &lldp_head, lldp) { - ops = np->ops; - if (ops->lldp_mod_ifup) - ops->lldp_mod_ifup(device_name); - } + destroy_cfg(); - if (!err) - printf("Device %s has been " - "added.\n", - device_name); + /* Disable Port */ + set_lldp_port_enable_state(device_name, 0); + if (route_type == RTM_DELLINK) { + printf("%s: %s: device removed!\n", + __func__, device_name); + remove_adapter(device_name); + } + break; + case IF_OPER_DORMANT: + printf("******* LINK DORMANT: %s\n", device_name); + err = is_valid_lldp_device(device_name); + if (!err) break; - default: + set_port_oper_delay(device_name); + oper_add_device(device_name); + break; + case IF_OPER_UP: + printf("******* LINK UP: %s\n", device_name); + err = is_valid_lldp_device(device_name); + if (!err) break; - } - } - break; + oper_add_device(device_name); + printf("%s: %s: Programming HW on LINK_UP\n", + __func__, device_name); + set_hw_all(device_name); + break; + default: + break; + } + break; case RTM_NEWADDR: case RTM_DELADDR: case RTM_GETADDR: @@ -282,7 +270,7 @@ static void event_iface_receive(int sock, void *eloop_ctx, void *sock_ctx) TRACE("PRINT BUF info.\n") device_name = NULL; - link_status = 0; + link_status = IF_OPER_UNKNOWN; nlh = (struct nlmsghdr *)buf; event_if_process_recvmsg(nlh); } diff --git a/include/drv_cfg.h b/include/drv_cfg.h index 0a4f6cb..a843310 100644 --- a/include/drv_cfg.h +++ b/include/drv_cfg.h @@ -177,7 +177,7 @@ struct tc_config { /* Maximum size of response requested or message sent */ -#define MAX_MSG_SIZE 1024 +#define MAX_MSG_SIZE 4096 int get_perm_hwaddr(const char *ifname, u8 *buf_perm, u8 *buf_san); int set_hw_state(char *device_name, int dcb_state); diff --git a/include/event_iface.h b/include/event_iface.h index 051bf14..b2c93f0 100644 --- a/include/event_iface.h +++ b/include/event_iface.h @@ -30,6 +30,7 @@ int event_iface_init(void); int event_iface_deinit(void); +int oper_add_device(char *device_name); void sendto_event_iface(char *buf, int len); diff --git a/include/lldp_util.h b/include/lldp_util.h index bca2ebd..3353067 100644 --- a/include/lldp_util.h +++ b/include/lldp_util.h @@ -32,6 +32,7 @@ #define ETHTOOL_GLINK 0x0000000a /* Get link status (ethtool_value) */ +int is_valid_lldp_device(const char *ifname); int is_active(const char *ifname); int is_bond(const char *ifname); int is_san_mac(u8 *addr); diff --git a/lldp/ports.c b/lldp/ports.c index 8e38f7a..75eab99 100644 --- a/lldp/ports.c +++ b/lldp/ports.c @@ -225,6 +225,44 @@ int get_port_hw_resetting(const char *ifname) return 0; } +int reinit_port(const char *ifname) +{ + struct port *port; + + port = porthead; + while (port != NULL) { + if (!strncmp(ifname, port->ifname, MAX_DEVICE_NAME_LEN)) + break; + port = port->next; + } + + if (!port) + return -1; + + /* Reset relevant port variables */ + port->tx.state = TX_LLDP_INITIALIZE; + port->rx.state = LLDP_WAIT_PORT_OPERATIONAL; + port->hw_resetting = FALSE; + port->portEnabled = FALSE; + port->tx.txTTL = 0; + port->msap.length1 = 0; + port->msap.msap1 = NULL; + port->msap.length2 = 0; + port->msap.msap2 = NULL; + port->lldpdu = FALSE; + port->timers.dormantDelay = DORMANT_DELAY; + + /* init & enable RX path */ + rxInitializeLLDP(port); + + /* init TX path */ + txInitializeLLDP(port); + port->tlvs.last_peer = NULL; + port->tlvs.cur_peer = NULL; + + return 0; +} + int add_port(const char *ifname) { struct port *newport; diff --git a/lldp/ports.h b/lldp/ports.h index 51de561..0138efe 100644 --- a/lldp/ports.h +++ b/lldp/ports.h @@ -178,6 +178,9 @@ int get_neighbor_tlvs(char *ifname, unsigned char *tlvs, int *size); int port_needs_shutdown(struct port *port); +void set_port_operstate(const char *ifname, int operstate); +int get_port_operstate(const char *ifname); + static inline struct port *port_find_by_name(const char *ifname) { struct port *port = porthead; diff --git a/lldp/rx.c b/lldp/rx.c index 6037a08..b950839 100644 --- a/lldp/rx.c +++ b/lldp/rx.c @@ -471,7 +471,6 @@ void run_rx_sm(struct port *port, int update_timers) boolean_t set_rx_state(struct port *port) { if ((port->rx.rxInfoAge == FALSE) && (port->portEnabled == FALSE)) { - printf("set_rx_state: portEnabled==FALSE \n"); rx_change_state(port, LLDP_WAIT_PORT_OPERATIONAL); } diff --git a/lldp/tx.c b/lldp/tx.c index 317a2e2..09f0f2e 100644 --- a/lldp/tx.c +++ b/lldp/tx.c @@ -118,7 +118,6 @@ void txInitializeLLDP(struct port *port) port->timers.txDelay = FASTSTART_TX_DELAY; port->timers.txShutdownWhile = 0; port->timers.txDelayWhile = 0; - l2_packet_get_port_state(port->l2, (u8 *)&(port->portEnabled)); return; } @@ -332,7 +331,6 @@ void tx_change_state(struct port *port, u8 newstate) port->portEnabled) { printf("tx_change_state: TX_LLDP_INITIALIZE \n"); assert(port->portEnabled); - assert(port->tx.state == TX_SHUTDOWN_FRAME); } break; case TX_IDLE: diff --git a/lldp_dcbx.c b/lldp_dcbx.c index 3ef4262..5f04d3a 100644 --- a/lldp_dcbx.c +++ b/lldp_dcbx.c @@ -91,8 +91,8 @@ static int dcbx_check_operstate(struct port *port) app_good = 1; if ((pfc_good && app_good) || port->timers.dormantDelay == 1) { - printf("%s: IF_OPER_UP delay, %u pfc oper %u app oper %u\n", - __func__, port->timers.dormantDelay, + printf("%s: %s: IF_OPER_UP delay, %u pfc oper %u app oper %u\n", + __func__, port->ifname, port->timers.dormantDelay, pfc_data.protocol.OperMode, app_data.protocol.OperMode); port->timers.dormantDelay = 0; diff --git a/lldp_rtnl.c b/lldp_rtnl.c index fed3ce6..1c0f648 100644 --- a/lldp_rtnl.c +++ b/lldp_rtnl.c @@ -52,11 +52,13 @@ #include "lldp_rtnl.h" #include "messages.h" +#include "common.h" #define NLMSG(c) ((struct nlmsghdr *) (c)) #define LOG(...) log_message(MSG_INFO_DEBUG_STRING, __VA_ARGS__) +#define NLMSG_SIZE 1024 /** * rtnl_recv - receive from a routing netlink socket * @s: routing netlink socket with data ready to be received @@ -207,10 +209,59 @@ static ssize_t rtnl_send_operstate(int s, int ifindex, return send(s, &req, req.nh.nlmsg_len, 0); } -static int rtnl_set_operstate(int ifindex, char *ifname, __u8 operstate) +static ssize_t rtnl_recv_operstate(int s, int ifindex, + char *ifname, __u8 *operstate) +{ + struct nlmsghdr *nlh; + struct ifinfomsg *ifi; + struct rtattr *rta; + int attrlen; + int rc = -1; + + nlh = malloc(NLMSG_SIZE); + if (!nlh) + return rc; + + /* send ifname request */ + nlh->nlmsg_len = NLMSG_LENGTH(sizeof(struct ifinfomsg)); + nlh->nlmsg_type = RTM_GETLINK; + nlh->nlmsg_flags = NLM_F_REQUEST; + + ifi = NLMSG_DATA(nlh); + ifi->ifi_family = AF_UNSPEC; + ifi->ifi_index = ifindex; + + if (ifname) + add_rtattr(nlh, IFLA_IFNAME, ifname, strlen(ifname)); + + rc = send(s, nlh, nlh->nlmsg_len, 0); + if (rc < 0) + goto out; + + /* recv ifname reply */ + memset(nlh, 0, NLMSG_SIZE); + rc = recv(s, (void *) nlh, NLMSG_SIZE, MSG_DONTWAIT); + if (rc < 0) + goto out; + ifi = NLMSG_DATA(nlh); + rta = IFLA_RTA(ifi); + attrlen = NLMSG_PAYLOAD(nlh, 0) - sizeof(struct ifinfomsg); + while (RTA_OK(rta, attrlen)) { + if (rta->rta_type == IFLA_OPERSTATE) + memcpy(operstate, RTA_DATA(rta), sizeof(__u8)); + rta = RTA_NEXT(rta, attrlen); + } + +out: + free(nlh); + return rc; +} + +int set_operstate(char *ifname, __u8 operstate) { int s; int rc; + int ifindex = 0; s = socket(PF_NETLINK, SOCK_DGRAM, NETLINK_ROUTE); if (s < 0) @@ -224,13 +275,33 @@ out: return rc; } -int set_linkmode(char *ifname, __u8 linkmode) +int get_operstate(char *ifname) { - return rtnl_set_linkmode(0, ifname, linkmode); + int s, ifq; + int ifindex = 0; + struct ifreq ifr; + __u8 operstate; + + /* fill in ifr_ifindex for kernel versions that require it */ + ifq = socket(PF_PACKET, SOCK_DGRAM, 0); + if (ifq < 0) + return ifq; + os_strncpy(ifr.ifr_name, ifname, sizeof(ifr.ifr_name) - 1); + if (ioctl(ifq, SIOCGIFINDEX, &ifr) == 0) + ifindex = ifr.ifr_ifindex; + close(ifq); + + s = socket(PF_NETLINK, SOCK_DGRAM, NETLINK_ROUTE); + if (s < 0) + return s; + + rtnl_recv_operstate(s, ifindex, ifname, &operstate); +out: + close(s); + return operstate; } -int set_operstate(char *ifname, __u8 operstate) +int set_linkmode(char *ifname, __u8 linkmode) { - return rtnl_set_operstate(0, ifname, operstate); + return rtnl_set_linkmode(0, ifname, linkmode); } - diff --git a/lldp_util.c b/lldp_util.c index 13cbd62..bb61dad 100644 --- a/lldp_util.c +++ b/lldp_util.c @@ -48,6 +48,15 @@ #include "drv_cfg.h" +int is_valid_lldp_device(const char *device_name) +{ + if (is_vlan(device_name)) + return 0; + if (is_bridge(device_name)) + return 0; + return 1; +} + /** * is_bond - check if interface is a bond interface * @ifname: name of the interface