[LEDE-DEV] [PATCH 1/2] ltq-atm: rewrite tx path to use IRQs

[Antti Seppälä]

On 8 January 2018 at 10:54, Alexander Couzens <lynxis at fe80.eu> wrote:
> The ATM subsystem is different from the generic ethernet NICs. The ATM
> subsystem requires a callback when a packet has been sent. It means a
> tx skb_buff need to be used after it has sent. While the generic NIC
> can fill up the TX ring and free skb_buffs if it encounter a ring buffer slot
> with an already sent skbuff.
> The ATM drivers need call the pop() function after it has send a
> single ATM package. The ATM subsystem controls via this ways the queuing.
>
> The ppe engine use DMA channels for read and write. Every atm_vcc has it's
> own TX DMA channel and each TX DMA channel has it's own ring buffer.
>
> The old driver had multiple issues:
> - Call the subsystem callback at the beginning of tx function (ppe_send).
>   Didn't allowed the ATM subsystem to control the enqueued package
>   amount.
> - Filled up the TX ring until full and fail futher
> - copy or decouple the skb from all other subsystem before giving it
>   over to TX ring
>
> The new tx path uses interupts.
> - call the subsystem callback _after_ it was sent by hardware
> - no need to copy our decouple the skb any more
> - gives back control to the atm subsystem over the enqueued packages
> - use an interupt for every sent atm package
>
> Using interupts shouldn't be a problem because of the slow uplink bandwidth of
> ADSL.
> The speed _through_ the DSL router was always as high as it should
> be, only traffic generated on the router itself were affected.
>
> After changing to new tx path, the speed of iperf's run on the
> router itself reached the same speed. The master/trunk wasn't as much
> affected because of TCP optimisations (reboot-5022-gb2ea46fe236a).
> The following results are taken on the remote server, which receives
> the stream over the internet and the DSL line.
>
> The sync moves between every sync a litte bit, but is so far stable
> Latency / Interleave Delay:               Down: Fast (0.25 ms) / Up: Fast (0.50 ms)
> Data Rate:                                Down: 13.287 Mb/s / Up: 1.151 Mb/s
>
> reboot-5521-g9f8d28285d without patch
> [ ID] Interval           Transfer     Bandwidth       Retr
> [  5]   0.00-10.04  sec   947 KBytes   773 Kbits/sec    0             sender
> [  5]   0.00-10.04  sec   928 KBytes   757 Kbits/sec                  receiver
>
> reboot-5521-g9f8d28285d with patch
> [  5]   0.00-10.06  sec  1.16 MBytes   970 Kbits/sec    0             sender
> [  5]   0.00-10.06  sec  1.15 MBytes   959 Kbits/sec                  receiver
>
> v17.01.4-239-g55c23e44f4 without patch
> [ ID] Interval           Transfer     Bandwidth       Retr
> [  5]   0.00-10.04  sec  87.4 KBytes  71.3 Kbits/sec    0             sender
> [  5]   0.00-10.04  sec  59.6 KBytes  48.7 Kbits/sec                  receiver
>
> v17.01.4-239-g55c23e44f4 with patch
> [ ID] Interval           Transfer     Bandwidth       Retr
> [  5]   0.00-10.05  sec  1.18 MBytes   983 Kbits/sec    1             sender
> [  5]   0.00-10.05  sec  1.15 MBytes   959 Kbits/sec                  receiver
>
> Signed-off-by: Alexander Couzens <lynxis at fe80.eu>

I tested this and am happy to report that the patch seems to fully
solve the annoying connection lag reported in LEDE flyspray
(https://bugs.lede-project.org/index.php?do=details&task_id=105) that
I've also been experiencing for quite a while now in one of my
devices.

Many thanks for your efforts.

> ---
>  .../kernel/lantiq/ltq-atm/src/ifxmips_atm_core.h   |   2 +
>  package/kernel/lantiq/ltq-atm/src/ltq_atm.c        | 132 ++++++++++++++-------
>  2 files changed, 88 insertions(+), 46 deletions(-)
>
> diff --git a/package/kernel/lantiq/ltq-atm/src/ifxmips_atm_core.h b/package/kernel/lantiq/ltq-atm/src/ifxmips_atm_core.h
> index 2f754c982b2a..398be7d8282a 100644
> --- a/package/kernel/lantiq/ltq-atm/src/ifxmips_atm_core.h
> +++ b/package/kernel/lantiq/ltq-atm/src/ifxmips_atm_core.h
> @@ -1799,7 +1840,6 @@ static int ltq_atm_probe(struct platform_device *pdev)
>         int ret;
>         int port_num;
>         struct port_cell_info port_cell = {0};
> -       int i, j;

One small nit about the patch though - the above change seems
unintended and causes a compilation failure.

-- 
Antti