diff --git a/net/igmp/Kconfig b/net/igmp/Kconfig index 015feab17d..25cd61bd7f 100644 --- a/net/igmp/Kconfig +++ b/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. diff --git a/net/igmp/igmp.h b/net/igmp/igmp.h index 69eae36ac7..2ecb5b640d 100644 --- a/net/igmp/igmp.h +++ b/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 * * Redistribution and use in source and binary forms, with or without @@ -77,6 +77,7 @@ #include +#include #include #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 diff --git a/net/igmp/igmp_group.c b/net/igmp/igmp_group.c index cb240b6c97..0c6aa68d32 100644 --- a/net/igmp/igmp_group.c +++ b/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 * * 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 */ - diff --git a/net/igmp/igmp_join.c b/net/igmp/igmp_join.c index 51fb4eb25b..ca145b02b5 100644 --- a/net/igmp/igmp_join.c +++ b/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 * * 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); - group = igmp_grpalloc(dev, &grpaddr->s_addr); - IGMP_STATINCR(g_netstats.igmp.joins); + ninfo("Join to new group: %08x\n", grpaddr->s_addr); + group = igmp_grpalloc(dev, &grpaddr->s_addr); + IGMP_STATINCR(g_netstats.igmp.joins); - /* Send the Membership Report */ + /* Send the Membership Report */ - IGMP_STATINCR(g_netstats.igmp.report_sched); - igmp_waitmsg(group, IGMPv2_MEMBERSHIP_REPORT); + IGMP_STATINCR(g_netstats.igmp.report_sched); + 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); - return OK; + igmp_addmcastmac(dev, (FAR in_addr_t *)&grpaddr->s_addr); + return OK; } /* Return EEXIST if the address is already a member of the group */ diff --git a/net/igmp/igmp_leave.c b/net/igmp/igmp_leave.c index 85d6d9a403..e518638b46 100644 --- a/net/igmp/igmp_leave.c +++ b/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 * * 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 */ diff --git a/net/igmp/igmp_msg.c b/net/igmp/igmp_msg.c index 35d2e20acf..2cb7cc031c 100644 --- a/net/igmp/igmp_msg.c +++ b/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 * * The NuttX implementation of IGMP was inspired by the IGMP add-on for the @@ -51,6 +51,7 @@ #include #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 */ - /* REVISIT: Need to notify the device that we have a packet to send */ + FAR struct net_driver_s *dev; + + 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 */ diff --git a/net/igmp/igmp_timer.c b/net/igmp/igmp_timer.c index e989039282..84aa8ed508 100644 --- a/net/igmp/igmp_timer.c +++ b/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 * * The NuttX implementation of IGMP was inspired by the IGMP add-on for the @@ -49,6 +49,7 @@ #include #include +#include #include #include #include @@ -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; - - /* If the state is DELAYING_MEMBER then we send a report for this group */ + int ret; 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 - * 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. + /* Recover the reference to the group */ + + group = (FAR struct igmp_group_s *)arg; + 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: - * 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]..." - */ + nerr("ERROR: Failed to queue timeout work: %d\n", ret); } }