net/igmp: Backport some MLD design improments/fixes.

net/igmp/Kconfig

@@ -8,6 +8,7 @@ config NET_IGMP
 	default n
 	depends on NET_IPv4
 	select NET_MCASTGROUP
+	select NETDEV_IFINDEX
 	---help---
 		Enable IGMPv2 client support.
 

net/igmp/igmp.h

@@ -1,7 +1,7 @@
 /****************************************************************************
  * net/igmp/igmp.h
  *
- *   Copyright (C) 2014 Gregory Nutt. All rights reserved.
+ *   Copyright (C) 2014, 2018 Gregory Nutt. All rights reserved.
  *   Author: Gregory Nutt <gnutt@nuttx.org>
  *
  * Redistribution and use in source and binary forms, with or without
@@ -77,6 +77,7 @@
 
 #include <sys/types.h>
 
+#include <nuttx/wqueue.h>
 #include <nuttx/net/ip.h>
 
 #ifdef CONFIG_NET_IGMP
@@ -123,10 +124,12 @@ typedef FAR struct wdog_s *WDOG_ID;
 struct igmp_group_s
 {
   struct igmp_group_s *next;    /* Implements a singly-linked list */
+  struct work_s        work;    /* For deferred timeout operations */
   in_addr_t            grpaddr; /* Group IPv4 address */
   WDOG_ID              wdog;    /* WDOG used to detect timeouts */
   sem_t                sem;     /* Used to wait for message transmission */
-  volatile uint8_t     flags;   /* See IGMP_ flags definitions */
+  uint8_t              ifindex; /* Interface index */
+  uint8_t              flags;   /* See IGMP_ flags definitions */
   uint8_t              msgid;   /* Pending message ID (if non-zero) */
 };
 
