EthernetPacketizer: Coalesce small PC-to-device packets

CMakeLists.txt

@@ -299,22 +299,28 @@ if(LIBICSNEO_BUILD_TESTS)
 		set(gtest_force_shared_crt ON CACHE BOOL "" FORCE)
 	endif()
 
+	if (NOT TARGET gtest)
 		add_subdirectory(third-party/googletest-master)
+	endif()
 
 	if (CMAKE_VERSION VERSION_LESS 2.8.11)
 		include_directories("${gtest_SOURCE_DIR}/include")
 	endif()
 
-	add_executable(runTests test/main.cpp test/eventmanagertest.cpp)
+	add_executable(libicsneo-tests
+		test/main.cpp
+		test/eventmanagertest.cpp
+		test/ethernetpacketizertest.cpp
+	)
 
-	target_link_libraries(runTests gtest gtest_main)
+	target_link_libraries(libicsneo-tests gtest gtest_main)
-	target_link_libraries(runTests icsneocpp)
+	target_link_libraries(libicsneo-tests icsneocpp)
 
-	target_include_directories(runTests PUBLIC ${gtest_SOURCE_DIR}/include ${gtest_SOURCE_DIR})
+	target_include_directories(libicsneo-tests PUBLIC ${gtest_SOURCE_DIR}/include ${gtest_SOURCE_DIR})
 
 	enable_testing()
 
-	add_test(NAME testSuite COMMAND runTests)
+	add_test(NAME libicsneo-test-suite COMMAND libicsneo-tests)
 endif()
 
 set(CPACK_PROJECT_NAME ${PROJECT_NAME})

communication/ethernetpacketizer.cpp

@@ -2,41 +2,61 @@
 #include <algorithm>
 #include <iterator>
 #include <cstring>
+#include <cassert>
 
 using namespace icsneo;
 
-#define MAX_PACKET_LEN (1490) // MTU - overhead
+const size_t EthernetPacketizer::MaxPacketLength = 1490; // MTU - overhead
-
 static const uint8_t BROADCAST_MAC[6] = { 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF };
 
