* Re: [PATCHv6 1/9] nl80211: New netlink command for TID specific configuration
From: Sergey Matyukevich @ 2019-07-11 12:51 UTC (permalink / raw)
To: Tamizh chelvam
Cc: johannes@sipsolutions.net, ath10k@lists.infradead.org,
linux-wireless@vger.kernel.org
In-Reply-To: <1560835632-17405-2-git-send-email-tamizhr@codeaurora.org>
> Add a new NL command, NL80211_CMD_SET_TID_CONFIG to support
> data TID specific configuration. This per TID configurations
> are passed in NL80211_ATTR_TID_CONFIG which is a nested
> attribute. This patch adds support to configure per TID
> noack policy through NL80211_TID_ATTR_CONFIG_NOACK attribute.
> Data TID value for this configuration will be passed through
> NL80211_TID_ATTR_CONFIG_TID attribute. When the user-space wants
> this configuration peer specific rather than being applied for
> all the connected stations, MAC address of the peer can be passed
> in NL80211_ATTR_MAC attribute. This patch introduced
> enum ieee80211_tid_conf_mask to notify the driver that which
> configuration modified.
> Driver supporting data TID specific noack policy configuration
> should be advertise through NL80211_EXT_FEATURE_PER_TID_NOACK_CONFIG
> and supporting per STA data TID noack policy configuration
> should be advertise through NL80211_EXT_FEATURE_PER_STA_NOACK_CONFIG
>
> Signed-off-by: Tamizh chelvam <tamizhr@codeaurora.org>
...
> +static int parse_tid_conf(struct cfg80211_registered_device *rdev,
> + struct nlattr *attrs[],
> + struct ieee80211_tid_cfg *tid_conf,
> + struct genl_info *info, const u8 *peer)
> +{
...
> +static int nl80211_set_tid_config(struct sk_buff *skb,
> + struct genl_info *info)
> +{
> + struct cfg80211_registered_device *rdev = info->user_ptr[0];
> + struct nlattr *attrs[NL80211_TID_ATTR_CONFIG_MAX + 1];
> + struct net_device *dev = info->user_ptr[1];
> + struct ieee80211_tid_config *tid_config;
> + struct nlattr *tid;
> + int conf_idx = 0, rem_conf;
> + int ret = -EINVAL;
> + u32 num_conf = 0;
> +
> + if (!info->attrs[NL80211_ATTR_TID_CONFIG])
> + return -EINVAL;
> +
> + if (!rdev->ops->set_tid_config)
> + return -EOPNOTSUPP;
> +
> + nla_for_each_nested(tid, info->attrs[NL80211_ATTR_TID_CONFIG],
> + rem_conf)
> + num_conf++;
> +
> + tid_config = kzalloc(struct_size(tid_config, tid_conf, num_conf),
> + GFP_KERNEL);
> + if (!tid_config)
> + return -ENOMEM;
> +
> + tid_config->n_tid_conf = num_conf;
> +
> + if (info->attrs[NL80211_ATTR_MAC])
> + tid_config->peer = nla_data(info->attrs[NL80211_ATTR_MAC]);
> +
> + nla_for_each_nested(tid, info->attrs[NL80211_ATTR_TID_CONFIG],
> + rem_conf) {
> + ret = nla_parse_nested_deprecated(attrs, NL80211_TID_ATTR_CONFIG_MAX,
> + tid, NULL, NULL);
> +
> + if (ret)
> + goto bad_tid_conf;
> +
> + if (!attrs[NL80211_TID_ATTR_CONFIG_TID]) {
> + ret = -EINVAL;
> + goto bad_tid_conf;
> + }
> +
> + ret = parse_tid_conf(rdev, attrs,
> + &tid_config->tid_conf[conf_idx],
> + tid_config->peer);
Argument 'info' is missing here from parse_tid_conf. IIUC, appropriate
fixup is included into patch #5. So it looks like rebase issue. But it
makes sense to fix it since this issue makes the patch set non-bisectable.
Regareds,
Sergey
^ permalink raw reply
* Re: [RFC 0/8] nl80211: add 6GHz band support
From: Arend Van Spriel @ 2019-07-11 11:30 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless, Jouni Malinen, Tova Mussai
In-Reply-To: <48a43287e65e1d24082dea2cc32ca14998acb8d1.camel@sipsolutions.net>
On 6/28/2019 3:04 PM, Johannes Berg wrote:
> Hi Arend, all,
>
> Sorry. No, my thoughts aren't really more concrete, but Tova is starting
> to work on this now.
>
>> This came up in discussion with my colleagues today and I would say from
>> mac80211 perspective there is more to it than just scanning. In short
>> the 6GHz band is for HE-only operation so for example only HE rates may
>> be used. As the bitrates are in ieee80211_supported_band having a
>> separate 6GHz band seems to have a (slight?) advantage.
>
> Hmm, that's a good point too, I hadn't really looked _too_ much at 6GHz
> stuff.
>
> Jouni, what do you think?
>
> Perhaps we should just have both. I mean, we can treat this as a
> separate band, and still have code to handle operating classes properly,
> right?
I assume user-space does not necessarily need the band, but hostapd
needs to be aware that it is operating in 6GHz to setup the correct
(information) elements in the beacon. Obviously the operating classes
can be used there, but I don't think there is any issue with nl80211
exposing a 6G band.
Regards,
Arend
^ permalink raw reply
* Re: [PATCH 1/3] cfg80211: clean up cfg80211_inform_single_bss_frame_data()
From: Sergey Matyukevich @ 2019-07-11 9:31 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless@vger.kernel.org, Johannes Berg
In-Reply-To: <20190703133823.10530-1-johannes@sipsolutions.net>
> From: Johannes Berg <johannes.berg@intel.com>
>
> cfg80211_inform_single_bss_frame_data() doesn't need the
> non_tx_data data argument since it's always NULL.
>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
For all 3 patches:
Reviewed-by: Sergey Matyukevich <sergey.matyukevich.os@quantenna.com>
Regards,
Sergey
^ permalink raw reply
* [PATCH 4/7] brcmfmac: clear events in brcmf_fweh_detach() will always fail
From: Arend van Spriel @ 2019-07-11 9:05 UTC (permalink / raw)
To: Kalle Valo; +Cc: linux-wireless, Arend van Spriel
In-Reply-To: <1562835912-1404-1-git-send-email-arend.vanspriel@broadcom.com>
Clearing firmware events in brcmf_fweh_detach() is always failing
because it is called only upon driver remove and communication
with firmware is no longer possible.
Reviewed-by: Hante Meuleman <hante.meuleman@broadcom.com>
Reviewed-by: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com>
Reviewed-by: Franky Lin <franky.lin@broadcom.com>
Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c | 9 ---------
1 file changed, 9 deletions(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c
index adedd4f..79c8a85 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c
@@ -303,16 +303,7 @@ void brcmf_fweh_attach(struct brcmf_pub *drvr)
void brcmf_fweh_detach(struct brcmf_pub *drvr)
{
struct brcmf_fweh_info *fweh = &drvr->fweh;
- struct brcmf_if *ifp = brcmf_get_ifp(drvr, 0);
- s8 eventmask[BRCMF_EVENTING_MASK_LEN];
- if (ifp) {
- /* clear all events */
- memset(eventmask, 0, BRCMF_EVENTING_MASK_LEN);
- (void)brcmf_fil_iovar_data_set(ifp, "event_msgs",
- eventmask,
- BRCMF_EVENTING_MASK_LEN);
- }
/* cancel the worker */
cancel_work_sync(&fweh->event_work);
WARN_ON(!list_empty(&fweh->event_q));
--
1.9.1
^ permalink raw reply related
* [PATCH 5/7] brcmfmac: avoid firmware commands when bus is down
From: Arend van Spriel @ 2019-07-11 9:05 UTC (permalink / raw)
To: Kalle Valo; +Cc: linux-wireless, Arend van Spriel
In-Reply-To: <1562835912-1404-1-git-send-email-arend.vanspriel@broadcom.com>
Upon rmmod a few attempts are made to inform firmware, but there is
no point as the bus is down and these will fail. Avoid them to keep
the logs clean.
Reported-by: Stefan Wahren <stefan.wahren@i2se.com>
Reviewed-by: Hante Meuleman <hante.meuleman@broadcom.com>
Reviewed-by: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com>
Reviewed-by: Franky Lin <franky.lin@broadcom.com>
Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
---
.../broadcom/brcm80211/brcmfmac/cfg80211.c | 23 ++++++++++++----------
1 file changed, 13 insertions(+), 10 deletions(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index b6d0df3..4e3e9d44 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -1267,17 +1267,21 @@ static void brcmf_link_down(struct brcmf_cfg80211_vif *vif, u16 reason)
{
struct brcmf_cfg80211_info *cfg = wiphy_to_cfg(vif->wdev.wiphy);
struct brcmf_pub *drvr = cfg->pub;
+ bool bus_up = drvr->bus_if->state == BRCMF_BUS_UP;
s32 err = 0;
brcmf_dbg(TRACE, "Enter\n");
if (test_and_clear_bit(BRCMF_VIF_STATUS_CONNECTED, &vif->sme_state)) {
- brcmf_dbg(INFO, "Call WLC_DISASSOC to stop excess roaming\n");
- err = brcmf_fil_cmd_data_set(vif->ifp,
- BRCMF_C_DISASSOC, NULL, 0);
- if (err) {
- bphy_err(drvr, "WLC_DISASSOC failed (%d)\n", err);
+ if (bus_up) {
+ brcmf_dbg(INFO, "Call WLC_DISASSOC to stop excess roaming\n");
+ err = brcmf_fil_cmd_data_set(vif->ifp,
+ BRCMF_C_DISASSOC, NULL, 0);
+ if (err)
+ bphy_err(drvr, "WLC_DISASSOC failed (%d)\n",
+ err);
}
+
if ((vif->wdev.iftype == NL80211_IFTYPE_STATION) ||
(vif->wdev.iftype == NL80211_IFTYPE_P2P_CLIENT))
cfg80211_disconnected(vif->wdev.netdev, reason, NULL, 0,
@@ -1287,7 +1291,8 @@ static void brcmf_link_down(struct brcmf_cfg80211_vif *vif, u16 reason)
clear_bit(BRCMF_SCAN_STATUS_SUPPRESS, &cfg->scan_status);
brcmf_btcoex_set_mode(vif, BRCMF_BTCOEX_ENABLED, 0);
if (vif->profile.use_fwsup != BRCMF_PROFILE_FWSUP_NONE) {
- brcmf_set_pmk(vif->ifp, NULL, 0);
+ if (bus_up)
+ brcmf_set_pmk(vif->ifp, NULL, 0);
vif->profile.use_fwsup = BRCMF_PROFILE_FWSUP_NONE;
}
brcmf_dbg(TRACE, "Exit\n");
@@ -4985,18 +4990,16 @@ static int brcmf_cfg80211_get_channel(struct wiphy *wiphy,
struct brcmf_cfg80211_info *cfg = wiphy_to_cfg(wiphy);
struct net_device *ndev = wdev->netdev;
struct brcmf_pub *drvr = cfg->pub;
- struct brcmf_if *ifp;
struct brcmu_chan ch;
enum nl80211_band band = 0;
enum nl80211_chan_width width = 0;
u32 chanspec;
int freq, err;
- if (!ndev)
+ if (!ndev || drvr->bus_if->state != BRCMF_BUS_UP)
return -ENODEV;
- ifp = netdev_priv(ndev);
- err = brcmf_fil_iovar_int_get(ifp, "chanspec", &chanspec);
+ err = brcmf_fil_iovar_int_get(netdev_priv(ndev), "chanspec", &chanspec);
if (err) {
bphy_err(drvr, "chanspec failed (%d)\n", err);
return err;
--
1.9.1
^ permalink raw reply related
* [PATCH 2/7] brcmfmac: change the order of things in brcmf_detach()
From: Arend van Spriel @ 2019-07-11 9:05 UTC (permalink / raw)
To: Kalle Valo; +Cc: linux-wireless, Arend van Spriel
In-Reply-To: <1562835912-1404-1-git-send-email-arend.vanspriel@broadcom.com>
When brcmf_detach() from the bus layer upon rmmod we can no longer
communicate. Hence we will set the bus state to DOWN and cleanup
the event and protocol layer. The network interfaces need to be
deleted before brcmf_cfg80211_detach() because the latter does the
wiphy_unregister() which issues a warning if there are still network
devices linked to the wiphy instance.
Reviewed-by: Hante Meuleman <hante.meuleman@broadcom.com>
Reviewed-by: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com>
Reviewed-by: Franky Lin <franky.lin@broadcom.com>
Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
---
.../wireless/broadcom/brcm80211/brcmfmac/core.c | 27 +++++++++++-----------
1 file changed, 14 insertions(+), 13 deletions(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
index fda6044..80d54d2 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
@@ -1307,25 +1307,26 @@ void brcmf_detach(struct device *dev)
unregister_inet6addr_notifier(&drvr->inet6addr_notifier);
#endif
- /* stop firmware event handling */
- brcmf_fweh_detach(drvr);
- if (drvr->config)
- brcmf_p2p_detach(&drvr->config->p2p);
-
brcmf_bus_change_state(bus_if, BRCMF_BUS_DOWN);
-
- /* make sure primary interface removed last */
- for (i = BRCMF_MAX_IFS-1; i > -1; i--)
- brcmf_remove_interface(drvr->iflist[i], false);
-
- brcmf_cfg80211_detach(drvr->config);
- drvr->config = NULL;
-
brcmf_bus_stop(drvr->bus_if);
+ brcmf_fweh_detach(drvr);
brcmf_proto_detach(drvr);
+ /* make sure primary interface removed last */
+ for (i = BRCMF_MAX_IFS - 1; i > -1; i--) {
+ if (drvr->iflist[i])
+ brcmf_del_if(drvr, drvr->iflist[i]->bsscfgidx, false);
+ }
+
+ if (drvr->config) {
+ brcmf_p2p_detach(&drvr->config->p2p);
+ brcmf_cfg80211_detach(drvr->config);
+ drvr->config = NULL;
+ }
+
bus_if->drvr = NULL;
+
wiphy_free(drvr->wiphy);
}
--
1.9.1
^ permalink raw reply related
* [PATCH 6/7] brcmfmac: simply remove flowring if bus is down
From: Arend van Spriel @ 2019-07-11 9:05 UTC (permalink / raw)
To: Kalle Valo; +Cc: linux-wireless, Arend van Spriel
In-Reply-To: <1562835912-1404-1-git-send-email-arend.vanspriel@broadcom.com>
When the bus is down, eg. due to rmmod, there is no need to
attempt to inform firmware about it.
Reviewed-by: Hante Meuleman <hante.meuleman@broadcom.com>
Reviewed-by: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com>
Reviewed-by: Franky Lin <franky.lin@broadcom.com>
Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c
index 241747b..8428be8 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c
@@ -1398,6 +1398,13 @@ void brcmf_msgbuf_delete_flowring(struct brcmf_pub *drvr, u16 flowid)
u8 ifidx;
int err;
+ /* no need to submit if firmware can not be reached */
+ if (drvr->bus_if->state != BRCMF_BUS_UP) {
+ brcmf_dbg(MSGBUF, "bus down, flowring will be removed\n");
+ brcmf_msgbuf_remove_flowring(msgbuf, flowid);
+ return;
+ }
+
commonring = msgbuf->commonrings[BRCMF_H2D_MSGRING_CONTROL_SUBMIT];
brcmf_commonring_lock(commonring);
ret_ptr = brcmf_commonring_reserve_for_write(commonring);
--
1.9.1
^ permalink raw reply related
* [PATCH 7/7] brcmfmac: remove unnecessary strlcpy() upon obtaining "ver" iovar
From: Arend van Spriel @ 2019-07-11 9:05 UTC (permalink / raw)
To: Kalle Valo; +Cc: linux-wireless, Arend van Spriel
In-Reply-To: <1562835912-1404-1-git-send-email-arend.vanspriel@broadcom.com>
Recently a strcpy() was replaced by strlcpy(). However, the strcpy()
was not needed in the first place. So removing that line of code.
Reviewed-by: Hante Meuleman <hante.meuleman@broadcom.com>
Reviewed-by: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com>
Reviewed-by: Franky Lin <franky.lin@broadcom.com>
Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
index aa89d62..dec25e4 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
@@ -258,7 +258,6 @@ int brcmf_c_preinit_dcmds(struct brcmf_if *ifp)
/* query for 'ver' to get version info from firmware */
memset(buf, 0, sizeof(buf));
- strlcpy(buf, "ver", sizeof(buf));
err = brcmf_fil_iovar_data_get(ifp, "ver", buf, sizeof(buf));
if (err < 0) {
bphy_err(drvr, "Retrieving version information failed, %d\n",
--
1.9.1
^ permalink raw reply related
* [PATCH 1/7] Revert "brcmfmac: fix NULL pointer derefence during USB disconnect"
From: Arend van Spriel @ 2019-07-11 9:05 UTC (permalink / raw)
To: Kalle Valo; +Cc: linux-wireless, Arend van Spriel
In-Reply-To: <1562835912-1404-1-git-send-email-arend.vanspriel@broadcom.com>
This reverts commit 5cdb0ef6144f47440850553579aa923c20a63f23. Subsequent
changes make rework the driver code fixing the issue differently.
Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcdc.c | 11 ++---------
drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcdc.h | 6 ++----
drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c | 4 +---
.../net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.c | 16 ++++------------
.../net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.h | 3 +--
drivers/net/wireless/broadcom/brcm80211/brcmfmac/proto.c | 10 ++--------
drivers/net/wireless/broadcom/brcm80211/brcmfmac/proto.h | 3 +--
7 files changed, 13 insertions(+), 40 deletions(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcdc.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcdc.c
index 322e913..2c95a08 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcdc.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcdc.c
@@ -479,18 +479,11 @@ int brcmf_proto_bcdc_attach(struct brcmf_pub *drvr)
return -ENOMEM;
}
-void brcmf_proto_bcdc_detach_pre_delif(struct brcmf_pub *drvr)
-{
- struct brcmf_bcdc *bcdc = drvr->proto->pd;
-
- brcmf_fws_detach_pre_delif(bcdc->fws);
-}
-
-void brcmf_proto_bcdc_detach_post_delif(struct brcmf_pub *drvr)
+void brcmf_proto_bcdc_detach(struct brcmf_pub *drvr)
{
struct brcmf_bcdc *bcdc = drvr->proto->pd;
drvr->proto->pd = NULL;
- brcmf_fws_detach_post_delif(bcdc->fws);
+ brcmf_fws_detach(bcdc->fws);
kfree(bcdc);
}
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcdc.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcdc.h
index 102e693..b051d28 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcdc.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcdc.h
@@ -7,16 +7,14 @@
#ifdef CONFIG_BRCMFMAC_PROTO_BCDC
int brcmf_proto_bcdc_attach(struct brcmf_pub *drvr);
-void brcmf_proto_bcdc_detach_pre_delif(struct brcmf_pub *drvr);
-void brcmf_proto_bcdc_detach_post_delif(struct brcmf_pub *drvr);
+void brcmf_proto_bcdc_detach(struct brcmf_pub *drvr);
void brcmf_proto_bcdc_txflowblock(struct device *dev, bool state);
void brcmf_proto_bcdc_txcomplete(struct device *dev, struct sk_buff *txp,
bool success);
struct brcmf_fws_info *drvr_to_fws(struct brcmf_pub *drvr);
#else
static inline int brcmf_proto_bcdc_attach(struct brcmf_pub *drvr) { return 0; }
-static void brcmf_proto_bcdc_detach_pre_delif(struct brcmf_pub *drvr) {};
-static inline void brcmf_proto_bcdc_detach_post_delif(struct brcmf_pub *drvr) {}
+static inline void brcmf_proto_bcdc_detach(struct brcmf_pub *drvr) {}
#endif
#endif /* BRCMFMAC_BCDC_H */
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
index bf18491..fda6044 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
@@ -1314,8 +1314,6 @@ void brcmf_detach(struct device *dev)
brcmf_bus_change_state(bus_if, BRCMF_BUS_DOWN);
- brcmf_proto_detach_pre_delif(drvr);
-
/* make sure primary interface removed last */
for (i = BRCMF_MAX_IFS-1; i > -1; i--)
brcmf_remove_interface(drvr->iflist[i], false);
@@ -1325,7 +1323,7 @@ void brcmf_detach(struct device *dev)
brcmf_bus_stop(drvr->bus_if);
- brcmf_proto_detach_post_delif(drvr);
+ brcmf_proto_detach(drvr);
bus_if->drvr = NULL;
wiphy_free(drvr->wiphy);
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.c
index b8452cb..2bd892d 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.c
@@ -2432,25 +2432,17 @@ struct brcmf_fws_info *brcmf_fws_attach(struct brcmf_pub *drvr)
return fws;
fail:
- brcmf_fws_detach_pre_delif(fws);
- brcmf_fws_detach_post_delif(fws);
+ brcmf_fws_detach(fws);
return ERR_PTR(rc);
}
-void brcmf_fws_detach_pre_delif(struct brcmf_fws_info *fws)
+void brcmf_fws_detach(struct brcmf_fws_info *fws)
{
if (!fws)
return;
- if (fws->fws_wq) {
- destroy_workqueue(fws->fws_wq);
- fws->fws_wq = NULL;
- }
-}
-void brcmf_fws_detach_post_delif(struct brcmf_fws_info *fws)
-{
- if (!fws)
- return;
+ if (fws->fws_wq)
+ destroy_workqueue(fws->fws_wq);
/* cleanup */
brcmf_fws_lock(fws);
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.h
index 10184ee..b486d57 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.h
@@ -7,8 +7,7 @@
#define FWSIGNAL_H_
struct brcmf_fws_info *brcmf_fws_attach(struct brcmf_pub *drvr);
-void brcmf_fws_detach_pre_delif(struct brcmf_fws_info *fws);
-void brcmf_fws_detach_post_delif(struct brcmf_fws_info *fws);
+void brcmf_fws_detach(struct brcmf_fws_info *fws);
void brcmf_fws_debugfs_create(struct brcmf_pub *drvr);
bool brcmf_fws_queue_skbs(struct brcmf_fws_info *fws);
bool brcmf_fws_fc_active(struct brcmf_fws_info *fws);
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/proto.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/proto.c
index e3d1b07..2e911d4 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/proto.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/proto.c
@@ -56,22 +56,16 @@ int brcmf_proto_attach(struct brcmf_pub *drvr)
return -ENOMEM;
}
-void brcmf_proto_detach_post_delif(struct brcmf_pub *drvr)
+void brcmf_proto_detach(struct brcmf_pub *drvr)
{
brcmf_dbg(TRACE, "Enter\n");
if (drvr->proto) {
if (drvr->bus_if->proto_type == BRCMF_PROTO_BCDC)
- brcmf_proto_bcdc_detach_post_delif(drvr);
+ brcmf_proto_bcdc_detach(drvr);
else if (drvr->bus_if->proto_type == BRCMF_PROTO_MSGBUF)
brcmf_proto_msgbuf_detach(drvr);
kfree(drvr->proto);
drvr->proto = NULL;
}
}
-
-void brcmf_proto_detach_pre_delif(struct brcmf_pub *drvr)
-{
- if (drvr->proto && drvr->bus_if->proto_type == BRCMF_PROTO_BCDC)
- brcmf_proto_bcdc_detach_pre_delif(drvr);
-}
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/proto.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/proto.h
index 8d55fad..bd08d3a 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/proto.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/proto.h
@@ -43,8 +43,7 @@ struct brcmf_proto {
int brcmf_proto_attach(struct brcmf_pub *drvr);
-void brcmf_proto_detach_pre_delif(struct brcmf_pub *drvr);
-void brcmf_proto_detach_post_delif(struct brcmf_pub *drvr);
+void brcmf_proto_detach(struct brcmf_pub *drvr);
static inline int brcmf_proto_hdrpull(struct brcmf_pub *drvr, bool do_fws,
struct sk_buff *skb,
--
1.9.1
^ permalink raw reply related
* [PATCH 3/7] brcmfmac: avoid firmware command in brcmf_netdev_open() when bus is down
From: Arend van Spriel @ 2019-07-11 9:05 UTC (permalink / raw)
To: Kalle Valo; +Cc: linux-wireless, Arend van Spriel
In-Reply-To: <1562835912-1404-1-git-send-email-arend.vanspriel@broadcom.com>
No point in sending a firmware command when bus is down so make it
conditional checking the state.
Reviewed-by: Hante Meuleman <hante.meuleman@broadcom.com>
Reviewed-by: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com>
Reviewed-by: Franky Lin <franky.lin@broadcom.com>
Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
index 80d54d2..705b8cc 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
@@ -579,7 +579,8 @@ static int brcmf_netdev_stop(struct net_device *ndev)
brcmf_cfg80211_down(ndev);
- brcmf_fil_iovar_data_set(ifp, "arp_hostip_clear", NULL, 0);
+ if (ifp->drvr->bus_if->state == BRCMF_BUS_UP)
+ brcmf_fil_iovar_data_set(ifp, "arp_hostip_clear", NULL, 0);
brcmf_net_setcarrier(ifp, false);
--
1.9.1
^ permalink raw reply related
* [PATCH 0/7] brcmfmac: rework probe/attach sequence
From: Arend van Spriel @ 2019-07-11 9:05 UTC (permalink / raw)
To: Kalle Valo; +Cc: linux-wireless, Arend van Spriel
The brcmfmac driver spews some error message upon unloading and Stefan
Wahren was wondering whether it could be cleaned up. Related to this
was a recent fix for NULL pointer deref. That fix introduced a construct
that added to the itch to rework the probe sequence. So this series
reverts commit 5cdb0ef6144f ("brcmfmac: fix NULL pointer derefence during
USB disconnect").
The changes in this series are:
* reorder brcmf_detach() code.
* avoid firmware interaction when bus is down.
* remove strlcpy() before issueing firmware version iovar.
This series applies to the master branch of the wireless-drivers-next
repository.
Arend van Spriel (7):
Revert "brcmfmac: fix NULL pointer derefence during USB disconnect"
brcmfmac: change the order of things in brcmf_detach()
brcmfmac: avoid firmware command in brcmf_netdev_open() when bus is
down
brcmfmac: clear events in brcmf_fweh_detach() will always fail
brcmfmac: avoid firmware commands when bus is down
brcmfmac: simply remove flowring if bus is down
brcmfmac: remove unnecessary strlcpy() upon obtaining "ver" iovar
.../wireless/broadcom/brcm80211/brcmfmac/bcdc.c | 11 ++------
.../wireless/broadcom/brcm80211/brcmfmac/bcdc.h | 6 ++---
.../broadcom/brcm80211/brcmfmac/cfg80211.c | 23 +++++++++--------
.../wireless/broadcom/brcm80211/brcmfmac/common.c | 1 -
.../wireless/broadcom/brcm80211/brcmfmac/core.c | 30 +++++++++++-----------
.../wireless/broadcom/brcm80211/brcmfmac/fweh.c | 9 -------
.../broadcom/brcm80211/brcmfmac/fwsignal.c | 16 +++---------
.../broadcom/brcm80211/brcmfmac/fwsignal.h | 3 +--
.../wireless/broadcom/brcm80211/brcmfmac/msgbuf.c | 7 +++++
.../wireless/broadcom/brcm80211/brcmfmac/proto.c | 10 ++------
.../wireless/broadcom/brcm80211/brcmfmac/proto.h | 3 +--
11 files changed, 47 insertions(+), 72 deletions(-)
--
1.9.1
^ permalink raw reply
* [PATCH 2/3] brcmfmac: enable DFS_OFFLOAD extended feature if supported
From: Arend van Spriel @ 2019-07-11 8:45 UTC (permalink / raw)
To: Kalle Valo; +Cc: linux-wireless, Arend van Spriel
In-Reply-To: <1562834732-31508-1-git-send-email-arend.vanspriel@broadcom.com>
If the firmware supports 802.11h and the device can operate in 5GHz
band we can enable DFS_OFFLOAD extended feature.
Reviewed-by: Hante Meuleman <hante.meuleman@broadcom.com>
Reviewed-by: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com>
Reviewed-by: Franky Lin <franky.lin@broadcom.com>
Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 5 +++++
drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c | 1 +
drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.h | 4 +++-
3 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index 5168d42..3f72dc1 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -6733,6 +6733,11 @@ static int brcmf_setup_wiphy(struct wiphy *wiphy, struct brcmf_if *ifp)
}
}
+ if (wiphy->bands[NL80211_BAND_5GHZ] &&
+ brcmf_feat_is_enabled(ifp, BRCMF_FEAT_DOT11H))
+ wiphy_ext_feature_set(wiphy,
+ NL80211_EXT_FEATURE_DFS_OFFLOAD);
+
wiphy_read_of_freq_limits(wiphy);
return 0;
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
index 73aff4e..2c3526a 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
@@ -39,6 +39,7 @@ struct brcmf_feat_fwcap {
{ BRCMF_FEAT_P2P, "p2p" },
{ BRCMF_FEAT_MONITOR, "monitor" },
{ BRCMF_FEAT_MONITOR_FMT_RADIOTAP, "rtap" },
+ { BRCMF_FEAT_DOT11H, "802.11h" }
};
#ifdef DEBUG
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.h
index f127eb2..736a817 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.h
@@ -25,6 +25,7 @@
* MONITOR: firmware can pass monitor packets to host.
* MONITOR_FMT_RADIOTAP: firmware provides monitor packets with radiotap header
* MONITOR_FMT_HW_RX_HDR: firmware provides monitor packets with hw/ucode header
+ * DOT11H: firmware supports 802.11h
*/
#define BRCMF_FEAT_LIST \
BRCMF_FEAT_DEF(MBSS) \
@@ -43,7 +44,8 @@
BRCMF_FEAT_DEF(FWSUP) \
BRCMF_FEAT_DEF(MONITOR) \
BRCMF_FEAT_DEF(MONITOR_FMT_RADIOTAP) \
- BRCMF_FEAT_DEF(MONITOR_FMT_HW_RX_HDR)
+ BRCMF_FEAT_DEF(MONITOR_FMT_HW_RX_HDR) \
+ BRCMF_FEAT_DEF(DOT11H)
/*
* Quirks:
--
1.9.1
^ permalink raw reply related
* [PATCH 1/3] brcmfmac: add 160MHz in chandef_to_chanspec()
From: Arend van Spriel @ 2019-07-11 8:45 UTC (permalink / raw)
To: Kalle Valo; +Cc: linux-wireless, Arend van Spriel
In-Reply-To: <1562834732-31508-1-git-send-email-arend.vanspriel@broadcom.com>
The function chandef_to_chanspec() was not handling 160MHz bandwidth
resulting in wrong encoding of the channel. That resulting in firmware
rejecting the provided channel specification.
Reviewed-by: Hante Meuleman <hante.meuleman@broadcom.com>
Reviewed-by: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com>
Reviewed-by: Franky Lin <franky.lin@broadcom.com>
Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
---
.../wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 21 ++++++++++++++++++++-
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index b6d0df3..5168d42 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -276,8 +276,26 @@ static u16 chandef_to_chanspec(struct brcmu_d11inf *d11inf,
else
ch_inf.sb = BRCMU_CHAN_SB_UU;
break;
- case NL80211_CHAN_WIDTH_80P80:
case NL80211_CHAN_WIDTH_160:
+ ch_inf.bw = BRCMU_CHAN_BW_160;
+ if (primary_offset == -70)
+ ch_inf.sb = BRCMU_CHAN_SB_LLL;
+ else if (primary_offset == -50)
+ ch_inf.sb = BRCMU_CHAN_SB_LLU;
+ else if (primary_offset == -30)
+ ch_inf.sb = BRCMU_CHAN_SB_LUL;
+ else if (primary_offset == -10)
+ ch_inf.sb = BRCMU_CHAN_SB_LUU;
+ else if (primary_offset == 10)
+ ch_inf.sb = BRCMU_CHAN_SB_ULL;
+ else if (primary_offset == 30)
+ ch_inf.sb = BRCMU_CHAN_SB_ULU;
+ else if (primary_offset == 50)
+ ch_inf.sb = BRCMU_CHAN_SB_UUL;
+ else
+ ch_inf.sb = BRCMU_CHAN_SB_UUU;
+ break;
+ case NL80211_CHAN_WIDTH_80P80:
case NL80211_CHAN_WIDTH_5:
case NL80211_CHAN_WIDTH_10:
default:
@@ -296,6 +314,7 @@ static u16 chandef_to_chanspec(struct brcmu_d11inf *d11inf,
}
d11inf->encchspec(&ch_inf);
+ brcmf_dbg(TRACE, "chanspec: 0x%x\n", ch_inf.chspec);
return ch_inf.chspec;
}
--
1.9.1
^ permalink raw reply related
* [PATCH 3/3] brcmfmac: allow 160MHz in custom regulatory rules
From: Arend van Spriel @ 2019-07-11 8:45 UTC (permalink / raw)
To: Kalle Valo; +Cc: linux-wireless, Arend van Spriel
In-Reply-To: <1562834732-31508-1-git-send-email-arend.vanspriel@broadcom.com>
The driver has custom regulatory rules which had maximum bandwidth
for 5GHz channels set to 80MHz. As a consequence the driver can
not use 160MHz in AP mode even when the device supports it. So
relax the rules allowing 160MHz. After wiphy_register() the channel
flags are updated according what the device actually supports.
Reviewed-by: Hante Meuleman <hante.meuleman@broadcom.com>
Reviewed-by: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com>
Reviewed-by: Franky Lin <franky.lin@broadcom.com>
Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index 3f72dc1..b692689 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -189,9 +189,9 @@ static bool check_vif_up(struct brcmf_cfg80211_vif *vif)
*/
REG_RULE(2484-10, 2484+10, 20, 6, 20, 0),
/* IEEE 802.11a, channel 36..64 */
- REG_RULE(5150-10, 5350+10, 80, 6, 20, 0),
+ REG_RULE(5150-10, 5350+10, 160, 6, 20, 0),
/* IEEE 802.11a, channel 100..165 */
- REG_RULE(5470-10, 5850+10, 80, 6, 20, 0), }
+ REG_RULE(5470-10, 5850+10, 160, 6, 20, 0), }
};
/* Note: brcmf_cipher_suites is an array of int defining which cipher suites
--
1.9.1
^ permalink raw reply related
* [PATCH 0/3] brcmfmac: 160MHz fixes and DFS offload
From: Arend van Spriel @ 2019-07-11 8:45 UTC (permalink / raw)
To: Kalle Valo; +Cc: linux-wireless, Arend van Spriel
The 160MHz AP mode was not properly working and limited due to lack of
DFS offload. So this series include following changes:
* encode 160MHz channel definition to firmware format.
* adapt custom regulatory rule allowing 160MHz.
* enable DFS offloading for firmwares supporting it.
The series applies to the master branch of the wireless-drivers-next
repository.
Arend van Spriel (3):
brcmfmac: add 160MHz in chandef_to_chanspec()
brcmfmac: enable DFS_OFFLOAD extended feature if supported
brcmfmac: allow 160MHz in custom regulatory rules
.../broadcom/brcm80211/brcmfmac/cfg80211.c | 30 +++++++++++++++++++---
.../wireless/broadcom/brcm80211/brcmfmac/feature.c | 1 +
.../wireless/broadcom/brcm80211/brcmfmac/feature.h | 4 ++-
3 files changed, 31 insertions(+), 4 deletions(-)
--
1.9.1
^ permalink raw reply
* Re: [PATCH v4 2/2] rtw88: pci: Use DMA sync instead of remapping in RX ISR
From: Jian-Hong Pan @ 2019-07-11 5:30 UTC (permalink / raw)
To: Yan-Hsuan Chuang, Kalle Valo, David S . Miller, Larry Finger,
David Laight, Christoph Hellwig
Cc: linux-wireless, Linux Netdev List, Linux Kernel,
Linux Upstreaming Team, Daniel Drake, stable
In-Reply-To: <20190711052427.5582-2-jian-hong@endlessm.com>
Jian-Hong Pan <jian-hong@endlessm.com> 於 2019年7月11日 週四 下午1:25寫道：
>
> Since each skb in RX ring is reused instead of new allocation, we can
> treat the DMA in a more efficient way by DMA synchronization.
>
> Signed-off-by: Jian-Hong Pan <jian-hong@endlessm.com>
> Cc: <stable@vger.kernel.org>
> ---
Sorry, also forget to place the version difference here
v2:
- New patch by following [PATCH v3 1/2] rtw88: pci: Rearrange the
memory usage for skb in RX ISR.
v3:
- Remove rtw_pci_sync_rx_desc_cpu and call dma_sync_single_for_cpu in
rtw_pci_rx_isr directly.
- Remove the return value of rtw_pci_sync_rx_desc_device.
- Use DMA_FROM_DEVICE instead of PCI_DMA_FROMDEVICE.
v4:
- Same as v3.
> drivers/net/wireless/realtek/rtw88/pci.c | 24 +++++++++++++++++++++---
> 1 file changed, 21 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/wireless/realtek/rtw88/pci.c b/drivers/net/wireless/realtek/rtw88/pci.c
> index c415f5e94fed..68fae52151dd 100644
> --- a/drivers/net/wireless/realtek/rtw88/pci.c
> +++ b/drivers/net/wireless/realtek/rtw88/pci.c
> @@ -206,6 +206,23 @@ static int rtw_pci_reset_rx_desc(struct rtw_dev *rtwdev, struct sk_buff *skb,
> return 0;
> }
>
> +static void rtw_pci_sync_rx_desc_device(struct rtw_dev *rtwdev, dma_addr_t dma,
> + struct rtw_pci_rx_ring *rx_ring,
> + u32 idx, u32 desc_sz)
> +{
> + struct device *dev = rtwdev->dev;
> + struct rtw_pci_rx_buffer_desc *buf_desc;
> + int buf_sz = RTK_PCI_RX_BUF_SIZE;
> +
> + dma_sync_single_for_device(dev, dma, buf_sz, DMA_FROM_DEVICE);
> +
> + buf_desc = (struct rtw_pci_rx_buffer_desc *)(rx_ring->r.head +
> + idx * desc_sz);
> + memset(buf_desc, 0, sizeof(*buf_desc));
> + buf_desc->buf_size = cpu_to_le16(RTK_PCI_RX_BUF_SIZE);
> + buf_desc->dma = cpu_to_le32(dma);
> +}
> +
> static int rtw_pci_init_rx_ring(struct rtw_dev *rtwdev,
> struct rtw_pci_rx_ring *rx_ring,
> u8 desc_size, u32 len)
> @@ -782,8 +799,8 @@ static void rtw_pci_rx_isr(struct rtw_dev *rtwdev, struct rtw_pci *rtwpci,
> rtw_pci_dma_check(rtwdev, ring, cur_rp);
> skb = ring->buf[cur_rp];
> dma = *((dma_addr_t *)skb->cb);
> - pci_unmap_single(rtwpci->pdev, dma, RTK_PCI_RX_BUF_SIZE,
> - PCI_DMA_FROMDEVICE);
> + dma_sync_single_for_cpu(rtwdev->dev, dma, RTK_PCI_RX_BUF_SIZE,
> + DMA_FROM_DEVICE);
> rx_desc = skb->data;
> chip->ops->query_rx_desc(rtwdev, rx_desc, &pkt_stat, &rx_status);
>
> @@ -818,7 +835,8 @@ static void rtw_pci_rx_isr(struct rtw_dev *rtwdev, struct rtw_pci *rtwpci,
>
> next_rp:
> /* new skb delivered to mac80211, re-enable original skb DMA */
> - rtw_pci_reset_rx_desc(rtwdev, skb, ring, cur_rp, buf_desc_sz);
> + rtw_pci_sync_rx_desc_device(rtwdev, dma, ring, cur_rp,
> + buf_desc_sz);
>
> /* host read next element in ring */
> if (++cur_rp >= ring->r.len)
> --
> 2.22.0
>
^ permalink raw reply
* Re: [PATCH v4 1/2] rtw88: pci: Rearrange the memory usage for skb in RX ISR
From: Jian-Hong Pan @ 2019-07-11 5:28 UTC (permalink / raw)
To: Yan-Hsuan Chuang, Kalle Valo, David S . Miller, Larry Finger,
David Laight, Christoph Hellwig
Cc: linux-wireless, Linux Netdev List, Linux Kernel,
Linux Upstreaming Team, Daniel Drake, stable
In-Reply-To: <20190711052427.5582-1-jian-hong@endlessm.com>
Jian-Hong Pan <jian-hong@endlessm.com> 於 2019年7月11日 週四 下午1:25寫道：
>
> Testing with RTL8822BE hardware, when available memory is low, we
> frequently see a kernel panic and system freeze.
>
> First, rtw_pci_rx_isr encounters a memory allocation failure (trimmed):
>
> rx routine starvation
> WARNING: CPU: 7 PID: 9871 at drivers/net/wireless/realtek/rtw88/pci.c:822 rtw_pci_rx_isr.constprop.25+0x35a/0x370 [rtwpci]
> [ 2356.580313] RIP: 0010:rtw_pci_rx_isr.constprop.25+0x35a/0x370 [rtwpci]
>
> Then we see a variety of different error conditions and kernel panics,
> such as this one (trimmed):
>
> rtw_pci 0000:02:00.0: pci bus timeout, check dma status
> skbuff: skb_over_panic: text:00000000091b6e66 len:415 put:415 head:00000000d2880c6f data:000000007a02b1ea tail:0x1df end:0xc0 dev:<NULL>
> ------------[ cut here ]------------
> kernel BUG at net/core/skbuff.c:105!
> invalid opcode: 0000 [#1] SMP NOPTI
> RIP: 0010:skb_panic+0x43/0x45
>
> When skb allocation fails and the "rx routine starvation" is hit, the
> function returns immediately without updating the RX ring. At this
> point, the RX ring may continue referencing an old skb which was already
> handed off to ieee80211_rx_irqsafe(). When it comes to be used again,
> bad things happen.
>
> This patch allocates a new, data-sized skb first in RX ISR. After
> copying the data in, we pass it to the upper layers. However, if skb
> allocation fails, we effectively drop the frame. In both cases, the
> original, full size ring skb is reused.
>
> In addition, to fixing the kernel crash, the RX routine should now
> generally behave better under low memory conditions.
>
> Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=204053
> Signed-off-by: Jian-Hong Pan <jian-hong@endlessm.com>
> Cc: <stable@vger.kernel.org>
> ---
Sorry, I forget to place the version difference here.
v2:
- Allocate new data-sized skb and put data into it, then pass it to
mac80211. Reuse the original skb in RX ring by DMA sync.
- Modify the commit message.
- Introduce following [PATCH v3 2/2] rtw88: pci: Use DMA sync instead
of remapping in RX ISR.
v3:
- Same as v2.
v4:
- Fix comment: allocate a new skb for this frame, discard the frame
if none available
> drivers/net/wireless/realtek/rtw88/pci.c | 49 +++++++++++-------------
> 1 file changed, 22 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/net/wireless/realtek/rtw88/pci.c b/drivers/net/wireless/realtek/rtw88/pci.c
> index cfe05ba7280d..c415f5e94fed 100644
> --- a/drivers/net/wireless/realtek/rtw88/pci.c
> +++ b/drivers/net/wireless/realtek/rtw88/pci.c
> @@ -763,6 +763,7 @@ static void rtw_pci_rx_isr(struct rtw_dev *rtwdev, struct rtw_pci *rtwpci,
> u32 pkt_offset;
> u32 pkt_desc_sz = chip->rx_pkt_desc_sz;
> u32 buf_desc_sz = chip->rx_buf_desc_sz;
> + u32 new_len;
> u8 *rx_desc;
> dma_addr_t dma;
>
> @@ -790,40 +791,34 @@ static void rtw_pci_rx_isr(struct rtw_dev *rtwdev, struct rtw_pci *rtwpci,
> pkt_offset = pkt_desc_sz + pkt_stat.drv_info_sz +
> pkt_stat.shift;
>
> - if (pkt_stat.is_c2h) {
> - /* keep rx_desc, halmac needs it */
> - skb_put(skb, pkt_stat.pkt_len + pkt_offset);
> + /* allocate a new skb for this frame,
> + * discard the frame if none available
> + */
> + new_len = pkt_stat.pkt_len + pkt_offset;
> + new = dev_alloc_skb(new_len);
> + if (WARN_ONCE(!new, "rx routine starvation\n"))
> + goto next_rp;
> +
> + /* put the DMA data including rx_desc from phy to new skb */
> + skb_put_data(new, skb->data, new_len);
>
> - /* pass offset for further operation */
> - *((u32 *)skb->cb) = pkt_offset;
> - skb_queue_tail(&rtwdev->c2h_queue, skb);
> + if (pkt_stat.is_c2h) {
> + /* pass rx_desc & offset for further operation */
> + *((u32 *)new->cb) = pkt_offset;
> + skb_queue_tail(&rtwdev->c2h_queue, new);
> ieee80211_queue_work(rtwdev->hw, &rtwdev->c2h_work);
> } else {
> - /* remove rx_desc, maybe use skb_pull? */
> - skb_put(skb, pkt_stat.pkt_len);
> - skb_reserve(skb, pkt_offset);
> -
> - /* alloc a smaller skb to mac80211 */
> - new = dev_alloc_skb(pkt_stat.pkt_len);
> - if (!new) {
> - new = skb;
> - } else {
> - skb_put_data(new, skb->data, skb->len);
> - dev_kfree_skb_any(skb);
> - }
> - /* TODO: merge into rx.c */
> - rtw_rx_stats(rtwdev, pkt_stat.vif, skb);
> + /* remove rx_desc */
> + skb_pull(new, pkt_offset);
> +
> + rtw_rx_stats(rtwdev, pkt_stat.vif, new);
> memcpy(new->cb, &rx_status, sizeof(rx_status));
> ieee80211_rx_irqsafe(rtwdev->hw, new);
> }
>
> - /* skb delivered to mac80211, alloc a new one in rx ring */
> - new = dev_alloc_skb(RTK_PCI_RX_BUF_SIZE);
> - if (WARN(!new, "rx routine starvation\n"))
> - return;
> -
> - ring->buf[cur_rp] = new;
> - rtw_pci_reset_rx_desc(rtwdev, new, ring, cur_rp, buf_desc_sz);
> +next_rp:
> + /* new skb delivered to mac80211, re-enable original skb DMA */
> + rtw_pci_reset_rx_desc(rtwdev, skb, ring, cur_rp, buf_desc_sz);
>
> /* host read next element in ring */
> if (++cur_rp >= ring->r.len)
> --
> 2.22.0
>
^ permalink raw reply
* [PATCH v4 2/2] rtw88: pci: Use DMA sync instead of remapping in RX ISR
From: Jian-Hong Pan @ 2019-07-11 5:24 UTC (permalink / raw)
To: Yan-Hsuan Chuang, Kalle Valo, David S . Miller, Larry Finger,
David Laight, Christoph Hellwig
Cc: linux-wireless, netdev, linux-kernel, linux, Daniel Drake,
Jian-Hong Pan, stable
In-Reply-To: <20190711052427.5582-1-jian-hong@endlessm.com>
Since each skb in RX ring is reused instead of new allocation, we can
treat the DMA in a more efficient way by DMA synchronization.
Signed-off-by: Jian-Hong Pan <jian-hong@endlessm.com>
Cc: <stable@vger.kernel.org>
---
drivers/net/wireless/realtek/rtw88/pci.c | 24 +++++++++++++++++++++---
1 file changed, 21 insertions(+), 3 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtw88/pci.c b/drivers/net/wireless/realtek/rtw88/pci.c
index c415f5e94fed..68fae52151dd 100644
--- a/drivers/net/wireless/realtek/rtw88/pci.c
+++ b/drivers/net/wireless/realtek/rtw88/pci.c
@@ -206,6 +206,23 @@ static int rtw_pci_reset_rx_desc(struct rtw_dev *rtwdev, struct sk_buff *skb,
return 0;
}
+static void rtw_pci_sync_rx_desc_device(struct rtw_dev *rtwdev, dma_addr_t dma,
+ struct rtw_pci_rx_ring *rx_ring,
+ u32 idx, u32 desc_sz)
+{
+ struct device *dev = rtwdev->dev;
+ struct rtw_pci_rx_buffer_desc *buf_desc;
+ int buf_sz = RTK_PCI_RX_BUF_SIZE;
+
+ dma_sync_single_for_device(dev, dma, buf_sz, DMA_FROM_DEVICE);
+
+ buf_desc = (struct rtw_pci_rx_buffer_desc *)(rx_ring->r.head +
+ idx * desc_sz);
+ memset(buf_desc, 0, sizeof(*buf_desc));
+ buf_desc->buf_size = cpu_to_le16(RTK_PCI_RX_BUF_SIZE);
+ buf_desc->dma = cpu_to_le32(dma);
+}
+
static int rtw_pci_init_rx_ring(struct rtw_dev *rtwdev,
struct rtw_pci_rx_ring *rx_ring,
u8 desc_size, u32 len)
@@ -782,8 +799,8 @@ static void rtw_pci_rx_isr(struct rtw_dev *rtwdev, struct rtw_pci *rtwpci,
rtw_pci_dma_check(rtwdev, ring, cur_rp);
skb = ring->buf[cur_rp];
dma = *((dma_addr_t *)skb->cb);
- pci_unmap_single(rtwpci->pdev, dma, RTK_PCI_RX_BUF_SIZE,
- PCI_DMA_FROMDEVICE);
+ dma_sync_single_for_cpu(rtwdev->dev, dma, RTK_PCI_RX_BUF_SIZE,
+ DMA_FROM_DEVICE);
rx_desc = skb->data;
chip->ops->query_rx_desc(rtwdev, rx_desc, &pkt_stat, &rx_status);
@@ -818,7 +835,8 @@ static void rtw_pci_rx_isr(struct rtw_dev *rtwdev, struct rtw_pci *rtwpci,
next_rp:
/* new skb delivered to mac80211, re-enable original skb DMA */
- rtw_pci_reset_rx_desc(rtwdev, skb, ring, cur_rp, buf_desc_sz);
+ rtw_pci_sync_rx_desc_device(rtwdev, dma, ring, cur_rp,
+ buf_desc_sz);
/* host read next element in ring */
if (++cur_rp >= ring->r.len)
--
2.22.0
^ permalink raw reply related
* [PATCH v4 1/2] rtw88: pci: Rearrange the memory usage for skb in RX ISR
From: Jian-Hong Pan @ 2019-07-11 5:24 UTC (permalink / raw)
To: Yan-Hsuan Chuang, Kalle Valo, David S . Miller, Larry Finger,
David Laight, Christoph Hellwig
Cc: linux-wireless, netdev, linux-kernel, linux, Daniel Drake,
Jian-Hong Pan, stable
In-Reply-To: <CAPpJ_edDcaBq+0DocPmS-yYM10B4MkWvBn=f6wwbYdqzSGmp_g@mail.gmail.com>
Testing with RTL8822BE hardware, when available memory is low, we
frequently see a kernel panic and system freeze.
First, rtw_pci_rx_isr encounters a memory allocation failure (trimmed):
rx routine starvation
WARNING: CPU: 7 PID: 9871 at drivers/net/wireless/realtek/rtw88/pci.c:822 rtw_pci_rx_isr.constprop.25+0x35a/0x370 [rtwpci]
[ 2356.580313] RIP: 0010:rtw_pci_rx_isr.constprop.25+0x35a/0x370 [rtwpci]
Then we see a variety of different error conditions and kernel panics,
such as this one (trimmed):
rtw_pci 0000:02:00.0: pci bus timeout, check dma status
skbuff: skb_over_panic: text:00000000091b6e66 len:415 put:415 head:00000000d2880c6f data:000000007a02b1ea tail:0x1df end:0xc0 dev:<NULL>
------------[ cut here ]------------
kernel BUG at net/core/skbuff.c:105!
invalid opcode: 0000 [#1] SMP NOPTI
RIP: 0010:skb_panic+0x43/0x45
When skb allocation fails and the "rx routine starvation" is hit, the
function returns immediately without updating the RX ring. At this
point, the RX ring may continue referencing an old skb which was already
handed off to ieee80211_rx_irqsafe(). When it comes to be used again,
bad things happen.
This patch allocates a new, data-sized skb first in RX ISR. After
copying the data in, we pass it to the upper layers. However, if skb
allocation fails, we effectively drop the frame. In both cases, the
original, full size ring skb is reused.
In addition, to fixing the kernel crash, the RX routine should now
generally behave better under low memory conditions.
Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=204053
Signed-off-by: Jian-Hong Pan <jian-hong@endlessm.com>
Cc: <stable@vger.kernel.org>
---
drivers/net/wireless/realtek/rtw88/pci.c | 49 +++++++++++-------------
1 file changed, 22 insertions(+), 27 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtw88/pci.c b/drivers/net/wireless/realtek/rtw88/pci.c
index cfe05ba7280d..c415f5e94fed 100644
--- a/drivers/net/wireless/realtek/rtw88/pci.c
+++ b/drivers/net/wireless/realtek/rtw88/pci.c
@@ -763,6 +763,7 @@ static void rtw_pci_rx_isr(struct rtw_dev *rtwdev, struct rtw_pci *rtwpci,
u32 pkt_offset;
u32 pkt_desc_sz = chip->rx_pkt_desc_sz;
u32 buf_desc_sz = chip->rx_buf_desc_sz;
+ u32 new_len;
u8 *rx_desc;
dma_addr_t dma;
@@ -790,40 +791,34 @@ static void rtw_pci_rx_isr(struct rtw_dev *rtwdev, struct rtw_pci *rtwpci,
pkt_offset = pkt_desc_sz + pkt_stat.drv_info_sz +
pkt_stat.shift;
- if (pkt_stat.is_c2h) {
- /* keep rx_desc, halmac needs it */
- skb_put(skb, pkt_stat.pkt_len + pkt_offset);
+ /* allocate a new skb for this frame,
+ * discard the frame if none available
+ */
+ new_len = pkt_stat.pkt_len + pkt_offset;
+ new = dev_alloc_skb(new_len);
+ if (WARN_ONCE(!new, "rx routine starvation\n"))
+ goto next_rp;
+
+ /* put the DMA data including rx_desc from phy to new skb */
+ skb_put_data(new, skb->data, new_len);
- /* pass offset for further operation */
- *((u32 *)skb->cb) = pkt_offset;
- skb_queue_tail(&rtwdev->c2h_queue, skb);
+ if (pkt_stat.is_c2h) {
+ /* pass rx_desc & offset for further operation */
+ *((u32 *)new->cb) = pkt_offset;
+ skb_queue_tail(&rtwdev->c2h_queue, new);
ieee80211_queue_work(rtwdev->hw, &rtwdev->c2h_work);
} else {
- /* remove rx_desc, maybe use skb_pull? */
- skb_put(skb, pkt_stat.pkt_len);
- skb_reserve(skb, pkt_offset);
-
- /* alloc a smaller skb to mac80211 */
- new = dev_alloc_skb(pkt_stat.pkt_len);
- if (!new) {
- new = skb;
- } else {
- skb_put_data(new, skb->data, skb->len);
- dev_kfree_skb_any(skb);
- }
- /* TODO: merge into rx.c */
- rtw_rx_stats(rtwdev, pkt_stat.vif, skb);
+ /* remove rx_desc */
+ skb_pull(new, pkt_offset);
+
+ rtw_rx_stats(rtwdev, pkt_stat.vif, new);
memcpy(new->cb, &rx_status, sizeof(rx_status));
ieee80211_rx_irqsafe(rtwdev->hw, new);
}
- /* skb delivered to mac80211, alloc a new one in rx ring */
- new = dev_alloc_skb(RTK_PCI_RX_BUF_SIZE);
- if (WARN(!new, "rx routine starvation\n"))
- return;
-
- ring->buf[cur_rp] = new;
- rtw_pci_reset_rx_desc(rtwdev, new, ring, cur_rp, buf_desc_sz);
+next_rp:
+ /* new skb delivered to mac80211, re-enable original skb DMA */
+ rtw_pci_reset_rx_desc(rtwdev, skb, ring, cur_rp, buf_desc_sz);
/* host read next element in ring */
if (++cur_rp >= ring->r.len)
--
2.22.0
^ permalink raw reply related
* Re: [PATCH v3 1/2] rtw88: pci: Rearrange the memory usage for skb in RX ISR
From: Jian-Hong Pan @ 2019-07-11 3:50 UTC (permalink / raw)
To: David Laight
Cc: Yan-Hsuan Chuang, Kalle Valo, David S . Miller, Larry Finger,
Christoph Hellwig, linux-wireless@vger.kernel.org,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
linux@endlessm.com, Daniel Drake, stable@vger.kernel.org
In-Reply-To: <81a2b91c4b084617bab8656fca932f6d@AcuMS.aculab.com>
David Laight <David.Laight@aculab.com> 於 2019年7月10日 週三 下午4:57寫道：
>
> From: Jian-Hong Pan
> > Sent: 10 July 2019 09:38
> >
> > Testing with RTL8822BE hardware, when available memory is low, we
> > frequently see a kernel panic and system freeze.
> >
> > First, rtw_pci_rx_isr encounters a memory allocation failure (trimmed):
> >
> > rx routine starvation
> > WARNING: CPU: 7 PID: 9871 at drivers/net/wireless/realtek/rtw88/pci.c:822
> > rtw_pci_rx_isr.constprop.25+0x35a/0x370 [rtwpci]
> > [ 2356.580313] RIP: 0010:rtw_pci_rx_isr.constprop.25+0x35a/0x370 [rtwpci]
> >
> > Then we see a variety of different error conditions and kernel panics,
> > such as this one (trimmed):
> >
> > rtw_pci 0000:02:00.0: pci bus timeout, check dma status
> > skbuff: skb_over_panic: text:00000000091b6e66 len:415 put:415 head:00000000d2880c6f
> > data:000000007a02b1ea tail:0x1df end:0xc0 dev:<NULL>
> > ------------[ cut here ]------------
> > kernel BUG at net/core/skbuff.c:105!
> > invalid opcode: 0000 [#1] SMP NOPTI
> > RIP: 0010:skb_panic+0x43/0x45
> >
> > When skb allocation fails and the "rx routine starvation" is hit, the
> > function returns immediately without updating the RX ring. At this
> > point, the RX ring may continue referencing an old skb which was already
> > handed off to ieee80211_rx_irqsafe(). When it comes to be used again,
> > bad things happen.
> >
> > This patch allocates a new, data-sized skb first in RX ISR. After
> > copying the data in, we pass it to the upper layers. However, if skb
> > allocation fails, we effectively drop the frame. In both cases, the
> > original, full size ring skb is reused.
> >
> > In addition, by fixing the kernel crash, the RX routine should now
> > generally behave better under low memory conditions.
>
> A couple of minor nits (see below).
> You may want to do a followup patch that changes the rx buffers
> (used by the hardware) to by just memory buffers.
> Nothing (probably) relies on them being skb with all the accociated
> baggage.
It is a good idea for later commit.
> David
>
> >
> > Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=204053
> > Signed-off-by: Jian-Hong Pan <jian-hong@endlessm.com>
> > Cc: <stable@vger.kernel.org>
> > ---
> > v2:
> > - Allocate new data-sized skb and put data into it, then pass it to
> > mac80211. Reuse the original skb in RX ring by DMA sync.
> > - Modify the commit message.
> > - Introduce following [PATCH v3 2/2] rtw88: pci: Use DMA sync instead
> > of remapping in RX ISR.
> >
> > v3:
> > - Same as v2.
> >
> > drivers/net/wireless/realtek/rtw88/pci.c | 49 +++++++++++-------------
> > 1 file changed, 22 insertions(+), 27 deletions(-)
> >
> > diff --git a/drivers/net/wireless/realtek/rtw88/pci.c b/drivers/net/wireless/realtek/rtw88/pci.c
> > index cfe05ba7280d..e9fe3ad896c8 100644
> > --- a/drivers/net/wireless/realtek/rtw88/pci.c
> > +++ b/drivers/net/wireless/realtek/rtw88/pci.c
> > @@ -763,6 +763,7 @@ static void rtw_pci_rx_isr(struct rtw_dev *rtwdev, struct rtw_pci *rtwpci,
> > u32 pkt_offset;
> > u32 pkt_desc_sz = chip->rx_pkt_desc_sz;
> > u32 buf_desc_sz = chip->rx_buf_desc_sz;
> > + u32 new_len;
> > u8 *rx_desc;
> > dma_addr_t dma;
> >
> > @@ -790,40 +791,34 @@ static void rtw_pci_rx_isr(struct rtw_dev *rtwdev, struct rtw_pci *rtwpci,
> > pkt_offset = pkt_desc_sz + pkt_stat.drv_info_sz +
> > pkt_stat.shift;
> >
> > - if (pkt_stat.is_c2h) {
> > - /* keep rx_desc, halmac needs it */
> > - skb_put(skb, pkt_stat.pkt_len + pkt_offset);
> > + /* discard current skb if the new skb cannot be allocated as a
> > + * new one in rx ring later
> > + */
>
> That comment isn't quite right.
> maybe: "Allocate a new skb for this frame, discard if none available"
Thanks! I will tweak it.
> > + new_len = pkt_stat.pkt_len + pkt_offset;
> > + new = dev_alloc_skb(new_len);
> > + if (WARN_ONCE(!new, "rx routine starvation\n"))
>
> I think you should count these??
Larry has a different idea here. [1]
I agree with Larry that just need to know not enough memory here.
[1] https://lkml.org/lkml/2019/7/8/1049
Jian-Hong Pan
> > + goto next_rp;
> > +
> > + /* put the DMA data including rx_desc from phy to new skb */
> > + skb_put_data(new, skb->data, new_len);
> >
> > - /* pass offset for further operation */
> > - *((u32 *)skb->cb) = pkt_offset;
> > - skb_queue_tail(&rtwdev->c2h_queue, skb);
> > + if (pkt_stat.is_c2h) {
> > + /* pass rx_desc & offset for further operation */
> > + *((u32 *)new->cb) = pkt_offset;
> > + skb_queue_tail(&rtwdev->c2h_queue, new);
> > ieee80211_queue_work(rtwdev->hw, &rtwdev->c2h_work);
> > } else {
> > - /* remove rx_desc, maybe use skb_pull? */
> > - skb_put(skb, pkt_stat.pkt_len);
> > - skb_reserve(skb, pkt_offset);
> > -
> > - /* alloc a smaller skb to mac80211 */
> > - new = dev_alloc_skb(pkt_stat.pkt_len);
> > - if (!new) {
> > - new = skb;
> > - } else {
> > - skb_put_data(new, skb->data, skb->len);
> > - dev_kfree_skb_any(skb);
> > - }
> > - /* TODO: merge into rx.c */
> > - rtw_rx_stats(rtwdev, pkt_stat.vif, skb);
> > + /* remove rx_desc */
> > + skb_pull(new, pkt_offset);
> > +
> > + rtw_rx_stats(rtwdev, pkt_stat.vif, new);
> > memcpy(new->cb, &rx_status, sizeof(rx_status));
> > ieee80211_rx_irqsafe(rtwdev->hw, new);
> > }
> >
> > - /* skb delivered to mac80211, alloc a new one in rx ring */
> > - new = dev_alloc_skb(RTK_PCI_RX_BUF_SIZE);
> > - if (WARN(!new, "rx routine starvation\n"))
> > - return;
> > -
> > - ring->buf[cur_rp] = new;
> > - rtw_pci_reset_rx_desc(rtwdev, new, ring, cur_rp, buf_desc_sz);
> > +next_rp:
> > + /* new skb delivered to mac80211, re-enable original skb DMA */
> > + rtw_pci_reset_rx_desc(rtwdev, skb, ring, cur_rp, buf_desc_sz);
> >
> > /* host read next element in ring */
> > if (++cur_rp >= ring->r.len)
> > --
> > 2.22.0
>
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> Registration No: 1397386 (Wales)
>
^ permalink raw reply
* Investment Transaction.......
From: Phil Moore @ 2019-07-08 23:53 UTC (permalink / raw)
To: linux-wireless
Greetings,
I have a business transaction i would like to discuss with you. contact me on skylinefin010@gmail.com for more details.
Yours Truly,
Phil Moore
^ permalink raw reply
* Re: [PATCH AUTOSEL 4.19 14/60] mwifiex: Abort at too short BSS descriptor element
From: Brian Norris @ 2019-07-10 21:12 UTC (permalink / raw)
To: Sasha Levin
Cc: Linux Kernel, stable, Takashi Iwai, Kalle Valo, linux-wireless,
<netdev@vger.kernel.org>
In-Reply-To: <20190710145112.GX10104@sasha-vm>
On Wed, Jul 10, 2019 at 7:51 AM Sasha Levin <sashal@kernel.org> wrote:
> I see that 63d7ef36103d didn't make it into 5.2, so I'll just drop this
> for now.
Yeah, I think it's stuck at net/master. Presumably it'll get into
5.3-rc somewhere.
Brian
^ permalink raw reply
* [RFC PATCH v3 1/2] cfg80211: refactor cfg80211_bss_update
From: Sergey Matyukevich @ 2019-07-10 17:37 UTC (permalink / raw)
To: linux-wireless@vger.kernel.org
Cc: Johannes Berg, Igor Mitsyanko, Mikhail Karpenko,
Sergey Matyukevich
In-Reply-To: <20190710173651.15770-1-sergey.matyukevich.os@quantenna.com>
This patch implements minor refactoring for cfg80211_bss_update function.
Code path for updating known BSS is extracted into dedicated
cfg80211_update_known_bss function.
Signed-off-by: Sergey Matyukevich <sergey.matyukevich.os@quantenna.com>
---
net/wireless/scan.c | 171 +++++++++++++++++++++++++++-------------------------
1 file changed, 89 insertions(+), 82 deletions(-)
diff --git a/net/wireless/scan.c b/net/wireless/scan.c
index d66e6d4b7555..9f21162f05e9 100644
--- a/net/wireless/scan.c
+++ b/net/wireless/scan.c
@@ -1091,6 +1091,93 @@ struct cfg80211_non_tx_bss {
u8 bssid_index;
};
+static bool
+cfg80211_update_known_bss(struct cfg80211_registered_device *rdev,
+ struct cfg80211_internal_bss *known,
+ struct cfg80211_internal_bss *new,
+ bool signal_valid)
+{
+ lockdep_assert_held(&rdev->bss_lock);
+
+ /* Update IEs */
+ if (rcu_access_pointer(new->pub.proberesp_ies)) {
+ const struct cfg80211_bss_ies *old;
+
+ old = rcu_access_pointer(known->pub.proberesp_ies);
+
+ rcu_assign_pointer(known->pub.proberesp_ies,
+ new->pub.proberesp_ies);
+ /* Override possible earlier Beacon frame IEs */
+ rcu_assign_pointer(known->pub.ies,
+ new->pub.proberesp_ies);
+ if (old)
+ kfree_rcu((struct cfg80211_bss_ies *)old, rcu_head);
+ } else if (rcu_access_pointer(new->pub.beacon_ies)) {
+ const struct cfg80211_bss_ies *old;
+ struct cfg80211_internal_bss *bss;
+
+ if (known->pub.hidden_beacon_bss &&
+ !list_empty(&known->hidden_list)) {
+ const struct cfg80211_bss_ies *f;
+
+ /* The known BSS struct is one of the probe
+ * response members of a group, but we're
+ * receiving a beacon (beacon_ies in the new
+ * bss is used). This can only mean that the
+ * AP changed its beacon from not having an
+ * SSID to showing it, which is confusing so
+ * drop this information.
+ */
+
+ f = rcu_access_pointer(new->pub.beacon_ies);
+ kfree_rcu((struct cfg80211_bss_ies *)f, rcu_head);
+ return false;
+ }
+
+ old = rcu_access_pointer(known->pub.beacon_ies);
+
+ rcu_assign_pointer(known->pub.beacon_ies, new->pub.beacon_ies);
+
+ /* Override IEs if they were from a beacon before */
+ if (old == rcu_access_pointer(known->pub.ies))
+ rcu_assign_pointer(known->pub.ies, new->pub.beacon_ies);
+
+ /* Assign beacon IEs to all sub entries */
+ list_for_each_entry(bss, &known->hidden_list, hidden_list) {
+ const struct cfg80211_bss_ies *ies;
+
+ ies = rcu_access_pointer(bss->pub.beacon_ies);
+ WARN_ON(ies != old);
+
+ rcu_assign_pointer(bss->pub.beacon_ies,
+ new->pub.beacon_ies);
+ }
+
+ if (old)
+ kfree_rcu((struct cfg80211_bss_ies *)old, rcu_head);
+ }
+
+ known->pub.beacon_interval = new->pub.beacon_interval;
+
+ /* don't update the signal if beacon was heard on
+ * adjacent channel.
+ */
+ if (signal_valid)
+ known->pub.signal = new->pub.signal;
+ known->pub.capability = new->pub.capability;
+ known->ts = new->ts;
+ known->ts_boottime = new->ts_boottime;
+ known->parent_tsf = new->parent_tsf;
+ known->pub.chains = new->pub.chains;
+ memcpy(known->pub.chain_signal, new->pub.chain_signal,
+ IEEE80211_MAX_CHAINS);
+ ether_addr_copy(known->parent_bssid, new->parent_bssid);
+ known->pub.max_bssid_indicator = new->pub.max_bssid_indicator;
+ known->pub.bssid_index = new->pub.bssid_index;
+
+ return true;
+}
+
/* Returned bss is reference counted and must be cleaned up appropriately. */
struct cfg80211_internal_bss *
cfg80211_bss_update(struct cfg80211_registered_device *rdev,
@@ -1114,88 +1201,8 @@ cfg80211_bss_update(struct cfg80211_registered_device *rdev,
found = rb_find_bss(rdev, tmp, BSS_CMP_REGULAR);
if (found) {
- /* Update IEs */
- if (rcu_access_pointer(tmp->pub.proberesp_ies)) {
- const struct cfg80211_bss_ies *old;
-
- old = rcu_access_pointer(found->pub.proberesp_ies);
-
- rcu_assign_pointer(found->pub.proberesp_ies,
- tmp->pub.proberesp_ies);
- /* Override possible earlier Beacon frame IEs */
- rcu_assign_pointer(found->pub.ies,
- tmp->pub.proberesp_ies);
- if (old)
- kfree_rcu((struct cfg80211_bss_ies *)old,
- rcu_head);
- } else if (rcu_access_pointer(tmp->pub.beacon_ies)) {
- const struct cfg80211_bss_ies *old;
- struct cfg80211_internal_bss *bss;
-
- if (found->pub.hidden_beacon_bss &&
- !list_empty(&found->hidden_list)) {
- const struct cfg80211_bss_ies *f;
-
- /*
- * The found BSS struct is one of the probe
- * response members of a group, but we're
- * receiving a beacon (beacon_ies in the tmp
- * bss is used). This can only mean that the
- * AP changed its beacon from not having an
- * SSID to showing it, which is confusing so
- * drop this information.
- */
-
- f = rcu_access_pointer(tmp->pub.beacon_ies);
- kfree_rcu((struct cfg80211_bss_ies *)f,
- rcu_head);
- goto drop;
- }
-
- old = rcu_access_pointer(found->pub.beacon_ies);
-
- rcu_assign_pointer(found->pub.beacon_ies,
- tmp->pub.beacon_ies);
-
- /* Override IEs if they were from a beacon before */
- if (old == rcu_access_pointer(found->pub.ies))
- rcu_assign_pointer(found->pub.ies,
- tmp->pub.beacon_ies);
-
- /* Assign beacon IEs to all sub entries */
- list_for_each_entry(bss, &found->hidden_list,
- hidden_list) {
- const struct cfg80211_bss_ies *ies;
-
- ies = rcu_access_pointer(bss->pub.beacon_ies);
- WARN_ON(ies != old);
-
- rcu_assign_pointer(bss->pub.beacon_ies,
- tmp->pub.beacon_ies);
- }
-
- if (old)
- kfree_rcu((struct cfg80211_bss_ies *)old,
- rcu_head);
- }
-
- found->pub.beacon_interval = tmp->pub.beacon_interval;
- /*
- * don't update the signal if beacon was heard on
- * adjacent channel.
- */
- if (signal_valid)
- found->pub.signal = tmp->pub.signal;
- found->pub.capability = tmp->pub.capability;
- found->ts = tmp->ts;
- found->ts_boottime = tmp->ts_boottime;
- found->parent_tsf = tmp->parent_tsf;
- found->pub.chains = tmp->pub.chains;
- memcpy(found->pub.chain_signal, tmp->pub.chain_signal,
- IEEE80211_MAX_CHAINS);
- ether_addr_copy(found->parent_bssid, tmp->parent_bssid);
- found->pub.max_bssid_indicator = tmp->pub.max_bssid_indicator;
- found->pub.bssid_index = tmp->pub.bssid_index;
+ if (!cfg80211_update_known_bss(rdev, found, tmp, signal_valid))
+ goto drop;
} else {
struct cfg80211_internal_bss *new;
struct cfg80211_internal_bss *hidden;
--
2.11.0
^ permalink raw reply related
* [RFC PATCH v3 2/2] cfg80211: fix duplicated scan entries after channel switch
From: Sergey Matyukevich @ 2019-07-10 17:37 UTC (permalink / raw)
To: linux-wireless@vger.kernel.org
Cc: Johannes Berg, Igor Mitsyanko, Mikhail Karpenko,
Sergey Matyukevich
In-Reply-To: <20190710173651.15770-1-sergey.matyukevich.os@quantenna.com>
When associated BSS completes channel switch procedure, its channel record
needs to be updated. The existing mac80211 solution was extended to
cfg80211 in commit 5dc8cdce1d72 ("mac80211/cfg80211: update bss
channel on channel switch")
However that solution still appears to be incomplete as it may lead
to duplicated scan entries for associated BSS after channel switch.
The root cause of the problem is as follows. Each BSS entry is
included into the following data structures:
- bss list rdev->bss_list
- bss search tree rdev->bss_tree
Updating BSS channel record without rebuilding bss_tree may break
tree search since cmp_bss considers all of the following: channel,
bssid, ssid. When BSS channel is updated, but its location in bss_tree
is not updated, then subsequent search operations may fail to locate
this BSS since they will be traversing bss_tree in wrong direction.
As a result, for scan performed after associated BSS channel switch,
cfg80211_bss_update may add the second entry for the same BSS to both
bss_list and bss_tree, rather then update the existing one.
To summarize, if BSS channel needs to be updated, then bss_tree should
be rebuilt in order to put updated BSS entry into a proper location.
This commit suggests the following straightforward solution:
- if new entry has been already created for BSS after channel switch,
then use its IEs to update known BSS entry and then remove new
entry completely
- use rb_erase/rb_insert_bss reinstall updated BSS in bss_tree
- update channel and location in rb-tree for non-transmitting bss entries
Signed-off-by: Sergey Matyukevich <sergey.matyukevich.os@quantenna.com>
---
Cover email is not attached to patchwork, so duplicate it here as well...
Suggested approach to handle non-transmitting BSS entries is simplified in the
following sense. If new entries have been already created after channel switch,
only transmitting bss will be updated using IEs of new entry for the same
transmitting bss. Non-transmitting bss entries will be updated as soon as
new mgmt frames are received. Updating non-transmitting bss entries seems
too expensive: nested nontrans_list traversing is needed since we can not
rely on the same order of old and new non-transmitting entries.
Basic use-case tested using both iwlwifi and qtnfmac. However multi-BSSID
support has not yet been tested.
v1 -> v2
- use IEs of new BSS entry to update known BSS entry
for this purpose extract BSS update code from cfg80211_bss_update
into a separate function cfg80211_update_known_bss
v2 -> v3
- minor cleanup according to review comments
- split cfg80211_update_known_bss function into a separate patch
- update channel and location in rb-tree for nontransmit bss entries
Regards,
Sergey
---
net/wireless/core.h | 2 ++
net/wireless/nl80211.c | 2 +-
net/wireless/scan.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 82 insertions(+), 1 deletion(-)
diff --git a/net/wireless/core.h b/net/wireless/core.h
index ee8388fe4a92..77556c58d9ac 100644
--- a/net/wireless/core.h
+++ b/net/wireless/core.h
@@ -306,6 +306,8 @@ void ieee80211_set_bitrate_flags(struct wiphy *wiphy);
void cfg80211_bss_expire(struct cfg80211_registered_device *rdev);
void cfg80211_bss_age(struct cfg80211_registered_device *rdev,
unsigned long age_secs);
+void cfg80211_update_assoc_bss_entry(struct wireless_dev *wdev,
+ struct ieee80211_channel *channel);
/* IBSS */
int __cfg80211_join_ibss(struct cfg80211_registered_device *rdev,
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index fc83dd179c1a..6ebb427883d0 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -16092,7 +16092,7 @@ void cfg80211_ch_switch_notify(struct net_device *dev,
if (wdev->iftype == NL80211_IFTYPE_STATION &&
!WARN_ON(!wdev->current_bss))
- wdev->current_bss->pub.channel = chandef->chan;
+ cfg80211_update_assoc_bss_entry(wdev, chandef->chan);
nl80211_ch_switch_notify(rdev, dev, chandef, GFP_KERNEL,
NL80211_CMD_CH_SWITCH_NOTIFY, 0);
diff --git a/net/wireless/scan.c b/net/wireless/scan.c
index 9f21162f05e9..30932b955ebc 100644
--- a/net/wireless/scan.c
+++ b/net/wireless/scan.c
@@ -2002,6 +2002,85 @@ void cfg80211_bss_iter(struct wiphy *wiphy,
}
EXPORT_SYMBOL(cfg80211_bss_iter);
+void cfg80211_update_assoc_bss_entry(struct wireless_dev *wdev,
+ struct ieee80211_channel *chan)
+{
+ struct wiphy *wiphy = wdev->wiphy;
+ struct cfg80211_registered_device *rdev = wiphy_to_rdev(wiphy);
+ struct cfg80211_internal_bss *cbss = wdev->current_bss;
+ struct cfg80211_internal_bss *new = NULL;
+ struct cfg80211_internal_bss *bss;
+ struct cfg80211_bss *nontrans_bss;
+ struct cfg80211_bss *tmp;
+
+ spin_lock_bh(&rdev->bss_lock);
+
+ if (WARN_ON(cbss->pub.channel == chan))
+ goto done;
+
+ /* use transmitting bss */
+ if (cbss->pub.transmitted_bss)
+ cbss = container_of(cbss->pub.transmitted_bss,
+ struct cfg80211_internal_bss,
+ pub);
+
+ cbss->pub.channel = chan;
+
+ list_for_each_entry(bss, &rdev->bss_list, list) {
+ if (!cfg80211_bss_type_match(bss->pub.capability,
+ bss->pub.channel->band,
+ wdev->conn_bss_type))
+ continue;
+
+ if (bss == cbss)
+ continue;
+
+ if (!cmp_bss(&bss->pub, &cbss->pub, BSS_CMP_REGULAR)) {
+ new = bss;
+ break;
+ }
+ }
+
+ if (new) {
+ /* to save time, update IEs for trasmitted bss only */
+ if (cfg80211_update_known_bss(rdev, cbss, new, false)) {
+ new->pub.proberesp_ies = NULL;
+ new->pub.beacon_ies = NULL;
+ }
+
+ list_for_each_entry_safe(nontrans_bss, tmp,
+ &new->pub.nontrans_list,
+ nontrans_list) {
+ bss = container_of(nontrans_bss,
+ struct cfg80211_internal_bss, pub);
+ if (__cfg80211_unlink_bss(rdev, bss))
+ rdev->bss_generation++;
+ }
+
+ WARN_ON(atomic_read(&new->hold));
+ if (!WARN_ON(!__cfg80211_unlink_bss(rdev, new)))
+ rdev->bss_generation++;
+ }
+
+ rb_erase(&cbss->rbn, &rdev->bss_tree);
+ rb_insert_bss(rdev, cbss);
+ rdev->bss_generation++;
+
+ list_for_each_entry_safe(nontrans_bss, tmp,
+ &cbss->pub.nontrans_list,
+ nontrans_list) {
+ bss = container_of(nontrans_bss,
+ struct cfg80211_internal_bss, pub);
+ bss->pub.channel = chan;
+ rb_erase(&bss->rbn, &rdev->bss_tree);
+ rb_insert_bss(rdev, bss);
+ rdev->bss_generation++;
+ }
+
+done:
+ spin_unlock_bh(&rdev->bss_lock);
+}
+
#ifdef CONFIG_CFG80211_WEXT
static struct cfg80211_registered_device *
cfg80211_get_dev_from_ifindex(struct net *net, int ifindex)
--
2.11.0
^ permalink raw reply related
* [RFC PATCH v3 0/2] cfg80211: fix duplicated scan entries after channel switch
From: Sergey Matyukevich @ 2019-07-10 17:36 UTC (permalink / raw)
To: linux-wireless@vger.kernel.org
Cc: Johannes Berg, Igor Mitsyanko, Mikhail Karpenko,
Sergey Matyukevich
Hi Johannes and all,
This is v3 of RFC patch aimed at fixing duplicated scan entries after channel
switch. The major change is updating non-transmitting bss entries. Since such
a bss cannot change channel without its transmitting bss (and vice versa),
the whole hierarchy of transmitting bss is updated, including channel and
location in rb-tree.
Suggested approach to handle non-transmitting BSS entries is simplified in the
following sense. If new entries have been already created after channel switch,
only transmitting bss will be updated using IEs of new entry for the same
transmitting bss. Non-transmitting bss entries will be updated as soon as
new mgmt frames are received. Updating non-transmitting bss entries seems
too expensive: nested nontrans_list traversing is needed since we can not
rely on the same order of old and new non-transmitting entries.
Basic use-case tested using both iwlwifi and qtnfmac.
However multi-BSSID support has not yet been tested.
Regards,
Sergey
v1 -> v2
- use IEs of new BSS entry to update known BSS entry
for this purpose extract BSS update code from cfg80211_bss_update
into a separate function cfg80211_update_known_bss
v2 -> v3
- minor cleanup according to review comments
- split cfg80211_update_known_bss function into a separate patch
- update channel and location in rb-tree for nontransmit bss entries
Sergey Matyukevich (2):
cfg80211: refactor cfg80211_bss_update
cfg80211: fix duplicated scan entries after channel switch
net/wireless/core.h | 2 +
net/wireless/nl80211.c | 2 +-
net/wireless/scan.c | 250 +++++++++++++++++++++++++++++++++----------------
3 files changed, 171 insertions(+), 83 deletions(-)
--
2.11.0
^ permalink raw reply
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