net/icmpv6/icmpv6_autoconfig.c and icmpv6_rnotify.c: Don't take the network device down when reconfiguring only the IP address from within ICMPv6 logic. Recommended by Xiang Xiao in order to avoid the long delays of bringing some networks back up.

Normally it is required that the network be in the "down" state when re-configuring the network interface.  This is thought not to be a necessary here because.

  1. The ICMPv6 logic here runs with the network locked so there can be no outgoing packets with bad source IP addresses from any asynchronous network activity using the device being reconfigured.
  2. Incoming packets depend only upon the MAC filtering.  Network drivers do not use the IP address; they filter incoming packets using only the MAC address which is not being changed here.
This commit is contained in:
Gregory Nutt 2018-11-10 07:13:54 -06:00
parent ae29df7445
commit 68a115aed9
5 changed files with 43 additions and 52 deletions

View File

@ -321,7 +321,7 @@ int arp_send(in_addr_t ipaddr)
* sending the ARP request if it is not.
*/
/* The optimal delay would be the work case round trip time. */
/* The optimal delay would be the worst case round trip time. */
delay.tv_sec = CONFIG_ARP_SEND_DELAYSEC;
delay.tv_nsec = CONFIG_ARP_SEND_DELAYNSEC;

View File

@ -492,9 +492,6 @@ int icmpv6_rwait(FAR struct icmpv6_rnotify_s *notify,
* wake-up any threads that may be waiting for this particular Router
* Advertisement.
*
* NOTE: On success the network has the new address applied and is in
* the down state.
*
* Assumptions:
* This function is called from the MAC device driver indirectly through
* icmpv6_icmpv6in() will execute with the network locked.

View File

@ -300,11 +300,21 @@ int icmpv6_autoconfig(FAR struct net_driver_s *dev)
DEBUGASSERT(dev);
ninfo("Auto-configuring %s\n", dev->d_ifname);
/* The interface should be in the down state */
/* Lock the network.
*
* NOTE: Normally it is required that the network be in the "down" state
* when re-configuring the network interface. This is thought not to be
* a problem here because.
*
* 1. The ICMPv6 logic here runs with the network locked so there can be
* no outgoing packets with bad source IP addresses from any
* asynchronous network activity using the device being reconfigured.
* 2. Incoming packets depend only upon the MAC filtering. Network
* drivers do not use the IP address; they filter incoming packets
* using only the MAC address which is not being changed here.
*/
net_lock();
netdev_ifdown(dev);
net_unlock();
/* IPv6 Stateless Autoconfiguration
* Reference: http://www.tcpipguide.com/free/t_IPv6AutoconfigurationandRenumbering.htm
@ -327,13 +337,7 @@ int icmpv6_autoconfig(FAR struct net_driver_s *dev)
lladdr[4], lladdr[6], lladdr[6], lladdr[7]);
#ifdef CONFIG_NET_ICMPv6_NEIGHBOR
/* Bring the interface up with no IP address */
net_lock();
netdev_ifup(dev);
net_unlock();
/* 2. Link-Local Address Uniqueness Test: The node tests to ensure that
/* 2. Link-Local Address Uniqueness Test: The node tests to ensure that
* the address it generated isn't for some reason already in use on the
* local network. (This is very unlikely to be an issue if the link-local
* address came from a MAC address but more likely if it was based on a
@ -346,20 +350,15 @@ int icmpv6_autoconfig(FAR struct net_driver_s *dev)
*/
ret = icmpv6_neighbor(lladdr);
/* Take the interface back down */
net_lock();
netdev_ifdown(dev);
net_unlock();
if (ret == OK)
if (ret >= 0)
{
/* Hmmm... someone else responded to our Neighbor Solicitation. We
* have not back-up plan in place. Just bail.
* have no back-up plan in place. Just bail.
*/
nerr("ERROR: IP conflict\n");
net_unlock();
return -EEXIST;
}
#endif
@ -370,14 +369,9 @@ int icmpv6_autoconfig(FAR struct net_driver_s *dev)
* on the wider Internet (since link-local addresses are not routed).
*/
net_lock();
net_ipv6addr_copy(dev->d_ipv6addr, lladdr);
/* Bring the interface up with the new, temporary IP address */
netdev_ifup(dev);
/* The optimal delay would be the work case round trip time. */
/* The optimal delay would be the worst case round trip time. */
delay.tv_sec = CONFIG_ICMPv6_AUTOCONF_DELAYSEC;
delay.tv_nsec = CONFIG_ICMPv6_AUTOCONF_DELAYNSEC;
@ -424,9 +418,7 @@ int icmpv6_autoconfig(FAR struct net_driver_s *dev)
ninfo("Timed out... retrying %d\n", retries + 1);
}
/* Check for failures. Note: On successful return, the network will be
* in the down state, but not in the event of failures.
*/
/* Check for failures. */
if (ret < 0)
{
@ -441,7 +433,6 @@ int icmpv6_autoconfig(FAR struct net_driver_s *dev)
if (ret < 0)
{
nerr("ERROR: Failed send neighbor advertisement: %d\n", ret);
netdev_ifdown(dev);
}
/* No off-link communications; No router address. */
@ -451,13 +442,6 @@ int icmpv6_autoconfig(FAR struct net_driver_s *dev)
/* Set a netmask for the local link address */
net_ipv6addr_copy(dev->d_ipv6netmask, g_ipv6_llnetmask);
/* Leave the network up and return success (even though things did not
* work out quite the way we wanted).
*/
net_unlock();
return ret;
}
/* 5. Router Direction: The router provides direction to the node on how to
@ -474,13 +458,10 @@ int icmpv6_autoconfig(FAR struct net_driver_s *dev)
* first step.
*/
/* On success, the new address was already set (in icmpv_rnotify()). We
* need only to bring the network back to the up state and return success.
*/
/* On success, the new address was already set (in icmpv_rnotify()). */
netdev_ifup(dev);
net_unlock();
return OK;
return ret;
}
#endif /* CONFIG_NET_ICMPv6_AUTOCONF */

