LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* v5.5-rc1 and beyond insta-kills some Comcast wifi routers
@ 2020-03-04  3:34 Mancini, Jason
  2020-03-04  4:05 ` Randy Dunlap
  0 siblings, 1 reply; 7+ messages in thread
From: Mancini, Jason @ 2020-03-04  3:34 UTC (permalink / raw)
  To: linux-kernel

[I can't seem to access the linux-net ml per kernel.org faq, apology
in advance.]

This change, which I think first appeared for v5.5-rc1, basically
within seconds, knocks out our [apparently buggy] Comcast wifi for
about 2-3 minutes.  Is there a boot option (or similar) where I can
achieve prior kernel behavior?  Otherwise I am stuck on kernel 5.4
(or Win10) it seems, or forever compiling custom kernels for my
choice of distribution [as I don't have physical access to the router
in question.]
Thanks!
Jason

================

127eef1d46f80056fe9f18406c6eab38778d8a06 is the first bad commit
commit 127eef1d46f80056fe9f18406c6eab38778d8a06
Author: Yan-Hsuan Chuang <yhchuang@realtek.com>
Date:   Wed Oct 2 14:35:23 2019 +0800

    rtw88: add TX-AMSDU support

    Based on the mac80211's TXQ implementation, TX-AMSDU can
    be used to get higher MAC efficiency. To make mac80211
    aggregate MSDUs, low level driver just need to leave skbs
    in the TXQ, and mac80211 will try to aggregate them if
    possible. As driver will schedule a tasklet when the TX
    queue is woke, until the tasklet being served, there will
    have some skbs in the queue if traffic is heavy.

    Driver can control the max AMSDU size depending on the
    current bit rate used by hardware/firmware. The higher
    rates are used, the larger AMSDU size can be.

    It is tested that can achieve higher T-Put at higher rates.
    If the environment is relatively clean, and the bit_rate
    is high enough, we can get about 80Mbps improvement.

    For lower bit rates, not much gain can we get, so leave
    the max_amsdu length low to prevent aggregation.

    Signed-off-by: Yan-Hsuan Chuang <yhchuang@realtek.com>
    Signed-off-by: Kalle Valo <kvalo@codeaurora.org>

 drivers/net/wireless/realtek/rtw88/fw.c   | 24 ++++++++++++++++++++++++
 drivers/net/wireless/realtek/rtw88/main.c |  1 +
 2 files changed, 25 insertions(+)

------------------- drivers/net/wireless/realtek/rtw88/fw.c -------------------
index 4b41bf531998..51649df7cc98 100644
@@ -29,6 +29,28 @@ static void rtw_fw_c2h_cmd_handle_ext(struct rtw_dev *rtwdev,
        }
 }
 
+static u16 get_max_amsdu_len(u32 bit_rate)
+{
+       /* lower than ofdm, do not aggregate */
+       if (bit_rate < 550)
+               return 1;
+
+       /* lower than 20M 2ss mcs8, make it small */
+       if (bit_rate < 1800)
+               return 1200;
+
+       /* lower than 40M 2ss mcs9, make it medium */
+       if (bit_rate < 4000)
+               return 2600;
+
+       /* not yet 80M 2ss mcs8/9, make it twice regular packet size */
+       if (bit_rate < 7000)
+               return 3500;
+
+       /* unlimited */
+       return 0;
+}
+
 struct rtw_fw_iter_ra_data {
        struct rtw_dev *rtwdev;
        u8 *payload;
@@ -83,6 +105,8 @@ static void rtw_fw_ra_report_iter(void *data, struct ieee80211_sta *sta)
 
        si->ra_report.desc_rate = rate;
        si->ra_report.bit_rate = bit_rate;
+
+       sta->max_rc_amsdu_len = get_max_amsdu_len(bit_rate);
 }
 
 static void rtw_fw_ra_report_handle(struct rtw_dev *rtwdev, u8 *payload,

------------------ drivers/net/wireless/realtek/rtw88/main.c ------------------
index 690a5c4d64e7..f7044e8bcb5b 100644
@@ -1310,6 +1310,7 @@ int rtw_register_hw(struct rtw_dev *rtwdev, struct ieee80211_hw *hw)
        ieee80211_hw_set(hw, SUPPORT_FAST_XMIT);
        ieee80211_hw_set(hw, SUPPORTS_AMSDU_IN_AMPDU);
        ieee80211_hw_set(hw, HAS_RATE_CONTROL);
+       ieee80211_hw_set(hw, TX_AMSDU);
 
        hw->wiphy->interface_modes = BIT(NL80211_IFTYPE_STATION) |
                                     BIT(NL80211_IFTYPE_AP) |

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: v5.5-rc1 and beyond insta-kills some Comcast wifi routers
  2020-03-04  3:34 v5.5-rc1 and beyond insta-kills some Comcast wifi routers Mancini, Jason
@ 2020-03-04  4:05 ` Randy Dunlap
  2020-03-04  5:05   ` Kalle Valo
  0 siblings, 1 reply; 7+ messages in thread
From: Randy Dunlap @ 2020-03-04  4:05 UTC (permalink / raw)
  To: Mancini, Jason, linux-kernel, netdev, Yan-Hsuan Chuang, Kalle Valo

[add netdev mailing list + 2 patch signers]

On 3/3/20 7:34 PM, Mancini, Jason wrote:
> [I can't seem to access the linux-net ml per kernel.org faq, apology
> in advance.]
> 
> This change, which I think first appeared for v5.5-rc1, basically
> within seconds, knocks out our [apparently buggy] Comcast wifi for
> about 2-3 minutes.  Is there a boot option (or similar) where I can
> achieve prior kernel behavior?  Otherwise I am stuck on kernel 5.4
> (or Win10) it seems, or forever compiling custom kernels for my
> choice of distribution [as I don't have physical access to the router
> in question.]
> Thanks!
> Jason
> 
> ================
> 
> 127eef1d46f80056fe9f18406c6eab38778d8a06 is the first bad commit
> commit 127eef1d46f80056fe9f18406c6eab38778d8a06
> Author: Yan-Hsuan Chuang <yhchuang@realtek.com>
> Date:   Wed Oct 2 14:35:23 2019 +0800
> 
>     rtw88: add TX-AMSDU support
> 
>     Based on the mac80211's TXQ implementation, TX-AMSDU can
>     be used to get higher MAC efficiency. To make mac80211
>     aggregate MSDUs, low level driver just need to leave skbs
>     in the TXQ, and mac80211 will try to aggregate them if
>     possible. As driver will schedule a tasklet when the TX
>     queue is woke, until the tasklet being served, there will
>     have some skbs in the queue if traffic is heavy.
> 
>     Driver can control the max AMSDU size depending on the
>     current bit rate used by hardware/firmware. The higher
>     rates are used, the larger AMSDU size can be.
> 
>     It is tested that can achieve higher T-Put at higher rates.
>     If the environment is relatively clean, and the bit_rate
>     is high enough, we can get about 80Mbps improvement.
> 
>     For lower bit rates, not much gain can we get, so leave
>     the max_amsdu length low to prevent aggregation.
> 
>     Signed-off-by: Yan-Hsuan Chuang <yhchuang@realtek.com>
>     Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
> 
>  drivers/net/wireless/realtek/rtw88/fw.c   | 24 ++++++++++++++++++++++++
>  drivers/net/wireless/realtek/rtw88/main.c |  1 +
>  2 files changed, 25 insertions(+)
> 
> ------------------- drivers/net/wireless/realtek/rtw88/fw.c -------------------
> index 4b41bf531998..51649df7cc98 100644
> @@ -29,6 +29,28 @@ static void rtw_fw_c2h_cmd_handle_ext(struct rtw_dev *rtwdev,
>         }
>  }
>  
> +static u16 get_max_amsdu_len(u32 bit_rate)
> +{
> +       /* lower than ofdm, do not aggregate */
> +       if (bit_rate < 550)
> +               return 1;
> +
> +       /* lower than 20M 2ss mcs8, make it small */
> +       if (bit_rate < 1800)
> +               return 1200;
> +
> +       /* lower than 40M 2ss mcs9, make it medium */
> +       if (bit_rate < 4000)
> +               return 2600;
> +
> +       /* not yet 80M 2ss mcs8/9, make it twice regular packet size */
> +       if (bit_rate < 7000)
> +               return 3500;
> +
> +       /* unlimited */
> +       return 0;
> +}
> +
>  struct rtw_fw_iter_ra_data {
>         struct rtw_dev *rtwdev;
>         u8 *payload;
> @@ -83,6 +105,8 @@ static void rtw_fw_ra_report_iter(void *data, struct ieee80211_sta *sta)
>  
>         si->ra_report.desc_rate = rate;
>         si->ra_report.bit_rate = bit_rate;
> +
> +       sta->max_rc_amsdu_len = get_max_amsdu_len(bit_rate);
>  }
>  
>  static void rtw_fw_ra_report_handle(struct rtw_dev *rtwdev, u8 *payload,
> 
> ------------------ drivers/net/wireless/realtek/rtw88/main.c ------------------
> index 690a5c4d64e7..f7044e8bcb5b 100644
> @@ -1310,6 +1310,7 @@ int rtw_register_hw(struct rtw_dev *rtwdev, struct ieee80211_hw *hw)
>         ieee80211_hw_set(hw, SUPPORT_FAST_XMIT);
>         ieee80211_hw_set(hw, SUPPORTS_AMSDU_IN_AMPDU);
>         ieee80211_hw_set(hw, HAS_RATE_CONTROL);
> +       ieee80211_hw_set(hw, TX_AMSDU);
>  
>         hw->wiphy->interface_modes = BIT(NL80211_IFTYPE_STATION) |
>                                      BIT(NL80211_IFTYPE_AP) |
> 


-- 
~Randy
Reported-by: Randy Dunlap <rdunlap@infradead.org>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: v5.5-rc1 and beyond insta-kills some Comcast wifi routers
  2020-03-04  4:05 ` Randy Dunlap
