From 0656cb568efda84e0562cdd06aaa65d70d16865a Mon Sep 17 00:00:00 2001 From: Paul Hollinsky Date: Thu, 23 Sep 2021 21:29:15 -0400 Subject: [PATCH] EthernetPacketizer: Coalesce small PC-to-device packets --- CMakeLists.txt | 32 ++- communication/ethernetpacketizer.cpp | 64 +++-- .../icsneo/communication/ethernetpacketizer.h | 8 +- test/ethernetpacketizertest.cpp | 253 ++++++++++++++++++ 4 files changed, 320 insertions(+), 37 deletions(-) create mode 100644 test/ethernetpacketizertest.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index b238e2f..7af251e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -297,24 +297,30 @@ endif() if(LIBICSNEO_BUILD_TESTS) if(WIN32) set(gtest_force_shared_crt ON CACHE BOOL "" FORCE) - endif() + endif() + + if (NOT TARGET gtest) + add_subdirectory(third-party/googletest-master) + endif() - add_subdirectory(third-party/googletest-master) - if (CMAKE_VERSION VERSION_LESS 2.8.11) include_directories("${gtest_SOURCE_DIR}/include") endif() - - add_executable(runTests test/main.cpp test/eventmanagertest.cpp) - - target_link_libraries(runTests gtest gtest_main) - target_link_libraries(runTests icsneocpp) - - target_include_directories(runTests PUBLIC ${gtest_SOURCE_DIR}/include ${gtest_SOURCE_DIR}) - + + add_executable(libicsneo-tests + test/main.cpp + test/eventmanagertest.cpp + test/ethernetpacketizertest.cpp + ) + + target_link_libraries(libicsneo-tests gtest gtest_main) + target_link_libraries(libicsneo-tests icsneocpp) + + 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}) diff --git a/communication/ethernetpacketizer.cpp b/communication/ethernetpacketizer.cpp index 5f06198..78578a0 100644 --- a/communication/ethernetpacketizer.cpp +++ b/communication/ethernetpacketizer.cpp @@ -2,41 +2,61 @@ #include #include #include +#include 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 bytes) { - EthernetPacket sendPacket; - std::copy(std::begin(hostMAC), std::end(hostMAC), std::begin(sendPacket.srcMAC)); - std::copy(std::begin(deviceMAC), std::end(deviceMAC), std::begin(sendPacket.destMAC)); +EthernetPacketizer::EthernetPacket& EthernetPacketizer::newSendPacket(bool first) { + processedDownPackets.emplace_back(); + EthernetPacket& ret = processedDownPackets.back(); + 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++; - sendPacket.payload = std::move(bytes); +void EthernetPacketizer::inputDown(std::vector bytes, bool first) { + 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 extraData; - if(sendPacket.payload.size() > MAX_PACKET_LEN) { - extraData.insert(extraData.end(), sendPacket.payload.begin() + MAX_PACKET_LEN, sendPacket.payload.end()); - sendPacket.payload.resize(MAX_PACKET_LEN); - sendPacket.lastPiece = 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()); + if(sendPacket->payload.size() > MaxPacketLength) { + extraData.insert(extraData.end(), sendPacket->payload.begin() + MaxPacketLength, sendPacket->payload.end()); + sendPacket->payload.resize(MaxPacketLength); + sendPacket->lastPiece = false; + inputDown(std::move(extraData), false); } } std::vector< std::vector > EthernetPacketizer::outputDown() { - std::vector< std::vector > ret = std::move(processedDownPackets); + std::vector< std::vector > ret; + ret.reserve(processedDownPackets.size()); + for(auto&& packet : std::move(processedDownPackets)) + ret.push_back(packet.getBytestream()); processedDownPackets.clear(); return ret; } diff --git a/include/icsneo/communication/ethernetpacketizer.h b/include/icsneo/communication/ethernetpacketizer.h index c14bcc3..bdc343d 100644 --- a/include/icsneo/communication/ethernetpacketizer.h +++ b/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 bytes); + void inputDown(std::vector bytes, bool first = true); std::vector< std::vector > outputDown(); /** @@ -66,9 +68,11 @@ private: uint16_t sequenceDown = 0; std::vector processedUpBytes; - std::vector< std::vector > processedDownPackets; + std::vector processedDownPackets; device_eventhandler_t report; + + EthernetPacket& newSendPacket(bool first); }; } diff --git a/test/ethernetpacketizertest.cpp b/test/ethernetpacketizertest.cpp new file mode 100644 index 0000000..a2b2853 --- /dev/null +++ b/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 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({ + 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({ + 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(1480)); // Near the max + const auto output = packetizer->outputDown(); + ASSERT_EQ(output.size(), 2); + EXPECT_EQ(output.front(), std::vector({ + 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 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(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 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({ + 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(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 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({ + 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(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 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 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({ + 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({ + 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({ + 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({ + 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 + })); +} \ No newline at end of file