I was pulling my hair out trying to track this down!
Long story short, while calculating the per-bit-position tunnel strength of the connection solutions, the input temppath doesn't get reset properly. This results in the calculated columnar tunnel strength being absolutely massive. This doesn't cause incorrect paths, but it does render a large number of the heuristic tests useless.
Bug Details
The code populates temppath and copies in the columnar conditions from newpath2 (the combined lower + upper paths).
However, when it clears the columnar conditions, it uses newpath (just the upper path) instead of newpath2. Any tunnel strength provided by the lower path doesn't get reset and ends up accumulating.
Results
Unfortunately, fixing this on its own actually makes the heuristics worse. 🙁
That corrected bit position tunnel strength calculation always ends up being too low. This is because the totaltunnelstrength function includes probabilistic tunnels which have a (hw(Q0Q1eq & Q1free & ~path[2].mask())>>1) component that always evaluates to zero.
Also, totaltunnelstrength is only accurate if the path contains only backward conditions, but the cleanup function isn't called until after the heuristic checks.
The heuristics are certainly worthwhile, but fixing them requires some non-trivial changes.
I was pulling my hair out trying to track this down!
Long story short, while calculating the per-bit-position tunnel strength of the connection solutions, the input temppath doesn't get reset properly. This results in the calculated columnar tunnel strength being absolutely massive. This doesn't cause incorrect paths, but it does render a large number of the heuristic tests useless.
Bug Details
The code populates temppath and copies in the columnar conditions from newpath2 (the combined lower + upper paths).
However, when it clears the columnar conditions, it uses newpath (just the upper path) instead of newpath2. Any tunnel strength provided by the lower path doesn't get reset and ends up accumulating.
Results
Unfortunately, fixing this on its own actually makes the heuristics worse. 🙁
That corrected bit position tunnel strength calculation always ends up being too low. This is because the totaltunnelstrength function includes probabilistic tunnels which have a
(hw(Q0Q1eq & Q1free & ~path[2].mask())>>1)component that always evaluates to zero.Also, totaltunnelstrength is only accurate if the path contains only backward conditions, but the cleanup function isn't called until after the heuristic checks.
The heuristics are certainly worthwhile, but fixing them requires some non-trivial changes.