@ 2020-03-04  5:05   ` Kalle Valo
  2020-03-04  5:16     ` Tony Chuang
  0 siblings, 1 reply; 7+ messages in thread
From: Kalle Valo @ 2020-03-04  5:05 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: Mancini, Jason, linux-kernel, netdev, Yan-Hsuan Chuang, linux-wireless

Randy Dunlap <rdunlap@infradead.org> writes:

> [add netdev mailing list + 2 patch signers]

Adding also linux-wireless. It's always best to send questions about any
wireless issues to linux-wireless

> On 3/3/20 7:34 PM, Mancini, Jason wrote:
>> [I can't seem to access the linux-net ml per kernel.org faq, apology
>> in advance.]
>> 
>> This change, which I think first appeared for v5.5-rc1, basically
>> within seconds, knocks out our [apparently buggy] Comcast wifi for
>> about 2-3 minutes.  Is there a boot option (or similar) where I can
>> achieve prior kernel behavior?  Otherwise I am stuck on kernel 5.4
>> (or Win10) it seems, or forever compiling custom kernels for my
>> choice of distribution [as I don't have physical access to the router
>> in question.]
>> Thanks!
>> Jason
>> 
>> ================
>> 
>> 127eef1d46f80056fe9f18406c6eab38778d8a06 is the first bad commit
>> commit 127eef1d46f80056fe9f18406c6eab38778d8a06
>> Author: Yan-Hsuan Chuang <yhchuang@realtek.com>
>> Date:   Wed Oct 2 14:35:23 2019 +0800

