Added packet reliability processing. #79

TimonPost · 2018-10-31T20:05:14Z

This PR is about packet processing described in RFC #65.

!!Note!!
This PR seems big (it is) but there are just a few files needing your attention. They are located in infrastructure module, everything else are just secondary concerns caused by fixing broken tests and moving code to the new channels.

net::connection::virtual_connection.rs and net::udp.rs are also important files where some things have changed.

Channel

A channel processes a packet with a certain reliability. A channel could be responsible for ordering, resending, handling dropped packets etc.

Channel Trait

/// This provides an abstraction for processing packets to their given reliability.
pub trait Channel {
    /// Process an packet before send and return an packet data instance with the given raw packet data.
    fn pre_process(&mut self, payload: &[u8], delivery_method: DeliveryMethod) -> NetworkResult<PacketData>;

    /// Progress an packet on receive and receive the processed data.
    fn process_packet(&mut self, cursor: &mut Cursor<&[u8]>) -> NetworkResult<Vec<u8>>;
}

So we have different instances of channels.

ReliableChannel
This channel should be used for reliable processing of packets. This channel also has an ordering option for ordering packet, however, this is not implemented yet.
SequencedChannel
This channel should be used for sequenced packets. Sequenced means only the newest data will be accepted. This channel has the option to be reliable.
UnreliableChannel
This channel should be used for unreliable processing of packets. This is just bear UDP with some basic header on top of it.

I suggest you look at the code for more information on how I implemented the channels in the code.

Packet Header change.

I also had changed the packet format a little. It was needed for implementing the channels got stuck at the old design. The nice thing now is that the header parser takes in a mutable buffer to which will be written to. It is some kind the same but the following table will show you how the packets are.

Standard header, which will be included in each packet.

pub struct StandardHeader {
    /// crc32 of the protocol version.
    pub protocol_version: u32,
    /// specifies the packet type.
    pub packet_type_id: PacketTypeId,
    /// specifies how this packet should be processed.
    pub delivery_method: DeliveryMethod,
}

Fragment header, this is the header containing header fragment information.
Witch also contains the standard header and the first fragment will also contain

pub struct FragmentHeader {
    standard_header: StandardHeader,
    sequence: u16,
    id: u8,
    num_fragments: u8,
    packet_header: Option<AckedPacketHeader>,
}

Acked packet header wich will be used for reliable packets.

standard_header: StandardHeader,
seq_num: u16,
last_seq: u16,
bit_field: u32,

Overall changes

Overal perfromance improvements
HeaderParser's parse function takes in mutalble buffer witch is more optimal performance wise than generating them local and return an clone of them.
Moved fragmentation stuff into it's own type.
Removed SocketState and PacketProcessor this logic can now be found in the ReliableChannel and Fragmenting type.

fhaynes

It looks ok with a quick skim, but it would take hours to do a proper PR with this. As long as the tests have been updated and pass, and they work with my docker-compose stuff, I'm ok with it.

src/net/constants.rs

src/infrastructure/channels/mod.rs

src/infrastructure/channels/reliable_channel.rs

src/net/connection/virtual_connection.rs

LucioFranco

This is looking really good! Honestly, great work! A lot of comments but mostly questions. I want to take some time to think about the channels. I'm not totally a fan just yet.

src/error/network_error.rs

LucioFranco · 2018-11-02T01:52:10Z

src/infrastructure/channels/mod.rs

+mod reliable_channel;
+
+use infrastructure::DeliveryMethod;
+use error::NetworkResult;


may have missed this before, but this should be called Result to be consistent.

Should be moved into another issue for error handling. We have to error types and Result already exists.

src/infrastructure/channels/mod.rs

src/infrastructure/channels/reliable_channel.rs

LucioFranco · 2018-11-02T01:54:36Z

src/infrastructure/channels/reliable_channel.rs

+    seq_num: u16,
+    waiting_packets: LocalAckRecord,
+    their_acks: ExternalAcks,
+    dropped_packets: Vec<Box<[u8]>>,


since vec is already on the heap, could you move a slice into it, instead of a double allocation?

This is somewhat complicated, and could better be moved to another issue. This is complicated because LocalAckRecord are gathering dropped ack's and returning them.

LucioFranco · 2018-11-02T02:35:45Z

