From 4e520cdd3009cf2e04c50c173737b379ff7d72a2 Mon Sep 17 00:00:00 2001 From: sg Date: Thu, 5 Mar 2015 20:57:43 +0100 Subject: [PATCH] fixed bug #37068 (netif up/down handling is unclear): correclty separated administrative status of a netif (up/down) from 'valid address' status ATTENTION: netif_set_up() now always has to be called, even when dhcp/autoip is used! --- CHANGELOG | 7 +++ src/core/dhcp.c | 36 +++++---------- src/core/ipv4/autoip.c | 27 +++++------ src/core/ipv4/ip4.c | 3 +- src/core/netif.c | 102 ++++++++++++++++++++++------------------- 5 files changed, 86 insertions(+), 89 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 2193b439..3163d810 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -6,6 +6,13 @@ HISTORY ++ New features: + 2015-03-05: Simon Goldschmidt + * netif.c, ip4.c, dhcp.c, autoip.c: fixed bug #37068 (netif up/down handling + is unclear): correclty separated administrative status of a netif (up/down) + from 'valid address' status + ATTENTION: netif_set_up() now always has to be called, even when dhcp/autoip + is used! + 2015-02-26: patch by TabascoEye * netif.c, udp.h/.c: fixed bug #40753 (re-bind UDP pcbs on change of IP address) diff --git a/src/core/dhcp.c b/src/core/dhcp.c index 05b55157..7596700c 100644 --- a/src/core/dhcp.c +++ b/src/core/dhcp.c @@ -221,12 +221,8 @@ dhcp_handle_nak(struct netif *netif) struct dhcp *dhcp = netif->dhcp; LWIP_DEBUGF(DHCP_DEBUG | LWIP_DBG_TRACE, ("dhcp_handle_nak(netif=%p) %c%c%"U16_F"\n", (void*)netif, netif->name[0], netif->name[1], (u16_t)netif->num)); - /* Set the interface down since the address must no longer be used, as per RFC2131 */ - netif_set_down(netif); - /* remove IP address from interface */ - netif_set_ipaddr(netif, IP_ADDR_ANY); - netif_set_gw(netif, IP_ADDR_ANY); - netif_set_netmask(netif, IP_ADDR_ANY); + /* remove IP address from interface (must no longer be used, as per RFC2131) */ + netif_set_addr(netif, IP_ADDR_ANY, IP_ADDR_ANY, IP_ADDR_ANY); /* Change to a defined state */ dhcp_set_state(dhcp, DHCP_BACKING_OFF); /* We can immediately restart discovery */ @@ -663,6 +659,7 @@ dhcp_start(struct netif *netif) err_t result; LWIP_ERROR("netif != NULL", (netif != NULL), return ERR_ARG;); + LWIP_ERROR("netif is not up, old style port?", netif_is_up(netif), return ERR_ARG;); dhcp = netif->dhcp; LWIP_DEBUGF(DHCP_DEBUG | LWIP_DBG_TRACE | LWIP_DBG_STATE, ("dhcp_start(netif=%p) %c%c%"U16_F"\n", (void*)netif, netif->name[0], netif->name[1], (u16_t)netif->num)); @@ -808,7 +805,6 @@ dhcp_network_changed(struct netif *netif) case DHCP_RENEWING: case DHCP_BOUND: case DHCP_REBOOTING: - netif_set_down(netif); dhcp->tries = 0; dhcp_reboot(netif); break; @@ -1034,17 +1030,11 @@ dhcp_bind(struct netif *netif) } #endif /* LWIP_DHCP_AUTOIP_COOP */ - LWIP_DEBUGF(DHCP_DEBUG | LWIP_DBG_STATE, ("dhcp_bind(): IP: 0x%08"X32_F"\n", - ip4_addr_get_u32(&dhcp->offered_ip_addr))); - netif_set_ipaddr(netif, &dhcp->offered_ip_addr); - LWIP_DEBUGF(DHCP_DEBUG | LWIP_DBG_STATE, ("dhcp_bind(): SN: 0x%08"X32_F"\n", - ip4_addr_get_u32(&sn_mask))); - netif_set_netmask(netif, &sn_mask); - LWIP_DEBUGF(DHCP_DEBUG | LWIP_DBG_STATE, ("dhcp_bind(): GW: 0x%08"X32_F"\n", - ip4_addr_get_u32(&gw_addr))); - netif_set_gw(netif, &gw_addr); - /* bring the interface up */ - netif_set_up(netif); + LWIP_DEBUGF(DHCP_DEBUG | LWIP_DBG_STATE, ("dhcp_bind(): IP: 0x%08"X32_F" SN: 0x%08"X32_F" GW: 0x%08"X32_F"\n", + ip4_addr_get_u32(&dhcp->offered_ip_addr), ip4_addr_get_u32(&sn_mask), ip4_addr_get_u32(&gw_addr))); + netif_set_addr(netif, &dhcp->offered_ip_addr, &sn_mask, &gw_addr); + /* interface is used by routing now that an address is set */ + /* netif is now bound to DHCP leased address */ dhcp_set_state(dhcp, DHCP_BOUND); } @@ -1240,13 +1230,9 @@ dhcp_release(struct netif *netif) msecs = dhcp->tries < 10 ? dhcp->tries * 1000 : 10 * 1000; dhcp->request_timeout = (msecs + DHCP_FINE_TIMER_MSECS - 1) / DHCP_FINE_TIMER_MSECS; LWIP_DEBUGF(DHCP_DEBUG | LWIP_DBG_TRACE | LWIP_DBG_STATE, ("dhcp_release(): set request timeout %"U16_F" msecs\n", msecs)); - /* bring the interface down */ - netif_set_down(netif); - /* remove IP address from interface */ - netif_set_ipaddr(netif, IP_ADDR_ANY); - netif_set_gw(netif, IP_ADDR_ANY); - netif_set_netmask(netif, IP_ADDR_ANY); - + /* remove IP address from interface (prevents routing from selecting this interface) */ + netif_set_addr(netif, IP_ADDR_ANY, IP_ADDR_ANY, IP_ADDR_ANY); + return result; } diff --git a/src/core/ipv4/autoip.c b/src/core/ipv4/autoip.c index 2e9df9ce..b98cc1bf 100644 --- a/src/core/ipv4/autoip.c +++ b/src/core/ipv4/autoip.c @@ -266,12 +266,8 @@ autoip_bind(struct netif *netif) IP4_ADDR(&sn_mask, 255, 255, 0, 0); IP4_ADDR(&gw_addr, 0, 0, 0, 0); - netif_set_ipaddr(netif, &autoip->llipaddr); - netif_set_netmask(netif, &sn_mask); - netif_set_gw(netif, &gw_addr); - - /* bring the interface up */ - netif_set_up(netif); + netif_set_addr(netif, &autoip->llipaddr, &sn_mask, &gw_addr); + /* interface is used by routing now that an address is set */ return ERR_OK; } @@ -287,16 +283,12 @@ autoip_start(struct netif *netif) struct autoip *autoip = netif->autoip; err_t result = ERR_OK; - if (netif_is_up(netif)) { - netif_set_down(netif); - } + LWIP_ERROR("netif is not up, old style port?", netif_is_up(netif), return ERR_ARG;); /* Set IP-Address, Netmask and Gateway to 0 to make sure that * ARP Packets are formed correctly */ - ip_addr_set_zero(&netif->ip_addr); - ip_addr_set_zero(&netif->netmask); - ip_addr_set_zero(&netif->gw); + netif_set_addr(netif, IP_ADDR_ANY, IP_ADDR_ANY, IP_ADDR_ANY); LWIP_DEBUGF(AUTOIP_DEBUG | LWIP_DBG_TRACE | LWIP_DBG_STATE, ("autoip_start(netif=%p) %c%c%"U16_F"\n", (void*)netif, netif->name[0], @@ -367,7 +359,6 @@ void autoip_network_changed(struct netif *netif) { if (netif->autoip && netif->autoip->state != AUTOIP_STATE_OFF) { - netif_set_down(netif); autoip_start_probing(netif); } } @@ -380,8 +371,12 @@ autoip_network_changed(struct netif *netif) err_t autoip_stop(struct netif *netif) { - netif->autoip->state = AUTOIP_STATE_OFF; - netif_set_down(netif); + if (netif->autoip) { + netif->autoip->state = AUTOIP_STATE_OFF; + if (ip_addr_islinklocal(&netif->ip_addr)) { + netif_set_addr(netif, IP_ADDR_ANY, IP_ADDR_ANY, IP_ADDR_ANY); + } + } return ERR_OK; } @@ -438,7 +433,7 @@ autoip_tmr() /* We are here the first time, so we waited ANNOUNCE_WAIT seconds * Now we can bind to an IP address and use it. * - * autoip_bind calls netif_set_up. This triggers a gratuitous ARP + * autoip_bind calls netif_set_addr. This triggers a gratuitous ARP * which counts as an announcement. */ autoip_bind(netif); diff --git a/src/core/ipv4/ip4.c b/src/core/ipv4/ip4.c index 6ef68354..625fd08b 100644 --- a/src/core/ipv4/ip4.c +++ b/src/core/ipv4/ip4.c @@ -153,7 +153,8 @@ ip_route(const ip_addr_t *dest) } } } - if ((netif_default == NULL) || (!netif_is_up(netif_default))) { + if ((netif_default == NULL) || !netif_is_up(netif_default) || + ip_addr_isany(&netif_default->ip_addr)) { LWIP_DEBUGF(IP_DEBUG | LWIP_DBG_LEVEL_SERIOUS, ("ip_route: No route to %"U16_F".%"U16_F".%"U16_F".%"U16_F"\n", ip4_addr1_16(dest), ip4_addr2_16(dest), ip4_addr3_16(dest), ip4_addr4_16(dest))); IP_STATS_INC(ip.rterr); diff --git a/src/core/netif.c b/src/core/netif.c index 02227f86..3dc8dc9b 100644 --- a/src/core/netif.c +++ b/src/core/netif.c @@ -85,6 +85,10 @@ struct netif *netif_default; static u8_t netif_num; +#define NETIF_REPORT_TYPE_IPV4 0x01 +#define NETIF_REPORT_TYPE_IPV6 0x02 +static void netif_issue_reports(struct netif* netif, u8_t report_type); + #if LWIP_IPV6 static err_t netif_null_output_ip6(struct netif *netif, struct pbuf *p, const ip6_addr_t *ipaddr); #endif /* LWIP_IPV6 */ @@ -281,9 +285,10 @@ void netif_set_addr(struct netif *netif, const ip_addr_t *ipaddr, const ip_addr_t *netmask, const ip_addr_t *gw) { - netif_set_ipaddr(netif, ipaddr); netif_set_netmask(netif, netmask); netif_set_gw(netif, gw); + /* set ipaddr last to ensure netmask/gw have been set when status callback is called */ + netif_set_ipaddr(netif, ipaddr); } /** @@ -394,8 +399,9 @@ netif_find(char *name) void netif_set_ipaddr(struct netif *netif, const ip_addr_t *ipaddr) { + ip_addr_t new_addr = (ipaddr ? *ipaddr : *IP_ADDR_ANY); /* address is actually being changed? */ - if (ipaddr && (ip_addr_cmp(ipaddr, &(netif->ip_addr))) == 0) { + if (ip_addr_cmp(&new_addr, &(netif->ip_addr)) == 0) { LWIP_DEBUGF(NETIF_DEBUG | LWIP_DBG_STATE, ("netif_set_ipaddr: netif address being changed\n")); #if LWIP_TCP tcp_netif_ipv4_addr_changed(&netif->ip_addr, ipaddr); @@ -403,14 +409,18 @@ netif_set_ipaddr(struct netif *netif, const ip_addr_t *ipaddr) #if LWIP_UDP udp_netif_ipv4_addr_changed(&netif->ip_addr, ipaddr); #endif /* LWIP_UDP */ - } - snmp_delete_ipaddridx_tree(netif); - snmp_delete_iprteidx_tree(0, netif); - /* set new IP address to netif */ - ip_addr_set(&(netif->ip_addr), ipaddr); - snmp_insert_ipaddridx_tree(netif); - snmp_insert_iprteidx_tree(0, netif); + snmp_delete_ipaddridx_tree(netif); + snmp_delete_iprteidx_tree(0, netif); + /* set new IP address to netif */ + ip_addr_set(&(netif->ip_addr), ipaddr); + snmp_insert_ipaddridx_tree(netif); + snmp_insert_iprteidx_tree(0, netif); + + netif_issue_reports(netif, NETIF_REPORT_TYPE_IPV4); + + NETIF_STATUS_CALLBACK(netif); + } LWIP_DEBUGF(NETIF_DEBUG | LWIP_DBG_TRACE | LWIP_DBG_STATE, ("netif: IP address of interface %c%c set to %"U16_F".%"U16_F".%"U16_F".%"U16_F"\n", netif->name[0], netif->name[1], @@ -498,7 +508,7 @@ void netif_set_up(struct netif *netif) { if (!(netif->flags & NETIF_FLAG_UP)) { netif->flags |= NETIF_FLAG_UP; - + #if LWIP_SNMP snmp_get_sysuptime(&netif->ts); #endif /* LWIP_SNMP */ @@ -506,31 +516,45 @@ void netif_set_up(struct netif *netif) NETIF_STATUS_CALLBACK(netif); if (netif->flags & NETIF_FLAG_LINK_UP) { + netif_issue_reports(netif, NETIF_REPORT_TYPE_IPV4|NETIF_REPORT_TYPE_IPV6); + } + } +} + +/** Send ARP/IGMP/MLD/RS events, e.g. on link-up/netif-up or addr-change + */ +static void +netif_issue_reports(struct netif* netif, u8_t report_type) +{ + if ((report_type & NETIF_REPORT_TYPE_IPV4) && + !ip_addr_isany(&netif->ip_addr)) { #if LWIP_ARP - /* For Ethernet network interfaces, we would like to send a "gratuitous ARP" */ - if (netif->flags & (NETIF_FLAG_ETHARP)) { - etharp_gratuitous(netif); - } + /* For Ethernet network interfaces, we would like to send a "gratuitous ARP" */ + if (netif->flags & (NETIF_FLAG_ETHARP)) { + etharp_gratuitous(netif); + } #endif /* LWIP_ARP */ #if LWIP_IGMP - /* resend IGMP memberships */ - if (netif->flags & NETIF_FLAG_IGMP) { - igmp_report_groups( netif); - } -#endif /* LWIP_IGMP */ -#if LWIP_IPV6 && LWIP_IPV6_MLD - /* send mld memberships */ - mld6_report_groups( netif); -#endif /* LWIP_IPV6 && LWIP_IPV6_MLD */ - -#if LWIP_IPV6_SEND_ROUTER_SOLICIT - /* Send Router Solicitation messages. */ - netif->rs_count = LWIP_ND6_MAX_MULTICAST_SOLICIT; -#endif /* LWIP_IPV6_SEND_ROUTER_SOLICIT */ - + /* resend IGMP memberships */ + if (netif->flags & NETIF_FLAG_IGMP) { + igmp_report_groups(netif); } +#endif /* LWIP_IGMP */ } + +#if LWIP_IPV6 + if (report_type & NETIF_REPORT_TYPE_IPV6) { +#if LWIP_IPV6_MLD + /* send mld memberships */ + mld6_report_groups(netif); +#endif /* LWIP_IPV6_MLD */ +#if LWIP_IPV6_SEND_ROUTER_SOLICIT + /* Send Router Solicitation messages. */ + netif->rs_count = LWIP_ND6_MAX_MULTICAST_SOLICIT; +#endif /* LWIP_IPV6_SEND_ROUTER_SOLICIT */ + } +#endif /* LWIP_IPV6 */ } /** @@ -560,7 +584,7 @@ void netif_set_down(struct netif *netif) #if LWIP_NETIF_STATUS_CALLBACK /** - * Set callback to be called when interface is brought up/down + * Set callback to be called when interface is brought up/down or address is changed while up */ void netif_set_status_callback(struct netif *netif, netif_status_callback_fn status_callback) { @@ -604,23 +628,7 @@ void netif_set_link_up(struct netif *netif ) #endif /* LWIP_AUTOIP */ if (netif->flags & NETIF_FLAG_UP) { -#if LWIP_ARP - /* For Ethernet network interfaces, we would like to send a "gratuitous ARP" */ - if (netif->flags & NETIF_FLAG_ETHARP) { - etharp_gratuitous(netif); - } -#endif /* LWIP_ARP */ - -#if LWIP_IGMP - /* resend IGMP memberships */ - if (netif->flags & NETIF_FLAG_IGMP) { - igmp_report_groups( netif); - } -#endif /* LWIP_IGMP */ -#if LWIP_IPV6 && LWIP_IPV6_MLD - /* send mld memberships */ - mld6_report_groups( netif); -#endif /* LWIP_IPV6 && LWIP_IPV6_MLD */ + netif_issue_reports(netif, NETIF_REPORT_TYPE_IPV4|NETIF_REPORT_TYPE_IPV6); } NETIF_LINK_CALLBACK(netif); }