Can you try if this fixes it:

https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers-next.git/commit/?id=74c3d72cc13401f9eb3e3c712855e9f8f2d2682b

-- 
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: v5.5-rc1 and beyond insta-kills some Comcast wifi routers
  2020-03-04  5:05   ` Kalle Valo
@ 2020-03-04  5:16     ` Tony Chuang
  2020-03-04  9:03       ` Mancini, Jason
  0 siblings, 1 reply; 7+ messages in thread
From: Tony Chuang @ 2020-03-04  5:16 UTC (permalink / raw)
  To: Kalle Valo, Randy Dunlap
  Cc: Mancini, Jason, linux-kernel, netdev, linux-wireless

Kalle Valo <kvalo@codeaurora.org> writes:
> 
> Randy Dunlap <rdunlap@infradead.org> writes:
> 
> > [add netdev mailing list + 2 patch signers]
> 
> Adding also linux-wireless. It's always best to send questions about any
> wireless issues to linux-wireless
> 
> > On 3/3/20 7:34 PM, Mancini, Jason wrote:
> >> [I can't seem to access the linux-net ml per kernel.org faq, apology
> >> in advance.]
> >>
> >> This change, which I think first appeared for v5.5-rc1, basically
> >> within seconds, knocks out our [apparently buggy] Comcast wifi for
> >> about 2-3 minutes.  Is there a boot option (or similar) where I can
> >> achieve prior kernel behavior?  Otherwise I am stuck on kernel 5.4
> >> (or Win10) it seems, or forever compiling custom kernels for my
> >> choice of distribution [as I don't have physical access to the router
> >> in question.]
> >> Thanks!
> >> Jason
> >>
> >> ================
> >>
> >> 127eef1d46f80056fe9f18406c6eab38778d8a06 is the first bad commit
> >> commit 127eef1d46f80056fe9f18406c6eab38778d8a06
> >> Author: Yan-Hsuan Chuang <yhchuang@realtek.com>
> >> Date:   Wed Oct 2 14:35:23 2019 +0800
> 
> Can you try if this fixes it:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers-next.gi
> t/commit/?id=74c3d72cc13401f9eb3e3c712855e9f8f2d2682b
> 

Kalle is providing the right possible patch to fix it.

The first bad commit you found, that causes this issue, introduced TX-AMSDU.
But we found that enabling TX-AMSDU on 2.4G band is not working while
connecting to some APs. So, you can try if the patch provided by Kalle works.
(I hope so). Otherwise, you can enable the kernel log debug mask by:
echo 0xffffffff > /sys/module/rtw88/parameters/debug_mask.
And collect the log to see if there's anything wrong.

Yen-Hsuan

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: v5.5-rc1 and beyond insta-kills some Comcast wifi routers
  2020-03-04  5:16     ` Tony Chuang
