From fb6451ef7b3f32e12d3ec0d7fe1de34476e4b4a0 Mon Sep 17 00:00:00 2001 From: Simon Duquennoy Date: Fri, 12 Oct 2018 11:04:40 +0200 Subject: [PATCH 1/4] RPL-Lite: rework DAO sending/resending/refreshing logic --- os/net/routing/rpl-lite/rpl-dag.c | 6 ++- os/net/routing/rpl-lite/rpl-icmp6.c | 6 +-- os/net/routing/rpl-lite/rpl-timers.c | 81 ++++++++++++++-------------- os/net/routing/rpl-lite/rpl-timers.h | 5 ++ os/net/routing/rpl-lite/rpl-types.h | 1 - 5 files changed, 53 insertions(+), 46 deletions(-) diff --git a/os/net/routing/rpl-lite/rpl-dag.c b/os/net/routing/rpl-lite/rpl-dag.c index 09caa9625..5ec080c15 100644 --- a/os/net/routing/rpl-lite/rpl-dag.c +++ b/os/net/routing/rpl-lite/rpl-dag.c @@ -104,7 +104,7 @@ rpl_dag_leave(void) /* Issue a no-path DAO */ if(!rpl_dag_root_is_root()) { - RPL_LOLLIPOP_INCREMENT(curr_instance.dag.dao_curr_seqno); + RPL_LOLLIPOP_INCREMENT(curr_instance.dag.dao_last_seqno); rpl_icmp6_dao_output(0); } @@ -507,7 +507,7 @@ init_dag(uint8_t instance_id, uip_ipaddr_t *dag_id, rpl_ocp_t ocp, curr_instance.dag.lowest_rank = RPL_INFINITE_RANK; curr_instance.dag.dao_last_seqno = RPL_LOLLIPOP_INIT; curr_instance.dag.dao_last_acked_seqno = RPL_LOLLIPOP_INIT; - curr_instance.dag.dao_curr_seqno = RPL_LOLLIPOP_INIT; + curr_instance.dag.dao_last_seqno = RPL_LOLLIPOP_INIT; memcpy(&curr_instance.dag.dag_id, dag_id, sizeof(curr_instance.dag.dag_id)); return 1; @@ -657,6 +657,8 @@ rpl_process_dao_ack(uint8_t sequence, uint8_t status) curr_instance.dag.state = DAG_REACHABLE; rpl_timers_dio_reset("Reachable"); } + /* Let the rpl-timers module know that we got an ACK for the last DAO */ + rpl_timers_notify_dao_ack(); if(!status_ok) { /* We got a NACK, start poisoning and leave */ diff --git a/os/net/routing/rpl-lite/rpl-icmp6.c b/os/net/routing/rpl-lite/rpl-icmp6.c index be5ea2948..8cf4c853c 100644 --- a/os/net/routing/rpl-lite/rpl-icmp6.c +++ b/os/net/routing/rpl-lite/rpl-icmp6.c @@ -583,7 +583,7 @@ rpl_icmp6_dao_output(uint8_t lifetime) #endif /* RPL_WITH_DAO_ACK */ ++pos; buffer[pos++] = 0; /* reserved */ - buffer[pos++] = curr_instance.dag.dao_curr_seqno; + buffer[pos++] = curr_instance.dag.dao_last_seqno; /* create target subopt */ prefixlen = sizeof(*prefix) * CHAR_BIT; @@ -610,7 +610,7 @@ rpl_icmp6_dao_output(uint8_t lifetime) LOG_INFO("sending a %sDAO seqno %u, tx count %u, lifetime %u, prefix ", lifetime == 0 ? "No-path " : "", - curr_instance.dag.dao_curr_seqno, curr_instance.dag.dao_transmissions, lifetime); + curr_instance.dag.dao_last_seqno, curr_instance.dag.dao_transmissions, lifetime); LOG_INFO_6ADDR(prefix); LOG_INFO_(" to "); LOG_INFO_6ADDR(&curr_instance.dag.dag_id); @@ -644,7 +644,7 @@ dao_ack_input(void) LOG_INFO("received a DAO-%s with seqno %d (%d %d) and status %d from ", status < RPL_DAO_ACK_UNABLE_TO_ACCEPT ? "ACK" : "NACK", sequence, - curr_instance.dag.dao_curr_seqno, curr_instance.dag.dao_curr_seqno, status); + curr_instance.dag.dao_last_seqno, curr_instance.dag.dao_last_seqno, status); LOG_INFO_6ADDR(&UIP_IP_BUF->srcipaddr); LOG_INFO_("\n"); diff --git a/os/net/routing/rpl-lite/rpl-timers.c b/os/net/routing/rpl-lite/rpl-timers.c index c41b92948..169226fd5 100644 --- a/os/net/routing/rpl-lite/rpl-timers.c +++ b/os/net/routing/rpl-lite/rpl-timers.c @@ -71,8 +71,9 @@ clock_time_t RPL_PROBING_DELAY_FUNC(void); static void handle_dis_timer(void *ptr); static void handle_dio_timer(void *ptr); static void handle_unicast_dio_timer(void *ptr); -static void handle_dao_timer(void *ptr); +static void send_new_dao(void *ptr); #if RPL_WITH_DAO_ACK +static void resend_dao(void *ptr); static void handle_dao_ack_timer(void *ptr); #endif /* RPL_WITH_DAO_ACK */ #if RPL_WITH_PROBING @@ -224,7 +225,7 @@ static void schedule_dao_retransmission(void) { clock_time_t expiration_time = RPL_DAO_RETRANSMISSION_TIMEOUT / 2 + (random_rand() % (RPL_DAO_RETRANSMISSION_TIMEOUT)); - ctimer_set(&curr_instance.dag.dao_timer, expiration_time, handle_dao_timer, NULL); + ctimer_set(&curr_instance.dag.dao_timer, expiration_time, resend_dao, NULL); } #endif /* RPL_WITH_DAO_ACK */ /*---------------------------------------------------------------------------*/ @@ -247,9 +248,8 @@ schedule_dao_refresh(void) target_refresh -= safety_margin; } - /* Increment next sequno */ - RPL_LOLLIPOP_INCREMENT(curr_instance.dag.dao_curr_seqno); - ctimer_set(&curr_instance.dag.dao_timer, target_refresh, handle_dao_timer, NULL); + /* Schedule transmission */ + ctimer_set(&curr_instance.dag.dao_timer, target_refresh, send_new_dao, NULL); } } /*---------------------------------------------------------------------------*/ @@ -261,36 +261,16 @@ rpl_timers_schedule_dao(void) * only serves storing mode. Use simple delay instead, with the only purpose * to reduce congestion. */ clock_time_t expiration_time = RPL_DAO_DELAY / 2 + (random_rand() % (RPL_DAO_DELAY)); - /* Increment next seqno */ - RPL_LOLLIPOP_INCREMENT(curr_instance.dag.dao_curr_seqno); - ctimer_set(&curr_instance.dag.dao_timer, expiration_time, handle_dao_timer, NULL); + ctimer_set(&curr_instance.dag.dao_timer, expiration_time, send_new_dao, NULL); } } /*---------------------------------------------------------------------------*/ static void -handle_dao_timer(void *ptr) +send_new_dao(void *ptr) { #if RPL_WITH_DAO_ACK - if(rpl_lollipop_greater_than(curr_instance.dag.dao_curr_seqno, - curr_instance.dag.dao_last_seqno)) { - /* We are sending a new DAO here. Prepare retransmissions */ - curr_instance.dag.dao_transmissions = 0; - } else { - /* We are called for the same DAO again */ - if(curr_instance.dag.dao_last_acked_seqno == curr_instance.dag.dao_last_seqno) { - /* The last seqno sent is ACKed! Schedule refresh to avoid route expiration */ - schedule_dao_refresh(); - return; - } - /* We need to re-send the last DAO */ - if(curr_instance.dag.dao_transmissions >= RPL_DAO_MAX_RETRANSMISSIONS) { - /* No more retransmissions. Perform local repair and hope to find another . */ - rpl_local_repair("DAO max rtx"); - return; - } - } - /* Increment transmission counter before sending */ - curr_instance.dag.dao_transmissions++; + /* We are sending a new DAO here. Prepare retransmissions */ + curr_instance.dag.dao_transmissions = 1; /* Schedule next retransmission */ schedule_dao_retransmission(); #else /* RPL_WITH_DAO_ACK */ @@ -299,19 +279,14 @@ handle_dao_timer(void *ptr) curr_instance.dag.state = DAG_REACHABLE; } rpl_timers_dio_reset("Reachable"); -#endif /* !RPL_WITH_DAO_ACK */ - - curr_instance.dag.dao_last_seqno = curr_instance.dag.dao_curr_seqno; - /* Send a DAO with own prefix as target and default lifetime */ - rpl_icmp6_dao_output(curr_instance.default_lifetime); - -#if !RPL_WITH_DAO_ACK - /* There is no DAO-ACK, schedule a refresh. Must be done after rpl_icmp6_dao_output, - because we increment curr_instance.dag.dao_curr_seqno for the next DAO (refresh). - Where there is DAO-ACK, the refresh is scheduled after reception of the ACK. - Happens when handle_dao_timer is called again next. */ + /* There is no DAO-ACK, schedule a refresh. */ schedule_dao_refresh(); #endif /* !RPL_WITH_DAO_ACK */ + + /* Increment seqno */ + RPL_LOLLIPOP_INCREMENT(curr_instance.dag.dao_last_seqno); + /* Send a DAO with own prefix as target and default lifetime */ + rpl_icmp6_dao_output(curr_instance.default_lifetime); } #if RPL_WITH_DAO_ACK /*---------------------------------------------------------------------------*/ @@ -334,6 +309,32 @@ handle_dao_ack_timer(void *ptr) rpl_icmp6_dao_ack_output(&curr_instance.dag.dao_ack_target, curr_instance.dag.dao_ack_sequence, RPL_DAO_ACK_UNCONDITIONAL_ACCEPT); } +/*---------------------------------------------------------------------------*/ +void +rpl_timers_notify_dao_ack(void) +{ + /* The last DAO was ACKed. Schedule refresh to avoid route expiration. This + implicitly de-schedules resend_dao, as both share curr_instance.dag.dao_timer */ + schedule_dao_refresh(); +} +/*---------------------------------------------------------------------------*/ +static void +resend_dao(void *ptr) +{ + /* Increment transmission counter before sending */ + curr_instance.dag.dao_transmissions++; + /* Send a DAO with own prefix as target and default lifetime */ + rpl_icmp6_dao_output(curr_instance.default_lifetime); + + /* Schedule next retransmission, or abort */ + if(curr_instance.dag.dao_transmissions < RPL_DAO_MAX_RETRANSMISSIONS) { + schedule_dao_retransmission(); + } else { + /* No more retransmissions. Perform local repair. */ + rpl_local_repair("DAO max rtx"); + return; + } +} #endif /* RPL_WITH_DAO_ACK */ /*---------------------------------------------------------------------------*/ /*------------------------------- Probing----------------------------------- */ diff --git a/os/net/routing/rpl-lite/rpl-timers.h b/os/net/routing/rpl-lite/rpl-timers.h index e153858fc..5d9498b97 100644 --- a/os/net/routing/rpl-lite/rpl-timers.h +++ b/os/net/routing/rpl-lite/rpl-timers.h @@ -99,6 +99,11 @@ void rpl_timers_schedule_dao(void); */ void rpl_timers_schedule_dao_ack(uip_ipaddr_t *target, uint16_t sequence); +/** + * Let the rpl-timers module know that the last DAO was ACKed +*/ +void rpl_timers_notify_dao_ack(void); + /** * Schedule probing with delay RPL_PROBING_DELAY_FUNC() */ diff --git a/os/net/routing/rpl-lite/rpl-types.h b/os/net/routing/rpl-lite/rpl-types.h index 16dfa057e..2c57bf664 100644 --- a/os/net/routing/rpl-lite/rpl-types.h +++ b/os/net/routing/rpl-lite/rpl-types.h @@ -198,7 +198,6 @@ struct rpl_dag { uint8_t dio_counter; /* internal trickle timer state: redundancy counter */ uint8_t dao_last_seqno; /* the node's last sent DAO seqno */ uint8_t dao_last_acked_seqno; /* the last seqno we got an ACK for */ - uint8_t dao_curr_seqno; /* the node's current DAO seqno (sent or to be sent) */ uint8_t dao_transmissions; /* the number of transmissions for the current DAO */ enum rpl_dag_state state; From b02367a4ed8b76e423a6a38effdefa682cc6ec6a Mon Sep 17 00:00:00 2001 From: Simon Duquennoy Date: Mon, 18 Feb 2019 15:04:14 +0100 Subject: [PATCH 2/4] Fix typos in comments --- os/net/netstack.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/os/net/netstack.h b/os/net/netstack.h index 8528e3fff..941bbfeb2 100644 --- a/os/net/netstack.h +++ b/os/net/netstack.h @@ -44,7 +44,7 @@ #include "contiki.h" /* Routing protocol configuration. The Routing protocol is configured through the Makefile, - via the flag MAC_ROUTING */ + via the flag MAKE_ROUTING */ #ifdef NETSTACK_CONF_ROUTING #define NETSTACK_ROUTING NETSTACK_CONF_ROUTING #else /* NETSTACK_CONF_ROUTING */ @@ -60,7 +60,7 @@ #endif /* NETSTACK_CONF_ROUTING */ /* Network layer configuration. The NET layer is configured through the Makefile, - via the flag MAC_NET */ + via the flag MAKE_NET */ #ifdef NETSTACK_CONF_NETWORK #define NETSTACK_NETWORK NETSTACK_CONF_NETWORK #else /* NETSTACK_CONF_NETWORK */ From 4a76191626cee709ac081fe6c4af89ad71ca93d5 Mon Sep 17 00:00:00 2001 From: Simon Duquennoy Date: Mon, 18 Feb 2019 15:21:59 +0100 Subject: [PATCH 3/4] Check that table is non-NULL in nbr_table_add_lladdr --- os/net/nbr-table.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/os/net/nbr-table.c b/os/net/nbr-table.c index 8a8eaeb78..588315e58 100644 --- a/os/net/nbr-table.c +++ b/os/net/nbr-table.c @@ -354,6 +354,10 @@ nbr_table_add_lladdr(nbr_table_t *table, const linkaddr_t *lladdr, nbr_table_rea nbr_table_item_t *item; nbr_table_key_t *key; + if(table == NULL) { + return NULL; + } + /* Allow lladdr-free insertion, useful e.g. for IPv6 ND. * Only one such entry is possible at a time, indexed by linkaddr_null. */ if(lladdr == NULL) { From 2879492799a22b843b2b655ddc5f2aab84d738b7 Mon Sep 17 00:00:00 2001 From: firmwareguru Date: Fri, 22 Feb 2019 07:46:48 -0700 Subject: [PATCH 4/4] fix mqtt string lengths greater than 255 and incorrect example keep-alive --- examples/mqtt-client/mqtt-client.c | 2 +- os/net/app-layer/mqtt/mqtt.c | 18 +++++++++--------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/examples/mqtt-client/mqtt-client.c b/examples/mqtt-client/mqtt-client.c index b71a193ea..35e669e24 100644 --- a/examples/mqtt-client/mqtt-client.c +++ b/examples/mqtt-client/mqtt-client.c @@ -572,7 +572,7 @@ connect_to_broker(void) { /* Connect to MQTT server */ mqtt_connect(&conn, conf.broker_ip, conf.broker_port, - conf.pub_interval * 3); + (conf.pub_interval * 3) / CLOCK_SECOND); state = STATE_CONNECTING; } diff --git a/os/net/app-layer/mqtt/mqtt.c b/os/net/app-layer/mqtt/mqtt.c index 9571244d8..47ffc6a7a 100644 --- a/os/net/app-layer/mqtt/mqtt.c +++ b/os/net/app-layer/mqtt/mqtt.c @@ -416,16 +416,16 @@ PT_THREAD(connect_pt(struct pt *pt, struct mqtt_connection *conn)) PT_MQTT_WRITE_BYTE(conn, conn->connect_vhdr_flags); PT_MQTT_WRITE_BYTE(conn, (conn->keep_alive >> 8)); PT_MQTT_WRITE_BYTE(conn, (conn->keep_alive & 0x00FF)); - PT_MQTT_WRITE_BYTE(conn, conn->client_id.length << 8); + PT_MQTT_WRITE_BYTE(conn, conn->client_id.length >> 8); PT_MQTT_WRITE_BYTE(conn, conn->client_id.length & 0x00FF); PT_MQTT_WRITE_BYTES(conn, (uint8_t *)conn->client_id.string, conn->client_id.length); if(conn->connect_vhdr_flags & MQTT_VHDR_WILL_FLAG) { - PT_MQTT_WRITE_BYTE(conn, conn->will.topic.length << 8); + PT_MQTT_WRITE_BYTE(conn, conn->will.topic.length >> 8); PT_MQTT_WRITE_BYTE(conn, conn->will.topic.length & 0x00FF); PT_MQTT_WRITE_BYTES(conn, (uint8_t *)conn->will.topic.string, conn->will.topic.length); - PT_MQTT_WRITE_BYTE(conn, conn->will.message.length << 8); + PT_MQTT_WRITE_BYTE(conn, conn->will.message.length >> 8); PT_MQTT_WRITE_BYTE(conn, conn->will.message.length & 0x00FF); PT_MQTT_WRITE_BYTES(conn, (uint8_t *)conn->will.message.string, conn->will.message.length); @@ -436,14 +436,14 @@ PT_THREAD(connect_pt(struct pt *pt, struct mqtt_connection *conn)) conn->will.message.length); } if(conn->connect_vhdr_flags & MQTT_VHDR_USERNAME_FLAG) { - PT_MQTT_WRITE_BYTE(conn, conn->credentials.username.length << 8); + PT_MQTT_WRITE_BYTE(conn, conn->credentials.username.length >> 8); PT_MQTT_WRITE_BYTE(conn, conn->credentials.username.length & 0x00FF); PT_MQTT_WRITE_BYTES(conn, (uint8_t *)conn->credentials.username.string, conn->credentials.username.length); } if(conn->connect_vhdr_flags & MQTT_VHDR_PASSWORD_FLAG) { - PT_MQTT_WRITE_BYTE(conn, conn->credentials.password.length << 8); + PT_MQTT_WRITE_BYTE(conn, conn->credentials.password.length >> 8); PT_MQTT_WRITE_BYTE(conn, conn->credentials.password.length & 0x00FF); PT_MQTT_WRITE_BYTES(conn, (uint8_t *)conn->credentials.password.string, @@ -534,7 +534,7 @@ PT_THREAD(subscribe_pt(struct pt *pt, struct mqtt_connection *conn)) conn->out_packet.remaining_length_enc, conn->out_packet.remaining_length_enc_bytes); /* Write Variable Header */ - PT_MQTT_WRITE_BYTE(conn, (conn->out_packet.mid << 8)); + PT_MQTT_WRITE_BYTE(conn, (conn->out_packet.mid >> 8)); PT_MQTT_WRITE_BYTE(conn, (conn->out_packet.mid & 0x00FF)); /* Write Payload */ PT_MQTT_WRITE_BYTE(conn, (conn->out_packet.topic_length >> 8)); @@ -596,7 +596,7 @@ PT_THREAD(unsubscribe_pt(struct pt *pt, struct mqtt_connection *conn)) PT_MQTT_WRITE_BYTES(conn, (uint8_t *)conn->out_packet.remaining_length_enc, conn->out_packet.remaining_length_enc_bytes); /* Write Variable Header */ - PT_MQTT_WRITE_BYTE(conn, (conn->out_packet.mid << 8)); + PT_MQTT_WRITE_BYTE(conn, (conn->out_packet.mid >> 8)); PT_MQTT_WRITE_BYTE(conn, (conn->out_packet.mid & 0x00FF)); /* Write Payload */ PT_MQTT_WRITE_BYTE(conn, (conn->out_packet.topic_length >> 8)); @@ -669,7 +669,7 @@ PT_THREAD(publish_pt(struct pt *pt, struct mqtt_connection *conn)) PT_MQTT_WRITE_BYTES(conn, (uint8_t *)conn->out_packet.topic, conn->out_packet.topic_length); if(conn->out_packet.qos > MQTT_QOS_LEVEL_0) { - PT_MQTT_WRITE_BYTE(conn, (conn->out_packet.mid << 8)); + PT_MQTT_WRITE_BYTE(conn, (conn->out_packet.mid >> 8)); PT_MQTT_WRITE_BYTE(conn, (conn->out_packet.mid & 0x00FF)); } /* Write Payload */ @@ -772,7 +772,7 @@ handle_connack(struct mqtt_connection *conn) static void handle_pingresp(struct mqtt_connection *conn) { - DBG("MQTT - Got RINGRESP\n"); + DBG("MQTT - Got PINGRESP\n"); } /*---------------------------------------------------------------------------*/ static void