* Re: [PATCH V2 1/3] cfg80211: allow passing struct device in the wiphy_new call
From: kbuild test robot @ 2017-01-02 22:10 UTC (permalink / raw)
To: Rafał Miłecki
Cc: kbuild-all, Johannes Berg, linux-wireless, Martin Blumenstingl,
Felix Fietkau, Arend van Spriel, Arnd Bergmann, devicetree,
Rafał Miłecki
In-Reply-To: <20170102132747.3491-1-zajec5@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 5168 bytes --]
Hi Rafał,
[auto build test WARNING on mac80211-next/master]
[also build test WARNING on v4.10-rc2 next-20161224]
[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/Rafa-Mi-ecki/cfg80211-allow-passing-struct-device-in-the-wiphy_new-call/20170103-014525
base: https://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next.git master
reproduce: make htmldocs
All warnings (new ones prefixed by >>):
make[3]: warning: jobserver unavailable: using -j1. Add '+' to parent make rule.
>> include/net/cfg80211.h:3734: warning: No description found for parameter 'dev'
include/net/cfg80211.h:3751: warning: No description found for parameter 'dev'
>> include/net/cfg80211.h:3734: warning: No description found for parameter 'dev'
include/net/cfg80211.h:3751: warning: No description found for parameter 'dev'
>> include/net/cfg80211.h:3734: warning: No description found for parameter 'dev'
include/net/cfg80211.h:3751: warning: No description found for parameter 'dev'
>> include/net/cfg80211.h:3734: warning: No description found for parameter 'dev'
include/net/cfg80211.h:3751: warning: No description found for parameter 'dev'
>> include/net/cfg80211.h:3734: warning: No description found for parameter 'dev'
include/net/cfg80211.h:3751: warning: No description found for parameter 'dev'
>> include/net/cfg80211.h:3734: warning: No description found for parameter 'dev'
include/net/cfg80211.h:3751: warning: No description found for parameter 'dev'
>> include/net/cfg80211.h:3734: warning: No description found for parameter 'dev'
include/net/cfg80211.h:3751: warning: No description found for parameter 'dev'
>> include/net/cfg80211.h:3734: warning: No description found for parameter 'dev'
include/net/cfg80211.h:3751: warning: No description found for parameter 'dev'
>> include/net/cfg80211.h:3734: warning: No description found for parameter 'dev'
include/net/cfg80211.h:3751: warning: No description found for parameter 'dev'
>> include/net/cfg80211.h:3734: warning: No description found for parameter 'dev'
include/net/cfg80211.h:3751: warning: No description found for parameter 'dev'
>> include/net/cfg80211.h:3734: warning: No description found for parameter 'dev'
include/net/cfg80211.h:3751: warning: No description found for parameter 'dev'
>> include/net/cfg80211.h:3734: warning: No description found for parameter 'dev'
include/net/cfg80211.h:3751: warning: No description found for parameter 'dev'
>> include/net/cfg80211.h:3734: warning: No description found for parameter 'dev'
include/net/cfg80211.h:3751: warning: No description found for parameter 'dev'
>> include/net/cfg80211.h:3734: warning: No description found for parameter 'dev'
include/net/cfg80211.h:3751: warning: No description found for parameter 'dev'
>> include/net/cfg80211.h:3734: warning: No description found for parameter 'dev'
include/net/cfg80211.h:3751: warning: No description found for parameter 'dev'
>> include/net/cfg80211.h:3734: warning: No description found for parameter 'dev'
include/net/cfg80211.h:3751: warning: No description found for parameter 'dev'
>> include/net/cfg80211.h:3734: warning: No description found for parameter 'dev'
include/net/cfg80211.h:3751: warning: No description found for parameter 'dev'
>> include/net/cfg80211.h:3734: warning: No description found for parameter 'dev'
include/net/cfg80211.h:3751: warning: No description found for parameter 'dev'
>> include/net/cfg80211.h:3734: warning: No description found for parameter 'dev'
include/net/cfg80211.h:3751: warning: No description found for parameter 'dev'
>> include/net/cfg80211.h:3734: warning: No description found for parameter 'dev'
include/net/cfg80211.h:3751: warning: No description found for parameter 'dev'
vim +/dev +3734 include/net/cfg80211.h
3718
3719 /**
3720 * wiphy_new_nm - create a new wiphy for use with cfg80211
3721 *
3722 * @ops: The configuration operations for this device
3723 * @sizeof_priv: The size of the private area to allocate
3724 * @requested_name: Request a particular name.
3725 * NULL is valid value, and means use the default phy%d naming.
3726 *
3727 * Create a new wiphy and associate the given operations with it.
3728 * @sizeof_priv bytes are allocated for private use.
3729 *
3730 * Return: A pointer to the new wiphy. This pointer must be
3731 * assigned to each netdev's ieee80211_ptr for proper operation.
3732 */
3733 struct wiphy *wiphy_new_nm(struct device *dev, const struct cfg80211_ops *ops,
> 3734 int sizeof_priv, const char *requested_name);
3735
3736 /**
3737 * wiphy_new - create a new wiphy for use with cfg80211
3738 *
3739 * @ops: The configuration operations for this device
3740 * @sizeof_priv: The size of the private area to allocate
3741 *
3742 * Create a new wiphy and associate the given operations with it.
---
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: 6421 bytes --]
^ permalink raw reply
* Re: [1/2] ath10k: add accounting for the extended peer statistics
From: Mohammed Shafi Shajakhan @ 2017-01-03 5:28 UTC (permalink / raw)
To: Christian Lamparter; +Cc: Kalle Valo, linux-wireless, ath10k
In-Reply-To: <CAAd0S9DcCM9XQOihZx1sVf_1=S3VvDye4k1gEMgWEmNWDaBk5Q@mail.gmail.com>
Hi Christian / Kalle,
On Fri, Dec 30, 2016 at 03:35:10PM +0100, Christian Lamparter wrote:
> On Thu, Dec 29, 2016 at 3:11 PM, Kalle Valo <kvalo@qca.qualcomm.com> wrote:
> > Christian Lamparter <chunkeey@googlemail.com> wrote:
> >> The 10.4 firmware adds extended peer information to the
> >> firmware's statistics payload. This additional info is
> >> stored as a separate data field and the elements are
> >> stored in their own "peers_extd" list.
> >>
> >> These elements can pile up in the same way as the peer
> >> information elements. This is because the
> >> ath10k_wmi_10_4_op_pull_fw_stats() function tries to
> >> pull the same amount (num_peer_stats) for every statistic
> >> data unit.
> >>
> >> Fixes: 4a49ae94a448faa ("ath10k: fix 10.4 extended peer stats update")
> >> Signed-off-by: Christian Lamparter <chunkeey@googlemail.com>
> >
> > My understanding is that I should skip this patch 1. Please let me know if
> > I misunderstood. But I'm still plannning to apply patch 2.
>
> I added Mohammed (I hope, he can reply to the open question when he
> returns), Since I'm not sure what he wants or not.
>
> As far as I'm aware, the "extended" boolean serves no purpose
> because it was only used in once place in debugfs_sta which was
> removed in the patch. ( "ath10k_sta_update_stats_rx_duration"
> and "ath10k_sta_update_extd_stats_rx_duration" have been unified).
[shafi] We will wait for Kalle to review from the de-ferred stage
and get his opinion as well(regarding the design change).
I have no concerns as long this does not changes the existing behaviour.
thank you !
regards,
shafi
^ permalink raw reply
* [PATCH resend 1/4] nfc: Add support RC-S380P to port100
From: OGAWA Hirofumi @ 2017-01-03 5:47 UTC (permalink / raw)
To: Samuel Ortiz
Cc: Aloisio Almeida Jr, Lauro Ramos Venancio, linux-wireless,
Andrew Morton, linux-kernel
Signed-off-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
---
drivers/nfc/port100.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff -puN drivers/nfc/port100.c~nfc-add-rcs380p drivers/nfc/port100.c
--- linux/drivers/nfc/port100.c~nfc-add-rcs380p 2016-12-18 22:16:53.503673411 +0900
+++ linux-hirofumi/drivers/nfc/port100.c 2016-12-18 22:16:53.504673416 +0900
@@ -21,8 +21,9 @@
#define VERSION "0.1"
-#define SONY_VENDOR_ID 0x054c
-#define RCS380_PRODUCT_ID 0x06c1
+#define SONY_VENDOR_ID 0x054c
+#define RCS380S_PRODUCT_ID 0x06c1
+#define RCS380P_PRODUCT_ID 0x06c3
#define PORT100_PROTOCOLS (NFC_PROTO_JEWEL_MASK | \
NFC_PROTO_MIFARE_MASK | \
@@ -1477,7 +1478,8 @@ static struct nfc_digital_ops port100_di
};
static const struct usb_device_id port100_table[] = {
- { USB_DEVICE(SONY_VENDOR_ID, RCS380_PRODUCT_ID), },
+ { USB_DEVICE(SONY_VENDOR_ID, RCS380S_PRODUCT_ID), },
+ { USB_DEVICE(SONY_VENDOR_ID, RCS380P_PRODUCT_ID), },
{ }
};
MODULE_DEVICE_TABLE(usb, port100_table);
_
--
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
^ permalink raw reply
* [PATCH resend 2/4] nfc: Send same info for both of NFC_CMD_GET_DEVICE and NFC_EVENT_DEVICE_ADDED
From: OGAWA Hirofumi @ 2017-01-03 5:47 UTC (permalink / raw)
To: Samuel Ortiz
Cc: Aloisio Almeida Jr, Lauro Ramos Venancio, linux-wireless,
Andrew Morton, linux-kernel
In-Reply-To: <87ful0lmop.fsf@mail.parknet.co.jp>
Now, NFC_EVENT_DEVICE_ADDED doesn't send NFC_ATTR_RF_MODE. But
NFC_CMD_GET_DEVICE send.
To get NFC_ATTR_RF_MODE, we have to call NFC_CMD_GET_DEVICE just for
NFC_ATTR_RF_MODE when get NFC_EVENT_DEVICE_ADDED.
This fixes those inconsistent.
Signed-off-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
---
net/nfc/netlink.c | 22 +++++++++++++---------
1 file changed, 13 insertions(+), 9 deletions(-)
diff -puN net/nfc/netlink.c~nfc-send-same-info net/nfc/netlink.c
--- linux/net/nfc/netlink.c~nfc-send-same-info 2016-12-18 22:16:55.805686290 +0900
+++ linux-hirofumi/net/nfc/netlink.c 2016-12-18 22:16:55.806686296 +0900
@@ -311,6 +311,17 @@ free_msg:
return -EMSGSIZE;
}
+static int nfc_genl_setup_device_added(struct nfc_dev *dev, struct sk_buff *msg)
+{
+ if (nla_put_string(msg, NFC_ATTR_DEVICE_NAME, nfc_device_name(dev)) ||
+ nla_put_u32(msg, NFC_ATTR_DEVICE_INDEX, dev->idx) ||
+ nla_put_u32(msg, NFC_ATTR_PROTOCOLS, dev->supported_protocols) ||
+ nla_put_u8(msg, NFC_ATTR_DEVICE_POWERED, dev->dev_up) ||
+ nla_put_u8(msg, NFC_ATTR_RF_MODE, dev->rf_mode))
+ return -1;
+ return 0;
+}
+
int nfc_genl_device_added(struct nfc_dev *dev)
{
struct sk_buff *msg;
@@ -325,10 +336,7 @@ int nfc_genl_device_added(struct nfc_dev
if (!hdr)
goto free_msg;
- if (nla_put_string(msg, NFC_ATTR_DEVICE_NAME, nfc_device_name(dev)) ||
- nla_put_u32(msg, NFC_ATTR_DEVICE_INDEX, dev->idx) ||
- nla_put_u32(msg, NFC_ATTR_PROTOCOLS, dev->supported_protocols) ||
- nla_put_u8(msg, NFC_ATTR_DEVICE_POWERED, dev->dev_up))
+ if (nfc_genl_setup_device_added(dev, msg))
goto nla_put_failure;
genlmsg_end(msg, hdr);
@@ -604,11 +612,7 @@ static int nfc_genl_send_device(struct s
if (cb)
genl_dump_check_consistent(cb, hdr, &nfc_genl_family);
- if (nla_put_string(msg, NFC_ATTR_DEVICE_NAME, nfc_device_name(dev)) ||
- nla_put_u32(msg, NFC_ATTR_DEVICE_INDEX, dev->idx) ||
- nla_put_u32(msg, NFC_ATTR_PROTOCOLS, dev->supported_protocols) ||
- nla_put_u8(msg, NFC_ATTR_DEVICE_POWERED, dev->dev_up) ||
- nla_put_u8(msg, NFC_ATTR_RF_MODE, dev->rf_mode))
+ if (nfc_genl_setup_device_added(dev, msg))
goto nla_put_failure;
genlmsg_end(msg, hdr);
_
--
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
^ permalink raw reply
* [PATCH resend 3/4] nfc: Fix RC-S380* needs zero-length packet
From: OGAWA Hirofumi @ 2017-01-03 5:48 UTC (permalink / raw)
To: Samuel Ortiz
Cc: Aloisio Almeida Jr, Lauro Ramos Venancio, linux-wireless,
Andrew Morton, linux-kernel
In-Reply-To: <87bmvolmnt.fsf@mail.parknet.co.jp>
If sent packet size is wMaxPacketSize boundary, this device doesn't
answer. To fix this, we have to send zero-length packet in usb spec.
Signed-off-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
---
drivers/nfc/port100.c | 1 +
1 file changed, 1 insertion(+)
diff -puN drivers/nfc/port100.c~nfc-need-zero-packet drivers/nfc/port100.c
--- linux-ibmpc/drivers/nfc/port100.c~nfc-need-zero-packet 2016-12-31 18:27:14.814753508 +0900
+++ linux-ibmpc-hirofumi/drivers/nfc/port100.c 2016-12-31 18:32:12.928334607 +0900
@@ -1540,6 +1540,7 @@ static int port100_probe(struct usb_inte
usb_fill_bulk_urb(dev->out_urb, dev->udev,
usb_sndbulkpipe(dev->udev, out_endpoint),
NULL, 0, port100_send_complete, dev);
+ dev->out_urb->transfer_flags = URB_ZERO_PACKET;
dev->skb_headroom = PORT100_FRAME_HEADER_LEN +
PORT100_COMM_RF_HEAD_MAX_LEN;
_
--
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
^ permalink raw reply
* [PATCH 4/4] nfc: Fix hangup of RC-S380* in port100_send_ack()
From: OGAWA Hirofumi @ 2017-01-03 5:48 UTC (permalink / raw)
To: Samuel Ortiz
Cc: Aloisio Almeida Jr, Lauro Ramos Venancio, linux-wireless,
Andrew Morton, linux-kernel
In-Reply-To: <877f6clmn2.fsf_-_@mail.parknet.co.jp>
If port100_send_ack() was called twice or more, it has race to hangup.
port100_send_ack() port100_send_ack()
init_completion()
[...]
dev->cmd_cancel = true
/* this removes previous from completion */
init_completion()
[...]
dev->cmd_cancel = true
wait_for_completion()
/* never be waked up */
wait_for_completion()
Like above race, this code is not assuming port100_send_ack() is
called twice or more.
To fix, this checks dev->cmd_cancel to know if prior cancel is
in-flight or not. And never be remove prior task from completion by
using reinit_completion(), so this guarantees to be waked up properly
soon or later.
Signed-off-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
---
drivers/nfc/port100.c | 37 ++++++++++++++++++++++++-------------
1 file changed, 24 insertions(+), 13 deletions(-)
diff -puN drivers/nfc/port100.c~nfc-port100-hang-fix drivers/nfc/port100.c
--- linux-ibmpc/drivers/nfc/port100.c~nfc-port100-hang-fix 2017-01-03 12:32:13.261785257 +0900
+++ linux-ibmpc-hirofumi/drivers/nfc/port100.c 2017-01-03 12:33:51.965364300 +0900
@@ -726,23 +726,33 @@ static int port100_submit_urb_for_ack(st
static int port100_send_ack(struct port100 *dev)
{
- int rc;
+ int rc = 0;
mutex_lock(&dev->out_urb_lock);
- init_completion(&dev->cmd_cancel_done);
-
- usb_kill_urb(dev->out_urb);
+ /*
+ * If prior cancel is in-flight (dev->cmd_cancel == true), we
+ * can skip to send cancel. Then this will wait the prior
+ * cancel, or merged into the next cancel rarely if next
+ * cancel was started before waiting done. In any case, this
+ * will be waked up soon or later.
+ */
+ if (!dev->cmd_cancel) {
+ reinit_completion(&dev->cmd_cancel_done);
- dev->out_urb->transfer_buffer = ack_frame;
- dev->out_urb->transfer_buffer_length = sizeof(ack_frame);
- rc = usb_submit_urb(dev->out_urb, GFP_KERNEL);
+ usb_kill_urb(dev->out_urb);
- /* Set the cmd_cancel flag only if the URB has been successfully
- * submitted. It will be reset by the out URB completion callback
- * port100_send_complete().
- */
- dev->cmd_cancel = !rc;
+ dev->out_urb->transfer_buffer = ack_frame;
+ dev->out_urb->transfer_buffer_length = sizeof(ack_frame);
+ rc = usb_submit_urb(dev->out_urb, GFP_KERNEL);
+
+ /*
+ * Set the cmd_cancel flag only if the URB has been
+ * successfully submitted. It will be reset by the out
+ * URB completion callback port100_send_complete().
+ */
+ dev->cmd_cancel = !rc;
+ }
mutex_unlock(&dev->out_urb_lock);
@@ -929,8 +939,8 @@ static void port100_send_complete(struct
struct port100 *dev = urb->context;
if (dev->cmd_cancel) {
+ complete_all(&dev->cmd_cancel_done);
dev->cmd_cancel = false;
- complete(&dev->cmd_cancel_done);
}
switch (urb->status) {
@@ -1546,6 +1556,7 @@ static int port100_probe(struct usb_inte
PORT100_COMM_RF_HEAD_MAX_LEN;
dev->skb_tailroom = PORT100_FRAME_TAIL_LEN;
+ init_completion(&dev->cmd_cancel_done);
INIT_WORK(&dev->cmd_complete_work, port100_wq_cmd_complete);
/* The first thing to do with the Port-100 is to set the command type
_
--
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
^ permalink raw reply
* Re: [PATCH V3 2/2] cfg80211: support ieee80211-freq-limit DT property
From: Johannes Berg @ 2017-01-03 7:00 UTC (permalink / raw)
To: Arend van Spriel, Rafał Miłecki, linux-wireless
Cc: Martin Blumenstingl, Felix Fietkau, Arend van Spriel,
Arnd Bergmann, devicetree, Rafał Miłecki
In-Reply-To: <e194b980-1048-8da6-624b-26f824ff655d@broadcom.com>
> I suppose this then can also be done early in the wiphy_register()
> function itself, right?
No, because of the shared channel data issue. If this is
unconditionally in wiphy_register() then we can no longer guarantee
that sharing it is acceptable - which it is today under certain
circumstances the driver controls. This would move it out of driver
control, making it never possible to share any more.
> So does it mean the function can go in core.c again :-p If it is
> likely there will be other properties being added it might justify
> adding a new source file, eg. of.c, and only compile it when
> CONFIG_OF is set. Just a thought.
Yeah, whatever :)
We can figure that out once we have the mechanisms in place.
johannes
^ permalink raw reply
* Re: [PATCH V3 2/2] cfg80211: support ieee80211-freq-limit DT property
From: Johannes Berg @ 2017-01-03 7:06 UTC (permalink / raw)
To: Rafał Miłecki
Cc: linux-wireless@vger.kernel.org, Martin Blumenstingl,
Felix Fietkau, Arend van Spriel, Arnd Bergmann,
devicetree@vger.kernel.org, Rafał Miłecki
In-Reply-To: <CACna6ryrYY5G=Fvmsrq_sD3iX5dWreyz7R8mGEd6ogUck=twnQ@mail.gmail.com>
> When driver uses custom regulatory it registers initial channels at
> init but it can also react to regdom changes using reg_notifier. Is
> that correct?
We can treat regulatory and OF data as entirely independent, I think.
At least that's my suggestion:
* use OF data to populate the original channel list, saying which
channels are valid (or not)
* use regulatory later to further restrict settings of the channels
> So I'm looking at brcmf_cfg80211_reg_notifier which calls
> brcmf_setup_wiphybands which calls brcmf_construct_chaninfo.
> That last one reworks all channels on every call. It first marks all
> existing channels as DISABLED then queries firmware for the list of
> supported channels and updates wiphy channels one by one.
> So if I understand this correctly, every regdom change can result in
> rebuilding channels pretty much from the scratch. That's why I
> believed I need to call wiphy_freq_limits_apply on runtime, not just
> during the init.
>
> Is there some flow in my understanding?
I think maybe there's a problem in my understanding :)
All the regulatory code usually takes into account channel->orig_flags.
If this code also did, then we could have the original DISABLED flag
taken from OF still be valid here.
johannes
johannes
^ permalink raw reply
* [PATCH] brcmfmac: avoid writing channel out of allocated array
From: Rafał Miłecki @ 2017-01-03 8:38 UTC (permalink / raw)
To: Kalle Valo
Cc: Arend van Spriel, Franky Lin, Hante Meuleman,
Pieter-Paul Giesberts, Franky Lin, linux-wireless,
brcm80211-dev-list.pdl, Rafał Miłecki
From: Rafał Miłecki <rafal@milecki.pl>
Our code was assigning number of channels to the index variable by
default. If firmware reported channel we didn't predict this would
result in using that initial index value and writing out of array.
Fix this by detecting unexpected channel and ignoring it.
Fixes: 58de92d2f95e ("brcmfmac: use static superset of channels for wiphy bands")
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
I'm not sure what kind of material it is. It fixes possible memory corruption
(serious thing?) but this bug was there since Apr 2015, so is it worth fixing
in 4.10? Or maybe I should even cc stable?
I don't think any released firmware reports any unexpected channel, so I guess
noone ever hit this problem. I just noticed this possible problem when working
on another feature.
---
.../broadcom/brcm80211/brcmfmac/cfg80211.c | 29 +++++++++++-----------
1 file changed, 14 insertions(+), 15 deletions(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index 13ca3eb..0babfc7 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -5825,7 +5825,6 @@ static int brcmf_construct_chaninfo(struct brcmf_cfg80211_info *cfg,
u32 i, j;
u32 total;
u32 chaninfo;
- u32 index;
pbuf = kzalloc(BRCMF_DCMD_MEDLEN, GFP_KERNEL);
@@ -5873,33 +5872,33 @@ static int brcmf_construct_chaninfo(struct brcmf_cfg80211_info *cfg,
ch.bw == BRCMU_CHAN_BW_80)
continue;
- channel = band->channels;
- index = band->n_channels;
+ channel = NULL;
for (j = 0; j < band->n_channels; j++) {
- if (channel[j].hw_value == ch.control_ch_num) {
- index = j;
+ if (band->channels[j].hw_value == ch.control_ch_num) {
+ channel = &band->channels[j];
break;
}
}
- channel[index].center_freq =
- ieee80211_channel_to_frequency(ch.control_ch_num,
- band->band);
- channel[index].hw_value = ch.control_ch_num;
+ if (!channel) {
+ brcmf_err("Firmware reported unexpected channel %d\n",
+ ch.control_ch_num);
+ continue;
+ }
/* assuming the chanspecs order is HT20,
* HT40 upper, HT40 lower, and VHT80.
*/
if (ch.bw == BRCMU_CHAN_BW_80) {
- channel[index].flags &= ~IEEE80211_CHAN_NO_80MHZ;
+ channel->flags &= ~IEEE80211_CHAN_NO_80MHZ;
} else if (ch.bw == BRCMU_CHAN_BW_40) {
- brcmf_update_bw40_channel_flag(&channel[index], &ch);
+ brcmf_update_bw40_channel_flag(channel, &ch);
} else {
/* enable the channel and disable other bandwidths
* for now as mentioned order assure they are enabled
* for subsequent chanspecs.
*/
- channel[index].flags = IEEE80211_CHAN_NO_HT40 |
- IEEE80211_CHAN_NO_80MHZ;
+ channel->flags = IEEE80211_CHAN_NO_HT40 |
+ IEEE80211_CHAN_NO_80MHZ;
ch.bw = BRCMU_CHAN_BW_20;
cfg->d11inf.encchspec(&ch);
chaninfo = ch.chspec;
@@ -5907,11 +5906,11 @@ static int brcmf_construct_chaninfo(struct brcmf_cfg80211_info *cfg,
&chaninfo);
if (!err) {
if (chaninfo & WL_CHAN_RADAR)
- channel[index].flags |=
+ channel->flags |=
(IEEE80211_CHAN_RADAR |
IEEE80211_CHAN_NO_IR);
if (chaninfo & WL_CHAN_PASSIVE)
- channel[index].flags |=
+ channel->flags |=
IEEE80211_CHAN_NO_IR;
}
}
--
2.10.1
^ permalink raw reply related
* mac80211 / ath9k / Monitor Mode
From: Thomas Graf @ 2017-01-03 9:39 UTC (permalink / raw)
To: linux-wireless
Hello Everyone,
Since I updated my Kernel to the current version 4.9 I don't get my tx
packets dumped in monitor mode. That worked at least with the old 4.1
Kernel.
That problem seems to have to do with IEEE80211_TX_CTL_REQ_TX_STATUS. In
the xmit.c of the ath9k driver only packets with that flag are passed on
to ieee80211_tx_status and otherwise to ieee80211_tx_status_noskb.
So if that flag is not set that packet won't get to the monitor
interface in mac80211.
My solution was to add that flag in ieee80211_xmit:
if (local->monitors)
{
info->flags |= IEEE80211_TX_CTL_REQ_TX_STATUS;
}
I'm not sure if that is the right position or if I'm missing something else.
Thanks,
Thomas
^ permalink raw reply
* Re: [PATCH] brcmfmac: avoid writing channel out of allocated array
From: Arend Van Spriel @ 2017-01-03 11:02 UTC (permalink / raw)
To: Rafał Miłecki, Kalle Valo
Cc: Franky Lin, Hante Meuleman, Pieter-Paul Giesberts, Franky Lin,
linux-wireless, brcm80211-dev-list.pdl, Rafał Miłecki
In-Reply-To: <20170103083858.6981-1-zajec5@gmail.com>
On 3-1-2017 9:38, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
>
> Our code was assigning number of channels to the index variable by
> default. If firmware reported channel we didn't predict this would
> result in using that initial index value and writing out of array.
>
> Fix this by detecting unexpected channel and ignoring it.
>
> Fixes: 58de92d2f95e ("brcmfmac: use static superset of channels for wiphy bands")
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
> I'm not sure what kind of material it is. It fixes possible memory corruption
> (serious thing?) but this bug was there since Apr 2015, so is it worth fixing
> in 4.10? Or maybe I should even cc stable?
> I don't think any released firmware reports any unexpected channel, so I guess
> noone ever hit this problem. I just noticed this possible problem when working
> on another feature.
Looking at the change I was going to ask if you actually hit the issue
you are addressing here. The channels in __wl_2ghz_channels and
__wl_5ghz_channels are complete list of channels for the particular band
so it would mean firmware behaves out-of-spec or firmware api was
changed. For robustness a change is acceptable I guess.
My general policy is to submit fixes to wireless-drivers (and stable)
only if it resolves a critical issue found during testing or a reported
issue.
> ---
> .../broadcom/brcm80211/brcmfmac/cfg80211.c | 29 +++++++++++-----------
> 1 file changed, 14 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> index 13ca3eb..0babfc7 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> @@ -5825,7 +5825,6 @@ static int brcmf_construct_chaninfo(struct brcmf_cfg80211_info *cfg,
> u32 i, j;
> u32 total;
> u32 chaninfo;
> - u32 index;
>
> pbuf = kzalloc(BRCMF_DCMD_MEDLEN, GFP_KERNEL);
>
> @@ -5873,33 +5872,33 @@ static int brcmf_construct_chaninfo(struct brcmf_cfg80211_info *cfg,
> ch.bw == BRCMU_CHAN_BW_80)
> continue;
>
> - channel = band->channels;
> - index = band->n_channels;
> + channel = NULL;
> for (j = 0; j < band->n_channels; j++) {
> - if (channel[j].hw_value == ch.control_ch_num) {
> - index = j;
> + if (band->channels[j].hw_value == ch.control_ch_num) {
> + channel = &band->channels[j];
> break;
> }
> }
You could have kept the index construct and simply check if j ==
band->n_channels here to determine something is wrong.
> - channel[index].center_freq =
> - ieee80211_channel_to_frequency(ch.control_ch_num,
> - band->band);
> - channel[index].hw_value = ch.control_ch_num;
So you are also removing these assignments which indeed seem redundant.
Maybe make note of that in the commit message?
> + if (!channel) {
> + brcmf_err("Firmware reported unexpected channel %d\n",
> + ch.control_ch_num);
> + continue;
> + }
As stated above something is really off when this happens so should we
continue and try to make sense of what firmware provides or simply fail.
Regards,
Arend
^ permalink raw reply
* [PATCH V4 1/2] dt-bindings: document common IEEE 802.11 frequency limit property
From: Rafał Miłecki @ 2017-01-03 11:03 UTC (permalink / raw)
To: Johannes Berg, linux-wireless
Cc: Martin Blumenstingl, Felix Fietkau, Arend van Spriel,
Arnd Bergmann, devicetree, Rafał Miłecki
From: Rafał Miłecki <rafal@milecki.pl>
This new file should be used for properties that apply to all wireless
devices.
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
V2: Switch to a single ieee80211-freq-limit property that allows specifying
*multiple* ranges. This resolves problem with more complex rules as pointed
by Felx.
Make description implementation agnostic as pointed by Arend.
Rename node to wifi as suggested by Martin.
V3: Use more real-life frequencies in the example.
---
.../devicetree/bindings/net/wireless/ieee80211.txt | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/wireless/ieee80211.txt
diff --git a/Documentation/devicetree/bindings/net/wireless/ieee80211.txt b/Documentation/devicetree/bindings/net/wireless/ieee80211.txt
new file mode 100644
index 0000000..0cd1219
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/wireless/ieee80211.txt
@@ -0,0 +1,18 @@
+Common IEEE 802.11 properties
+
+This provides documentation of common properties that are valid for all wireless
+devices.
+
+Optional properties:
+ - ieee80211-freq-limit : list of supported frequency ranges in KHz
+
+Example:
+
+pcie@0,0 {
+ reg = <0x0000 0 0 0 0>;
+ wifi@0,0 {
+ reg = <0x0000 0 0 0 0>;
+ ieee80211-freq-limit = <2402000 2482000>,
+ <5170000 5250000>;
+ };
+};
--
2.10.1
^ permalink raw reply related
* [PATCH V4 2/2] cfg80211: support ieee80211-freq-limit DT property
From: Rafał Miłecki @ 2017-01-03 11:03 UTC (permalink / raw)
To: Johannes Berg, linux-wireless
Cc: Martin Blumenstingl, Felix Fietkau, Arend van Spriel,
Arnd Bergmann, devicetree, Rafał Miłecki
In-Reply-To: <20170103110340.23249-1-zajec5@gmail.com>
From: Rafał Miłecki <rafal@milecki.pl>
This patch adds a helper for reading that new property and applying
limitations or supported channels specified this way.
It may be useful for specifying single band devices or devices that
support only some part of the whole band. It's common that tri-band
routers have separated radios for lower and higher part of 5 GHz band.
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
V2: Put main code in core.c as it isn't strictly part of regulatory - pointed
by Arend.
Update to support ieee80211-freq-limit (new property).
V3: Introduce separated wiphy_read_of_freq_limits function.
Add extra sanity checks for DT data.
Move code back to reg.c as suggested by Johannes.
V4: Move code to of.c
Use one helper called at init time (no runtime hooks)
Modify orig_flags
---
include/net/cfg80211.h | 26 ++++++++++
net/wireless/Makefile | 1 +
net/wireless/of.c | 137 +++++++++++++++++++++++++++++++++++++++++++++++++
net/wireless/reg.c | 4 +-
net/wireless/reg.h | 2 +
5 files changed, 168 insertions(+), 2 deletions(-)
create mode 100644 net/wireless/of.c
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index ca2ac1c..d7723a8 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -311,6 +311,32 @@ struct ieee80211_supported_band {
struct ieee80211_sta_vht_cap vht_cap;
};
+/**
+ * wiphy_read_of_freq_limits - read frequency limits from device tree
+ *
+ * @wiphy: the wireless device to get extra limits for
+ *
+ * Some devices may have extra limitations specified in DT. This may be useful
+ * for chipsets that normally support more bands but are limited due to board
+ * design (e.g. by antennas or extermal power amplifier).
+ *
+ * This function reads info from DT and uses it to *modify* channels (disable
+ * unavailable ones). It's usually a *bad* idea to use it in drivers with
+ * shared channel data as DT limitations are device specific.
+ *
+ * As this function access device node it has to be called after set_wiphy_dev.
+ * It also modifies channels so they have to be set first.
+ */
+#ifdef CONFIG_OF
+int wiphy_read_of_freq_limits(struct wiphy *wiphy);
+#else /* CONFIG_OF */
+static inline int wiphy_read_of_freq_limits(struct wiphy *wiphy)
+{
+ return 0;
+}
+#endif /* !CONFIG_OF */
+
+
/*
* Wireless hardware/device configuration structures and methods
*/
diff --git a/net/wireless/Makefile b/net/wireless/Makefile
index 4c9e39f..95b4c09 100644
--- a/net/wireless/Makefile
+++ b/net/wireless/Makefile
@@ -11,6 +11,7 @@ obj-$(CONFIG_WEXT_PRIV) += wext-priv.o
cfg80211-y += core.o sysfs.o radiotap.o util.o reg.o scan.o nl80211.o
cfg80211-y += mlme.o ibss.o sme.o chan.o ethtool.o mesh.o ap.o trace.o ocb.o
+cfg80211-$(CONFIG_OF) += of.o
cfg80211-$(CONFIG_CFG80211_DEBUGFS) += debugfs.o
cfg80211-$(CONFIG_CFG80211_WEXT) += wext-compat.o wext-sme.o
cfg80211-$(CONFIG_CFG80211_INTERNAL_REGDB) += regdb.o
diff --git a/net/wireless/of.c b/net/wireless/of.c
new file mode 100644
index 0000000..d5791c8
--- /dev/null
+++ b/net/wireless/of.c
@@ -0,0 +1,137 @@
+/*
+ * Copyright (C) 2017 Rafał Miłecki <rafal@milecki.pl>
+ *
+ * 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/of.h>
+#include <net/cfg80211.h>
+#include "reg.h"
+
+static bool wiphy_freq_limits_valid_chan(struct wiphy *wiphy,
+ struct ieee80211_freq_range *freq_limits,
+ unsigned int n_freq_limits,
+ struct ieee80211_channel *chan)
+{
+ u32 bw = MHZ_TO_KHZ(20);
+ int i;
+
+ for (i = 0; i < n_freq_limits; i++) {
+ struct ieee80211_freq_range *limit = &freq_limits[i];
+
+ if (reg_does_bw_fit(limit, MHZ_TO_KHZ(chan->center_freq), bw))
+ return true;
+ }
+
+ return false;
+}
+
+static void wiphy_freq_limits_apply(struct wiphy *wiphy,
+ struct ieee80211_freq_range *freq_limits,
+ unsigned int n_freq_limits)
+{
+ enum nl80211_band band;
+ int i;
+
+ if (WARN_ON(!n_freq_limits))
+ return;
+
+ for (band = 0; band < NUM_NL80211_BANDS; band++) {
+ struct ieee80211_supported_band *sband = wiphy->bands[band];
+
+ if (!sband)
+ continue;
+
+ for (i = 0; i < sband->n_channels; i++) {
+ struct ieee80211_channel *chan = &sband->channels[i];
+
+ if (chan->orig_flags & IEEE80211_CHAN_DISABLED)
+ continue;
+
+ if (!wiphy_freq_limits_valid_chan(wiphy, freq_limits,
+ n_freq_limits,
+ chan)) {
+ pr_debug("Disabling freq %d MHz as it's out of OF limits\n",
+ chan->center_freq);
+ chan->orig_flags |= IEEE80211_CHAN_DISABLED;
+ }
+ }
+ }
+}
+
+int wiphy_read_of_freq_limits(struct wiphy *wiphy)
+{
+ struct device *dev = wiphy_dev(wiphy);
+ struct device_node *np;
+ struct property *prop;
+ struct ieee80211_freq_range *freq_limits;
+ unsigned int n_freq_limits;
+ const __be32 *p;
+ int len, i, err;
+
+ if (!dev)
+ return 0;
+ np = dev_of_node(dev);
+ if (!np)
+ return 0;
+
+ prop = of_find_property(np, "ieee80211-freq-limit", &len);
+ if (!prop)
+ return 0;
+
+ if (!len || len % sizeof(u32) || len / sizeof(u32) % 2) {
+ dev_err(dev, "ieee80211-freq-limit wrong format");
+ return -EPROTO;
+ }
+ n_freq_limits = len / sizeof(u32) / 2;
+
+ freq_limits = kcalloc(n_freq_limits, sizeof(*freq_limits), GFP_KERNEL);
+ if (!freq_limits) {
+ err = -ENOMEM;
+ goto out;
+ }
+
+ p = NULL;
+ for (i = 0; i < n_freq_limits; i++) {
+ struct ieee80211_freq_range *limit = &freq_limits[i];
+
+ p = of_prop_next_u32(prop, p, &limit->start_freq_khz);
+ if (!p) {
+ err = -EINVAL;
+ goto out;
+ }
+
+ p = of_prop_next_u32(prop, p, &limit->end_freq_khz);
+ if (!p) {
+ err = -EINVAL;
+ goto out;
+ }
+
+ if (!limit->start_freq_khz ||
+ !limit->end_freq_khz ||
+ limit->start_freq_khz >= limit->end_freq_khz) {
+ err = -EINVAL;
+ goto out;
+ }
+ }
+
+ wiphy_freq_limits_apply(wiphy, freq_limits, n_freq_limits);
+
+ return 0;
+
+out:
+ dev_err(dev, "Failed to get limits: %d\n", err);
+ kfree(freq_limits);
+ return err;
+}
+EXPORT_SYMBOL(wiphy_read_of_freq_limits);
diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 5dbac37..bda0e9e 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -748,8 +748,8 @@ static bool is_valid_rd(const struct ieee80211_regdomain *rd)
return true;
}
-static bool reg_does_bw_fit(const struct ieee80211_freq_range *freq_range,
- u32 center_freq_khz, u32 bw_khz)
+bool reg_does_bw_fit(const struct ieee80211_freq_range *freq_range,
+ u32 center_freq_khz, u32 bw_khz)
{
u32 start_freq_khz, end_freq_khz;
diff --git a/net/wireless/reg.h b/net/wireless/reg.h
index f6ced31..b8e44ef 100644
--- a/net/wireless/reg.h
+++ b/net/wireless/reg.h
@@ -54,6 +54,8 @@ void regulatory_exit(void);
int set_regdom(const struct ieee80211_regdomain *rd,
enum ieee80211_regd_source regd_src);
+bool reg_does_bw_fit(const struct ieee80211_freq_range *freq_range,
+ u32 center_freq_khz, u32 bw_khz);
unsigned int reg_get_max_bandwidth(const struct ieee80211_regdomain *rd,
const struct ieee80211_reg_rule *rule);
--
2.10.1
^ permalink raw reply related
* [PATCH V4 3/2] brcmfmac: use wiphy_read_of_freq_limits to respect extra limits
From: Rafał Miłecki @ 2017-01-03 11:03 UTC (permalink / raw)
To: Johannes Berg, linux-wireless
Cc: Martin Blumenstingl, Felix Fietkau, Arend van Spriel,
Arnd Bergmann, devicetree, Rafał Miłecki
In-Reply-To: <20170103110340.23249-1-zajec5@gmail.com>
From: Rafał Miłecki <rafal@milecki.pl>
There are some devices (e.g. Netgear R8000 home router) with one chipset
model used for different radios, some of them limited to subbands. NVRAM
entries don't contain any extra info on such limitations and firmware
reports full list of channels to us. We need to store extra limitation
info on DT to support such devices properly.
This patch adds check for channel being disabled with orig_flags which
is how this wiphy helper works.
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
This patch should probably go through wireless-driver-next, I'm sending
it just as a proof of concept. It was succesfully tested on SmartRG
SR400ac with BCM43602.
V4: Respect IEEE80211_CHAN_DISABLED in orig_flags
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index ccae3bb..f95e316 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -5886,6 +5886,9 @@ static int brcmf_construct_chaninfo(struct brcmf_cfg80211_info *cfg,
band->band);
channel[index].hw_value = ch.control_ch_num;
+ if (channel->orig_flags & IEEE80211_CHAN_DISABLED)
+ continue;
+
/* assuming the chanspecs order is HT20,
* HT40 upper, HT40 lower, and VHT80.
*/
@@ -6477,6 +6480,7 @@ static int brcmf_setup_wiphy(struct wiphy *wiphy, struct brcmf_if *ifp)
wiphy->bands[NL80211_BAND_5GHZ] = band;
}
}
+ wiphy_read_of_freq_limits(wiphy);
err = brcmf_setup_wiphybands(wiphy);
return err;
}
--
2.10.1
^ permalink raw reply related
* Re: [PATCH] brcmfmac: avoid writing channel out of allocated array
From: Rafał Miłecki @ 2017-01-03 11:31 UTC (permalink / raw)
To: Arend Van Spriel
Cc: Kalle Valo, Franky Lin, Hante Meuleman, Pieter-Paul Giesberts,
Franky Lin, linux-wireless@vger.kernel.org,
open list:BROADCOM BRCM80211 IEEE802.11n WIRELESS DRIVER,
Rafał Miłecki
In-Reply-To: <f74a07d7-94a6-f9c6-ba12-adec6f708c3a@broadcom.com>
On 3 January 2017 at 12:02, Arend Van Spriel
<arend.vanspriel@broadcom.com> wrote:
> On 3-1-2017 9:38, Rafa=C5=82 Mi=C5=82ecki wrote:
>> From: Rafa=C5=82 Mi=C5=82ecki <rafal@milecki.pl>
>>
>> Our code was assigning number of channels to the index variable by
>> default. If firmware reported channel we didn't predict this would
>> result in using that initial index value and writing out of array.
>>
>> Fix this by detecting unexpected channel and ignoring it.
>>
>> Fixes: 58de92d2f95e ("brcmfmac: use static superset of channels for wiph=
y bands")
>> Signed-off-by: Rafa=C5=82 Mi=C5=82ecki <rafal@milecki.pl>
>> ---
>> I'm not sure what kind of material it is. It fixes possible memory corru=
ption
>> (serious thing?) but this bug was there since Apr 2015, so is it worth f=
ixing
>> in 4.10? Or maybe I should even cc stable?
>> I don't think any released firmware reports any unexpected channel, so I=
guess
>> noone ever hit this problem. I just noticed this possible problem when w=
orking
>> on another feature.
>
> Looking at the change I was going to ask if you actually hit the issue
> you are addressing here. The channels in __wl_2ghz_channels and
> __wl_5ghz_channels are complete list of channels for the particular band
> so it would mean firmware behaves out-of-spec or firmware api was
> changed. For robustness a change is acceptable I guess.
I guess that point of view depends on one's trust to the firmware. I
think it's wrong if a wrong/bugged/hacked firmware can result in
memory corruption in the kernel. Even if it's only about sizeof(struct
ieee80211_channel).
> My general policy is to submit fixes to wireless-drivers (and stable)
> only if it resolves a critical issue found during testing or a reported
> issue.
I'm OK with that.
>> ---
>> .../broadcom/brcm80211/brcmfmac/cfg80211.c | 29 +++++++++++----=
-------
>> 1 file changed, 14 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c=
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
>> index 13ca3eb..0babfc7 100644
>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
>> @@ -5825,7 +5825,6 @@ static int brcmf_construct_chaninfo(struct brcmf_c=
fg80211_info *cfg,
>> u32 i, j;
>> u32 total;
>> u32 chaninfo;
>> - u32 index;
>>
>> pbuf =3D kzalloc(BRCMF_DCMD_MEDLEN, GFP_KERNEL);
>>
>> @@ -5873,33 +5872,33 @@ static int brcmf_construct_chaninfo(struct brcmf=
_cfg80211_info *cfg,
>> ch.bw =3D=3D BRCMU_CHAN_BW_80)
>> continue;
>>
>> - channel =3D band->channels;
>> - index =3D band->n_channels;
>> + channel =3D NULL;
>> for (j =3D 0; j < band->n_channels; j++) {
>> - if (channel[j].hw_value =3D=3D ch.control_ch_num) =
{
>> - index =3D j;
>> + if (band->channels[j].hw_value =3D=3D ch.control_c=
h_num) {
>> + channel =3D &band->channels[j];
>> break;
>> }
>> }
>
> You could have kept the index construct and simply check if j =3D=3D
> band->n_channels here to determine something is wrong.
I wanted to simplify code at the same time. Having channel[index]
repeated 7 times was a hint for me it could be handled better. I
should have made that clear, I'll fix improve this in V2.
>> - channel[index].center_freq =3D
>> - ieee80211_channel_to_frequency(ch.control_ch_num,
>> - band->band);
>> - channel[index].hw_value =3D ch.control_ch_num;
>
> So you are also removing these assignments which indeed seem redundant.
> Maybe make note of that in the commit message?
Good idea.
>> + if (!channel) {
>> + brcmf_err("Firmware reported unexpected channel %d=
\n",
>> + ch.control_ch_num);
>> + continue;
>> + }
>
> As stated above something is really off when this happens so should we
> continue and try to make sense of what firmware provides or simply fail.
Well, I could image something like this happening and not being critical.
The simplest case: Broadcom team releases a new firmware which
supports extra 5 GHz channels (e.g. due to the IEEE standard change).
Why should we refuse to run & support all "old" channel just because of tha=
t?
What do you mean by "make sense of what firmware provides"? Would kind
of solution would you suggest?
--=20
Rafa=C5=82
^ permalink raw reply
* Re: [PATCH net-next] bridge: multicast to unicast
From: Nikolay Aleksandrov @ 2017-01-03 11:58 UTC (permalink / raw)
To: Linus Lüssing, netdev
Cc: David S . Miller, Stephen Hemminger, bridge, linux-kernel,
linux-wireless, Felix Fietkau
In-Reply-To: <20170102193214.31723-1-linus.luessing@c0d3.blue>
On 02/01/17 20:32, Linus Lüssing wrote:
> Implements an optional, per bridge port flag and feature to deliver
> multicast packets to any host on the according port via unicast
> individually. This is done by copying the packet per host and
> changing the multicast destination MAC to a unicast one accordingly.
>
> multicast-to-unicast works on top of the multicast snooping feature of
> the bridge. Which means unicast copies are only delivered to hosts which
> are interested in it and signalized this via IGMP/MLD reports
> previously.
>
> This feature is intended for interface types which have a more reliable
> and/or efficient way to deliver unicast packets than broadcast ones
> (e.g. wifi).
>
> However, it should only be enabled on interfaces where no IGMPv2/MLDv1
> report suppression takes place. This feature is disabled by default.
>
> The initial patch and idea is from Felix Fietkau.
>
> Cc: Felix Fietkau <nbd@nbd.name>
> Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue>
>
> ---
>
Hi Linus,
A few comments below, in general I have 2 concerns: the new mcast fast-path
tests and cache line ref, and adding netlink support for the new flag.
> This feature is used and enabled by default in OpenWRT and LEDE for AP
> interfaces for more than a year now to allow both a more robust multicast
> delivery and multicast at higher rates (e.g. multicast streaming).
>
> In OpenWRT/LEDE the IGMP/MLD report suppression issue is overcome by
> the network daemon enabling AP isolation and by that separating all STAs.
> Delivery of STA-to-STA IP mulitcast is made possible again by
> enabling and utilizing the bridge hairpin mode, which considers the
> incoming port as a potential outgoing port, too.
>
> Hairpin-mode is performed after multicast snooping, therefore leading to
> only deliver reports to STAs running a multicast router.
> ---
> include/linux/if_bridge.h | 1 +
> net/bridge/br_forward.c | 44 +++++++++++++++++++++--
> net/bridge/br_mdb.c | 2 +-
> net/bridge/br_multicast.c | 92 ++++++++++++++++++++++++++++++++++-------------
> net/bridge/br_private.h | 4 ++-
> net/bridge/br_sysfs_if.c | 2 ++
> 6 files changed, 115 insertions(+), 30 deletions(-)
>
> diff --git a/include/linux/if_bridge.h b/include/linux/if_bridge.h
> index c6587c0..f1b0d78 100644
> --- a/include/linux/if_bridge.h
> +++ b/include/linux/if_bridge.h
> @@ -46,6 +46,7 @@ struct br_ip_list {
> #define BR_LEARNING_SYNC BIT(9)
> #define BR_PROXYARP_WIFI BIT(10)
> #define BR_MCAST_FLOOD BIT(11)
> +#define BR_MULTICAST_TO_UCAST BIT(12)
>
> #define BR_DEFAULT_AGEING_TIME (300 * HZ)
>
> diff --git a/net/bridge/br_forward.c b/net/bridge/br_forward.c
> index 7cb41ae..49d742d 100644
> --- a/net/bridge/br_forward.c
> +++ b/net/bridge/br_forward.c
> @@ -174,6 +174,33 @@ static struct net_bridge_port *maybe_deliver(
> return p;
> }
>
> +static struct net_bridge_port *maybe_deliver_addr(
> + struct net_bridge_port *prev, struct net_bridge_port *p,
> + struct sk_buff *skb, const unsigned char *addr,
> + bool local_orig)
> +{
> + struct net_device *dev = BR_INPUT_SKB_CB(skb)->brdev;
> + const unsigned char *src = eth_hdr(skb)->h_source;
> +
> + if (!should_deliver(p, skb))
> + return prev;
> +
> + /* Even with hairpin, no soliloquies - prevent breaking IPv6 DAD */
> + if (skb->dev == p->dev && ether_addr_equal(src, addr))
> + return prev;
> +
> + skb = skb_copy(skb, GFP_ATOMIC);
> + if (!skb) {
> + dev->stats.tx_dropped++;
> + return prev;
> + }
> +
> + memcpy(eth_hdr(skb)->h_dest, addr, ETH_ALEN);
> + __br_forward(p, skb, local_orig);
> +
> + return prev;
> +}
> +
> /* called under rcu_read_lock */
> void br_flood(struct net_bridge *br, struct sk_buff *skb,
> enum br_pkt_type pkt_type, bool local_rcv, bool local_orig)
> @@ -231,6 +258,7 @@ void br_multicast_flood(struct net_bridge_mdb_entry *mdst,
> struct net_bridge_port *prev = NULL;
> struct net_bridge_port_group *p;
> struct hlist_node *rp;
> + const unsigned char *addr;
nit: please arrange these into reverse christmas tree
>
> rp = rcu_dereference(hlist_first_rcu(&br->router_list));
> p = mdst ? rcu_dereference(mdst->ports) : NULL;
> @@ -241,10 +269,20 @@ void br_multicast_flood(struct net_bridge_mdb_entry *mdst,
> rport = rp ? hlist_entry(rp, struct net_bridge_port, rlist) :
> NULL;
>
> - port = (unsigned long)lport > (unsigned long)rport ?
> - lport : rport;
> + if ((unsigned long)lport > (unsigned long)rport) {
> + port = lport;
> + addr = p->unicast ? p->eth_addr : NULL;
> + } else {
> + port = rport;
> + addr = NULL;
> + }
> +
> + if (addr)
> + prev = maybe_deliver_addr(prev, port, skb, addr,
> + local_orig);
> + else
> + prev = maybe_deliver(prev, port, skb, local_orig);
This hunk adds 2 new tests and an additional cache line ref to all mcast forwarding,
regardless if the new (special case) flag is set or not.
Also are you intentionally sending the original skb through the last port ?
>
> - prev = maybe_deliver(prev, port, skb, local_orig);
> if (IS_ERR(prev))
> goto out;
> if (prev == port)
> diff --git a/net/bridge/br_mdb.c b/net/bridge/br_mdb.c
> index 7dbc80d..056e6ac 100644
> --- a/net/bridge/br_mdb.c
> +++ b/net/bridge/br_mdb.c
> @@ -531,7 +531,7 @@ static int br_mdb_add_group(struct net_bridge *br, struct net_bridge_port *port,
> break;
> }
>
> - p = br_multicast_new_port_group(port, group, *pp, state);
> + p = br_multicast_new_port_group(port, group, *pp, state, NULL);
> if (unlikely(!p))
> return -ENOMEM;
> rcu_assign_pointer(*pp, p);
> diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
> index b30e77e..470a2409 100644
> --- a/net/bridge/br_multicast.c
> +++ b/net/bridge/br_multicast.c
> @@ -43,12 +43,14 @@ static void br_multicast_add_router(struct net_bridge *br,
> static void br_ip4_multicast_leave_group(struct net_bridge *br,
> struct net_bridge_port *port,
> __be32 group,
> - __u16 vid);
> + __u16 vid,
> + const unsigned char *src);
> +
> #if IS_ENABLED(CONFIG_IPV6)
> static void br_ip6_multicast_leave_group(struct net_bridge *br,
> struct net_bridge_port *port,
> const struct in6_addr *group,
> - __u16 vid);
> + __u16 vid, const unsigned char *src);
> #endif
> unsigned int br_mdb_rehash_seq;
>
> @@ -711,7 +713,8 @@ struct net_bridge_port_group *br_multicast_new_port_group(
> struct net_bridge_port *port,
> struct br_ip *group,
> struct net_bridge_port_group __rcu *next,
> - unsigned char flags)
> + unsigned char flags,
> + const unsigned char *src)
> {
> struct net_bridge_port_group *p;
>
> @@ -726,12 +729,35 @@ struct net_bridge_port_group *br_multicast_new_port_group(
> hlist_add_head(&p->mglist, &port->mglist);
> setup_timer(&p->timer, br_multicast_port_group_expired,
> (unsigned long)p);
> +
> + if ((port->flags & BR_MULTICAST_TO_UCAST) && src) {
> + memcpy(p->eth_addr, src, ETH_ALEN);
> + p->unicast = true;
> + }
> +
> return p;
> }
>
> +static bool br_port_group_equal(struct net_bridge_port_group *p,
> + struct net_bridge_port *port,
> + const unsigned char *src)
> +{
> + if (p->port != port)
> + return false;
> +
> + if (!p->unicast)
> + return true;
> +
> + if (!src)
> + return false;
> +
> + return ether_addr_equal(src, p->eth_addr);
> +}
> +
> static int br_multicast_add_group(struct net_bridge *br,
> struct net_bridge_port *port,
> - struct br_ip *group)
> + struct br_ip *group,
> + const unsigned char *src)
> {
> struct net_bridge_port_group __rcu **pp;
> struct net_bridge_port_group *p;
> @@ -758,13 +784,13 @@ static int br_multicast_add_group(struct net_bridge *br,
> for (pp = &mp->ports;
> (p = mlock_dereference(*pp, br)) != NULL;
> pp = &p->next) {
> - if (p->port == port)
> + if (br_port_group_equal(p, port, src))
> goto found;
> if ((unsigned long)p->port < (unsigned long)port)
> break;
> }
>
> - p = br_multicast_new_port_group(port, group, *pp, 0);
> + p = br_multicast_new_port_group(port, group, *pp, 0, src);
> if (unlikely(!p))
> goto err;
> rcu_assign_pointer(*pp, p);
> @@ -783,7 +809,8 @@ static int br_multicast_add_group(struct net_bridge *br,
> static int br_ip4_multicast_add_group(struct net_bridge *br,
> struct net_bridge_port *port,
> __be32 group,
> - __u16 vid)
> + __u16 vid,
> + const unsigned char *src)
> {
> struct br_ip br_group;
>
> @@ -794,14 +821,15 @@ static int br_ip4_multicast_add_group(struct net_bridge *br,
> br_group.proto = htons(ETH_P_IP);
> br_group.vid = vid;
>
> - return br_multicast_add_group(br, port, &br_group);
> + return br_multicast_add_group(br, port, &br_group, src);
> }
>
> #if IS_ENABLED(CONFIG_IPV6)
> static int br_ip6_multicast_add_group(struct net_bridge *br,
> struct net_bridge_port *port,
> const struct in6_addr *group,
> - __u16 vid)
> + __u16 vid,
> + const unsigned char *src)
> {
> struct br_ip br_group;
>
> @@ -812,7 +840,7 @@ static int br_ip6_multicast_add_group(struct net_bridge *br,
> br_group.proto = htons(ETH_P_IPV6);
> br_group.vid = vid;
>
> - return br_multicast_add_group(br, port, &br_group);
> + return br_multicast_add_group(br, port, &br_group, src);
> }
> #endif
>
> @@ -1081,6 +1109,7 @@ static int br_ip4_multicast_igmp3_report(struct net_bridge *br,
> struct sk_buff *skb,
> u16 vid)
> {
> + const unsigned char *src;
> struct igmpv3_report *ih;
> struct igmpv3_grec *grec;
> int i;
> @@ -1121,12 +1150,14 @@ static int br_ip4_multicast_igmp3_report(struct net_bridge *br,
> continue;
> }
>
> + src = eth_hdr(skb)->h_source;
> if ((type == IGMPV3_CHANGE_TO_INCLUDE ||
> type == IGMPV3_MODE_IS_INCLUDE) &&
> ntohs(grec->grec_nsrcs) == 0) {
> - br_ip4_multicast_leave_group(br, port, group, vid);
> + br_ip4_multicast_leave_group(br, port, group, vid, src);
> } else {
> - err = br_ip4_multicast_add_group(br, port, group, vid);
> + err = br_ip4_multicast_add_group(br, port, group, vid,
> + src);
> if (err)
> break;
> }
> @@ -1141,6 +1172,7 @@ static int br_ip6_multicast_mld2_report(struct net_bridge *br,
> struct sk_buff *skb,
> u16 vid)
> {
> + const unsigned char *src = eth_hdr(skb)->h_source;
> struct icmp6hdr *icmp6h;
> struct mld2_grec *grec;
> int i;
> @@ -1192,10 +1224,11 @@ static int br_ip6_multicast_mld2_report(struct net_bridge *br,
> grec->grec_type == MLD2_MODE_IS_INCLUDE) &&
> ntohs(*nsrcs) == 0) {
> br_ip6_multicast_leave_group(br, port, &grec->grec_mca,
> - vid);
> + vid, src);
> } else {
> err = br_ip6_multicast_add_group(br, port,
> - &grec->grec_mca, vid);
> + &grec->grec_mca, vid,
> + src);
> if (err)
> break;
> }
> @@ -1511,7 +1544,8 @@ br_multicast_leave_group(struct net_bridge *br,
> struct net_bridge_port *port,
> struct br_ip *group,
> struct bridge_mcast_other_query *other_query,
> - struct bridge_mcast_own_query *own_query)
> + struct bridge_mcast_own_query *own_query,
> + const unsigned char *src)
> {
> struct net_bridge_mdb_htable *mdb;
> struct net_bridge_mdb_entry *mp;
> @@ -1535,7 +1569,7 @@ br_multicast_leave_group(struct net_bridge *br,
> for (pp = &mp->ports;
> (p = mlock_dereference(*pp, br)) != NULL;
> pp = &p->next) {
> - if (p->port != port)
> + if (!br_port_group_equal(p, port, src))
> continue;
>
> rcu_assign_pointer(*pp, p->next);
> @@ -1566,7 +1600,7 @@ br_multicast_leave_group(struct net_bridge *br,
> for (p = mlock_dereference(mp->ports, br);
> p != NULL;
> p = mlock_dereference(p->next, br)) {
> - if (p->port != port)
> + if (!br_port_group_equal(p, port, src))
> continue;
>
> if (!hlist_unhashed(&p->mglist) &&
> @@ -1617,7 +1651,8 @@ br_multicast_leave_group(struct net_bridge *br,
> static void br_ip4_multicast_leave_group(struct net_bridge *br,
> struct net_bridge_port *port,
> __be32 group,
> - __u16 vid)
> + __u16 vid,
> + const unsigned char *src)
> {
> struct br_ip br_group;
> struct bridge_mcast_own_query *own_query;
> @@ -1632,14 +1667,15 @@ static void br_ip4_multicast_leave_group(struct net_bridge *br,
> br_group.vid = vid;
>
> br_multicast_leave_group(br, port, &br_group, &br->ip4_other_query,
> - own_query);
> + own_query, src);
> }
>
> #if IS_ENABLED(CONFIG_IPV6)
> static void br_ip6_multicast_leave_group(struct net_bridge *br,
> struct net_bridge_port *port,
> const struct in6_addr *group,
> - __u16 vid)
> + __u16 vid,
> + const unsigned char *src)
> {
> struct br_ip br_group;
> struct bridge_mcast_own_query *own_query;
> @@ -1654,7 +1690,7 @@ static void br_ip6_multicast_leave_group(struct net_bridge *br,
> br_group.vid = vid;
>
> br_multicast_leave_group(br, port, &br_group, &br->ip6_other_query,
> - own_query);
> + own_query, src);
> }
> #endif
>
> @@ -1711,6 +1747,7 @@ static int br_multicast_ipv4_rcv(struct net_bridge *br,
> struct sk_buff *skb,
> u16 vid)
> {
> + const unsigned char *src;
nit: please arrange these in reverse christmas tree
> struct sk_buff *skb_trimmed = NULL;
> struct igmphdr *ih;
> int err;
> @@ -1731,13 +1768,14 @@ static int br_multicast_ipv4_rcv(struct net_bridge *br,
> }
>
> ih = igmp_hdr(skb);
> + src = eth_hdr(skb)->h_source;
> BR_INPUT_SKB_CB(skb)->igmp = ih->type;
>
> switch (ih->type) {
> case IGMP_HOST_MEMBERSHIP_REPORT:
> case IGMPV2_HOST_MEMBERSHIP_REPORT:
> BR_INPUT_SKB_CB(skb)->mrouters_only = 1;
> - err = br_ip4_multicast_add_group(br, port, ih->group, vid);
> + err = br_ip4_multicast_add_group(br, port, ih->group, vid, src);
> break;
> case IGMPV3_HOST_MEMBERSHIP_REPORT:
> err = br_ip4_multicast_igmp3_report(br, port, skb_trimmed, vid);
> @@ -1746,7 +1784,7 @@ static int br_multicast_ipv4_rcv(struct net_bridge *br,
> err = br_ip4_multicast_query(br, port, skb_trimmed, vid);
> break;
> case IGMP_HOST_LEAVE_MESSAGE:
> - br_ip4_multicast_leave_group(br, port, ih->group, vid);
> + br_ip4_multicast_leave_group(br, port, ih->group, vid, src);
> break;
> }
>
> @@ -1765,6 +1803,7 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br,
> struct sk_buff *skb,
> u16 vid)
> {
> + const unsigned char *src;
nit: same about arrangement
> struct sk_buff *skb_trimmed = NULL;
> struct mld_msg *mld;
> int err;
> @@ -1785,8 +1824,10 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br,
>
> switch (mld->mld_type) {
> case ICMPV6_MGM_REPORT:
> + src = eth_hdr(skb)->h_source;
> BR_INPUT_SKB_CB(skb)->mrouters_only = 1;
> - err = br_ip6_multicast_add_group(br, port, &mld->mld_mca, vid);
> + err = br_ip6_multicast_add_group(br, port, &mld->mld_mca, vid,
> + src);
> break;
> case ICMPV6_MLD2_REPORT:
> err = br_ip6_multicast_mld2_report(br, port, skb_trimmed, vid);
> @@ -1795,7 +1836,8 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br,
> err = br_ip6_multicast_query(br, port, skb_trimmed, vid);
> break;
> case ICMPV6_MGM_REDUCTION:
> - br_ip6_multicast_leave_group(br, port, &mld->mld_mca, vid);
> + src = eth_hdr(skb)->h_source;
> + br_ip6_multicast_leave_group(br, port, &mld->mld_mca, vid, src);
> break;
> }
>
> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> index 8ce621e..cc55100 100644
> --- a/net/bridge/br_private.h
> +++ b/net/bridge/br_private.h
> @@ -177,6 +177,8 @@ struct net_bridge_port_group {
> struct timer_list timer;
> struct br_ip addr;
> unsigned char flags;
> + unsigned char eth_addr[ETH_ALEN];
> + bool unicast;
I think you can remove the boolean unicast here and either use the "flags" or
the eth_addr itself.
This structure needs a serious re-arrangement.
> };
>
> struct net_bridge_mdb_entry
> @@ -599,7 +601,7 @@ void br_multicast_free_pg(struct rcu_head *head);
> struct net_bridge_port_group *
> br_multicast_new_port_group(struct net_bridge_port *port, struct br_ip *group,
> struct net_bridge_port_group __rcu *next,
> - unsigned char flags);
> + unsigned char flags, const unsigned char *src);
> void br_mdb_init(void);
> void br_mdb_uninit(void);
> void br_mdb_notify(struct net_device *dev, struct net_bridge_port *port,
> diff --git a/net/bridge/br_sysfs_if.c b/net/bridge/br_sysfs_if.c
> index 8bd5696..1730278 100644
> --- a/net/bridge/br_sysfs_if.c
> +++ b/net/bridge/br_sysfs_if.c
> @@ -188,6 +188,7 @@ static BRPORT_ATTR(multicast_router, S_IRUGO | S_IWUSR, show_multicast_router,
> store_multicast_router);
>
> BRPORT_ATTR_FLAG(multicast_fast_leave, BR_MULTICAST_FAST_LEAVE);
> +BRPORT_ATTR_FLAG(multicast_to_unicast, BR_MULTICAST_TO_UCAST);
> #endif
>
> static const struct brport_attribute *brport_attrs[] = {
> @@ -214,6 +215,7 @@ static const struct brport_attribute *brport_attrs[] = {
> #ifdef CONFIG_BRIDGE_IGMP_SNOOPING
> &brport_attr_multicast_router,
> &brport_attr_multicast_fast_leave,
> + &brport_attr_multicast_to_unicast,
> #endif
> &brport_attr_proxyarp,
> &brport_attr_proxyarp_wifi,
>
Please also add netlink support, we've been working hard at adding support for all bridge
options via netlink.
Thanks,
Nik
^ permalink raw reply
* Re: [PATCH V4 2/2] cfg80211: support ieee80211-freq-limit DT property
From: Arend Van Spriel @ 2017-01-03 12:10 UTC (permalink / raw)
To: Rafał Miłecki, Johannes Berg, linux-wireless
Cc: Martin Blumenstingl, Felix Fietkau, Arend van Spriel,
Arnd Bergmann, devicetree, Rafał Miłecki
In-Reply-To: <20170103110340.23249-2-zajec5@gmail.com>
On 3-1-2017 12:03, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
>
> This patch adds a helper for reading that new property and applying
> limitations or supported channels specified this way.
> It may be useful for specifying single band devices or devices that
> support only some part of the whole band. It's common that tri-band
> routers have separated radios for lower and higher part of 5 GHz band.
>
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
> V2: Put main code in core.c as it isn't strictly part of regulatory - pointed
> by Arend.
> Update to support ieee80211-freq-limit (new property).
> V3: Introduce separated wiphy_read_of_freq_limits function.
> Add extra sanity checks for DT data.
> Move code back to reg.c as suggested by Johannes.
> V4: Move code to of.c
> Use one helper called at init time (no runtime hooks)
> Modify orig_flags
> ---
> include/net/cfg80211.h | 26 ++++++++++
> net/wireless/Makefile | 1 +
> net/wireless/of.c | 137 +++++++++++++++++++++++++++++++++++++++++++++++++
> net/wireless/reg.c | 4 +-
> net/wireless/reg.h | 2 +
> 5 files changed, 168 insertions(+), 2 deletions(-)
> create mode 100644 net/wireless/of.c
>
> diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
> index ca2ac1c..d7723a8 100644
> --- a/include/net/cfg80211.h
> +++ b/include/net/cfg80211.h
> @@ -311,6 +311,32 @@ struct ieee80211_supported_band {
> struct ieee80211_sta_vht_cap vht_cap;
> };
>
> +/**
> + * wiphy_read_of_freq_limits - read frequency limits from device tree
> + *
> + * @wiphy: the wireless device to get extra limits for
> + *
> + * Some devices may have extra limitations specified in DT. This may be useful
> + * for chipsets that normally support more bands but are limited due to board
> + * design (e.g. by antennas or extermal power amplifier).
> + *
> + * This function reads info from DT and uses it to *modify* channels (disable
> + * unavailable ones). It's usually a *bad* idea to use it in drivers with
> + * shared channel data as DT limitations are device specific.
> + *
> + * As this function access device node it has to be called after set_wiphy_dev.
You are aware that you need to modify this description with earlier
patch "cfg80211: allow passing struct device in the wiphy_new call",
right? :-p
> + * It also modifies channels so they have to be set first.
> + */
> +#ifdef CONFIG_OF
> +int wiphy_read_of_freq_limits(struct wiphy *wiphy);
> +#else /* CONFIG_OF */
> +static inline int wiphy_read_of_freq_limits(struct wiphy *wiphy)
> +{
> + return 0;
> +}
> +#endif /* !CONFIG_OF */
> +
> +
[...]
> diff --git a/net/wireless/reg.c b/net/wireless/reg.c
> index 5dbac37..bda0e9e 100644
> --- a/net/wireless/reg.c
> +++ b/net/wireless/reg.c
> @@ -748,8 +748,8 @@ static bool is_valid_rd(const struct ieee80211_regdomain *rd)
> return true;
> }
>
> -static bool reg_does_bw_fit(const struct ieee80211_freq_range *freq_range,
> - u32 center_freq_khz, u32 bw_khz)
> +bool reg_does_bw_fit(const struct ieee80211_freq_range *freq_range,
> + u32 center_freq_khz, u32 bw_khz)
> {
> u32 start_freq_khz, end_freq_khz;
would it be more appropriate to move this function to util.c?
Regards,
Arend
^ permalink raw reply
* Re: [RFC] nl80211: allow multiple active scheduled scan requests
From: Arend Van Spriel @ 2017-01-03 12:25 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless, Dmitry Shmidt
In-Reply-To: <1483353841.4596.2.camel@sipsolutions.net>
On 2-1-2017 11:44, Johannes Berg wrote:
>
>> + /*
>> + * allow only one legacy scheduled scan if user-space
>> + * does not indicate multiple scheduled scan support.
>> + */
>> + if (!info->attrs[NL80211_ATTR_SCHED_SCAN_MULTI] &&
>> + cfg80211_legacy_sched_scan_active(rdev))
>> return -EINPROGRESS;
>
> That probably doesn't go far enough - if legacy one is active then we
> probably shouldn't allow a new MULTI one either (or abandon the legacy
> one) so that older userspace doesn't get confused with multiple
> notifications from sched scans it didn't start.
I considered that although not taking the notifications into account.
Will change it. Abandoning the legacy one would be a behavioral change
so probably not acceptable, right?
>> + if (rdev->sched_scan_req_count == rdev->wiphy.max_sched_scan_reqs)
>> + return -ENOSPC;
>
> Do we really want to do the double-accounting, just to avoid counting
> the list length here?
Ok. I have no strong preference.
>> + /* leave request id zero for legacy request */
>
> why? The ID would be ignored, so why special-case it?
It makes the function cfg80211_legacy_sched_scan_active() easier, ie.
not needing a is_legacy flag in struct cfg80211_sched_scan_request.
>> +static void cfg80211_del_sched_scan_req(struct
>> cfg80211_registered_device *rdev,
>> + struct
>> cfg80211_sched_scan_request *req)
>> +{
>> + list_del_rcu(&req->list);
>> + kfree_rcu(req, rcu_head);
>> + synchronize_rcu();
>> + rdev->sched_scan_req_count--;
>> +}
>
> That's bogus - either you use kfree_rcu() or synchronize_rcu() (the
> former is much better); combining both makes no sense.
Thanks. Both functions mentioned the rcu grace period so I was doubtful.
Will change it.
>> +bool cfg80211_legacy_sched_scan_active(struct
>> cfg80211_registered_device *rdev)
>> +{
>> + struct cfg80211_sched_scan_request *req;
>> +
>> + req = list_first_or_null_rcu(&rdev->sched_scan_req_list,
>> + struct
>> cfg80211_sched_scan_request, list);
>> + /* request id 0 indicates legacy request in progress */
>> + return req && !req->reqid;
>> +}
>
> Ok, fair enough.
I guess your remark means this clarifies your earlier question about the
request id, right?
Regards,
Arend
^ permalink raw reply
* Re: [PATCH net-next] bridge: multicast to unicast
From: Felix Fietkau @ 2017-01-03 13:15 UTC (permalink / raw)
To: Linus Lüssing, netdev
Cc: David S . Miller, Stephen Hemminger, bridge, linux-kernel,
linux-wireless
In-Reply-To: <20170102193214.31723-1-linus.luessing@c0d3.blue>
On 2017-01-02 20:32, Linus Lüssing wrote:
> Implements an optional, per bridge port flag and feature to deliver
> multicast packets to any host on the according port via unicast
> individually. This is done by copying the packet per host and
> changing the multicast destination MAC to a unicast one accordingly.
>
> multicast-to-unicast works on top of the multicast snooping feature of
> the bridge. Which means unicast copies are only delivered to hosts which
> are interested in it and signalized this via IGMP/MLD reports
> previously.
>
> This feature is intended for interface types which have a more reliable
> and/or efficient way to deliver unicast packets than broadcast ones
> (e.g. wifi).
>
> However, it should only be enabled on interfaces where no IGMPv2/MLDv1
> report suppression takes place. This feature is disabled by default.
>
> The initial patch and idea is from Felix Fietkau.
>
> Cc: Felix Fietkau <nbd@nbd.name>
> Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue>
Please add Signed-off-by: Felix Fietkau <nbd@nbd.name>
in the next version, and maybe also From:
Thanks,
- Felix
^ permalink raw reply
* Re: [PATCH] brcmfmac: avoid writing channel out of allocated array
From: Arend Van Spriel @ 2017-01-03 13:19 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Kalle Valo, Franky Lin, Hante Meuleman, Pieter-Paul Giesberts,
Franky Lin, linux-wireless@vger.kernel.org,
open list:BROADCOM BRCM80211 IEEE802.11n WIRELESS DRIVER,
Rafał Miłecki
In-Reply-To: <CACna6rzGRHLCdzxS_+MHXeZwF=BW4S52pDb9g9MuanNGyToEOw@mail.gmail.com>
On 3-1-2017 12:31, Rafał Miłecki wrote:
>>> + if (!channel) {
>>> + brcmf_err("Firmware reported unexpected channel %d\n",
>>> + ch.control_ch_num);
>>> + continue;
>>> + }
>> As stated above something is really off when this happens so should we
>> continue and try to make sense of what firmware provides or simply fail.
> Well, I could image something like this happening and not being critical.
> The simplest case: Broadcom team releases a new firmware which
> supports extra 5 GHz channels (e.g. due to the IEEE standard change).
> Why should we refuse to run & support all "old" channel just because of that?
Fair enough. I was assuming we keep __wl_{2,5}ghz_channels up to date
with IEEE standard.
> What do you mean by "make sense of what firmware provides"? Would kind
> of solution would you suggest?
When the above assumption can be assured (by us) the only other scenario
would be a change in the firmware API where we wrongly interpret the
information retrieved. In this case all subsequent channels will likely
result in bogus or accidental matches hence it seems better to bail out
early.
Regards,
Arend
^ permalink raw reply
* Re: [PATCH V4 3/2] brcmfmac: use wiphy_read_of_freq_limits to respect extra limits
From: Arend Van Spriel @ 2017-01-03 13:29 UTC (permalink / raw)
To: Rafał Miłecki, Johannes Berg, linux-wireless
Cc: Martin Blumenstingl, Felix Fietkau, Arend van Spriel,
Arnd Bergmann, devicetree, Rafał Miłecki
In-Reply-To: <20170103110340.23249-3-zajec5@gmail.com>
What is with the patch numbering, ie. 3/2?
On 3-1-2017 12:03, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
>
> There are some devices (e.g. Netgear R8000 home router) with one chipset
> model used for different radios, some of them limited to subbands. NVRAM
> entries don't contain any extra info on such limitations and firmware
> reports full list of channels to us. We need to store extra limitation
> info on DT to support such devices properly.
>
> This patch adds check for channel being disabled with orig_flags which
> is how this wiphy helper works.
this is the first mention about the wiphy helper. Probably need
statement here that call to wiphy_read_of_freq_limits() was added in
this patch which applies the extra limitation info read from DT.
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
> This patch should probably go through wireless-driver-next, I'm sending
> it just as a proof of concept. It was succesfully tested on SmartRG
> SR400ac with BCM43602.
>
> V4: Respect IEEE80211_CHAN_DISABLED in orig_flags
> ---
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> index ccae3bb..f95e316 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> @@ -5886,6 +5886,9 @@ static int brcmf_construct_chaninfo(struct brcmf_cfg80211_info *cfg,
> band->band);
> channel[index].hw_value = ch.control_ch_num;
>
> + if (channel->orig_flags & IEEE80211_CHAN_DISABLED)
> + continue;
> +
> /* assuming the chanspecs order is HT20,
> * HT40 upper, HT40 lower, and VHT80.
> */
> @@ -6477,6 +6480,7 @@ static int brcmf_setup_wiphy(struct wiphy *wiphy, struct brcmf_if *ifp)
> wiphy->bands[NL80211_BAND_5GHZ] = band;
> }
> }
> + wiphy_read_of_freq_limits(wiphy);
The return value is ignored, which I suppose is fine. So does the
function need a return value at all? Is there a scenario where the DT
info *must* be supplied?
Regards,
Arend
^ permalink raw reply
* Re: [PATCH] brcmfmac: avoid writing channel out of allocated array
From: Rafał Miłecki @ 2017-01-03 14:14 UTC (permalink / raw)
To: Arend Van Spriel
Cc: Kalle Valo, Franky Lin, Hante Meuleman, Pieter-Paul Giesberts,
Franky Lin, linux-wireless@vger.kernel.org,
open list:BROADCOM BRCM80211 IEEE802.11n WIRELESS DRIVER,
Rafał Miłecki
In-Reply-To: <0c0c9680-2cc8-ad0a-3aa0-ba406a838ab8@broadcom.com>
On 3 January 2017 at 14:19, Arend Van Spriel
<arend.vanspriel@broadcom.com> wrote:
> On 3-1-2017 12:31, Rafa=C5=82 Mi=C5=82ecki wrote:
>>>> + if (!channel) {
>>>> + brcmf_err("Firmware reported unexpected channel =
%d\n",
>>>> + ch.control_ch_num);
>>>> + continue;
>>>> + }
>>> As stated above something is really off when this happens so should we
>>> continue and try to make sense of what firmware provides or simply fail=
.
>> Well, I could image something like this happening and not being critical=
.
>> The simplest case: Broadcom team releases a new firmware which
>> supports extra 5 GHz channels (e.g. due to the IEEE standard change).
>> Why should we refuse to run & support all "old" channel just because of =
that?
>
> Fair enough. I was assuming we keep __wl_{2,5}ghz_channels up to date
> with IEEE standard.
>
>> What do you mean by "make sense of what firmware provides"? Would kind
>> of solution would you suggest?
>
> When the above assumption can be assured (by us) the only other scenario
> would be a change in the firmware API where we wrongly interpret the
> information retrieved. In this case all subsequent channels will likely
> result in bogus or accidental matches hence it seems better to bail out
> early.
Good point, this actually gave me an idea for a small nice
improvement. I remember I saw something like WL_VER in wl ioctl
protocol, it was already bumped at some point by Broadcom, when
chanspec format has changed. We should probably read this number from
firmware and maybe refuse to run if version is newer than the one we
know.
--=20
Rafa=C5=82
^ permalink raw reply
* Re: [PATCH V4 2/2] cfg80211: support ieee80211-freq-limit DT property
From: Rafał Miłecki @ 2017-01-03 14:17 UTC (permalink / raw)
To: Arend Van Spriel
Cc: Johannes Berg, linux-wireless@vger.kernel.org,
Martin Blumenstingl, Felix Fietkau, Arend van Spriel,
Arnd Bergmann, devicetree@vger.kernel.org,
Rafał Miłecki
In-Reply-To: <a67789ba-3346-6e10-89c2-2df419a80910@broadcom.com>
On 3 January 2017 at 13:10, Arend Van Spriel
<arend.vanspriel@broadcom.com> wrote:
> On 3-1-2017 12:03, Rafa=C5=82 Mi=C5=82ecki wrote:
>> From: Rafa=C5=82 Mi=C5=82ecki <rafal@milecki.pl>
>>
>> This patch adds a helper for reading that new property and applying
>> limitations or supported channels specified this way.
>> It may be useful for specifying single band devices or devices that
>> support only some part of the whole band. It's common that tri-band
>> routers have separated radios for lower and higher part of 5 GHz band.
>>
>> Signed-off-by: Rafa=C5=82 Mi=C5=82ecki <rafal@milecki.pl>
>> ---
>> V2: Put main code in core.c as it isn't strictly part of regulatory - po=
inted
>> by Arend.
>> Update to support ieee80211-freq-limit (new property).
>> V3: Introduce separated wiphy_read_of_freq_limits function.
>> Add extra sanity checks for DT data.
>> Move code back to reg.c as suggested by Johannes.
>> V4: Move code to of.c
>> Use one helper called at init time (no runtime hooks)
>> Modify orig_flags
>> ---
>> include/net/cfg80211.h | 26 ++++++++++
>> net/wireless/Makefile | 1 +
>> net/wireless/of.c | 137 ++++++++++++++++++++++++++++++++++++++++++=
+++++++
>> net/wireless/reg.c | 4 +-
>> net/wireless/reg.h | 2 +
>> 5 files changed, 168 insertions(+), 2 deletions(-)
>> create mode 100644 net/wireless/of.c
>>
>> diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
>> index ca2ac1c..d7723a8 100644
>> --- a/include/net/cfg80211.h
>> +++ b/include/net/cfg80211.h
>> @@ -311,6 +311,32 @@ struct ieee80211_supported_band {
>> struct ieee80211_sta_vht_cap vht_cap;
>> };
>>
>> +/**
>> + * wiphy_read_of_freq_limits - read frequency limits from device tree
>> + *
>> + * @wiphy: the wireless device to get extra limits for
>> + *
>> + * Some devices may have extra limitations specified in DT. This may be=
useful
>> + * for chipsets that normally support more bands but are limited due to=
board
>> + * design (e.g. by antennas or extermal power amplifier).
>> + *
>> + * This function reads info from DT and uses it to *modify* channels (d=
isable
>> + * unavailable ones). It's usually a *bad* idea to use it in drivers wi=
th
>> + * shared channel data as DT limitations are device specific.
>> + *
>> + * As this function access device node it has to be called after set_wi=
phy_dev.
>
> You are aware that you need to modify this description with earlier
> patch "cfg80211: allow passing struct device in the wiphy_new call",
> right? :-p
I dropped that earlier patch for now as it's no longer a requirement
for this change. If someone find is useful though, I'll be happy to
resume my work on it later. And update this documentation as you
pointed out ;)
>> + * It also modifies channels so they have to be set first.
>> + */
>> +#ifdef CONFIG_OF
>> +int wiphy_read_of_freq_limits(struct wiphy *wiphy);
>> +#else /* CONFIG_OF */
>> +static inline int wiphy_read_of_freq_limits(struct wiphy *wiphy)
>> +{
>> + return 0;
>> +}
>> +#endif /* !CONFIG_OF */
>> +
>> +
>
> [...]
>
>> diff --git a/net/wireless/reg.c b/net/wireless/reg.c
>> index 5dbac37..bda0e9e 100644
>> --- a/net/wireless/reg.c
>> +++ b/net/wireless/reg.c
>> @@ -748,8 +748,8 @@ static bool is_valid_rd(const struct ieee80211_regdo=
main *rd)
>> return true;
>> }
>>
>> -static bool reg_does_bw_fit(const struct ieee80211_freq_range *freq_ran=
ge,
>> - u32 center_freq_khz, u32 bw_khz)
>> +bool reg_does_bw_fit(const struct ieee80211_freq_range *freq_range,
>> + u32 center_freq_khz, u32 bw_khz)
>> {
>> u32 start_freq_khz, end_freq_khz;
>
> would it be more appropriate to move this function to util.c?
I'm OK with moving this function (and maybe struct
ieee80211_freq_range as well). Any objections?
--=20
Rafa=C5=82
^ permalink raw reply
* Re: [PATCH V4 3/2] brcmfmac: use wiphy_read_of_freq_limits to respect extra limits
From: Rafał Miłecki @ 2017-01-03 14:24 UTC (permalink / raw)
To: Arend Van Spriel
Cc: Johannes Berg, linux-wireless@vger.kernel.org,
Martin Blumenstingl, Felix Fietkau, Arend van Spriel,
Arnd Bergmann, devicetree@vger.kernel.org,
Rafał Miłecki
In-Reply-To: <d306c928-3725-f9d3-e3c5-e75a1849bd8b@broadcom.com>
On 3 January 2017 at 14:29, Arend Van Spriel
<arend.vanspriel@broadcom.com> wrote:
> What is with the patch numbering, ie. 3/2?
It's my small trick related to the "This patch should probably go
through wireless-driver-next" ;) I wanted to make it clear that only 2
patches are strictly targeted for the mac80211-next tree.
> On 3-1-2017 12:03, Rafa=C5=82 Mi=C5=82ecki wrote:
>> From: Rafa=C5=82 Mi=C5=82ecki <rafal@milecki.pl>
>>
>> There are some devices (e.g. Netgear R8000 home router) with one chipset
>> model used for different radios, some of them limited to subbands. NVRAM
>> entries don't contain any extra info on such limitations and firmware
>> reports full list of channels to us. We need to store extra limitation
>> info on DT to support such devices properly.
>>
>> This patch adds check for channel being disabled with orig_flags which
>> is how this wiphy helper works.
>
> this is the first mention about the wiphy helper. Probably need
> statement here that call to wiphy_read_of_freq_limits() was added in
> this patch which applies the extra limitation info read from DT.
OK, I'll improve this description.
>> Signed-off-by: Rafa=C5=82 Mi=C5=82ecki <rafal@milecki.pl>
>> ---
>> This patch should probably go through wireless-driver-next, I'm sending
>> it just as a proof of concept. It was succesfully tested on SmartRG
>> SR400ac with BCM43602.
>>
>> V4: Respect IEEE80211_CHAN_DISABLED in orig_flags
>> ---
>> drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c=
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
>> index ccae3bb..f95e316 100644
>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
>> @@ -5886,6 +5886,9 @@ static int brcmf_construct_chaninfo(struct brcmf_c=
fg80211_info *cfg,
>> band->band);
>> channel[index].hw_value =3D ch.control_ch_num;
>>
>> + if (channel->orig_flags & IEEE80211_CHAN_DISABLED)
>> + continue;
>> +
>> /* assuming the chanspecs order is HT20,
>> * HT40 upper, HT40 lower, and VHT80.
>> */
>> @@ -6477,6 +6480,7 @@ static int brcmf_setup_wiphy(struct wiphy *wiphy, =
struct brcmf_if *ifp)
>> wiphy->bands[NL80211_BAND_5GHZ] =3D band;
>> }
>> }
>> + wiphy_read_of_freq_limits(wiphy);
>
> The return value is ignored, which I suppose is fine. So does the
> function need a return value at all? Is there a scenario where the DT
> info *must* be supplied?
To be honest, I can't decide. Right now I don't see a point of
checking that function result (as you noticed, it should never be
required). If no one objects, I'll try switching that function to
void.
--=20
Rafa=C5=82
^ permalink raw reply
* Re: kernel 4.9 iwlwifi startup error
From: Alexander Morozov @ 2017-01-03 15:41 UTC (permalink / raw)
To: Andrew Donnellan; +Cc: Fabio Coatti, LKML, linux-wireless@vger.kernel.org
In-Reply-To: <942b4775-6027-2b99-84c7-37b7cde124b9@au1.ibm.com>
I have a similar problem on Gentoo. But in my case, it just can't load
firmware: "no suitable firmware found". I've tried to reinstall
firmware with no luck. Everything is ok with 4.8.6.
On Mon, Jan 2, 2017 at 6:42 PM, Andrew Donnellan
<andrew.donnellan@au1.ibm.com> wrote:
> On 02/01/17 21:12, Fabio Coatti wrote:
>>
>> Hi all,
>> I'm using kernel 4.9 and maybe half of the times I boot my laptop I get
>> the
>> error reported below, and the wifi does not work. I have to remove iwlwifi
>> (like
>> modprobe -r iwldvm iwlwifi) and insert it again to get things workig
>> again.
>> This seems a bit random, it does not happens all the times so it could be
>> a
>> timing issue or even a flaky hardware (unlikely, as I see only this issue
>> with
>> wifi, once it starts it works just fine)
>> I'm pretty sure to have seen the same behaviour at some point in 4.8.X
>> release, but right now I lost the related notes.
>> Environment:
>> Distro: gentoo
>> gcc 5.4.0
>> HW: Hewlett-Packard HP EliteBook Folio 9470m/18DF, BIOS 68IBD Ver. F.63
>> 04/26/2016
>> Intel(R) Core(TM) i5-3427U CPU @ 1.80GHz
>> Network controller: Intel Corporation Centrino Advanced-N 6235 (rev 24)
>>
>> Of course if more info is needed, just drop me a note.
>>
>> I'm not subscribed to mailing lists, so please keep me in CC: for any
>> information request.
>>
>> Many thanks.
>
>
> I've so far seen this once on my laptop, a Samsung NP540U3C (don't have it
> with me right now, so I'm not sure what the wifi chipset is), running with a
> Debian 4.9 kernel.
>
> --
> Andrew Donnellan OzLabs, ADL Canberra
> andrew.donnellan@au1.ibm.com IBM Australia Limited
>
^ 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