sergiotarxz/nuttx
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Networking: The network device list was protected by a re-entrant semaphore. With the recent change to support network device callback, the network stack needs to access the network device list too. Some drivers, however, run the network stack from the interrupt level -- this is bad but a fact in the current state. Of course,those drivers are unable to take the semaphore and will assert.

Browse Source 
The solution here is to eliminate the device devices semaphore altogether.  This eliminates netdev_semtake() and netdev_semgive() and replaces them with net_lock() and net_unlock() which have larger scope as needed for this purpose.
This commit is contained in:
Gregory Nutt 2015-05-31 08:34:03 -06:00
parent 6687e156e6
commit 33085cb309
16 changed files with 74 additions and 265 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
include/nuttx/net/net.h
View File

@ -49,6 +49,10 @@
#include <stdarg.h>
#include <semaphore.h>
#ifndef CONFIG_NET_NOINTS
# include <arch/irq.h>
#endif
/****************************************************************************
* Pre-processor Definitions
****************************************************************************/

6
net/net_initialize.c
View File

@ -176,12 +176,6 @@ void net_setup(void)
net_initroute();
#endif
#if CONFIG_NSOCKET_DESCRIPTORS > 0
/* Initialize the socket layer */
netdev_seminit();
#endif
}
/****************************************************************************

4
net/netdev/Make.defs
View File

@ -37,8 +37,8 @@
NETDEV_CSRCS += netdev_register.c netdev_ioctl.c netdev_txnotify.c
NETDEV_CSRCS += netdev_findbyname.c netdev_findbyaddr.c netdev_count.c
NETDEV_CSRCS += netdev_foreach.c netdev_unregister.c netdev_sem.c
NETDEV_CSRCS += netdev_carrier.c netdev_default.c netdev_verify.c
NETDEV_CSRCS += netdev_foreach.c netdev_unregister.c netdev_carrier.c
NETDEV_CSRCS += netdev_default.c netdev_verify.c
ifeq ($(CONFIG_NET_RXAVAIL),y)
NETDEV_CSRCS += netdev_rxnotify.c

44
net/netdev/netdev.h
View File

@ -1,7 +1,7 @@
/****************************************************************************
* net/netdev/netdev.h
*
* Copyright (C) 2014 Gregory Nutt. All rights reserved.
* Copyright (C) 2014-2015 Gregory Nutt. All rights reserved.
* Author: Gregory Nutt <gnutt@nuttx.org>
*
* Redistribution and use in source and binary forms, with or without
@ -69,8 +69,10 @@ extern "C"
#endif
#if CONFIG_NSOCKET_DESCRIPTORS > 0
/* List of registered Ethernet device drivers. This duplicates a declaration
* in net/netdev/netdev.h
/* List of registered Ethernet device drivers. You must have the network
* locked in order to access this list.
*
* NOTE that this duplicates a declaration in net/tcp/tcp.h
*/
EXTERN struct net_driver_s *g_netdevices;
@ -80,42 +82,6 @@ EXTERN struct net_driver_s *g_netdevices;
* Public Function Prototypes
****************************************************************************/
/****************************************************************************
* Function: netdev_seminit
*
* Description:
* Initialize the network device semaphore.
*
****************************************************************************/
#if CONFIG_NSOCKET_DESCRIPTORS > 0
void netdev_seminit(void);
#endif
/****************************************************************************
* Function: netdev_semtake
*
* Description:
* Get exclusive access to the network device list.
*
****************************************************************************/
#if CONFIG_NSOCKET_DESCRIPTORS > 0
void netdev_semtake(void);
#endif
/****************************************************************************
* Function: netdev_semgive
*
* Description:
* Release exclusive access to the network device list
*
****************************************************************************/
#if CONFIG_NSOCKET_DESCRIPTORS > 0
void netdev_semgive(void);
#endif
/****************************************************************************
* Name: netdev_ifup / netdev_ifdown
*

8
net/netdev/netdev_count.c
View File

@ -1,7 +1,7 @@
/****************************************************************************
* net/netdev/netdev_count.c
*
* Copyright (C) 2007-2009 Gregory Nutt. All rights reserved.
* Copyright (C) 2007-2009, 2015 Gregory Nutt. All rights reserved.
* Author: Gregory Nutt <gnutt@nuttx.org>
*
* Redistribution and use in source and binary forms, with or without
@ -45,6 +45,7 @@
#include <nuttx/net/netdev.h>
#include "utils/utils.h"
#include "netdev/netdev.h"
/****************************************************************************
@ -91,11 +92,12 @@
int netdev_count(void)
{
struct net_driver_s *dev;
net_lock_t save;
int ndev;
netdev_semtake();
save = net_lock();
for (dev = g_netdevices, ndev = 0; dev; dev = dev->flink, ndev++);
netdev_semgive();
net_unlock(save);
return ndev;
}

11
net/netdev/netdev_default.c
View File

@ -1,7 +1,7 @@
/****************************************************************************
* net/netdev/netdev_default.c
*
* Copyright (C) 2014 Gregory Nutt. All rights reserved.
* Copyright (C) 2014-2015 Gregory Nutt. All rights reserved.
* Author: Gregory Nutt <gnutt@nuttx.org>
*
* Redistribution and use in source and binary forms, with or without
@ -41,6 +41,8 @@
#if defined(CONFIG_NET) && CONFIG_NSOCKET_DESCRIPTORS > 0
#include <nuttx/net/netdev.h>
#include "utils/utils.h"
#include "netdev/netdev.h"
/****************************************************************************
@ -98,10 +100,11 @@
FAR struct net_driver_s *netdev_default(void)
{
FAR struct net_driver_s *dev;
net_lock_t save;
/* Examine each registered network device */
netdev_semtake();
save = net_lock();
for (dev = g_netdevices; dev; dev = dev->flink)
{
/* Is the interface in the "up" state? */
@ -112,12 +115,12 @@ FAR struct net_driver_s *netdev_default(void)
* state.
*/
netdev_semgive();
net_unlock(save);
return dev;
}
}
netdev_semgive();
net_unlock(save);
return NULL;
}

