* Re: [PATCH 1/3] mt76: remove redundant mt76_txq_schedule_all
From: Felix Fietkau @ 2019-08-23 9:27 UTC (permalink / raw)
To: Stanislaw Gruszka, linux-wireless; +Cc: Lorenzo Bianconi, Ryder Lee, Roy Luo
In-Reply-To: <1566550337-6287-2-git-send-email-sgruszka@redhat.com>
On 2019-08-23 10:52, Stanislaw Gruszka wrote:
> Waking tx queues will cause that txq's will be scheduled. Calling
> mt76_txq_schedule_all() while queues are blocked is not necessary.
> We will not get any skb's from ieee80211_tx_dequeue() anyway, but
> patch changes that transmit of mtxq->retry_q skb's will be a bit
> deferred (on the moment after channel switch or other situation
> when we wake up queues).
>
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
Waking tx queues will not always cause txqs to be scheduled - only if an
attempt to dequeue was blocked because queues were stopped at that time.
Because of that, I don't think this patch is correct.
- Felix
^ permalink raw reply
* Re: [PATCH 1/3] mt76: remove redundant mt76_txq_schedule_all
From: Stanislaw Gruszka @ 2019-08-23 10:20 UTC (permalink / raw)
To: Felix Fietkau; +Cc: linux-wireless, Lorenzo Bianconi, Ryder Lee, Roy Luo
In-Reply-To: <a7ba0815-5e5c-c88f-c07c-c7d2a6a0714d@nbd.name>
On Fri, Aug 23, 2019 at 11:27:41AM +0200, Felix Fietkau wrote:
> On 2019-08-23 10:52, Stanislaw Gruszka wrote:
> > Waking tx queues will cause that txq's will be scheduled. Calling
> > mt76_txq_schedule_all() while queues are blocked is not necessary.
> > We will not get any skb's from ieee80211_tx_dequeue() anyway, but
> > patch changes that transmit of mtxq->retry_q skb's will be a bit
> > deferred (on the moment after channel switch or other situation
> > when we wake up queues).
> >
> > Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
> Waking tx queues will not always cause txqs to be scheduled - only if an
> attempt to dequeue was blocked because queues were stopped at that time.
> Because of that, I don't think this patch is correct.
Ok, please drop it.
Stanislaw
^ permalink raw reply
* Re: Implementing Mikrotik IE
From: Johannes Berg @ 2019-08-23 10:54 UTC (permalink / raw)
To: Josef Miegl; +Cc: Sebastian Gottschall, linux-wireless
In-Reply-To: <68A3B9AF-8864-4C0F-A50B-71CCB76AE81D@miegl.cz>
> Works great. Is there a possibility that a toggle for this could be
> accepted upstream? After all, WDS isn't really standardized.
I general, I'd say yes. However!
There's ongoing to work to make EAPOL frames go over nl80211 instead,
see e.g. ieee80211_tx_control_port() in mac80211, and this patch for
hostapd:
https://patchwork.ozlabs.org/patch/1108185/
I'd prefer to have it in this path, by having a flag like the "bool
unencrypted" passed to ieee80211_tx_control_port(), (and replace "bool
unencrypted" by "unsigned int flags"). That way, we don't have to
actually keep any state.
In mac80211, we can pass this down to __ieee80211_subif_start_xmit() and
ieee80211_build_hdr() in the ctrl_flags or something like that. It
doesn't actually need to be a control or info flag, so we could add yet
another argument to save the space, but dunno if that's worth it now.
johannes
^ permalink raw reply
* Re: [PATCH][next] mac80211: minstrel_ht: fix infinite loop because supported is not being shifted
From: Felix Fietkau @ 2019-08-23 11:07 UTC (permalink / raw)
To: Colin King, Johannes Berg, David S . Miller, linux-wireless,
netdev
Cc: kernel-janitors, linux-kernel
In-Reply-To: <20190822122034.28664-1-colin.king@canonical.com>
On 2019-08-22 14:20, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> Currently the for-loop will spin forever if variable supported is
> non-zero because supported is never changed. Fix this by adding in
> the missing right shift of supported.
>
> Addresses-Coverity: ("Infinite loop")
> Fixes: 48cb39522a9d ("mac80211: minstrel_ht: improve rate probing for devices with static fallback")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Acked-by: Felix Fietkau <nbd@nbd.name>
Thanks,
- Felix
^ permalink raw reply
* Re: [PATCH 31/49] ath11k: add mac.c
From: Vasanthakumar Thiagarajan @ 2019-08-23 12:15 UTC (permalink / raw)
To: Johannes Berg; +Cc: Kalle Valo, linux-wireless, devicetree, ath11k
In-Reply-To: <4076919b34cad119eb4146025f587285ef40e37c.camel@sipsolutions.net>
On 2019-08-21 02:16, Johannes Berg wrote:
> On Tue, 2019-08-20 at 18:47 +0300, Kalle Valo wrote:
>
>> +static int ath11k_mac_op_config(struct ieee80211_hw *hw, u32 changed)
>> +{
>> + struct ath11k *ar = hw->priv;
>> + int ret = 0;
>> +
>> + /* mac80211 requires this op to be present and that's why
>> + * there's an empty function, this can be extended when
>> + * required.
>> + */
>
> Well, oops. Maybe it shouldn't be required?
I think we require this for some configuration handling. The comment is
to be updated with
proper information. We'll address that.
>
>> + mutex_lock(&ar->conf_mutex);
>> +
>> + /* TODO: Handle configuration changes as appropriate */
>> +
>> + mutex_unlock(&ar->conf_mutex);
>
> It's not actually empty though - why bother locking the mutex for
> nothing?
Sure, we'll remove this locking.
>
>> + if (sta->mfp) {
>> + /* TODO: Need to check if FW supports PMF? */
>
> Probably not? shouldn't get a sta with MFP unless you advertised
> support
> for it. At least I'd think so, and consider it a mac80211 bug if you
> still did.
>
I could see driver getting sta with MFP irrespective of whether driver
advertises it's support in hw_flags by setting IEEE80211_HW_MFP_CAPABLE.
I see MFP station in driver even when I remove the support for the MFP
cipher
suits in STA mode. I agree all these needs to be handled in mac80211.
>> + /* This is a workaround for HT-enabled STAs which break the spec
>> + * and have no HT capabilities RX mask (no HT RX MCS map).
>> + *
>> + * As per spec, in section 20.3.5 Modulation and coding scheme
>> (MCS),
>> + * MCS 0 through 7 are mandatory in 20MHz with 800 ns GI at all
>> STAs.
>
> Wouldn't that better be in mac80211?
Right.
>
>> + ampdu_factor = (vht_cap->cap &
>> + IEEE80211_VHT_CAP_MAX_A_MPDU_LENGTH_EXPONENT_MASK) >>
>> + IEEE80211_VHT_CAP_MAX_A_MPDU_LENGTH_EXPONENT_SHIFT;
>
> consider u32_get_bits() or something like that from bitfield.h
>
>> + /* Workaround: Some Netgear/Linksys 11ac APs set Rx A-MPDU factor to
>> + * zero in VHT IE. Using it would result in degraded throughput.
>> + * arg->peer_max_mpdu at this point contains HT max_mpdu so keep
>> + * it if VHT max_mpdu is smaller.
>> + */
>> + arg->peer_max_mpdu = max(arg->peer_max_mpdu,
>> + (1U << (IEEE80211_HT_MAX_AMPDU_FACTOR +
>> + ampdu_factor)) - 1);
>
> Wait, that seems familiar. Again, put it into mac80211?
>
Sure.
>> +static void ath11k_peer_assoc_h_smps(struct ieee80211_sta *sta,
>> + struct peer_assoc_params *arg)
>> +{
>> + const struct ieee80211_sta_ht_cap *ht_cap = &sta->ht_cap;
>> + int smps;
>> +
>> + if (!ht_cap->ht_supported)
>> + return;
>> +
>> + smps = ht_cap->cap & IEEE80211_HT_CAP_SM_PS;
>> + smps >>= IEEE80211_HT_CAP_SM_PS_SHIFT;
>
> also here, u*_get_bits() or something might be nicer
>
> (and yes, I've written tons of code like this myself before that
> existed, which is why I'm pointing it out - it's much nicer)
>
Ok.
>> +void ath11k_mac_drain_tx(struct ath11k *ar)
>> +{
>> + /* make sure rcu-protected mac80211 tx path itself is drained */
>> + synchronize_net();
>
> Doesn't mac80211 ensure that in the relevant places like flush()? But
> then again, not sure where you call this.
This tx drain cleans up any pending management frames in the software
queue.
This will be done from hw_restart and drv_start callback to make sure we
do not have any pending management frames.
>
>> + ath11k_dbg(ar->ab, ATH11K_DBG_MAC, "mac set fixed rate params vdev
>> %i rate 0x%02hhx nss %hhu sgi %hhu\n",
>> + arvif->vdev_id, rate, nss, sgi);
>
> nit: that could use a line-break
>
>> + vdev_param = WMI_VDEV_PARAM_FIXED_RATE;
>> + ret = ath11k_wmi_vdev_set_param_cmd(ar, arvif->vdev_id,
>> + vdev_param, rate);
>> + if (ret) {
>> + ath11k_warn(ar->ab, "failed to set fixed rate param 0x%02x: %d\n",
>>
>> + /* TODO: Check if HT capability advertised from firmware is
>> different
>> + * for each band for a dual band capable radio. It will be tricky to
>> + * handle it when the ht capability different for each band.
>> + */
>
> For each band shouldn't really be that tricky?
>
Sure, we'll review and address this TODO.
Thanks,
Vasanth
> _______________________________________________
> ath11k mailing list
> ath11k@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/ath11k
^ permalink raw reply
* [PATCH 5.3] rt2x00: clear up IV's on key removal
From: Stanislaw Gruszka @ 2019-08-23 12:48 UTC (permalink / raw)
To: linux-wireless
Cc: Felix Fietkau, Daniel Golle, Tomislav Požega, Mathias Kresin,
Emil Karlson, Fredrik Noring
After looking at code I realized that my previous fix
95844124385e ("rt2x00: clear IV's on start to fix AP mode regression")
was incomplete. We can still have wrong IV's after re-keyring.
To fix that, clear up IV's also on key removal.
Fixes: 710e6cc1595e ("rt2800: do not nullify initialization vector data")
Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
drivers/net/wireless/ralink/rt2x00/rt2800lib.c | 19 ++++++++++++-------
1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
index ecbe78b8027b..28e2de04834e 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
@@ -1654,13 +1654,18 @@ static void rt2800_config_wcid_attr_cipher(struct rt2x00_dev *rt2x00dev,
offset = MAC_IVEIV_ENTRY(key->hw_key_idx);
- rt2800_register_multiread(rt2x00dev, offset,
- &iveiv_entry, sizeof(iveiv_entry));
- if ((crypto->cipher == CIPHER_TKIP) ||
- (crypto->cipher == CIPHER_TKIP_NO_MIC) ||
- (crypto->cipher == CIPHER_AES))
- iveiv_entry.iv[3] |= 0x20;
- iveiv_entry.iv[3] |= key->keyidx << 6;
+ if (crypto->cmd == SET_KEY) {
+ rt2800_register_multiread(rt2x00dev, offset,
+ &iveiv_entry, sizeof(iveiv_entry));
+ if ((crypto->cipher == CIPHER_TKIP) ||
+ (crypto->cipher == CIPHER_TKIP_NO_MIC) ||
+ (crypto->cipher == CIPHER_AES))
+ iveiv_entry.iv[3] |= 0x20;
+ iveiv_entry.iv[3] |= key->keyidx << 6;
+ } else {
+ memset(&iveiv_entry, 0, sizeof(iveiv_entry));
+ }
+
rt2800_register_multiwrite(rt2x00dev, offset,
&iveiv_entry, sizeof(iveiv_entry));
}
--
1.9.3
^ permalink raw reply related
* Re: [PATCH 31/49] ath11k: add mac.c
From: Nicolas Cavallari @ 2019-08-23 15:02 UTC (permalink / raw)
To: Kalle Valo, linux-wireless; +Cc: ath11k, devicetree
In-Reply-To: <1566316095-27507-32-git-send-email-kvalo@codeaurora.org>
On 20/08/2019 17:47, Kalle Valo wrote:
> + ar->hw->wiphy->interface_modes = BIT(NL80211_IFTYPE_STATION) |
> + BIT(NL80211_IFTYPE_AP) |
> + BIT(NL80211_IFTYPE_MESH_POINT);
[...]
> + ar->hw->wiphy->flags |= WIPHY_FLAG_IBSS_RSN;
so IBSS-RSN is supported without IBSS support ?
^ permalink raw reply
* Re: [PATCH v2] wireless-regdb: update regulatory rules for Russia (RU) on 5GHz
From: Seth Forshee @ 2019-08-23 19:00 UTC (permalink / raw)
To: Dmitry Tunin; +Cc: wireless-regdb, linux-wireless
In-Reply-To: <1564427246-25592-1-git-send-email-hanipouspilot@gmail.com>
On Mon, Jul 29, 2019 at 10:07:26PM +0300, Dmitry Tunin wrote:
> Russian entry is incorrect. According to the last regulations document of Feb 29, 2016,
> 160 MHz channels and 802.11ad are allowed.
>
> http://rfs-rf.ru/upload/medialibrary/c1a/prilozhenie-1-k-resheniyu-gkrch-_-16_36_03.pdf
>
> Note that there was never a DFS requirement in Russia, but always was
> NO-OUTDOOR on 5GHz.
> Maximum power is 200mW that is ~23dBm on all 5GHz channels.
> Also Russia has never been regulated by ETSI.
>
> Signed-off-by: Dmitry Tunin <hanipouspilot@gmail.com>
> ---
> db.txt | 10 ++++------
> 1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/db.txt b/db.txt
> index 37393e6..d95ed5e 100644
> --- a/db.txt
> +++ b/db.txt
> @@ -1097,14 +1097,12 @@ country RS: DFS-ETSI
> # 60 GHz band channels 1-4, ref: Etsi En 302 567
> (57000 - 66000 @ 2160), (40)
>
> -country RU: DFS-ETSI
> +country RU:
> (2402 - 2482 @ 40), (20)
> - (5170 - 5250 @ 80), (20), AUTO-BW
> - (5250 - 5330 @ 80), (20), DFS, AUTO-BW
> - (5650 - 5730 @ 80), (30), DFS
> - (5735 - 5835 @ 80), (30)
> + (5170 - 5350 @ 160), (23), NO-OUTDOOR
> + (5650 - 5850 @ 160), (23), NO-OUTDOOR
Based on the translation I've read of the document you've linked to as
well as a couple of others, it sounds like the use of these ranges
requires TPC. Since TPC is not supported in Linux, we need to reduce the
max EIRP for these ranges to 20 dBm.
While modifying them, let's also update the 5170-5350 range to 5150-5350
to match the regulations.
Thanks,
Seth
> # 60 GHz band channels 1-4, ref: Changes to NLA 124_Order №129_22042015.pdf
> - (57000 - 66000 @ 2160), (40)
> + (57000 - 66000 @ 2160), (40), NO-OUTDOOR
>
> country RW: DFS-FCC
> (2402 - 2482 @ 40), (20)
> --
> 2.7.4
>
^ permalink raw reply
* Re: [PATCH v2] wireless-regdb: update regulatory rules for Russia (RU) on 5GHz
From: Dmitry Tunin @ 2019-08-23 19:08 UTC (permalink / raw)
To: Seth Forshee; +Cc: wireless-regdb, linux-wireless
In-Reply-To: <20190823190019.GZ3071@ubuntu-xps13>
пт, 23 авг. 2019 г. в 22:00, Seth Forshee <seth.forshee@canonical.com>:
>
> On Mon, Jul 29, 2019 at 10:07:26PM +0300, Dmitry Tunin wrote:
> > Russian entry is incorrect. According to the last regulations document of Feb 29, 2016,
> > 160 MHz channels and 802.11ad are allowed.
> >
> > http://rfs-rf.ru/upload/medialibrary/c1a/prilozhenie-1-k-resheniyu-gkrch-_-16_36_03.pdf
> >
> > Note that there was never a DFS requirement in Russia, but always was
> > NO-OUTDOOR on 5GHz.
> > Maximum power is 200mW that is ~23dBm on all 5GHz channels.
> > Also Russia has never been regulated by ETSI.
> >
> > Signed-off-by: Dmitry Tunin <hanipouspilot@gmail.com>
> > ---
> > db.txt | 10 ++++------
> > 1 file changed, 4 insertions(+), 6 deletions(-)
> >
> > diff --git a/db.txt b/db.txt
> > index 37393e6..d95ed5e 100644
> > --- a/db.txt
> > +++ b/db.txt
> > @@ -1097,14 +1097,12 @@ country RS: DFS-ETSI
> > # 60 GHz band channels 1-4, ref: Etsi En 302 567
> > (57000 - 66000 @ 2160), (40)
> >
> > -country RU: DFS-ETSI
> > +country RU:
> > (2402 - 2482 @ 40), (20)
> > - (5170 - 5250 @ 80), (20), AUTO-BW
> > - (5250 - 5330 @ 80), (20), DFS, AUTO-BW
> > - (5650 - 5730 @ 80), (30), DFS
> > - (5735 - 5835 @ 80), (30)
> > + (5170 - 5350 @ 160), (23), NO-OUTDOOR
> > + (5650 - 5850 @ 160), (23), NO-OUTDOOR
>
> Based on the translation I've read of the document you've linked to as
> well as a couple of others, it sounds like the use of these ranges
> requires TPC. Since TPC is not supported in Linux, we need to reduce the
> max EIRP for these ranges to 20 dBm.
>
> While modifying them, let's also update the 5170-5350 range to 5150-5350
> to match the regulations.
It is not very clearly stated, but I agree that it looks like TPC.
Where is it stated that we need 20 dBm on these ranges if they require
TPC?
Unfortunately I don't have devices with OEM software certified in
Russia to see what is the limit.
^ permalink raw reply
* Re: [PATCH v2] wireless-regdb: update regulatory rules for Russia (RU) on 5GHz
From: Seth Forshee @ 2019-08-23 19:21 UTC (permalink / raw)
To: Dmitry Tunin; +Cc: wireless-regdb, linux-wireless
In-Reply-To: <CANoib0F5U7suFoHdKYuqQvjB+XN5bkqzNtBd6RkUi_Rnt7_OrA@mail.gmail.com>
On Fri, Aug 23, 2019 at 10:08:39PM +0300, Dmitry Tunin wrote:
> пт, 23 авг. 2019 г. в 22:00, Seth Forshee <seth.forshee@canonical.com>:
> >
> > On Mon, Jul 29, 2019 at 10:07:26PM +0300, Dmitry Tunin wrote:
> > > Russian entry is incorrect. According to the last regulations document of Feb 29, 2016,
> > > 160 MHz channels and 802.11ad are allowed.
> > >
> > > http://rfs-rf.ru/upload/medialibrary/c1a/prilozhenie-1-k-resheniyu-gkrch-_-16_36_03.pdf
> > >
> > > Note that there was never a DFS requirement in Russia, but always was
> > > NO-OUTDOOR on 5GHz.
> > > Maximum power is 200mW that is ~23dBm on all 5GHz channels.
> > > Also Russia has never been regulated by ETSI.
> > >
> > > Signed-off-by: Dmitry Tunin <hanipouspilot@gmail.com>
> > > ---
> > > db.txt | 10 ++++------
> > > 1 file changed, 4 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/db.txt b/db.txt
> > > index 37393e6..d95ed5e 100644
> > > --- a/db.txt
> > > +++ b/db.txt
> > > @@ -1097,14 +1097,12 @@ country RS: DFS-ETSI
> > > # 60 GHz band channels 1-4, ref: Etsi En 302 567
> > > (57000 - 66000 @ 2160), (40)
> > >
> > > -country RU: DFS-ETSI
> > > +country RU:
> > > (2402 - 2482 @ 40), (20)
> > > - (5170 - 5250 @ 80), (20), AUTO-BW
> > > - (5250 - 5330 @ 80), (20), DFS, AUTO-BW
> > > - (5650 - 5730 @ 80), (30), DFS
> > > - (5735 - 5835 @ 80), (30)
> > > + (5170 - 5350 @ 160), (23), NO-OUTDOOR
> > > + (5650 - 5850 @ 160), (23), NO-OUTDOOR
> >
> > Based on the translation I've read of the document you've linked to as
> > well as a couple of others, it sounds like the use of these ranges
> > requires TPC. Since TPC is not supported in Linux, we need to reduce the
> > max EIRP for these ranges to 20 dBm.
> >
> > While modifying them, let's also update the 5170-5350 range to 5150-5350
> > to match the regulations.
>
>
> It is not very clearly stated, but I agree that it looks like TPC.
> Where is it stated that we need 20 dBm on these ranges if they require
> TPC?
> Unfortunately I don't have devices with OEM software certified in
> Russia to see what is the limit.
It's not specifically 20 dBm, it's halving the power limit (i.e.
reducing it by 3 dBm).
Seth
^ permalink raw reply
* Re: [PATCH v2] wireless-regdb: update regulatory rules for Russia (RU) on 5GHz
From: Dmitry Tunin @ 2019-08-24 5:19 UTC (permalink / raw)
To: Seth Forshee; +Cc: wireless-regdb, linux-wireless
In-Reply-To: <20190823192143.GA3071@ubuntu-xps13>
> It's not specifically 20 dBm, it's halving the power limit (i.e.
> reducing it by 3 dBm).
I suggest to leave it on 23dBm. It is low enough especially for 5650 - 5850.
I am sure that a lot of Linux devices are certified that have this limit.
The TPC mentioned in the reg doc doesn't explain what it means.
^ permalink raw reply
* [PATCH v3] Russian entry is incorrect. According to the last regulations document of Feb 29, 2016, 160 MHz channels and 802.11ad are allowed.
From: Dmitry Tunin @ 2019-08-24 8:48 UTC (permalink / raw)
To: Seth Forshee; +Cc: wireless-regdb, linux-wireless, linux-kernel, Dmitry Tunin
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=y, Size: 1215 bytes --]
http://rfs-rf.ru/upload/medialibrary/c1a/prilozhenie-1-k-resheniyu-gkrch-_-16_36_03.pdf
Note that there was never a DFS requirement in Russia, but always was
NO-OUTDOOR on 5GHz.
Maximum power is 200mW that is ~23dBm on all 5GHz channels.
Also Russia has never been regulated by ETSI.
EIRP has been reduced by 4dBm because of TPC requirement.
Signed-off-by: Dmitry Tunin <hanipouspilot@gmail.com>
---
db.txt | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/db.txt b/db.txt
index 37393e6..9e4dc27 100644
--- a/db.txt
+++ b/db.txt
@@ -1097,14 +1097,12 @@ country RS: DFS-ETSI
# 60 GHz band channels 1-4, ref: Etsi En 302 567
(57000 - 66000 @ 2160), (40)
-country RU: DFS-ETSI
- (2402 - 2482 @ 40), (20)
- (5170 - 5250 @ 80), (20), AUTO-BW
- (5250 - 5330 @ 80), (20), DFS, AUTO-BW
- (5650 - 5730 @ 80), (30), DFS
- (5735 - 5835 @ 80), (30)
+country RU:
+ (2400 - 2483.5 @ 40), (20)
+ (5150 - 5350 @ 160), (20), NO-OUTDOOR
+ (5650 - 5850 @ 160), (20), NO-OUTDOOR
# 60 GHz band channels 1-4, ref: Changes to NLA 124_Order №129_22042015.pdf
- (57000 - 66000 @ 2160), (40)
+ (57000 - 66000 @ 2160), (40), NO-OUTDOOR
country RW: DFS-FCC
(2402 - 2482 @ 40), (20)
--
2.7.4
^ permalink raw reply related
* Re: [PATCH v3] Russian entry is incorrect. According to the last regulations document of Feb 29, 2016, 160 MHz channels and 802.11ad are allowed.
From: Dmitry Tunin @ 2019-08-24 8:50 UTC (permalink / raw)
To: Seth Forshee; +Cc: wireless-regdb, linux-wireless, linux-kernel
In-Reply-To: <1566636490-3438-1-git-send-email-hanipouspilot@gmail.com>
> EIRP has been reduced by 4dBm because of TPC requirement.
This is a typo. It's actuall 3dBm for 5 GHz.
^ permalink raw reply
* [PATCH] ath10k: remove TX lock from ath10k_htt_tx_inc_pending
From: Erik Stromdahl @ 2019-08-24 13:48 UTC (permalink / raw)
To: kvalo, linux-wireless, ath10k; +Cc: Erik Stromdahl
This commit removes the call to ath10k_mac_tx_lock() from
ath10k_htt_tx_inc_pending() in case the high water mark is reached.
ath10k_mac_tx_lock() calls ieee80211_stop_queues() in order to stop
mac80211 from pushing more TX data to the driver (this is the TX lock).
If a driver is trying to fetch an skb from a queue while the queue is
stopped, ieee80211_tx_dequeue() will return NULL.
So, in ath10k_mac_tx_push_txq(), there is a risk that the call to
ath10k_htt_tx_inc_pending() results in a stop of the mac80211 TX queues
just before the skb is fetched.
This will cause ieee80211_tx_dequeue() to return NULL and
ath10k_mac_tx_push_txq() to exit prematurely and return -ENOENT.
Before the function returns ath10k_htt_tx_dec_pending() will be called.
This call will re-enable the TX queues through ath10k_mac_tx_unlock().
When ath10k_mac_tx_push_txq() has returned, the TX queue will be
returned back to mac80211 with ieee80211_return_txq() without the skb
being properly consumed.
Since the TX queues were re-enabled in the error exit path of
ath10k_mac_tx_push_txq(), mac80211 can continue pushing data to the
driver. If the hardware does not consume the data, the above mentioned
case will be repeated over and over.
A case when the hardware is not able to transmit the data from the host
is when a STA has been dis-associated from an AP and has not yet been
able to re-associate. In this case there will be no TX_COMPL_INDs from
the hardware, resulting in the TX counter not be decremented.
This phenomenon has been observed in both a real and a test setup.
In order to fix this, the actual TX locking (the call to
ath10k_mac_tx_lock()) was removed from ath10k_htt_tx_inc_pending().
Instead, ath10k_mac_tx_lock() is called separately after the skb has
been fetched (after the call to ieee80211_tx_dequeue()). At this point
it is OK to stop the queues.
Signed-off-by: Erik Stromdahl <erik.stromdahl@gmail.com>
---
drivers/net/wireless/ath/ath10k/htt_tx.c | 2 --
drivers/net/wireless/ath/ath10k/mac.c | 8 ++++++++
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/htt_tx.c b/drivers/net/wireless/ath/ath10k/htt_tx.c
index 2ef717f18795..32fd71f28ef9 100644
--- a/drivers/net/wireless/ath/ath10k/htt_tx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_tx.c
@@ -157,8 +157,6 @@ int ath10k_htt_tx_inc_pending(struct ath10k_htt *htt)
return -EBUSY;
htt->num_pending_tx++;
- if (htt->num_pending_tx == htt->max_num_pending_tx)
- ath10k_mac_tx_lock(htt->ar, ATH10K_TX_PAUSE_Q_FULL);
return 0;
}
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index c1bf1167a2ce..8d357c79ca59 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -4078,6 +4078,11 @@ int ath10k_mac_tx_push_txq(struct ieee80211_hw *hw,
return -ENOENT;
}
+ spin_lock_bh(&ar->htt.tx_lock);
+ if (htt->num_pending_tx == htt->max_num_pending_tx)
+ ath10k_mac_tx_lock(ar, ATH10K_TX_PAUSE_Q_FULL);
+ spin_unlock_bh(&ar->htt.tx_lock);
+
airtime = ath10k_mac_update_airtime(ar, txq, skb);
ath10k_mac_tx_h_fill_cb(ar, vif, txq, skb, airtime);
@@ -4370,6 +4375,9 @@ static void ath10k_mac_op_tx(struct ieee80211_hw *hw,
return;
}
+ if (htt->num_pending_tx == htt->max_num_pending_tx)
+ ath10k_mac_tx_lock(ar, ATH10K_TX_PAUSE_Q_FULL);
+
ret = ath10k_htt_tx_mgmt_inc_pending(htt, is_mgmt, is_presp);
if (ret) {
ath10k_dbg(ar, ATH10K_DBG_MAC, "failed to increase tx mgmt pending count: %d, dropping\n",
--
2.22.0
^ permalink raw reply related
* Re: [PATCH] bcma: fix incorrect update of BCMA_CORE_PCI_MDIO_DATA
From: Rafał Miłecki @ 2019-08-25 19:59 UTC (permalink / raw)
To: Colin Ian King
Cc: Larry Finger, Hauke Mehrtens, linux-wireless@vger.kernel.org,
kernel-janitors, Linux Kernel Mailing List
In-Reply-To: <31258833-174f-080b-489e-85d3556bd1de@canonical.com>
On Thu, 22 Aug 2019 at 18:11, Colin Ian King <colin.king@canonical.com> wrote:
> On 22/08/2019 17:03, Larry Finger wrote:
> > On 8/22/19 8:35 AM, Colin King wrote:
> >> From: Colin Ian King <colin.king@canonical.com>
> >>
> >> An earlier commit re-worked the setting of the bitmask and is now
> >> assigning v with some bit flags rather than bitwise or-ing them
> >> into v, consequently the earlier bit-settings of v are being lost.
> >> Fix this by replacing an assignment with the bitwise or instead.
> >>
> >> Addresses-Coverity: ("Unused value")
> >> Fixes: 2be25cac8402 ("bcma: add constants for PCI and use them")
> >> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> >> ---
> >> drivers/bcma/driver_pci.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/bcma/driver_pci.c b/drivers/bcma/driver_pci.c
> >> index f499a469e66d..d219ee947c07 100644
> >> --- a/drivers/bcma/driver_pci.c
> >> +++ b/drivers/bcma/driver_pci.c
> >> @@ -78,7 +78,7 @@ static u16 bcma_pcie_mdio_read(struct bcma_drv_pci
> >> *pc, u16 device, u8 address)
> >> v |= (address << BCMA_CORE_PCI_MDIODATA_REGADDR_SHF_OLD);
> >> }
> >> - v = BCMA_CORE_PCI_MDIODATA_START;
> >> + v |= BCMA_CORE_PCI_MDIODATA_START;
> >> v |= BCMA_CORE_PCI_MDIODATA_READ;
> >> v |= BCMA_CORE_PCI_MDIODATA_TA;
> >
> > I'm not sure the "Fixes" attribute is correct.
> >
> > The changes for this section in commit 2be25cac8402 are
> >
> > - v = (1 << 30); /* Start of Transaction */
> > - v |= (1 << 28); /* Write Transaction */
> > - v |= (1 << 17); /* Turnaround */
> > - v |= (0x1F << 18);
> > + v = BCMA_CORE_PCI_MDIODATA_START;
> > + v |= BCMA_CORE_PCI_MDIODATA_WRITE;
> > + v |= (BCMA_CORE_PCI_MDIODATA_DEV_ADDR <<
> > + BCMA_CORE_PCI_MDIODATA_DEVADDR_SHF);
> > + v |= (BCMA_CORE_PCI_MDIODATA_BLK_ADDR <<
> > + BCMA_CORE_PCI_MDIODATA_REGADDR_SHF);
> > + v |= BCMA_CORE_PCI_MDIODATA_TA;
> >
> > Because the code has done quite a bit of work on v just above this
> > section, I agree that this is likely an error, but that error happened
> > in an earlier commit. Thus 2be25cac8402 did not introduce the error,
> > merely copied it.
>
> Ugh, this goes back further. I didn't spot that. I'm less confident of
> what the correct settings should be now.
>
> >
> > Has this change been tested?
>
> Afraid not, I don't have the H/W.
Please send V2 with updated commit message (Fixes tag) +
bcma_pcie_mdio_write fixed. I'll try to test it.
--
Rafał
^ permalink raw reply
* RE: [PATCH] ath10k: remove TX lock from ath10k_htt_tx_inc_pending
From: Wen Gong @ 2019-08-26 2:44 UTC (permalink / raw)
To: Erik Stromdahl, Kalle Valo, linux-wireless@vger.kernel.org,
ath10k@lists.infradead.org
In-Reply-To: <20190824134857.4094-1-erik.stromdahl@gmail.com>
> -----Original Message-----
> From: ath10k <ath10k-bounces@lists.infradead.org> On Behalf Of Erik
> Stromdahl
> Sent: Saturday, August 24, 2019 9:49 PM
> To: Kalle Valo <kvalo@qca.qualcomm.com>; linux-wireless@vger.kernel.org;
> ath10k@lists.infradead.org
> Cc: Erik Stromdahl <erik.stromdahl@gmail.com>
> Subject: [EXT] [PATCH] ath10k: remove TX lock from
> ath10k_htt_tx_inc_pending
>
> This commit removes the call to ath10k_mac_tx_lock() from
> ath10k_htt_tx_inc_pending() in case the high water mark is reached.
>
> ath10k_mac_tx_lock() calls ieee80211_stop_queues() in order to stop
> mac80211 from pushing more TX data to the driver (this is the TX lock).
>
> If a driver is trying to fetch an skb from a queue while the queue is
> stopped, ieee80211_tx_dequeue() will return NULL.
>
> So, in ath10k_mac_tx_push_txq(), there is a risk that the call to
> ath10k_htt_tx_inc_pending() results in a stop of the mac80211 TX queues
> just before the skb is fetched.
>
> This will cause ieee80211_tx_dequeue() to return NULL and
> ath10k_mac_tx_push_txq() to exit prematurely and return -ENOENT.
> Before the function returns ath10k_htt_tx_dec_pending() will be called.
> This call will re-enable the TX queues through ath10k_mac_tx_unlock().
> When ath10k_mac_tx_push_txq() has returned, the TX queue will be
> returned back to mac80211 with ieee80211_return_txq() without the skb
> being properly consumed.
>
> Since the TX queues were re-enabled in the error exit path of
> ath10k_mac_tx_push_txq(), mac80211 can continue pushing data to the
> driver. If the hardware does not consume the data, the above mentioned
> case will be repeated over and over.
>
> A case when the hardware is not able to transmit the data from the host
> is when a STA has been dis-associated from an AP and has not yet been
> able to re-associate. In this case there will be no TX_COMPL_INDs from
> the hardware, resulting in the TX counter not be decremented.
>
> This phenomenon has been observed in both a real and a test setup.
>
> In order to fix this, the actual TX locking (the call to
> ath10k_mac_tx_lock()) was removed from ath10k_htt_tx_inc_pending().
> Instead, ath10k_mac_tx_lock() is called separately after the skb has
> been fetched (after the call to ieee80211_tx_dequeue()). At this point
> it is OK to stop the queues.
Is this patch will impact throughput?
>
>
> _______________________________________________
> ath10k mailing list
> ath10k@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/ath10k
^ permalink raw reply
* [PATCH v4] rtw88: pci: Move a mass of jobs in hw IRQ to soft IRQ
From: Jian-Hong Pan @ 2019-08-26 7:08 UTC (permalink / raw)
To: Yan-Hsuan Chuang, Kalle Valo, David S . Miller
Cc: linux-wireless, netdev, linux-kernel, linux, Jian-Hong Pan
In-Reply-To: <F7CD281DE3E379468C6D07993EA72F84D18A5786@RTITMBSVM04.realtek.com.tw>
There is a mass of jobs between spin lock and unlock in the hardware
IRQ which will occupy much time originally. To make system work more
efficiently, this patch moves the jobs to the soft IRQ (bottom half) to
reduce the time in hardware IRQ.
Signed-off-by: Jian-Hong Pan <jian-hong@endlessm.com>
---
v2:
Change the spin_lock_irqsave/unlock_irqrestore to spin_lock/unlock in
rtw_pci_interrupt_handler. Because the interrupts are already disabled
in the hardware interrupt handler.
v3:
Extend the spin lock protecting area for the TX path in
rtw_pci_interrupt_threadfn by Realtek's suggestion
v4:
Remove the WiFi running check in rtw_pci_interrupt_threadfn to avoid AP
connection failed by Realtek's suggestion.
drivers/net/wireless/realtek/rtw88/pci.c | 32 +++++++++++++++++++-----
1 file changed, 26 insertions(+), 6 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtw88/pci.c b/drivers/net/wireless/realtek/rtw88/pci.c
index 00ef229552d5..955dd6c6fb57 100644
--- a/drivers/net/wireless/realtek/rtw88/pci.c
+++ b/drivers/net/wireless/realtek/rtw88/pci.c
@@ -866,12 +866,29 @@ static irqreturn_t rtw_pci_interrupt_handler(int irq, void *dev)
{
struct rtw_dev *rtwdev = dev;
struct rtw_pci *rtwpci = (struct rtw_pci *)rtwdev->priv;
- u32 irq_status[4];
spin_lock(&rtwpci->irq_lock);
if (!rtwpci->irq_enabled)
goto out;
+ /* disable RTW PCI interrupt to avoid more interrupts before the end of
+ * thread function
+ */
+ rtw_pci_disable_interrupt(rtwdev, rtwpci);
+out:
+ spin_unlock(&rtwpci->irq_lock);
+
+ return IRQ_WAKE_THREAD;
+}
+
+static irqreturn_t rtw_pci_interrupt_threadfn(int irq, void *dev)
+{
+ struct rtw_dev *rtwdev = dev;
+ struct rtw_pci *rtwpci = (struct rtw_pci *)rtwdev->priv;
+ unsigned long flags;
+ u32 irq_status[4];
+
+ spin_lock_irqsave(&rtwpci->irq_lock, flags);
rtw_pci_irq_recognized(rtwdev, rtwpci, irq_status);
if (irq_status[0] & IMR_MGNTDOK)
@@ -891,8 +908,9 @@ static irqreturn_t rtw_pci_interrupt_handler(int irq, void *dev)
if (irq_status[0] & IMR_ROK)
rtw_pci_rx_isr(rtwdev, rtwpci, RTW_RX_QUEUE_MPDU);
-out:
- spin_unlock(&rtwpci->irq_lock);
+ /* all of the jobs for this interrupt have been done */
+ rtw_pci_enable_interrupt(rtwdev, rtwpci);
+ spin_unlock_irqrestore(&rtwpci->irq_lock, flags);
return IRQ_HANDLED;
}
@@ -1152,8 +1170,10 @@ static int rtw_pci_probe(struct pci_dev *pdev,
goto err_destroy_pci;
}
- ret = request_irq(pdev->irq, &rtw_pci_interrupt_handler,
- IRQF_SHARED, KBUILD_MODNAME, rtwdev);
+ ret = devm_request_threaded_irq(rtwdev->dev, pdev->irq,
+ rtw_pci_interrupt_handler,
+ rtw_pci_interrupt_threadfn,
+ IRQF_SHARED, KBUILD_MODNAME, rtwdev);
if (ret) {
ieee80211_unregister_hw(hw);
goto err_destroy_pci;
@@ -1192,7 +1212,7 @@ static void rtw_pci_remove(struct pci_dev *pdev)
rtw_pci_disable_interrupt(rtwdev, rtwpci);
rtw_pci_destroy(rtwdev, pdev);
rtw_pci_declaim(rtwdev, pdev);
- free_irq(rtwpci->pdev->irq, rtwdev);
+ devm_free_irq(rtwdev->dev, rtwpci->pdev->irq, rtwdev);
rtw_core_deinit(rtwdev);
ieee80211_free_hw(hw);
}
--
2.20.1
^ permalink raw reply related
* Re: [PATCH v3] rtw88: pci: Move a mass of jobs in hw IRQ to soft IRQ
From: Jian-Hong Pan @ 2019-08-26 7:21 UTC (permalink / raw)
To: Tony Chuang
Cc: Kalle Valo, David S . Miller, linux-wireless@vger.kernel.org,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
linux@endlessm.com
In-Reply-To: <F7CD281DE3E379468C6D07993EA72F84D18A5786@RTITMBSVM04.realtek.com.tw>
Tony Chuang <yhchuang@realtek.com> 於 2019年8月21日 週三 下午4:16寫道：
>
> Hi,
>
> > From: Jian-Hong Pan [mailto:jian-hong@endlessm.com]
> >
> > There is a mass of jobs between spin lock and unlock in the hardware
> > IRQ which will occupy much time originally. To make system work more
> > efficiently, this patch moves the jobs to the soft IRQ (bottom half) to
> > reduce the time in hardware IRQ.
> >
> > Signed-off-by: Jian-Hong Pan <jian-hong@endlessm.com>
> > ---
> > v2:
> > Change the spin_lock_irqsave/unlock_irqrestore to spin_lock/unlock in
> > rtw_pci_interrupt_handler. Because the interrupts are already disabled
> > in the hardware interrupt handler.
> >
> > v3:
> > Extend the spin lock protecting area for the TX path in
> > rtw_pci_interrupt_threadfn by Realtek's suggestion
> >
> > drivers/net/wireless/realtek/rtw88/pci.c | 33 +++++++++++++++++++-----
> > 1 file changed, 27 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/net/wireless/realtek/rtw88/pci.c
> > b/drivers/net/wireless/realtek/rtw88/pci.c
> > index 00ef229552d5..a8c17a01f318 100644
> > --- a/drivers/net/wireless/realtek/rtw88/pci.c
> > +++ b/drivers/net/wireless/realtek/rtw88/pci.c
> > @@ -866,12 +866,29 @@ static irqreturn_t rtw_pci_interrupt_handler(int irq,
> > void *dev)
> > {
> > struct rtw_dev *rtwdev = dev;
> > struct rtw_pci *rtwpci = (struct rtw_pci *)rtwdev->priv;
> > - u32 irq_status[4];
> >
> > spin_lock(&rtwpci->irq_lock);
> > if (!rtwpci->irq_enabled)
> > goto out;
> >
> > + /* disable RTW PCI interrupt to avoid more interrupts before the end of
> > + * thread function
> > + */
> > + rtw_pci_disable_interrupt(rtwdev, rtwpci);
> > +out:
> > + spin_unlock(&rtwpci->irq_lock);
> > +
> > + return IRQ_WAKE_THREAD;
> > +}
> > +
> > +static irqreturn_t rtw_pci_interrupt_threadfn(int irq, void *dev)
> > +{
> > + struct rtw_dev *rtwdev = dev;
> > + struct rtw_pci *rtwpci = (struct rtw_pci *)rtwdev->priv;
> > + unsigned long flags;
> > + u32 irq_status[4];
> > +
> > + spin_lock_irqsave(&rtwpci->irq_lock, flags);
> > rtw_pci_irq_recognized(rtwdev, rtwpci, irq_status);
> >
> > if (irq_status[0] & IMR_MGNTDOK)
> > @@ -891,8 +908,10 @@ static irqreturn_t rtw_pci_interrupt_handler(int irq,
> > void *dev)
> > if (irq_status[0] & IMR_ROK)
> > rtw_pci_rx_isr(rtwdev, rtwpci, RTW_RX_QUEUE_MPDU);
> >
> > -out:
> > - spin_unlock(&rtwpci->irq_lock);
> > + /* all of the jobs for this interrupt have been done */
> > + if (rtw_flag_check(rtwdev, RTW_FLAG_RUNNING))
> > + rtw_pci_enable_interrupt(rtwdev, rtwpci);
>
> I've applied this patch and tested it.
> But I failed to connect to AP, it seems that the
> scan_result is empty. And when I failed to connect
> to the AP, I found that the IMR is not enabled.
> I guess the check bypass the interrupt enable function.
>
> And I also found that *without MSI*, the driver is
> able to connect to the AP. Could you please verify
> this patch again with MSI interrupt enabled and
> send a v4?
>
> You can find my MSI patch on
> https://patchwork.kernel.org/patch/11081539/
I have just sent v4 patch. Also tested the modified MSI patch like below:
The WiFi works fine on ASUS X512DK (including MSI enabled).
Jian-Hong Pan
diff --git a/drivers/net/wireless/realtek/rtw88/pci.c
b/drivers/net/wireless/realtek/rtw88/pci.c
index 955dd6c6fb57..d18f5aae1585 100644
--- a/drivers/net/wireless/realtek/rtw88/pci.c
+++ b/drivers/net/wireless/realtek/rtw88/pci.c
@@ -11,6 +11,10 @@
#include "fw.h"
#include "debug.h"
+static bool rtw_disable_msi;
+module_param_named(disable_msi, rtw_disable_msi, bool, 0644);
+MODULE_PARM_DESC(disable_msi, "Set Y to disable MSI interrupt support");
+
static u32 rtw_pci_tx_queue_idx_addr[] = {
[RTW_TX_QUEUE_BK] = RTK_PCI_TXBD_IDX_BKQ,
[RTW_TX_QUEUE_BE] = RTK_PCI_TXBD_IDX_BEQ,
@@ -1116,6 +1120,48 @@ static struct rtw_hci_ops rtw_pci_ops = {
.write_data_h2c = rtw_pci_write_data_h2c,
};
+static int rtw_pci_request_irq(struct rtw_dev *rtwdev, struct pci_dev *pdev)
+{
+ struct rtw_pci *rtwpci = (struct rtw_pci *)rtwdev->priv;
+ int ret;
+
+ if (!rtw_disable_msi) {
+ ret = pci_enable_msi(pdev);
+ if (ret) {
+ rtw_warn(rtwdev, "failed to enable msi %d, using legacy irq\n",
+ ret);
+ } else {
+ rtw_warn(rtwdev, "pci msi enabled\n");
+ rtwpci->msi_enabled = true;
+ }
+ }
+
+ ret = devm_request_threaded_irq(rtwdev->dev, pdev->irq,
+ rtw_pci_interrupt_handler,
+ rtw_pci_interrupt_threadfn,
+ IRQF_SHARED, KBUILD_MODNAME, rtwdev);
+ if (ret) {
+ rtw_err(rtwdev, "failed to request irq %d\n", ret);
+ if (rtwpci->msi_enabled) {
+ pci_disable_msi(pdev);
+ rtwpci->msi_enabled = false;
+ }
+ }
+
+ return ret;
+}
+
+static void rtw_pci_free_irq(struct rtw_dev *rtwdev, struct pci_dev *pdev)
+{
+ struct rtw_pci *rtwpci = (struct rtw_pci *)rtwdev->priv;
+
+ devm_free_irq(rtwdev->dev, rtwpci->pdev->irq, rtwdev);
+ if (rtwpci->msi_enabled) {
+ pci_disable_msi(pdev);
+ rtwpci->msi_enabled = false;
+ }
+}
+
static int rtw_pci_probe(struct pci_dev *pdev,
const struct pci_device_id *id)
{
@@ -1170,10 +1216,7 @@ static int rtw_pci_probe(struct pci_dev *pdev,
goto err_destroy_pci;
}
- ret = devm_request_threaded_irq(rtwdev->dev, pdev->irq,
- rtw_pci_interrupt_handler,
- rtw_pci_interrupt_threadfn,
- IRQF_SHARED, KBUILD_MODNAME, rtwdev);
+ ret = rtw_pci_request_irq(rtwdev, pdev);
if (ret) {
ieee80211_unregister_hw(hw);
goto err_destroy_pci;
@@ -1212,7 +1255,7 @@ static void rtw_pci_remove(struct pci_dev *pdev)
rtw_pci_disable_interrupt(rtwdev, rtwpci);
rtw_pci_destroy(rtwdev, pdev);
rtw_pci_declaim(rtwdev, pdev);
- devm_free_irq(rtwdev->dev, rtwpci->pdev->irq, rtwdev);
+ rtw_pci_free_irq(rtwdev, pdev);
rtw_core_deinit(rtwdev);
ieee80211_free_hw(hw);
}
diff --git a/drivers/net/wireless/realtek/rtw88/pci.h
b/drivers/net/wireless/realtek/rtw88/pci.h
index 87824a4caba9..a8e369c5eaca 100644
--- a/drivers/net/wireless/realtek/rtw88/pci.h
+++ b/drivers/net/wireless/realtek/rtw88/pci.h
@@ -186,6 +186,7 @@ struct rtw_pci {
spinlock_t irq_lock;
u32 irq_mask[4];
bool irq_enabled;
+ bool msi_enabled;
u16 rx_tag;
struct rtw_pci_tx_ring tx_rings[RTK_MAX_TX_QUEUE_NUM];
> > + spin_unlock_irqrestore(&rtwpci->irq_lock, flags);
> >
> > return IRQ_HANDLED;
> > }
> > @@ -1152,8 +1171,10 @@ static int rtw_pci_probe(struct pci_dev *pdev,
> > goto err_destroy_pci;
> > }
> >
> > - ret = request_irq(pdev->irq, &rtw_pci_interrupt_handler,
> > - IRQF_SHARED, KBUILD_MODNAME, rtwdev);
> > + ret = devm_request_threaded_irq(rtwdev->dev, pdev->irq,
> > + rtw_pci_interrupt_handler,
> > + rtw_pci_interrupt_threadfn,
> > + IRQF_SHARED, KBUILD_MODNAME, rtwdev);
> > if (ret) {
> > ieee80211_unregister_hw(hw);
> > goto err_destroy_pci;
> > @@ -1192,7 +1213,7 @@ static void rtw_pci_remove(struct pci_dev *pdev)
> > rtw_pci_disable_interrupt(rtwdev, rtwpci);
> > rtw_pci_destroy(rtwdev, pdev);
> > rtw_pci_declaim(rtwdev, pdev);
> > - free_irq(rtwpci->pdev->irq, rtwdev);
> > + devm_free_irq(rtwdev->dev, rtwpci->pdev->irq, rtwdev);
> > rtw_core_deinit(rtwdev);
> > ieee80211_free_hw(hw);
> > }
> > --
>
>
> NACK
> Need to verify it with MSI https://patchwork.kernel.org/patch/11081539/
> And hope v4 could fix it.
>
> Yan-Hsuan
>
^ permalink raw reply related
* Re: [PATCH 10/49] ath11k: add debug.c
From: Sven Eckelmann @ 2019-08-26 13:47 UTC (permalink / raw)
To: ath11k; +Cc: Kalle Valo, linux-wireless, devicetree
In-Reply-To: <1566316095-27507-11-git-send-email-kvalo@codeaurora.org>
[-- Attachment #1: Type: text/plain, Size: 9812 bytes --]
On Tuesday, 20 August 2019 17:47:36 CEST Kalle Valo wrote:
> +static ssize_t ath11k_read_simulate_fw_crash(struct file *file,
> + char __user *user_buf,
> + size_t count, loff_t *ppos)
> +{
> + const char buf[] =
> + "To simulate firmware crash write one of the keywords to this file:\n"
> + "`assert` - this will send WMI_FORCE_FW_HANG_CMDID to firmware to cause assert.\n"
> + "`hw-restart` - this will simply queue hw restart without fw/hw actually crashing.\n";
> +
> + return simple_read_from_buffer(user_buf, count, ppos, buf, strlen(buf));
> +}
There is nothing in the write handler which handles "hw-restart". It just
causes an -EINVAL.
> +
> +/* Simulate firmware crash:
> + * 'soft': Call wmi command causing firmware hang. This firmware hang is
> + * recoverable by warm firmware reset.
> + * 'hard': Force firmware crash by setting any vdev parameter for not allowed
> + * vdev id. This is hard firmware crash because it is recoverable only by cold
> + * firmware reset.
> + */
> +static ssize_t ath11k_write_simulate_fw_crash(struct file *file,
> + const char __user *user_buf,
> + size_t count, loff_t *ppos)
> +{
> + struct ath11k_base *ab = file->private_data;
> + struct ath11k_pdev *pdev;
> + struct ath11k *ar = ab->pdevs[0].ar;
> + char buf[32] = {0};
> + ssize_t rc;
> + int i, ret, radioup;
> +
> + for (i = 0; i < ab->num_radios; i++) {
> + pdev = &ab->pdevs[i];
> + ar = pdev->ar;
> + if (ar && ar->state == ATH11K_STATE_ON) {
> + radioup = 1;
> + break;
> + }
> + }
> + /* filter partial writes and invalid commands */
> + if (*ppos != 0 || count >= sizeof(buf) || count == 0)
> + return -EINVAL;
> +
> + rc = simple_write_to_buffer(buf, sizeof(buf) - 1, ppos, user_buf, count);
> + if (rc < 0)
> + return rc;
> +
> + /* drop the possible '\n' from the end */
> + if (buf[*ppos - 1] == '\n')
> + buf[*ppos - 1] = '\0';
> +
> + if (radioup == 0) {
> + ret = -ENETDOWN;
> + goto exit;
> + }
> +
> + if (!strcmp(buf, "assert")) {
> + ath11k_info(ab, "simulating firmware assert crash\n");
> + ret = ath11k_wmi_force_fw_hang_cmd(ar,
> + ATH11K_WMI_FW_HANG_ASSERT_TYPE,
> + ATH11K_WMI_FW_HANG_DELAY);
> + } else {
> + ret = -EINVAL;
> + goto exit;
> + }
> +
> + if (ret) {
> + ath11k_warn(ab, "failed to simulate firmware crash: %d\n", ret);
> + goto exit;
> + }
> +
> + ret = count;
> +
> +exit:
> + return ret;
> +}
And right now, the write of an "assert" to this file just causes an fatal error for the system:
[ 4312.409255] qcom-q6v5-wcss-pil cd00000.qcom_q6v5_wcss: fatal error received:
[ 4312.409255] QC Image Version: QC_IMAGE_VERSION_STRING=WLAN.HK.2.1.0.1-00410-QCAHKSWPL_SILICONZ-2
[ 4312.409255] Image Variant : IMAGE_VARIANT_STRING=8074.wlanfw.eval_v2Q
[ 4312.409255]
[ 4312.409255] wlan_wmi.c:234 Assertion 0 failedparam0 :zero, param1 :zero, param2 :zero.
[ 4312.409255] Thread ID : 0x00000069 Thread name : WLAN RT0 Process ID : 0
[ 4312.409255] Register:
[ 4312.409255] SP : 0x4c168d58
[ 4312.409255] FP : 0x4c168d60
[ 4312.409255] PC : 0x4b1c8850
[ 4312.409255] SSR : 0x00000008
[ 4312.409255] BADVA : 0x00020000
[ 4312.409255] LR : 0x4b1c7c68
[ 4312.409255]
[ 4312.409255] Stack Dump
[ 4312.409255] from : 0x4c168d58
[ 4312.409255] to : 0x4c168f00
[ 4312.409255]
[ 4312.455997] remoteproc remoteproc0: crash detected in cd00000.qcom_q6v5_wcss: type fatal error
[ 4312.478259] remoteproc remoteproc0: handling crash #1 in cd00000.qcom_q6v5_wcss
[ 4312.486826] Kernel panic - not syncing: remoteproc remoteproc0: Resetting the SoC - cd00000.qcom_q6v5_wcss crashed
[ 4312.494028] CPU: 2 PID: 5590 Comm: kworker/2:0 Tainted: G W 4.4.60 #0
[ 4312.504436] Hardware name: Generic DT based system
[ 4312.511991] Workqueue: events rproc_crash_handler_work
[ 4312.521880] [<8021e86c>] (unwind_backtrace) from [<8021b404>] (show_stack+0x10/0x14)
[ 4312.521979] [<8021b404>] (show_stack) from [<803dd818>] (dump_stack+0x7c/0x9c)
[ 4312.529789] [<803dd818>] (dump_stack) from [<80225d80>] (panic+0x84/0x1f8)
[ 4312.536818] [<80225d80>] (panic) from [<80555278>] (rproc_crash_handler_work+0x90/0x98)
[ 4312.543678] [<80555278>] (rproc_crash_handler_work) from [<802380e8>] (process_one_work+0x1c0/0x2f8)
[ 4312.551578] [<802380e8>] (process_one_work) from [<80238d24>] (worker_thread+0x2b0/0x3ec)
[ 4312.560952] [<80238d24>] (worker_thread) from [<8023cf84>] (kthread+0xd8/0xec)
[ 4312.569023] [<8023cf84>] (kthread) from [<80209be8>] (ret_from_fork+0x14/0x2c)
[ 4312.576141] CPU0: stopping
[ 4312.583335] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G W 4.4.60 #0
[ 4312.586032] Hardware name: Generic DT based system
[ 4312.593237] [<8021e86c>] (unwind_backtrace) from [<8021b404>] (show_stack+0x10/0x14)
[ 4312.597930] [<8021b404>] (show_stack) from [<803dd818>] (dump_stack+0x7c/0x9c)
[ 4312.605827] [<803dd818>] (dump_stack) from [<8021dc3c>] (handle_IPI+0xe8/0x180)
[ 4312.612858] [<8021dc3c>] (handle_IPI) from [<802093a4>] (gic_handle_irq+0x78/0x94)
[ 4312.620063] [<802093a4>] (gic_handle_irq) from [<8020a480>] (__irq_svc+0x40/0x74)
[ 4312.627701] Exception stack(0x80c67f60 to 0x80c67fa8)
[ 4312.635249] 7f60: 00000001 00000000 00000000 8020b320 00000000 80c66000 00000000 80c612cc
[ 4312.640291] 7f80: 80c67fb8 808f3a30 80cae010 00000000 00000000 80c67fb0 80218edc 80218ee0
[ 4312.648448] 7fa0: 60000013 ffffffff
[ 4312.656601] [<8020a480>] (__irq_svc) from [<80218ee0>] (arch_cpu_idle+0x2c/0x50)
[ 4312.659909] [<80218ee0>] (arch_cpu_idle) from [<80254b38>] (cpu_startup_entry+0x134/0x214)
[ 4312.667553] [<80254b38>] (cpu_startup_entry) from [<808cac48>] (start_kernel+0x380/0x404)
[ 4312.675620] CPU1: stopping
[ 4312.683855] CPU: 1 PID: 0 Comm: swapper/1 Tainted: G W 4.4.60 #0
[ 4312.686466] Hardware name: Generic DT based system
[ 4312.693671] [<8021e86c>] (unwind_backtrace) from [<8021b404>] (show_stack+0x10/0x14)
[ 4312.698363] [<8021b404>] (show_stack) from [<803dd818>] (dump_stack+0x7c/0x9c)
[ 4312.706263] [<803dd818>] (dump_stack) from [<8021dc3c>] (handle_IPI+0xe8/0x180)
[ 4312.713293] [<8021dc3c>] (handle_IPI) from [<802093a4>] (gic_handle_irq+0x78/0x94)
[ 4312.720497] [<802093a4>] (gic_handle_irq) from [<8020a480>] (__irq_svc+0x40/0x74)
[ 4312.728135] Exception stack(0xbe083f98 to 0xbe083fe0)
[ 4312.735683] 3f80: 00000001 00000000
[ 4312.740725] 3fa0: 00000000 8020b320 00000000 be082000 00000000 80c612cc be083ff0 410fd034
[ 4312.748884] 3fc0: 00000000 00000000 00000000 be083fe8 80218edc 80218ee0 60000013 ffffffff
[ 4312.757045] [<8020a480>] (__irq_svc) from [<80218ee0>] (arch_cpu_idle+0x2c/0x50)
[ 4312.765203] [<80218ee0>] (arch_cpu_idle) from [<80254b38>] (cpu_startup_entry+0x134/0x214)
[ 4312.772669] [<80254b38>] (cpu_startup_entry) from [<4120944c>] (0x4120944c)
[ 4312.780737] CPU3: stopping
[ 4312.787589] CPU: 3 PID: 0 Comm: swapper/3 Tainted: G W 4.4.60 #0
[ 4312.790372] Hardware name: Generic DT based system
[ 4312.797577] [<8021e86c>] (unwind_backtrace) from [<8021b404>] (show_stack+0x10/0x14)
[ 4312.802270] [<8021b404>] (show_stack) from [<803dd818>] (dump_stack+0x7c/0x9c)
[ 4312.810167] [<803dd818>] (dump_stack) from [<8021dc3c>] (handle_IPI+0xe8/0x180)
[ 4312.817199] [<8021dc3c>] (handle_IPI) from [<802093a4>] (gic_handle_irq+0x78/0x94)
[ 4312.824403] [<802093a4>] (gic_handle_irq) from [<8020a480>] (__irq_svc+0x40/0x74)
[ 4312.832041] Exception stack(0xbe087f98 to 0xbe087fe0)
[ 4312.839588] 7f80: 00000001 00000000
[ 4312.844630] 7fa0: 00000000 8020b320 00000000 be086000 00000000 80c612cc be087ff0 410fd034
[ 4312.852791] 7fc0: 00000000 00000000 00000000 be087fe8 80218edc 80218ee0 60000013 ffffffff
[ 4312.860951] [<8020a480>] (__irq_svc) from [<80218ee0>] (arch_cpu_idle+0x2c/0x50)
[ 4312.869109] [<80218ee0>] (arch_cpu_idle) from [<80254b38>] (cpu_startup_entry+0x134/0x214)
[ 4312.876576] [<80254b38>] (cpu_startup_entry) from [<4120944c>] (0x4120944c)
[ 4312.884650] The reading for sensor 4 is 0x002041f7
[ 4312.891499] The reading for sensor 5 is 0x002051f4
[ 4312.896415] Couldn't get reading for sensor 6
[ 4312.901189] Couldn't get reading for sensor 7
[ 4312.905561] The reading for sensor 8 is 0x002081e0
[ 4312.909902] The reading for sensor 9 is 0x002091f7
[ 4312.914645] Couldn't get reading for sensor 10
[ 4312.919364] The reading for sensor 11 is 0x0020b1fa
[ 4312.923791] The reading for sensor 12 is 0x0020c1fa
[ 4312.928621] Couldn't get reading for sensor 13
[ 4312.933425] The reading for sensor 14 is 0x0020e1f4
[ 4312.937941] The reading for sensor 15 is 0x0020f1e7
[ 4313.942700] Rebooting in 3 seconds..
Maybe can be fixed by a different kernel (for the remoteproc). But I don't
have this kernel at the moment.
Kind regards,
Sven
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [PATCH 1/2] nl80211: Add NL80211_EXT_FEATURE_LIVE_IFTYPE_CHANGE
From: Denis Kenzior @ 2019-08-26 16:26 UTC (permalink / raw)
To: linux-wireless; +Cc: Denis Kenzior
There is some ambiguity in how various drivers support
NL80211_CMD_SET_INTERFACE on devices where the underlying netdev is UP.
mac80211 for example supports this if the underlying driver provides a
change_interface operation. However, most devices do not. For FullMac
devices, the situation is even less clear.
This patch introduces a new feature flag that lets userspace know
whether it can expect a mode change (via SET_INTERFACE) to work while
the device is still UP or it should bring down the device first.
This commit also updates the documentation for SET_INTERFACE with hints
as to how it should be used.
Signed-off-by: Denis Kenzior <denkenz@gmail.com>
---
include/uapi/linux/nl80211.h | 36 ++++++++++++++++++++++++++++++++++++
1 file changed, 36 insertions(+)
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index bf7c4222f512..a9ca2fe67f52 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -275,6 +275,29 @@
* single %NL80211_ATTR_IFINDEX is supported.
* @NL80211_CMD_SET_INTERFACE: Set type of a virtual interface, requires
* %NL80211_ATTR_IFINDEX and %NL80211_ATTR_IFTYPE.
+ *
+ * Note that it is driver-dependent whether a SET_INTERFACE will be
+ * allowed if the underlying netdev is currently UP. Userspace
+ * can obtain a hint as to whether this is allowed by looking at
+ * the %NL80211_EXT_FEATURE_LIVE_IFTYPE_CHANGE, but certain restrictions
+ * will still apply.
+ *
+ * Prior to Kernel 5.4, userspace applications should implement the
+ * following behavior:
+ * 1. (Optionally) Attempt SET_INTERFACE on a wireless device
+ * with the underlying netdev in the UP state. If -EBUSY
+ * is returned proceed to 2. Note that a SET_INTERFACE
+ * which results in -EBUSY might still result in other
+ * side-effects, such as Deauthentication, exiting AP mode,
+ * etc.
+ * 2. Bring the netdev DOWN via RTNL
+ * 3. Attempt SET_INTERFACE on the underlying netdev in the DOWN
+ * state. If successful, proceed to 4.
+ * 4. Bring the netdev UP via RTNL
+ *
+ * Note that bringing down the device might trigger a firmware reset /
+ * power cycling and/or other effects that are driver dependent.
+ *
* @NL80211_CMD_NEW_INTERFACE: Newly created virtual interface or response
* to %NL80211_CMD_GET_INTERFACE. Has %NL80211_ATTR_IFINDEX,
* %NL80211_ATTR_WIPHY and %NL80211_ATTR_IFTYPE attributes. Can also
@@ -5481,6 +5504,18 @@ enum nl80211_feature_flags {
* @NL80211_EXT_FEATURE_SAE_OFFLOAD: Device wants to do SAE authentication in
* station mode (SAE password is passed as part of the connect command).
*
+ * @NL80211_EXT_FEATURE_LIVE_IFTYPE_CHANGE: This device supports switching
+ * the IFTYPE of an interface without having to bring the device DOWN
+ * first via RTNL. Exact semantics of this feature is driver
+ * implementation dependent. For mac80211, the following restrictions
+ * apply:
+ * - Only devices currently in IFTYPE AP, P2P_GO, P2P_CLIENT,
+ * STATION, ADHOC and OCB can be switched.
+ * - The target IFTYPE must be one of: AP, P2P_GO, P2P_CLIENT,
+ * STATION, ADHOC or OCB.
+ * Other drivers are expected to follow similar restrictions.
+ * This flag was introduced in Kernel v5.4
+ *
* @NUM_NL80211_EXT_FEATURES: number of extended features.
* @MAX_NL80211_EXT_FEATURES: highest extended feature index.
*/
@@ -5526,6 +5561,7 @@ enum nl80211_ext_feature_index {
NL80211_EXT_FEATURE_EXT_KEY_ID,
NL80211_EXT_FEATURE_STA_TX_PWR,
NL80211_EXT_FEATURE_SAE_OFFLOAD,
+ NL80211_EXT_FEATURE_LIVE_IFTYPE_CHANGE,
/* add new features before the definition below */
NUM_NL80211_EXT_FEATURES,
--
2.19.2
^ permalink raw reply related
* [PATCH 2/2] mac80211: Set LIVE_IFTYPE_CHANGE if op provided
From: Denis Kenzior @ 2019-08-26 16:26 UTC (permalink / raw)
To: linux-wireless; +Cc: Denis Kenzior
In-Reply-To: <20190826162637.7535-1-denkenz@gmail.com>
Signed-off-by: Denis Kenzior <denkenz@gmail.com>
---
net/mac80211/main.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index 29b9d57df1a3..073e5d10a48e 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -597,6 +597,10 @@ struct ieee80211_hw *ieee80211_alloc_hw_nm(size_t priv_data_len,
wiphy_ext_feature_set(wiphy, NL80211_EXT_FEATURE_RRM);
+ if (ops->change_interface)
+ wiphy_ext_feature_set(wiphy,
+ NL80211_EXT_FEATURE_LIVE_IFTYPE_CHANGE);
+
wiphy->bss_priv_size = sizeof(struct ieee80211_bss);
local = wiphy_priv(wiphy);
--
2.19.2
^ permalink raw reply related
* Re: [PATCH] ath10k: remove TX lock from ath10k_htt_tx_inc_pending
From: Erik Stromdahl @ 2019-08-26 16:55 UTC (permalink / raw)
To: Wen Gong, Kalle Valo, linux-wireless@vger.kernel.org,
ath10k@lists.infradead.org
In-Reply-To: <4cd30880ae754f5599e6b1a4c1ac6a74@aptaiexm02f.ap.qualcomm.com>
On 8/26/19 4:44 AM, Wen Gong wrote:
>> -----Original Message-----
>> From: ath10k <ath10k-bounces@lists.infradead.org> On Behalf Of Erik
>> Stromdahl
>> Sent: Saturday, August 24, 2019 9:49 PM
>> To: Kalle Valo <kvalo@qca.qualcomm.com>; linux-wireless@vger.kernel.org;
>> ath10k@lists.infradead.org
>> Cc: Erik Stromdahl <erik.stromdahl@gmail.com>
>> Subject: [EXT] [PATCH] ath10k: remove TX lock from
>> ath10k_htt_tx_inc_pending
>>
>> This commit removes the call to ath10k_mac_tx_lock() from
>> ath10k_htt_tx_inc_pending() in case the high water mark is reached.
>>
>> ath10k_mac_tx_lock() calls ieee80211_stop_queues() in order to stop
>> mac80211 from pushing more TX data to the driver (this is the TX lock).
>>
>> If a driver is trying to fetch an skb from a queue while the queue is
>> stopped, ieee80211_tx_dequeue() will return NULL.
>>
>> So, in ath10k_mac_tx_push_txq(), there is a risk that the call to
>> ath10k_htt_tx_inc_pending() results in a stop of the mac80211 TX queues
>> just before the skb is fetched.
>>
>> This will cause ieee80211_tx_dequeue() to return NULL and
>> ath10k_mac_tx_push_txq() to exit prematurely and return -ENOENT.
>> Before the function returns ath10k_htt_tx_dec_pending() will be called.
>> This call will re-enable the TX queues through ath10k_mac_tx_unlock().
>> When ath10k_mac_tx_push_txq() has returned, the TX queue will be
>> returned back to mac80211 with ieee80211_return_txq() without the skb
>> being properly consumed.
>>
>> Since the TX queues were re-enabled in the error exit path of
>> ath10k_mac_tx_push_txq(), mac80211 can continue pushing data to the
>> driver. If the hardware does not consume the data, the above mentioned
>> case will be repeated over and over.
>>
>> A case when the hardware is not able to transmit the data from the host
>> is when a STA has been dis-associated from an AP and has not yet been
>> able to re-associate. In this case there will be no TX_COMPL_INDs from
>> the hardware, resulting in the TX counter not be decremented.
>>
>> This phenomenon has been observed in both a real and a test setup.
>>
>> In order to fix this, the actual TX locking (the call to
>> ath10k_mac_tx_lock()) was removed from ath10k_htt_tx_inc_pending().
>> Instead, ath10k_mac_tx_lock() is called separately after the skb has
>> been fetched (after the call to ieee80211_tx_dequeue()). At this point
>> it is OK to stop the queues.
> Is this patch will impact throughput?
No, I haven't seen any performance degradation.
I can't see that it should impact the throughput even in theory (rather the opposite
actually since we reduce the likelihood of dropping packets).
^ permalink raw reply
* [PATCH] rtlwifi: rtl_pci: Fix memory leak then hardware init fails
From: Larry Finger @ 2019-08-26 20:34 UTC (permalink / raw)
To: kvalo; +Cc: linux-wireless, pkshih, Larry Finger
If the call to hw_init() fails for any of the drivers, the driver will
leak memory that was allocated in BT coexistence setup. Technically, each
of the drivers should have done this free; however placing it in rtl_pci
fixes all the drivers with only a single patch.
Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
---
Kalle,
This bug has been present in the driver since it was added to the
kernel, there is no particular rush in applying it.
Larry
drivers/net/wireless/realtek/rtlwifi/pci.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/wireless/realtek/rtlwifi/pci.c b/drivers/net/wireless/realtek/rtlwifi/pci.c
index 4055e0ab75ba..c275fc932adb 100644
--- a/drivers/net/wireless/realtek/rtlwifi/pci.c
+++ b/drivers/net/wireless/realtek/rtlwifi/pci.c
@@ -1793,6 +1793,8 @@ static int rtl_pci_start(struct ieee80211_hw *hw)
if (err) {
RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
"Failed to config hardware!\n");
+ kfree(rtlpriv->btcoexist.btc_context);
+ kfree(rtlpriv->btcoexist.wifi_only_context);
return err;
}
rtlpriv->cfg->ops->set_hw_reg(hw, HW_VAR_RETRY_LIMIT,
--
2.22.1
^ permalink raw reply related
* [PATCH v2] rtlwifi: rtl_pci: Fix memory leak when hardware init fails
From: Larry Finger @ 2019-08-26 22:03 UTC (permalink / raw)
To: kvalo; +Cc: linux-wireless, pkshih, Larry Finger
If the call to hw_init() fails for any of the drivers, the driver will
leak memory that was allocated in BT coexistence setup. Technically, each
of the drivers should have done this free; however placing it in rtl_pci
fixes all the drivers with only a single patch.
Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
---
v2 - Fix typo in subject line
---
Kalle,
This bug has been present in the driver since it was added to the
kernel, there is no particular rush in applying it.
Larry
---
drivers/net/wireless/realtek/rtlwifi/pci.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/wireless/realtek/rtlwifi/pci.c b/drivers/net/wireless/realtek/rtlwifi/pci.c
index 4055e0ab75ba..c275fc932adb 100644
--- a/drivers/net/wireless/realtek/rtlwifi/pci.c
+++ b/drivers/net/wireless/realtek/rtlwifi/pci.c
@@ -1793,6 +1793,8 @@ static int rtl_pci_start(struct ieee80211_hw *hw)
if (err) {
RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
"Failed to config hardware!\n");
+ kfree(rtlpriv->btcoexist.btc_context);
+ kfree(rtlpriv->btcoexist.wifi_only_context);
return err;
}
rtlpriv->cfg->ops->set_hw_reg(hw, HW_VAR_RETRY_LIMIT,
--
2.22.1
^ permalink raw reply related
* [PATCH,RFC] ath10k: Fix skb->len (properly) in ath10k_sdio_mbox_rx_packet
From: Nicolas Boichat @ 2019-08-27 0:33 UTC (permalink / raw)
To: Kalle Valo
Cc: David S . Miller, ath10k, linux-wireless, netdev, linux-kernel,
wgong, Niklas Cassel, Alagu Sankar, briannorris, tientzu
(not a formal patch, take this as a bug report for now, I can clean
up depending on the feedback I get here)
There's at least 3 issues here, and the patch fixes 2/3 only, I'm not sure
how/if 1 should be handled.
1. ath10k_sdio_mbox_rx_alloc allocating skb of a incorrect size (too
small)
2. ath10k_sdio_mbox_rx_packet calling skb_put with that incorrect size.
3. ath10k_sdio_mbox_rx_process_packet attempts to fixup the size, but
does not use proper skb_put commands to do so, so we end up with
a mismatch between skb->head + skb->tail and skb->data + skb->len.
Let's start with 3, this is quite serious as this and causes corruptions
in the TCP stack, as the stack tries to coalesce packets, and relies on
skb->tail being correct (that is, skb_tail_pointer must point to the
first byte _after_ the data): one must never manipulate skb->len
directly.
Instead, we need to use skb_put to allocate more space (which updates
skb->len and skb->tail). But it seems odd to do that in
ath10k_sdio_mbox_rx_process_packet, so I move the code to
ath10k_sdio_mbox_rx_packet (point 2 above).
However, there is still something strange (point 1 above), why is
ath10k_sdio_mbox_rx_alloc allocating packets of the incorrect
(too small?) size? What happens if the packet is bigger than alloc_len?
Does this lead to corruption/lost data?
Fixes: 8530b4e7b22bc3b ("ath10k: sdio: set skb len for all rx packets")
Signed-off-by: Nicolas Boichat <drinkcat@chromium.org>
---
One simple way to test this is this scriplet, that sends a lot of
small packets over SSH:
(for i in `seq 1 300`; do echo $i; sleep 0.1; done) | ssh $IP cat
In my testing it rarely ever reach 300 without failure.
drivers/net/wireless/ath/ath10k/sdio.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/sdio.c b/drivers/net/wireless/ath/ath10k/sdio.c
index 8ed4fbd8d6c3888..a9f5002863ee7bb 100644
--- a/drivers/net/wireless/ath/ath10k/sdio.c
+++ b/drivers/net/wireless/ath/ath10k/sdio.c
@@ -381,16 +381,14 @@ static int ath10k_sdio_mbox_rx_process_packet(struct ath10k *ar,
struct ath10k_htc_hdr *htc_hdr = (struct ath10k_htc_hdr *)skb->data;
bool trailer_present = htc_hdr->flags & ATH10K_HTC_FLAG_TRAILER_PRESENT;
enum ath10k_htc_ep_id eid;
- u16 payload_len;
u8 *trailer;
int ret;
- payload_len = le16_to_cpu(htc_hdr->len);
- skb->len = payload_len + sizeof(struct ath10k_htc_hdr);
+ /* TODO: Remove this? */
+ WARN_ON(skb->len != le16_to_cpu(htc_hdr->len) + sizeof(*htc_hdr));
if (trailer_present) {
- trailer = skb->data + sizeof(*htc_hdr) +
- payload_len - htc_hdr->trailer_len;
+ trailer = skb->data + skb->len - htc_hdr->trailer_len;
eid = pipe_id_to_eid(htc_hdr->eid);
@@ -637,8 +635,16 @@ static int ath10k_sdio_mbox_rx_packet(struct ath10k *ar,
ret = ath10k_sdio_readsb(ar, ar_sdio->mbox_info.htc_addr,
skb->data, pkt->alloc_len);
pkt->status = ret;
- if (!ret)
+ if (!ret) {
+ /* Update actual length. */
+ /* FIXME: This looks quite wrong, why is pkt->act_len not
+ * correct in the first place?
+ */
+ struct ath10k_htc_hdr *htc_hdr =
+ (struct ath10k_htc_hdr *)skb->data;
+ pkt->act_len = le16_to_cpu(htc_hdr->len) + sizeof(*htc_hdr);
skb_put(skb, pkt->act_len);
+ }
return ret;
}
--
2.23.0.187.g17f5b7556c-goog
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox