Help with enums and protocol architecture

Hi there, I'm writing a protocol that operates over TCP. Some time down the line I'll be very interested to post a full code review on my code and the protocol itself, but right now it's too young for that. Also keep in mind that this is my first heavy project in Rust or any systems language.

These are the building blocks of my protocol:

// Types of messages sent
#[derive(FromPrimitive, ToPrimitive, Debug, PartialEq, Serialize, Deserialize)]
pub enum Type {
    DscReq= 1, // Discovery Request
    DscRes= 2, // Discover Response
    SeqReq= 3, // Sequence Request
    SeqRes= 4, // Sequence Response
}

// Header of a message
#[derive(Serialize, Deserialize, PartialEq, Debug)]
pub struct Header {
    pub id: u16,
    pub size: u16,
    pub meta: u16,
    pub parcel_type: Type, // u16
}

impl Header {
    pub fn new(id: u16, size: u16, meta: u16, parcel_type: Type) -> Self {
        Header { id, size, meta, parcel_type }
    }
}

// Body of a Message with the different types
#[derive(Serialize, Deserialize)]
pub enum Body {
    DscReq { identity: Node },
    DscRes {
        size: u16,
        neighbours: Vec<Node>,
    },
}

#[derive(Serialize, Deserialize)]
pub struct ProtoParcel {
    pub header: Header,
    pub body: Body,
}

And this is what I'm trying to implement.

I'm open to all types of anecdotes, architectural and code-based. I started out with doing manual byte serialization/deserialization but then realized that serde does that for you. I thought it would be a good idea that the protocol has an 8-byte header and variable sized body, followed by the application-specific bytes. Ignore what is said in the protocol doc about byte ordering, I'll probably stick to a widely-used standard for serialization such as CBOR or MessagePack and ditch bincode since it's too language/library-specific. My protocol approach is probably bad and I should just forget the whole header/body abstraction and use a single struct/enum for the whole message. Any thoughts on this?

This has turned into more of a code-review for now but there's still a lot of things I need to figure out.
My general programming question is: How should I structure my message abstraction properly while still keeping protocol correctness? As in, I still want my header to contain a Type field that defines the body that follows but I don't want that field to belong to the Parcel (the whole message). And how would I implement the following rusty non-compileable pseudocode:


        let mut res_parcel: ProtoParcel = bincode::deserialize(res.as_slice()).unwrap();
        let body = res_parcel.body as Body::DscRes; // DscRes expected
        Ok(body.neighbours) // return field 
        

I also want to say that I've been really enjoying the language and system programming as a whole!

I can't speak to your architecture, but I'd advise against putting uncommented acronyms in your code if you will be sharing it, but also, follow the idioms in other Rust code (enum members aren't CAPS_CASE, but PascalCase).

Thanks for the heads up, edited accordingly.

Still learning Rust, but I know a thing or two about protocols.

If you want to put your protocol directly over TCP there are some things you need to do which I did not see you mentioning anywhere:

  • Add a version number, magic number / identifier of your protocol. How would you otherwise know this is a valid packet of your protocol or you are receiving a packet from other protocol which happened to use the same port? (e.g. HTTP starts or ends the first line with HTTP/1.1)
  • TCP is a stream-oriented protocol - which means for application layer design of reception that you need to take care of multiple messages yourself. If you could ever have 2 messages on the wire, they could be delivered to you at the same time. So you need a separator.
  • You seem to be using a lot of u16 and in your description you say "verify the sequence of an unbounded stream of events." . Be careful with too short identifiers, like e.g. wrap around of a sequence number. In the application layer you do not need really to care too much about saving some space on header (compare that to HTTP or SIP)...
  • Your serialized data should starts with a version. Otherwise you cannot make new versions in the future.
2 Likes

At work I needed to do something similar to this when communicating with a device over a RS232 serial connection. Depending on which message was being sent its payload could be laid out radically differently and have different sets of metadata. I was also constrained because this protocol had been defined several years before and can't be changed without breaking machines in the wild.

To start off with, I defined a dumb low-level format for the packet that gets sent back and forth.

struct Packet<'a> {
    /// Which message type is being sent?
    message_id: u8,
    /// Some messages have a sequence number (modulo 256) to help
    /// detect dropped packets or when a packet was resent and we 
    /// can ignore the duplicate.
    sequence_number: Option<u8>,
    /// Simple checksum as a sanity check.
    body_checksum: u8,
    /// The actual type and whether it is owned/borrowed doesn't 
    /// matter here, just that we've got an opaque blob of bytes 
    /// for the message body.
    body: &'a [u8],
}

I'd recommend writing a wrapper around some Read + Write type (e.g. a TcpStream) which buffers reads and scans through looking for potential packets. One project I did at uni used the anpp crate for the wire format, and the anpp::Decoder type shows a really nice way of implementing this.


The next layer up had a bunch of more strongly-typed messages that could be converted to/from a Packet and would be used by the application layer.

