diff --git a/TODO b/TODO index 4810210e04..34ed0b7398 100644 --- a/TODO +++ b/TODO @@ -1,4 +1,4 @@ -NuttX TODO List (Last updated November 6, 2018) +NuttX TODO List (Last updated November 10, 2018) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ This file summarizes known NuttX bugs, limitations, inconsistencies with @@ -1489,6 +1489,17 @@ o Network (net/, drivers/net) anything but a well-known point-to-point configuration impossible. + Title: MLD QUERIER DESIGN + Description: The MLD implementation does not follow the RFC correct when it is + the Querier. The Querier should use a general query and get + query messages from all members of all groups. This would be + driven by a single timer since all groups are queried at once. + Instead, the design currently uses a Multicast Address Specific + Query with one timer per group and ignores groups that we are + not members of. + Status: Open + Priority: Low. There are no customers of MLD as far as I know. + o USB (drivers/usbdev, drivers/usbhost) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/net/devif/ipv6_input.c b/net/devif/ipv6_input.c index 90cbabefba..c596051c81 100644 --- a/net/devif/ipv6_input.c +++ b/net/devif/ipv6_input.c @@ -277,7 +277,7 @@ int ipv6_input(FAR struct net_driver_s *dev) FAR uint8_t *payload; uint16_t llhdrlen; uint16_t iphdrlen; - uint16_t pktlen; + uint16_t paylen; uint8_t nxthdr; #ifdef CONFIG_NET_IPFORWARD int ret; @@ -321,6 +321,37 @@ int ipv6_input(FAR struct net_driver_s *dev) IFF_SET_IPv6(dev->d_flags); + /* Check the size of the packet. If the size reported to us in d_len is + * smaller the size reported in the IP header, we assume that the packet + * has been corrupted in transit. If the size of d_len is larger than the + * size reported in the IP packet header, the packet has been padded and + * we set d_len to the correct value. + * + * The length reported in the IPv6 header is the length of the payload + * that follows the header. The device interface uses the d_len variable + * for holding the size of the entire packet, including the IP header but + * without the link layer header (subtracted out above). + * + * NOTE: The payload length in the includes the size of the Ipv6 extension + * options, but not the size of the IPv6 header. + * + * REVISIT: Length will be set to zero if the extension header carries + * a Jumbo payload option. + */ + + paylen = ((uint16_t)ipv6->len[0] << 8) + (uint16_t)ipv6->len[1] + + IPv6_HDRLEN; + + if (paylen <= dev->d_len) + { + dev->d_len = paylen; + } + else + { + nwarn("WARNING: IP packet shorter than length in IP header\n"); + goto drop; + } + /* Parse IPv6 extension headers (parsed but ignored) */ payload = PAYLOAD; /* Assume payload starts right after IPv6 header */ @@ -338,48 +369,9 @@ int ipv6_input(FAR struct net_driver_s *dev) extlen = EXTHDR_LEN((unsigned int)exthdr->len); payload += extlen; iphdrlen += extlen; - - /* Check for a short packet */ - - if (iphdrlen > dev->d_len) - { - nwarn("WARNING: Packet shorter than IPv6 header\n"); - goto drop; - } - - /* Set up for the next time through the loop */ - - exthdr = (FAR struct ipv6_extension_s *)payload; nxthdr = exthdr->nxthdr; } - /* Check the size of the packet. If the size reported to us in d_len is - * smaller the size reported in the IP header, we assume that the packet - * has been corrupted in transit. If the size of d_len is larger than the - * size reported in the IP packet header, the packet has been padded and - * we set d_len to the correct value. - * - * The length reported in the IPv6 header is the length of the payload - * that follows the header. The device interface uses the d_len variable - * for holding the size of the entire packet, including the IP header but - * without the link layer header. - * - * REVISIT: Length will be set to zero if the extension header carries - * a Jumbo payload option. - */ - - pktlen = ((uint16_t)ipv6->len[0] << 8) + (uint16_t)ipv6->len[1] + iphdrlen; - - if (pktlen <= dev->d_len) - { - dev->d_len = pktlen; - } - else - { - nwarn("WARNING: IP packet shorter than length in IP header\n"); - goto drop; - } - #ifdef CONFIG_NET_BROADCAST /* Check for a multicast packet, which may be destined to us (even if * there is no IP address yet assigned to the device). We only expect diff --git a/net/icmpv6/icmpv6_input.c b/net/icmpv6/icmpv6_input.c index ed2555b12a..4f7ac2b0ac 100644 --- a/net/icmpv6/icmpv6_input.c +++ b/net/icmpv6/icmpv6_input.c @@ -67,7 +67,7 @@ #define IPv6BUF ((FAR struct ipv6_hdr_s *)&dev->d_buf[NET_LL_HDRLEN(dev)]) #define ICMPv6BUF ((FAR struct icmpv6_hdr_s *) \ - (&dev->d_buf[NET_LL_HDRLEN(dev)]) + iplen) + (&dev->d_buf[NET_LL_HDRLEN(dev)] + iplen)) #define ICMPv6REPLY ((FAR struct icmpv6_echo_reply_s *)icmpv6) #define ICMPv6SIZE ((dev)->d_len - iplen) diff --git a/net/mld/mld.h b/net/mld/mld.h index 2b5823ddd0..793a322e40 100644 --- a/net/mld/mld.h +++ b/net/mld/mld.h @@ -189,6 +189,7 @@ enum mld_msgtype_e { MLD_SEND_NONE = 0, /* Nothing to send */ MLD_SEND_GENQUERY, /* Send General Query */ + MLD_SEND_MASQUERY, /* Send Multicast Address Specific (MAS) Query */ MLD_SEND_V1REPORT, /* Send MLDv1 Report message */ MLD_SEND_V2REPORT, /* Send MLDv2 Report message */ MLD_SEND_DONE /* Send Done message */ diff --git a/net/mld/mld_group.c b/net/mld/mld_group.c index b34e442ab8..5d2d319cd8 100644 --- a/net/mld/mld_group.c +++ b/net/mld/mld_group.c @@ -156,7 +156,9 @@ FAR struct mld_group_s *mld_grpfind(FAR struct net_driver_s *dev, { FAR struct mld_group_s *group; - mldinfo("Searching for addr %08x\n", (int)*addr); + mldinfo("Searching for group: %04x:%04x:%04x:%04x:%04x:%04x:%04x:%04x\n", + addr[0], addr[1], addr[2], addr[3], addr[4], addr[5], addr[6], + addr[7]); for (group = (FAR struct mld_group_s *)dev->d_mld_grplist.head; group; @@ -166,9 +168,6 @@ FAR struct mld_group_s *mld_grpfind(FAR struct net_driver_s *dev, group->grpaddr[0], group->grpaddr[1], group->grpaddr[2], group->grpaddr[3], group->grpaddr[4], group->grpaddr[5], group->grpaddr[6], group->grpaddr[7]); - mldinfo("Versus: %04x:%04x:%04x:%04x:%04x:%04x:%04x:%04x\n", - addr[0], addr[1], addr[2], addr[3], - addr[4], addr[5], addr[6], addr[7]); if (net_ipv6addr_cmp(group->grpaddr, addr)) { diff --git a/net/mld/mld_query.c b/net/mld/mld_query.c index 6fbf689e55..2da2b4c264 100644 --- a/net/mld/mld_query.c +++ b/net/mld/mld_query.c @@ -453,13 +453,14 @@ int mld_query(FAR struct net_driver_s *dev, return OK; } - /* Find the group associated with this group address. For the purpose of + /* All of other Queries are sent with the group address set. Find the + * group instance 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); - if (group != NULL) + if (group == NULL) { mldinfo("We are not a member of this group\n"); @@ -487,9 +488,12 @@ int mld_query(FAR struct net_driver_s *dev, mldinfo("WARNING: MLDv2 query received in MLDv1 compatibility mode\n"); } - /* Check for Multicast Address Specific Query */ + /* Check for Multicast Address Specific (MAS) Query or a Multicast Address + * and Source Specific (MASS) Query. MAS and MASS queries are sent with + * an IP destination address equal to the multicast address of interest. + */ - if (net_ipv6addr_cmp(ipv6->destipaddr, g_ipv6_allrouters)) + if (net_ipv6addr_cmp(ipv6->destipaddr, group->grpaddr)) { if (query->nsources == 0) { diff --git a/net/mld/mld_report.c b/net/mld/mld_report.c index 85e1407059..67004cc732 100644 --- a/net/mld/mld_report.c +++ b/net/mld/mld_report.c @@ -187,6 +187,7 @@ int mld_report_v1(FAR struct net_driver_s *dev, int mld_report_v2(FAR struct net_driver_s *dev, FAR const struct mld_mcast_listen_report_v2_s *report) { + uint16_t naddrec; int ret = -ENOENT; int i; @@ -195,7 +196,8 @@ int mld_report_v2(FAR struct net_driver_s *dev, MLD_STATINCR(g_netstats.mld.v2report_received); - for (i = 0; i < report->naddrec; i++) + naddrec = NTOHS(report->naddrec); + for (i = 0; i < naddrec; i++) { /* Handle this mcast address in the list */ diff --git a/net/mld/mld_send.c b/net/mld/mld_send.c index d6bc595589..e9e7617df4 100644 --- a/net/mld/mld_send.c +++ b/net/mld/mld_send.c @@ -129,6 +129,7 @@ void mld_send(FAR struct net_driver_s *dev, FAR struct mld_group_s *group, switch (msgtype) { case MLD_SEND_GENQUERY: /* Send General Query */ + case MLD_SEND_MASQUERY: /* Send Multicast Address Specific (MAS) Query */ { mldinfo("Send General Query, flags=%02x\n", group->flags); mldsize = SIZEOF_MLD_MCAST_LISTEN_QUERY_S(0); @@ -201,15 +202,16 @@ void mld_send(FAR struct net_driver_s *dev, FAR struct mld_group_s *group, * being sent: * * MESSAGE DESTINATION ADDRESS - * General Query Message: The link-local, all nodes multicast address - * MAS Query Messages: The group multicast address - * Report Message: The group multicast address + * General Query Message: The link-local, all nodes multicast address. + * MAS Query Messages: The group multicast address. + * Report Message: The group multicast address. * Done Message: The link-local, all routers multicast address. */ switch (msgtype) { case MLD_SEND_GENQUERY: /* Send General Query */ + case MLD_SEND_MASQUERY: /* Send Multicast Address Specific (MAS) Query */ destipaddr = g_ipv6_allnodes; break; @@ -253,7 +255,8 @@ void mld_send(FAR struct net_driver_s *dev, FAR struct mld_group_s *group, switch (msgtype) { - case MLD_SEND_GENQUERY: + case MLD_SEND_GENQUERY: /* Send General Query */ + case MLD_SEND_MASQUERY: /* Send Multicast Address Specific (MAS) Query */ { FAR struct mld_mcast_listen_query_s *query = QUERYBUF; @@ -266,10 +269,24 @@ void mld_send(FAR struct net_driver_s *dev, FAR struct mld_group_s *group, */ memset(query, 0, sizeof(struct mld_mcast_listen_query_s)); - net_ipv6addr_hdrcopy(query->grpaddr, &group->grpaddr); query->type = ICMPV6_MCAST_LISTEN_QUERY; query->mrc = MLD_QRESP_MSEC; + /* The General Query and the MAS Query differ only in that the + * setting of the group multicast address field. This field + * is the unspecified address for General Query, but the group + * multicast address for the MAS query. + */ + + if (msgtype == MLD_SEND_GENQUERY) + { + net_ipv6addr_hdrcopy(query->grpaddr, g_ipv6_unspecaddr); + } + else + { + net_ipv6addr_hdrcopy(query->grpaddr, group->grpaddr); + } + /* Fields unique to the extended MLDv2 query */ if (!IS_MLD_V1COMPAT(group->flags))