PhirePhly / aprx

A highly configurable APRS I-gate/Digipeater Daemon
http://thelifeofkenneth.com/aprx/
BSD 3-Clause "New" or "Revised" License
150 stars 67 forks source link

SSn-N not supported correctly. #23

Open dandrzejewski opened 8 years ago

dandrzejewski commented 8 years ago

I want my digipeater to support OH7-7. So I configured it as such:

 <source>
   source         $mycall
   viscous-delay 1
   <trace>
     maxreq      3
     maxdone     3
     keys        WIDE,TRACE
   </trace>
   <wide>
     maxreq        7
     maxdone       7
     keys          OH
   </wide>
</source>

Yet, when I send an OH7-7 packet:

2015-11-15 16:12:33.823 NEWBRY    R KD8TWG>APWW10,OH7-7:=/9PDM9sX`- sTTesting OH7-7.
2015-11-15 16:12:34.585 NEWBRY    T KD8TWG>APWW10,NEWBRY*,OH7-7*:=/9PDM9sX`- sTTesting OH7-7.
2015-11-15 16:12:34.588 NEWBRY    R KD8TWG>APWW10,KF8YK-2*,OH7-6:=/9PDM9sX`- sTTesting OH7-7.
2015-11-15 16:12:35.589 NEWBRY    R KD8TWG>APWW10,N8BHU-1*,OH7-6:=/9PDM9sX`- sTTesting OH7-7.
2015-11-15 16:12:37.888 NEWBRY    R KD8TWG>APWW10,K8UI-1*,OH7-5:=/9PDM9sX`- sTTesting OH7-7.

As you can see, my digipeater (NEWBRY) is not decrementing the path correctly.

PhirePhly commented 8 years ago

Section 3.9.1 of the aprx manual reads:

The system has a special “heard direct” behaviour when maxreq or maxdone is reached or exceeded, or when a new-n paradigm style “KEYn-N” entry has N > n: The system will insert itself at first VIA slot, and mark original request VIA data with H-bits (“digipeating completed”) and transmit the resulting packet. If packet was not recognized as “heard direct”, then it is silently dropped.

So since you have OH7 which is equal to your maxreq 7, it wipes out the whole path. Also since you are above 5, it wipes out the whole path. (No idea how I came up with that)

So I guess, thankfully, it seems to be behaving how it should be. What happens if you configure it with maxreq 8 or send it OH6-6?

PhirePhly commented 8 years ago

Wow. First, I clearly should never try and respond to bug reports when I'm this tired. Second, I think the maxreq/maxdone checking is very broken inside of APRX, or the code is much more clever than I am. I'd still be interested in seeing OH6-6 behavior, but I'm convinced there's a real problem here.

PhirePhly commented 7 years ago

So I think I see why this didn't work. the digipeater_receive_backend() function checks all paths against the maximum hop count for both TRACE and WIDE (non-traced) aliases regardless of which one it it. If any of the path limits are exceeded on an SSn-N packet (including the un-related 3 hops for WIDE), the packet is marked as "fixall", all the hops are marked as consumed in place, and if it's the first hop digipeated.

if (state.v.hopsreq   > digi->trace->maxreq  ||
            state.v.hopsreq   > digi->wide->maxreq   ||
            state.v.tracereq  > digi->trace->maxreq  ||
            state.v.digidone  > digi->trace->maxdone ||
            state.v.digidone  > digi->wide->maxdone  ||
            state.v.hopsdone  > digi->trace->maxdone ||
            state.v.hopsdone  > digi->wide->maxdone  ||
            state.v.tracedone > digi->trace->maxdone) {
    if (!state.v.probably_heard_direct) {
        if (debug) printf(".. discard.\n");
        return;
    } else {
            state.v.fixall = 1;
    }
}

The alias handling in Aprx for TRACE vs WIDE is a total mess. I plan to replace all of it with a simpler system that only supports traced aliases instead of trying to fix it.

PhirePhly commented 7 years ago

It would seem that source block alias limit definitions aren't correctly parsed... digipeater block definitions are...

This leaves all the limits at the default four:

<source>
   source         $mycall
   viscous-delay 1
   <trace>
     maxreq      3
     maxdone     3
     keys        WIDE,TRACE
   </trace>
   <wide>
     maxreq        7
     maxdone       7
     keys          OH
   </wide>
</source>

This! This actually changes the limits:

<digipeater>
   <trace>
     maxreq      3
     maxdone     3
     keys        WIDE,TRACE
   </trace>
   <wide>
     maxreq        7
     maxdone       7
     keys          OH
   </wide>
  <source>
    source         $mycall
   viscous-delay 1
  </source>
</digipeater>

But this still leaves the issue that the trap is broken. Seems like the errant condition is the first one, state.v.hopsreq > digi->trace->maxreq. hopsreq is the limit for wide aliases, but trace->maxreq is the requested hops for trace aliases; this is half of each of the next two conditions... Removing the first condition seems to have fixed the whole kit and kaboodle.

  if (
    state.v.hopsreq   > digi->wide->maxreq   ||
    state.v.tracereq  > digi->trace->maxreq  ||
    state.v.digidone  > digi->trace->maxdone ||
    state.v.digidone  > digi->wide->maxdone  ||
    state.v.hopsdone  > digi->trace->maxdone ||
    state.v.hopsdone  > digi->wide->maxdone  ||
    state.v.tracedone > digi->trace->maxdone) {