* Re: [RFC] cfg80211/mac80211: drop GTK-protected unicast IP packets
From: Krishna Chaitanya @ 2013-12-03 9:20 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless, j, Johannes Berg
In-Reply-To: <1386010316-2540-1-git-send-email-johannes@sipsolutions.net>
On Tue, Dec 3, 2013 at 12:21 AM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> From: Johannes Berg <johannes.berg@intel.com>
>
> The GTK is shared by all stations in an 802.11 BSS and as such any
> one of them can send forged group-addressed frames. To prevent this
> kind of attack, drop unicast IP packets if they were protected with
> the GTK, i.e. were multicast packets at the 802.11 layer.
>
> Based in part on a patch by Jouni that did the same but in the IP
> stack, which was considered too intrusive.
>
As per RFC 1122 this is an invalid case:
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.
We can simply drop this frame irrespective of GTK/PTK is used.
^ permalink raw reply
* Re: [RFC] cfg80211/mac80211: drop GTK-protected unicast IP packets
From: Johannes Berg @ 2013-12-03 9:34 UTC (permalink / raw)
To: Krishna Chaitanya; +Cc: linux-wireless, j
In-Reply-To: <CABPxzYJfBG0YtCrfkg_PqV4MP+MdLePhD2QdFQF9D+EfPObnZQ@mail.gmail.com>
On Tue, 2013-12-03 at 14:50 +0530, Krishna Chaitanya wrote:
> On Tue, Dec 3, 2013 at 12:21 AM, Johannes Berg
> <johannes@sipsolutions.net> wrote:
> > From: Johannes Berg <johannes.berg@intel.com>
> >
> > The GTK is shared by all stations in an 802.11 BSS and as such any
> > one of them can send forged group-addressed frames. To prevent this
> > kind of attack, drop unicast IP packets if they were protected with
> > the GTK, i.e. were multicast packets at the 802.11 layer.
> >
> > Based in part on a patch by Jouni that did the same but in the IP
> > stack, which was considered too intrusive.
> >
> As per RFC 1122 this is an invalid case:
> 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.
>
> We can simply drop this frame irrespective of GTK/PTK is used.
Interesting. Can you point out where this is implemented in the IP
stack(s)?
johannes
^ permalink raw reply
* Re: [RFC] cfg80211/mac80211: drop GTK-protected unicast IP packets
From: Nicolas Cavallari @ 2013-12-03 9:44 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless, j, Johannes Berg
In-Reply-To: <1386010316-2540-1-git-send-email-johannes@sipsolutions.net>
On 02/12/2013 19:51, Johannes Berg wrote:
> + if (!ipv4_is_multicast(ip.hdr4.daddr))
> + return -1;
So broadcasting to e.g. 192.168.255.255 is now forbidden ?
^ permalink raw reply
* Re: [RFC] cfg80211/mac80211: drop GTK-protected unicast IP packets
From: Johannes Berg @ 2013-12-03 9:45 UTC (permalink / raw)
To: Nicolas Cavallari; +Cc: linux-wireless, j
In-Reply-To: <529DA7FB.6020600@lri.fr>
On Tue, 2013-12-03 at 10:44 +0100, Nicolas Cavallari wrote:
> On 02/12/2013 19:51, Johannes Berg wrote:
> > + if (!ipv4_is_multicast(ip.hdr4.daddr))
> > + return -1;
>
> So broadcasting to e.g. 192.168.255.255 is now forbidden ?
Please, read the patch :)
johannes
^ permalink raw reply
* Re: [RFC] cfg80211/mac80211: drop GTK-protected unicast IP packets
From: Nicolas Cavallari @ 2013-12-03 10:41 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless, j
In-Reply-To: <1386063957.4393.4.camel@jlt4.sipsolutions.net>
On 03/12/2013 10:45, Johannes Berg wrote:
> On Tue, 2013-12-03 at 10:44 +0100, Nicolas Cavallari wrote:
>> On 02/12/2013 19:51, Johannes Berg wrote:
>>> + if (!ipv4_is_multicast(ip.hdr4.daddr))
>>> + return -1;
>>
>> So broadcasting to e.g. 192.168.255.255 is now forbidden ?
>
> Please, read the patch :)
I read the patch further. ipv4_is_multicast only checks if the
address is in 224/4, so this patch makes __ieee80211_data_to_8023
returns 0 for 224.0.0.0 to 239.255.255.255, and returns -1 for
everything else, including the 255.255.255.255, 192.168.255.255 and
other limited broadcast addresses, which are actually indistinguishable
from unicast addresses if you don't know the IP configuration.
If __ieee80211_data_to_8023 returns -1, the packet is dropped as
being unusable -- no less.
^ permalink raw reply
* Re: [RFC] cfg80211/mac80211: drop GTK-protected unicast IP packets
From: Nicolas Cavallari @ 2013-12-03 10:43 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless, j
In-Reply-To: <529DB560.40805@lri.fr>
On 03/12/2013 11:41, Nicolas Cavallari wrote:
> On 03/12/2013 10:45, Johannes Berg wrote:
>> On Tue, 2013-12-03 at 10:44 +0100, Nicolas Cavallari wrote:
>>> On 02/12/2013 19:51, Johannes Berg wrote:
>>>> + if (!ipv4_is_multicast(ip.hdr4.daddr))
>>>> + return -1;
>>>
>>> So broadcasting to e.g. 192.168.255.255 is now forbidden ?
>>
>> Please, read the patch :)
>
> I read the patch further. ipv4_is_multicast only checks if the
> address is in 224/4, so this patch makes __ieee80211_data_to_8023
> returns 0 for 224.0.0.0 to 239.255.255.255, and returns -1 for
> everything else, including the 255.255.255.255, 192.168.255.255 and
> other limited broadcast addresses
Err, i mean directed broadcast addresses.
^ permalink raw reply
* Re: [RFC] cfg80211/mac80211: drop GTK-protected unicast IP packets
From: Johannes Berg @ 2013-12-03 10:48 UTC (permalink / raw)
To: Nicolas Cavallari; +Cc: linux-wireless, j
In-Reply-To: <529DB560.40805@lri.fr>
On Tue, 2013-12-03 at 11:41 +0100, Nicolas Cavallari wrote:
> On 03/12/2013 10:45, Johannes Berg wrote:
> > On Tue, 2013-12-03 at 10:44 +0100, Nicolas Cavallari wrote:
> >> On 02/12/2013 19:51, Johannes Berg wrote:
> >>> + if (!ipv4_is_multicast(ip.hdr4.daddr))
> >>> + return -1;
> >>
> >> So broadcasting to e.g. 192.168.255.255 is now forbidden ?
> >
> > Please, read the patch :)
>
> I read the patch further. ipv4_is_multicast only checks if the
> address is in 224/4, so this patch makes __ieee80211_data_to_8023
> returns 0 for 224.0.0.0 to 239.255.255.255, and returns -1 for
> everything else, including the 255.255.255.255, 192.168.255.255 and
> other limited broadcast addresses, which are actually indistinguishable
> from unicast addresses if you don't know the IP configuration.
>
> If __ieee80211_data_to_8023 returns -1, the packet is dropped as
> being unusable -- no less.
You still haven't even begun to understand the patch. It only cares
about GTK-encrypted frames.
johannes
^ permalink raw reply
* Re: [RFC] cfg80211/mac80211: drop GTK-protected unicast IP packets
From: Johannes Berg @ 2013-12-03 10:51 UTC (permalink / raw)
To: Nicolas Cavallari; +Cc: linux-wireless, j
In-Reply-To: <1386067682.4393.5.camel@jlt4.sipsolutions.net>
On Tue, 2013-12-03 at 11:48 +0100, Johannes Berg wrote:
> On Tue, 2013-12-03 at 11:41 +0100, Nicolas Cavallari wrote:
> > On 03/12/2013 10:45, Johannes Berg wrote:
> > > On Tue, 2013-12-03 at 10:44 +0100, Nicolas Cavallari wrote:
> > >> On 02/12/2013 19:51, Johannes Berg wrote:
> > >>> + if (!ipv4_is_multicast(ip.hdr4.daddr))
> > >>> + return -1;
> > >>
> > >> So broadcasting to e.g. 192.168.255.255 is now forbidden ?
> > >
> > > Please, read the patch :)
> >
> > I read the patch further. ipv4_is_multicast only checks if the
> > address is in 224/4, so this patch makes __ieee80211_data_to_8023
> > returns 0 for 224.0.0.0 to 239.255.255.255, and returns -1 for
> > everything else, including the 255.255.255.255, 192.168.255.255 and
> > other limited broadcast addresses, which are actually indistinguishable
> > from unicast addresses if you don't know the IP configuration.
> >
> > If __ieee80211_data_to_8023 returns -1, the packet is dropped as
> > being unusable -- no less.
>
> You still haven't even begun to understand the patch. It only cares
> about GTK-encrypted frames.
Also, all your analysis is basically saying that I missed some cases.
That's fine and not much to worry about I guess (in particular if the
patch isn't needed at all.)
johannes
^ permalink raw reply
* Re: [RFC] cfg80211/mac80211: drop GTK-protected unicast IP packets
From: Krishna Chaitanya @ 2013-12-03 11:16 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless, j
In-Reply-To: <1386063265.4393.3.camel@jlt4.sipsolutions.net>
On Tue, Dec 3, 2013 at 3:04 PM, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Tue, 2013-12-03 at 14:50 +0530, Krishna Chaitanya wrote:
>> On Tue, Dec 3, 2013 at 12:21 AM, Johannes Berg
>> <johannes@sipsolutions.net> wrote:
>> > From: Johannes Berg <johannes.berg@intel.com>
>> >
>> > The GTK is shared by all stations in an 802.11 BSS and as such any
>> > one of them can send forged group-addressed frames. To prevent this
>> > kind of attack, drop unicast IP packets if they were protected with
>> > the GTK, i.e. were multicast packets at the 802.11 layer.
>> >
>> > Based in part on a patch by Jouni that did the same but in the IP
>> > stack, which was considered too intrusive.
>> >
>> As per RFC 1122 this is an invalid case:
>> 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.
>>
>> We can simply drop this frame irrespective of GTK/PTK is used.
>
> Interesting. Can you point out where this is implemented in the IP
> stack(s)?
AFAIK there is no check in the drivers/IP Stack for this. May be we can
implement this IP stack in a generic way confirming to RFC1122?
^ permalink raw reply
* [PATCH v3] cfg80211: fix dfs channel state after stopping AP
From: Marek Puzyniak @ 2013-12-03 12:24 UTC (permalink / raw)
To: linux-wireless; +Cc: johannes, Marek Puzyniak
In AP mode DFS channel state is changed to DFS_AVAILABLE
after successful CAC and remains as such until a radar
signal is detected during the In-Service Monitoring.
When AP is stopped it is no longer monitoring current channel
for radar signals. DFS channel state should be changed back
to DFS_USABLE. Starting AP again on that channel will start CAC
instead of starting radiation.
Signed-off-by: Marek Puzyniak <marek.puzyniak@tieto.com>
---
net/wireless/ap.c | 3 +++
net/wireless/chan.c | 39 +++++++++++++++++++++++++++++++++++++++
net/wireless/core.h | 11 +++++++++++
3 files changed, 53 insertions(+)
diff --git a/net/wireless/ap.c b/net/wireless/ap.c
index 324e8d8..9349773 100644
--- a/net/wireless/ap.c
+++ b/net/wireless/ap.c
@@ -26,6 +26,9 @@ static int __cfg80211_stop_ap(struct cfg80211_registered_device *rdev,
err = rdev_stop_ap(rdev, dev);
if (!err) {
+ if (cfg80211_chandef_dfs_required(wdev->wiphy, &wdev->preset_chandef))
+ cfg80211_leave_dfs_chandef(wdev->wiphy, &wdev->preset_chandef);
+
wdev->beacon_interval = 0;
wdev->channel = NULL;
wdev->ssid_len = 0;
diff --git a/net/wireless/chan.c b/net/wireless/chan.c
index 78559b5..f0cf780 100644
--- a/net/wireless/chan.c
+++ b/net/wireless/chan.c
@@ -490,6 +490,45 @@ static bool cfg80211_chandef_dfs_available(struct wiphy *wiphy,
return r;
}
+static void cfg80211_leave_dfs_chans(struct wiphy *wiphy,
+ u32 center_freq,
+ u32 bandwidth)
+{
+ struct ieee80211_channel *c;
+ u32 freq, start_freq, end_freq;
+
+ start_freq = cfg80211_get_start_freq(center_freq, bandwidth);
+ end_freq = cfg80211_get_end_freq(center_freq, bandwidth);
+
+ for (freq = start_freq; freq <= end_freq; freq += 20) {
+ c = ieee80211_get_channel(wiphy, freq);
+ if (!c)
+ continue;
+ if (!(c->flags & IEEE80211_CHAN_RADAR))
+ continue;
+ if (c->dfs_state != NL80211_DFS_AVAILABLE)
+ continue;
+
+ cfg80211_set_chans_dfs_state(wiphy, freq, 20, NL80211_DFS_USABLE);
+ }
+}
+
+void cfg80211_leave_dfs_chandef(struct wiphy *wiphy,
+ struct cfg80211_chan_def *chandef)
+{
+ int width;
+
+ if (WARN_ON(!cfg80211_chandef_valid(chandef)))
+ return;
+
+ width = cfg80211_chandef_get_width(chandef);
+ cfg80211_leave_dfs_chans(wiphy, chandef->center_freq1, width);
+
+ if (!chandef->center_freq2)
+ return;
+
+ cfg80211_leave_dfs_chans(wiphy, chandef->center_freq2, width);
+}
static bool cfg80211_secondary_chans_ok(struct wiphy *wiphy,
u32 center_freq, u32 bandwidth,
diff --git a/net/wireless/core.h b/net/wireless/core.h
index 6716c5c..ca230a2 100644
--- a/net/wireless/core.h
+++ b/net/wireless/core.h
@@ -400,6 +400,17 @@ void cfg80211_set_dfs_state(struct wiphy *wiphy,
void cfg80211_dfs_channels_update_work(struct work_struct *work);
+/**
+ * cfg80211_leave_dfs_chandef - Leaving dfs chandef
+ * @wiphy: the wiphy
+ * @chandef: chandef for the current channel
+ *
+ * This function is called when dfs chandef is being not used for different
+ * reasons. Change channels DFS_AVAILABLE to DFS_USABLE again. Leave channels
+ * DFS_UNAVAILABLE untouched.
+ */
+void cfg80211_leave_dfs_chandef(struct wiphy *wiphy,
+ struct cfg80211_chan_def *chandef);
static inline int
cfg80211_can_change_interface(struct cfg80211_registered_device *rdev,
--
1.8.1.2
^ permalink raw reply related
* Re: [PATCH 2/2 V3] mac80211: Update opmode during adding new station
From: Johannes Berg @ 2013-12-03 12:47 UTC (permalink / raw)
To: Marek Kwaczynski; +Cc: linux-wireless
In-Reply-To: <1386061500-5750-2-git-send-email-marek.kwaczynski@tieto.com>
On Tue, 2013-12-03 at 10:05 +0100, Marek Kwaczynski wrote:
> -void ieee80211_vht_handle_opmode(struct ieee80211_sub_if_data *sdata,
> - struct sta_info *sta, u8 opmode,
> - enum ieee80211_band band, bool nss_only)
> +int ieee80211_vht_recalc_handle_opmode(struct ieee80211_sub_if_data *sdata,
> + struct sta_info *sta, u8 opmode,
> + enum ieee80211_band band, bool nss_only,
> + u32 *changed)
Err, why don't you just return a u32 with the change flags?
johannes
^ permalink raw reply
* Re: [PATCH 1/2 V3] nl80211/cfg80211: Set Operating Mode Notification
From: Johannes Berg @ 2013-12-03 12:47 UTC (permalink / raw)
To: Marek Kwaczynski; +Cc: linux-wireless
In-Reply-To: <1386061500-5750-1-git-send-email-marek.kwaczynski@tieto.com>
On Tue, 2013-12-03 at 10:04 +0100, Marek Kwaczynski wrote:
> This attribuite is needed for setting Operating Mode Notification
> in AP mode from User Space. This functionality is required when
> User Space received Assoc Reqeust contains Operation Mode
> Notification element.
Applied.
johannes
^ permalink raw reply
* Re: [PATCH] nl80211: allow the use of DFS channel in mesh
From: Johannes Berg @ 2013-12-03 12:50 UTC (permalink / raw)
To: Chun-Yeow Yeoh; +Cc: linux-wireless, linville, devel, distro11s
In-Reply-To: <1385971550-4892-1-git-send-email-yeohchunyeow@gmail.com>
On Mon, 2013-12-02 at 16:05 +0800, Chun-Yeow Yeoh wrote:
> This permits the use of DFS channel once the CAC is conducted and
> no radar is detected.
This seems pretty pointless right now? There's nothing supporting it, is
there?
johannes
^ permalink raw reply
* Re: [PATCH v3 1/4] cfg80211: add reg_get_dfs_region()
From: Johannes Berg @ 2013-12-03 12:54 UTC (permalink / raw)
To: Janusz Dziedzic; +Cc: linux-wireless, Luis R. Rodriguez
In-Reply-To: <1385409372-4229-1-git-send-email-janusz.dziedzic@tieto.com>
On Mon, 2013-11-25 at 20:56 +0100, Janusz Dziedzic wrote:
> From: "Luis R. Rodriguez" <mcgrof@do-not-panic.com>
>
> This can be used outside of the regulatory context for any checks
> on the DFS region. The central cfg80211 dfs_region is always used
> and if it does not match with the wiphy a debug print is issued.
Ok I've applied patches 1 and 2
johannes
^ permalink raw reply
* Re: [PATCH v4] mac80211_hwsim: claim CSA support for AP
From: Johannes Berg @ 2013-12-03 12:55 UTC (permalink / raw)
To: Karl Beldan
Cc: linux-wireless, Karl Beldan, Simon Wunderlich,
Andrei Otcheretianski
In-Reply-To: <1385224698-12294-1-git-send-email-karl.beldan@gmail.com>
On Sat, 2013-11-23 at 17:38 +0100, Karl Beldan wrote:
> This adds a per vif bool csa_finished that is set after a call to
> ieee80211_csa_finish() and used to skip beaconing while csa_active is
> set in case a beacon is scheduled prior to csa_finalize_work completion.
> This bool and the number of beacons transmitted during the CSA up to the
> call to ieee80211_csa_finish() are reset in channel_switch_beacon op.
Andrei says:
Overall it seems ok, but all the purpose of csa_finished is not very
clear..
It looks that this patch tries to avoid extra beaconing on the previous
channel/hitting the warning..
But the problem is much bigger here, it means that we didn't switch in
time (before the next beacon) so it's ok to hit the warning here and
transmit extra beacon with count==1.
So if we see a lot of such warnings, maybe we need to fix
ieee80211_csa_finish instead (not using work for example)
johannes
^ permalink raw reply
* [PATCH] mac80211: remove duplicate code
From: Emmanuel Grumbach @ 2013-12-03 13:02 UTC (permalink / raw)
To: johannes; +Cc: linux-wireless, Eliad Peller
From: Eliad Peller <eliad@wizery.com>
The same code appears just a few lines below.
Signed-off-by: Eliad Peller <eliad@wizery.com>
---
net/mac80211/scan.c | 8 --------
1 file changed, 8 deletions(-)
diff --git a/net/mac80211/scan.c b/net/mac80211/scan.c
index 5ad66a8..6a94d48 100644
--- a/net/mac80211/scan.c
+++ b/net/mac80211/scan.c
@@ -786,14 +786,6 @@ void ieee80211_scan_work(struct work_struct *work)
}
/*
- * Avoid re-scheduling when the sdata is going away.
- */
- if (!ieee80211_sdata_running(sdata)) {
- aborted = true;
- goto out_complete;
- }
-
- /*
* as long as no delay is required advance immediately
* without scheduling a new work
*/
--
1.8.3.2
^ permalink raw reply related
* Re: [PATCHv3] cfg80211: add support for frequency interference event
From: Johannes Berg @ 2013-12-03 13:25 UTC (permalink / raw)
To: Rajesh Chauhan
Cc: rodrigue, linux-wireless, jouni, hbahini, jjohnson, schang, xunl,
sameert, c_arifh
In-Reply-To: <1384198356-427-1-git-send-email-rajeshc@qca.qualcomm.com>
On Mon, 2013-11-11 at 11:32 -0800, Rajesh Chauhan wrote:
> + * This structure provides frequency range(s) that should be avoided
> + * because of interference.
extra tab that should be a space only
> + * @interference_source: enum nl80211_freq_interference_source_type
> + * is used to specify source of interference.
Do we expect that different sources would be treated differently? If
not, what use is this?
> + * @start_freq: start of frequency range. Frequency is specified in
> + * KHz and is center frequency in 20 MHz channel bandwidth
> + * @end_freq: end of frequency range. Frequency is specified in KHz
> + * and is center frequency in 20 MHz channel bandwidth
Why would those be required to be center frequencies of some wireless
channels? That doesn't make much sense, e.g. if you have raw
interference data from a modem, no?
> /**
> + * cfg80211_avoid_frequency_event - frequency interference event
> + * @netdev: network device
> + * @freq_list: frequency range(s) information
> + * @gfp: allocation flags
> + *
> + * Wireless driver calls this function to notify userspace about frequency
> + * range(s) that should be avoided because of interference.
> + */
> +void cfg80211_avoid_frequency_event(struct net_device *netdev,
> + struct cfg80211_avoid_frequency_list *freq_list, gfp_t gfp);
Indentation. Also, that freq_list parameter should probably be const?
> + * @NL80211_CMD_AVOID_FREQUENCIES_EVENT: Send range(s) of interfering
> + * frequencies that should be avoided, from the wireless driver to the
> + * user space. If SoftAP/P2P-GO is operating on interfering frequency
> + * then user space should stop and resart them avoiding interfering
typo: restart
> + * frequency range(s). User space may decide to continue operation on
> + * interfering frequency, but in such case, there might be impact on
> + * performance.
Also, I think describing policy here ("userspace should stop and
restart") is wrong. This should be up to userspace. It could
alternatively do a real CSA for example.
> + * @NL80211_ATTR_AVOID_FREQUENCIES: Avoid frequencies container
> information
That should probably describe in more detail how the information in it
is formatted.
> +/**,
> + * enum nl80211_avoid_frequency_attr - avoid frequency attributes
> + * @__NL80211_FREQUENCY_ATTR_INVALID: attribute number 0 is reserved
> + * @NL80211_AVOID_FREQUENCY_ATTR_INTERFERENCE_SOURCE_TYPE: interference
> + * source
> + * @NL80211_AVOID_FREQUENCY_ATTR_START_FREQ: Start of frequency range.
> + * Frequency is specified in KHz and is center frequency in 20 MHz
> + * channel bandwidth.
> + * @NL80211_AVOID_FREQUENCY_ATTR_END_FREQ: End of frequency range.
> + * Frequency is specified in KHz and is center frequency in 20 MHz
> + * channel bandwidth.
> + * @__NL80211_FREQUENCY_ATTR_AFTER_LAST: internal use
missing docs for the other remaining enum members
> +void cfg80211_avoid_frequency_event(struct net_device *netdev,
> + struct cfg80211_avoid_frequency_list *freq_list, gfp_t gfp)
indentation
> + struct wiphy *wiphy;
> + struct cfg80211_registered_device *rdev;
> + struct sk_buff *msg;
> + void *hdr;
> + struct nlattr *nl_ranges, *nl_range;
> + int err = -EINVAL;
wtf? that value is never used - don't randomly initialize variables. Is
the variable itself even used?
> + hdr = nl80211hdr_put(msg, 0, 0, 0, NL80211_CMD_AVOID_FREQUENCIES_EVENT);
> + if (!hdr) {
> + nlmsg_free(msg);
> + return;
> + }
> +
> + if (nla_put_u32(msg, NL80211_ATTR_WIPHY, rdev->wiphy_idx) ||
> + nla_put_u32(msg, NL80211_ATTR_IFINDEX, netdev->ifindex))
> + goto nla_put_failure;
wdev index is missing, however, see above.
> + for (i = 0; i < freq_list->n_freq_ranges; i++) {
> + nl_range = nla_nest_start(msg, i);
0 is invalid, I'm pretty sure.
> + genlmsg_multicast_netns(wiphy_net(&rdev->wiphy), msg, 0,
> + nl80211_mlme_mcgrp.id, gfp);
This is no longer the right API
johannes
^ permalink raw reply
* Re: [PATCH] mac80211: remove duplicate code
From: Johannes Berg @ 2013-12-03 13:26 UTC (permalink / raw)
To: Emmanuel Grumbach; +Cc: linux-wireless, Eliad Peller
In-Reply-To: <1386075735-28589-1-git-send-email-emmanuel.grumbach@intel.com>
On Tue, 2013-12-03 at 15:02 +0200, Emmanuel Grumbach wrote:
> From: Eliad Peller <eliad@wizery.com>
>
> The same code appears just a few lines below.
Applied.
johannes
^ permalink raw reply
* [PATCH v6 1/5] mac80211: refactor ieee80211_ibss_process_chanswitch()
From: Luciano Coelho @ 2013-12-03 13:48 UTC (permalink / raw)
To: linux-wireless, sw, yeohchunyeow; +Cc: yeohchunyeow
In-Reply-To: <1386078502-19845-1-git-send-email-luciano.coelho@intel.com>
Refactor ieee80211_ibss_process_chanswitch() to use
ieee80211_channel_switch() and avoid code duplication.
Change-Id: I265a12c7f825dc20535bad1197a81437310d0086
Signed-off-by: Luciano Coelho <luciano.coelho@intel.com>
---
net/mac80211/cfg.c | 4 ++--
net/mac80211/ibss.c | 58 +++++++---------------------------------------
net/mac80211/ieee80211_i.h | 2 ++
3 files changed, 12 insertions(+), 52 deletions(-)
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 754069c..b5bc27e 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -3048,8 +3048,8 @@ unlock:
sdata_unlock(sdata);
}
-static int ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev,
- struct cfg80211_csa_settings *params)
+int ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev,
+ struct cfg80211_csa_settings *params)
{
struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
struct ieee80211_local *local = sdata->local;
diff --git a/net/mac80211/ibss.c b/net/mac80211/ibss.c
index 0f1fb5d..3514aab 100644
--- a/net/mac80211/ibss.c
+++ b/net/mac80211/ibss.c
@@ -784,18 +784,10 @@ ieee80211_ibss_process_chanswitch(struct ieee80211_sub_if_data *sdata,
struct cfg80211_csa_settings params;
struct ieee80211_csa_ie csa_ie;
struct ieee80211_if_ibss *ifibss = &sdata->u.ibss;
- struct ieee80211_chanctx_conf *chanctx_conf;
- struct ieee80211_chanctx *chanctx;
enum nl80211_channel_type ch_type;
- int err, num_chanctx;
+ int err;
u32 sta_flags;
- if (sdata->vif.csa_active)
- return true;
-
- if (!sdata->vif.bss_conf.ibss_joined)
- return false;
-
sta_flags = IEEE80211_STA_DISABLE_VHT;
switch (ifibss->chandef.width) {
case NL80211_CHAN_WIDTH_5:
@@ -826,9 +818,6 @@ ieee80211_ibss_process_chanswitch(struct ieee80211_sub_if_data *sdata,
params.count = csa_ie.count;
params.chandef = csa_ie.chandef;
- if (ifibss->chandef.chan->band != params.chandef.chan->band)
- goto disconnect;
-
switch (ifibss->chandef.width) {
case NL80211_CHAN_WIDTH_20_NOHT:
case NL80211_CHAN_WIDTH_20:
@@ -884,29 +873,6 @@ ieee80211_ibss_process_chanswitch(struct ieee80211_sub_if_data *sdata,
params.radar_required = true;
}
- rcu_read_lock();
- chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
- if (!chanctx_conf) {
- rcu_read_unlock();
- goto disconnect;
- }
-
- /* don't handle for multi-VIF cases */
- chanctx = container_of(chanctx_conf, struct ieee80211_chanctx, conf);
- if (chanctx->refcount > 1) {
- rcu_read_unlock();
- goto disconnect;
- }
- num_chanctx = 0;
- list_for_each_entry_rcu(chanctx, &sdata->local->chanctx_list, list)
- num_chanctx++;
-
- if (num_chanctx > 1) {
- rcu_read_unlock();
- goto disconnect;
- }
- rcu_read_unlock();
-
/* all checks done, now perform the channel switch. */
ibss_dbg(sdata,
"received channel switch announcement to go to channel %d MHz\n",
@@ -914,19 +880,9 @@ ieee80211_ibss_process_chanswitch(struct ieee80211_sub_if_data *sdata,
params.block_tx = !!csa_ie.mode;
- ieee80211_ibss_csa_beacon(sdata, ¶ms);
- sdata->csa_radar_required = params.radar_required;
-
- if (params.block_tx)
- ieee80211_stop_queues_by_reason(&sdata->local->hw,
- IEEE80211_MAX_QUEUE_MAP,
- IEEE80211_QUEUE_STOP_REASON_CSA);
-
- sdata->csa_chandef = params.chandef;
- sdata->vif.csa_active = true;
-
- ieee80211_bss_info_change_notify(sdata, err);
- drv_channel_switch_beacon(sdata, ¶ms.chandef);
+ if (ieee80211_channel_switch(sdata->local->hw.wiphy, sdata->dev,
+ ¶ms))
+ goto disconnect;
ieee80211_ibss_csa_mark_radar(sdata);
@@ -962,7 +918,8 @@ ieee80211_rx_mgmt_spectrum_mgmt(struct ieee80211_sub_if_data *sdata,
if (len < required_len)
return;
- ieee80211_ibss_process_chanswitch(sdata, elems, false);
+ if (!sdata->vif.csa_active)
+ ieee80211_ibss_process_chanswitch(sdata, elems, false);
}
static void ieee80211_rx_mgmt_deauth_ibss(struct ieee80211_sub_if_data *sdata,
@@ -1143,7 +1100,8 @@ static void ieee80211_rx_bss_info(struct ieee80211_sub_if_data *sdata,
goto put_bss;
/* process channel switch */
- if (ieee80211_ibss_process_chanswitch(sdata, elems, true))
+ if (sdata->vif.csa_active ||
+ ieee80211_ibss_process_chanswitch(sdata, elems, true))
goto put_bss;
/* same BSSID */
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 32bae21..a7b4d55 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -1442,6 +1442,8 @@ void ieee80211_handle_roc_started(struct ieee80211_roc_work *roc);
/* channel switch handling */
void ieee80211_csa_finalize_work(struct work_struct *work);
+int ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev,
+ struct cfg80211_csa_settings *params);
/* interface handling */
int ieee80211_iface_init(void);
--
1.8.4.2
^ permalink raw reply related
* [PATCH v6 0/5] CSA beacon count changes
From: Luciano Coelho @ 2013-12-03 13:48 UTC (permalink / raw)
To: linux-wireless, sw, yeohchunyeow; +Cc: yeohchunyeow
Hi,
This is a new version of my patchset. Now it's rebased on the latest
mac80211-next, so all the locking changes are included. I hope the
warnings observed with mesh and IBSS go away now that the locks are
working properly.
Simon and Chun-Yeow, it would be great if you could re-test this
updated version and let me know how it goes.
Thanks!
Luca.
Luciano Coelho (5):
mac80211: refactor ieee80211_ibss_process_chanswitch()
mac80211: align ieee80211_ibss_csa_beacon() with
ieee80211_csa_beacon()
mac80211: refactor ieee80211_mesh_process_chanswitch()
mac80211: align ieee80211_mesh_csa_beacon() with
ieee80211_csa_beacon()
mac80211: only set CSA beacon when at least one beacon must be
transmitted
include/net/mac80211.h | 10 ++--
net/mac80211/cfg.c | 120 +++++++++++++++++++++++++++++++++++----------
net/mac80211/ibss.c | 78 +++++++----------------------
net/mac80211/ieee80211_i.h | 11 +++--
net/mac80211/mesh.c | 78 ++++++-----------------------
net/mac80211/tx.c | 19 +++----
net/mac80211/util.c | 1 -
7 files changed, 148 insertions(+), 169 deletions(-)
--
1.8.4.2
^ permalink raw reply
* [PATCH v6 3/5] mac80211: refactor ieee80211_mesh_process_chanswitch()
From: Luciano Coelho @ 2013-12-03 13:48 UTC (permalink / raw)
To: linux-wireless, sw, yeohchunyeow; +Cc: yeohchunyeow
In-Reply-To: <1386078502-19845-1-git-send-email-luciano.coelho@intel.com>
Refactor ieee80211_mesh_process_chanswitch() to use
ieee80211_channel_switch() and avoid code duplication.
Signed-off-by: Luciano Coelho <luciano.coelho@intel.com>
---
net/mac80211/cfg.c | 10 +++++--
net/mac80211/ieee80211_i.h | 6 +++-
net/mac80211/mesh.c | 68 ++++++++++------------------------------------
net/mac80211/util.c | 1 -
4 files changed, 27 insertions(+), 58 deletions(-)
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index b5bc27e..790c15d 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -3156,9 +3156,15 @@ int ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev,
params->chandef.chan->band)
return -EINVAL;
- err = ieee80211_mesh_csa_beacon(sdata, params, true);
- if (err < 0)
+ if (ifmsh->csa_role == IEEE80211_MESH_CSA_ROLE_NONE)
+ ifmsh->csa_role = IEEE80211_MESH_CSA_ROLE_INIT;
+
+ err = ieee80211_mesh_csa_beacon(sdata, params,
+ (ifmsh->csa_role == IEEE80211_MESH_CSA_ROLE_INIT));
+ if (err < 0) {
+ ifmsh->csa_role = IEEE80211_MESH_CSA_ROLE_NONE;
return err;
+ }
break;
#endif
default:
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index a7b4d55..cf38a74 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -611,7 +611,11 @@ struct ieee80211_if_mesh {
struct ps_data ps;
/* Channel Switching Support */
struct mesh_csa_settings __rcu *csa;
- bool chsw_init;
+ enum {
+ IEEE80211_MESH_CSA_ROLE_NONE,
+ IEEE80211_MESH_CSA_ROLE_INIT,
+ IEEE80211_MESH_CSA_ROLE_REPEATER,
+ } csa_role;
u8 chsw_ttl;
u16 pre_value;
};
diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c
index 330d1f7..980cc12 100644
--- a/net/mac80211/mesh.c
+++ b/net/mac80211/mesh.c
@@ -685,7 +685,7 @@ ieee80211_mesh_build_beacon(struct ieee80211_if_mesh *ifmsh)
*pos++ = csa->settings.count;
*pos++ = WLAN_EID_CHAN_SWITCH_PARAM;
*pos++ = 6;
- if (ifmsh->chsw_init) {
+ if (ifmsh->csa_role == IEEE80211_MESH_CSA_ROLE_INIT) {
*pos++ = ifmsh->mshcfg.dot11MeshTTL;
*pos |= WLAN_EID_CHAN_SWITCH_PARAM_INITIATOR;
} else {
@@ -853,19 +853,11 @@ ieee80211_mesh_process_chnswitch(struct ieee80211_sub_if_data *sdata,
{
struct cfg80211_csa_settings params;
struct ieee80211_csa_ie csa_ie;
- struct ieee80211_chanctx_conf *chanctx_conf;
- struct ieee80211_chanctx *chanctx;
struct ieee80211_if_mesh *ifmsh = &sdata->u.mesh;
enum ieee80211_band band = ieee80211_get_sdata_band(sdata);
- int err, num_chanctx;
+ int err;
u32 sta_flags;
- if (sdata->vif.csa_active)
- return true;
-
- if (!ifmsh->mesh_id)
- return false;
-
sta_flags = IEEE80211_STA_DISABLE_VHT;
switch (sdata->vif.bss_conf.chandef.width) {
case NL80211_CHAN_WIDTH_20_NOHT:
@@ -890,10 +882,6 @@ ieee80211_mesh_process_chnswitch(struct ieee80211_sub_if_data *sdata,
params.chandef = csa_ie.chandef;
params.count = csa_ie.count;
- if (sdata->vif.bss_conf.chandef.chan->band !=
- params.chandef.chan->band)
- return false;
-
if (!cfg80211_chandef_usable(sdata->local->hw.wiphy, ¶ms.chandef,
IEEE80211_CHAN_DISABLED)) {
sdata_info(sdata,
@@ -916,25 +904,6 @@ ieee80211_mesh_process_chnswitch(struct ieee80211_sub_if_data *sdata,
return false;
}
- rcu_read_lock();
- chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
- if (!chanctx_conf)
- goto failed_chswitch;
-
- /* don't handle for multi-VIF cases */
- chanctx = container_of(chanctx_conf, struct ieee80211_chanctx, conf);
- if (chanctx->refcount > 1)
- goto failed_chswitch;
-
- num_chanctx = 0;
- list_for_each_entry_rcu(chanctx, &sdata->local->chanctx_list, list)
- num_chanctx++;
-
- if (num_chanctx > 1)
- goto failed_chswitch;
-
- rcu_read_unlock();
-
mcsa_dbg(sdata,
"received channel switch announcement to go to channel %d MHz\n",
params.chandef.chan->center_freq);
@@ -945,27 +914,16 @@ ieee80211_mesh_process_chnswitch(struct ieee80211_sub_if_data *sdata,
else
ifmsh->chsw_ttl = 0;
- if (ifmsh->chsw_ttl > 0)
- if (ieee80211_mesh_csa_beacon(sdata, ¶ms, false) < 0)
- return false;
-
- sdata->csa_radar_required = params.radar_required;
-
- if (params.block_tx)
- ieee80211_stop_queues_by_reason(&sdata->local->hw,
- IEEE80211_MAX_QUEUE_MAP,
- IEEE80211_QUEUE_STOP_REASON_CSA);
+ if (ifmsh->chsw_ttl == 0)
+ return false;
- sdata->csa_chandef = params.chandef;
- sdata->vif.csa_active = true;
+ ifmsh->csa_role = IEEE80211_MESH_CSA_ROLE_REPEATER;
- ieee80211_bss_info_change_notify(sdata, err);
- drv_channel_switch_beacon(sdata, ¶ms.chandef);
+ if (ieee80211_channel_switch(sdata->local->hw.wiphy, sdata->dev,
+ ¶ms) < 0)
+ return false;
return true;
-failed_chswitch:
- rcu_read_unlock();
- return false;
}
static void
@@ -1075,7 +1033,8 @@ static void ieee80211_mesh_rx_bcn_presp(struct ieee80211_sub_if_data *sdata,
ifmsh->sync_ops->rx_bcn_presp(sdata,
stype, mgmt, &elems, rx_status);
- if (!ifmsh->chsw_init)
+ if (ifmsh->csa_role != IEEE80211_MESH_CSA_ROLE_INIT &&
+ !sdata->vif.csa_active)
ieee80211_mesh_process_chnswitch(sdata, &elems, true);
}
@@ -1086,7 +1045,7 @@ int ieee80211_mesh_finish_csa(struct ieee80211_sub_if_data *sdata)
int ret = 0;
/* Reset the TTL value and Initiator flag */
- ifmsh->chsw_init = false;
+ ifmsh->csa_role = IEEE80211_MESH_CSA_ROLE_NONE;
ifmsh->chsw_ttl = 0;
/* Remove the CSA and MCSP elements from the beacon */
@@ -1200,7 +1159,8 @@ static void mesh_rx_csa_frame(struct ieee80211_sub_if_data *sdata,
ifmsh->pre_value = pre_value;
- if (!ieee80211_mesh_process_chnswitch(sdata, &elems, false)) {
+ if (!sdata->vif.csa_active &&
+ !ieee80211_mesh_process_chnswitch(sdata, &elems, false)) {
mcsa_dbg(sdata, "Failed to process CSA action frame");
return;
}
@@ -1355,7 +1315,7 @@ void ieee80211_mesh_init_sdata(struct ieee80211_sub_if_data *sdata)
mesh_rmc_init(sdata);
ifmsh->last_preq = jiffies;
ifmsh->next_perr = jiffies;
- ifmsh->chsw_init = false;
+ ifmsh->csa_role = IEEE80211_MESH_CSA_ROLE_NONE;
/* Allocate all mesh structures when creating the first mesh interface. */
if (!mesh_allocated)
ieee80211s_init();
diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index 06265d7..f153ab4 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -2487,7 +2487,6 @@ int ieee80211_send_action_csa(struct ieee80211_sub_if_data *sdata,
ifmsh->pre_value++;
put_unaligned_le16(ifmsh->pre_value, pos);/* Precedence Value */
pos += 2;
- ifmsh->chsw_init = true;
}
ieee80211_tx_skb(sdata, skb);
--
1.8.4.2
^ permalink raw reply related
* [PATCH v6 4/5] mac80211: align ieee80211_mesh_csa_beacon() with ieee80211_csa_beacon()
From: Luciano Coelho @ 2013-12-03 13:48 UTC (permalink / raw)
To: linux-wireless, sw, yeohchunyeow; +Cc: yeohchunyeow
In-Reply-To: <1386078502-19845-1-git-send-email-luciano.coelho@intel.com>
The return value of ieee80211_mesh_csa_beacon is not aligned with the
return value of ieee80211_csa_beacon() and
ieee80211_ibss_csa_beacon(). For consistency and to be able to use
both functions with similar code, change ieee80211_mesh_csa_beacon()
not to send the bss changed notification itself, but return what has
changed so the caller can send the notification instead.
Change-Id: If4c111fd874a3e17a5df98d306f7f1c8fcaa6a1a
Signed-off-by: Luciano Coelho <luciano.coelho@intel.com>
---
net/mac80211/mesh.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c
index 980cc12..5476ad9 100644
--- a/net/mac80211/mesh.c
+++ b/net/mac80211/mesh.c
@@ -1089,12 +1089,10 @@ int ieee80211_mesh_csa_beacon(struct ieee80211_sub_if_data *sdata,
return ret;
}
- ieee80211_bss_info_change_notify(sdata, BSS_CHANGED_BEACON);
-
if (csa_action)
ieee80211_send_action_csa(sdata, csa_settings);
- return 0;
+ return BSS_CHANGED_BEACON;
}
static int mesh_fwd_csa_frame(struct ieee80211_sub_if_data *sdata,
--
1.8.4.2
^ permalink raw reply related
* [PATCH v6 2/5] mac80211: align ieee80211_ibss_csa_beacon() with ieee80211_csa_beacon()
From: Luciano Coelho @ 2013-12-03 13:48 UTC (permalink / raw)
To: linux-wireless, sw, yeohchunyeow; +Cc: yeohchunyeow
In-Reply-To: <1386078502-19845-1-git-send-email-luciano.coelho@intel.com>
The return value of ieee80211_ibss_csa_beacon is not aligned with the
return value of ieee80211_csa_beacon(). For consistency and to be
able to use both functions with similar code, change
ieee80211_ibss_csa_beacon() not to send the bss changed notification
itself, but return what has changed so the caller can send the
notification instead.
Signed-off-by: Luciano Coelho <luciano.coelho@intel.com>
---
net/mac80211/ibss.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/net/mac80211/ibss.c b/net/mac80211/ibss.c
index 3514aab..23e035f 100644
--- a/net/mac80211/ibss.c
+++ b/net/mac80211/ibss.c
@@ -522,7 +522,7 @@ int ieee80211_ibss_csa_beacon(struct ieee80211_sub_if_data *sdata,
if (csa_settings)
ieee80211_send_action_csa(sdata, csa_settings);
- ieee80211_bss_info_change_notify(sdata, BSS_CHANGED_BEACON);
+ return BSS_CHANGED_BEACON;
out:
return ret;
}
@@ -559,11 +559,17 @@ int ieee80211_ibss_finish_csa(struct ieee80211_sub_if_data *sdata)
/* generate the beacon */
err = ieee80211_ibss_csa_beacon(sdata, NULL);
- sdata_unlock(sdata);
if (err < 0)
- return err;
+ goto out;
- return 0;
+ if (err) {
+ ieee80211_bss_info_change_notify(sdata, err);
+ err = 0;
+ }
+out:
+ sdata_unlock(sdata);
+
+ return err;
}
void ieee80211_ibss_stop(struct ieee80211_sub_if_data *sdata)
--
1.8.4.2
^ permalink raw reply related
* [PATCH v6 5/5] mac80211: only set CSA beacon when at least one beacon must be transmitted
From: Luciano Coelho @ 2013-12-03 13:48 UTC (permalink / raw)
To: linux-wireless, sw, yeohchunyeow; +Cc: yeohchunyeow
In-Reply-To: <1386078502-19845-1-git-send-email-luciano.coelho@intel.com>
A beacon should never have a Channel Switch Announcement information
element with a count of 0, because a count of 1 means switch just
before the next beacon. So, if a count of 0 was valid in a beacon, it
would have been transmitted in the next channel already, which is
useless. A CSA count equal to zero is only meaningful in action
frames or probe_responses.
Fix the ieee80211_csa_is_complete() and ieee80211_update_csa()
functions accordingly.
With a CSA count of 0, we won't transmit any CSA beacons, because the
switch will happen before the next TBTT. To avoid extra work and
potential confusion in the drivers, complete the CSA immediately,
instead of waiting for the driver to call ieee80211_csa_finish().
To keep things simpler, we also switch immediately when the CSA count
is 1, while in theory we should delay the switch until just before the
next TBTT.
Additionally, move the ieee80211_csa_finish() function to cfg.c,
where it makes more sense.
Signed-off-by: Luciano Coelho <luciano.coelho@intel.com>
---
include/net/mac80211.h | 10 ++--
net/mac80211/cfg.c | 114 ++++++++++++++++++++++++++++++++++-----------
net/mac80211/ibss.c | 6 ---
net/mac80211/ieee80211_i.h | 3 +-
net/mac80211/mesh.c | 6 +--
net/mac80211/tx.c | 19 +++-----
6 files changed, 102 insertions(+), 56 deletions(-)
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 73d99bc..a72d408 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -2717,11 +2717,13 @@ enum ieee80211_roc_type {
* @channel_switch_beacon: Starts a channel switch to a new channel.
* Beacons are modified to include CSA or ECSA IEs before calling this
* function. The corresponding count fields in these IEs must be
- * decremented, and when they reach zero the driver must call
+ * decremented, and when they reach 1 the driver must call
* ieee80211_csa_finish(). Drivers which use ieee80211_beacon_get()
* get the csa counter decremented by mac80211, but must check if it is
- * zero using ieee80211_csa_is_complete() after the beacon has been
+ * 1 using ieee80211_csa_is_complete() after the beacon has been
* transmitted and then call ieee80211_csa_finish().
+ * If the CSA count starts as zero or 1, this function will not be called,
+ * since there won't be any time to beacon before the switch anyway.
*
* @join_ibss: Join an IBSS (on an IBSS interface); this is called after all
* information in bss_conf is set up and the beacon can be retrieved. A
@@ -3416,13 +3418,13 @@ static inline struct sk_buff *ieee80211_beacon_get(struct ieee80211_hw *hw,
* @vif: &struct ieee80211_vif pointer from the add_interface callback.
*
* After a channel switch announcement was scheduled and the counter in this
- * announcement hit zero, this function must be called by the driver to
+ * announcement hits 1, this function must be called by the driver to
* notify mac80211 that the channel can be changed.
*/
void ieee80211_csa_finish(struct ieee80211_vif *vif);
/**
- * ieee80211_csa_is_complete - find out if counters reached zero
+ * ieee80211_csa_is_complete - find out if counters reached 1
* @vif: &struct ieee80211_vif pointer from the add_interface callback.
*
* This function returns whether the channel switch counters reached zero.
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 790c15d..0ebce55 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -1,4 +1,4 @@
-/*
+ /*
* mac80211 configuration hooks for cfg80211
*
* Copyright 2006-2010 Johannes Berg <johannes@sipsolutions.net>
@@ -2982,21 +2982,19 @@ cfg80211_beacon_dup(struct cfg80211_beacon_data *beacon)
return new_beacon;
}
-void ieee80211_csa_finalize_work(struct work_struct *work)
+void ieee80211_csa_finish(struct ieee80211_vif *vif)
{
- struct ieee80211_sub_if_data *sdata =
- container_of(work, struct ieee80211_sub_if_data,
- csa_finalize_work);
- struct ieee80211_local *local = sdata->local;
- int err, changed = 0;
+ struct ieee80211_sub_if_data *sdata = vif_to_sdata(vif);
- sdata_lock(sdata);
- /* AP might have been stopped while waiting for the lock. */
- if (!sdata->vif.csa_active)
- goto unlock;
+ ieee80211_queue_work(&sdata->local->hw,
+ &sdata->csa_finalize_work);
+}
+EXPORT_SYMBOL(ieee80211_csa_finish);
- if (!ieee80211_sdata_running(sdata))
- goto unlock;
+static void ieee80211_csa_finalize(struct ieee80211_sub_if_data *sdata)
+{
+ struct ieee80211_local *local = sdata->local;
+ int err, changed = 0;
sdata->radar_required = sdata->csa_radar_required;
err = ieee80211_vif_change_channel(sdata, &changed);
@@ -3048,6 +3046,29 @@ unlock:
sdata_unlock(sdata);
}
+void ieee80211_csa_finalize_work(struct work_struct *work)
+{
+ struct ieee80211_sub_if_data *sdata =
+ container_of(work, struct ieee80211_sub_if_data,
+ csa_finalize_work);
+
+ sdata_lock(sdata);
+ /* AP might have been stopped while waiting for the lock. */
+ if (!sdata->vif.csa_active)
+ goto unlock;
+
+ if (!ieee80211_sdata_running(sdata))
+ goto unlock;
+
+ if (!ieee80211_sdata_running(sdata))
+ return;
+
+ ieee80211_csa_finalize(sdata);
+
+unlock:
+ sdata_unlock(sdata);
+}
+
int ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev,
struct cfg80211_csa_settings *params)
{
@@ -3056,7 +3077,7 @@ int ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev,
struct ieee80211_chanctx_conf *chanctx_conf;
struct ieee80211_chanctx *chanctx;
struct ieee80211_if_mesh __maybe_unused *ifmsh;
- int err, num_chanctx;
+ int err, num_chanctx, changed = 0;
lockdep_assert_held(&sdata->wdev.mtx);
@@ -3097,19 +3118,40 @@ int ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev,
switch (sdata->vif.type) {
case NL80211_IFTYPE_AP:
- sdata->csa_counter_offset_beacon =
- params->counter_offset_beacon;
- sdata->csa_counter_offset_presp = params->counter_offset_presp;
sdata->u.ap.next_beacon =
cfg80211_beacon_dup(¶ms->beacon_after);
if (!sdata->u.ap.next_beacon)
return -ENOMEM;
+ /*
+ * With a count of 0, we don't have to wait for any
+ * TBTT before switching, so complete the CSA
+ * immediately. In theory, with a count == 1 we
+ * should delay the switch until just before the next
+ * TBTT, but that would complicate things so we switch
+ * immediately too. If we would delay the switch
+ * until the next TBTT, we would have to set the probe
+ * response here.
+ *
+ * TODO: A channel switch with count <= 1 without
+ * sending a CSA action frame is kind of useless,
+ * because the clients won't know we're changing
+ * channels. The action frame must be implemented
+ * either here or in the userspace.
+ */
+ if (params->count <= 1)
+ break;
+
+ sdata->csa_counter_offset_beacon =
+ params->counter_offset_beacon;
+ sdata->csa_counter_offset_presp = params->counter_offset_presp;
err = ieee80211_assign_beacon(sdata, ¶ms->beacon_csa);
if (err < 0) {
kfree(sdata->u.ap.next_beacon);
return err;
}
+ changed |= err;
+
break;
case NL80211_IFTYPE_ADHOC:
if (!sdata->vif.bss_conf.ibss_joined)
@@ -3137,9 +3179,16 @@ int ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev,
params->chandef.chan->band)
return -EINVAL;
- err = ieee80211_ibss_csa_beacon(sdata, params);
- if (err < 0)
- return err;
+ /* see comments and TODO in the NL80211_IFTYPE_AP block */
+ if (params->count > 1) {
+ err = ieee80211_ibss_csa_beacon(sdata, params);
+ if (err < 0)
+ return err;
+ changed |= err;
+ }
+
+ ieee80211_send_action_csa(sdata, params);
+
break;
#ifdef CONFIG_MAC80211_MESH
case NL80211_IFTYPE_MESH_POINT:
@@ -3159,12 +3208,18 @@ int ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev,
if (ifmsh->csa_role == IEEE80211_MESH_CSA_ROLE_NONE)
ifmsh->csa_role = IEEE80211_MESH_CSA_ROLE_INIT;
- err = ieee80211_mesh_csa_beacon(sdata, params,
- (ifmsh->csa_role == IEEE80211_MESH_CSA_ROLE_INIT));
- if (err < 0) {
- ifmsh->csa_role = IEEE80211_MESH_CSA_ROLE_NONE;
- return err;
+ if (params->count > 1) {
+ err = ieee80211_mesh_csa_beacon(sdata, params);
+ if (err < 0) {
+ ifmsh->csa_role = IEEE80211_MESH_CSA_ROLE_NONE;
+ return err;
+ }
+ changed |= err;
}
+
+ if (ifmsh->csa_role == IEEE80211_MESH_CSA_ROLE_INIT)
+ ieee80211_send_action_csa(sdata, params);
+
break;
#endif
default:
@@ -3181,8 +3236,13 @@ int ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev,
sdata->csa_chandef = params->chandef;
sdata->vif.csa_active = true;
- ieee80211_bss_info_change_notify(sdata, err);
- drv_channel_switch_beacon(sdata, ¶ms->chandef);
+ if (changed) {
+ ieee80211_bss_info_change_notify(sdata, changed);
+ drv_channel_switch_beacon(sdata, ¶ms->chandef);
+ } else {
+ /* if the beacon didn't change, we can finalize immediately */
+ ieee80211_csa_finalize(sdata);
+ }
return 0;
}
diff --git a/net/mac80211/ibss.c b/net/mac80211/ibss.c
index 23e035f..595902c 100644
--- a/net/mac80211/ibss.c
+++ b/net/mac80211/ibss.c
@@ -516,12 +516,6 @@ int ieee80211_ibss_csa_beacon(struct ieee80211_sub_if_data *sdata,
if (old_presp)
kfree_rcu(old_presp, rcu_head);
- /* it might not send the beacon for a while. send an action frame
- * immediately to announce the channel switch.
- */
- if (csa_settings)
- ieee80211_send_action_csa(sdata, csa_settings);
-
return BSS_CHANGED_BEACON;
out:
return ret;
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index cf38a74..9b5f1bc 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -1400,8 +1400,7 @@ void ieee80211_mesh_work(struct ieee80211_sub_if_data *sdata);
void ieee80211_mesh_rx_queued_mgmt(struct ieee80211_sub_if_data *sdata,
struct sk_buff *skb);
int ieee80211_mesh_csa_beacon(struct ieee80211_sub_if_data *sdata,
- struct cfg80211_csa_settings *csa_settings,
- bool csa_action);
+ struct cfg80211_csa_settings *csa_settings);
int ieee80211_mesh_finish_csa(struct ieee80211_sub_if_data *sdata);
/* scan/BSS handling */
diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c
index 5476ad9..e604b9e 100644
--- a/net/mac80211/mesh.c
+++ b/net/mac80211/mesh.c
@@ -1064,8 +1064,7 @@ int ieee80211_mesh_finish_csa(struct ieee80211_sub_if_data *sdata)
}
int ieee80211_mesh_csa_beacon(struct ieee80211_sub_if_data *sdata,
- struct cfg80211_csa_settings *csa_settings,
- bool csa_action)
+ struct cfg80211_csa_settings *csa_settings)
{
struct ieee80211_if_mesh *ifmsh = &sdata->u.mesh;
struct mesh_csa_settings *tmp_csa_settings;
@@ -1089,9 +1088,6 @@ int ieee80211_mesh_csa_beacon(struct ieee80211_sub_if_data *sdata,
return ret;
}
- if (csa_action)
- ieee80211_send_action_csa(sdata, csa_settings);
-
return BSS_CHANGED_BEACON;
}
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 6d59e21..b86a679 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -2397,15 +2397,6 @@ static int ieee80211_beacon_add_tim(struct ieee80211_sub_if_data *sdata,
return 0;
}
-void ieee80211_csa_finish(struct ieee80211_vif *vif)
-{
- struct ieee80211_sub_if_data *sdata = vif_to_sdata(vif);
-
- ieee80211_queue_work(&sdata->local->hw,
- &sdata->csa_finalize_work);
-}
-EXPORT_SYMBOL(ieee80211_csa_finish);
-
static void ieee80211_update_csa(struct ieee80211_sub_if_data *sdata,
struct beacon_data *beacon)
{
@@ -2434,8 +2425,12 @@ static void ieee80211_update_csa(struct ieee80211_sub_if_data *sdata,
if (WARN_ON(counter_offset_beacon >= beacon_data_len))
return;
- /* warn if the driver did not check for/react to csa completeness */
- if (WARN_ON(beacon_data[counter_offset_beacon] == 0))
+ /* Warn if the driver did not check for/react to csa
+ * completeness. A beacon with CSA counter set to 0 should
+ * never occur, because a counter of 1 means switch just
+ * before the next beacon.
+ */
+ if (WARN_ON(beacon_data[counter_offset_beacon] == 1))
return;
beacon_data[counter_offset_beacon]--;
@@ -2501,7 +2496,7 @@ bool ieee80211_csa_is_complete(struct ieee80211_vif *vif)
if (WARN_ON(counter_beacon > beacon_data_len))
goto out;
- if (beacon_data[counter_beacon] == 0)
+ if (beacon_data[counter_beacon] == 1)
ret = true;
out:
rcu_read_unlock();
--
1.8.4.2
^ permalink raw reply related
* Re: RTL8187 bugs
From: Nikita N. @ 2013-12-03 13:59 UTC (permalink / raw)
To: Larry Finger, linux-wireless
In-Reply-To: <529A36C5.4060209@lwfinger.net>
Hi Larry :) Im afraid I didnt receive your answer to my last email.. in
case you sent, please resend, thanks :)
As for "dissecting" the issue, as you suggested, I have been filling
your code with debug lines.
Very interesting in my opinion is the report from
drivers/net/wireless/rtl818x/rtl8187/dev.c#rtl8187_configure_filter(),
as follows:
IN>changed_flags=240,total_flags=2147483888,multicast=0,priv->rx_conf=2425682954
>FIF_CONTROL
>FIF_OTHER_BSS
>not FIF_ALLMULTI
drivers/net/wireless/rtl818x/rtl8187/dev.c#rtl818x_iowrite32_async
drivers/net/wireless/rtl818x/rtl8187/dev.c#rtl8187_iowrite_async
OUT>total_flags=96,priv->rx_conf=2426207243
.. after, some daemon calls it again multiple times, but result is
always the same as follows:
IN>changed_flags=144,total_flags=2147483888,multicast=0,priv->rx_conf=2426207243
>not FIF_ALLMULTI
drivers/net/wireless/rtl818x/rtl8187/dev.c#rtl818x_iowrite32_async
drivers/net/wireless/rtl818x/rtl8187/dev.c#rtl8187_iowrite_async
net/mac80211/util.c#ieee80211_queue_work
OUT>total_flags=96,priv->rx_conf=2426207243
.. seeing anything unusual?
I have the strange feeling that "IN" value of priv->rx_conf could be
bugged.. as result the relative OUT value is maybe wrong.. :P
Anyway, could you please perform the same test on your working
interfaces, and report me the output?
The values of IN/OUT and so on, of rtl8187_configure_filter(), *AFTER*
calling "airmon-ng start wlan0"?
>From your probe, my interface type is as follow:
- "RTL8187vB (default)"
- PCI_EEPROM_WIDTH_93C46
- customer id 0
Please insert the following debug lines, as I did:
static void rtl8187_configure_filter()
{
printk(KERN_WARNING
"drivers/net/wireless/rtl818x/rtl8187/dev.c#rtl8187_configure_filter");
struct rtl8187_priv *priv = dev->priv;
printk(KERN_WARNING
"IN>changed_flags=%u,total_flags=%u,multicast=%llu,priv->rx_conf=%u",changed_flags,*total_flags,multicast,priv->rx_conf);
if (changed_flags & FIF_FCSFAIL) {
priv->rx_conf ^= RTL818X_RX_CONF_FCS;
printk(KERN_WARNING ">FIF_FCSFAIL");}
if (changed_flags & FIF_CONTROL) {
priv->rx_conf ^= RTL818X_RX_CONF_CTRL;
printk(KERN_WARNING ">FIF_CONTROL");}
if (changed_flags & FIF_OTHER_BSS) {
priv->rx_conf ^= RTL818X_RX_CONF_MONITOR;
printk(KERN_WARNING ">FIF_OTHER_BSS");}
if (*total_flags & FIF_ALLMULTI || multicast > 0) {
priv->rx_conf |= RTL818X_RX_CONF_MULTICAST;
printk(KERN_WARNING ">FIF_ALLMULTI");}
else {
priv->rx_conf &= ~RTL818X_RX_CONF_MULTICAST;
printk(KERN_WARNING ">not FIF_ALLMULTI");}
rtl818x_iowrite32_async(priv, &priv->map->RX_CONF,
priv->rx_conf);
printk(KERN_WARNING
"OUT>total_flags=%u,priv->rx_conf=%u",*total_flags,priv->rx_conf);
}
If you get different outputs also between your interfaces, please send
me all different outputs!
thanks :)
On Sat, Nov 30, 2013, at 11:04 AM, Larry Finger wrote:
> If you want to send me one of your devices, I'll check it out. Otherwise,
> there
> is little I can do. It would be impossible to duplicate the path you have
> followed.
>
> Larry
>
>
>
--
http://www.fastmail.fm - A fast, anti-spam email service.
^ 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