View File

@ -295,7 +295,7 @@ int icmpv6_neighbor(const net_ipv6addr_t ipaddr)
* re-sending the Neighbor Solicitation if it is not.
*/
/* The optimal delay would be the work case round trip time. */
/* The optimal delay would be the worst case round trip time. */
delay.tv_sec = CONFIG_ICMPv6_NEIGHBOR_DELAYSEC;
delay.tv_nsec = CONFIG_ICMPv6_NEIGHBOR_DELAYNSEC;
@ -367,8 +367,10 @@ int icmpv6_neighbor(const net_ipv6addr_t ipaddr)
nxsem_destroy(&state.snd_sem);
icmpv6_callback_free(dev, state.snd_cb);
errout_with_lock:
net_unlock();
errout:
return ret;
}

View File

@ -94,9 +94,21 @@ static void icmpv6_setaddresses(FAR struct net_driver_s *dev,
{
unsigned int i;
/* Make sure that the network is down before changing any addresses */
/* Lock the network.
*
* NOTE: Normally it is required that the network be in the "down" state
* when re-configuring the network interface. This is thought not to be
* a problem here because.
*
* 1. The ICMPv6 logic here runs with the network locked so there can be
* no outgoing packets with bad source IP addresses from any
* asynchronous network activity using the device being reconfigured.
* 2. Incoming packets depend only upon the MAC filtering. Network
* drivers do not use the IP address; they filter incoming packets
* using only the MAC address which is not being changed here.
*/
netdev_ifdown(dev);
net_lock();
/* Create an address mask from the prefix */
@ -136,6 +148,8 @@ static void icmpv6_setaddresses(FAR struct net_driver_s *dev,
dev->d_ipv6draddr[0], dev->d_ipv6draddr[1], dev->d_ipv6draddr[2],
dev->d_ipv6draddr[3], dev->d_ipv6draddr[4], dev->d_ipv6draddr[6],
dev->d_ipv6draddr[6], dev->d_ipv6draddr[7]);
net_unlock();
}
/****************************************************************************
@ -203,7 +217,7 @@ int icmpv6_rwait_cancel(FAR struct icmpv6_rnotify_s *notify)
irqstate_t flags;
int ret = -ENOENT;
ninfo("Cancelling...\n");
ninfo("Canceling...\n");
/* Remove our wait structure from the list (we may no longer be at the
* head of the list).
@ -299,9 +313,6 @@ int icmpv6_rwait(FAR struct icmpv6_rnotify_s *notify,
* wake-up any threads that may be waiting for this particular Router
* Advertisement.
*
* NOTE: On success the network has the new address applied and is in
* the down state.
*
* Assumptions:
* This function is called from the MAC device driver indirectly through
* icmpv6_icmpv6in() will execute with the network locked.