From eae25d622d3cdcde5770c8ceeb9257985999cf0b Mon Sep 17 00:00:00 2001 From: Timofei Istomin Date: Fri, 8 May 2015 18:31:06 +0200 Subject: [PATCH 1/3] Check the frame preamble and MPDU length before parsing Due to errors in mspsim and/or radio drivers, packets of incorrect length are sometimes transmitted. The length might be larger than the 127-byte maximum (considered negative in the current code) or not matching the actual number of transmitted bytes. This leads to wrong packet delimiting when converting from the mspsim-level stream of bytes to Cooja-level packets causing unhandled exceptions that terminate the simulation. This patch checks the frame preamble (0000007A) and the length field. If they are wrong, no decoding attempt is done. The transmitted bytes are still delivered to the receivers untouched. The connection is terminated when the radio state is changed (which alway s happens when TX is done). --- .../CC2420RadioPacketConverter.java | 2 +- .../mspmote/interfaces/Msp802154Radio.java | 86 +++++++++---------- 2 files changed, 43 insertions(+), 45 deletions(-) diff --git a/tools/cooja/apps/mspsim/src/org/contikios/cooja/mspmote/interfaces/CC2420RadioPacketConverter.java b/tools/cooja/apps/mspsim/src/org/contikios/cooja/mspmote/interfaces/CC2420RadioPacketConverter.java index fd62da50f..3fda7681d 100644 --- a/tools/cooja/apps/mspsim/src/org/contikios/cooja/mspmote/interfaces/CC2420RadioPacketConverter.java +++ b/tools/cooja/apps/mspsim/src/org/contikios/cooja/mspmote/interfaces/CC2420RadioPacketConverter.java @@ -165,7 +165,7 @@ public class CC2420RadioPacketConverter { } /* 1 byte length */ - len = data[pos]; + len = data[pos] & 0xFF; originalLen = len; pos += 1; diff --git a/tools/cooja/apps/mspsim/src/org/contikios/cooja/mspmote/interfaces/Msp802154Radio.java b/tools/cooja/apps/mspsim/src/org/contikios/cooja/mspmote/interfaces/Msp802154Radio.java index 18dcffb37..c99710f3d 100644 --- a/tools/cooja/apps/mspsim/src/org/contikios/cooja/mspmote/interfaces/Msp802154Radio.java +++ b/tools/cooja/apps/mspsim/src/org/contikios/cooja/mspmote/interfaces/Msp802154Radio.java @@ -75,6 +75,7 @@ public class Msp802154Radio extends Radio implements CustomDataRadio { private boolean isInterfered = false; private boolean isTransmitting = false; private boolean isReceiving = false; + private boolean isSynchronized = false; private byte lastOutgoingByte; private byte lastIncomingByte; @@ -91,22 +92,19 @@ public class Msp802154Radio extends Radio implements CustomDataRadio { radio.addRFListener(new RFListener() { int len = 0; - int expLen = 0; - byte[] buffer = new byte[127 + 15]; + int expMpduLen = 0; + byte[] buffer = new byte[127 + 6]; + final private byte[] syncSeq = {0,0,0,0,0x7A}; + public void receivedByte(byte data) { if (!isTransmitting()) { lastEvent = RadioEvent.TRANSMISSION_STARTED; isTransmitting = true; len = 0; - /*logger.debug("----- 802.15.4 TRANSMISSION STARTED -----");*/ + expMpduLen = 0; setChanged(); notifyObservers(); - } - - if (len >= buffer.length) { - /* Bad size packet, too large */ - logger.debug("Error: bad size: " + len + ", dropping outgoing byte: " + data); - return; + /*logger.debug("----- 802.15.4 TRANSMISSION STARTED -----");*/ } /* send this byte to all nodes */ @@ -115,31 +113,40 @@ public class Msp802154Radio extends Radio implements CustomDataRadio { setChanged(); notifyObservers(); - buffer[len++] = data; + if (len < buffer.length) + buffer[len] = data; - if (len == 6) { + len ++; + + if (len == 5) { + isSynchronized = true; + for (int i=0; i<5; i++) { + if (buffer[i] != syncSeq[i]) { + // this should never happen, but it happens + logger.error(String.format("Bad outgoing sync sequence %x %x %x %x %x", buffer[0], buffer[1], buffer[2], buffer[3], buffer[4])); + isSynchronized = false; + break; + } + } + } + else if (len == 6) { // System.out.println("## CC2420 Packet of length: " + data + " expected..."); - expLen = data + 6; + expMpduLen = data & 0xFF; + if ((expMpduLen & 0x80) != 0) { + logger.error("Outgoing length field is larger than 127: " + expMpduLen); + } } - if (len == expLen) { - /*logger.debug("----- 802.15.4 CUSTOM DATA TRANSMITTED -----");*/ - + if (((expMpduLen & 0x80) == 0) && len == expMpduLen + 6 && isSynchronized) { lastOutgoingPacket = CC2420RadioPacketConverter.fromCC2420ToCooja(buffer); lastEvent = RadioEvent.PACKET_TRANSMITTED; - /*logger.debug("----- 802.15.4 PACKET TRANSMITTED -----");*/ + //logger.debug("----- 802.15.4 PACKET TRANSMITTED -----"); setChanged(); notifyObservers(); - - /*logger.debug("----- 802.15.4 TRANSMISSION FINISHED -----");*/ - isTransmitting = false; - lastEvent = RadioEvent.TRANSMISSION_FINISHED; - setChanged(); - notifyObservers(); - len = 0; + isSynchronized = false; } } - }); + }); /* addRFListener */ radio.addOperatingModeListener(new OperatingModeListener() { public void modeChanged(Chip source, int mode) { @@ -148,7 +155,7 @@ public class Msp802154Radio extends Radio implements CustomDataRadio { setChanged(); notifyObservers(); } else { - radioOff(); + radioOff(); // actually it is a state change, not necessarily to OFF } } }); @@ -163,32 +170,23 @@ public class Msp802154Radio extends Radio implements CustomDataRadio { }); } - private void radioOff() { - /* Radio was turned off during transmission. - * May for example happen if watchdog triggers */ + + private void finishTransmission() + { if (isTransmitting()) { - logger.warn("Turning off radio while transmitting, ending packet prematurely"); - - /* Simulate end of packet */ - lastOutgoingPacket = new RadioPacket() { - public byte[] getPacketData() { - return new byte[0]; - } - }; - - lastEvent = RadioEvent.PACKET_TRANSMITTED; - /*logger.debug("----- 802.15.4 PACKET TRANSMITTED -----");*/ - setChanged(); - notifyObservers(); - - /* Register that transmission ended in radio medium */ - /*logger.debug("----- 802.15.4 TRANSMISSION FINISHED -----");*/ + //logger.debug("----- 802.15.4 TRANSMISSION FINISHED -----"); isTransmitting = false; + isSynchronized = false; lastEvent = RadioEvent.TRANSMISSION_FINISHED; setChanged(); notifyObservers(); } + } + private void radioOff() { + if (isSynchronized) + logger.warn("Turning off radio while transmitting a packet"); + finishTransmission(); lastEvent = RadioEvent.HW_OFF; setChanged(); notifyObservers(); From 034a97eed2e6f6521e3d82d667eba57d400e68b6 Mon Sep 17 00:00:00 2001 From: Timofei Istomin Date: Thu, 28 May 2015 21:51:02 +0200 Subject: [PATCH 2/3] Don't generate a zero-length packet in case of errors. The packet converter used to generate packets of length zero when it encountered errors during conversion. This caused exceptions in packet analyzers. Now the converter returns null in case of error. Appropriate checks have been added to the code that uses the return value. --- .../interfaces/CC2420RadioPacketConverter.java | 2 +- .../cooja/mspmote/interfaces/Msp802154Radio.java | 11 +++++++---- .../cooja/motes/AbstractApplicationMote.java | 3 ++- .../java/org/contikios/cooja/plugins/RadioLogger.java | 4 +++- 4 files changed, 13 insertions(+), 7 deletions(-) diff --git a/tools/cooja/apps/mspsim/src/org/contikios/cooja/mspmote/interfaces/CC2420RadioPacketConverter.java b/tools/cooja/apps/mspsim/src/org/contikios/cooja/mspmote/interfaces/CC2420RadioPacketConverter.java index 3fda7681d..946e2398c 100644 --- a/tools/cooja/apps/mspsim/src/org/contikios/cooja/mspmote/interfaces/CC2420RadioPacketConverter.java +++ b/tools/cooja/apps/mspsim/src/org/contikios/cooja/mspmote/interfaces/CC2420RadioPacketConverter.java @@ -198,7 +198,7 @@ public class CC2420RadioPacketConverter { System.arraycopy(data, 6 /* skipping preamble+synch+len */, originalData, 0, originalLen); if (len < 0) { /*logger.warn("No cross-level conversion available: negative packet length");*/ - return new ConvertedRadioPacket(new byte[0], originalData); + return null; } byte convertedData[] = new byte[len]; System.arraycopy(data, pos, convertedData, 0, len); diff --git a/tools/cooja/apps/mspsim/src/org/contikios/cooja/mspmote/interfaces/Msp802154Radio.java b/tools/cooja/apps/mspsim/src/org/contikios/cooja/mspmote/interfaces/Msp802154Radio.java index c99710f3d..9937e6a74 100644 --- a/tools/cooja/apps/mspsim/src/org/contikios/cooja/mspmote/interfaces/Msp802154Radio.java +++ b/tools/cooja/apps/mspsim/src/org/contikios/cooja/mspmote/interfaces/Msp802154Radio.java @@ -99,6 +99,7 @@ public class Msp802154Radio extends Radio implements CustomDataRadio { public void receivedByte(byte data) { if (!isTransmitting()) { lastEvent = RadioEvent.TRANSMISSION_STARTED; + lastOutgoingPacket = null; isTransmitting = true; len = 0; expMpduLen = 0; @@ -139,10 +140,12 @@ public class Msp802154Radio extends Radio implements CustomDataRadio { if (((expMpduLen & 0x80) == 0) && len == expMpduLen + 6 && isSynchronized) { lastOutgoingPacket = CC2420RadioPacketConverter.fromCC2420ToCooja(buffer); - lastEvent = RadioEvent.PACKET_TRANSMITTED; - //logger.debug("----- 802.15.4 PACKET TRANSMITTED -----"); - setChanged(); - notifyObservers(); + if (lastOutgoingPacket != null) { + lastEvent = RadioEvent.PACKET_TRANSMITTED; + //logger.debug("----- 802.15.4 PACKET TRANSMITTED -----"); + setChanged(); + notifyObservers(); + } isSynchronized = false; } } diff --git a/tools/cooja/java/org/contikios/cooja/motes/AbstractApplicationMote.java b/tools/cooja/java/org/contikios/cooja/motes/AbstractApplicationMote.java index 98de78327..fd16d37a3 100644 --- a/tools/cooja/java/org/contikios/cooja/motes/AbstractApplicationMote.java +++ b/tools/cooja/java/org/contikios/cooja/motes/AbstractApplicationMote.java @@ -76,7 +76,8 @@ public abstract class AbstractApplicationMote extends AbstractWakeupMote impleme if (radio.getLastPacketReceived() != null) receivedPacket(radio.getLastPacketReceived()); } else if (radio.getLastEvent() == Radio.RadioEvent.TRANSMISSION_FINISHED) { - sentPacket(radio.getLastPacketTransmitted()); + if (radio.getLastPacketTransmitted() != null) + sentPacket(radio.getLastPacketTransmitted()); } } }; diff --git a/tools/cooja/java/org/contikios/cooja/plugins/RadioLogger.java b/tools/cooja/java/org/contikios/cooja/plugins/RadioLogger.java index 17f5417c2..abdc76f6c 100644 --- a/tools/cooja/java/org/contikios/cooja/plugins/RadioLogger.java +++ b/tools/cooja/java/org/contikios/cooja/plugins/RadioLogger.java @@ -517,10 +517,12 @@ public class RadioLogger extends VisPlugin { return; } final RadioConnectionLog loggedConn = new RadioConnectionLog(); + loggedConn.packet = conn.getSource().getLastPacketTransmitted(); + if (loggedConn.packet == null) + return; loggedConn.startTime = conn.getStartTime(); loggedConn.endTime = simulation.getSimulationTime(); loggedConn.connection = conn; - loggedConn.packet = conn.getSource().getLastPacketTransmitted(); java.awt.EventQueue.invokeLater(new Runnable() { @Override public void run() { From eb4bdde9ca6ce7f2b20089d2cf097512f9986f78 Mon Sep 17 00:00:00 2001 From: Timofei Istomin Date: Fri, 5 Jun 2015 15:13:45 +0200 Subject: [PATCH 3/3] Finish the transmission at the end of a correct packet This patch restores the original behaviour of Cooja when the transmitted packet is correct (which is true in a vast majority of cases). In case of a wrong outgoing packet (wrong length, wrong preamble) the transmission will end when the radio changes its state (which should always happen after transmitting a packet). Benchmarks with RPL (33 runs, 50 nodes, 3 hours of simulated time each) yield the same results (PDR, delay, number of transmitted packets) as with the unmodified Cooja. --- .../org/contikios/cooja/mspmote/interfaces/Msp802154Radio.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/cooja/apps/mspsim/src/org/contikios/cooja/mspmote/interfaces/Msp802154Radio.java b/tools/cooja/apps/mspsim/src/org/contikios/cooja/mspmote/interfaces/Msp802154Radio.java index 9937e6a74..31aa30377 100644 --- a/tools/cooja/apps/mspsim/src/org/contikios/cooja/mspmote/interfaces/Msp802154Radio.java +++ b/tools/cooja/apps/mspsim/src/org/contikios/cooja/mspmote/interfaces/Msp802154Radio.java @@ -146,7 +146,7 @@ public class Msp802154Radio extends Radio implements CustomDataRadio { setChanged(); notifyObservers(); } - isSynchronized = false; + finishTransmission(); } } }); /* addRFListener */