trait Message<'a>: Sized + 'a {
    fn serialize<W: Write>(&self, writer: W) -> Result<(), SerializeError>;
    fn deserialize(packet: Packet<'a>) -> Result<Self, DeserializeError>;
}

struct Ping;
struct Pong;
impl<'a> Message<'a> for Ping { ... }
impl<'a> Message<'a> for Pong { ... }

struct StatusRequest;

struct StatusResponse {
    fault_code: u32,
    current_state: MachineState,
}
enum MachineState { ... }
impl<'a> Message<'a> for StatusRequest { ... }
impl<'a> Message<'a> for StatusResponse { ... }

For my application the deserialize() and serialize() implementations needed to be coded by hand, but I'd strongly recommend avoiding that if possible. If you've already parsed the data into raw Packets, you could do a match on the message_id and shell out to CBOR or MessagePack or Protobufs for the actual serializing/deserializing.

Which format you use doesn't really matter because they'll all be within the same ballpark in terms of efficiency, the main thing you want to focus on is being able to implement new message types with little effort by the developer.


I was lucky because the protocol (mostly) followed a request-response where only one request-response pair could be in flight at a time. So out-of-order messages could be assumed to be due to dropped packets or interference and the sender would retry after a fixed timeotu.

And at the very top level I'd have some sort of Client which exposes methods that'll do the IO, serializing a request to be sent over the connection.

It looked something like this:

struct Client {
  data_stream: Stream,
  outgoing_buffer: Vec<u8>,
  incoming_decoder: Decoder,
}

impl Client {
    pub fn send_ping(&mut self) -> Result<(), Error> {
       let msg = Ping;
        let _response: Pong = self.send(&msg)?;

        Ok(())
    }

    fn send<P, Q>(&mut self, request: &P> -> Result<Q, Error> {
        // Note: this would probably be done in a loop to provide timeout and retry logic

        // send the request
        self.outgoing_buffer.clear();
        request.serialize(&mut self.outgoing_buffer)?;
        
        // keep reading from the stream until we find a message
        let mut incoming_buffer = [0; 256];
        loop {
            let bytes_read = self.data_stream.read(&mut incoming_buffer[..])?;
            self.incoming_decoder.push_data(&incoming_buffer[..bytes_read]);

            // have we received a full message yet?
            match self.incoming_decoder.decode() {
                Ok(packet) => return Q::deserialize(packet),
                Err(DecodeError::RequiresMoreData) => continue,
                Err(other) => return Err(other),
            }
        }
    }
}

This is all very much idealised. In practice, things were a little messier because you need to account for edge cases and things tend to evolve organically over time.

The overall structure and separation of layers worked out well though, because lower layers could handle the ugliness of working in the real world and meant the higher layers could focus on other things.

1 Like

Thanks for reminding me a bunch of things I should've done already. In the end I think it'd be a better idea I bump up the 'header' size.

You also reminded me I'm working with TCP and not http and as such I don't really have a guaranteed request/response structure. I'm thinking of separating requests from responses and having a main state thread that sends protocol packets and a second listener thread that handles recieved protocol packets and dispatches actions/updates states through a mutex.

This type of layering looks like a nice abstraction. I'm thinking manually parsing the header might be a good idea and I'd really love to think more about the serializing/deserializing technique but that'd be too much work for now. I want to make the actual messaging part of the protocol with some form of sequencing. Checksums sound like a good idea too and again, thanks for making me realize I'm working over TCP and not HTTP. In my case I could be receiving a different message (e.g node sends an error) than the one I'm expecting (application-level acknowledge or else).

I'm not sure if you'll need to worry about checksums because the TCP and Ethernet layers have error checking built in. My RS232 use case is more akin to transmitting Ethernet packets myself, so I had to manage the data integrity side of things myself.

In theory, you should be able to read the header directly off the front of the incoming data stream because lower layers will have taken care of things like error correction and putting packets back in their original order.

You're lucky in that Rust's slices, write!(), and iterators (in particular some_iterator.windows()) make manually parsing bytes considerably easier than other languages. Serde also gives you a nice way of generating serialization code that a particular protocol (e.g. JSON or MessagePack) can just plug into. I've had to do similar things in C# and Python, and it's noticeably less ergonomic and more error-prone.

So you've got the tools and flexibility take whichever approach you like, and traits/encapsulation/abstractions let you change your mind later on without having to start over. I'd recommend prototyping both options to see which works better for you.

The libsignal-service-rs project had to deal with something quite similar, where most the client would use a request-response when asking for certain things (in this case, it talked to a REST interface), but the server could also push unrelated packets to the client via a websocket.

Seeing as all the packets come through the one stream, the tricky part would be writing a thing that'll look at an incoming packet's ID and decide whether to pass it to the out-of-band channel or use it as the response to a pending request. You'd be implementing very similar logic to what a HTTP client does when handing websockets and normal HTTP requests multiplexed over the same TCP socket.