[Asterisk-Dev] libpri LAPD T200 timer

[Liam Kenny]

Hi,

Im using the E100P cards with libpri (Just Q921) to run X25.
I came across an issue with the Q921 implementation I thought
Id share:

Under sustained data transfer using q921_transmit_iframe, libpri
soon gets itself into a state where it starts spuriously retransmitting
I frames, resulting in a major degradation in the throughput.

The problem is as follows:

Under sustained data transfer, pri->txqueue may constantly be populated
by a minimum of 2 packets awaiting acknowlegement. The T200 timer
guards against acknowledgement not being received and triggers 
retransmission of packets in the pri->txqueue when it expires.
The problem is that the T200 timer is only stopped (in q921_ack_rx)
when the pri->txqueue becomes empty. Under sustained data transfer
this list may constantly contain packets which means the T200 will
expire in error.

I think there may be some further issues once this retransmission state
is entered (my thoughtput went down much further than I would have
expected), but I didnt investigate any further.

A quick (and dirty?) fix in my case was to always reset T200 if the 
oldest packet in pri->txqueue was acknowledged. A more complete
solution might be to timestamp each packet in the txqueue, so that 
when T200 is restarted, it can be set to the time left of the now oldest
packet in the txqueue.

My fix was:

static void q921_ack_rx(struct pri_q921inst *pri, int ack)
{
        int x;
        int cnt=0;
        q921_frame *oldestPacket = pri->txqueue;

        .
        .
        .

        /* Added a second condition to the existing if */
        if (!pri->txqueue || pri->txqueue != oldestPacket) {
                if (pri->pri->debug &  PRI_DEBUG_Q921_STATE)
                        pri_message("-- Stopping T200 counter\n");
                /* Something was ACK'd.  Stop T200 counter */
                pri_schedule_del(pri->pri, pri->t200_timer);
                pri->t200_timer = 0;
        }

        .
        .
        .

}

Most people will probably never encounter this issue as the Q931
traffic is typically light enough not to exercise the problem.

Any comments ?
Liam.