17
net/netdev/netdev_findbyaddr.c
View File

@ -50,9 +50,10 @@
#include <nuttx/net/netdev.h>
#include <nuttx/net/ip.h>
#include "netdev/netdev.h"
#include "utils/utils.h"
#include "devif/devif.h"
#include "route/route.h"
#include "netdev/netdev.h"
/****************************************************************************
* Pre-processor Definitions
@ -97,10 +98,11 @@
static FAR struct net_driver_s *netdev_finddevice_ipv4addr(in_addr_t ripaddr)
{
FAR struct net_driver_s *dev;
net_lock_t save;
/* Examine each registered network device */
netdev_semtake();
save = net_lock();
for (dev = g_netdevices; dev; dev = dev->flink)
{
/* Is the interface in the "up" state? */
@ -114,7 +116,7 @@ static FAR struct net_driver_s *netdev_finddevice_ipv4addr(in_addr_t ripaddr)
{
/* Its a match */
netdev_semgive();
net_unlock(save);
return dev;
}
}
@ -122,7 +124,7 @@ static FAR struct net_driver_s *netdev_finddevice_ipv4addr(in_addr_t ripaddr)
/* No device with the matching address found */
netdev_semgive();
net_unlock(save);
return NULL;
}
#endif /* CONFIG_NET_IPv4 */
@ -151,10 +153,11 @@ static FAR struct net_driver_s *
netdev_finddevice_ipv6addr(const net_ipv6addr_t ripaddr)
{
FAR struct net_driver_s *dev;
net_lock_t save;
/* Examine each registered network device */
netdev_semtake();
save = net_lock();
for (dev = g_netdevices; dev; dev = dev->flink)
{
/* Is the interface in the "up" state? */
@ -168,7 +171,7 @@ netdev_finddevice_ipv6addr(const net_ipv6addr_t ripaddr)
{
/* Its a match */
netdev_semgive();
net_unlock(save);
return dev;
}
}
@ -176,7 +179,7 @@ netdev_finddevice_ipv6addr(const net_ipv6addr_t ripaddr)
/* No device with the matching address found */
netdev_semgive();
net_unlock(save);
return NULL;
}
#endif /* CONFIG_NET_IPv6 */

