greearb / ath10k-ct

Stand-alone ath10k driver based on Candela Technologies Linux kernel.
111 stars 41 forks source link

Unchainig A-MSDU is not implemented #86

Open socketpair opened 5 years ago

socketpair commented 5 years ago

I've made a patch:

--- a/ath10k-4.16/htt_rx.c
+++ b/ath10k-4.16/htt_rx.c
@@ -25,6 +25,12 @@
.
 #include <linux/log2.h>
.
+/* For `ath10k_htt_rx_h_unchain` frame-drop logging */
+#include <linux/jiffies.h>
+#include <linux/kern_levels.h>
+#include <linux/printk.h>
+#include <linux/ratelimit.h>
+
 /* when under memory pressure rx ring refill may fail and needs a retry */
 #define HTT_RX_RING_REFILL_RETRY_MS 50
.
@@ -1731,6 +1737,21 @@ static int ath10k_unchain_msdu(struct at
        return 0;
 }
.
+static inline const char *ideco_get_decap_format_str(enum rx_msdu_decap_format decap)
+{
+       switch (decap) {
+       case RX_MSDU_DECAP_RAW:
+               return "rx_msdu_decap_raw";
+       case RX_MSDU_DECAP_NATIVE_WIFI:
+               return "rx_msdu_decap_native_wifi";
+       case RX_MSDU_DECAP_ETHERNET2_DIX:
+               return "rx_msdu_decap_ethernet2_dix";
+       case RX_MSDU_DECAP_8023_SNAP_LLC:
+               return "rx_msdu_decap_8023_snap_llc";
+       }
+       return "unknown_decap_format";
+}
+
 static void ath10k_htt_rx_h_unchain(struct ath10k *ar,
                                    struct sk_buff_head *amsdu)
 {
@@ -1750,9 +1771,28 @@ static void ath10k_htt_rx_h_unchain(stru
         */
        if (decap != RX_MSDU_DECAP_RAW ||
            skb_queue_len(amsdu) != 1 + rxd->frag_info.ring2_more_count) {
+               static DEFINE_RATELIMIT_STATE(state, 5 * HZ, 10);
 #ifdef CONFIG_ATH10K_DEBUGFS
                ar->debug.rx_drop_decap_non_raw_chained++;
 #endif
+               if (___ratelimit(&state, __func__)) {
+                       /*
+                               We prefer to use `printk_emit` over standard `printk` since it allows more control over
+                               logging behaviour. For instance `printk` always invokes console driver.
+                               The latter eventually calls UART driver which is always registered on system start-up.
+                               On the other hand `LOGLEVEL_SCHED` skips console driver invokation.
+                               So it can save a lot of time in atomic context.
+                       */
+                       printk_emit(0, /* facility */
+                                   LOGLEVEL_SCHED, /* level */
+                                   NULL, /* dict */
+                                   0, /* dictlen */
+                                   KERN_WARNING "%s: %s: dropped %u chained frames with decap format = \"%s\"\n",
+                                   KBUILD_MODNAME,
+                                   __func__,
+                                   skb_queue_len(amsdu),
+                                   ideco_get_decap_format_str(decap));
+               }
                __skb_queue_purge(amsdu);
                return;
        }

And it shows that many packages (especially on QCA9884) fall into this logging. In the same time I have zero throughput. Also, If traffic is not fast everything works fine. Seems, someone merges packets to A-MSDU form, but driver is not able to unchain them. Does not it ? What is unchainig ? splitting A-MSDU to separate MSDUs ?

This happens on both vanilla and -ct firmware. Driver is -ct. I use 80211.s interfaces. This happen only during benchmarks (i.e. massive traffic amount) and makes the whole transfer unavailable (i.e. 0 bytes/sec in iperf)

socketpair commented 5 years ago

Driver has a comment:

/* FIXME: Current unchaining logic can only handle simple case of raw
 * msdu chaining. If decapping is other than raw the chaining may be
 * more complex and this isn't handled by the current code. Don't even
 * try re-constructing such frames - it'll be pretty much garbage.
 */

So, Is there any workaround for my problem ?

greearb commented 5 years ago

What is the decap type in the case that fails? You only see this with .11s traffic, or you can reproduce it with AP/STA?

socketpair commented 5 years ago

@greearb RX_MSDU_DECAP_NATIVE_WIFI. I can not check right now (will check if you insist on), but as far as I remember, yes, it reproduces with AP mode.

greearb commented 5 years ago

I don't have a good way to test .11s, but if it is reproducible in AP/STA mode then maybe I can work on it. Please do test in AP/STA mode to make sure it is reproducible. Also, maybe add WARN_ON_ONCE to your error checking code and post the dmesg output of a few of the printk_emits and splat.

greearb commented 5 years ago

I did some tests today, and I see zero movement in that counter, but then I realized we always use 'raw' mode for RX path, so we would not see this error. This is likely not an issue specific to my driver, so maybe you could try reproducing with stock firmware/driver, and then report a bug to the ath10k mailing list and see if they can help fix it? I could back port a working patch into my driver easily enough most likely.

socketpair commented 5 years ago

@greearb

https://github.com/torvalds/linux/blob/9c7db5004280767566e91a33445bf93aa479ef02/drivers/net/wireless/ath/ath10k/core.c#L2220

Maybe this line should be changed ?

Also, seems echo 1 64 > /sys/kernel/debug/ieee80211/phy0/ath1 0k/htt_max_amsdu_ampdu does not work.

greearb commented 5 years ago

I am not sure stock firmware can change just the rx-encap, but ct-firmware can. We do this so we can send native-wifi and receive un-decrypted raw frames for our network testing solution. I think this feature is probably only available in our paid-commercial-use firmware variant since the main use of this feature is network testing. There were some upstream patches posted a month or so ago to the ath10k mailing list that enabled 'eth' encap. Supposedly this improved performance quite a bit. Maybe see if you can find and use those?

Mirraz commented 5 years ago

Hello. I'm working on the same problem with @socketpair

I don't have a good way to test .11s, but if it is reproducible in AP/STA mode then maybe I can work on it. Please do test in AP/STA mode to make sure it is reproducible.

I've reproduced this issue in AP/STA mode:

I start iperf-clinet on one device:

# iperf3 -c fe80::7080:cdff:fe48:41e2%sta_1
Connecting to host fe80::7080:cdff:fe48:41e2%sta_1, port 5201
[  5] local fe80::cc6f:82ff:fe26:7ed2 port 50804 connected to fe80::7080:cdff:fe48:41e2 port 5201
[ ID] Interval           Transfer     Bitrate         Retr  Cwnd
[  5]   0.00-1.01   sec  15.7 MBytes   131 Mbits/sec    0    248 KBytes       
[  5]   1.01-2.00   sec  19.8 MBytes   167 Mbits/sec    0    339 KBytes       
[  5]   2.00-3.01   sec  18.6 MBytes   155 Mbits/sec    0    357 KBytes       
[  5]   3.01-4.01   sec  18.8 MBytes   157 Mbits/sec    0    375 KBytes       
[  5]   4.01-5.00   sec  18.3 MBytes   155 Mbits/sec    0    417 KBytes       
[  5]   5.00-6.00   sec  16.6 MBytes   139 Mbits/sec    0    438 KBytes       
[  5]   6.00-7.03   sec  18.0 MBytes   148 Mbits/sec    0    469 KBytes       
[  5]   7.03-8.00   sec  17.7 MBytes   153 Mbits/sec    0    491 KBytes       
[  5]   8.00-9.00   sec  19.2 MBytes   161 Mbits/sec    0    491 KBytes       
[  5]   9.00-10.00  sec  19.2 MBytes   161 Mbits/sec    0    491 KBytes       
- - - - - - - - - - - - - - - - - - - - - - - - -
[ ID] Interval           Transfer     Bitrate         Retr
[  5]   0.00-10.00  sec   182 MBytes   153 Mbits/sec    0             sender
[  5]   0.00-10.04  sec   182 MBytes   152 Mbits/sec                  receiver

After this on other device (where iperf-server is working) I see these messages in dmesg:

[116072.253823] ath10k_core: ath10k_htt_rx_h_unchain: dropped 2 chained frames with decap format = "rx_msdu_decap_native_wifi"
[116073.162316] ath10k_core: ath10k_htt_rx_h_unchain: dropped 2 chained frames with decap format = "rx_msdu_decap_native_wifi"
[116073.363637] ath10k_core: ath10k_htt_rx_h_unchain: dropped 2 chained frames with decap format = "rx_msdu_decap_native_wifi"
[116073.583169] ath10k_core: ath10k_htt_rx_h_unchain: dropped 3 chained frames with decap format = "rx_msdu_decap_native_wifi"
[116073.685856] ath10k_core: ath10k_htt_rx_h_unchain: dropped 2 chained frames with decap format = "rx_msdu_decap_native_wifi"
[116073.936458] ath10k_core: ath10k_htt_rx_h_unchain: dropped 2 chained frames with decap format = "rx_msdu_decap_native_wifi"
[116074.035705] ath10k_core: ath10k_htt_rx_h_unchain: dropped 2 chained frames with decap format = "rx_msdu_decap_native_wifi"
[116074.035747] ath10k_core: ath10k_htt_rx_h_unchain: dropped 2 chained frames with decap format = "rx_msdu_decap_native_wifi"
[116074.035847] ath10k_core: ath10k_htt_rx_h_unchain: dropped 2 chained frames with decap format = "rx_msdu_decap_native_wifi"
[116074.035856] ath10k_core: ath10k_htt_rx_h_unchain: dropped 2 chained frames with decap format = "rx_msdu_decap_native_wifi"
[116077.273218] ath10k_htt_rx_h_unchain: 20 callbacks suppressed
[116077.273226] ath10k_core: ath10k_htt_rx_h_unchain: dropped 4 chained frames with decap format = "rx_msdu_decap_native_wifi"
[116077.422415] ath10k_core: ath10k_htt_rx_h_unchain: dropped 2 chained frames with decap format = "rx_msdu_decap_native_wifi"
[116077.926027] ath10k_core: ath10k_htt_rx_h_unchain: dropped 2 chained frames with decap format = "rx_msdu_decap_native_wifi"
[116078.053296] ath10k_core: ath10k_htt_rx_h_unchain: dropped 2 chained frames with decap format = "rx_msdu_decap_native_wifi"
[116078.150566] ath10k_core: ath10k_htt_rx_h_unchain: dropped 2 chained frames with decap format = "rx_msdu_decap_native_wifi"
[116078.209935] ath10k_core: ath10k_htt_rx_h_unchain: dropped 2 chained frames with decap format = "rx_msdu_decap_native_wifi"
[116078.410997] ath10k_core: ath10k_htt_rx_h_unchain: dropped 2 chained frames with decap format = "rx_msdu_decap_native_wifi"
[116078.881709] ath10k_core: ath10k_htt_rx_h_unchain: dropped 2 chained frames with decap format = "rx_msdu_decap_native_wifi"
[116079.024767] ath10k_core: ath10k_htt_rx_h_unchain: dropped 2 chained frames with decap format = "rx_msdu_decap_native_wifi"
[116079.026693] ath10k_core: ath10k_htt_rx_h_unchain: dropped 2 chained frames with decap format = "rx_msdu_decap_native_wifi"

But it seems it is not affecting throughput very much in AP/STA mode comparing to tremendous throughput drop in MESH mode.

greearb commented 5 years ago

I think you should reproduce this with upstream firmware/driver and report to ath10k mailing list. They might can fix it properly and for free.