* [PATCH v2] nfc: don't be making arch specific unaligned decisions at driver level.
From: Paul Gortmaker @ 2017-01-09 17:52 UTC (permalink / raw)
To: linux-kernel
Cc: kbuild test robot, Paul Gortmaker, Lauro Ramos Venancio,
Aloisio Almeida Jr, Samuel Ortiz, Tony Luck, Fenghua Yu,
linux-ia64, linux-wireless
In-Reply-To: <201701090839.5u37Hdez%fengguang.wu@intel.com>
Currently ia64 fails building allmodconfig with variations of:
In file included from drivers/nfc/nxp-nci/i2c.c:39:0:
./include/linux/unaligned/access_ok.h:62:29: error: redefinition of ‘put_unaligned_be64’
static __always_inline void put_unaligned_be64(u64 val, void *p)
^~~~~~~~~~~~~~~~~~
In file included from ./arch/ia64/include/asm/unaligned.h:5:0,
from ./arch/ia64/include/asm/io.h:22,
from ./arch/ia64/include/asm/smp.h:20,
from ./include/linux/smp.h:59,
from ./include/linux/topology.h:33,
from ./include/linux/gfp.h:8,
from ./include/linux/slab.h:14,
from ./include/linux/resource_ext.h:19,
from ./include/linux/acpi.h:26,
from drivers/nfc/nxp-nci/i2c.c:28:
./include/linux/unaligned/be_byteshift.h:65:20: note: previous definition of ‘put_unaligned_be64’ was here
static inline void put_unaligned_be64(u64 val, void *p)
^~~~~~~~~~~~~~~~~~
scripts/Makefile.build:293: recipe for target 'drivers/nfc/nxp-nci/i2c.o' failed
The easiest explanation for this is to look at the non-arch users in
the following output:
linux$git grep include.*access_ok.h
arch/arm64/crypto/crc32-arm64.c:#include <linux/unaligned/access_ok.h>
arch/cris/include/asm/unaligned.h:#include <linux/unaligned/access_ok.h>
arch/m68k/include/asm/unaligned.h:#include <linux/unaligned/access_ok.h>
arch/mn10300/include/asm/unaligned.h:#include <linux/unaligned/access_ok.h>
arch/powerpc/include/asm/unaligned.h:#include <linux/unaligned/access_ok.h>
arch/s390/include/asm/unaligned.h:#include <linux/unaligned/access_ok.h>
arch/x86/include/asm/unaligned.h:#include <linux/unaligned/access_ok.h>
drivers/nfc/nfcmrvl/fw_dnld.c:#include <linux/unaligned/access_ok.h>
drivers/nfc/nxp-nci/firmware.c:#include <linux/unaligned/access_ok.h>
drivers/nfc/nxp-nci/i2c.c:#include <linux/unaligned/access_ok.h>
include/asm-generic/unaligned.h:# include <linux/unaligned/access_ok.h>
Note that nfc is essentially the only non-arch user in the above.
When it forces use of access_ok.h, it will break any arch that has
already selected be_byteshift.h (or other conflicting implementations)
at the arch level.
The decision of what variant for unaligned access to use needs to be
left to the arch level and not used at the driver level. Since not
all arch will have sourced asm/unaligned.h already, we need to call
it out and then the arch can give us just the one definition that
is needed.
See commit 064106a91be5 ("kernel: add common infrastructure for
unaligned access") as a reference.
Cc: Lauro Ramos Venancio <lauro.venancio@openbossa.org>
Cc: Aloisio Almeida Jr <aloisio.almeida@openbossa.org>
Cc: Samuel Ortiz <sameo@linux.intel.com>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: linux-ia64@vger.kernel.org
Cc: linux-wireless@vger.kernel.org
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
[v2: explicitly include asm/uaccess.h since some arch won't be
getting any variant of an unaligned access header without it.
Build test allmodconfig on x86-64, i386, arm64, ia64. ]
drivers/nfc/nfcmrvl/fw_dnld.c | 2 +-
drivers/nfc/nxp-nci/firmware.c | 2 +-
drivers/nfc/nxp-nci/i2c.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/nfc/nfcmrvl/fw_dnld.c b/drivers/nfc/nfcmrvl/fw_dnld.c
index f8dcdf4b24f6..f3f246ddae06 100644
--- a/drivers/nfc/nfcmrvl/fw_dnld.c
+++ b/drivers/nfc/nfcmrvl/fw_dnld.c
@@ -17,11 +17,11 @@
*/
#include <linux/module.h>
-#include <linux/unaligned/access_ok.h>
#include <linux/firmware.h>
#include <linux/nfc.h>
#include <net/nfc/nci.h>
#include <net/nfc/nci_core.h>
+#include <asm/unaligned.h>
#include "nfcmrvl.h"
#define FW_DNLD_TIMEOUT 15000
diff --git a/drivers/nfc/nxp-nci/firmware.c b/drivers/nfc/nxp-nci/firmware.c
index 5291797324ba..553011f58339 100644
--- a/drivers/nfc/nxp-nci/firmware.c
+++ b/drivers/nfc/nxp-nci/firmware.c
@@ -24,7 +24,7 @@
#include <linux/completion.h>
#include <linux/firmware.h>
#include <linux/nfc.h>
-#include <linux/unaligned/access_ok.h>
+#include <asm/unaligned.h>
#include "nxp-nci.h"
diff --git a/drivers/nfc/nxp-nci/i2c.c b/drivers/nfc/nxp-nci/i2c.c
index 36099e557730..20243dafea21 100644
--- a/drivers/nfc/nxp-nci/i2c.c
+++ b/drivers/nfc/nxp-nci/i2c.c
@@ -36,9 +36,9 @@
#include <linux/of_gpio.h>
#include <linux/of_irq.h>
#include <linux/platform_data/nxp-nci.h>
-#include <linux/unaligned/access_ok.h>
#include <net/nfc/nfc.h>
+#include <asm/unaligned.h>
#include "nxp-nci.h"
--
2.11.0
^ permalink raw reply related
* [PATCH] ath9k: move RELAY and DEBUG_FS to ATH9K[_HTC]_DEBUGFS
From: Christian Lamparter @ 2017-01-09 17:48 UTC (permalink / raw)
To: QCA ath9k Development, linux-wireless, ath9k-devel; +Cc: Kalle Valo
Currently, the common ath9k_common module needs to have a
dependency on RELAY and DEBUG_FS in order to built. This
is usually not a problem. But for RAM and FLASH starved
AR71XX devices, every little bit counts.
This patch adds a new symbol CONFIG_ATH9K_COMMON_DEBUG
which makes it possible to drop the RELAY and DEBUG_FS
dependency there and move it to ATH_(HTC)_DEBUGFS.
Note: The shared FFT/spectral code (which is the only user
of the relayfs in ath9k*) needs DEBUG_FS to export the relayfs
interface to dump the data to userspace. So it makes no sense
to have the functions compiled in, if DEBUG_FS is not there.
Signed-off-by: Christian Lamparter <chunkeey@googlemail.com>
---
Link to the RFC: <https://patchwork.kernel.org/patch/9501361/>.
---
drivers/net/wireless/ath/ath9k/Kconfig | 9 ++++++--
drivers/net/wireless/ath/ath9k/Makefile | 5 +++--
drivers/net/wireless/ath/ath9k/common-debug.h | 27 ++++++++++++++++++++++++
drivers/net/wireless/ath/ath9k/common-spectral.c | 2 +-
drivers/net/wireless/ath/ath9k/common-spectral.h | 23 ++++++++++++++++++++
drivers/net/wireless/ath/ath9k/eeprom_4k.c | 2 +-
drivers/net/wireless/ath/ath9k/eeprom_9287.c | 2 +-
drivers/net/wireless/ath/ath9k/eeprom_def.c | 2 +-
8 files changed, 64 insertions(+), 8 deletions(-)
diff --git a/drivers/net/wireless/ath/ath9k/Kconfig b/drivers/net/wireless/ath/ath9k/Kconfig
index 8f231c67dd51..783a38f1a626 100644
--- a/drivers/net/wireless/ath/ath9k/Kconfig
+++ b/drivers/net/wireless/ath/ath9k/Kconfig
@@ -3,8 +3,8 @@ config ATH9K_HW
config ATH9K_COMMON
tristate
select ATH_COMMON
- select DEBUG_FS
- select RELAY
+config ATH9K_COMMON_DEBUG
+ bool
config ATH9K_DFS_DEBUGFS
def_bool y
depends on ATH9K_DEBUGFS && ATH9K_DFS_CERTIFIED
@@ -60,12 +60,14 @@ config ATH9K_DEBUGFS
bool "Atheros ath9k debugging"
depends on ATH9K && DEBUG_FS
select MAC80211_DEBUGFS
+ select ATH9K_COMMON_DEBUG
select RELAY
---help---
Say Y, if you need access to ath9k's statistics for
interrupts, rate control, etc.
Also required for changing debug message flags at run time.
+ As well as access to the FFT/spectral data and TX99.
config ATH9K_STATION_STATISTICS
bool "Detailed station statistics"
@@ -174,8 +176,11 @@ config ATH9K_HTC
config ATH9K_HTC_DEBUGFS
bool "Atheros ath9k_htc debugging"
depends on ATH9K_HTC && DEBUG_FS
+ select ATH9K_COMMON_DEBUG
+ select RELAY
---help---
Say Y, if you need access to ath9k_htc's statistics.
+ As well as access to the FFT/spectral data.
config ATH9K_HWRNG
bool "Random number generator support"
diff --git a/drivers/net/wireless/ath/ath9k/Makefile b/drivers/net/wireless/ath/ath9k/Makefile
index 76f9dc37500b..36a40ffdce15 100644
--- a/drivers/net/wireless/ath/ath9k/Makefile
+++ b/drivers/net/wireless/ath/ath9k/Makefile
@@ -60,8 +60,9 @@ obj-$(CONFIG_ATH9K_COMMON) += ath9k_common.o
ath9k_common-y:= common.o \
common-init.o \
common-beacon.o \
- common-debug.o \
- common-spectral.o
+
+ath9k_common-$(CONFIG_ATH9K_COMMON_DEBUG) += common-debug.o \
+ common-spectral.o
ath9k_htc-y += htc_hst.o \
hif_usb.o \
diff --git a/drivers/net/wireless/ath/ath9k/common-debug.h b/drivers/net/wireless/ath/ath9k/common-debug.h
index 7c9788490f7f..3376990d3a24 100644
--- a/drivers/net/wireless/ath/ath9k/common-debug.h
+++ b/drivers/net/wireless/ath/ath9k/common-debug.h
@@ -60,6 +60,7 @@ struct ath_rx_stats {
u32 rx_spectral;
};
+#ifdef CONFIG_ATH9K_COMMON_DEBUG
void ath9k_cmn_debug_modal_eeprom(struct dentry *debugfs_phy,
struct ath_hw *ah);
void ath9k_cmn_debug_base_eeprom(struct dentry *debugfs_phy,
@@ -70,3 +71,29 @@ void ath9k_cmn_debug_recv(struct dentry *debugfs_phy,
struct ath_rx_stats *rxstats);
void ath9k_cmn_debug_phy_err(struct dentry *debugfs_phy,
struct ath_rx_stats *rxstats);
+#else
+static inline void ath9k_cmn_debug_modal_eeprom(struct dentry *debugfs_phy,
+ struct ath_hw *ah)
+{
+}
+
+static inline void ath9k_cmn_debug_base_eeprom(struct dentry *debugfs_phy,
+ struct ath_hw *ah)
+{
+}
+
+static inline void ath9k_cmn_debug_stat_rx(struct ath_rx_stats *rxstats,
+ struct ath_rx_status *rs)
+{
+}
+
+static inline void ath9k_cmn_debug_recv(struct dentry *debugfs_phy,
+ struct ath_rx_stats *rxstats)
+{
+}
+
+static inline void ath9k_cmn_debug_phy_err(struct dentry *debugfs_phy,
+ struct ath_rx_stats *rxstats)
+{
+}
+#endif /* CONFIG_ATH9K_COMMON_DEBUG */
diff --git a/drivers/net/wireless/ath/ath9k/common-spectral.c b/drivers/net/wireless/ath/ath9k/common-spectral.c
index eedf86b67cf5..789a3dbe8341 100644
--- a/drivers/net/wireless/ath/ath9k/common-spectral.c
+++ b/drivers/net/wireless/ath/ath9k/common-spectral.c
@@ -1075,7 +1075,7 @@ static struct rchan_callbacks rfs_spec_scan_cb = {
void ath9k_cmn_spectral_deinit_debug(struct ath_spec_scan_priv *spec_priv)
{
- if (IS_ENABLED(CONFIG_ATH9K_DEBUGFS) && spec_priv->rfs_chan_spec_scan) {
+ if (spec_priv->rfs_chan_spec_scan) {
relay_close(spec_priv->rfs_chan_spec_scan);
spec_priv->rfs_chan_spec_scan = NULL;
}
diff --git a/drivers/net/wireless/ath/ath9k/common-spectral.h b/drivers/net/wireless/ath/ath9k/common-spectral.h
index 998743be9c67..5d1a51d83aa6 100644
--- a/drivers/net/wireless/ath/ath9k/common-spectral.h
+++ b/drivers/net/wireless/ath/ath9k/common-spectral.h
@@ -151,6 +151,7 @@ static inline u8 spectral_bitmap_weight(u8 *bins)
return bins[0] & 0x3f;
}
+#ifdef CONFIG_ATH9K_COMMON_DEBUG
void ath9k_cmn_spectral_init_debug(struct ath_spec_scan_priv *spec_priv, struct dentry *debugfs_phy);
void ath9k_cmn_spectral_deinit_debug(struct ath_spec_scan_priv *spec_priv);
@@ -161,5 +162,27 @@ int ath9k_cmn_spectral_scan_config(struct ath_common *common,
enum spectral_mode spectral_mode);
int ath_cmn_process_fft(struct ath_spec_scan_priv *spec_priv, struct ieee80211_hdr *hdr,
struct ath_rx_status *rs, u64 tsf);
+#else
+static inline void ath9k_cmn_spectral_init_debug(struct ath_spec_scan_priv *spec_priv,
+ struct dentry *debugfs_phy)
+{
+}
+
+static inline void ath9k_cmn_spectral_deinit_debug(struct ath_spec_scan_priv *spec_priv)
+{
+}
+
+static inline void ath9k_cmn_spectral_scan_trigger(struct ath_common *common,
+ struct ath_spec_scan_priv *spec_priv)
+{
+}
+
+static inline int ath_cmn_process_fft(struct ath_spec_scan_priv *spec_priv,
+ struct ieee80211_hdr *hdr,
+ struct ath_rx_status *rs, u64 tsf)
+{
+ return 0;
+}
+#endif /* CONFIG_ATH9K_COMMON_DEBUG */
#endif /* SPECTRAL_H */
diff --git a/drivers/net/wireless/ath/ath9k/eeprom_4k.c b/drivers/net/wireless/ath/ath9k/eeprom_4k.c
index 4a01ebe53053..b8c0a08066a0 100644
--- a/drivers/net/wireless/ath/ath9k/eeprom_4k.c
+++ b/drivers/net/wireless/ath/ath9k/eeprom_4k.c
@@ -72,7 +72,7 @@ static bool ath9k_hw_4k_fill_eeprom(struct ath_hw *ah)
return __ath9k_hw_4k_fill_eeprom(ah);
}
-#if defined(CONFIG_ATH9K_DEBUGFS) || defined(CONFIG_ATH9K_HTC_DEBUGFS)
+#ifdef CONFIG_ATH9K_COMMON_DEBUG
static u32 ath9k_dump_4k_modal_eeprom(char *buf, u32 len, u32 size,
struct modal_eep_4k_header *modal_hdr)
{
diff --git a/drivers/net/wireless/ath/ath9k/eeprom_9287.c b/drivers/net/wireless/ath/ath9k/eeprom_9287.c
index 9611f020f7c0..3caa149b1013 100644
--- a/drivers/net/wireless/ath/ath9k/eeprom_9287.c
+++ b/drivers/net/wireless/ath/ath9k/eeprom_9287.c
@@ -75,7 +75,7 @@ static bool ath9k_hw_ar9287_fill_eeprom(struct ath_hw *ah)
return __ath9k_hw_ar9287_fill_eeprom(ah);
}
-#if defined(CONFIG_ATH9K_DEBUGFS) || defined(CONFIG_ATH9K_HTC_DEBUGFS)
+#ifdef CONFIG_ATH9K_COMMON_DEBUG
static u32 ar9287_dump_modal_eeprom(char *buf, u32 len, u32 size,
struct modal_eep_ar9287_header *modal_hdr)
{
diff --git a/drivers/net/wireless/ath/ath9k/eeprom_def.c b/drivers/net/wireless/ath/ath9k/eeprom_def.c
index 7d5223451ce9..56b44fc7a8e6 100644
--- a/drivers/net/wireless/ath/ath9k/eeprom_def.c
+++ b/drivers/net/wireless/ath/ath9k/eeprom_def.c
@@ -131,7 +131,7 @@ static bool ath9k_hw_def_fill_eeprom(struct ath_hw *ah)
return __ath9k_hw_def_fill_eeprom(ah);
}
-#if defined(CONFIG_ATH9K_DEBUGFS) || defined(CONFIG_ATH9K_HTC_DEBUGFS)
+#ifdef CONFIG_ATH9K_COMMON_DEBUG
static u32 ath9k_def_dump_modal_eeprom(char *buf, u32 len, u32 size,
struct modal_eep_header *modal_hdr)
{
--
2.11.0
^ permalink raw reply related
* Re: [PATCH net-next] bridge: multicast to unicast
From: Johannes Berg @ 2017-01-09 15:47 UTC (permalink / raw)
To: michael-dev
Cc: Linus Lüssing, Felix Fietkau, netdev, David S . Miller,
Stephen Hemminger, bridge, linux-kernel, linux-wireless
In-Reply-To: <ebc5050e12c04273fa518e35d7f0b8bc@fami-braun.de>
On Mon, 2017-01-09 at 16:25 +0100, michael-dev wrote:
> Am 09.01.2017 13:15, schrieb Johannes Berg:
> > > That is bridge fdb entries (need to) expire so the bridge might
> > > "forget" a still-connected station not sending but only consuming
> > > broadcast traffic.
> >
> > Ok, that I don't know. Somehow if you address a unicast packet
> > there
> > the bridge has to make a decision - so it really should know?
>
> If the bridge has not learned the unicast destination mac address on
> any port, it will flood the packet on all ports except the port it
> received the packet on.
Ok, so this really needs to be done in mac80211.
I'll send out the pull request soon then.
johannes
^ permalink raw reply
* Re: [PATCH net-next] bridge: multicast to unicast
From: michael-dev @ 2017-01-09 15:25 UTC (permalink / raw)
To: Johannes Berg
Cc: Linus Lüssing, Felix Fietkau, netdev, David S . Miller,
Stephen Hemminger, bridge, linux-kernel, linux-wireless
In-Reply-To: <1483964159.17582.34.camel@sipsolutions.net>
Am 09.01.2017 13:15, schrieb Johannes Berg:
>> That is bridge fdb entries (need to) expire so the bridge might
>> "forget" a still-connected station not sending but only consuming
>> broadcast traffic.
>
> Ok, that I don't know. Somehow if you address a unicast packet there
> the bridge has to make a decision - so it really should know?
If the bridge has not learned the unicast destination mac address on any
port, it will flood the packet on all ports except the port it received
the packet on.
Regards,
M. Braun
^ permalink raw reply
* Re: [RFC] ath9k: move RELAY and DEBUG_FS to ATH9K[_HTC]_DEBUGFS
From: Kalle Valo @ 2017-01-09 14:45 UTC (permalink / raw)
To: Christian Lamparter; +Cc: QCA ath9k Development, linux-wireless, ath9k-devel
In-Reply-To: <20170106154852.22735-1-chunkeey@googlemail.com>
Christian Lamparter <chunkeey@googlemail.com> writes:
> Currently, the common ath9k_common module needs to have a
> dependency on RELAY and DEBUG_FS in order to built. This
> is usually not a problem. But for RAM and FLASH starved
> AR71XX devices, every little bit counts.
>
> This patch adds a new symbol CONFIG_ATH9K_COMMON_DEBUG
> which makes it possible to drop the RELAY and DEBUG_FS
> dependency there and move it to ATH_(HTC)_DEBUGFS.
>
> Note: The shared FFT/spectral code (which is the only user
> of the relayfs in ath9k*) needs DEBUG_FS to export the relayfs
> interface to dump the data to userspace. So it makes no sense
> to have the functions compiled in, if DEBUG_FS is not there.
>
> Signed-off-by: Christian Lamparter <chunkeey@googlemail.com>
> ---
> Here are some numbers for my WD Range Extender (AR7370 with a AR9300):
> For both configurations MAC80211_DEBUGFS and ATH_DEBUG is disabled.
> (if they are enabled, there should be no change). All sizes are in
> bytes. And I only test with or without the patch applied.
>
> module | file size | .text size |
> ath9k_common.ko (w/o) | 32208 | 12832 |
> ath9k_common.ko (with) | 12204 | 3456 |
>
> Note: The kernel with the patch, doesn't need RELAY support anymore.
> Therefore it shrinks a bit as well.
>
> | lzma uimage size | .text size |
> kernel (w/o) | 1181777 | 3004592 |
> kernel (with) | 1179666 | 2999448 |
>
> If anyone wants to play with it, I made a test-patch For LEDE [0].
> Just remember to disable CONFIG_PACKAGE_MAC80211_DEBUGFS and
> CONFIG_PACKAGE_ATH_DEBUG.
>
> There are more ways to do this. Let's hear if there's support for
> it or not. The main motivation was that relayfs can be very costly
> on the RAM as well (on ath10k in can eat like 4MiB with VM
> debugging etc...).
>
> [0] <https://github.com/chunkeey/apm82181-lede/commit/5ef8d4e6497b0b41f0c562650347251e96d73ec8>
No complaints so far so I guess people don't have any issues :) Please
submit this as a proper patch and if there are no comments I'll apply
it.
--
Kalle Valo
^ permalink raw reply
* Re: [PATCH] cfg80211: size various nl80211 messages correctly
From: IgorMitsyanko @ 2017-01-09 14:39 UTC (permalink / raw)
To: Johannes Berg; +Cc: Arend Van Spriel, linux-wireless
In-Reply-To: <d6a54188-8538-87e4-d0db-95ed11102501@quantenna.com>
On 01/09/2017 05:28 PM, IgorMitsyanko wrote:
> On 01/09/2017 03:39 PM, Johannes Berg wrote:
>>>> - msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
>>>> + msg = nlmsg_new(100 + ie_len, GFP_KERNEL);
>>>
>>> Don't you want the '100' to be a define?
>>
>> I didn't want to glorify it too much - some places may need more or
>> less over time. There's no significance to this number.
>>
>> johannes
>>
>
> Looking at what NLMSG_DEFAULT_SIZE is, wouldn't it be more descriptive
> to replace 100 with something like:
>
> #define NLMSG_OVERHEAD (SKB_DATA_ALIGN(sizeof(struct skb_shared_info)) +
> NLMSG_HDRLEN)
>
Now I see, it's not for msg overhead, but for other fixed-width netlink
attributes like WIPHY, IFINDEX etc
^ permalink raw reply
* Re: [PATCH] cfg80211: size various nl80211 messages correctly
From: IgorMitsyanko @ 2017-01-09 14:28 UTC (permalink / raw)
To: Johannes Berg; +Cc: Arend Van Spriel, linux-wireless
In-Reply-To: <1483965571.17582.35.camel@sipsolutions.net>
On 01/09/2017 03:39 PM, Johannes Berg wrote:
>>> - msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
>>> + msg = nlmsg_new(100 + ie_len, GFP_KERNEL);
>>
>> Don't you want the '100' to be a define?
>
> I didn't want to glorify it too much - some places may need more or
> less over time. There's no significance to this number.
>
> johannes
>
Looking at what NLMSG_DEFAULT_SIZE is, wouldn't it be more descriptive
to replace 100 with something like:
#define NLMSG_OVERHEAD (SKB_DATA_ALIGN(sizeof(struct skb_shared_info)) +
NLMSG_HDRLEN)
^ permalink raw reply
* [PATCH] cfg80211: wext does not need to set monitor channel in managed mode
From: Jorge Ramirez-Ortiz @ 2017-01-09 14:25 UTC (permalink / raw)
To: jorge.ramirez-ortiz, johannes
Cc: linux-wireless, daniel.lezcano, daniel.thompson
There is not a valid reason to attempt setting the monitor channel
while in managed mode. Since this code path only deals with this mode,
remove the code block.
Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>
---
net/wireless/wext-sme.c | 23 -----------------------
1 file changed, 23 deletions(-)
diff --git a/net/wireless/wext-sme.c b/net/wireless/wext-sme.c
index 9951638..c434f19 100644
--- a/net/wireless/wext-sme.c
+++ b/net/wireless/wext-sme.c
@@ -105,30 +105,7 @@ int cfg80211_mgd_wext_siwfreq(struct net_device *dev,
goto out;
}
-
wdev->wext.connect.channel = chan;
-
- /*
- * SSID is not set, we just want to switch monitor channel,
- * this is really just backward compatibility, if the SSID
- * is set then we use the channel to select the BSS to use
- * to connect to instead. If we were connected on another
- * channel we disconnected above and reconnect below.
- */
- if (chan && !wdev->wext.connect.ssid_len) {
- struct cfg80211_chan_def chandef = {
- .width = NL80211_CHAN_WIDTH_20_NOHT,
- .center_freq1 = freq,
- };
-
- chandef.chan = ieee80211_get_channel(&rdev->wiphy, freq);
- if (chandef.chan)
- err = cfg80211_set_monitor_channel(rdev, &chandef);
- else
- err = -EINVAL;
- goto out;
- }
-
err = cfg80211_mgd_wext_connect(rdev, wdev);
out:
wdev_unlock(wdev);
--
2.7.4
^ permalink raw reply related
* Re: [PATCH net-next] bridge: multicast to unicast
From: Johannes Berg @ 2017-01-09 12:44 UTC (permalink / raw)
To: Linus Lüssing
Cc: netdev, David S . Miller, Stephen Hemminger, bridge, linux-kernel,
linux-wireless, Felix Fietkau, Michael Braun
In-Reply-To: <20170109124231.GA9086@otheros>
> > A host SHOULD silently discard a datagram that is received via
> > a link-layer broadcast (see Section 2.4) but does not specify
> > an IP multicast or broadcast destination address.
>
> This example is the other way round. It specifies how the IP
> destination should look like in case of link-layer broadcast. Not
> how the link-layer destination should look like in case of a
> multicast/broadcast IP destination.
You stopped reading too early - snipped the context part for you :)
johannes
^ permalink raw reply
* Re: [PATCH net-next] bridge: multicast to unicast
From: Linus Lüssing @ 2017-01-09 12:42 UTC (permalink / raw)
To: Johannes Berg
Cc: netdev, David S . Miller, Stephen Hemminger, bridge, linux-kernel,
linux-wireless, Felix Fietkau, Michael Braun
In-Reply-To: <1483949149.17582.1.camel@sipsolutions.net>
On Mon, Jan 09, 2017 at 09:05:49AM +0100, Johannes Berg wrote:
> On Sat, 2017-01-07 at 16:15 +0100, Linus Lüssing wrote:
>
> > Actually, I do not quite understand that remark in the mac80211
> > multicast-to-unicast patch. IP should not care about the ethernet
> > header?
>
> But it does, for example RFC 1122 states:
>
> When a host sends a datagram to a link-layer broadcast address,
> the IP destination address MUST be a legal IP broadcast or IP
> multicast address.
>
> A host SHOULD silently discard a datagram that is received via
> a link-layer broadcast (see Section 2.4) but does not specify
> an IP multicast or broadcast destination address.
This example is the other way round. It specifies how the IP
destination should look like in case of link-layer broadcast. Not
how the link-layer destination should look like in case of a
multicast/broadcast IP destination.
Any other examples?
^ permalink raw reply
* Re: [PATCH] cfg80211: size various nl80211 messages correctly
From: Johannes Berg @ 2017-01-09 12:39 UTC (permalink / raw)
To: Arend Van Spriel, linux-wireless
In-Reply-To: <ec9fb205-91ca-9591-0b0b-92672d093395@broadcom.com>
> > - msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
> > + msg = nlmsg_new(100 + ie_len, GFP_KERNEL);
>
> Don't you want the '100' to be a define?
I didn't want to glorify it too much - some places may need more or
less over time. There's no significance to this number.
johannes
^ permalink raw reply
* Re: [PATCH] cfg80211: size various nl80211 messages correctly
From: Arend Van Spriel @ 2017-01-09 12:36 UTC (permalink / raw)
To: Johannes Berg, linux-wireless; +Cc: Johannes Berg
In-Reply-To: <20170109101042.23919-1-johannes@sipsolutions.net>
On 9-1-2017 11:10, Johannes Berg wrote:
> From: Johannes Berg <johannes.berg@intel.com>
>
> Ilan reported that sometimes nl80211 messages weren't working if
> the frames being transported got very large, which was really a
> problem for userspace-to-kernel messages, but prompted me to look
> at the code.
>
> Upon review, I found various places where variable-length data is
> transported in an nl80211 message but the message isn't allocated
> taking that into account. This shouldn't cause any problems since
> the frames aren't really that long, apart in one place where two
> (possibly very long frames) might not fit.
>
> Fix all the places (that I found) that get variable length data
> from the driver and put it into a message to take the length of
> the variable data into account. The 100 there is just a safe
> constant for the remaining message overhead (it's usually around
> 50 for most messages.)
>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
> ---
> net/wireless/nl80211.c | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
> index 23692658fe98..f55b251e4b0d 100644
> --- a/net/wireless/nl80211.c
> +++ b/net/wireless/nl80211.c
> @@ -13249,7 +13249,7 @@ void nl80211_send_disconnected(struct cfg80211_registered_device *rdev,
> struct sk_buff *msg;
> void *hdr;
>
> - msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
> + msg = nlmsg_new(100 + ie_len, GFP_KERNEL);
Don't you want the '100' to be a define?
Regards,
Arend
^ permalink raw reply
* Re: [PATCH net-next] bridge: multicast to unicast
From: Johannes Berg @ 2017-01-09 12:15 UTC (permalink / raw)
To: M. Braun, Linus Lüssing
Cc: Felix Fietkau, netdev, David S . Miller, Stephen Hemminger,
bridge, linux-kernel, linux-wireless
In-Reply-To: <6f5ec9f1-800a-2bc4-2f41-9d803343bb22@fami-braun.de>
On Mon, 2017-01-09 at 12:44 +0100, M. Braun wrote:
> Am 09.01.2017 um 09:08 schrieb Johannes Berg:
> > Does it make sense to implement the two in separate layers though?
> >
> > Clearly, this part needs to be implemented in the bridge layer due
> > to
> > the snooping knowledge, but the code is very similar to what
> > mac80211
> > has now.
>
> Does the bridge always know about all stations connected?
>
> That is bridge fdb entries (need to) expire so the bridge might
> "forget" a still-connected station not sending but only consuming
> broadcast traffic.
>
> E.g. there is a television broadcast station here that receives a
> video stream (via wifi, udp packets) and then airs it (dvb-t) but (on
> its own) would not send any data packet on wifi (static ip, etc.).
Ok, that I don't know. Somehow if you address a unicast packet there
the bridge has to make a decision - so it really should know? Would it
query the port somehow to see if the device is behind it, if getting a
packet for a station it forgot about?
> An other reason to implement this in mac80211 initially was that
> mac80211 could encapsulate broacast/multicast ethernet packtes in
> unicast A-MSDU packets in a way, so that the receiver would still see
> process ethernet packets (after conversion) but have unicast wifi
> frames. This cannot be done in bridge easily but one might want to
> add this later to mac80211.
Yes, DMG would have to be done in mac80211, but that's a lot clearer
case too since it requires negotiation functionality etc.
johannes
^ permalink raw reply
* Re: [PATCH] RFC: Universal scan proposal
From: Arend Van Spriel @ 2017-01-09 12:07 UTC (permalink / raw)
To: Johannes Berg, Dmitry Shmidt; +Cc: linux-wireless
In-Reply-To: <1483958921.17582.20.camel@sipsolutions.net>
On 9-1-2017 11:48, Johannes Berg wrote:
> On Thu, 2017-01-05 at 20:59 +0100, Arend Van Spriel wrote:
>>
>> From what Dmitry listed I guess there's really only one.
>
> Ok. I guess I need to go back to that then.
>
>> Early on in the thread Luca gave some other examples of scan
>> extensions so may need to consider notification/dump methods that are
>> extensible. It seems awkward to have a single "initiate" command and
>> a couple of "notification/retrieval" commands. It may not be so bad
>> as long as it is clear which retrieval command goes with a
>> notification.
>
> Well, we might not even need different commands. We need different
> storage internally, but if you request the results for a given scan ID
> then you might get a totally different result format? Though that
> wouldn't lend itself well to query "everything you have" which is also
> useful. But even then, it could be done by passing the appropriate
> "report type" attribute to the dump command - we need that anyway for
> trigger.
True. With "report type" attribute you do not mean the actual
report_type thing, right. Hopefully you mean the parameter attribute
that implicitly relates to a "report type". The risk here is that it
requires careful description of what user-space needs to look for if it
gets a notification. I think having separate notification/retrieval
commands lowers the risk of misinterpretation.
> I think with that discussion we're getting ahead of ourselves though -
> do we really know that we just need the two result types
>
> * full, and
> * partial (for history scan)
>
> and have we even defined the attributes we want in the partial one?
Not sure if we're getting ahead of ourselves. Yes, we have to determine
attributes for each scan "report type", but it is not a prerequisite for
the other topic. I guess to answer the question about the partial
results attributes we need to know what the possible higher-level
use-cases are. Other source of information would be to look what is done
for g-scan in Android "M" or "N", but not sure if that is best approach
as we may not consider all use-cases.
Regards,
Arend
^ permalink raw reply
* Re: [PATCH net-next] bridge: multicast to unicast
From: M. Braun @ 2017-01-09 11:44 UTC (permalink / raw)
To: Johannes Berg, Linus Lüssing
Cc: Felix Fietkau, netdev, David S . Miller, Stephen Hemminger,
bridge, linux-kernel, linux-wireless
In-Reply-To: <1483949336.17582.3.camel@sipsolutions.net>
Am 09.01.2017 um 09:08 schrieb Johannes Berg:
> Does it make sense to implement the two in separate layers though?
>
> Clearly, this part needs to be implemented in the bridge layer due to
> the snooping knowledge, but the code is very similar to what mac80211
> has now.
Does the bridge always know about all stations connected?
That is bridge fdb entries (need to) expire so the bridge might "forget"
a still-connected station not sending but only consuming broadcast traffic.
E.g. there is a television broadcast station here that receives a video
stream (via wifi, udp packets) and then airs it (dvb-t) but (on its own)
would not send any data packet on wifi (static ip, etc.).
An other reason to implement this in mac80211 initially was that
mac80211 could encapsulate broacast/multicast ethernet packtes in
unicast A-MSDU packets in a way, so that the receiver would still see
process ethernet packets (after conversion) but have unicast wifi
frames. This cannot be done in bridge easily but one might want to add
this later to mac80211.
Michael
^ permalink raw reply
* Re: [PATCH] cfg80211: size various nl80211 messages correctly
From: kbuild test robot @ 2017-01-09 11:39 UTC (permalink / raw)
To: Johannes Berg; +Cc: kbuild-all, linux-wireless, Johannes Berg
In-Reply-To: <20170109101042.23919-1-johannes@sipsolutions.net>
[-- Attachment #1: Type: text/plain, Size: 5369 bytes --]
Hi Johannes,
[auto build test ERROR on mac80211-next/master]
[also build test ERROR on v4.10-rc3 next-20170106]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Johannes-Berg/cfg80211-size-various-nl80211-messages-correctly/20170109-185808
base: https://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next.git master
config: x86_64-kexec (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All errors (new ones prefixed by >>):
net/wireless/nl80211.c: In function 'nl80211_michael_mic_failure':
>> net/wireless/nl80211.c:13365:24: error: 'req_ie_len' undeclared (first use in this function)
msg = nlmsg_new(100 + req_ie_len + resp_ie_len, gfp);
^~~~~~~~~~
net/wireless/nl80211.c:13365:24: note: each undeclared identifier is reported only once for each function it appears in
>> net/wireless/nl80211.c:13365:37: error: 'resp_ie_len' undeclared (first use in this function)
msg = nlmsg_new(100 + req_ie_len + resp_ie_len, gfp);
^~~~~~~~~~~
net/wireless/nl80211.c: In function 'nl80211_send_remain_on_chan_event':
>> net/wireless/nl80211.c:13459:24: error: 'len' undeclared (first use in this function)
msg = nlmsg_new(100 + len, gfp);
^~~
net/wireless/nl80211.c: In function 'nl80211_ch_switch_notify':
net/wireless/nl80211.c:14049:24: error: 'req_ie_len' undeclared (first use in this function)
msg = nlmsg_new(100 + req_ie_len + resp_ie_len, gfp);
^~~~~~~~~~
net/wireless/nl80211.c:14049:37: error: 'resp_ie_len' undeclared (first use in this function)
msg = nlmsg_new(100 + req_ie_len + resp_ie_len, gfp);
^~~~~~~~~~~
vim +/req_ie_len +13365 net/wireless/nl80211.c
13359 enum nl80211_key_type key_type, int key_id,
13360 const u8 *tsc, gfp_t gfp)
13361 {
13362 struct sk_buff *msg;
13363 void *hdr;
13364
13365 msg = nlmsg_new(100 + req_ie_len + resp_ie_len, gfp);
13366 if (!msg)
13367 return;
13368
13369 hdr = nl80211hdr_put(msg, 0, 0, 0, NL80211_CMD_MICHAEL_MIC_FAILURE);
13370 if (!hdr) {
13371 nlmsg_free(msg);
13372 return;
13373 }
13374
13375 if (nla_put_u32(msg, NL80211_ATTR_WIPHY, rdev->wiphy_idx) ||
13376 nla_put_u32(msg, NL80211_ATTR_IFINDEX, netdev->ifindex) ||
13377 (addr && nla_put(msg, NL80211_ATTR_MAC, ETH_ALEN, addr)) ||
13378 nla_put_u32(msg, NL80211_ATTR_KEY_TYPE, key_type) ||
13379 (key_id != -1 &&
13380 nla_put_u8(msg, NL80211_ATTR_KEY_IDX, key_id)) ||
13381 (tsc && nla_put(msg, NL80211_ATTR_KEY_SEQ, 6, tsc)))
13382 goto nla_put_failure;
13383
13384 genlmsg_end(msg, hdr);
13385
13386 genlmsg_multicast_netns(&nl80211_fam, wiphy_net(&rdev->wiphy), msg, 0,
13387 NL80211_MCGRP_MLME, gfp);
13388 return;
13389
13390 nla_put_failure:
13391 genlmsg_cancel(msg, hdr);
13392 nlmsg_free(msg);
13393 }
13394
13395 void nl80211_send_beacon_hint_event(struct wiphy *wiphy,
13396 struct ieee80211_channel *channel_before,
13397 struct ieee80211_channel *channel_after)
13398 {
13399 struct sk_buff *msg;
13400 void *hdr;
13401 struct nlattr *nl_freq;
13402
13403 msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC);
13404 if (!msg)
13405 return;
13406
13407 hdr = nl80211hdr_put(msg, 0, 0, 0, NL80211_CMD_REG_BEACON_HINT);
13408 if (!hdr) {
13409 nlmsg_free(msg);
13410 return;
13411 }
13412
13413 /*
13414 * Since we are applying the beacon hint to a wiphy we know its
13415 * wiphy_idx is valid
13416 */
13417 if (nla_put_u32(msg, NL80211_ATTR_WIPHY, get_wiphy_idx(wiphy)))
13418 goto nla_put_failure;
13419
13420 /* Before */
13421 nl_freq = nla_nest_start(msg, NL80211_ATTR_FREQ_BEFORE);
13422 if (!nl_freq)
13423 goto nla_put_failure;
13424 if (nl80211_msg_put_channel(msg, channel_before, false))
13425 goto nla_put_failure;
13426 nla_nest_end(msg, nl_freq);
13427
13428 /* After */
13429 nl_freq = nla_nest_start(msg, NL80211_ATTR_FREQ_AFTER);
13430 if (!nl_freq)
13431 goto nla_put_failure;
13432 if (nl80211_msg_put_channel(msg, channel_after, false))
13433 goto nla_put_failure;
13434 nla_nest_end(msg, nl_freq);
13435
13436 genlmsg_end(msg, hdr);
13437
13438 rcu_read_lock();
13439 genlmsg_multicast_allns(&nl80211_fam, msg, 0,
13440 NL80211_MCGRP_REGULATORY, GFP_ATOMIC);
13441 rcu_read_unlock();
13442
13443 return;
13444
13445 nla_put_failure:
13446 genlmsg_cancel(msg, hdr);
13447 nlmsg_free(msg);
13448 }
13449
13450 static void nl80211_send_remain_on_chan_event(
13451 int cmd, struct cfg80211_registered_device *rdev,
13452 struct wireless_dev *wdev, u64 cookie,
13453 struct ieee80211_channel *chan,
13454 unsigned int duration, gfp_t gfp)
13455 {
13456 struct sk_buff *msg;
13457 void *hdr;
13458
13459 msg = nlmsg_new(100 + len, gfp);
13460 if (!msg)
13461 return;
13462
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 24305 bytes --]
^ permalink raw reply
* Re: [PATCH v5] cfg80211: NL80211_ATTR_SOCKET_OWNER support for CMD_CONNECT
From: Johannes Berg @ 2017-01-09 11:23 UTC (permalink / raw)
To: Andrew Zaborowski, linux-wireless
In-Reply-To: <20170106213343.6598-1-andrew.zaborowski@intel.com>
On Fri, 2017-01-06 at 16:33 -0500, Andrew Zaborowski wrote:
> Disconnect or deauthenticate when the owning socket is closed if this
> flag is supplied to CMD_CONNECT or CMD_ASSOCIATE. This may be used
> to ensure userspace daemon doesn't leave an unmanaged connection
> behind.
>
> In some situations it would be possible to account for that, to some
> degree, in the deamon restart code or in the up/down scripts without
> the use of this attribute. But there will be systems where the
> daemon
> can go away for varying periods without a warning due to local
> resource
> management.
>
Applied. Turns out there's no conflict with the other change after all,
since the context here doesn't include the pieces I changed (and this
time the "else if" is correct, since disconnecting explicitly is
pointless when the interface is about to be destroyed)
johannes
^ permalink raw reply
* Re: [PATCH] RFC: Universal scan proposal
From: Arend Van Spriel @ 2017-01-09 11:19 UTC (permalink / raw)
To: Johannes Berg, Dmitry Shmidt; +Cc: linux-wireless
In-Reply-To: <1483958725.17582.18.camel@sipsolutions.net>
On 9-1-2017 11:45, Johannes Berg wrote:
> On Thu, 2017-01-05 at 12:45 -0800, Dmitry Shmidt wrote:
>>
>>> Oh, then again, maybe you're thinking of full-MAC devices - does a
>>> roam/autojoin scan really already *imply* a new connection? And if
>>> so, do we have to do it that way, or can we remove that type of
>>> action and make a connection decision in higher layers, so it's
>>> really the same as "report when suitable results are found"?
>>
>> We need to consider case when FW may do some actions like connection
>> during roaming/autojoin.
>
> Ok. I was unsure if that was happening. So you're saying that the scan
> parameters are determined by the host, and the scan is triggered from
> there, but the action (like roaming) is taken by the firmware?
>
> How does that differ to
>
> 1) the scan being started by the firmware, possibly based on the BSS
> selection configuration Arend added?
Currently, the BSS selection configuration is used in CMD_CONNECT. It
can probably be used for scanning as well. The presence of the attribute
would indicate the scan may result in a roam or connect event.
> 2) the scan result being reported to the host, and BSS selection done
> there?
Not sure if we need to consider this one, do we?
>> It depends how we want to make it flexible. For example we
>> may allow to FW to report even usual scan results not one by one
>> but as a chunk.
>
> Firmware can do that, but is there any point in doing that in the
> cfg80211 API? If it properly has full results, the driver can always
> unbundle them and call the report function for each BSS and everything
> should work just fine, no?
Agree.
>>> There's a bit more complication wrt. the level of detail in results
>>> though - sometimes the result may include all IEs (normal/sched
>>> scan), sometimes it may not ("history scan") - are we sure we
>>> really only need one new get_scan_results()?
>>
>> Maybe not - it is possible I missed something.
>
> I was hoping you could clarify the requirements :-)
>
>> Also looking at our
>> conversation I think we should consider separate command pair
>> for history scan.
>
> Perhaps, yes. Although perhaps having it triggered through the same
> (new or extended) command, but results reported depending on the
> "report type" would make sense. I think we need to clarify the exact
> requirements before we make that call.
Leaning towards single initiate command and a notification and results
command per "report type".
Regards,
Arend
^ permalink raw reply
* Re: pull-request: mac80211 2017-01-06
From: Johannes Berg @ 2017-01-09 11:17 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-wireless
In-Reply-To: <20170106.162725.481874783893198568.davem@davemloft.net>
On Fri, 2017-01-06 at 16:27 -0500, David Miller wrote:
> From: Johannes Berg <johannes@sipsolutions.net>
> Date: Fri, 6 Jan 2017 13:37:20 +0100
>
> > Here's another fix for something I noticed while reviewing the code
> in
> > a new suggested patch that added another netlink socket destroy
> path.
> >
> > Since the new patch would otherwise cause conflicts, it might be
> good
> > to pull net or Linus's next RC containing it into net-next, if you
> can.
> >
> > Please pull and let me know if there's any problem.
>
> Pulled,
Thanks :)
> I'll try to get this moving into net-next over the weekend.
> Remind me about this early next week if that ends up slipping through
> the cracks.
Actually, I just got the new version of the other patch and it turns
out that it's not necessary since the context for that new bit is small
enough to not have included the difference - so since you haven't done
that yet, no need to bother, sorry I didn't realize that earlier.
johannes
^ permalink raw reply
* Re: drivers/net/wireless/intel/iwlwifi/pcie/trans.c: 2 * suspicious code ?
From: Luca Coelho @ 2017-01-09 11:16 UTC (permalink / raw)
To: David Binderman, johannes.berg@intel.com,
emmanuel.grumbach@intel.com, linuxwifi@intel.com,
kvalo@codeaurora.org, linux-wireless@vger.kernel.org,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <VI1PR08MB1022D85586276BFDE1125B539C630@VI1PR08MB1022.eurprd08.prod.outlook.com>
On Fri, 2017-01-06 at 17:47 +0000, David Binderman wrote:
> Hello there,
Hi David,
> 1.
>
> drivers/net/wireless/intel/iwlwifi/pcie/trans.c:2039:14: warning: decrement of a boolean expression [-Wbool-operation]
>
> Source code is
>
> txq->block--;
>
> Maybe someone got a bool and a int mixed up ?
>
> 2.
>
> drivers/net/wireless/intel/iwlwifi/pcie/trans.c:2045:14: warning: increment of a boolean expression [-Wbool-operation]
>
> Duplicate a few lines further down.
Emmanuel has fixed this in our internal tree and I'll be sending it out
together with our normal upstreaming process.
Thanks for reporting!
--
Cheers,
Luca.
^ permalink raw reply
* Re: [PATCH V6 4/3] brcmfmac: use wiphy_read_of_freq_limits to respect extra limits
From: Johannes Berg @ 2017-01-09 11:07 UTC (permalink / raw)
To: Arend Van Spriel, Rafał Miłecki
Cc: linux-wireless@vger.kernel.org, Martin Blumenstingl,
Felix Fietkau, Arend van Spriel, Arnd Bergmann,
devicetree@vger.kernel.org, Rob Herring, Rafał Miłecki
In-Reply-To: <684d1aff-a9ce-ae42-0c11-5840d3a92daf@broadcom.com>
On Mon, 2017-01-09 at 12:02 +0100, Arend Van Spriel wrote:
> > However, with the OF, I argued (succesfully it seems :P) that the
> > sensible thing to do was to register with the DISABLED flag and
> > thereby
> > "permanently" disable the channels that OF didn't think were
> > usable,
> > but in this case now the driver has to adhere to the cfg80211 logic
> > of
> > preserving orig_flags forever.
>
> By adhere you mean we should not enable channes for which orig_flags
> indicate DISABLED?
Well, the regulatory code will "OR" in all the orig_flags after
modifications. If you don't use any of the others before registering,
and you don't use any other helpers other than the OF one that we know
sets only DISABLED, then keeping only the DISABLED flag would be OK -
however, it should *also* be OK to always keep *all* of the orig_flags,
just like the normal regulatory code would?
johannes
^ permalink raw reply
* Re: SIOCSIWFREQ while in NL80211_IFTYPE_STATION
From: Johannes Berg @ 2017-01-09 11:05 UTC (permalink / raw)
To: Jorge Ramirez, netdev, Daniel Lezcano; +Cc: linux-wireless
In-Reply-To: <f8e8b557-3e7b-cc1a-c31a-91f9d754e683@linaro.org>
On Thu, 2017-01-05 at 15:38 +0100, Jorge Ramirez wrote:
> do you mean this?
>
> [jramirez@igloo ~ (debian-qcom-dragonboard410c-16.09-local $)]$ git
> diff
> diff --git a/net/wireless/wext-sme.c b/net/wireless/wext-sme.c
> index a4e8af3..c56bac5 100644
> --- a/net/wireless/wext-sme.c
> +++ b/net/wireless/wext-sme.c
> @@ -106,30 +106,7 @@ int cfg80211_mgd_wext_siwfreq(struct net_device
> *dev,
> goto out;
> }
>
> -
> wdev->wext.connect.channel = chan;
> -
> - /*
> - * SSID is not set, we just want to switch monitor channel,
> - * this is really just backward compatibility, if the SSID
> - * is set then we use the channel to select the BSS to use
> - * to connect to instead. If we were connected on another
> - * channel we disconnected above and reconnect below.
> - */
> - if (chan && !wdev->wext.connect.ssid_len) {
> - struct cfg80211_chan_def chandef = {
> - .width = NL80211_CHAN_WIDTH_20_NOHT,
> - .center_freq1 = freq,
> - };
> -
> - chandef.chan = ieee80211_get_channel(&rdev->wiphy, freq);
> - if (chandef.chan)
> - err = cfg80211_set_monitor_channel(rdev,
> &chandef);
> - else
> - err = -EINVAL;
> - goto out;
> - }
> -
> err = cfg80211_mgd_wext_connect(rdev, wdev);
> out:
> wdev_unlock(wdev);
Yeah. Frankly, I don't even understand that comment anymore - if the
interface is in managed mode, why set the monitor channel, it's not
monitoring? And if it's not in managed mode we don't get here.
>
>
> I tested the change above: we can now modify the channel/frequency
> when
> the SSID is not set in managed mode.
> When the SSID is set however iwconfig does not report any error but
> channel/frequency doesn't change.
>
> if you think this is acceptable I can submit a patch
I think it looks fine, though writing the commit message may be tricky
:)
johannes
^ permalink raw reply
* Re: [PATCH V6 4/3] brcmfmac: use wiphy_read_of_freq_limits to respect extra limits
From: Arend Van Spriel @ 2017-01-09 11:02 UTC (permalink / raw)
To: Johannes Berg, Rafał Miłecki
Cc: linux-wireless@vger.kernel.org, Martin Blumenstingl,
Felix Fietkau, Arend van Spriel, Arnd Bergmann,
devicetree@vger.kernel.org, Rob Herring, Rafał Miłecki
In-Reply-To: <1483952320.17582.13.camel@sipsolutions.net>
On 9-1-2017 9:58, Johannes Berg wrote:
> On Sat, 2017-01-07 at 13:58 +0100, Rafał Miłecki wrote:
>
>>> I indeed prefer to talk about the driver instead of we. Indeed it
>>> is true due to the orig_flags behavior although that only seems to
>>> involve regulatory code. Could it be that brcmfmac undo that
>>> through the notifier?
>>
>> I guess you could touch orig_flags, but I don't know if it's
>> preferred way. This is probably question to Johannes & cfg80211 guys.
>
> Right now - before the OF patch - there can't really be any orig_flags
> with DISABLED since the driver doesn't set flags to DISABLED before
> registering, does it? While registering, flags are copied to orig_flags
> so the driver can register with flags like DFS or NO_IR already enabled
> - say the firmware requires that - and they will never be overwritten
> by cfg80211.
Actually, in brcmfmac we do set channels to DISABLED before registering.
I was blissfully unaware of the orig_flags when I added the channel
setup in our probe sequence.
> Arguably, what the driver does today - before OF - isn't incorrect
> either, since it simply doesn't care about anything it registered with
> at all.
Given the statement above I think brcmfmac is incorrect.
> However, with the OF, I argued (succesfully it seems :P) that the
> sensible thing to do was to register with the DISABLED flag and thereby
> "permanently" disable the channels that OF didn't think were usable,
> but in this case now the driver has to adhere to the cfg80211 logic of
> preserving orig_flags forever.
By adhere you mean we should not enable channes for which orig_flags
indicate DISABLED?
Regards,
Arend
^ permalink raw reply
* Re: [PATCH] RFC: Universal scan proposal
From: Johannes Berg @ 2017-01-09 10:48 UTC (permalink / raw)
To: Arend Van Spriel, Dmitry Shmidt; +Cc: linux-wireless
In-Reply-To: <ac6c15d8-5d4f-722c-7d7e-c43f6a118b53@broadcom.com>
On Thu, 2017-01-05 at 20:59 +0100, Arend Van Spriel wrote:
>
> From what Dmitry listed I guess there's really only one.
Ok. I guess I need to go back to that then.
> Early on in the thread Luca gave some other examples of scan
> extensions so may need to consider notification/dump methods that are
> extensible. It seems awkward to have a single "initiate" command and
> a couple of "notification/retrieval" commands. It may not be so bad
> as long as it is clear which retrieval command goes with a
> notification.
Well, we might not even need different commands. We need different
storage internally, but if you request the results for a given scan ID
then you might get a totally different result format? Though that
wouldn't lend itself well to query "everything you have" which is also
useful. But even then, it could be done by passing the appropriate
"report type" attribute to the dump command - we need that anyway for
trigger.
I think with that discussion we're getting ahead of ourselves though -
do we really know that we just need the two result types
* full, and
* partial (for history scan)
and have we even defined the attributes we want in the partial one?
johannes
^ permalink raw reply
* Re: [PATCH] RFC: Universal scan proposal
From: Johannes Berg @ 2017-01-09 10:45 UTC (permalink / raw)
To: Dmitry Shmidt; +Cc: Arend Van Spriel, linux-wireless
In-Reply-To: <CAH7ZN-w6KF2ReeU2=a7qafQBknEDqR1KLvv3gmgOY2tf70MM2w@mail.gmail.com>
On Thu, 2017-01-05 at 12:45 -0800, Dmitry Shmidt wrote:
>
> > Oh, then again, maybe you're thinking of full-MAC devices - does a
> > roam/autojoin scan really already *imply* a new connection? And if
> > so, do we have to do it that way, or can we remove that type of
> > action and make a connection decision in higher layers, so it's
> > really the same as "report when suitable results are found"?
>
> We need to consider case when FW may do some actions like connection
> during roaming/autojoin.
Ok. I was unsure if that was happening. So you're saying that the scan
parameters are determined by the host, and the scan is triggered from
there, but the action (like roaming) is taken by the firmware?
How does that differ to
1) the scan being started by the firmware, possibly based on the BSS
selection configuration Arend added?
2) the scan result being reported to the host, and BSS selection done
there?
> It depends how we want to make it flexible. For example we
> may allow to FW to report even usual scan results not one by one
> but as a chunk.
Firmware can do that, but is there any point in doing that in the
cfg80211 API? If it properly has full results, the driver can always
unbundle them and call the report function for each BSS and everything
should work just fine, no?
> > There's a bit more complication wrt. the level of detail in results
> > though - sometimes the result may include all IEs (normal/sched
> > scan), sometimes it may not ("history scan") - are we sure we
> > really only need one new get_scan_results()?
>
> Maybe not - it is possible I missed something.
I was hoping you could clarify the requirements :-)
> Also looking at our
> conversation I think we should consider separate command pair
> for history scan.
Perhaps, yes. Although perhaps having it triggered through the same
(new or extended) command, but results reported depending on the
"report type" would make sense. I think we need to clarify the exact
requirements before we make that call.
johannes
^ 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