As 802.11 and Bluetooth devices can interfere with each other
several coexistence techniques exist.
This is particularly important for devices that share antenna
and analog front end between Wifi and Bluetooth.
There are situations when software control over the coexistence
algorithm behavior would be beneficial - ex. giving EAPOL handshake
or DHCP negotiation high priority.
This two patches add possibility to control Wifi-Bluetooth coexistence mode.
First one extends wiphy params with coexistence mode setting.
Second one implements coexistence control for brcmfmac.
[RFC] cfg80211: configuration of Bluetooth coexistence mode
brcmfmac: control BT coexistence parameters
--
1.7.9.5
On 02/27/13 11:27, Dan Williams wrote:
> On Mon, 2013-02-25 at 14:07 +0100, Felix Fietkau wrote:
>> On 2013-02-25 11:25 AM, Johannes Berg wrote:
>>> On Mon, 2013-02-25 at 06:54 +0100, Felix Fietkau wrote:
>>>
>>>> Most devices have some kind of connection manager that has a high-level
>>>> perspective of when it's fully connected (which includes DHCP/bootp).
>>>> Why not just let that connection manager set a sane maximum network
>>>> latency value via pm_qos network_latency and derive btcoex weight
>>>> changing and multi-channel settings from that?
>>>
>>> Frankly, I don't think that's going to work well. We tried using the
>>> pm_qos framework once and nothing ever used it. Android isn't going to
>>> change to it, so we'd be stuck with hacks like setting pm_qos in
>>> wpa_supplicant which is just as awkward.
>> If only the connection manager gets changed to use it, that would
>> already be enough. It doesn't have to be pushed into dhcp clients and
>> other applications.
>>
>>> Also, what you mostly want isn't really so much a weight but rather a
>>> time-based approach to give it high priority until the connection
>>> handshake completes (we already do for auth/assoc/... until authorized)
>>> so I think using the pm_qos framework to give priority wouldn't work
>>> very well since there'd also be no way to tell when it was "done"
>> Just release the latency requirement in the connection manager once the
>> handshake is done. It knows...
>
> We also don't know what IP configuration method will get used; whether
> it will be IPv6 RA, DHCPv4 or DHCPv6, IPv4 autoconf, or static. Only
> the connection manager knows that. Only the connection manager/DHCP
> client know when they expect a lease renew operation to start too.
> wpa_supplicant doesn't know any of these things either since it doesn't
> do anything IP related.
>
> I think the best approach here is to allow the higher layers to hint to
> the driver that some operations that are about to start must be "more
> reliable". That includes EAPOL, DHCP, IP autoconfiguration, etc. Then
> when the higher layers know the operation is finished, they can indicate
> the operations are done and the driver can go do whatever it wants.
Indeed the RFC approach was explicit about the scope of this interface
being BT coex or actually BT coex override. Johannes proposes one
dedicated to DHCP as a similar interface in Android is used for that
right now. Abstracting it to "more reliable" mainly avoids renaming it
when someone comes up with a use-case other than BT coex or DHCP.
> The driver/stack may wish to do any of [set 1Mb rate, block rate
> control, change BT coex, turn on microwave protection, whatever] and
> that's great, the upper layers don't care about what the driver does,
> just that the reliability of the operation is preserved.
It is actually the reliability of the connection, but it may depend on
what you mean by "operation" here. I think from user-space perpective
this API is at most a notification to the driver, which *may* result in
a more reliable protocol exchange in terms of reliability and/or latency.
Regards,
Arend
On 24 February 2013 09:28, Johannes Berg <[email protected]> wrote:
>> In the driver we could inspect each sk_buff and boost priority of any
>> BOOTP packets so that it will end up in the AC_VO fifo and have
>> hardware coexistence handle it further. I consider that more a
>> pragmatic fix, which is not always the worst thing to go for.
>
> Well, I don't really think that's the best idea. Sniffing the protocol
> is clumsy at best.
Sure, but it's the kind of thing that commercial devices do in order
to work around real world issues.
So it'd be nice to have a programmatic way to detect things (eg bootp
packets going out) and signal the driver via some side channel to do
things like btcoex weight changing.
But it'd also be nice to do it based on QoS too.
>> However, I would prefer a solution is which user-space, ie. dhcp
>> client can control the priority and/or BT coexistence.
>
> It's not just BT coex btw, with multi-channel support it might also be
> interesting to not switch channels while DHCP is going on, for example.
Right.
Adrian
Some devices share antenna/analog front end between Wifi
and Bluetooth. The hardware coexistence interface allows
them to do so, but there are situations when it would
be beneficial if software had a way to have influence on it
as well. It can be used to protect time sensitive
traffic in presence of Bluetooth voice stream,
for an example EAPOL handshake or DHCP negotiation.
This patch adds new attribute to SET_WIPHY command
and a new field in struct wiphy to allow control of the
coexistence behavior. Devices that do not share resources
with Bluetooth can ignore this parameter.
Reviewed-by: Pieter-Paul Giesberts <[email protected]>
Reviewed-by: Hante Meuleman <[email protected]>
Reviewed-by: Franky (Zhenhui) Lin <[email protected]>
Reviewed-by: Arend van Spriel <[email protected]>
Signed-off-by: Piotr Haber <[email protected]>
---
include/net/cfg80211.h | 4 ++++
include/uapi/linux/nl80211.h | 20 ++++++++++++++++++++
net/wireless/nl80211.c | 22 +++++++++++++++++++++-
3 files changed, 45 insertions(+), 1 deletion(-)
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 36e076e..3db21be 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1526,6 +1526,7 @@ struct cfg80211_connect_params {
* @WIPHY_PARAM_FRAG_THRESHOLD: wiphy->frag_threshold has changed
* @WIPHY_PARAM_RTS_THRESHOLD: wiphy->rts_threshold has changed
* @WIPHY_PARAM_COVERAGE_CLASS: coverage class changed
+ * @WIPHY_PARAM_BTCOEX: Bluetooth coexistence mode changed
*/
enum wiphy_params_flags {
WIPHY_PARAM_RETRY_SHORT = 1 << 0,
@@ -1533,6 +1534,7 @@ enum wiphy_params_flags {
WIPHY_PARAM_FRAG_THRESHOLD = 1 << 2,
WIPHY_PARAM_RTS_THRESHOLD = 1 << 3,
WIPHY_PARAM_COVERAGE_CLASS = 1 << 4,
+ WIPHY_PARAM_BTCOEX_MODE = 1 << 5,
};
/*
@@ -2322,6 +2324,7 @@ struct wiphy_wowlan_support {
* @max_sched_scan_ie_len: same as max_scan_ie_len, but for scheduled
* scans
* @coverage_class: current coverage class
+ * @btcoex_mode: Wifi-Bluetooth coexistence mode
* @fw_version: firmware version for ethtool reporting
* @hw_version: hardware version for ethtool reporting
* @max_num_pmkids: maximum number of PMKIDs supported by device
@@ -2401,6 +2404,7 @@ struct wiphy {
u32 frag_threshold;
u32 rts_threshold;
u8 coverage_class;
+ enum nl80211_btcoex_mode btcoex_mode;
char fw_version[ETHTOOL_BUSINFO_LEN];
u32 hw_version;
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 5b7dbc1..a96e1c9 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -1336,6 +1336,9 @@ enum nl80211_commands {
* number of MAC addresses that a device can support for MAC
* ACL.
*
+ * @NL80211_ATTR_WIPHY_BTCOEX_MODE: u8 attribute to control
+ * Wifi - Bluetooth coexistence mode
+ *
* @NL80211_ATTR_MAX: highest attribute number currently defined
* @__NL80211_ATTR_AFTER_LAST: internal use
*/
@@ -1614,6 +1617,8 @@ enum nl80211_attrs {
NL80211_ATTR_MAC_ACL_MAX,
+ NL80211_ATTR_WIPHY_BTCOEX_MODE,
+
/* add attributes here, update the policy in nl80211.c */
__NL80211_ATTR_AFTER_LAST,
@@ -3323,4 +3328,19 @@ enum nl80211_acl_policy {
NL80211_ACL_POLICY_DENY_UNLESS_LISTED,
};
+/**
+ * enum nl80211_btcoex_mode - Bluetooth coexistence mode
+ *
+ * Control Wifi - Bluetooth coexistence mode,
+ * to be used with %NL80211_ATTR_WIPHY_BTCOEX_MODE.
+ *
+ * @NL80211_BTCOEX_ENABLED: enable coexistence
+ * @NL80211_BTCOEX_DISABLED: disable coexistence, give Wifi
+ * traffic priority over Bluetooth
+ */
+enum nl80211_btcoex_mode {
+ NL80211_BTCOEX_ENABLED,
+ NL80211_BTCOEX_DISABLED,
+};
+
#endif /* __LINUX_NL80211_H */
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index b5978ab..f8f99ef 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -367,6 +367,7 @@ static const struct nla_policy nl80211_policy[NL80211_ATTR_MAX+1] = {
[NL80211_ATTR_P2P_OPPPS] = { .type = NLA_U8 },
[NL80211_ATTR_ACL_POLICY] = {. type = NLA_U32 },
[NL80211_ATTR_MAC_ADDRS] = { .type = NLA_NESTED },
+ [NL80211_ATTR_WIPHY_BTCOEX_MODE] = { .type = NLA_U8},
};
/* policy for the key attributes */
@@ -914,7 +915,9 @@ static int nl80211_send_wiphy(struct sk_buff *msg, u32 portid, u32 seq, int flag
nla_put_u16(msg, NL80211_ATTR_MAX_SCHED_SCAN_IE_LEN,
dev->wiphy.max_sched_scan_ie_len) ||
nla_put_u8(msg, NL80211_ATTR_MAX_MATCH_SETS,
- dev->wiphy.max_match_sets))
+ dev->wiphy.max_match_sets) ||
+ nla_put_u8(msg, NL80211_ATTR_WIPHY_BTCOEX_MODE,
+ dev->wiphy.btcoex_mode))
goto nla_put_failure;
if ((dev->wiphy.flags & WIPHY_FLAG_IBSS_RSN) &&
@@ -1528,6 +1531,7 @@ static int nl80211_set_wiphy(struct sk_buff *skb, struct genl_info *info)
u8 retry_short = 0, retry_long = 0;
u32 frag_threshold = 0, rts_threshold = 0;
u8 coverage_class = 0;
+ enum nl80211_btcoex_mode btcoex_mode = NL80211_BTCOEX_ENABLED;
/*
* Try to find the wiphy and netdev. Normally this
@@ -1745,10 +1749,22 @@ static int nl80211_set_wiphy(struct sk_buff *skb, struct genl_info *info)
changed |= WIPHY_PARAM_COVERAGE_CLASS;
}
+ if (info->attrs[NL80211_ATTR_WIPHY_BTCOEX_MODE]) {
+ btcoex_mode = nla_get_u8(
+ info->attrs[NL80211_ATTR_WIPHY_BTCOEX_MODE]);
+ if (btcoex_mode != NL80211_BTCOEX_ENABLED &&
+ btcoex_mode != NL80211_BTCOEX_DISABLED) {
+ result = -EINVAL;
+ goto bad_res;
+ }
+ changed |= WIPHY_PARAM_BTCOEX_MODE;
+ }
+
if (changed) {
u8 old_retry_short, old_retry_long;
u32 old_frag_threshold, old_rts_threshold;
u8 old_coverage_class;
+ enum nl80211_btcoex_mode old_btcoex_mode;
if (!rdev->ops->set_wiphy_params) {
result = -EOPNOTSUPP;
@@ -1760,6 +1776,7 @@ static int nl80211_set_wiphy(struct sk_buff *skb, struct genl_info *info)
old_frag_threshold = rdev->wiphy.frag_threshold;
old_rts_threshold = rdev->wiphy.rts_threshold;
old_coverage_class = rdev->wiphy.coverage_class;
+ old_btcoex_mode = rdev->wiphy.btcoex_mode;
if (changed & WIPHY_PARAM_RETRY_SHORT)
rdev->wiphy.retry_short = retry_short;
@@ -1771,6 +1788,8 @@ static int nl80211_set_wiphy(struct sk_buff *skb, struct genl_info *info)
rdev->wiphy.rts_threshold = rts_threshold;
if (changed & WIPHY_PARAM_COVERAGE_CLASS)
rdev->wiphy.coverage_class = coverage_class;
+ if (changed & WIPHY_PARAM_BTCOEX_MODE)
+ rdev->wiphy.btcoex_mode = btcoex_mode;
result = rdev_set_wiphy_params(rdev, changed);
if (result) {
@@ -1779,6 +1798,7 @@ static int nl80211_set_wiphy(struct sk_buff *skb, struct genl_info *info)
rdev->wiphy.frag_threshold = old_frag_threshold;
rdev->wiphy.rts_threshold = old_rts_threshold;
rdev->wiphy.coverage_class = old_coverage_class;
+ rdev->wiphy.btcoex_mode = old_btcoex_mode;
}
}
--
1.7.9.5
>> From: Johannes Berg [[email protected]]
>> Sent: Friday, February 22, 2013 12:52 PM
>> To: Piotr Haber
>> Cc: [email protected]
>> Subject: Re: [PATCH 1/2] [RFC] cfg80211: configuration of Bluetooth coexistence mode
>On Fri, 2013-02-22 at 10:08 +0100, Piotr Haber wrote:
>> Some devices share antenna/analog front end between Wifi
>> and Bluetooth. The hardware coexistence interface allows
>> them to do so, but there are situations when it would
>> be beneficial if software had a way to have influence on it
>> as well. It can be used to protect time sensitive
>> traffic in presence of Bluetooth voice stream,
>> for an example EAPOL handshake or DHCP negotiation.
>>
>> This patch adds new attribute to SET_WIPHY command
>> and a new field in struct wiphy to allow control of the
>> coexistence behavior. Devices that do not share resources
>> with Bluetooth can ignore this parameter.
> Apart from a few minor technical comments that I'll omit for now, I'm
> not sure what value this really has? EAPOL can already be "protected" by
> way of knowing when the station is marked authorized, and DHCP is pretty
> tricky because it could take "forever", might not be there at all, etc.
By "protect" I meant give Wifi a priority over BT so these time sensitive things
can finish quicker/on first try, limiting the possibility of dropping packets because of BT
using the medium.
This is supposed to be temporary and time limited, so if say DHCP finishes in the window
we give it - great, if not the coexistence goes back to default behavior and Wifi traffic is
treated as usual.
> What application would actually call this? I don't really see how it
> could be integrated like that.
For EAPOL wpa_supplicant might use it. For DHCP it could be used from enter/exit hooks
via iw or some other utility able to send nl messages.
This feature is styled after Android one.
There a Wifi state machine tries to "protect" DHCP traffic.
It uses extra wpa_supplicant command (DRIVER) with linked driver specific library.
Which means there can be only one wifi device and one needs to recompile wpa_supplicant
to use another.
I wanted to make it more device independent.
Piotr
Introduce feature to give Wifi traffic priority over Bluetooth.
Reviewed-by: Pieter-Paul Giesberts <[email protected]>
Reviewed-by: Hante Meuleman <[email protected]>
Reviewed-by: Arend van Spriel <[email protected]>
Signed-off-by: Piotr Haber <[email protected]>
---
drivers/net/wireless/brcm80211/brcmfmac/Makefile | 3 +-
drivers/net/wireless/brcm80211/brcmfmac/btcoex.c | 472 ++++++++++++++++++++
drivers/net/wireless/brcm80211/brcmfmac/btcoex.h | 24 +
.../net/wireless/brcm80211/brcmfmac/wl_cfg80211.c | 11 +
.../net/wireless/brcm80211/brcmfmac/wl_cfg80211.h | 3 +
5 files changed, 512 insertions(+), 1 deletion(-)
create mode 100644 drivers/net/wireless/brcm80211/brcmfmac/btcoex.c
create mode 100644 drivers/net/wireless/brcm80211/brcmfmac/btcoex.h
diff --git a/drivers/net/wireless/brcm80211/brcmfmac/Makefile b/drivers/net/wireless/brcm80211/brcmfmac/Makefile
index 4d58aee..6421926 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/Makefile
+++ b/drivers/net/wireless/brcm80211/brcmfmac/Makefile
@@ -30,7 +30,8 @@ brcmfmac-objs += \
p2p.o \
dhd_cdc.o \
dhd_common.o \
- dhd_linux.o
+ dhd_linux.o \
+ btcoex.o
brcmfmac-$(CONFIG_BRCMFMAC_SDIO) += \
dhd_sdio.o \
bcmsdh.o \
diff --git a/drivers/net/wireless/brcm80211/brcmfmac/btcoex.c b/drivers/net/wireless/brcm80211/brcmfmac/btcoex.c
new file mode 100644
index 0000000..09f5721
--- /dev/null
+++ b/drivers/net/wireless/brcm80211/brcmfmac/btcoex.c
@@ -0,0 +1,472 @@
+/*
+ * Copyright (c) 2013 Broadcom Corporation
+ *
+ * Permission to use, copy, modify, and/or distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY
+ * SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION
+ * OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN
+ * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+#include <linux/slab.h>
+#include <linux/netdevice.h>
+#include <net/cfg80211.h>
+
+#include <brcmu_wifi.h>
+#include <brcmu_utils.h>
+#include <defs.h>
+#include <dhd.h>
+#include <dhd_dbg.h>
+#include "fwil.h"
+#include "fwil_types.h"
+#include "btcoex.h"
+#include "p2p.h"
+#include "wl_cfg80211.h"
+
+/* T1 start SCO/eSCO priority suppression */
+#define BRCMF_BT_DHCP_OPPR_WIN_TIME 2500
+/* T2 turn off SCO/eSCO suppresion */
+#define BRCMF_BT_DHCP_FLAG_FORCE_TIME 5500
+
+/* BT registers values during DHCP */
+#define BRCMF_BT_DHCP_REG50 0x8022
+#define BRCMF_BT_DHCP_REG51 0
+#define BRCMF_BT_DHCP_REG64 0
+#define BRCMF_BT_DHCP_REG65 0
+#define BRCMF_BT_DHCP_REG71 0
+#define BRCMF_BT_DHCP_REG66 0x2710
+#define BRCMF_BT_DHCP_REG41 0x33
+#define BRCMF_BT_DHCP_REG68 0x190
+
+/* number of samples for SCO detection */
+#define BRCMF_BT_SCO_SAMPLES 12
+
+/**
+* enum brcmf_btcoex_state - BT coex DHCP state machine states
+* @BRCMF_BT_DHCP_IDLE: DCHP is idle
+* @BRCMF_BT_DHCP_START: DHCP started, wait before
+* boosting wifi priority
+* @BRCMF_BT_DHCP_OPPR_WIN: graceful DHCP opportunity ended,
+* boost wifi priority
+* @BRCMF_BT_DHCP_FLAG_FORCE_TIMEOUT: wifi priority boost end,
+* restore defaults
+*/
+enum brcmf_btcoex_state {
+ BRCMF_BT_DHCP_IDLE,
+ BRCMF_BT_DHCP_START,
+ BRCMF_BT_DHCP_OPPR_WIN,
+ BRCMF_BT_DHCP_FLAG_FORCE_TIMEOUT
+};
+
+/**
+* struct brcmf_btcoex_info - BT coex related information
+* @timer: timer for DHCP state machine
+* @timer_on: DHCP timer active
+* @dhcp_done: DHCP finished before T1/T2 timer expiration
+* @bt_state: DHCP state machine state
+* @work: DHCP state machine work
+* @cfg: driver private data for cfg80211 interface
+* @saved_reg66: saved value of btc_params 66
+* @saved_reg41: saved value of btc_params 41
+* @saved_reg68: saved value of btc_params 68
+* @saved_regs_part1: flag indicating regs 66,41,68
+* have been saved
+* @saved_reg51: saved value of btc_params 51
+* @saved_reg64: saved value of btc_params 64
+* @saved_reg65: saved value of btc_params 65
+* @saved_reg71: saved value of btc_params 71
+* @saved_regs_part1: flag indicating regs 50,51,64,65,71
+* have been saved
+*/
+struct brcmf_btcoex_info {
+ struct timer_list timer;
+ bool timer_on;
+ bool dhcp_done;
+ enum brcmf_btcoex_state bt_state;
+ struct work_struct work;
+ struct brcmf_cfg80211_info *cfg;
+ u32 saved_reg66;
+ u32 saved_reg41;
+ u32 saved_reg68;
+ bool saved_regs_part1;
+ u32 saved_reg50;
+ u32 saved_reg51;
+ u32 saved_reg64;
+ u32 saved_reg65;
+ u32 saved_reg71;
+ bool saved_regs_part2;
+};
+
+/**
+ * brcmf_btcoex_btcparams_write() - write bts_params firmware variable
+ * @ifp: interface
+ * @addr: btc_params register number
+ * @data: data to write
+ */
+static s32 brcmf_btcoex_btcparams_write(struct brcmf_if *ifp,
+ u32 addr, u32 data)
+{
+ struct {
+ __le32 addr;
+ __le32 data;
+ } reg_write;
+
+ reg_write.addr = cpu_to_le32(addr);
+ reg_write.data = cpu_to_le32(data);
+ return brcmf_fil_iovar_data_set(ifp, "btc_params",
+ ®_write, sizeof(reg_write));
+}
+
+/**
+ * brcmf_btcoex_btcparams_read() - read bts_params firmware variable
+ * @ifp: interface
+ * @addr: btc_params register number
+ * @data: read data
+ */
+static s32 brcmf_btcoex_btcparams_read(struct brcmf_if *ifp,
+ u32 addr, u32 *data)
+{
+ *data = addr;
+
+ return brcmf_fil_iovar_int_get(ifp, "btc_params", data);
+}
+
+/**
+ * brcmf_btcoex_boost_wifi() - control BT SCO/eSCO parameters
+ * @btcx_inf: BT coex info
+ * @trump_sco:
+ * true - set SCO/eSCO parameters for compatibility
+ * during DHCP window
+ * false - restore saved parameter values
+ *
+ * Enhanced BT COEX settings for eSCO compatibility during DHCP window
+ */
+static void brcmf_btcoex_boost_wifi(struct brcmf_btcoex_info *btcx_inf,
+ bool trump_sco)
+{
+ struct brcmf_if *ifp = btcx_inf->cfg->pub->iflist[0];
+
+ if (trump_sco && !btcx_inf->saved_regs_part2) {
+ /* this should reduce eSCO agressive
+ * retransmit w/o breaking it
+ */
+
+ /* save current */
+ brcmf_dbg(TRACE, "new SCO/eSCO coex algo {save & override}\n");
+ brcmf_btcoex_btcparams_read(ifp, 50,
+ &btcx_inf->saved_reg50);
+ brcmf_btcoex_btcparams_read(ifp, 51,
+ &btcx_inf->saved_reg51);
+ brcmf_btcoex_btcparams_read(ifp, 64,
+ &btcx_inf->saved_reg64);
+ brcmf_btcoex_btcparams_read(ifp, 65,
+ &btcx_inf->saved_reg65);
+ brcmf_btcoex_btcparams_read(ifp, 71,
+ &btcx_inf->saved_reg71);
+
+ btcx_inf->saved_regs_part2 = true;
+ brcmf_dbg(TRACE,
+ "saved bt_params[50,51,64,65,71]: 0x%x 0x%x 0x%x 0x%x 0x%x\n",
+ btcx_inf->saved_reg50, btcx_inf->saved_reg51,
+ btcx_inf->saved_reg64, btcx_inf->saved_reg65,
+ btcx_inf->saved_reg71);
+
+ /* pacify the eSco */
+ brcmf_btcoex_btcparams_write(ifp, 50,
+ BRCMF_BT_DHCP_REG50);
+ brcmf_btcoex_btcparams_write(ifp, 51,
+ BRCMF_BT_DHCP_REG51);
+ brcmf_btcoex_btcparams_write(ifp, 64,
+ BRCMF_BT_DHCP_REG64);
+ brcmf_btcoex_btcparams_write(ifp, 65,
+ BRCMF_BT_DHCP_REG65);
+ brcmf_btcoex_btcparams_write(ifp, 71,
+ BRCMF_BT_DHCP_REG71);
+
+ } else if (btcx_inf->saved_regs_part2) {
+ /* restore previously saved bt params */
+ brcmf_dbg(TRACE, "Do new SCO/eSCO coex algo {restore}\n");
+ brcmf_btcoex_btcparams_write(ifp, 50,
+ btcx_inf->saved_reg50);
+ brcmf_btcoex_btcparams_write(ifp, 51,
+ btcx_inf->saved_reg51);
+ brcmf_btcoex_btcparams_write(ifp, 64,
+ btcx_inf->saved_reg64);
+ brcmf_btcoex_btcparams_write(ifp, 65,
+ btcx_inf->saved_reg65);
+ brcmf_btcoex_btcparams_write(ifp, 71,
+ btcx_inf->saved_reg71);
+
+ brcmf_dbg(TRACE,
+ "restored bt_params[50,51,64,65,71]: 0x%x 0x%x 0x%x 0x%x 0x%x\n",
+ btcx_inf->saved_reg50, btcx_inf->saved_reg51,
+ btcx_inf->saved_reg64, btcx_inf->saved_reg65,
+ btcx_inf->saved_reg71);
+
+ btcx_inf->saved_regs_part2 = false;
+ } else {
+ brcmf_err("attempted to restore not saved BTCOEX params\n");
+ }
+}
+
+/**
+ * brcmf_btcoex_is_sco_active() - check if SCO/eSCO is active
+ * @ifp: interface
+ *
+ * return: true if SCO/eSCO session is active
+ */
+static bool brcmf_btcoex_is_sco_active(struct brcmf_if *ifp)
+{
+ int ioc_res = 0;
+ bool res = false;
+ int sco_id_cnt = 0;
+ u32 param27;
+ int i;
+
+ for (i = 0; i < BRCMF_BT_SCO_SAMPLES; i++) {
+ ioc_res = brcmf_btcoex_btcparams_read(ifp, 27,
+ ¶m27);
+
+ if (ioc_res < 0) {
+ brcmf_err("ioc read btc params error\n");
+ break;
+ }
+
+ brcmf_dbg(TRACE, "sample[%d], btc_params 27:%x\n", i, param27);
+
+ if ((param27 & 0x6) == 2) { /* count both sco & esco */
+ sco_id_cnt++;
+ }
+
+ if (sco_id_cnt > 2) {
+ brcmf_dbg(TRACE,
+ "sco/esco detected, pkt id_cnt:%d samples:%d\n",
+ sco_id_cnt, i);
+ res = true;
+ break;
+ }
+ }
+ return res;
+}
+
+/**
+ * brcmf_btcoex_timerfunc() - BT coex timer callback
+ */
+static void brcmf_btcoex_timerfunc(ulong data)
+{
+ struct brcmf_btcoex_info *bt_local = (struct brcmf_btcoex_info *)data;
+ brcmf_dbg(TRACE, "enter\n");
+
+ bt_local->timer_on = false;
+ schedule_work(&bt_local->work);
+}
+
+/**
+ * brcmf_btcoex_handler() - BT coex state machine work handler
+ * @work: work
+ */
+static void brcmf_btcoex_handler(struct work_struct *work)
+{
+ struct brcmf_btcoex_info *btcx_inf;
+ btcx_inf = container_of(work, struct brcmf_btcoex_info, work);
+ if (btcx_inf->timer_on) {
+ btcx_inf->timer_on = false;
+ del_timer_sync(&btcx_inf->timer);
+ }
+
+ switch (btcx_inf->bt_state) {
+ case BRCMF_BT_DHCP_START:
+ /* DHCP started provide OPPORTUNITY window
+ to get DHCP address
+ */
+ brcmf_dbg(TRACE, "DHCP started\n");
+ btcx_inf->bt_state = BRCMF_BT_DHCP_OPPR_WIN;
+ mod_timer(&btcx_inf->timer, jiffies +
+ msecs_to_jiffies(BRCMF_BT_DHCP_OPPR_WIN_TIME));
+ btcx_inf->timer_on = true;
+ break;
+
+ case BRCMF_BT_DHCP_OPPR_WIN:
+ if (btcx_inf->dhcp_done) {
+ brcmf_dbg(TRACE, "DHCP done before T1 expiration\n");
+ goto btc_coex_idle;
+ }
+
+ /* DHCP is not over yet, start lowering BT priority */
+ brcmf_dbg(TRACE, "DHCP T1:%d expired\n",
+ BRCMF_BT_DHCP_OPPR_WIN_TIME);
+ brcmf_btcoex_boost_wifi(btcx_inf, true);
+
+ btcx_inf->bt_state = BRCMF_BT_DHCP_FLAG_FORCE_TIMEOUT;
+ mod_timer(&btcx_inf->timer, jiffies +
+ msecs_to_jiffies(BRCMF_BT_DHCP_FLAG_FORCE_TIME));
+ btcx_inf->timer_on = true;
+ break;
+
+ case BRCMF_BT_DHCP_FLAG_FORCE_TIMEOUT:
+ if (btcx_inf->dhcp_done)
+ brcmf_dbg(TRACE, "DHCP done before T2 expiration\n");
+ else
+ brcmf_dbg(TRACE, "DHCP T2:%d expired\n",
+ BRCMF_BT_DHCP_FLAG_FORCE_TIME);
+
+ /* Restoring default bt priority */
+ brcmf_btcoex_boost_wifi(btcx_inf, false);
+btc_coex_idle:
+ btcx_inf->bt_state = BRCMF_BT_DHCP_IDLE;
+ btcx_inf->timer_on = false;
+ break;
+
+ default:
+ brcmf_dbg(TRACE, "error state=%d !!!\n", btcx_inf->bt_state);
+ brcmf_btcoex_boost_wifi(btcx_inf, false);
+ btcx_inf->bt_state = BRCMF_BT_DHCP_IDLE;
+ btcx_inf->timer_on = false;
+ break;
+ }
+}
+
+/**
+ * brcmf_btcoex_init() - initialize BT coex data
+ * @cfg: driver private cfg80211 data
+ *
+ * return: 0 on success
+ */
+int brcmf_btcoex_init(struct brcmf_cfg80211_info *cfg)
+{
+ struct brcmf_btcoex_info *btco_inf = NULL;
+ brcmf_dbg(TRACE, "enter\n");
+
+ btco_inf = kmalloc(sizeof(struct brcmf_btcoex_info), GFP_KERNEL);
+ if (!btco_inf)
+ return -ENOMEM;
+
+ btco_inf->bt_state = BRCMF_BT_DHCP_IDLE;
+
+ /* Set up timer for BT */
+ btco_inf->timer_on = false;
+ init_timer(&btco_inf->timer);
+ btco_inf->timer.data = (ulong)btco_inf;
+ btco_inf->timer.function = brcmf_btcoex_timerfunc;
+ btco_inf->cfg = cfg;
+ btco_inf->saved_regs_part1 = false;
+ btco_inf->saved_regs_part2 = false;
+
+ INIT_WORK(&btco_inf->work, brcmf_btcoex_handler);
+
+ cfg->btcoex = btco_inf;
+ return 0;
+}
+
+/**
+ * brcmf_btcoex_deinit - clean BT coex data
+ * @cfg: driver private cfg80211 data
+ */
+void brcmf_btcoex_deinit(struct brcmf_cfg80211_info *cfg)
+{
+ brcmf_dbg(TRACE, "enter\n");
+
+ if (!cfg->btcoex)
+ return;
+
+ if (cfg->btcoex->timer_on) {
+ cfg->btcoex->timer_on = false;
+ del_timer_sync(&cfg->btcoex->timer);
+ }
+
+ cancel_work_sync(&cfg->btcoex->work);
+
+ kfree(cfg->btcoex);
+ cfg->btcoex = NULL;
+}
+
+/**
+ * brcmf_btcoex_set_mode - set BT coex mode
+ * @cfg: driver private cfg80211 data
+ * @mode: Wifi-Bluetooth coexistence mode
+ *
+ * return: 0 on success
+ */
+int brcmf_btcoex_set_mode(struct brcmf_cfg80211_info *cfg,
+ enum nl80211_btcoex_mode mode)
+{
+ struct brcmf_btcoex_info *btco_inf = cfg->btcoex;
+ struct brcmf_if *ifp = cfg->pub->iflist[0];
+
+ switch (mode) {
+ case NL80211_BTCOEX_DISABLED:
+ brcmf_dbg(TRACE, "DHCP session starts\n");
+
+ if (!btco_inf->saved_regs_part1) {
+ /* Retrieve and save original reg value */
+ brcmf_btcoex_btcparams_read(ifp, 66,
+ &btco_inf->saved_reg66);
+ brcmf_btcoex_btcparams_read(ifp, 41,
+ &btco_inf->saved_reg41);
+ brcmf_btcoex_btcparams_read(ifp, 68,
+ &btco_inf->saved_reg68);
+
+ btco_inf->saved_regs_part1 = true;
+ brcmf_dbg(TRACE,
+ "saved btc_params regs (66,41,68) 0x%x 0x%x 0x%x\n",
+ btco_inf->saved_reg66, btco_inf->saved_reg41,
+ btco_inf->saved_reg68);
+ }
+ /* Start BT timer only for SCO connection */
+ if (brcmf_btcoex_is_sco_active(ifp)) {
+ /* set new regs values */
+ brcmf_btcoex_btcparams_write(ifp, 66,
+ BRCMF_BT_DHCP_REG66);
+ brcmf_btcoex_btcparams_write(ifp, 41,
+ BRCMF_BT_DHCP_REG41);
+ brcmf_btcoex_btcparams_write(ifp, 68,
+ BRCMF_BT_DHCP_REG68);
+ btco_inf->dhcp_done = false;
+ btco_inf->bt_state = BRCMF_BT_DHCP_START;
+ btco_inf->timer_on = true;
+ mod_timer(&btco_inf->timer, btco_inf->timer.expires);
+ brcmf_dbg(TRACE, "enable BT DHCP Timer\n");
+ }
+ break;
+
+ case NL80211_BTCOEX_ENABLED:
+ brcmf_dbg(TRACE, "DHCP session ends\n");
+ /* Stop any bt timer because DHCP session is done */
+ btco_inf->dhcp_done = true;
+ if (btco_inf->timer_on) {
+ brcmf_dbg(TRACE, "disable BT DHCP Timer\n");
+ btco_inf->timer_on = false;
+ del_timer_sync(&btco_inf->timer);
+ if (btco_inf->bt_state != BRCMF_BT_DHCP_IDLE) {
+ brcmf_dbg(TRACE, "bt_state:%d\n",
+ btco_inf->bt_state);
+ /* wake up btcoex thread */
+ schedule_work(&btco_inf->work);
+ }
+ }
+ /* Restore original values */
+ if (btco_inf->saved_regs_part1) {
+ /* btc_params */
+ brcmf_btcoex_btcparams_write(ifp, 66,
+ btco_inf->saved_reg66);
+ brcmf_btcoex_btcparams_write(ifp, 41,
+ btco_inf->saved_reg41);
+ brcmf_btcoex_btcparams_write(ifp, 68,
+ btco_inf->saved_reg68);
+ brcmf_dbg(TRACE,
+ "restored btc_params regs {66,41,68} 0x%x 0x%x 0x%x\n",
+ btco_inf->saved_reg66, btco_inf->saved_reg41,
+ btco_inf->saved_reg68);
+ }
+ btco_inf->saved_regs_part1 = false;
+ break;
+ default:
+ brcmf_dbg(TRACE, "Unknown mode, ignored\n");
+ }
+ return 0;
+}
diff --git a/drivers/net/wireless/brcm80211/brcmfmac/btcoex.h b/drivers/net/wireless/brcm80211/brcmfmac/btcoex.h
new file mode 100644
index 0000000..5867123
--- /dev/null
+++ b/drivers/net/wireless/brcm80211/brcmfmac/btcoex.h
@@ -0,0 +1,24 @@
+/*
+ * Copyright (c) 2013 Broadcom Corporation
+ *
+ * Permission to use, copy, modify, and/or distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY
+ * SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION
+ * OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN
+ * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+#ifndef WL_BTCOEX_H_
+#define WL_BTCOEX_H_
+
+int brcmf_btcoex_init(struct brcmf_cfg80211_info *cfg);
+void brcmf_btcoex_deinit(struct brcmf_cfg80211_info *cfg);
+int brcmf_btcoex_set_mode(struct brcmf_cfg80211_info *cfg,
+ enum nl80211_btcoex_mode mode);
+
+#endif /* WL_BTCOEX_H_ */
diff --git a/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c b/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
index a4f27f6..188d142 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
@@ -28,6 +28,7 @@
#include "dhd_dbg.h"
#include "fwil_types.h"
#include "p2p.h"
+#include "btcoex.h"
#include "wl_cfg80211.h"
#include "fwil.h"
@@ -1084,6 +1085,13 @@ static s32 brcmf_cfg80211_set_wiphy_params(struct wiphy *wiphy, u32 changed)
if (!err)
goto done;
}
+ if (changed & WIPHY_PARAM_BTCOEX_MODE &&
+ (cfg->conf->btcoex_mode != wiphy->btcoex_mode)) {
+ cfg->conf->btcoex_mode = wiphy->btcoex_mode;
+ err = brcmf_btcoex_set_mode(cfg, cfg->conf->btcoex_mode);
+ if (!err)
+ goto done;
+ }
done:
brcmf_dbg(TRACE, "Exit\n");
@@ -4657,6 +4665,7 @@ static void brcmf_init_conf(struct brcmf_cfg80211_conf *conf)
conf->retry_short = (u32)-1;
conf->retry_long = (u32)-1;
conf->tx_power = -1;
+ conf->btcoex_mode = NL80211_BTCOEX_ENABLED;
}
static void brcmf_register_event_handlers(struct brcmf_cfg80211_info *cfg)
@@ -4814,6 +4823,7 @@ struct brcmf_cfg80211_info *brcmf_cfg80211_attach(struct brcmf_pub *drvr,
brcmf_err("P2P initilisation failed (%d)\n", err);
goto cfg80211_p2p_attach_out;
}
+ brcmf_btcoex_init(cfg);
return cfg;
@@ -4832,6 +4842,7 @@ void brcmf_cfg80211_detach(struct brcmf_cfg80211_info *cfg)
struct brcmf_cfg80211_vif *tmp;
wl_deinit_priv(cfg);
+ brcmf_btcoex_deinit(cfg);
list_for_each_entry_safe(vif, tmp, &cfg->vif_list, list) {
brcmf_free_vif(vif);
}
diff --git a/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.h b/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.h
index 8b5d498..f764925 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.h
+++ b/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.h
@@ -107,6 +107,7 @@ struct brcmf_cfg80211_conf {
u32 retry_long;
s32 tx_power;
struct ieee80211_channel channel;
+ enum nl80211_btcoex_mode btcoex_mode;
};
/* basic structure of scan request */
@@ -347,6 +348,7 @@ struct brcmf_cfg80211_vif_event {
* @wiphy: wiphy object for cfg80211 interface.
* @conf: dongle configuration.
* @p2p: peer-to-peer specific information.
+ * @btcoex: Bluetooth coexistence information.
* @scan_request: cfg80211 scan request object.
* @usr_sync: mainly for dongle up/down synchronization.
* @bss_list: bss_list holding scanned ap information.
@@ -380,6 +382,7 @@ struct brcmf_cfg80211_info {
struct wiphy *wiphy;
struct brcmf_cfg80211_conf *conf;
struct brcmf_p2p_info p2p;
+ struct brcmf_btcoex_info *btcoex;
struct cfg80211_scan_request *scan_request;
struct mutex usr_sync;
struct brcmf_scan_results *bss_list;
--
1.7.9.5
On 02/25/13 06:54, Felix Fietkau wrote:
> On 2013-02-25 6:08 AM, Adrian Chadd wrote:
>> On 24 February 2013 09:28, Johannes Berg<[email protected]> wrote:
>>
>>>> In the driver we could inspect each sk_buff and boost priority of any
>>>> BOOTP packets so that it will end up in the AC_VO fifo and have
>>>> hardware coexistence handle it further. I consider that more a
>>>> pragmatic fix, which is not always the worst thing to go for.
>>>
>>> Well, I don't really think that's the best idea. Sniffing the protocol
>>> is clumsy at best.
>>
>> Sure, but it's the kind of thing that commercial devices do in order
>> to work around real world issues.
> Just because commercial devices do this crap to weasel out of fixing
> things properly doesn't mean we have to do the same.
>
>> So it'd be nice to have a programmatic way to detect things (eg bootp
>> packets going out) and signal the driver via some side channel to do
>> things like btcoex weight changing.
> I disagree, that approach clumsy and fragile and it's trying to address
> the issue in the wrong place.
>
> Most devices have some kind of connection manager that has a high-level
> perspective of when it's fully connected (which includes DHCP/bootp).
Hi Felix,
I agree with you here. I think the responsibility is indeed to be placed
at a higher level.
> Why not just let that connection manager set a sane maximum network
> latency value via pm_qos network_latency and derive btcoex weight
> changing and multi-channel settings from that?
Now you are loosing me, but that can be helped. Could you educate me
here a bit or some pointers to reading material? If there is a proper
means already in place to communicate the information we might as well
use it.
> - Felix
>
Regards,
Arend
On 2013-02-25 6:08 AM, Adrian Chadd wrote:
> On 24 February 2013 09:28, Johannes Berg <[email protected]> wrote:
>
>>> In the driver we could inspect each sk_buff and boost priority of any
>>> BOOTP packets so that it will end up in the AC_VO fifo and have
>>> hardware coexistence handle it further. I consider that more a
>>> pragmatic fix, which is not always the worst thing to go for.
>>
>> Well, I don't really think that's the best idea. Sniffing the protocol
>> is clumsy at best.
>
> Sure, but it's the kind of thing that commercial devices do in order
> to work around real world issues.
Just because commercial devices do this crap to weasel out of fixing
things properly doesn't mean we have to do the same.
> So it'd be nice to have a programmatic way to detect things (eg bootp
> packets going out) and signal the driver via some side channel to do
> things like btcoex weight changing.
I disagree, that approach clumsy and fragile and it's trying to address
the issue in the wrong place.
Most devices have some kind of connection manager that has a high-level
perspective of when it's fully connected (which includes DHCP/bootp).
Why not just let that connection manager set a sane maximum network
latency value via pm_qos network_latency and derive btcoex weight
changing and multi-channel settings from that?
- Felix
On Sat, 2013-02-23 at 17:47 +0000, Arend Van Spriel wrote:
> On Saturday, February 23, 2013 1:30 AM Adrian Chadd wrote:
> >
> > ... why not tag such traffic with a higher QOS priority and then have
> > the BT coex module snoop thaT?
> >
>
> In the driver we could inspect each sk_buff and boost priority of any
> BOOTP packets so that it will end up in the AC_VO fifo and have
> hardware coexistence handle it further. I consider that more a
> pragmatic fix, which is not always the worst thing to go for.
Well, I don't really think that's the best idea. Sniffing the protocol
is clumsy at best.
> However, I would prefer a solution is which user-space, ie. dhcp
> client can control the priority and/or BT coexistence.
It's not just BT coex btw, with multi-channel support it might also be
interesting to not switch channels while DHCP is going on, for example.
johannes
On Mon, 2013-02-25 at 06:54 +0100, Felix Fietkau wrote:
> Most devices have some kind of connection manager that has a high-level
> perspective of when it's fully connected (which includes DHCP/bootp).
> Why not just let that connection manager set a sane maximum network
> latency value via pm_qos network_latency and derive btcoex weight
> changing and multi-channel settings from that?
Frankly, I don't think that's going to work well. We tried using the
pm_qos framework once and nothing ever used it. Android isn't going to
change to it, so we'd be stuck with hacks like setting pm_qos in
wpa_supplicant which is just as awkward.
Also, what you mostly want isn't really so much a weight but rather a
time-based approach to give it high priority until the connection
handshake completes (we already do for auth/assoc/... until authorized)
so I think using the pm_qos framework to give priority wouldn't work
very well since there'd also be no way to tell when it was "done"
johannes
On Fri, 2013-02-22 at 10:08 +0100, Piotr Haber wrote:
> Some devices share antenna/analog front end between Wifi
> and Bluetooth. The hardware coexistence interface allows
> them to do so, but there are situations when it would
> be beneficial if software had a way to have influence on it
> as well. It can be used to protect time sensitive
> traffic in presence of Bluetooth voice stream,
> for an example EAPOL handshake or DHCP negotiation.
>
> This patch adds new attribute to SET_WIPHY command
> and a new field in struct wiphy to allow control of the
> coexistence behavior. Devices that do not share resources
> with Bluetooth can ignore this parameter.
Apart from a few minor technical comments that I'll omit for now, I'm
not sure what value this really has? EAPOL can already be "protected" by
way of knowing when the station is marked authorized, and DHCP is pretty
tricky because it could take "forever", might not be there at all, etc.
What application would actually call this? I don't really see how it
could be integrated like that.
johannes
On Fri, 2013-02-22 at 13:32 +0000, Piotr Haber wrote:
> > Apart from a few minor technical comments that I'll omit for now, I'm
> > not sure what value this really has? EAPOL can already be "protected" by
> > way of knowing when the station is marked authorized, and DHCP is pretty
> > tricky because it could take "forever", might not be there at all, etc.
> By "protect" I meant give Wifi a priority over BT so these time sensitive things
> can finish quicker/on first try, limiting the possibility of dropping packets because of BT
> using the medium.
I know :)
> This is supposed to be temporary and time limited, so if say DHCP finishes in the window
> we give it - great, if not the coexistence goes back to default behavior and Wifi traffic is
> treated as usual.
That's not even documented/implemented, the way I read the patch you'd
have to set it back manually.
> > What application would actually call this? I don't really see how it
> > could be integrated like that.
> For EAPOL wpa_supplicant might use it. For DHCP it could be used from enter/exit hooks
> via iw or some other utility able to send nl messages.
See that's the thing, I don't really see the point for EAPOL: you could
just as well start protecting when the association is done, and end it
when the station is marked authorized. That will have protected any
EAPOL (or other protocols for that matter) traffic.
Similarly, you could give it a certain timeout to protect DHCP traffic.
I guess the only thing that would seem necessary would be a notification
of "DHCP done" that would allow you to drop the protection right away.
> This feature is styled after Android one.
I know, I'm (vaguely) familiar with that.
> There a Wifi state machine tries to "protect" DHCP traffic.
Is there any *reason* for it though? Would it ever call it after the
connection is fully established?
To me this seems not very well thought out.
johannes
On Wed, 2013-02-27 at 11:27 +0100, Dan Williams wrote:
>
> We also don't know what IP configuration method will get used; whether
> it will be IPv6 RA, DHCPv4 or DHCPv6, IPv4 autoconf, or static. Only
> the connection manager knows that. Only the connection manager/DHCP
> client know when they expect a lease renew operation to start too.
> wpa_supplicant doesn't know any of these things either since it doesn't
> do anything IP related.
Right, and if it's static you don't want any of this. I don't think
network latency stuff is a good method, even though on the face of it
the idea here is to reduce the initial connection latency.
> I think the best approach here is to allow the higher layers to hint to
> the driver that some operations that are about to start must be "more
> reliable". That includes EAPOL, DHCP, IP autoconfiguration, etc. Then
> when the higher layers know the operation is finished, they can indicate
> the operations are done and the driver can go do whatever it wants.
Yeah, I agree.
Note that it's less about being "more reliable" and more about being
"fast" (for some definition of that), in particular in the multi-channel
scenario you'd want to attempt to not switch away from the channel (if
possible) until DHCP finishes, or at least give it a chance to finish
quickly. But for example for static IP configuration you don't need to
and don't necessarily want to block being on that channel just in case
DHCP happens. Similarly with BT Coex of course, though there I guess
it's more about priority.
In any case it's about completing the connection to the network quickly
before being forced to go to powersave. In particular IPv6 might also
require multicast so doing powersave there really hurts (DTIM beacon.)
> The driver/stack may wish to do any of [set 1Mb rate, block rate
> control, change BT coex, turn on microwave protection, whatever] and
> that's great, the upper layers don't care about what the driver does,
> just that the reliability of the operation is preserved.
I'd say "reliability of the operation is _increased_" I guess :-) But
yeah.
johannes
On Mon, 2013-02-25 at 14:07 +0100, Felix Fietkau wrote:
> On 2013-02-25 11:25 AM, Johannes Berg wrote:
> > On Mon, 2013-02-25 at 06:54 +0100, Felix Fietkau wrote:
> >
> >> Most devices have some kind of connection manager that has a high-level
> >> perspective of when it's fully connected (which includes DHCP/bootp).
> >> Why not just let that connection manager set a sane maximum network
> >> latency value via pm_qos network_latency and derive btcoex weight
> >> changing and multi-channel settings from that?
> >
> > Frankly, I don't think that's going to work well. We tried using the
> > pm_qos framework once and nothing ever used it. Android isn't going to
> > change to it, so we'd be stuck with hacks like setting pm_qos in
> > wpa_supplicant which is just as awkward.
> If only the connection manager gets changed to use it, that would
> already be enough. It doesn't have to be pushed into dhcp clients and
> other applications.
>
> > Also, what you mostly want isn't really so much a weight but rather a
> > time-based approach to give it high priority until the connection
> > handshake completes (we already do for auth/assoc/... until authorized)
> > so I think using the pm_qos framework to give priority wouldn't work
> > very well since there'd also be no way to tell when it was "done"
> Just release the latency requirement in the connection manager once the
> handshake is done. It knows...
We also don't know what IP configuration method will get used; whether
it will be IPv6 RA, DHCPv4 or DHCPv6, IPv4 autoconf, or static. Only
the connection manager knows that. Only the connection manager/DHCP
client know when they expect a lease renew operation to start too.
wpa_supplicant doesn't know any of these things either since it doesn't
do anything IP related.
I think the best approach here is to allow the higher layers to hint to
the driver that some operations that are about to start must be "more
reliable". That includes EAPOL, DHCP, IP autoconfiguration, etc. Then
when the higher layers know the operation is finished, they can indicate
the operations are done and the driver can go do whatever it wants.
The driver/stack may wish to do any of [set 1Mb rate, block rate
control, change BT coex, turn on microwave protection, whatever] and
that's great, the upper layers don't care about what the driver does,
just that the reliability of the operation is preserved.
Dan
> > This is supposed to be temporary and time limited, so if say DHCP finishes in the window
> > we give it - great, if not the coexistence goes back to default behavior and Wifi traffic is
> > treated as usual.
> That's not even documented/implemented, the way I read the patch you'd
> have to set it back manually.
You are right, we implement it this way in brcmfmac but it is not documented
in nl80211 change. I thought of DISABLED as a trigger (for timed boost)
and ENABLED as a way to notify that whoever requested it is done.
The interface could be made to reflect this "design"
> > > What application would actually call this? I don't really see how it
> > > could be integrated like that.
> > For EAPOL wpa_supplicant might use it. For DHCP it could be used from enter/exit hooks
> > via iw or some other utility able to send nl messages.
> See that's the thing, I don't really see the point for EAPOL: you could
> just as well start protecting when the association is done, and end it
> when the station is marked authorized. That will have protected any
> EAPOL (or other protocols for that matter) traffic.
That is true, this could be totally automatic.
> Similarly, you could give it a certain timeout to protect DHCP traffic.
> I guess the only thing that would seem necessary would be a notification
> of "DHCP done" that would allow you to drop the protection right away.
I guess that is the only reason we need this interface.
Still if we set the "protection" timeout to be reasonable it can do without
it...
> > This feature is styled after Android one.
> I know, I'm (vaguely) familiar with that.
> > There a Wifi state machine tries to "protect" DHCP traffic.
> Is there any *reason* for it though? Would it ever call it after the
> connection is fully established?
As for reason I can't tell, someone found one to implement it in Android :)
It is not used after connection is established.
Piotr
On 02/27/13 18:44, Arend van Spriel wrote:
>> I think the best approach here is to allow the higher layers to hint to
>> the driver that some operations that are about to start must be "more
>> reliable". That includes EAPOL, DHCP, IP autoconfiguration, etc. Then
>> when the higher layers know the operation is finished, they can indicate
>> the operations are done and the driver can go do whatever it wants.
>
> Indeed the RFC approach was explicit about the scope of this interface being BT coex or actually BT
> coex override. Johannes proposes one dedicated to DHCP as a similar interface in Android is used for
> that right now. Abstracting it to "more reliable" mainly avoids renaming it when someone comes up
> with a use-case other than BT coex or DHCP.
Definitely a more generic interface would be great as there are many driver independent things
that can be done to improve "reliability" (defer scanning, prevent band/channel switching).
But I think it is a good idea to keep this "reliability boost" time constrained on cfg80211 level,
with userspace signaling that it is done resulting in return to default operation earlier.
Piotr
On 02/25/13 14:07, Felix Fietkau wrote:
> On 2013-02-25 11:25 AM, Johannes Berg wrote:
>> On Mon, 2013-02-25 at 06:54 +0100, Felix Fietkau wrote:
>>
>>> Most devices have some kind of connection manager that has a high-level
>>> perspective of when it's fully connected (which includes DHCP/bootp).
>>> Why not just let that connection manager set a sane maximum network
>>> latency value via pm_qos network_latency and derive btcoex weight
>>> changing and multi-channel settings from that?
>>
>> Frankly, I don't think that's going to work well. We tried using the
>> pm_qos framework once and nothing ever used it. Android isn't going to
>> change to it, so we'd be stuck with hacks like setting pm_qos in
>> wpa_supplicant which is just as awkward.
> If only the connection manager gets changed to use it, that would
> already be enough. It doesn't have to be pushed into dhcp clients and
> other applications.
Reading back some slides about pm_qos it (from Intel OSTC ;-) ) seems to
be intended to make kernel drivers aware of performance requirements in
other kernel parts or user-space. Sounds like a match here although it
is a one-to-many notification framework. Also not exactly what we want
(I think).
Gr. AvS
On 2013-02-25 11:25 AM, Johannes Berg wrote:
> On Mon, 2013-02-25 at 06:54 +0100, Felix Fietkau wrote:
>
>> Most devices have some kind of connection manager that has a high-level
>> perspective of when it's fully connected (which includes DHCP/bootp).
>> Why not just let that connection manager set a sane maximum network
>> latency value via pm_qos network_latency and derive btcoex weight
>> changing and multi-channel settings from that?
>
> Frankly, I don't think that's going to work well. We tried using the
> pm_qos framework once and nothing ever used it. Android isn't going to
> change to it, so we'd be stuck with hacks like setting pm_qos in
> wpa_supplicant which is just as awkward.
If only the connection manager gets changed to use it, that would
already be enough. It doesn't have to be pushed into dhcp clients and
other applications.
> Also, what you mostly want isn't really so much a weight but rather a
> time-based approach to give it high priority until the connection
> handshake completes (we already do for auth/assoc/... until authorized)
> so I think using the pm_qos framework to give priority wouldn't work
> very well since there'd also be no way to tell when it was "done"
Just release the latency requirement in the connection manager once the
handshake is done. It knows...
- Felix