isLooped checks for any duplicate prefixes

[everett1992]

previous implemenation only checked for duplicates self_id. A node would happily forward a packet that had signs it was already looped. Since not all nodes in a network will enable loop detection I want configured nodes to drop these packets.

This is a quadratic loop, but it's upper bound is 64. It can be improved with a better data structure but I wanted to keep it simple.

The package doesn't have a test framework so I wrote a small harness to double check the behavior. All the 'self' tests pass with the previous impl.

runTest(1, {0x11, 0x22, 0x33}, max_loop_strict, false, "no dups");
runTest(1, {0xAA, 0x22}, max_loop_strict, true, "dup with self");
runTest(1, {0x11, 0x11, 0x22, 0x33}, max_loop_strict, true, "dup in path");
runTest(2, {0xAA, 0x22, 0xBB, 0x44}, max_loop_strict, false, "2-byte no dup");
runTest(2, {0xAA, 0xBB, 0xCC, 0xDD}, max_loop_strict, true, "2-byte dup with self");
runTest(2, {0x00, 0x11, 0x00, 0x11}, max_loop_strict, true, "2-byte dup in path");
runTest(3, {0xAA, 0xBB, 0xCC}, max_loop_strict, true, "3-byte dup with self");
runTest(3, {0x11, 0x22, 0x33, 0x11, 0x22, 0x33}, max_loop_strict, true, "3-byte dup in path");

runTest(1, {0x11, 0x11, 0x11, 0x11}, max_loop_minimal, false, "4 repeats: under minimal limit");
runTest(1, {0x11, 0x11, 0x11, 0x11, 0x11}, max_loop_minimal, true, "5 repeats: over minimal limit");
runTest(1, {0xAA, 0xAA, 0xAA}, max_loop_minimal, false, "3 repeats and self: under minimal limit");
runTest(1, {0xAA, 0xAA, 0xAA, 0xAA}, max_loop_minimal, true, "4 repeats and self: over minimal limit");

[devurandomhex]

Ahh, I should have rechecked PRs before I implemented my own very similar version of this. Agree that it's absolutely needed though, especially in the case where the misbehaving repeater is not running vanilla firmware - this has exhausted the airtime for our region multiple times in the last month.

I made one potential variant here after considering things a bit further. In particular, I wanted to get around a potential blackhole scenario where long hauls might have a prefix collision on one byte and any strict loop detection setting would drop packets. Think [aa]->[bb]->|other hops|->[bb]->[cc] being caught out by the bb. Thus, falling back to sequential duped path bytes since that's by and large what we've seen when individual repeaters go off.

Both still do tighten the parameters around prefix-collisions within earshot of each other (especially on strict), but I feel like that's a little more expected/easy to admin your way out of (mostly have seen this when someone is flashing a bunch of nodes at home with vanity prefixes).

In general I prefer the counting-full-path method since it also catches things like bitflips which result in a U-turn packet, but wanted to lay out the case for "this might change current behavior slightly" if it lands as-is. Nice work!

[robekl]

loop detection will treat any repeated path hash segment as a loop, so equal 1-byte hashes can be considered loops if they collide and will cause healthy packets to be dropped. Is that okay?

[everett1992]

That's been the case since loop detection was added, tho the check and false positive rate is lower without this change because it only triggered on collisions with the self-id.

There are different thresholds for loop detection,

These two tests are passing with original code and my change. An ID has to appear 4 times, and match your ID to be dropped.

runTest(1, {0xAA, 0xAA, 0xAA}, max_loop_minimal, false, "3 repeats and self: under minimal limit");
runTest(1, {0xAA, 0xAA, 0xAA, 0xAA}, max_loop_minimal, true, "4 repeats and self: over minimal limit");

These tests pass with my change, the id must appear 5 times.

runTest(1, {0x11, 0x11, 0x11, 0x11}, max_loop_minimal, false, "4 repeats: under minimal limit");
runTest(1, {0x11, 0x11, 0x11, 0x11, 0x11}, max_loop_minimal, true, "5 repeats: over minimal limit");

Assuming random distribution of node ids there's a 0.15% chance to drop a full 64 hop packet.

About 0.15% — roughly 1 in 664 chance that when you pick 64 random numbers from 1–254, at least one
number shows up 5 or more times.