src/packet/packet_data.rs

 use error::NetworkResult;

 /// Contains the raw data this packet exists of. Note that a packet can be divided into separate fragments
 #[derive(Debug, Default)]
 pub struct PacketData {
-    parts: Vec<RawPacketData>,
+    parts: Vec<Vec<u8>>,


vec of a vec? could you explain this more and why you choose the inner as a vec?

Because I removed RawPacketData this was just some unnecessarily code which could be worked away. First I did it since I thought I could have slices here. But I could not accomplish that.

src/packet/packet_data.rs

src/protocol_version.rs

tests/common.rs

tests/fragments.rs

LucioFranco · 2018-11-02T16:15:09Z

is this pr still WIP? if not please update the title

- Channels for processing packets of different reliabilities. - Changed header format a little to fit current design. - Moved fragmentation logic to it's own type. - Global performance improvements by preventing unnecesary alocations with vectors. - Removed 'SocketState' and 'PacketProcesor' types.

fhaynes · 2018-11-05T07:55:31Z

bors r+

79: Added packet reliability processing. r=fhaynes a=TimonPost This PR is about packet processing described in RFC #65. **!!Note!!** This PR seems big (it is) but there are just a few files needing your attention. They are located in `infrastructure` module, everything else are just secondary concerns caused by fixing broken tests and moving code to the new channels. `net::connection::virtual_connection.rs` and `net::udp.rs` are also important files where some things have changed. ## Channel A channel processes a packet with a certain reliability. A channel could be responsible for ordering, resending, handling dropped packets etc. _Channel Trait_ ```rust /// This provides an abstraction for processing packets to their given reliability. pub trait Channel { /// Process an packet before send and return an packet data instance with the given raw packet data. fn pre_process(&mut self, payload: &[u8], delivery_method: DeliveryMethod) -> NetworkResult<PacketData>; /// Progress an packet on receive and receive the processed data. fn process_packet(&mut self, cursor: &mut Cursor<&[u8]>) -> NetworkResult<Vec<u8>>; } ``` So we have different instances of channels. 1. ReliableChannel This channel should be used for reliable processing of packets. This channel also has an ordering option for ordering packet, however, this is not implemented yet. 2. SequencedChannel This channel should be used for sequenced packets. Sequenced means only the newest data will be accepted. This channel has the option to be reliable. 3. UnreliableChannel This channel should be used for unreliable processing of packets. This is just bear UDP with some basic header on top of it. I suggest you look at the code for more information on how I implemented the channels in the code. ## Packet Header change. I also had changed the packet format a little. It was needed for implementing the channels got stuck at the old design. The nice thing now is that the header parser takes in a mutable buffer to which will be written to. It is some kind the same but the following table will show you how the packets are. Standard header, which will be included in each packet. ```rust pub struct StandardHeader { /// crc32 of the protocol version. pub protocol_version: u32, /// specifies the packet type. pub packet_type_id: PacketTypeId, /// specifies how this packet should be processed. pub delivery_method: DeliveryMethod, } ``` Fragment header, this is the header containing header fragment information. Witch also contains the standard header and the first fragment will also contain ```rust pub struct FragmentHeader { standard_header: StandardHeader, sequence: u16, id: u8, num_fragments: u8, packet_header: Option<AckedPacketHeader>, } ``` Acked packet header wich will be used for reliable packets. ```rust standard_header: StandardHeader, seq_num: u16, last_seq: u16, bit_field: u32, ``` ## Overall changes - Overal perfromance improvements - HeaderParser's `parse` function takes in mutalble buffer witch is more optimal performance wise than generating them local and return an clone of them. - Moved fragmentation stuff into it's own type. - Removed `SocketState` and `PacketProcessor` this logic can now be found in the `ReliableChannel` and `Fragmenting` type. ![bonus](https://media.discordapp.net/attachments/425694112973586452/506815279125495843/DjaXbaVW4AInQXP.png?width=601&height=664) Co-authored-by: Timon Post <timonpost@hotmail.nl>

bors · 2018-11-05T08:35:11Z

Build succeeded

continuous-integration/travis-ci/push

TimonPost changed the title ~~Added packet reliability processing~~ WIP | Added packet reliability processing. Oct 31, 2018

TimonPost requested review from LucioFranco and fhaynes and removed request for LucioFranco October 31, 2018 20:05

fhaynes reviewed Oct 31, 2018

View reviewed changes

src/net/constants.rs Outdated Show resolved Hide resolved

TimonPost requested a review from LucioFranco October 31, 2018 20:17

fhaynes requested changes Nov 1, 2018

View reviewed changes

LucioFranco suggested changes Nov 2, 2018

View reviewed changes

TimonPost mentioned this pull request Nov 2, 2018

Resend dropped packets. #81

Closed

fhaynes approved these changes Nov 2, 2018

View reviewed changes

TimonPost changed the title ~~WIP | Added packet reliability processing.~~ Added packet reliability processing. Nov 2, 2018

bors bot merged commit db9c0fe into TimonPost:master Nov 5, 2018

TimonPost deleted the channel branch November 26, 2018 09:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added packet reliability processing. #79

Added packet reliability processing. #79

TimonPost commented Oct 31, 2018 •

edited

Loading

fhaynes left a comment

LucioFranco left a comment

LucioFranco Nov 2, 2018

TimonPost Nov 2, 2018

LucioFranco Nov 2, 2018

TimonPost Nov 2, 2018

LucioFranco Nov 2, 2018

TimonPost Nov 2, 2018

LucioFranco commented Nov 2, 2018 •

edited

Loading

fhaynes commented Nov 5, 2018

bors bot commented Nov 5, 2018

Added packet reliability processing. #79

Added packet reliability processing. #79

Conversation

TimonPost commented Oct 31, 2018 • edited Loading

Channel

Packet Header change.

Overall changes

fhaynes left a comment

Choose a reason for hiding this comment

LucioFranco left a comment

Choose a reason for hiding this comment

LucioFranco Nov 2, 2018

Choose a reason for hiding this comment

TimonPost Nov 2, 2018

Choose a reason for hiding this comment

LucioFranco Nov 2, 2018

Choose a reason for hiding this comment

TimonPost Nov 2, 2018

Choose a reason for hiding this comment

LucioFranco Nov 2, 2018

Choose a reason for hiding this comment

TimonPost Nov 2, 2018

Choose a reason for hiding this comment

LucioFranco commented Nov 2, 2018 • edited Loading

fhaynes commented Nov 5, 2018

bors bot commented Nov 5, 2018

Build succeeded

TimonPost commented Oct 31, 2018 •

edited

Loading

LucioFranco commented Nov 2, 2018 •

edited

Loading