11
net/netdev/netdev_findbyname.c
View File

@ -1,7 +1,7 @@
/****************************************************************************
* net/netdev/netdev_findbyname.c
*
* Copyright (C) 2007, 2008 Gregory Nutt. All rights reserved.
* Copyright (C) 2007, 2008, 2015 Gregory Nutt. All rights reserved.
* Author: Gregory Nutt <gnutt@nuttx.org>
*
* Redistribution and use in source and binary forms, with or without
@ -45,6 +45,7 @@
#include <nuttx/net/netdev.h>
#include "utils/utils.h"
#include "netdev/netdev.h"
/****************************************************************************
@ -92,19 +93,21 @@
FAR struct net_driver_s *netdev_findbyname(FAR const char *ifname)
{
FAR struct net_driver_s *dev;
net_lock_t save;
if (ifname)
{
netdev_semtake();
save = net_lock();
for (dev = g_netdevices; dev; dev = dev->flink)
{
if (strcmp(ifname, dev->d_ifname) == 0)
{
netdev_semgive();
net_unlock(save);
return dev;
}
}
netdev_semgive();
net_unlock(save);
}
return NULL;

13
net/netdev/netdev_foreach.c
View File

@ -1,7 +1,7 @@
/****************************************************************************
* net/netdev/netdev_foreach.c
*
* Copyright (C) 2007-2009, 2012 Gregory Nutt. All rights reserved.
* Copyright (C) 2007-2009, 2012, 2015 Gregory Nutt. All rights reserved.
* Author: Gregory Nutt <gnutt@nuttx.org>
*
* Redistribution and use in source and binary forms, with or without
@ -90,14 +90,15 @@
*
****************************************************************************/
int netdev_foreach(netdev_callback_t callback, void *arg)
int netdev_foreach(netdev_callback_t callback, FAR void *arg)
{
struct net_driver_s *dev;
FAR struct net_driver_s *dev;
net_lock_t save;
int ret = 0;
if (callback)
{
netdev_semtake();
save = net_lock();
for (dev = g_netdevices; dev; dev = dev->flink)
{
if (callback(dev, arg) != 0)
@ -106,8 +107,10 @@ int netdev_foreach(netdev_callback_t callback, void *arg)
break;
}
}
netdev_semgive();
net_unlock(save);
}
return ret;
}

8
net/netdev/netdev_register.c
View File

