From 0aa448f190e45f0344392459ba681e924670215f Mon Sep 17 00:00:00 2001 From: Adam Dunkels Date: Sun, 28 Jul 2013 22:32:09 +0200 Subject: [PATCH] 6lowpan fragmentation bugfix: the 6lowpan code had an unfortunate interaction with the behavior of the rdc layer. If the first packet of a fragment transmission was lost, the remaining packets would get dropped on reception. Moreover, the reception code contained a bug that sometimes would cause fragments to be misidentified as fragments. Taken together, these problems would result in a pathelogical network breakdown if too many fragmented packets would occur simultaneously. --- core/net/mac/nullrdc.c | 59 +++++++++++++++++++++++++++++++++--------- core/net/sicslowpan.c | 26 +++++++++++++++++++ 2 files changed, 73 insertions(+), 12 deletions(-) diff --git a/core/net/mac/nullrdc.c b/core/net/mac/nullrdc.c index ed497c327..ff367cf63 100644 --- a/core/net/mac/nullrdc.c +++ b/core/net/mac/nullrdc.c @@ -119,10 +119,12 @@ static struct seqno received_seqnos[MAX_SEQNOS]; #endif /* NULLRDC_802154_AUTOACK || NULLRDC_802154_AUTOACK_HW */ /*---------------------------------------------------------------------------*/ -static void -send_packet(mac_callback_t sent, void *ptr) +static int +send_one_packet(mac_callback_t sent, void *ptr) { int ret; + int last_sent_ok = 0; + packetbuf_set_addr(PACKETBUF_ADDR_SENDER, &rimeaddr_node_addr); #if NULLRDC_802154_AUTOACK || NULLRDC_802154_AUTOACK_HW packetbuf_set_attr(PACKETBUF_ATTR_MAC_ACK, 1); @@ -228,22 +230,49 @@ send_packet(mac_callback_t sent, void *ptr) #endif /* ! NULLRDC_802154_AUTOACK */ } + if(ret == MAC_TX_OK) { + last_sent_ok = 1; + } mac_call_sent_callback(sent, ptr, ret, 1); + return last_sent_ok; +} +/*---------------------------------------------------------------------------*/ +static void +send_packet(mac_callback_t sent, void *ptr) +{ + send_one_packet(sent, ptr); } /*---------------------------------------------------------------------------*/ static void send_list(mac_callback_t sent, void *ptr, struct rdc_buf_list *buf_list) { while(buf_list != NULL) { + /* We backup the next pointer, as it may be nullified by + * mac_call_sent_callback() */ + struct rdc_buf_list *next = buf_list->next; + int last_sent_ok; + queuebuf_to_packetbuf(buf_list->buf); - send_packet(sent, ptr); - buf_list = buf_list->next; + last_sent_ok = send_one_packet(sent, ptr); + + /* If packet transmission was not successful, we should back off and let + * upper layers retransmit, rather than potentially sending out-of-order + * packet fragments. */ + if(!last_sent_ok) { + return; + } + buf_list = next; } } /*---------------------------------------------------------------------------*/ static void packet_input(void) { + int original_datalen; + uint8_t *original_dataptr; + + original_datalen = packetbuf_datalen(); + original_dataptr = packetbuf_dataptr(); #ifdef NETSTACK_DECRYPT NETSTACK_DECRYPT(); #endif /* NETSTACK_DECRYPT */ @@ -264,6 +293,8 @@ packet_input(void) PRINTF("nullrdc: not for us\n"); #endif /* NULLRDC_ADDRESS_FILTER */ } else { + int duplicate = 0; + #if NULLRDC_802154_AUTOACK || NULLRDC_802154_AUTOACK_HW /* Check for duplicate packet by comparing the sequence number of the incoming packet with the last few ones we saw. */ @@ -275,16 +306,18 @@ packet_input(void) /* Drop the packet. */ PRINTF("nullrdc: drop duplicate link layer packet %u\n", packetbuf_attr(PACKETBUF_ATTR_PACKET_ID)); - return; + duplicate = 1; } } - for(i = MAX_SEQNOS - 1; i > 0; --i) { - memcpy(&received_seqnos[i], &received_seqnos[i - 1], - sizeof(struct seqno)); + if(!duplicate) { + for(i = MAX_SEQNOS - 1; i > 0; --i) { + memcpy(&received_seqnos[i], &received_seqnos[i - 1], + sizeof(struct seqno)); + } + received_seqnos[0].seqno = packetbuf_attr(PACKETBUF_ATTR_PACKET_ID); + rimeaddr_copy(&received_seqnos[0].sender, + packetbuf_addr(PACKETBUF_ADDR_SENDER)); } - received_seqnos[0].seqno = packetbuf_attr(PACKETBUF_ATTR_PACKET_ID); - rimeaddr_copy(&received_seqnos[0].sender, - packetbuf_addr(PACKETBUF_ADDR_SENDER)); #endif /* NULLRDC_802154_AUTOACK */ #if NULLRDC_SEND_802154_ACK @@ -304,7 +337,9 @@ packet_input(void) } } #endif /* NULLRDC_SEND_ACK */ - NETSTACK_MAC.input(); + if(!duplicate) { + NETSTACK_MAC.input(); + } } } /*---------------------------------------------------------------------------*/ diff --git a/core/net/sicslowpan.c b/core/net/sicslowpan.c index 78bc0dceb..ca55ee9ec 100644 --- a/core/net/sicslowpan.c +++ b/core/net/sicslowpan.c @@ -1590,6 +1590,7 @@ input(void) uint16_t frag_size = 0; /* offset of the fragment in the IP packet */ uint8_t frag_offset = 0; + uint8_t is_fragment = 0; #if SICSLOWPAN_CONF_FRAG /* tag of the fragment */ uint16_t frag_tag = 0; @@ -1626,6 +1627,7 @@ input(void) rime_hdr_len += SICSLOWPAN_FRAG1_HDR_LEN; /* printf("frag1 %d %d\n", reass_tag, frag_tag);*/ first_fragment = 1; + is_fragment = 1; break; case SICSLOWPAN_DISPATCH_FRAGN: /* @@ -1648,11 +1650,29 @@ input(void) if(processed_ip_in_len + packetbuf_datalen() - rime_hdr_len >= frag_size) { last_fragment = 1; } + is_fragment = 1; break; default: break; } + /* We are currently reassembling a packet, but have just received the first + * fragment of another packet. We can either ignore it and hope to receive + * the rest of the under-reassembly packet fragments, or we can discard the + * previous packet altogether, and start reassembling the new packet. + * + * We discard the previous packet, and start reassembling the new packet. + * This lessens the negative impacts of too high SICSLOWPAN_REASS_MAXAGE. + */ +#define PRIORITIZE_NEW_PACKETS 1 +#if PRIORITIZE_NEW_PACKETS + if(processed_ip_in_len > 0 && first_fragment + && !rimeaddr_cmp(&frag_sender, packetbuf_addr(PACKETBUF_ADDR_SENDER))) { + sicslowpan_len = 0; + processed_ip_in_len = 0; + } +#endif /* PRIORITIZE_NEW_PACKETS */ + if(processed_ip_in_len > 0) { /* reassembly is ongoing */ /* printf("frag %d %d\n", reass_tag, frag_tag);*/ @@ -1674,6 +1694,12 @@ input(void) * start it if we received a fragment */ if((frag_size > 0) && (frag_size <= UIP_BUFSIZE)) { + /* We are currently not reassembling a packet, but have received a packet fragment + * that is not the first one. */ + if(is_fragment && !first_fragment) { + return; + } + sicslowpan_len = frag_size; reass_tag = frag_tag; timer_set(&reass_timer, SICSLOWPAN_REASS_MAXAGE * CLOCK_SECOND / 16);