From 08c1e69d7123e292e39177bb590987f038ed9b21 Mon Sep 17 00:00:00 2001 From: Sebastian Sumpf Date: Mon, 6 Dec 2021 09:01:57 +0100 Subject: [PATCH] nic/packet_allocator: align allocations to 2 bytes Override 'try_alloc/free' because ethernet frame headers are 14 bytes (src/dst mac (12) + ethertype (2)) causing the IP header to be 2 byte aligned, leading to problems on platforms that require load/store operations to be naturally aligned when reading, for example, 4 byte IP addresses. Therefore, we align the allocation to 2 bytes, so the IP header is aligned to 4. issue #4312 --- repos/os/include/nic/packet_allocator.h | 61 ++++++++++++++++++++- repos/os/src/drivers/nic/spec/linux/main.cc | 6 +- repos/os/src/lib/genode_c_api/uplink.cc | 2 +- 3 files changed, 64 insertions(+), 5 deletions(-) diff --git a/repos/os/include/nic/packet_allocator.h b/repos/os/include/nic/packet_allocator.h index e0086074c2..7cbb077678 100644 --- a/repos/os/include/nic/packet_allocator.h +++ b/repos/os/include/nic/packet_allocator.h @@ -17,16 +17,37 @@ #define _INCLUDE__NIC__PACKET_ALLOCATOR__ #include +#include namespace Nic { struct Packet_allocator; } /** * Packet allocator used for packet streaming in nic sessions. + * + * We override the allocator interface to align the IP packet to a 32-bit + * address. The ethernet frame header contains src/dst mac (12) + ethertype (2) + * causing the IP header to be at offset 14 in the packet. This leads to + * problems on platforms that require load/store operations to be naturally + * aligned when reading, for example, 4 byte IP addresses. Therefore, we + * allocate packet size plus OFFSET and offset the returned packet allocation + * at 2 bytes, which effectively aligns the IP header to 4 bytes. + * + * Note, this tweak reduces the usable bytes in the allocated packets to + * DEFAULT_PACKET_SIZE - OFFSET and assumes word-aligned allocations in the + * Genode::Packet_allocator. As DEFAULT_PACKET_SIZE is used for the + * transmission-buffer calculation we could not change it without breaking the + * API. OFFSET_PACKET_SIZE reflects the actual (usable) packet-buffer size. */ struct Nic::Packet_allocator : Genode::Packet_allocator { - enum { DEFAULT_PACKET_SIZE = 1600 }; + enum { + DEFAULT_PACKET_SIZE = 1600, + OFFSET = 2, + OFFSET_PACKET_SIZE = DEFAULT_PACKET_SIZE - OFFSET, + }; + + typedef Genode::size_t size_t; /** * Constructor @@ -35,6 +56,44 @@ struct Nic::Packet_allocator : Genode::Packet_allocator */ Packet_allocator(Genode::Allocator *md_alloc) : Genode::Packet_allocator(md_alloc, DEFAULT_PACKET_SIZE) {} + + Alloc_result try_alloc(size_t size) override + { + if (!size || size > OFFSET_PACKET_SIZE) { + Genode::error("unsupported NIC packet size ", size); + return Alloc_result { Alloc_error::DENIED }; + } + + Alloc_result result = Genode::Packet_allocator::try_alloc(size + OFFSET); + + result.with_result( + [&] (void *content) { + /* assume word-aligned packet buffer and offset packet by 2 bytes */ + if ((Genode::addr_t)content & 0b11) { + Genode::error("NIC packet allocation not word-aligned"); + result = { Alloc_error::DENIED }; + } else { + result = Alloc_result { + reinterpret_cast((Genode::uint8_t *)content + OFFSET) }; + } + }, + [] (Alloc_error) { } + ); + + return result; + } + + void free(void *addr, size_t size) override + { + if (!size || size > OFFSET_PACKET_SIZE) { + Genode::error("unsupported NIC packet size ", size); + return; + } + + Genode::Packet_allocator::free((Genode::uint8_t *)addr - OFFSET, size + OFFSET); + } + + }; #endif /* _INCLUDE__NIC__PACKET_ALLOCATOR__ */ diff --git a/repos/os/src/drivers/nic/spec/linux/main.cc b/repos/os/src/drivers/nic/spec/linux/main.cc index 65d52cb3c1..4b7d7296bf 100644 --- a/repos/os/src/drivers/nic/spec/linux/main.cc +++ b/repos/os/src/drivers/nic/spec/linux/main.cc @@ -122,15 +122,15 @@ class Uplink_client : public Uplink_client_base progress = false; size_t const max_pkt_size { - Nic::Packet_allocator::DEFAULT_PACKET_SIZE }; + Nic::Packet_allocator::OFFSET_PACKET_SIZE }; _drv_rx_handle_pkt( max_pkt_size, [&] (void *conn_tx_pkt_base, size_t &adjusted_conn_tx_pkt_size) { - long int const read_result { - read(_tap_fd, conn_tx_pkt_base, max_pkt_size) }; + ssize_t const read_result { + ::read(_tap_fd, conn_tx_pkt_base, max_pkt_size) }; if (read_result <= 0) { diff --git a/repos/os/src/lib/genode_c_api/uplink.cc b/repos/os/src/lib/genode_c_api/uplink.cc index 2973dff3f8..e1d3e431ff 100644 --- a/repos/os/src/lib/genode_c_api/uplink.cc +++ b/repos/os/src/lib/genode_c_api/uplink.cc @@ -98,7 +98,7 @@ struct genode_uplink : private Noncopyable, private Interface typedef Uplink::Packet_descriptor Packet_descriptor; Packet_descriptor packet { }; - size_t const max_bytes = Nic::Packet_allocator::DEFAULT_PACKET_SIZE; + size_t const max_bytes = Nic::Packet_allocator::OFFSET_PACKET_SIZE; try { packet = tx_source.alloc_packet(max_bytes); } catch (Uplink::Session::Tx::Source::Packet_alloc_failed) {