@@ -233,7 +236,7 @@ void igmp_grpfree(FAR struct net_driver_s *dev,
  *
  ****************************************************************************/
 
-void igmp_schedmsg(FAR struct igmp_group_s *group, uint8_t msgid);
+int igmp_schedmsg(FAR struct igmp_group_s *group, uint8_t msgid);
 
 /****************************************************************************
  * Name: igmp_waitmsg
@@ -244,7 +247,7 @@ void igmp_schedmsg(FAR struct igmp_group_s *group, uint8_t msgid);
  *
  ****************************************************************************/
 
-void igmp_waitmsg(FAR struct igmp_group_s *group, uint8_t msgid);
+int igmp_waitmsg(FAR struct igmp_group_s *group, uint8_t msgid);
 
 /****************************************************************************
  * Name:  igmp_poll

net/igmp/igmp_group.c

@@ -2,7 +2,7 @@
  * net/igmp/igmp_group.c
  * IGMP group data structure management logic
  *
- *   Copyright (C) 2010, 2013-2014, 2016 Gregory Nutt. All rights reserved.
+ *   Copyright (C) 2010, 2013-2014, 2016, 2018 Gregory Nutt. All rights reserved.
  *   Author: Gregory Nutt <gnutt@nuttx.org>
  *
  * The NuttX implementation of IGMP was inspired by the IGMP add-on for the
@@ -105,6 +105,9 @@
  * Description:
  *   Allocate a new group from heap memory.
  *
+ * Assumptions:
+ *   The network is locked.
+ *
  ****************************************************************************/
 
 FAR struct igmp_group_s *igmp_grpalloc(FAR struct net_driver_s *dev,
@@ -137,14 +140,13 @@ FAR struct igmp_group_s *igmp_grpalloc(FAR struct net_driver_s *dev,
       group->wdog = wd_create();
       DEBUGASSERT(group->wdog);
 
-      /* Interrupts must be disabled in order to modify the group list */
+      /* Save the interface index */
 
-      net_lock();
+      group->ifindex = dev->d_ifindex;
 
       /* Add the group structure to the list in the device structure */
 
       sq_addfirst((FAR sq_entry_t *)group, &dev->d_igmp_grplist);
-      net_unlock();
     }
 
   return group;
@@ -156,6 +158,9 @@ FAR struct igmp_group_s *igmp_grpalloc(FAR struct net_driver_s *dev,
  * Description:
  *   Find an existing group.
  *
+ * Assumptions:
+ *   The network is locked.
+ *
  ****************************************************************************/
 
 FAR struct igmp_group_s *igmp_grpfind(FAR struct net_driver_s *dev,
@@ -165,7 +170,6 @@ FAR struct igmp_group_s *igmp_grpfind(FAR struct net_driver_s *dev,
 
   grpinfo("Searching for addr %08x\n", (int)*addr);
 
-  net_lock();
   for (group = (FAR struct igmp_group_s *)dev->d_igmp_grplist.head;
        group;
        group = group->next)
@@ -174,11 +178,11 @@ FAR struct igmp_group_s *igmp_grpfind(FAR struct net_driver_s *dev,
       if (net_ipv4addr_cmp(group->grpaddr, *addr))
         {
           grpinfo("Match!\n");
+          DEBUGASSERT(group->ifindex == dev->d_ifindex);
           break;
         }
     }
 
-  net_unlock();
   return group;
 }
 
@@ -189,6 +193,9 @@ FAR struct igmp_group_s *igmp_grpfind(FAR struct net_driver_s *dev,
  *   Find an existing group.  If not found, create a new group for the
  *   address.
  *
+ * Assumptions:
+ *   The network is locked.
+ *
  ****************************************************************************/
 
 FAR struct igmp_group_s *igmp_grpallocfind(FAR struct net_driver_s *dev,
@@ -212,6 +219,9 @@ FAR struct igmp_group_s *igmp_grpallocfind(FAR struct net_driver_s *dev,
  * Description:
  *   Release a previously allocated group.
  *
+ * Assumptions:
+ *   The network is locked.
+ *
  ****************************************************************************/
 
 void igmp_grpfree(FAR struct net_driver_s *dev, FAR struct igmp_group_s *group)
@@ -220,7 +230,6 @@ void igmp_grpfree(FAR struct net_driver_s *dev, FAR struct igmp_group_s *group)
 
   /* Cancel the wdog */
 
-  net_lock();
   wd_cancel(group->wdog);
 
   /* Remove the group structure from the group list in the device structure */
@@ -237,10 +246,8 @@ void igmp_grpfree(FAR struct net_driver_s *dev, FAR struct igmp_group_s *group)
 
   /* Then release the group structure resources. */
 
-  net_unlock();
   grpinfo("Call sched_kfree()\n");
   sched_kfree(group);
 }
 
 #endif /* CONFIG_NET_IGMP */
-

net/igmp/igmp_join.c

@@ -1,7 +1,7 @@
 /****************************************************************************
  * net/igmp/igmp_join.c
  *
- *   Copyright (C) 2010 Gregory Nutt. All rights reserved.
+ *   Copyright (C) 2010, 2018 Gregory Nutt. All rights reserved.
  *   Author: Gregory Nutt <gnutt@nuttx.org>
  *
  * The NuttX implementation of IGMP was inspired by the IGMP add-on for the
@@ -111,38 +111,48 @@
  *   |  < current timer) |
  *   +-------------------+
  *
+ * Assumptions:
+ *   The network is locked.
+ *
  ****************************************************************************/
 
 int igmp_joingroup(struct net_driver_s *dev, FAR const struct in_addr *grpaddr)
 {
   struct igmp_group_s *group;
+  int ret;
 
-  DEBUGASSERT(dev && grpaddr);
+  DEBUGASSERT(dev != NULL && grpaddr != NULL);
 
   /* Check if a this address is already in the group */
 
   group = igmp_grpfind(dev, &grpaddr->s_addr);
   if (!group)
     {
-       /* No... allocate a new entry */
+      /* No... allocate a new entry */
 
-       ninfo("Join to new group: %08x\n", grpaddr->s_addr);
+      ninfo("Join to new group: %08x\n", grpaddr->s_addr);
-       group = igmp_grpalloc(dev, &grpaddr->s_addr);
+      group = igmp_grpalloc(dev, &grpaddr->s_addr);
-       IGMP_STATINCR(g_netstats.igmp.joins);
+      IGMP_STATINCR(g_netstats.igmp.joins);
 
-       /* Send the Membership Report */
+      /* Send the Membership Report */
 
-       IGMP_STATINCR(g_netstats.igmp.report_sched);
+      IGMP_STATINCR(g_netstats.igmp.report_sched);
-       igmp_waitmsg(group, IGMPv2_MEMBERSHIP_REPORT);
+      ret = igmp_waitmsg(group, IGMPv2_MEMBERSHIP_REPORT);
+      if (ret < 0)
+        {
+          nerr("ERROR: Failed to schedule message: %d\n", ret);
+          igmp_grpfree(dev, group);
+          return ret;
+        }
 
-       /* And start the timer at 10*100 msec */
+      /* And start the timer at 10*100 msec */
 
-       igmp_starttimer(group, 10);
+      igmp_starttimer(group, 10);
 
-       /* Add the group (MAC) address to the ether drivers MAC filter list */
+      /* Add the group (MAC) address to the ether drivers MAC filter list */
 
-       igmp_addmcastmac(dev, (FAR in_addr_t *)&grpaddr->s_addr);
+      igmp_addmcastmac(dev, (FAR in_addr_t *)&grpaddr->s_addr);
-       return OK;
+      return OK;
     }
 
   /* Return EEXIST if the address is already a member of the group */

net/igmp/igmp_leave.c

@@ -1,7 +1,7 @@
 /****************************************************************************
  * net/igmp/igmp_leave.c
  *
- *   Copyright (C) 2010-2011, 2014 Gregory Nutt. All rights reserved.
+ *   Copyright (C) 2010-2011, 2014, 2018 Gregory Nutt. All rights reserved.
  *   Author: Gregory Nutt <gnutt@nuttx.org>
  *
  * The NuttX implementation of IGMP was inspired by the IGMP add-on for the
@@ -119,11 +119,16 @@
  *   |  < current timer) |
  *   +-------------------+
  *
+ * Assumptions:
+ *   The network is locked.
+ *
  ****************************************************************************/
 
-int igmp_leavegroup(struct net_driver_s *dev, FAR const struct in_addr *grpaddr)
+int igmp_leavegroup(struct net_driver_s *dev,
+                    FAR const struct in_addr *grpaddr)
 {
   struct igmp_group_s *group;
+  int ret;
 
   DEBUGASSERT(dev && grpaddr);
 
@@ -139,11 +144,9 @@ int igmp_leavegroup(struct net_driver_s *dev, FAR const struct in_addr *grpaddr)
        * could interfere with the Leave Group.
        */
 
-      net_lock();
       wd_cancel(group->wdog);
       CLR_SCHEDMSG(group->flags);
       CLR_WAITMSG(group->flags);
-      net_unlock();
 
       IGMP_STATINCR(g_netstats.igmp.leaves);
 
@@ -153,7 +156,12 @@ int igmp_leavegroup(struct net_driver_s *dev, FAR const struct in_addr *grpaddr)
         {
           ninfo("Schedule Leave Group message\n");
           IGMP_STATINCR(g_netstats.igmp.leave_sched);
-          igmp_waitmsg(group, IGMP_LEAVE_GROUP);
+
+          ret = igmp_waitmsg(group, IGMP_LEAVE_GROUP);
+          if (ret < 0)
+            {
+              nerr("ERROR: Failed to schedule message: %d\n", ret);
+            }
         }
 
       /* Free the group structure (state is now Non-Member */

net/igmp/igmp_msg.c

@@ -1,7 +1,7 @@
 /****************************************************************************
  * net/igmp/igmp_msg.c
  *
- *   Copyright (C) 2010-2011 Gregory Nutt. All rights reserved.
+ *   Copyright (C) 2010-2011, 2018 Gregory Nutt. All rights reserved.
  *   Author: Gregory Nutt <gnutt@nuttx.org>
  *
  * The NuttX implementation of IGMP was inspired by the IGMP add-on for the
@@ -51,6 +51,7 @@
 #include <nuttx/net/igmp.h>
 
 #include "devif/devif.h"
+#include "netdev/netdev.h"
 #include "igmp/igmp.h"
 
 #ifdef CONFIG_NET_IGMP
@@ -66,20 +67,36 @@
  *   Schedule a message to be send at the next driver polling interval.
  *
  * Assumptions:
- *   This function may be called in most any context.
+ *   The network is locked.
  *
  ****************************************************************************/
 
-void igmp_schedmsg(FAR struct igmp_group_s *group, uint8_t msgid)
+int igmp_schedmsg(FAR struct igmp_group_s *group, uint8_t msgid)
 {
-  /* The following should be atomic */
+  FAR struct net_driver_s *dev;
-  /* REVISIT: Need to notify the device that we have a packet to send */
+
+  DEBUGASSERT(group != NULL && !IS_SCHEDMSG(group->flags));
+  DEBUGASSERT(group->ifindex > 0);
+
+  /* Get the device instance associated with the interface index of the group */
+
+  dev = netdev_findbyindex(group->ifindex);
+  if (dev == NULL)
+    {
+      nerr("ERROR: No device for this interface index: %u\n",
+           group->ifindex);
+      return -ENODEV;
+    }
+
+  /* Schedule the message */
 
-  net_lock();
-  DEBUGASSERT(!IS_SCHEDMSG(group->flags));
   group->msgid = msgid;
   SET_SCHEDMSG(group->flags);
-  net_unlock();
+
+  /* Notify the device that we have a packet to send */
+
+  netdev_txnotify_dev(dev);
+  return OK;
 }
 
 /****************************************************************************
@@ -89,18 +106,26 @@ void igmp_schedmsg(FAR struct igmp_group_s *group, uint8_t msgid)
  *   Schedule a message to be send at the next driver polling interval and
  *   block, waiting for the message to be sent.
  *
+ * Assumptions:
+ *   The network is locked.
+ *
  ****************************************************************************/
 
-void igmp_waitmsg(FAR struct igmp_group_s *group, uint8_t msgid)
+int igmp_waitmsg(FAR struct igmp_group_s *group, uint8_t msgid)
 {
   int ret;
 
   /* Schedule to send the message */
 
-  net_lock();
   DEBUGASSERT(!IS_WAITMSG(group->flags));
   SET_WAITMSG(group->flags);
-  igmp_schedmsg(group, msgid);
+
+  ret = igmp_schedmsg(group, msgid);
+  if (ret < 0)
+    {
+      nerr("ERROR: Failed to schedule the message: %d\n", ret);
+      goto errout;
+    }
 
   /* Then wait for the message to be sent */
 
@@ -111,19 +136,24 @@ void igmp_waitmsg(FAR struct igmp_group_s *group, uint8_t msgid)
       while ((ret = net_lockedwait(&group->sem)) < 0)
         {
           /* The only error that should occur from net_lockedwait() is if
-           * the wait is awakened by a signal.
+           * the wait is awakened by a signal or, perhaps, canceled.
            */
 
           DEBUGASSERT(ret == -EINTR || ret == -ECANCELED);
-        }
+          if (ret != -EINTR)
+            {
+              break;
+            }
 
-      UNUSED(ret);
+          ret = OK;
+        }
     }
 
   /* The message has been sent and we are no longer waiting */
 
+errout:
   CLR_WAITMSG(group->flags);
-  net_unlock();
+  return ret;
 }
 
 #endif /* CONFIG_NET_IGMP */

net/igmp/igmp_timer.c

@@ -1,7 +1,7 @@
 /****************************************************************************
  * net/igmp/igmp_timer.c
  *
- *   Copyright (C) 2010-2011, 2014 Gregory Nutt. All rights reserved.
+ *   Copyright (C) 2010-2011, 2014, 2018 Gregory Nutt. All rights reserved.
  *   Author: Gregory Nutt <gnutt@nuttx.org>
  *
  * The NuttX implementation of IGMP was inspired by the IGMP add-on for the
@@ -49,6 +49,7 @@
 
 #include <nuttx/wdog.h>
 #include <nuttx/irq.h>
+#include <nuttx/wqueue.h>
 #include <nuttx/net/netconfig.h>
 #include <nuttx/net/net.h>
 #include <nuttx/net/netstats.h>
@@ -94,6 +95,60 @@
  * Private Functions
  ****************************************************************************/
 
+/****************************************************************************
+ * Name:  igmp_timeout_work
+ *
+ * Description:
+ *   Timeout watchdog work.
+ *
+ * Assumptions:
+ *   This function is called from a work queue thread.
+ *
+ ****************************************************************************/
+
+static void igmp_timeout_work(FAR void *arg)
+{
+  FAR struct igmp_group_s *group;
+  int ret;
+
+  /* If the state is DELAYING_MEMBER then we send a report for this group */
+
+  group = (FAR struct igmp_group_s *)arg;
+  DEBUGASSERT(group != NULL);
+
+  /* If the group exists and is no an IDLE MEMBER, then it must be a DELAYING
+   * member.  Race conditions are avoided because (1) the timer is not started
+   * until after the first IGMPv2_MEMBERSHIP_REPORT during the join, and (2)
+   * the timer is cancelled before sending the IGMP_LEAVE_GROUP during a leave.
+   */
+
+  net_lock();
+  if (!IS_IDLEMEMBER(group->flags))
+    {
+      /* Schedule (and forget) the Membership Report.  NOTE:
+       * Since we are executing from a timer interrupt, we cannot wait
+       * for the message to be sent.
+       */
+
+      IGMP_STATINCR(g_netstats.igmp.report_sched);
+      ret = igmp_schedmsg(group, IGMPv2_MEMBERSHIP_REPORT);
+      if (ret < 0)
+        {
+          nerr("ERROR: Failed to schedule message: %d\n", ret);
+        }
+
+      /* Also note:  The Membership Report is sent at most two times becasue
+       * the timer is not reset here.  Hmm.. does this mean that the group
+       * is stranded if both reports were lost?  This is consistent with the
+       * RFC that states: "To cover the possibility of the initial Membership
+       * Report being lost or damaged, it is recommended that it be repeated
+       * once or twice after short delays [Unsolicited Report Interval]..."
+       */
+    }
+
+  net_unlock();
+}
+
 /****************************************************************************
  * Name:  igmp_timeout
  *
@@ -109,36 +164,23 @@
 static void igmp_timeout(int argc, uint32_t arg, ...)
 {
   FAR struct igmp_group_s *group;
-
+  int ret;
-  /* If the state is DELAYING_MEMBER then we send a report for this group */
 
   ninfo("Timeout!\n");
-  group = (FAR struct igmp_group_s *)arg;
-  DEBUGASSERT(argc == 1 && group);
 
-  /* If the group exists and is no an IDLE MEMBER, then it must be a DELAYING
+  /* Recover the reference to the group */
-   * member.  Race conditions are avoided because (1) the timer is not started
+
-   * until after the first IGMPv2_MEMBERSHIP_REPORT during the join, and (2)
+  group = (FAR struct igmp_group_s *)arg;
-   * the timer is cancelled before sending the IGMP_LEAVE_GROUP during a leave.
+  DEBUGASSERT(argc == 1 && group != NULL);
+
+  /* Perform the timeout-related operations on (preferably) the low priority
+   * work queue.
    */
 
-  if (!IS_IDLEMEMBER(group->flags))
+  ret = work_queue(LPWORK, &group->work, igmp_timeout_work, group, 0);
+  if (ret < 0)
     {
-      /* Schedule (and forget) the Membership Report.  NOTE:
+      nerr("ERROR: Failed to queue timeout work: %d\n", ret);
-       * Since we are executing from a timer interrupt, we cannot wait
-       * for the message to be sent.
-       */
-
-      IGMP_STATINCR(g_netstats.igmp.report_sched);
-      igmp_schedmsg(group, IGMPv2_MEMBERSHIP_REPORT);
-
-      /* Also note:  The Membership Report is sent at most two times becasue
-       * the timer is not reset here.  Hmm.. does this mean that the group
-       * is stranded if both reports were lost?  This is consistent with the
-       * RFC that states: "To cover the possibility of the initial Membership
-       * Report being lost or damaged, it is recommended that it be repeated
-       * once or twice after short delays [Unsolicited Report Interval]..."
-       */
     }
 }