@ 2020-03-04  9:03       ` Mancini, Jason
  2020-03-04 10:41         ` Tony Chuang
  0 siblings, 1 reply; 7+ messages in thread
From: Mancini, Jason @ 2020-03-04  9:03 UTC (permalink / raw)
  To: Tony Chuang, Kalle Valo, Randy Dunlap
  Cc: linux-kernel, netdev, linux-wireless

[AMD Official Use Only - Internal Distribution Only]

I tested Kalle's patch.  Laptop connects via 5GHz band by default.  Comcast router still
crashed in a hurry.  I blocked (via NM.conf) the 5GHz mac of the router, and rebooted
the laptop. Checked that the router was using 2.4 for the laptop.  Still hung the router!

What I've done temporarily is change the unlimited return value from 0 to 4000.
Somewhere around 5325 the Comcast router gets cranky/weird, and at 5350 it is
resetting the wifi stack (without resetting the entire router).

So there's no boot time flag to turn the feature off currently?

Jason

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: v5.5-rc1 and beyond insta-kills some Comcast wifi routers
  2020-03-04  9:03       ` Mancini, Jason
@ 2020-03-04 10:41         ` Tony Chuang
  2020-03-05  7:06           ` Jason Mancini
  0 siblings, 1 reply; 7+ messages in thread
From: Tony Chuang @ 2020-03-04 10:41 UTC (permalink / raw)
  To: Mancini, Jason, Kalle Valo, Randy Dunlap
  Cc: linux-kernel, netdev, linux-wireless

> [AMD Official Use Only - Internal Distribution Only]
> 
> I tested Kalle's patch.  Laptop connects via 5GHz band by default.  Comcast
> router still
> crashed in a hurry.  I blocked (via NM.conf) the 5GHz mac of the router, and
> rebooted
> the laptop. Checked that the router was using 2.4 for the laptop.  Still hung
> the router!
> 
> What I've done temporarily is change the unlimited return value from 0 to
> 4000.
> Somewhere around 5325 the Comcast router gets cranky/weird, and at 5350
> it is
> resetting the wifi stack (without resetting the entire router).
> 
> So there's no boot time flag to turn the feature off currently?
> 

Unfortunately, no, there's no flag to turn off this.

But, from your experiments, if you applied that patch,
("rtw88: disable TX-AMSDU on 2.4G band") connect to AP on 2.4G, and still crash
the Comcast AP, then it looks like it's not TX-AMSDU to be blamed.

Assume the return value you mentioned is max_rc_amsdu_len, if you always
return 1, it will just disable all of the AMSDU process.
You can try it, and to see if sending AMSDU will crash the router or not.

Yen-Hsuan

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: v5.5-rc1 and beyond insta-kills some Comcast wifi routers
  2020-03-04 10:41         ` Tony Chuang