-void EthernetPacketizer::inputDown(std::vector<uint8_t> bytes) {
+EthernetPacketizer::EthernetPacket& EthernetPacketizer::newSendPacket(bool first) {
-	EthernetPacket sendPacket;
+	processedDownPackets.emplace_back();
-	std::copy(std::begin(hostMAC), std::end(hostMAC), std::begin(sendPacket.srcMAC));
+	EthernetPacket& ret = processedDownPackets.back();
-	std::copy(std::begin(deviceMAC), std::end(deviceMAC), std::begin(sendPacket.destMAC));
+	if(first) {
+		ret.packetNumber = sequenceDown++;
+	} else {
+		ret.firstPiece = false;
+		if(processedDownPackets.size() > 1)
+			ret.packetNumber = (processedDownPackets.rbegin() + 1)->packetNumber;
+		else
+			assert(false); // This should never be called with !first if there are no packets in the queue
+	}
+	std::copy(std::begin(hostMAC), std::end(hostMAC), std::begin(ret.srcMAC));
+	std::copy(std::begin(deviceMAC), std::end(deviceMAC), std::begin(ret.destMAC));
+	return ret;
+}
 
-	sendPacket.packetNumber = sequenceDown++;
+void EthernetPacketizer::inputDown(std::vector<uint8_t> bytes, bool first) {
-	sendPacket.payload = std::move(bytes);
+	EthernetPacket* sendPacket = nullptr;
+	if(first && !processedDownPackets.empty()) {
+		// We have some packets already, let's see if we can add this to the last one
+		if(processedDownPackets.back().payload.size() + bytes.size() <= MaxPacketLength)
+			sendPacket = &processedDownPackets.back();
+	}
+
+	if(sendPacket == nullptr)
+		sendPacket = &newSendPacket(first);
+
+	if(sendPacket->payload.empty())
+		sendPacket->payload = std::move(bytes);
+	else
+		sendPacket->payload.insert(sendPacket->payload.end(), bytes.begin(), bytes.end());
 
 	// Split packets larger than MTU
 	std::vector<uint8_t> extraData;
-	if(sendPacket.payload.size() > MAX_PACKET_LEN) {
+	if(sendPacket->payload.size() > MaxPacketLength) {
-		extraData.insert(extraData.end(), sendPacket.payload.begin() + MAX_PACKET_LEN, sendPacket.payload.end());
+		extraData.insert(extraData.end(), sendPacket->payload.begin() + MaxPacketLength, sendPacket->payload.end());
-		sendPacket.payload.resize(MAX_PACKET_LEN);
+		sendPacket->payload.resize(MaxPacketLength);
-		sendPacket.lastPiece = false;
+		sendPacket->lastPiece = false;
-	}
+		inputDown(std::move(extraData), false);
-
-	processedDownPackets.push_back(sendPacket.getBytestream());
-
-	if(!extraData.empty()) {
-		sendPacket.payload = std::move(extraData);
-		sendPacket.firstPiece = false;
-		sendPacket.lastPiece = true;
-		processedDownPackets.push_back(sendPacket.getBytestream());
 	}
 }
 
 std::vector< std::vector<uint8_t> > EthernetPacketizer::outputDown() {
-	std::vector< std::vector<uint8_t> > ret = std::move(processedDownPackets);
+	std::vector< std::vector<uint8_t> > ret;
+	ret.reserve(processedDownPackets.size());
+	for(auto&& packet : std::move(processedDownPackets))
+		ret.push_back(packet.getBytestream());
 	processedDownPackets.clear();
 	return ret;
 }

include/icsneo/communication/ethernetpacketizer.h

@@ -18,6 +18,8 @@ namespace icsneo {
  */
 class EthernetPacketizer {
 public:
+	static const size_t MaxPacketLength;
+
 	EthernetPacketizer(device_eventhandler_t report) : report(report) {}
 
 	/**
@@ -25,7 +27,7 @@ public:
 	 * outputDown to get the results. Passing in multiple
 	 * packets may result in better packing.
 	 */
-	void inputDown(std::vector<uint8_t> bytes);
+	void inputDown(std::vector<uint8_t> bytes, bool first = true);
 	std::vector< std::vector<uint8_t> > outputDown();
 
 	/**
@@ -66,9 +68,11 @@ private:
 	uint16_t sequenceDown = 0;
 
 	std::vector<uint8_t> processedUpBytes;
-	std::vector< std::vector<uint8_t> > processedDownPackets;
+	std::vector<EthernetPacket> processedDownPackets;
 
 	device_eventhandler_t report;
+
+	EthernetPacket& newSendPacket(bool first);
 };
 
 }

test/ethernetpacketizertest.cpp

@@ -0,0 +1,253 @@
+#include "icsneo/communication/ethernetpacketizer.h"
+#include "icsneo/platform/optional.h"
+#include "gtest/gtest.h"
+
+using namespace icsneo;
+
+#define MAC_SIZE (6)
+static const uint8_t correctDeviceMAC[MAC_SIZE] = {0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff};
+static const uint8_t correctHostMAC[MAC_SIZE] = {0x12, 0x23, 0x34, 0x45, 0x56, 0x67};
+
+class EthernetPacketizerTest : public ::testing::Test {
+protected:
+	// Start with a clean instance of eventmanager for every test
+	void SetUp() override {
+		onError = [](APIEvent::Type, APIEvent::Severity) {
+			// Unless caught by the test, the packetizer should not throw errors
+			EXPECT_TRUE(false);
+		};
+		packetizer.emplace([this](APIEvent::Type t, APIEvent::Severity s) {
+			onError(t, s);
+		});
+		memcpy(packetizer->deviceMAC, correctDeviceMAC, MAC_SIZE);
+		memcpy(packetizer->hostMAC, correctHostMAC, MAC_SIZE);
+	}
+
+	void TearDown() override {
+		packetizer.reset();
+	}
+
+	optional<EthernetPacketizer> packetizer;
+	device_eventhandler_t onError;
+};
+
+TEST_F(EthernetPacketizerTest, DownSmallSinglePacket)
+{
+	packetizer->inputDown({ 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, 0x99 });
+	const auto output = packetizer->outputDown();
+	ASSERT_EQ(output.size(), 1);
+	EXPECT_EQ(output.front(), std::vector<uint8_t>({
+		0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff,
+		0x12, 0x23, 0x34, 0x45, 0x56, 0x67,
+		0xca, 0xb1,
+		0xaa, 0xaa, 0x55, 0x55,
+		0x09, 0x00, // 9 bytes
+		0x00, 0x00, // packet number
+		0x03, 0x01, // first and last piece, version 1
+		0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, 0x99
+	}));
+}
+
+TEST_F(EthernetPacketizerTest, DownSmallMultiplePackets)
+{
+	packetizer->inputDown({ 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, 0x99 });
+	packetizer->inputDown({ 0xc0, 0xff, 0xee, 0xc0, 0xff, 0xee, 0xc0, 0xff, 0xee });
+	const auto output = packetizer->outputDown();
+	ASSERT_EQ(output.size(), 1);
+	EXPECT_EQ(output.front(), std::vector<uint8_t>({
+		0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff,
+		0x12, 0x23, 0x34, 0x45, 0x56, 0x67,
+		0xca, 0xb1,
+		0xaa, 0xaa, 0x55, 0x55,
+		0x12, 0x00, // 18 bytes
+		0x00, 0x00, // packet number
+		0x03, 0x01, // first and last piece, version 1
+		0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, 0x99,
+		0xc0, 0xff, 0xee, 0xc0, 0xff, 0xee, 0xc0, 0xff, 0xee
+	}));
+}
+
+TEST_F(EthernetPacketizerTest, DownSmallMultiplePacketsOverflow)
+{
+	packetizer->inputDown({ 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, 0x99 });
+	packetizer->inputDown({ 0xc0, 0xff, 0xee, 0xc0, 0xff, 0xee, 0xc0, 0xff, 0xee });
+	packetizer->inputDown(std::vector<uint8_t>(1480)); // Near the max
+	const auto output = packetizer->outputDown();
+	ASSERT_EQ(output.size(), 2);
+	EXPECT_EQ(output.front(), std::vector<uint8_t>({
+		0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff,
+		0x12, 0x23, 0x34, 0x45, 0x56, 0x67,
+		0xca, 0xb1,
+		0xaa, 0xaa, 0x55, 0x55,
+		0x12, 0x00, // 18 bytes
+		0x00, 0x00, // packet number
+		0x03, 0x01, // first and last piece, version 1
+		0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, 0x99,
+		0xc0, 0xff, 0xee, 0xc0, 0xff, 0xee, 0xc0, 0xff, 0xee
+	}));
+	std::vector<uint8_t> bigOutput({
+		0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff,
+		0x12, 0x23, 0x34, 0x45, 0x56, 0x67,
+		0xca, 0xb1,
+		0xaa, 0xaa, 0x55, 0x55,
+		0xc8, 0x05, // 1480 bytes
+		0x01, 0x00, // packet number
+		0x03, 0x01, // first and last piece, version 1
+	});
+	bigOutput.resize(1480 + 24);
+	EXPECT_EQ(output.back().size(), bigOutput.size());
+	EXPECT_EQ(output.back(), bigOutput);
+}
+
+TEST_F(EthernetPacketizerTest, DownOverflowSmallMultiplePackets)
+{
+	packetizer->inputDown(std::vector<uint8_t>(1486)); // Near the max, not enough room for the next packet
+	packetizer->inputDown({ 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, 0x99 });
+	packetizer->inputDown({ 0xc0, 0xff, 0xee, 0xc0, 0xff, 0xee, 0xc0, 0xff, 0xee });
+	const auto output = packetizer->outputDown();
+	ASSERT_EQ(output.size(), 2);
+	std::vector<uint8_t> bigOutput({
+		0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff,
+		0x12, 0x23, 0x34, 0x45, 0x56, 0x67,
+		0xca, 0xb1,
+		0xaa, 0xaa, 0x55, 0x55,
+		0xce, 0x05, // 1486 bytes
+		0x00, 0x00, // packet number
+		0x03, 0x01, // first and last piece, version 1
+	});
+	bigOutput.resize(1486 + 24);
+	EXPECT_EQ(output.front().size(), bigOutput.size());
+	EXPECT_EQ(output.front(), bigOutput);
+	EXPECT_EQ(output.back(), std::vector<uint8_t>({
+		0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff,
+		0x12, 0x23, 0x34, 0x45, 0x56, 0x67,
+		0xca, 0xb1,
+		0xaa, 0xaa, 0x55, 0x55,
+		0x12, 0x00, // 18 bytes
+		0x01, 0x00, // packet number
+		0x03, 0x01, // first and last piece, version 1
+		0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, 0x99,
+		0xc0, 0xff, 0xee, 0xc0, 0xff, 0xee, 0xc0, 0xff, 0xee
+	}));
+}
+
+TEST_F(EthernetPacketizerTest, DownBigSmallSmall)
+{
+	packetizer->inputDown(std::vector<uint8_t>(1480)); // Near the max, enough room for the next packet
+	packetizer->inputDown({ 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, 0x99 });
+	packetizer->inputDown({ 0xc0, 0xff, 0xee, 0xc0, 0xff, 0xee, 0xc0, 0xff, 0xee }); // Not enough room for this one
+	const auto output = packetizer->outputDown();
+	ASSERT_EQ(output.size(), 2);
+	std::vector<uint8_t> bigOutput({
+		0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff,
+		0x12, 0x23, 0x34, 0x45, 0x56, 0x67,
+		0xca, 0xb1,
+		0xaa, 0xaa, 0x55, 0x55,
+		0xd1, 0x05, // 1486 bytes
+		0x00, 0x00, // packet number
+		0x03, 0x01, // first and last piece, version 1
+	});
+	bigOutput.resize(1480 + 24);
+	bigOutput.insert(bigOutput.end(), { 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, 0x99 });
+	EXPECT_EQ(output.front().size(), bigOutput.size());
+	EXPECT_EQ(output.front(), bigOutput);
+	EXPECT_EQ(output.back(), std::vector<uint8_t>({
+		0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff,
+		0x12, 0x23, 0x34, 0x45, 0x56, 0x67,
+		0xca, 0xb1,
+		0xaa, 0xaa, 0x55, 0x55,
+		0x9, 0x00, // 9 bytes
+		0x01, 0x00, // packet number
+		0x03, 0x01, // first and last piece, version 1
+		0xc0, 0xff, 0xee, 0xc0, 0xff, 0xee, 0xc0, 0xff, 0xee
+	}));
+}
+
+TEST_F(EthernetPacketizerTest, DownJumboSmallSmall)
+{
+	packetizer->inputDown(std::vector<uint8_t>(3000)); // Two full packets plus 20 bytes
+	packetizer->inputDown({ 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, 0x99 });
+	packetizer->inputDown({ 0xc0, 0xff, 0xee, 0xc0, 0xff, 0xee, 0xc0, 0xff, 0xee });
+	const auto output = packetizer->outputDown();
+	ASSERT_EQ(output.size(), 3);
+	std::vector<uint8_t> bigOutput({
+		0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff,
+		0x12, 0x23, 0x34, 0x45, 0x56, 0x67,
+		0xca, 0xb1,
+		0xaa, 0xaa, 0x55, 0x55,
+		0xd2, 0x05, // 1490 bytes
+		0x00, 0x00, // packet number
+		0x01, 0x01, // first piece, version 1
+	});
+	bigOutput.resize(1490 + 24); // Full packet
+	EXPECT_EQ(output.front(), bigOutput);
+	std::vector<uint8_t> bigOutput2({
+		0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff,
+		0x12, 0x23, 0x34, 0x45, 0x56, 0x67,
+		0xca, 0xb1,
+		0xaa, 0xaa, 0x55, 0x55,
+		0xd2, 0x05, // 1490 bytes
+		0x00, 0x00, // packet number
+		0x00, 0x01, // mid piece, version 1
+	});
+	bigOutput2.resize(1490 + 24); // Full packet
+	EXPECT_EQ(output[1], bigOutput2);
+	EXPECT_EQ(output.back(), std::vector<uint8_t>({
+		0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff,
+		0x12, 0x23, 0x34, 0x45, 0x56, 0x67,
+		0xca, 0xb1,
+		0xaa, 0xaa, 0x55, 0x55,
+		0x26, 0x00, // 38 bytes
+		0x00, 0x00, // packet number
+		0x02, 0x01, // last piece, version 1
+		0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+		0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+		0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, 0x99,
+		0xc0, 0xff, 0xee, 0xc0, 0xff, 0xee, 0xc0, 0xff, 0xee
+	}));
+}
+
+TEST_F(EthernetPacketizerTest, PacketNumberIncrement)
+{
+	packetizer->inputDown({ 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, 0x99 });
+	auto output = packetizer->outputDown();
+	ASSERT_EQ(output.size(), 1);
+	EXPECT_EQ(output.front(), std::vector<uint8_t>({
+		0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff,
+		0x12, 0x23, 0x34, 0x45, 0x56, 0x67,
+		0xca, 0xb1,
+		0xaa, 0xaa, 0x55, 0x55,
+		0x09, 0x00, // 9 bytes
+		0x00, 0x00, // packet number
+		0x03, 0x01, // first and last piece, version 1
+		0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, 0x99
+	}));
+
+	packetizer->inputDown({ 0x12, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, 0x99 });
+	output = packetizer->outputDown();
+	ASSERT_EQ(output.size(), 1);
+	EXPECT_EQ(output.front(), std::vector<uint8_t>({
+		0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff,
+		0x12, 0x23, 0x34, 0x45, 0x56, 0x67,
+		0xca, 0xb1,
+		0xaa, 0xaa, 0x55, 0x55,
+		0x09, 0x00, // 9 bytes
+		0x01, 0x00, // packet number
+		0x03, 0x01, // first and last piece, version 1
+		0x12, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, 0x99
+	}));
+
+	packetizer->inputDown({ 0x13, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, 0x99 });
+	output = packetizer->outputDown();
+	ASSERT_EQ(output.size(), 1);
+	EXPECT_EQ(output.front(), std::vector<uint8_t>({
+		0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff,
+		0x12, 0x23, 0x34, 0x45, 0x56, 0x67,
+		0xca, 0xb1,
+		0xaa, 0xaa, 0x55, 0x55,
+		0x09, 0x00, // 9 bytes
+		0x02, 0x00, // packet number
+		0x03, 0x01, // first and last piece, version 1
+		0x13, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, 0x99
+	}));
+}