I see 2 callback functions defined:

void (*transmit_packet_function)(void*,int,uint16_t,uint8_t*,int); is fired whenever a packet is ready to be sent. it's implementation should handle actually sending data over the socket.

int (*process_packet_function)(void*,int,uint16_t,uint8_t*,int); is fired whenever a packet is received (either a regular packet or a fragmented packet that's been fully re-assembled)

I'm surprised that there is no callback function that is invoked whenever a packet goes from non-acked -> acked. Isn't this the kind of callback one would need to build a reliable message system on top of reliable.io ? Otherwise you'd have to duplicate all of https://github.com/networkprotocol/reliable.io/blob/master/reliable.c#L1090-L1120 logic inside your process_packet_function. Or is there a better way to handle this?

Not an issue but more of a philosophical question.

When processing a packet, it would be a normal operation to reply on the same endpoint using reliable_endpoint_send_packet. As of now we receive a <context,index> pair (btw, not sure what is the index utility).

Reading the docs and the code (no comments!), I am not sure how to proceed. So I am sending a packet over UDP using a reliable "endpoint". Does reliable.io handles re-transmission, I see that reliable_endpoint_receive_packet does some ACK computations but never writes back to sending endpoint. I think a step-by-step explanation would do wonder for this little library.

while looking at:

struct reliable_endpoint_t
{
   .
   .
   .
    uint16_t * acks;
    uint16_t sequence;
    struct reliable_sequence_buffer_t * sent_packets;
    struct reliable_sequence_buffer_t * received_packets;

I don't understand the usage sequence and sent_packets as used on these lines:

https://github.com/networkprotocol/reliable.io/blob/master/reliable.c#L725 https://github.com/networkprotocol/reliable.io/blob/master/reliable.c#L733

Is this redundant? Why effectively store 3 sequence numbers when the 2 above lines seem to conflate sequence with sent_packets->sequence ?

@gafferongames in working on a javascript port, I was going through the code in this library. It's so well written that I could follow along very easily. Very refreshing!

There are 3 areas where there is a little head scratching:

I suspect this code has to do with handling very rare cases of extreme packet loss with sequence buffer wrap around: https://github.com/networkprotocol/reliable.io/blob/master/reliable.c#L249-L256

similarly, there's some logic in `_remove_entries which is a little confusing: https://github.com/networkprotocol/reliable.io/blob/master/reliable.c#L207-L226

It's not clear to me what the _with_cleanup variants of sequence buffer insertion/removal are for https://github.com/networkprotocol/reliable.io/blob/master/reliable.c#L264 https://github.com/networkprotocol/reliable.io/blob/master/reliable.c#L264

I think comments in here about what these little bits are would be immensely helpful. Everything else makes total sense.

Thanks again for a great codebase!

Hi Glenn!

I read source, and I can't get the way retransmits are handled. Let's say we have next situation. A send to B single packet, which is dropped during transmission. A -[DATA1]-XXXXX (missing)-> B Because B miss this packet, it will never send ACK to A (or any other thing).

How A should resolve this situation? Likely there should be some timer, which should retransmit non-ACKed packets after some time. Like (simplified):

while (reliable_endpoint_pending(endpoint)) {
   sleep(reliable_endpoint_wait_for_ack_timeout(endpoint));
   if (reliable_endpoint_pending(endpoint)) 
       reliable_endpoint_retransmit(endpoint);
}

Or am I missing something?

reliable_endpoint_send_packet will verify that the amount of data sent is not greater than max_packet_size, but then it will add data on top of that (up to RELIABLE_FRAGMENT_HEADER_BYTES + RELIABLE_MAX_PACKET_HEADER_BYTES AFAICT).

On the other end, reliable_endpoint_receive_packet will also verify that the amount of data received is not greater than max_packet_size -- but since there are header bytes added, this can now exceed the max_packet_size, even if both sender and recipient agree on this number.

I'd propose a pull request, but I'm not sure which approach is best:

1. reliable_endpoint_send_packet requires the caller to make sure they're not passing more than max_packet_size - (RELIABLE_FRAGMENT_HEADER_BYTES + RELIABLE_MAX_PACKET_HEADER_BYTES) bytes, so that max_packet_size correctly reflects the largest packet to be sent as seen on the wire, or
2. reliable_endpoint_receive_packet allows packets up to RELIABLE_FRAGMENT_HEADER_BYTES + RELIABLE_MAX_PACKET_HEADER_BYTES bytes larger than max_packet_size to account for the header added.

Thoughts?

If the sequence buffers wrap around while a fragmented packet is still waiting for reassembly and we receive a non-fragmented packet that cause us to advance the fragmented sequence buffer past the incomplete fragmented packet, we would not call a cleanup function on the partially reassembled packet, leaving the packet_data to leak.

This changes the call to reliable_sequence_buffer_advance on the reassembly sequence buffer in the non-fragmented code path to a newly introduced reliable_sequence_buffer_advance_with_cleanup which does the correct cleanup on the reassembly sequence buffer entries.

I am currently playing around with this library and trying to understand how to handle packet retransmission.

Let's suppose i send a packet with sequence X, i don't get an ack in for some time and decide to resend it. After reading issue #14 i think i should send a new packet with sequence number Y and it is my responsibility to remember that packets with sequence numbers X and Y are actually the same.

This seems rather inconvenient, increasingly so with multiple re-transmissions of the same packet. Is this how this library is supposed to be used?

this test shows that the current implementation has a bug when sending a long sequence of regular packets (32767) and then a fragmented packet. since the receiver uses a separate sequence buffer for regular and fragmented packets, they get out of sync, and the fragment_reassembly sequence buffer disregards the packets

I've got a port nearly finished, including all of your unit/soak/fuzz tests (which pass!) The only thing remaining is to put in packet fragmentation/re-assembly. Would you be willing to link to it in this project's README?

I intend to keep it a faithful port in terms of structure and naming.

Totally happy to add you (and/or anyone you designate as qualified) as a maintainer.

I'd also like to publish this to npm