From 49946163461a0e2b1a15f660f998a9f225e57ce5 Mon Sep 17 00:00:00 2001 From: Sebastian Sumpf Date: Tue, 31 May 2016 18:53:57 +0200 Subject: [PATCH] os: packets without playload in packet stream issue #1988 --- repos/dde_ipxe/src/drivers/nic/main.cc | 2 +- .../src/lib/usb/include/usb_nic_component.h | 6 +++--- repos/dde_linux/src/lib/wifi/nic.cc | 2 +- repos/libports/src/lib/qemu-usb/host.cc | 11 +--------- repos/os/include/block/component.h | 2 +- repos/os/include/os/packet_stream.h | 21 ++++++++++++------- repos/os/include/usb/packet_handler.h | 10 --------- repos/os/src/drivers/ahci/ata_driver.h | 6 +++--- repos/os/src/drivers/ahci/atapi_driver.h | 4 ++-- repos/os/src/drivers/nic/gem/cadence_gem.h | 2 +- .../os/src/drivers/nic/spec/lan9118/lan9118.h | 2 +- repos/os/src/drivers/nic/spec/linux/main.cc | 2 +- repos/os/src/server/blk_cache/driver.h | 4 ++-- .../src/server/nic_bridge/packet_handler.cc | 2 +- repos/os/src/server/nic_loopback/main.cc | 4 ++-- repos/os/src/server/part_blk/component.h | 2 +- repos/ports/src/app/openvpn/main.cc | 2 +- 17 files changed, 35 insertions(+), 49 deletions(-) diff --git a/repos/dde_ipxe/src/drivers/nic/main.cc b/repos/dde_ipxe/src/drivers/nic/main.cc index 6ae85c3ad3..0b19b1d5bf 100644 --- a/repos/dde_ipxe/src/drivers/nic/main.cc +++ b/repos/dde_ipxe/src/drivers/nic/main.cc @@ -59,7 +59,7 @@ class Ipxe_session_component : public Nic::Session_component return false; Packet_descriptor packet = _tx.sink()->get_packet(); - if (!packet.valid()) { + if (!packet.size()) { PWRN("Invalid tx packet"); return true; } diff --git a/repos/dde_linux/src/lib/usb/include/usb_nic_component.h b/repos/dde_linux/src/lib/usb/include/usb_nic_component.h index fd1da5973d..c7a01eca6a 100644 --- a/repos/dde_linux/src/lib/usb/include/usb_nic_component.h +++ b/repos/dde_linux/src/lib/usb/include/usb_nic_component.h @@ -100,7 +100,7 @@ class Usb_nic::Session_component : public Nic::Session_component unsigned char *ptr = nullptr; /* submit received packets to lower layer */ - while (((_tx.sink()->packet_avail() || save.valid()) && _tx.sink()->ready_to_ack())) + while (((_tx.sink()->packet_avail() || save.size()) && _tx.sink()->ready_to_ack())) { /* alloc skb */ if (!skb) { @@ -111,7 +111,7 @@ class Usb_nic::Session_component : public Nic::Session_component work_skb.data = nullptr; } - Packet_descriptor packet = save.valid() ? save : _tx.sink()->get_packet(); + Packet_descriptor packet = save.size() ? save : _tx.sink()->get_packet(); save = Packet_descriptor(); if (!_device->skb_fill(&work_skb, ptr, packet.size(), skb->end)) { @@ -147,7 +147,7 @@ class Usb_nic::Session_component : public Nic::Session_component return false; Genode::Packet_descriptor packet = _tx.sink()->get_packet(); - if (!packet.valid()) { + if (!packet.size()) { PWRN("Invalid tx packet"); return true; } diff --git a/repos/dde_linux/src/lib/wifi/nic.cc b/repos/dde_linux/src/lib/wifi/nic.cc index 1489a266d2..c3ca820a39 100644 --- a/repos/dde_linux/src/lib/wifi/nic.cc +++ b/repos/dde_linux/src/lib/wifi/nic.cc @@ -62,7 +62,7 @@ class Wifi_session_component : public Nic::Session_component return false; Packet_descriptor packet = _tx.sink()->get_packet(); - if (!packet.valid()) { + if (!packet.size()) { PWRN("Invalid tx packet"); return true; } diff --git a/repos/libports/src/lib/qemu-usb/host.cc b/repos/libports/src/lib/qemu-usb/host.cc index 0083f9c5ca..6255b6b73f 100644 --- a/repos/libports/src/lib/qemu-usb/host.cc +++ b/repos/libports/src/lib/qemu-usb/host.cc @@ -228,19 +228,10 @@ struct Usb_host_device : List::Element Usb::Packet_descriptor alloc_packet(int length) { + if (!usb_raw.source()->ready_to_submit()) throw -1; - if (length <= 0) { - /* - * XXX Packets without payload are not supported by the packet stream - * currently. Therefore, we use a (small) bogus length - * here and depend on the USB driver to handle this case - * correctly. - */ - length = 4; - } - Usb::Packet_descriptor packet = usb_raw.source()->alloc_packet(length); packet.completion = alloc_completion(); return packet; diff --git a/repos/os/include/block/component.h b/repos/os/include/block/component.h index 9a06344510..0b7bf15b65 100644 --- a/repos/os/include/block/component.h +++ b/repos/os/include/block/component.h @@ -103,7 +103,7 @@ class Block::Session_component : public Block::Session_component_base, _p_to_handle.succeeded(false); /* ignore invalid packets */ - if (!packet.valid() || !_range_check(_p_to_handle)) { + if (!packet.size() || !_range_check(_p_to_handle)) { _ack_packet(_p_to_handle); return; } diff --git a/repos/os/include/os/packet_stream.h b/repos/os/include/os/packet_stream.h index ada7f9367b..43d9fcffd2 100644 --- a/repos/os/include/os/packet_stream.h +++ b/repos/os/include/os/packet_stream.h @@ -143,8 +143,6 @@ class Genode::Packet_descriptor Genode::off_t offset() const { return _offset; } Genode::size_t size() const { return _size; } - - bool valid() const { return _size != 0; } }; @@ -645,12 +643,19 @@ class Genode::Packet_stream_source : private Packet_stream_base Packet_descriptor alloc_packet(Genode::size_t size, int align = POLICY::Packet_descriptor::PACKET_ALIGNMENT) { void *base = 0; - if (_packet_alloc->alloc_aligned(size, &base, align).error()) + if (size && _packet_alloc->alloc_aligned(size, &base, align).error()) throw Packet_alloc_failed(); return Packet_descriptor((Genode::off_t)base, size); } + bool packet_valid(Packet_descriptor packet) + { + return (packet.offset() >= _bulk_buffer_offset + && packet.offset() < _bulk_buffer_offset + (Genode::off_t)_bulk_buffer_size + && packet.offset() + packet.size() <= _bulk_buffer_offset + _bulk_buffer_size); + } + /** * Get pointer to the content of the specified packet * @@ -658,7 +663,7 @@ class Genode::Packet_stream_source : private Packet_stream_base */ Content_type *packet_content(Packet_descriptor packet) { - if (!packet.valid() || packet.size() < sizeof(Content_type)) + if (!packet_valid(packet) || packet.size() < sizeof(Content_type)) return 0; return (Content_type *)((Genode::addr_t)_ds_local_base + packet.offset()); @@ -700,7 +705,8 @@ class Genode::Packet_stream_source : private Packet_stream_base */ void release_packet(Packet_descriptor packet) { - _packet_alloc->free((void *)packet.offset(), packet.size()); + if (packet.size()) + _packet_alloc->free((void *)packet.offset(), packet.size()); } void debug_print_buffers() { @@ -807,8 +813,7 @@ class Genode::Packet_stream_sink : private Packet_stream_base Packet_descriptor get_packet() { Packet_descriptor packet; - do { _submit_receiver.rx(&packet); } - while (!packet_valid(packet)); + _submit_receiver.rx(&packet); return packet; } @@ -829,7 +834,7 @@ class Genode::Packet_stream_sink : private Packet_stream_base */ Content_type *packet_content(Packet_descriptor packet) { - if (!packet.valid() || packet.size() < sizeof(Content_type)) + if (!packet_valid(packet) || packet.size() < sizeof(Content_type)) return 0; return (Content_type *)((Genode::addr_t)_ds_local_base + packet.offset()); diff --git a/repos/os/include/usb/packet_handler.h b/repos/os/include/usb/packet_handler.h index 2ad6ee637d..f739f08867 100644 --- a/repos/os/include/usb/packet_handler.h +++ b/repos/os/include/usb/packet_handler.h @@ -85,16 +85,6 @@ class Usb::Packet_handler throw Usb::Session::Tx::Source::Packet_alloc_failed(); } - if (size == 0) { - /* - * XXX Packets without payload are not supported by the packet - * stream currently. Therefore, we use a (small) bogus - * length here and depend on the USB driver to handle this - * case correctly. - */ - size = 4; - } - while (true) { try { Packet_descriptor p = _connection.source()->alloc_packet(size); diff --git a/repos/os/src/drivers/ahci/ata_driver.h b/repos/os/src/drivers/ahci/ata_driver.h index 587705c662..861989cb19 100644 --- a/repos/os/src/drivers/ahci/ata_driver.h +++ b/repos/os/src/drivers/ahci/ata_driver.h @@ -198,7 +198,7 @@ struct Ata_driver : Port_driver unsigned find_free_cmd_slot() { for (unsigned slot = 0; slot < cmd_slots; slot++) - if (!pending[slot].valid()) + if (!pending[slot].size()) return slot; throw Block::Driver::Request_congestion(); @@ -209,7 +209,7 @@ struct Ata_driver : Port_driver unsigned slots = Port::read() | Port::read(); for (unsigned slot = 0; slot < cmd_slots; slot++) { - if ((slots & (1U << slot)) || !pending[slot].valid()) + if ((slots & (1U << slot)) || !pending[slot].size()) continue; Block::Packet_descriptor p = pending[slot]; @@ -224,7 +224,7 @@ struct Ata_driver : Port_driver Block::sector_t end = block_number + count - 1; for (unsigned slot = 0; slot < cmd_slots; slot++) { - if (!pending[slot].valid()) + if (!pending[slot].size()) continue; Block::sector_t pending_start = pending[slot].block_number(); diff --git a/repos/os/src/drivers/ahci/atapi_driver.h b/repos/os/src/drivers/ahci/atapi_driver.h index 45aa67b2f8..3d8c8fa940 100644 --- a/repos/os/src/drivers/ahci/atapi_driver.h +++ b/repos/os/src/drivers/ahci/atapi_driver.h @@ -86,7 +86,7 @@ struct Atapi_driver : Port_driver { unsigned slots = Port::read(); - if (slots & 1 || !pending.valid()) + if (slots & 1 || !pending.size()) return; Block::Packet_descriptor p = pending; @@ -176,7 +176,7 @@ struct Atapi_driver : Port_driver addr_t phys, Block::Packet_descriptor &packet) override { - if (pending.valid()) + if (pending.size()) throw Block::Driver::Request_congestion(); sanity_check(block_number, count); diff --git a/repos/os/src/drivers/nic/gem/cadence_gem.h b/repos/os/src/drivers/nic/gem/cadence_gem.h index 127a73d818..67293f3fb0 100644 --- a/repos/os/src/drivers/nic/gem/cadence_gem.h +++ b/repos/os/src/drivers/nic/gem/cadence_gem.h @@ -461,7 +461,7 @@ namespace Genode return false; Genode::Packet_descriptor packet = _tx.sink()->get_packet(); - if (!packet.valid()) { + if (!packet.size()) { PWRN("Invalid tx packet"); return true; } diff --git a/repos/os/src/drivers/nic/spec/lan9118/lan9118.h b/repos/os/src/drivers/nic/spec/lan9118/lan9118.h index b6d32e844a..c6b0188c98 100644 --- a/repos/os/src/drivers/nic/spec/lan9118/lan9118.h +++ b/repos/os/src/drivers/nic/spec/lan9118/lan9118.h @@ -199,7 +199,7 @@ class Lan9118 : public Nic::Session_component, return false; Genode::Packet_descriptor packet = _tx.sink()->get_packet(); - if (!packet.valid()) { + if (!packet.size()) { PWRN("Invalid tx packet"); return true; } diff --git a/repos/os/src/drivers/nic/spec/linux/main.cc b/repos/os/src/drivers/nic/spec/linux/main.cc index 1b8c58fd0a..3dd7cb3c4e 100644 --- a/repos/os/src/drivers/nic/spec/linux/main.cc +++ b/repos/os/src/drivers/nic/spec/linux/main.cc @@ -128,7 +128,7 @@ class Linux_session_component : public Nic::Session_component return false; Packet_descriptor packet = _tx.sink()->get_packet(); - if (!packet.valid()) { + if (!packet.size()) { PWRN("Invalid tx packet"); return true; } diff --git a/repos/os/src/server/blk_cache/driver.h b/repos/os/src/server/blk_cache/driver.h index cd4da8d2b5..6435144020 100644 --- a/repos/os/src/server/blk_cache/driver.h +++ b/repos/os/src/server/blk_cache/driver.h @@ -254,8 +254,8 @@ class Driver : public Block::Driver } catch(Block::Session::Tx::Source::Packet_alloc_failed) { throw Request_congestion(); } catch(Genode::Allocator::Out_of_memory) { - if (p_to_dev.valid()) /* clean up */ - _blk.tx()->release_packet(p_to_dev); + /* clean up */ + _blk.tx()->release_packet(p_to_dev); throw Request_congestion(); } } diff --git a/repos/os/src/server/nic_bridge/packet_handler.cc b/repos/os/src/server/nic_bridge/packet_handler.cc index d885aade00..f7f674cb79 100644 --- a/repos/os/src/server/nic_bridge/packet_handler.cc +++ b/repos/os/src/server/nic_bridge/packet_handler.cc @@ -30,7 +30,7 @@ void Packet_handler::_ready_to_submit(unsigned) /* as long as packets are available, and we can ack them */ while (sink()->packet_avail()) { _packet = sink()->get_packet(); - if (!_packet.valid()) continue; + if (!_packet.size()) continue; handle_ethernet(sink()->packet_content(_packet), _packet.size()); if (!sink()->ready_to_ack()) { diff --git a/repos/os/src/server/nic_loopback/main.cc b/repos/os/src/server/nic_loopback/main.cc index 07c9744538..7adb339fb0 100644 --- a/repos/os/src/server/nic_loopback/main.cc +++ b/repos/os/src/server/nic_loopback/main.cc @@ -124,8 +124,8 @@ void Nic::Loopback_component::_handle_packet_stream() /* obtain packet */ Packet_descriptor const packet_from_client = _tx.sink()->get_packet(); - if (!packet_from_client.valid()) { - PWRN("received invalid packet"); + if (!packet_from_client.size()) { + PWRN("received zero-size packet"); _rx.source()->release_packet(packet_to_client); continue; } diff --git a/repos/os/src/server/part_blk/component.h b/repos/os/src/server/part_blk/component.h index 3c6b65cdfd..d28cbdc474 100644 --- a/repos/os/src/server/part_blk/component.h +++ b/repos/os/src/server/part_blk/component.h @@ -73,7 +73,7 @@ class Block::Session_component : public Block::Session_rpc_object, _p_to_handle.succeeded(false); /* ignore invalid packets */ - if (!packet.valid() || !_range_check(_p_to_handle)) { + if (!packet.size() || !_range_check(_p_to_handle)) { _ack_packet(_p_to_handle); return; } diff --git a/repos/ports/src/app/openvpn/main.cc b/repos/ports/src/app/openvpn/main.cc index ff6a92244d..398e5788d3 100644 --- a/repos/ports/src/app/openvpn/main.cc +++ b/repos/ports/src/app/openvpn/main.cc @@ -110,7 +110,7 @@ class Openvpn_component : public Tuntap_device, return false; Packet_descriptor packet = _tx.sink()->get_packet(); - if (!packet.valid()) { + if (!packet.size()) { PWRN("Invalid tx packet"); return true; }