@ -53,8 +53,9 @@
#include <nuttx/net/netdev.h>
#include <nuttx/net/arp.h>
#include "netdev/netdev.h"
#include "utils/utils.h"
#include "igmp/igmp.h"
#include "netdev/netdev.h"
/****************************************************************************
* Pre-processor Definitions
@ -175,6 +176,7 @@ int netdev_register(FAR struct net_driver_s *dev, enum net_lltype_e lltype)
#ifdef CONFIG_NET_USER_DEVFMT
FAR const char devfmt_str[IFNAMSIZ];
#endif
net_lock_t save;
int devnum;
if (dev)
@ -256,7 +258,7 @@ int netdev_register(FAR struct net_driver_s *dev, enum net_lltype_e lltype)
* the interface
*/
netdev_semtake();
save = net_lock();
#ifdef CONFIG_NET_MULTILINK
devnum = find_devnum(devfmt);
#else
@ -282,7 +284,7 @@ int netdev_register(FAR struct net_driver_s *dev, enum net_lltype_e lltype)
#ifdef CONFIG_NET_IGMP
igmp_devinit(dev);
#endif
netdev_semgive();
net_unlock(save);
#ifdef CONFIG_NET_ETHERNET
nlldbg("Registered MAC: %02x:%02x:%02x:%02x:%02x:%02x as dev: %s\n",

180
net/netdev/netdev_sem.c
View File

@ -1,180 +0,0 @@
/****************************************************************************
* net/netdev/netdev_sem.c
*
* Copyright (C) 2012 Gregory Nutt. All rights reserved.
* Author: Gregory Nutt <gnutt@nuttx.org>
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
* are met:
*
* 1. Redistributions of source code must retain the above copyright
* notice, this list of conditions and the following disclaimer.
* 2. Redistributions in binary form must reproduce the above copyright
* notice, this list of conditions and the following disclaimer in
* the documentation and/or other materials provided with the
* distribution.
* 3. Neither the name NuttX nor the names of its contributors may be
* used to endorse or promote products derived from this software
* without specific prior written permission.
*
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
* "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
* LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
* FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
* COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
* INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
* BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS
* OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
* AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
* LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN
* ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
* POSSIBILITY OF SUCH DAMAGE.
*
****************************************************************************/
/****************************************************************************
* Included Files
****************************************************************************/
#include <nuttx/config.h>
#if defined(CONFIG_NET) && CONFIG_NSOCKET_DESCRIPTORS > 0
#include <sys/types.h>
#include <unistd.h>
#include <semaphore.h>
#include <assert.h>
#include <errno.h>
#include <nuttx/net/net.h>
#include "netdev/netdev.h"
/****************************************************************************
* Pre-processor Definitions
****************************************************************************/
#define NO_HOLDER (pid_t)-1
/****************************************************************************
* Priviate Types
****************************************************************************/
/* There is at least on context in which recursive semaphores are required:
* When netdev_foreach is used with a telnet client, we will deadlock if we
* do not provide this capability.
*/
struct netdev_sem_s
{
sem_t sem;
pid_t holder;
unsigned int count;
};
/****************************************************************************
* Private Data
****************************************************************************/
/****************************************************************************
* Public Data
****************************************************************************/
static struct netdev_sem_s g_devlock;
/****************************************************************************
* Private Functions
****************************************************************************/
/****************************************************************************
* Public Functions
****************************************************************************/
/****************************************************************************
* Function: netdev_seminit
*
* Description:
* Initialize the network device semaphore.
*
****************************************************************************/
void netdev_seminit(void)
{
sem_init(&g_devlock.sem, 0, 1);
g_devlock.holder = NO_HOLDER;
g_devlock.count = 0;
}
/****************************************************************************
* Function: netdev_semtake
*
* Description:
* Get exclusive access to the network device list.
*
****************************************************************************/
void netdev_semtake(void)
{
pid_t me = getpid();
/* Does this thread already hold the semaphore? */
if (g_devlock.holder == me)
{
/* Yes.. just increment the reference count */
g_devlock.count++;
}
else
{
/* No.. take the semaphore (perhaps waiting) */
while (net_lockedwait(&g_devlock.sem) != 0)
{
/* The only case that an error should occur here is if
* the wait was awakened by a signal.
*/
ASSERT(errno == EINTR);
}
/* Now this thread holds the semaphore */
g_devlock.holder = me;
g_devlock.count = 1;
}
}
/****************************************************************************
* Function: netdev_semtake
*
* Description:
* Release exclusive access to the network device list
*
****************************************************************************/
void netdev_semgive(void)
{
DEBUGASSERT(g_devlock.holder == getpid() && g_devlock.count > 0);
/* If the count would go to zero, then release the semaphore */
if (g_devlock.count == 1)
{
/* We no longer hold the semaphore */
g_devlock.holder = NO_HOLDER;
g_devlock.count = 0;
sem_post(&g_devlock.sem);
}
else
{
/* We still hold the semaphore. Just decrement the count */
g_devlock.count--;
}
}
#endif /* CONFIG_NET && CONFIG_NSOCKET_DESCRIPTORS */

6
net/netdev/netdev_unregister.c
View File

@ -52,6 +52,7 @@
#include <net/ethernet.h>
#include <nuttx/net/netdev.h>
#include "utils/utils.h"
#include "netdev/netdev.h"
/****************************************************************************
@ -106,10 +107,11 @@ int netdev_unregister(FAR struct net_driver_s *dev)
{
struct net_driver_s *prev;
struct net_driver_s *curr;
net_lock_t save;
if (dev)
{
netdev_semtake();
save = net_lock();
/* Find the device in the list of known network devices */
@ -139,7 +141,7 @@ int netdev_unregister(FAR struct net_driver_s *dev)
curr->flink = NULL;
}
netdev_semgive();
net_unlock(save);
#ifdef CONFIG_NET_ETHERNET
nlldbg("Unregistered MAC: %02x:%02x:%02x:%02x:%02x:%02x as dev: %s\n",

6
net/netdev/netdev_verify.c
View File

@ -43,6 +43,7 @@
#include <nuttx/net/netdev.h>
#include "utils/utils.h"
#include "netdev/netdev.h"
/****************************************************************************
@ -63,11 +64,12 @@
bool netdev_verify(FAR struct net_driver_s *dev)
{
FAR struct net_driver_s *chkdev;
net_lock_t save;
bool valid = false;
/* Search the list of registered devices */
netdev_semtake();
save = net_lock();
for (chkdev = g_netdevices; chkdev != NULL; chkdev = chkdev->flink)
{
/* Is the the network device that we are looking for? */
@ -81,6 +83,6 @@ bool netdev_verify(FAR struct net_driver_s *dev)
}
}
netdev_semgive();
net_unlock(save);
return valid;
}

