From ac215ca601393dc111643b119835ea39e6c9bd23 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Sat, 14 Dec 2019 11:07:37 +0100 Subject: [PATCH] Net: Try to reuse incoming packet buffers to avoid allocation churn The majority of the time in NetworkTask was being spent in allocating and deallocating KBuffers for each incoming packet. We'll now keep up to 100 buffers around and reuse them for new packets if the next incoming packet fits in an old buffer. This is pretty naively implemented but definitely cuts down on time spent here. --- Kernel/Net/EthernetFrameHeader.h | 2 ++ Kernel/Net/NetworkAdapter.cpp | 35 ++++++++++++++++++++++++++----- Kernel/Net/NetworkAdapter.h | 4 +++- Kernel/Net/NetworkTask.cpp | 36 ++++++++++++++++---------------- 4 files changed, 53 insertions(+), 24 deletions(-) diff --git a/Kernel/Net/EthernetFrameHeader.h b/Kernel/Net/EthernetFrameHeader.h index c42aee8649..eab2c85ac9 100644 --- a/Kernel/Net/EthernetFrameHeader.h +++ b/Kernel/Net/EthernetFrameHeader.h @@ -3,6 +3,8 @@ #include #include +#pragma GCC diagnostic ignored "-Warray-bounds" + class [[gnu::packed]] EthernetFrameHeader { public: diff --git a/Kernel/Net/NetworkAdapter.cpp b/Kernel/Net/NetworkAdapter.cpp index f20a963966..5db9d5c286 100644 --- a/Kernel/Net/NetworkAdapter.cpp +++ b/Kernel/Net/NetworkAdapter.cpp @@ -1,11 +1,11 @@ #include #include +#include #include #include #include #include #include -#include static Lockable>& all_adapters() { @@ -103,17 +103,42 @@ void NetworkAdapter::did_receive(const u8* data, int length) InterruptDisabler disabler; m_packets_in++; m_bytes_in += length; - m_packet_queue.append(KBuffer::copy(data, length)); + + Optional buffer; + + if (m_unused_packet_buffers.is_empty()) { + buffer = KBuffer::copy(data, length); + } else { + buffer = m_unused_packet_buffers.take_first(); + --m_unused_packet_buffers_count; + if ((size_t)length <= buffer.value().size()) { + memcpy(buffer.value().data(), data, length); + buffer.value().set_size(length); + } else { + buffer = KBuffer::copy(data, length); + } + } + + m_packet_queue.append(buffer.value()); + if (on_receive) on_receive(); } -Optional NetworkAdapter::dequeue_packet() +size_t NetworkAdapter::dequeue_packet(u8* buffer, size_t buffer_size) { InterruptDisabler disabler; if (m_packet_queue.is_empty()) - return {}; - return m_packet_queue.take_first(); + return 0; + auto packet = m_packet_queue.take_first(); + size_t packet_size = packet.size(); + ASSERT(packet_size <= buffer_size); + memcpy(buffer, packet.data(), packet_size); + if (m_unused_packet_buffers_count < 100) { + m_unused_packet_buffers.append(packet); + ++m_unused_packet_buffers_count; + } + return packet_size; } void NetworkAdapter::set_ipv4_address(const IPv4Address& address) diff --git a/Kernel/Net/NetworkAdapter.h b/Kernel/Net/NetworkAdapter.h index b776121a20..e13fb07939 100644 --- a/Kernel/Net/NetworkAdapter.h +++ b/Kernel/Net/NetworkAdapter.h @@ -37,7 +37,7 @@ public: void send(const MACAddress&, const ARPPacket&); void send_ipv4(const MACAddress&, const IPv4Address&, IPv4Protocol, const u8* payload, size_t payload_size, u8 ttl); - Optional dequeue_packet(); + size_t dequeue_packet(u8* buffer, size_t buffer_size); bool has_queued_packets() const { return !m_packet_queue.is_empty(); } @@ -64,6 +64,8 @@ private: IPv4Address m_ipv4_netmask; IPv4Address m_ipv4_gateway; SinglyLinkedList m_packet_queue; + SinglyLinkedList m_unused_packet_buffers; + size_t m_unused_packet_buffers_count { 0 }; String m_name; u32 m_packets_in { 0 }; u32 m_bytes_in { 0 }; diff --git a/Kernel/Net/NetworkTask.cpp b/Kernel/Net/NetworkTask.cpp index 06881db392..3e6f44f231 100644 --- a/Kernel/Net/NetworkTask.cpp +++ b/Kernel/Net/NetworkTask.cpp @@ -57,47 +57,47 @@ void NetworkTask_main() }; }); - auto dequeue_packet = [&pending_packets]() -> Optional { + auto dequeue_packet = [&pending_packets](u8* buffer, size_t buffer_size) -> size_t { if (pending_packets == 0) - return {}; - Optional packet; - NetworkAdapter::for_each([&packet, &pending_packets](auto& adapter) { - if (packet.has_value() || !adapter.has_queued_packets()) + return 0; + size_t packet_size = 0; + NetworkAdapter::for_each([&](auto& adapter) { + if (packet_size || !adapter.has_queued_packets()) return; - packet = adapter.dequeue_packet(); + packet_size = adapter.dequeue_packet(buffer, buffer_size); pending_packets--; #ifdef NETWORK_TASK_DEBUG kprintf("NetworkTask: Dequeued packet from %s (%d bytes)\n", adapter.name().characters(), packet.value().size()); #endif }); - return packet; + return packet_size; }; kprintf("NetworkTask: Enter main loop.\n"); for (;;) { - auto packet_maybe_null = dequeue_packet(); - if (!packet_maybe_null.has_value()) { + u8 packet[64 * KB]; + size_t packet_size = dequeue_packet(packet, sizeof(packet)); + if (!packet_size) { current->wait_on(packet_wait_queue); continue; } - auto& packet = packet_maybe_null.value(); - if (packet.size() < sizeof(EthernetFrameHeader)) { - kprintf("NetworkTask: Packet is too small to be an Ethernet packet! (%zu)\n", packet.size()); + if (packet_size < sizeof(EthernetFrameHeader)) { + kprintf("NetworkTask: Packet is too small to be an Ethernet packet! (%zu)\n", packet_size); continue; } - auto& eth = *(const EthernetFrameHeader*)packet.data(); + auto& eth = *(const EthernetFrameHeader*)packet; #ifdef ETHERNET_DEBUG kprintf("NetworkTask: From %s to %s, ether_type=%w, packet_length=%u\n", eth.source().to_string().characters(), eth.destination().to_string().characters(), eth.ether_type(), - packet.size()); + packet_size); #endif #ifdef ETHERNET_VERY_DEBUG - u8* data = packet.data(); + u8* data = packet; - for (size_t i = 0; i < packet.size(); i++) { + for (size_t i = 0; i < packet_size; i++) { kprintf("%b", data[i]); switch (i % 16) { @@ -118,10 +118,10 @@ void NetworkTask_main() switch (eth.ether_type()) { case EtherType::ARP: - handle_arp(eth, packet.size()); + handle_arp(eth, packet_size); break; case EtherType::IPv4: - handle_ipv4(eth, packet.size()); + handle_ipv4(eth, packet_size); break; case EtherType::IPv6: // ignore