@ 2020-03-05  7:06           ` Jason Mancini
  0 siblings, 0 replies; 7+ messages in thread
From: Jason Mancini @ 2020-03-05  7:06 UTC (permalink / raw)
  To: Tony Chuang, Kalle Valo, Randy Dunlap
  Cc: linux-kernel, netdev, linux-wireless

On 3/4/20 2:41 AM, Tony Chuang wrote:
> Unfortunately, no, there's no flag to turn off this.
> But, from your experiments, if you applied that patch,
> ("rtw88: disable TX-AMSDU on 2.4G band") connect to AP on 2.4G, and still crash
> the Comcast AP, then it looks like it's not TX-AMSDU to be blamed.
>
> Assume the return value you mentioned is max_rc_amsdu_len, if you always
> return 1, it will just disable all of the AMSDU process.
> You can try it, and to see if sending AMSDU will crash the router or not.
>
> Yen-Hsuan

(Specifically, this Comcast router is "Arris TG1682G" firmware 10.1.27B.SIP.PC20.CT hardware version 9.0)

I re-tested tonight, here are the results, from *unpatched* kernels:

(1) 2.4G only w/5G disabled via router control panel: kernel 5.5/5.6 seemingly doesn't upset router.
(2) 5G only w/2.4G disabled via router control panel: kernel 5.5/5.6 definitely kill router wifi.
(3) 5G only w/2.4G disabled via router control panel: plus get_max_amsdu_len forced to return 1: kernel 5.6-rc4 seemingly doesn't upset router.

As you can see, the suggested patch isn't going to help result (2), and apparently isn't needed for (1).  And this router's 5G seems
allergic to amsdu per (3), so somehow amsdu is involved it seems.

Well, I'll just work around it with (3) custom kernels, or (1) leave the router in 2.4G mode.  But be aware that apparently there's at least
one common buggy wifi router that's going to puke on 5G + amsdu.

Jason


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2020-03-05  7:07 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-04  3:34 v5.5-rc1 and beyond insta-kills some Comcast wifi routers Mancini, Jason
2020-03-04  4:05 ` Randy Dunlap
2020-03-04  5:05   ` Kalle Valo
2020-03-04  5:16     ` Tony Chuang
2020-03-04  9:03       ` Mancini, Jason
2020-03-04 10:41         ` Tony Chuang
2020-03-05  7:06           ` Jason Mancini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).