/net/devif/ipv6_input.c: Correct handling of IPv6 extension headers. The main confusion was that the payload length in the IPv6 header does not include its extension headers.

net/icmpv6/icmpv6_input.c:  Correct caculation of the ICMPv6 header address
    net/mld/mld_query.c:  Correct back test for group found.
    net/mld/mld_report.c: Fix host vs. network order problem.
    net/mld/mld_send.c: Correct the address used in sending the General Query.  It should be the unspecified address in that case.
    net/mld:  Querying workaround.  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.
This commit is contained in:
Gregory Nutt 2018-11-10 11:29:02 -06:00
parent b0ba5b69c4
commit 854046a931
8 changed files with 82 additions and 56 deletions

13
TODO
View file

@ -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)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

View file

@ -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

View file

@ -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)

View file

@ -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 */

View file

@ -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))
{

View file

@ -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)
{

View file

@ -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 */

View file

@ -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))