From 02f83334d58196d627fcee1d077843da297663a3 Mon Sep 17 00:00:00 2001 From: Gregory Nutt Date: Wed, 7 Nov 2018 10:39:51 -0600 Subject: [PATCH] Squashed commit of the following: net/mld: Fix a couple of places where I forgot to unlock the network in the previous commit. net/mld: Implement 'Other Querier Present Timer'. This timer is used to revert to Querier mode if there is no other querier on the network. Also, fix some naming: The Done message is not just Version 1 but is used with Version 2 as well. net/igmp: Back out some blind, backported improvements to IGMP from MLD. There are too many subtle differences in the protocols for this to be safe. --- include/nuttx/net/icmpv6.h | 2 +- include/nuttx/net/mld.h | 6 +-- net/icmpv6/icmpv6_input.c | 8 ++-- net/igmp/igmp_input.c | 87 +++---------------------------------- net/mld/mld.h | 13 +++--- net/mld/mld_done.c | 65 +++++++++------------------- net/mld/mld_join.c | 7 ++- net/mld/mld_leave.c | 11 +++-- net/mld/mld_query.c | 20 +++++---- net/mld/mld_report.c | 2 - net/mld/mld_send.c | 16 +++---- net/mld/mld_timer.c | 89 +++++++++++++++++++++++++------------- 12 files changed, 130 insertions(+), 196 deletions(-) diff --git a/include/nuttx/net/icmpv6.h b/include/nuttx/net/icmpv6.h index 33e75bdb69..787ce3a8cc 100644 --- a/include/nuttx/net/icmpv6.h +++ b/include/nuttx/net/icmpv6.h @@ -73,7 +73,7 @@ #define ICMPv6_ECHO_REPLY 129 #define ICMPV6_MCAST_LISTEN_QUERY 130 /* RFC 2710 and 3810 */ #define ICMPV6_MCAST_LISTEN_REPORT_V1 131 /* RFC 2710 */ -#define ICMPV6_MCAST_LISTEN_DONE_V1 132 /* RFC 2710 */ +#define ICMPV6_MCAST_LISTEN_DONE 132 /* RFC 2710 */ #define ICMPV6_ROUTER_SOLICIT 133 /* RFC 4861 */ #define ICMPV6_ROUTER_ADVERTISE 134 #define ICMPv6_NEIGHBOR_SOLICIT 135 diff --git a/include/nuttx/net/mld.h b/include/nuttx/net/mld.h index 51e24574af..44bcf15ada 100644 --- a/include/nuttx/net/mld.h +++ b/include/nuttx/net/mld.h @@ -358,11 +358,11 @@ struct mld_mcast_listen_report_v2_s sizeof(struct mld_mcast_addrec_v2_s) + \ (addreclen)) -/* Version 1 Multicast Listener Done (RFC 2710) */ +/* Multicast Listener Done (RFC 2710) */ -struct mld_mcast_listen_done_v1_s +struct mld_mcast_listen_done_s { - uint8_t type; /* Message Type: ICMPV6_MCAST_LISTEN_DONE_V1 */ + uint8_t type; /* Message Type: ICMPV6_MCAST_LISTEN_DONE */ uint8_t reserved1; /* Reserved, must be zero on transmission */ uint16_t chksum; /* Checksum of ICMP header and data */ uint16_t reserved2; /* Reserved, must be zero on transmission */ diff --git a/net/icmpv6/icmpv6_input.c b/net/icmpv6/icmpv6_input.c index 24d0cba13c..9007812119 100644 --- a/net/icmpv6/icmpv6_input.c +++ b/net/icmpv6/icmpv6_input.c @@ -78,7 +78,7 @@ #define MLDQUERY ((FAR struct mld_mcast_listen_query_s *)icmpv6) #define MLDREPORT_V1 ((FAR struct mld_mcast_listen_report_v1_s *)icmpv6) #define MLDREPORT_V2 ((FAR struct mld_mcast_listen_report_v2_s *)icmpv6) -#define MLDDONE_V1 ((FAR struct mld_mcast_listen_done_v1_s *)icmpv6) +#define MLDDONE ((FAR struct mld_mcast_listen_done_s *)icmpv6) /**************************************************************************** * Private Functions @@ -525,12 +525,12 @@ void icmpv6_input(FAR struct net_driver_s *dev, unsigned int iplen) } break; - case ICMPV6_MCAST_LISTEN_DONE_V1: /* Version 1 Multicast Listener Done, RFC 2710 */ + case ICMPV6_MCAST_LISTEN_DONE: /* Multicast Listener Done, RFC 2710 */ { - FAR struct mld_mcast_listen_done_v1_s *done = MLDDONE_V1; + FAR struct mld_mcast_listen_done_s *done = MLDDONE; int ret; - ret = mld_done_v1(dev, done); + ret = mld_done(dev, done); if (ret < 0) { goto icmpv6_drop_packet; diff --git a/net/igmp/igmp_input.c b/net/igmp/igmp_input.c index 8e19f578a0..5a8674b5f2 100644 --- a/net/igmp/igmp_input.c +++ b/net/igmp/igmp_input.c @@ -137,33 +137,17 @@ void igmp_input(struct net_driver_s *dev) return; } - /* Find the group (or create a new one) using the incoming IP address. - * If we are not a router (and I assume we are not), then can ignore - * querys for and reports from groups that we are not a member of. - * - * REVISIT: Router support is not yet implemented. - */ + /* Find the group (or create a new one) using the incoming IP address. */ destipaddr = net_ip4addr_conv32(IGMPBUF->destipaddr); -#ifdef CONFIG_IGMP_ROUTER group = igmp_grpallocfind(dev, &destipaddr); if (group == NULL) { - nerr("ERROR: Failed to allocate group: %08x\n", destipaddr); + nerr("ERROR: Failed to find/allocate group: %08x\n", destipaddr); return; } -#else - group = igmp_grpfind(dev, &destipaddr); - if (group == NULL) - { - nwarn("WARNING: Ignoring group. We are not a member: %08x\n", - destipaddr); - return; - } -#endif - /* Now handle the message based on the IGMP message type */ switch (IGMPBUF->type) @@ -200,7 +184,6 @@ void igmp_input(struct net_driver_s *dev) if (IGMPBUF->grpaddr == 0) { FAR struct igmp_group_s *member; - bool rptsent = false; /* This is the general query */ @@ -215,10 +198,6 @@ void igmp_input(struct net_driver_s *dev) IGMP_STATINCR(g_netstats.igmp.query_received); - /* Two passes through the member list. On the first, just - * perform IDLE member checks. - */ - for (member = (FAR struct igmp_group_s *)dev->d_igmp_grplist.head; member; member = member->next) @@ -236,42 +215,6 @@ void igmp_input(struct net_driver_s *dev) } } } - - /* On the second time through, we send the Report in - * response to the query. This has to be done twice because - * because there is only a single packet buffer that is used - * for both incoming and outgoing packets. When the report - * is sent, it will clobber the incoming query. Any attempt - * to send an additional Report would also clobber a preceding - * report - * - * REVISIT: This is a design flaw: Only a single report can - * be sent in this context because there is no mechanism to - * preserve the incoming request nor to queue multiple - * outgoing reports. - */ - - for (member = (FAR struct igmp_group_s *)dev->d_igmp_grplist.head; - member; - member = member->next) - { - /* Skip over the all systems group entry */ - - if (!net_ipv4addr_cmp(member->grpaddr, g_ipv4_allsystems)) - { - /* Send one REPORT and break out of the loop. */ - - igmp_send(dev, member, &member->grpaddr, - IGMPv2_MEMBERSHIP_REPORT); - rptsent = true; - break; - } - } - - if (!rptsent) - { - goto noresponse; - } } else /* if (IGMPBUF->grpaddr != 0) */ { @@ -284,7 +227,7 @@ void igmp_input(struct net_driver_s *dev) IGMP_STATINCR(g_netstats.igmp.ucast_query); grpaddr = net_ip4addr_conv32(IGMPBUF->grpaddr); - group = igmp_grpfind(dev, &grpaddr); + group = igmp_grpallocfind(dev, &grpaddr); if (group != NULL) { @@ -295,15 +238,6 @@ void igmp_input(struct net_driver_s *dev) igmp_startticks(group, ticks); CLR_IDLEMEMBER(group->flags); } - - /* Send the REPORT */ - - igmp_send(dev, group, &group->grpaddr, - IGMPv2_MEMBERSHIP_REPORT); - } - else - { - goto noresponse; } } } @@ -323,11 +257,6 @@ void igmp_input(struct net_driver_s *dev) igmp_startticks(group, ticks); CLR_IDLEMEMBER(group->flags); } - - /* Send the REPORT */ - - igmp_send(dev, group, &group->grpaddr, - IGMPv2_MEMBERSHIP_REPORT); } break; @@ -344,22 +273,16 @@ void igmp_input(struct net_driver_s *dev) SET_IDLEMEMBER(group->flags); CLR_LASTREPORT(group->flags); } - - goto noresponse; } - break; + break; default: { nwarn("WARNING: Unexpected msg %02x\n", IGMPBUF->type); - goto noresponse; } - break; + break; } - return; - -noresponse: dev->d_len = 0; return; } diff --git a/net/mld/mld.h b/net/mld/mld.h index d94ce6df3c..2b5823ddd0 100644 --- a/net/mld/mld.h +++ b/net/mld/mld.h @@ -191,7 +191,7 @@ enum mld_msgtype_e MLD_SEND_GENQUERY, /* Send General Query */ MLD_SEND_V1REPORT, /* Send MLDv1 Report message */ MLD_SEND_V2REPORT, /* Send MLDv2 Report message */ - MLD_SEND_V1DONE /* Send MLDv1 Done message */ + MLD_SEND_DONE /* Send Done message */ }; /* This structure represents one group member. There is a list of groups @@ -243,7 +243,7 @@ struct net_driver_s; /* Forward reference */ struct mld_mcast_listen_query_s; /* Forward reference */ struct mld_mcast_listen_report_v1_s; /* Forward reference */ struct mld_mcast_listen_report_v2_s; /* Forward reference */ -struct mld_mcast_listen_done_v1_s; /* Forward reference */ +struct mld_mcast_listen_done_s; /* Forward reference */ /**************************************************************************** * Name: mld_initialize() @@ -303,16 +303,15 @@ int mld_report_v2(FAR struct net_driver_s *dev, FAR const struct mld_mcast_listen_report_v2_s *report); /**************************************************************************** - * Name: mld_done_v1 + * Name: mld_done * * Description: - * Called from icmpv6_input() when a Version 1 Multicast Listener Done is - * received. + * Called from icmpv6_input() when a Multicast Listener Done is received. * ****************************************************************************/ -int mld_done_v1(FAR struct net_driver_s *dev, - FAR const struct mld_mcast_listen_done_v1_s *done); +int mld_done(FAR struct net_driver_s *dev, + FAR const struct mld_mcast_listen_done_s *done); /**************************************************************************** * Name: mld_grpalloc diff --git a/net/mld/mld_done.c b/net/mld/mld_done.c index 2413998173..34406c2fa3 100644 --- a/net/mld/mld_done.c +++ b/net/mld/mld_done.c @@ -46,22 +46,15 @@ #include "devif/devif.h" #include "mld/mld.h" -/**************************************************************************** - * Pre-processor Definitions - ****************************************************************************/ - -#define IPv6BUF ((FAR struct ipv6_hdr_s *)&dev->d_buf[NET_LL_HDRLEN(dev)]) - /**************************************************************************** * Public Functions ****************************************************************************/ /**************************************************************************** - * Name: mld_done_v1 + * Name: mld_done * * Description: - * Called from icmpv6_input() when a Version 1 Multicast Listener Done is - * received. + * Called from icmpv6_input() when a Multicast Listener Done is received. * * When a router in Querier state receives a Done message from a link, * if the Multicast Address identified in the message is present in the @@ -78,47 +71,29 @@ * ****************************************************************************/ -int mld_done_v1(FAR struct net_driver_s *dev, - FAR const struct mld_mcast_listen_done_v1_s *done) +int mld_done(FAR struct net_driver_s *dev, + FAR const struct mld_mcast_listen_done_s *done) { -#ifdef CONFIG_NET_MLD_ROUTER - FAR struct ipv6_hdr_s *ipv6 = IPv6BUF; - FAR struct mld_group_s *group; - - mldinfo("Version 1 Multicast Listener Done\n"); + mldinfo("Multicast Listener Done\n"); MLD_STATINCR(g_netstats.mld.done_received); - /* The done message is sent to the link-local, all routers multicast - * address. Find the group using the group address in the Done message. + /* The Done message is sent to the link-local, all routers multicast + * address. We basically ignore the Done message: + * + * We cannot free the group if there are other members of the group. We + * know how many local tasks have joined the group, but we are less + * certain of how many non-local members of the group there are. + * + * The RFC requires that we send Multicast-Address-Specific Queries + * repeatedly before removing the group to assure that the no listeners + * are present. + * + * If we are the Querier, then the Query timer logic will accomplish + * this requirement for us. If there is another Querier on the subnet, + * it will drive the Queries. No Querier? We will let the 'Other + * Querier Present Timeout' handle that case. */ - group = mld_grpfind(dev, done->grpaddr); - if (group == NULL) - { - /* We know nothing of this group */ - - return -ENOENT; - } - - /* Ignore the Done message is this is not a Querier */ - - if (IS_MLD_QUERIER(group->flags)) - { - /* REVISIT: Here we just remove the group from this list immediately. - * The RFC requires that we send Multicast-Address-Specific Queries - * repeatedly before doing this to assure that the listener is not - * present. - */ - - mld_grpfree(dev, group); - } -#else - /* We are not a router so we can just ignore Done messages */ - - mldinfo("Version 1 Multicast Listener Done\n"); - MLD_STATINCR(g_netstats.mld.done_received); -#endif - /* Need to set d_len to zero to indication that nothing is being sent */ dev->d_len = 0; diff --git a/net/mld/mld_join.c b/net/mld/mld_join.c index b0cc2ed6d1..d8bb5d9f5f 100644 --- a/net/mld/mld_join.c +++ b/net/mld/mld_join.c @@ -192,6 +192,11 @@ int mld_joingroup(FAR const struct ipv6_mreq *mrec) } MLD_STATINCR(g_netstats.mld.njoins); + + /* REVISIT: It is expected that higher level logic will set up + * the routing table entry for the new multicast address. That + * is not done here. + */ } else { @@ -222,7 +227,7 @@ int mld_joingroup(FAR const struct ipv6_mreq *mrec) } } #else - /* Not a router? There there must be another join from this host or + /* Not a router? Then there must be another join from this host or * how could the group have been created? */ diff --git a/net/mld/mld_leave.c b/net/mld/mld_leave.c index 78655cad02..f1c41a519f 100644 --- a/net/mld/mld_leave.c +++ b/net/mld/mld_leave.c @@ -165,7 +165,7 @@ int mld_leavegroup(FAR const struct ipv6_mreq *mrec) * waiting for a message to be sent. Can that happen? */ - ret = mld_waitmsg(group, MLD_SEND_V1DONE); + ret = mld_waitmsg(group, MLD_SEND_DONE); if (ret < 0) { mlderr("ERROR: Failed to schedule message: %d\n", ret); @@ -178,8 +178,8 @@ int mld_leavegroup(FAR const struct ipv6_mreq *mrec) mld_removemcastmac(dev, mrec->ipv6mr_multiaddr.s6_addr16); - /* Perform additional actions if not a router OR if a router and the - * number of members not on this host is also zero. + /* Perform additional the number of members not on this host is + * also zero. */ #ifdef CONFIG_NET_MLD_ROUTER @@ -200,6 +200,11 @@ int mld_leavegroup(FAR const struct ipv6_mreq *mrec) /* Free the group structure */ mld_grpfree(dev, group); + + /* REVISIT: It is expected that higher level logic will remove + * the routing table entry for the old multicast address. That + * is not done here. + */ } } diff --git a/net/mld/mld_query.c b/net/mld/mld_query.c index 78060c561e..656877336e 100644 --- a/net/mld/mld_query.c +++ b/net/mld/mld_query.c @@ -238,20 +238,22 @@ static void mld_check_querier(FAR struct net_driver_s *dev, if (mld_cmpaddr(dev, ipv6->srcipaddr)) { + /* Switch to non-Querier mode */ + + CLR_MLD_QUERIER(group->flags); + /* Are we past the start up phase (where the timer is used for a * different purpose)? */ if (!IS_MLD_STARTUP(group->flags)) { - /* Yes.. cancel the poll timer */ + /* Yes.. cancel the poll timer and start the 'Other Querier + * Present' Timeout. + */ - wd_cancel(group->polldog); + mld_start_polltimer(group, MSEC2TICK(MLD_OQUERY_MSEC)); } - - /* Switch to non-Querier mode */ - - CLR_MLD_QUERIER(group->flags); } } } @@ -450,9 +452,9 @@ int mld_query(FAR struct net_driver_s *dev, return OK; } - /* Find the group using associated with this group address. For the purpose - * of sending reports, we only care about the query if we are a member of - * the group. + /* Find the group associated with this group address. For the purpose of + * sending reports, we only care about the query if we are a member of the + * group. */ group = mld_grpfind(dev, query->grpaddr); diff --git a/net/mld/mld_report.c b/net/mld/mld_report.c index 05a7b7ff23..994481c717 100644 --- a/net/mld/mld_report.c +++ b/net/mld/mld_report.c @@ -76,8 +76,6 @@ int mld_report(FAR struct net_driver_s *dev, FAR const net_ipv6addr_t grpaddr) /* Find the group (or create a new one) using the incoming IP address. * If we are not a router (and I assume we are not), then can ignore * reports from groups that we are not a member of. - * - * REVISIT: Router support is not yet implemented. */ #ifdef CONFIG_NET_MLD_ROUTER diff --git a/net/mld/mld_send.c b/net/mld/mld_send.c index fe24b6fef0..d6bc595589 100644 --- a/net/mld/mld_send.c +++ b/net/mld/mld_send.c @@ -84,7 +84,7 @@ (&dev->d_buf[NET_LL_HDRLEN(dev)] + MLD_HDRLEN)) #define V2REPORTBUF ((FAR struct mld_mcast_listen_report_v2_s *) \ (&dev->d_buf[NET_LL_HDRLEN(dev)] + MLD_HDRLEN)) -#define DONEBUF ((FAR struct mld_mcast_listen_done_v1_s *) \ +#define DONEBUF ((FAR struct mld_mcast_listen_done_s *) \ (&dev->d_buf[NET_LL_HDRLEN(dev)] + MLD_HDRLEN)) /**************************************************************************** @@ -152,10 +152,10 @@ void mld_send(FAR struct net_driver_s *dev, FAR struct mld_group_s *group, } break; - case MLD_SEND_V1DONE: /* Send MLDv1 Done message */ + case MLD_SEND_DONE: /* Send Done message */ { mldinfo("Send Done message, flags=%02x\n", group->flags); - mldsize = sizeof(struct mld_mcast_listen_done_v1_s); + mldsize = sizeof(struct mld_mcast_listen_done_s); } break; @@ -221,7 +221,7 @@ void mld_send(FAR struct net_driver_s *dev, FAR struct mld_group_s *group, destipaddr = g_ipv6_allmldv2routers; break; - case MLD_SEND_V1DONE: /* Send MLDv1 Done message */ + case MLD_SEND_DONE: /* Send Done message */ destipaddr = g_ipv6_allrouters; break; @@ -341,14 +341,14 @@ void mld_send(FAR struct net_driver_s *dev, FAR struct mld_group_s *group, } break; - case MLD_SEND_V1DONE: /* Send MLDv1 Done message */ + case MLD_SEND_DONE: /* Send Done message */ { - FAR struct mld_mcast_listen_done_v1_s *done = DONEBUF; + FAR struct mld_mcast_listen_done_s *done = DONEBUF; /* Initialize the Done payload */ - memset(done, 0, sizeof(struct mld_mcast_listen_done_v1_s)); - done->type = ICMPV6_MCAST_LISTEN_DONE_V1; + memset(done, 0, sizeof(struct mld_mcast_listen_done_s)); + done->type = ICMPV6_MCAST_LISTEN_DONE; net_ipv6addr_hdrcopy(done->mcastaddr, &group->grpaddr); /* Calculate the ICMPv6 checksum. */ diff --git a/net/mld/mld_timer.c b/net/mld/mld_timer.c index 6aad12f06a..b5815f36a6 100644 --- a/net/mld/mld_timer.c +++ b/net/mld/mld_timer.c @@ -139,46 +139,73 @@ static void mld_polldog_work(FAR void *arg) mld_start_polltimer(group, MSEC2TICK(MLD_QUERY_MSEC)); } } + + net_unlock(); + return; } - /* Check if this is querier */ - - else if (IS_MLD_QUERIER(group->flags)) - { #ifdef CONFIG_NET_MLD_ROUTER - if (group->njoins == 0 group->members == 0 && group->lstmbrs == 0) - { - /* Cancel the timers and discard any queued Reports. Canceling - * the timer will prevent any new Reports from being sent; - * clearing the flags will discard any pending Reports that - * could interfere with freeing the group. - */ + /* This is a Query-related timeout. Destroy the group if there are + * no members of the group detected in the last two Query cycles. + */ - wd_cancel(group->polldog); - wd_cancel(group->v1dog); - CLR_MLD_SCHEDMSG(group->flags); - CLR_MLD_WAITMSG(group->flags); + if (group->njoins == 0 group->members == 0 && group->lstmbrs == 0) + { + /* Cancel the timers and discard any queued Reports. Canceling the + * timer will prevent any new Reports from being sent; clearing the + * the flags will discard any pending Reports that could interfere + * with freeing the group. + */ - /* Free the group structure */ + wd_cancel(group->polldog); + wd_cancel(group->v1dog); + CLR_MLD_SCHEDMSG(group->flags); + CLR_MLD_WAITMSG(group->flags); - mld_grpfree(dev, group); - } - else + /* Free the group structure */ + + mld_grpfree(dev, group); + net_unlock(); + return; + } #endif + + /* Check for an Other Querier Present Timeout. This timer is set in non- + * Querier mode to detect the case where we have lost the Querier. + */ + + if (!IS_MLD_QUERIER(group->flags)) + { + /* We are not the Querier. This is an Other Querier Present Timeout. + * If this timeout expires, it means that there are no Queriers for + * the group. Let's revert to Querier mode. + */ + + SET_MLD_QUERIER(group->flags); + } + + /* Check if this is Querier */ + + if (IS_MLD_QUERIER(group->flags)) + { + /* Schedule (and forget) the general query. */ + + MLD_STATINCR(g_netstats.mld.query_sched); + ret = mld_schedmsg(group, MLD_SEND_GENQUERY); + if (ret < 0) { - /* Schedule (and forget) the general query. */ - - MLD_STATINCR(g_netstats.mld.query_sched); - ret = mld_schedmsg(group, MLD_SEND_GENQUERY); - if (ret < 0) - { - mlderr("ERROR: Failed to schedule message: %d\n", ret); - } - - /* Restart the Querier timer */ - - mld_start_polltimer(group, MSEC2TICK(MLD_QUERY_MSEC)); + mlderr("ERROR: Failed to schedule message: %d\n", ret); } + + /* Restart the Querier timer */ + + mld_start_polltimer(group, MSEC2TICK(MLD_QUERY_MSEC)); + } + else + { + /* Not the Querier... Restart the Other Querier Present Timeout */ + + mld_start_polltimer(group, MSEC2TICK(MLD_OQUERY_MSEC)); } net_unlock();