packetnet-connections icon indicating copy to clipboard operation
packetnet-connections copied to clipboard

FIXME: we don't handle rolling sequence numbers properly in here

Open Tiax opened this issue 3 years ago • 3 comments

I've been looking into problems with (seemingly random) receiving only "OutOfSequence" packets after a while (and usually only when lots of packets went over the wire), when I found this comment in TcpStreamGenerator.cs#L235:

//FIXME: we don't handle rolling sequence numbers properly in here
// so after 4GB of packets we will have issues

Which might explain my problem: when the initial Seq number is already a big number, we'd probably reach the uint32 limit rather quickly, causing it to not recognize the packet sequence anymore after it rolled over?

I'm not too familiar with TCP internas - How hard would this be to address? I naively imagine it's just a couple modulo operations here and there, or is there more to it?

Tiax avatar Oct 04 '21 14:10 Tiax

        private static bool SeqLT(uint a, uint b) => ((int)a - (int)b) < 0;
        private static bool SeqGT(uint a, uint b) => ((int)a - (int)b) > 0;

Looks like using those for seq number comparison may be all it needed, since C# rolls over uint32 anyway, so no need for any modulos (unchecked by default).

Tiax avatar Oct 06 '21 11:10 Tiax

@Tiax not sure how hard it is to address, I'm guessing it's just a handful of changes where we care about sequence number comparison. Do you have a fix for the comparisons? I've been pretty inactive with the library for a while now since I'm not actively using it but would appreciate a PR if you do fix it.

chmorgan avatar Feb 04 '23 23:02 chmorgan

It worked for me with using the snippet above for lesser/greater than comparisons for the Sequence. Ended up using a different library though in the end, so cannot really tell.

Tiax avatar Feb 16 '23 20:02 Tiax