14
net/socket/getsockname.c
View File

@ -49,6 +49,7 @@
#include <nuttx/net/netdev.h>
#include <nuttx/net/udp.h>
#include "utils/utils.h"
#include "netdev/netdev.h"
#include "tcp/tcp.h"
#include "udp/udp.h"
@ -98,6 +99,7 @@ int ipv4_getsockname(FAR struct socket *psock, FAR struct sockaddr *addr,
in_addr_t lipaddr;
in_addr_t ripaddr;
#endif
net_lock_t save;
/* Check if enough space has been provided for the full address */
@ -150,7 +152,7 @@ int ipv4_getsockname(FAR struct socket *psock, FAR struct sockaddr *addr,
* a single network device and only the network device knows the IP address.
*/
netdev_semtake();
save = net_lock();
#ifdef CONFIG_NETDEV_MULTINIC
/* Find the device matching the IPv4 address in the connection structure */
@ -164,7 +166,7 @@ int ipv4_getsockname(FAR struct socket *psock, FAR struct sockaddr *addr,
if (!dev)
{
netdev_semgive();
net_unlock(save);
return -EINVAL;
}
@ -175,7 +177,7 @@ int ipv4_getsockname(FAR struct socket *psock, FAR struct sockaddr *addr,
outaddr->sin_addr.s_addr = dev->d_ipaddr;
*addrlen = sizeof(struct sockaddr_in);
#endif
netdev_semgive();
net_unlock(save);
/* Return success */
@ -273,7 +275,7 @@ int ipv6_getsockname(FAR struct socket *psock, FAR struct sockaddr *addr,
* a single network device and only the network device knows the IP address.
*/
netdev_semtake();
save = net_lock();
#ifdef CONFIG_NETDEV_MULTINIC
/* Find the device matching the IPv6 address in the connection structure */
@ -287,7 +289,7 @@ int ipv6_getsockname(FAR struct socket *psock, FAR struct sockaddr *addr,
if (!dev)
{
netdev_semgive();
net_unlock(save);
return -EINVAL;
}
@ -298,7 +300,7 @@ int ipv6_getsockname(FAR struct socket *psock, FAR struct sockaddr *addr,
memcpy(outaddr->sin6_addr.in6_u.u6_addr8, dev->d_ipv6addr, 16);
*addrlen = sizeof(struct sockaddr_in6);
#endif
netdev_semgive();
net_unlock(save);
/* Return success */

6
net/tcp/tcp.h
View File

@ -332,8 +332,10 @@ extern "C"
#endif
#if CONFIG_NSOCKET_DESCRIPTORS > 0
/* List of registered Ethernet device drivers. This duplicates a declaration
* in net/tcp/tcp.h
/* List of registered Ethernet device drivers. You must have the network
* locked in order to access this list.
*
* NOTE that this duplicates a declaration in net/netdev/netdev.h
*/
EXTERN struct net_driver_s *g_netdevices;

1
net/utils/utils.h
View File

@ -41,6 +41,7 @@
****************************************************************************/
#include <nuttx/config.h>
#include <nuttx/net/net.h>
#include <nuttx/net/ip.h>
/****************************************************************************