Re: pull-request: mac80211-next 2016-06-09

[Johannes Berg]

On Fri, 2016-06-10 at 23:16 -0700, David Miller wrote:

> > Can I ask you to pull net into net-next, preferably before merging
> > this?

> Looks good, pulled, thanks!

Thanks! I see you also had pulled net in for other reasons, so I'll
include the hwsim namespace patch in the next pull request :)

johannes

Re: [PATCH] mac80211_hwsim: Allow wmediumd to attach to radios created in its netns

[Johannes Berg]

I was about to apply this (with a typo fix for "responsile"), but
noticed these messages:

>   printk(KERN_DEBUG "mac80211_hwsim: received a REGISTER, "
>      "switching to wmediumd mode with pid %d\n", info-
> >snd_portid);


> + if (notify->portid == hwsim_net_get_wmediumd(notify->net)) {
>   printk(KERN_INFO "mac80211_hwsim: wmediumd released
> netlink"
>      " socket, switching to perfect channel
> medium\n");
> 

I wonder if we can do something better about them? Or perhaps if we
should remove them, so other namespaces won't mess up the kernel log?

johannes

Re: [PATCH] mac80211_hwsim: Allow wmediumd to attach to radios created in its netns

[Johannes Berg]

On Wed, 2016-06-15 at 14:37 +0200, Martin Willi wrote:
> > 
> > 
> > >  printk(KERN_INFO "mac80211_hwsim: wmediumd released netlink"
> > >         " socket, switching to perfect channel medium\n");
> 
> > I wonder if we can do something better about them? Or perhaps if we
> > should remove them, so other namespaces won't mess up the kernel
> > log
> 
> This is in fact not very nice, but not specific to hwsim. Any
> namespace
> can mess up the kernel log from different (networking) subsystems.
> This
> has been discussed some time ago [1], but AFAIK there is no real
> solution so far.
> 
> For this patch I think we have the following options:
>  * Keep the printk() messages as proposed
>  * Remove those callable from non-initial namespaces completely
>  * Suppress them when called from non-initial namespaces
>  * Include the associated "netgroup" in the message
> 
> I personally would prefer the first option, as this problem is not
> specific to hwsim or mac80211, but many subsystems. So we certainly
> can
> add some work-around, but there is not much to gain if other modules
> don't.
> 

Fair enough.

johannes

Re: [PATCH] mac80211_hwsim: Add missing check for HWSIM_ATTR_SIGNAL

[Johannes Berg]

On Fri, 2016-05-13 at 12:41 +0200, Martin Willi wrote:
> A wmediumd that does not send this attribute causes a NULL pointer
> dereference, as the attribute is accessed even if it does not exist.
> 
> The attribute was required but never checked ever since userspace
> frame
> forwarding has been introduced. The issue gets more problematic once
> we
> allow wmediumd registration from user namespaces.
> 
Applied, thanks.

johannes

pull-request: mac80211 2016-06-01

[Johannes Berg]

Hi Dave,

For now, I have just three fixes for the current cycle.

One of them, the hwsim one, becomes interesting with some pending
work to make hwsim work for unprivileged users in namespaces, and
I don't want to allow them to crash the system ... I'll thus wait
with that patch until you pull this into net-next also, so I can
pull it back for mac80211-next.

Let me know if there's any problem.

Thanks,
johannes



The following changes since commit 0e6b5259824e97a0f7e7b450421ff12865d3b0e2:

  net: l2tp: Make l2tp_ip6 namespace aware (2016-05-30 00:03:53 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211.git 
tags/mac80211-for-davem-2016-06-01

for you to fetch changes up to 6fe04128f158c5ad27e7504bfdf1b12e63331bc9:

  mac80211: fix fast_tx header alignment (2016-05-31 12:14:04 +0200)


Three small fixes for the current cycle:
 * missing netlink attribute check in hwsim wmediumd (Martin)
 * fast xmit structure alignment fix (Felix)
 * mesh path flush/synchronisation fix (Bob)


Bob Copeland (1):
  mac80211: mesh: flush mesh paths unconditionally

Felix Fietkau (1):
  mac80211: fix fast_tx header alignment

Martin Willi (1):
  mac80211_hwsim: Add missing check for HWSIM_ATTR_SIGNAL

 drivers/net/wireless/mac80211_hwsim.c | 1 +
 net/mac80211/mesh.c   | 4 
 net/mac80211/sta_info.h   | 2 +-
 3 files changed, 6 insertions(+), 1 deletion(-)

Re: [PATCH v2 03/10] nl80211: Prefer ether_addr_copy

[Johannes Berg]

> The requirement is to be __aligned(2). I've added 4 instances of
> ether_addr_copy with 8 addresses as arguments.  Of these, the 4
> src arguments are really the same type (i.e. nla_data acting on a
> const nlattr*), so I'll try to reason about the 5 total cases below -
> 1. cfg->dst_mac should be 16-bit aligned due to the layout of
> struct cfg80211_wowlan_tcp. Its offset is 10 or 12 bytes in the
> structure depending on the system.

I wouldn't want to rely on that, since internal structures can be
changed pretty easily. If necessary, add __aligned(2) to the struct
members where appropriate.

> 2 and 3. For mac_addr and mac_addr_mask, nl80211_parse_random_mac
> takes these in as u8* (and hence does not guarantee alignment?)
> Both the callers of this function today pass in arguments that are
> explicitly __aligned(2). But this cannot be said of future potential
> callers
> - so perhaps my patch introduces a bug?

That should be documented, but it's also pretty tricky to
review/maintain that.

I general, I don't really see much reason to use ether_addr_copy() in
any of these code paths - it's not really a performance thing (in part
due to the alignment, ether_addr_copy can be faster)

> 4. Based on struct cfg80211_acl_data, acl->mac_addrs[i] should be not
> guaranteed to be __aligned(2).

See above.

> 5. For all the nla_data src arguments, the nla_data function returns
> ((char*) foo + 5) for pointer foo. So likely not __aligned(2).

Those should be aligned since the data is copied into an SKB, and +5
doesn't seem right - nla_data() should be +4.

johannes

Re: [PATCH 1/3] nl80211: Fix checkpatch warnings

[Johannes Berg]

On Mon, 2016-05-30 at 10:30 +1000, Julian Calaby wrote:
> Hi All,
> 
> On Sun, May 29, 2016 at 1:30 PM, Kirtika Ruchandani
>  wrote:
> > 
> > This patch fixes the following checkpatch.pl warnings about
> > comments in nl80211.c :
> > - networking block comments don't use an empty '/*' line
> > - block comments use a trailing '*/' on a separate line
> > 
> > Signed-off-by: Kirtika Ruchandani 
> The change and logic behind it are sound, so it gets my:
> 
> Reviewed-by: Julian Calaby 
> 
> however I'm concerned that this file is a deliberate exception to the
> networking comment rules.
> 

It's kinda mixed, I never really enforced one style or the other ...
I'm kinda taking both now, with a slight preference towards the
networking style perhaps (to please davem :) )

That said, in general I'm not really sure of the value of all of these
patches - perhaps the kcalloc() one makes sense, not for checkpatch
reasons but rather for size limit/integer overflow reasons, and the
ether address assign for general readability, but overall ...

I'll have to make up my mind :)

johannes

Re: [PATCH 3/3] mac80211: mesh: Add support for HW RC implementation

[Johannes Berg]

> - if (sta->mesh->fail_avg >= 100)
> - return MAX_METRIC;
> + /* try to get rate based on HW RC algorithm */
> + rate = drv_get_expected_throughput(local, >sta);

This doesn't look correct, since you should use the rate control op if
available, to get data from rate control algorithms.

You should call sta_set_sinfo(), but that's far too much, so more
likely you should factor out the last few lines of that into a new
function and call that new function in both places.

johannes

Re: [RESEND PATCH 1/3] rfkill: Create "rfkill-airplane-mode" LED trigger

[Johannes Berg]

On Mon, 2016-06-13 at 23:21 +0200, Pavel Machek wrote:
> 
> (Actually, "::wifi" is super confusing, I'd expect that to be a led
> that blinks when wifi is active.)

Agree with that, yeah, that'd be confusing.

> Well... "airplane" is quite confusing. I'd we use "rfkill" for
> disabling radios, and I believe we should stick with that.
> 
> But small problem might be polarity. You may need both "::rfkill" and
> "::not-rfkill".

I'd argue that "airplane" matches better what you're likely to find on
the chassis of the system.

In either case I think the suffixes should be documented, for both
future kernel and application developers.

johannes

Re: [PATCH] wlcore: time sync : add support for 64 bit clock

[Johannes Berg]

On Thu, 2016-06-23 at 14:12 +0300, Yaniv Machani wrote:
> Changed the configuration to support 64bit instead of 32bit
> this in order to offload the driver from handling a wraparound.

[...]

Since you Cc'ed me, and presumably want me to review it, I'll say that
this looks like a terrible idea:

> @@ -74,10 +74,16 @@ struct wl18xx_event_mailbox {

This struct is evidently used for firmware/host communication.

>   __le16 bss_loss_bitmap;
>  
>   /* bitmap of stations (by HLID) which exceeded max tx
> retries */
> - __le32 tx_retry_exceeded_bitmap;
> + __le16 tx_retry_exceeded_bitmap;
> +
> + /* time sync high msb*/
> + u16 time_sync_tsf_high_msb;

So first of all, just using u16 instead of __le16 seems wrong.

Additionally, this looks like it changes the firmware API, so that
older firmware images will no longer work?

johannes

Re: [PATCH] wlcore: time sync : add support for 64 bit clock

[Johannes Berg]

> > Additionally, this looks like it changes the firmware API, so that 
> > older firmware images will no longer work?
> 
> It is backwards compatible, 
> although it changes a API structure, older firmware are using only
> u16 for the field so there is no impact on that.
> 

Oh, ok. I had also thought that the size changed, but missed that you
replaced a u32 with two u16. Thanks for checking :)

johannes

Re: [PATCH] cfg80211/nl80211: add wifi tx power mode switching support

[Johannes Berg]

On Thu, 2016-05-12 at 17:34 +0800, Wei-Ning Huang wrote:
> 
> Johannes, I feel like being able to set calibration data at runtime
> is something common to all wireless drivers, so instead of using
> vendor commands what do you think if I pass the calibration data name
> instead of using those magic constants? This way, userspace does not
> need to know the details of what band/range power limit the driver
> supports. It allows for flexible driver side implementation and
> easier for userspace to control.
> 

Sorry - I dropped this thread accidentally.

I'm not really sure I understand the situation fully, but right now to
me this seems very strange.

The physical antennas probably don't really change between "clamshell"
and "tablet" mode, do the physical radiation properties change enough
to actually require different *calibration*? To me, that sounds very
strange.

Assuming they don't really change fundamentally, then I understand the
need to set different power levels, per band/channel/whatever
granularity. But that can be achieved in very different ways, and in
fact if you look at Chrome then for our iwl7000 driver there we do have
a command to do something similar (currently a vendor command, but that
can be changed) without ever changing the *calibration*.

So to me, the whole premise of the patch is confusing and/or wrong.

johannes

Re: [PATCH] nl80211: improve nl80211_parse_mesh_config type checking

[Johannes Berg]

On Wed, 2016-06-15 at 22:29 +0200, Arnd Bergmann wrote:
> When building a kernel with W=1, the nl80211.c file causes a number
> of
> warnings, all about the same problem:
> 
> net/wireless/nl80211.c: In function 'nl80211_parse_mesh_config':
> net/wireless/nl80211.c:5287:103: error: comparison is always false
> due to limited range of data type [-Werror=type-limits]
> net/wireless/nl80211.c:5290:96: error: comparison is always false due
> to limited range of data type [-Werror=type-limits]
> net/wireless/nl80211.c:5293:124: error: comparison is always false
> due to limited range of data type [-Werror=type-limits]
> net/wireless/nl80211.c:5295:148: error: comparison is always false
> due to limited range of data type [-Werror=type-limits]
> net/wireless/nl80211.c:5298:106: error: comparison is always false
> due to limited range of data type [-Werror=type-limits]
> net/wireless/nl80211.c:5305:116: error: comparison is always false
> due to limited range of data type [-Werror=type-limits]
> 
> The problem is that gcc does not notice that the check is generate
> by a macro, so it complains about comparing an unsigned type against
> 0.
> 
> I've tried to come up with a way to rephrase that code in a way that
> avoids the warnings and otherwise improves the code as well.
> 
> This uses a set of new helper functions that perform the range
> checking,
> and should provide slightly better type safety than the older patch,
> at the expense of adding 44 lines to the code. Binary code size is
> basically unchanged though (20 bytes added to 126561 bytes .text).
> 
Applied.

johannes

Re: [PATCH] mac80211_hwsim: Added vendor echo command

[Johannes Berg]

On Sat, 2016-06-25 at 21:32 +0300, Jouni Malinen wrote:
> 
> All you need to do is to prepare a hostap.git contribution that
> requests a new subcommand/attribute to be assigned and once that gets
> applied to the hostap.git master branch, the values have been
> assigned and can be used for whatever purpose they were assigned,
> e.g., in mac80211_hwsim.
> 

Assuming, of course, that there's actually a point in the original
patch. I haven't quite figured out any reason to actually have this
facility.

johannes

pull-request: mac80211 2016-06-29

[Johannes Berg]

Hi Dave,

Another (pretty old) bug showed up, and I have a single fix for it
from Jouni; would be nice to still get it in, but it's in mesh which
seems to mostly have users who integrate everything themselves.

Let me know if there's any problem.

Thanks,
johannes



The following changes since commit 3d5fdff46c4b2b9534fa2f9fc78e90a48e0ff724:

  wext: Fix 32 bit iwpriv compatibility issue with 64 bit Kernel (2016-06-09 
09:56:11 +0200)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211.git 
tags/mac80211-for-davem-2016-06-29

for you to fetch changes up to 126e7557328a1cd576be4fca95b133a2695283ff:

  mac80211: Fix mesh estab_plinks counting in STA removal case (2016-06-28 
12:39:50 +0200)


A single fix:
 * fix mesh peer link counter, decrement wasn't always done at all


Jouni Malinen (1):
  mac80211: Fix mesh estab_plinks counting in STA removal case

 net/mac80211/mesh.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

Re: [PATCH 3/4] mac80211: mesh: fixed HT ies in beacon template

[Johannes Berg]

On Tue, 2016-06-28 at 14:13 +0300, Yaniv Machani wrote:
> 
>  net/mac80211/mesh.c | 33 -
>  net/mac80211/util.c |  3 ---
>  net/wireless/mesh.c |  2 +-

That's not a good patch - one change is mac80211 and the other
cfg80211.

> - .ht_opmode = IEEE80211_HT_OP_MODE_PROTECTION_NONHT_MIXED,
> + .ht_opmode = IEEE80211_HT_OP_MODE_PROTECTION_NONE,
> 
How are you planning to comply with 802.11 now?


The HT Protection field in a mesh STA may be set to no protection mode
only if
— All STAs detected in the primary or the secondary channel are HT
  STAs, and
— All mesh STA members of this MBSS that are one-hop neighbors of the
  transmitting mesh STA are either:
  — 20/40 MHz HT mesh STAs in a 20/40 MHz MBSS, or
  — 20 MHz HT mesh STAs in a 20 MHz MBSS.

johannes

Re: [PATCH 1/4] mac80211: mesh: flush stations before beacons are stopped

[Johannes Berg]

On Tue, 2016-06-28 at 14:13 +0300, Yaniv Machani wrote:
> From: Maital Hahn 
> 
> Some drivers (e.g. wl18xx) expect that the last stage in the
> de-initialization process will be stopping the beacons, similar to
> ap. Update ieee80211_stop_mesh() flow accordingly.
> 
How well have you tested that with other drivers?

Changing behaviour to something a single driver desires isn't
necessarily the best thing to do, since there always are multiple
drivers.

If you're able to demonstrate that it works with the other drivers I'm
willing to take that - the change makes sense after all, and it seems
drivers must support this ordering since peers are also removed
dynamically... But still. Don't just make a change like that without
even giving any indication why you think it's fine for other drivers!

johannes

Re: [PATCH] mac80211: util: mesh is not connected properly after recovery

[Johannes Berg]

On Tue, 2016-06-28 at 14:15 +0300, Yaniv Machani wrote:
> From: Maital Hahn 
> 
> In the reconfigure process for mesh interface, moved the
> reconfiguration
> of the mesh peers to be done only after restarting the beacons,
> the same as it is done for AP.
> 
> Signed-off-by: Maital Hahn 
> Acked-by: Yaniv Machani 
> 
Same here, and this also needs a description of why this is OK with
other drivers, since presumably it already works there.

johannes

Re: [PATCH] mac80211: util: mesh is not connected properly after recovery

[Johannes Berg]

Also - your subject lines should explain the *fix*, not the *bug*

johannes

Re: [PATCH] mac80211: rx: frames received out of order

[Johannes Berg]

On Tue, 2016-06-28 at 14:15 +0300, Yaniv Machani wrote:
> From: Meirav Kama 
> 
> MP received data frames from another MP. Frames are forwarded
> from Rx to Tx to be transmitted to a third MP.
> Upon cloning the skb, the tx_info was zeroed, and the
> hw_queue wasn't set correctly, causing frames to be
> inserted to queue 0 (VOICE). If re-queue occurred for some
> reason, frame will be inserted to correct queue 2 (BE).
> In this case frames are now dequeued from 2 different queues and
> sent out of order.

Please rewrite this commit log in active voice, and explain the fix
too.

> Signed-off-by: Meirav Kama 
> Acked-by: Yaniv Machani 

See Julian's comment.

johannes

Re: [PATCH 8/9] rfkill: Userspace control for airplane mode

[Johannes Berg]

On 2016-02-10 17:53, Dan Williams wrote:

Yeah, I get that now.  It's just that to me, something called
"AIRPLANE_MODE_CHANGE" seems like it should actually change airplane
mode on/off, which implies killing radios.  I wouldn't have had the
problem if it was named AIRPLANE_MODE_INDICATOR_CHANGE, which makes it
clear this is simply an indicator and has no actual effect on anything
other than a LED.


Yeah, I agree. I'm not even sure that it's a good idea to subsume this 
into the regular operations in the API, although that's obviously the 
easiest extension. I'll need to think about that a bit more with the 
code at hand though.


johannes

Re: [PATCH 8/9] rfkill: Userspace control for airplane mode

[Johannes Berg]

On Mon, 2016-02-08 at 10:11 -0600, Dan Williams wrote:
> I'd like to clarify a bit, so tell me if I'm correct or not.  Using
> RFKILL_OP_AIRPLANE_MODE_CHANGE does not actually change any device
> state. It's just an indicator with no relationship to any of the
> registered rfkill switches, right?

Yes. I'm not sure I'm totally happy with this userspace API.

> I wonder if setting RFKILL_OP_AIRPLANE_MODE_CHANGE(true) shouldn't
> also
> softblock all switches, otherwise you can set airplane mode all day
> long with RFKILL_OP_AIRPLANE_MODE_CHANGE and it doesn't actually
> enable
> airplane mode at all?

No, this is kinda the point that you could implement your policy in
userspace now.

johannes

[PATCH v3 4/4] ipv6: add option to drop unsolicited neighbor advertisements

[Johannes Berg]

From: Johannes Berg <johannes.b...@intel.com>

In certain 802.11 wireless deployments, there will be NA proxies
that use knowledge of the network to correctly answer requests.
To prevent unsolicitd advertisements on the shared medium from
being a problem, on such deployments wireless needs to drop them.

Enable this by providing an option called "drop_unsolicited_na".

Signed-off-by: Johannes Berg <johannes.b...@intel.com>
---
 Documentation/networking/ip-sysctl.txt | 7 +++
 include/linux/ipv6.h   | 1 +
 include/uapi/linux/ipv6.h  | 1 +
 net/ipv6/addrconf.c| 8 
 net/ipv6/ndisc.c   | 9 +
 5 files changed, 26 insertions(+)

diff --git a/Documentation/networking/ip-sysctl.txt 
b/Documentation/networking/ip-sysctl.txt
index 428fb48a19fc..77992f1173c3 100644
--- a/Documentation/networking/ip-sysctl.txt
+++ b/Documentation/networking/ip-sysctl.txt
@@ -1672,6 +1672,13 @@ drop_unicast_in_l2_multicast - BOOLEAN
 
By default this is turned off.
 
+drop_unsolicited_na - BOOLEAN
+   Drop all unsolicited neighbor advertisements, for example if there's
+   a known good NA proxy on the network and such frames need not be used
+   (or in the case of 802.11, must not be used to prevent attacks.)
+
+   By default this is turned off.
+
 icmp/*:
 ratelimit - INTEGER
Limit the maximal rates for sending ICMPv6 packets.
diff --git a/include/linux/ipv6.h b/include/linux/ipv6.h
index 34317cb6a6fc..9231bfdc7c92 100644
--- a/include/linux/ipv6.h
+++ b/include/linux/ipv6.h
@@ -56,6 +56,7 @@ struct ipv6_devconf {
__s32   ndisc_notify;
__s32   suppress_frag_ndisc;
__s32   accept_ra_mtu;
+   __s32   drop_unsolicited_na;
struct ipv6_stable_secret {
bool initialized;
struct in6_addr secret;
diff --git a/include/uapi/linux/ipv6.h b/include/uapi/linux/ipv6.h
index 4c413570efe8..ec117b65d5a5 100644
--- a/include/uapi/linux/ipv6.h
+++ b/include/uapi/linux/ipv6.h
@@ -175,6 +175,7 @@ enum {
DEVCONF_ACCEPT_RA_MIN_HOP_LIMIT,
DEVCONF_IGNORE_ROUTES_WITH_LINKDOWN,
DEVCONF_DROP_UNICAST_IN_L2_MULTICAST,
+   DEVCONF_DROP_UNSOLICITED_NA,
DEVCONF_MAX
 };
 
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 35f880bcf626..e7dd0a0c5126 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -4673,6 +4673,7 @@ static inline void ipv6_store_devconf(struct ipv6_devconf 
*cnf,
/* we omit DEVCONF_STABLE_SECRET for now */
array[DEVCONF_USE_OIF_ADDRS_ONLY] = cnf->use_oif_addrs_only;
array[DEVCONF_DROP_UNICAST_IN_L2_MULTICAST] = 
cnf->drop_unicast_in_l2_multicast;
+   array[DEVCONF_DROP_UNSOLICITED_NA] = cnf->drop_unsolicited_na;
 }
 
 static inline size_t inet6_ifla6_size(void)
@@ -5742,6 +5743,13 @@ static struct addrconf_sysctl_table
.proc_handler   = proc_dointvec,
},
{
+   .procname   = "drop_unsolicited_na",
+   .data   = _devconf.drop_unsolicited_na,
+   .maxlen = sizeof(int),
+   .mode   = 0644,
+   .proc_handler   = proc_dointvec,
+   },
+   {
/* sentinel */
}
},
diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
index 3e0f855e1bea..12c84a53df4f 100644
--- a/net/ipv6/ndisc.c
+++ b/net/ipv6/ndisc.c
@@ -887,6 +887,7 @@ static void ndisc_recv_na(struct sk_buff *skb)
offsetof(struct nd_msg, opt));
struct ndisc_options ndopts;
struct net_device *dev = skb->dev;
+   struct inet6_dev *idev = __in6_dev_get(dev);
struct inet6_ifaddr *ifp;
struct neighbour *neigh;
 
@@ -906,6 +907,14 @@ static void ndisc_recv_na(struct sk_buff *skb)
return;
}
 
+   /* For some 802.11 wireless deployments (and possibly other networks),
+* there will be a NA proxy and unsolicitd packets are attacks
+* and thus should not be accepted.
+*/
+   if (!msg->icmph.icmp6_solicited && idev &&
+   idev->cnf.drop_unsolicited_na)
+   return;
+
if (!ndisc_parse_options(msg->opt, ndoptlen, )) {
ND_PRINTK(2, warn, "NS: invalid ND option\n");
return;
-- 
2.7.0

[PATCH v3 3/4] ipv6: add option to drop unicast encapsulated in L2 multicast

[Johannes Berg]

From: Johannes Berg <johannes.b...@intel.com>

In order to solve a problem with 802.11, the so-called hole-196 attack,
add an option (sysctl) called "drop_unicast_in_l2_multicast" which, if
enabled, causes the stack to drop IPv6 unicast packets encapsulated in
link-layer multi- or broadcast frames. Such frames can (as an attack)
be created by any member of the same wireless network and transmitted
as valid encrypted frames since the symmetric key for broadcast frames
is shared between all stations.

Reviewed-by: Julian Anastasov <j...@ssi.bg>
Signed-off-by: Johannes Berg <johannes.b...@intel.com>
---
 Documentation/networking/ip-sysctl.txt |  6 ++
 include/linux/ipv6.h   |  1 +
 include/uapi/linux/ipv6.h  |  1 +
 net/ipv6/addrconf.c|  8 
 net/ipv6/ip6_input.c   | 10 ++
 5 files changed, 26 insertions(+)

diff --git a/Documentation/networking/ip-sysctl.txt 
b/Documentation/networking/ip-sysctl.txt
index 56bb6dd881bd..428fb48a19fc 100644
--- a/Documentation/networking/ip-sysctl.txt
+++ b/Documentation/networking/ip-sysctl.txt
@@ -1666,6 +1666,12 @@ stable_secret - IPv6 address
 
By default the stable secret is unset.
 
+drop_unicast_in_l2_multicast - BOOLEAN
+   Drop any unicast IPv6 packets that are received in link-layer
+   multicast (or broadcast) frames.
+
+   By default this is turned off.
+
 icmp/*:
 ratelimit - INTEGER
Limit the maximal rates for sending ICMPv6 packets.
diff --git a/include/linux/ipv6.h b/include/linux/ipv6.h
index 0ef2a97ccdb5..34317cb6a6fc 100644
--- a/include/linux/ipv6.h
+++ b/include/linux/ipv6.h
@@ -50,6 +50,7 @@ struct ipv6_devconf {
__s32   mc_forwarding;
 #endif
__s32   disable_ipv6;
+   __s32   drop_unicast_in_l2_multicast;
__s32   accept_dad;
__s32   force_tllao;
__s32   ndisc_notify;
diff --git a/include/uapi/linux/ipv6.h b/include/uapi/linux/ipv6.h
index 38b4fef20219..4c413570efe8 100644
--- a/include/uapi/linux/ipv6.h
+++ b/include/uapi/linux/ipv6.h
@@ -174,6 +174,7 @@ enum {
DEVCONF_USE_OIF_ADDRS_ONLY,
DEVCONF_ACCEPT_RA_MIN_HOP_LIMIT,
DEVCONF_IGNORE_ROUTES_WITH_LINKDOWN,
+   DEVCONF_DROP_UNICAST_IN_L2_MULTICAST,
DEVCONF_MAX
 };
 
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index d72fa90d6feb..35f880bcf626 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -4672,6 +4672,7 @@ static inline void ipv6_store_devconf(struct ipv6_devconf 
*cnf,
array[DEVCONF_IGNORE_ROUTES_WITH_LINKDOWN] = 
cnf->ignore_routes_with_linkdown;
/* we omit DEVCONF_STABLE_SECRET for now */
array[DEVCONF_USE_OIF_ADDRS_ONLY] = cnf->use_oif_addrs_only;
+   array[DEVCONF_DROP_UNICAST_IN_L2_MULTICAST] = 
cnf->drop_unicast_in_l2_multicast;
 }
 
 static inline size_t inet6_ifla6_size(void)
@@ -5734,6 +5735,13 @@ static struct addrconf_sysctl_table
.proc_handler   = 
addrconf_sysctl_ignore_routes_with_linkdown,
},
{
+   .procname   = "drop_unicast_in_l2_multicast",
+   .data   = 
_devconf.drop_unicast_in_l2_multicast,
+   .maxlen = sizeof(int),
+   .mode   = 0644,
+   .proc_handler   = proc_dointvec,
+   },
+   {
/* sentinel */
}
},
diff --git a/net/ipv6/ip6_input.c b/net/ipv6/ip6_input.c
index 9075acf081dd..31ac3c56da4b 100644
--- a/net/ipv6/ip6_input.c
+++ b/net/ipv6/ip6_input.c
@@ -134,6 +134,16 @@ int ipv6_rcv(struct sk_buff *skb, struct net_device *dev, 
struct packet_type *pt
IPV6_ADDR_MC_SCOPE(>daddr) == 1)
goto err;
 
+   /* If enabled, drop unicast packets that were encapsulated in link-layer
+* multicast or broadcast to protected against the so-called "hole-196"
+* attack in 802.11 wireless.
+*/
+   if (!ipv6_addr_is_multicast(>daddr) &&
+   (skb->pkt_type == PACKET_BROADCAST ||
+skb->pkt_type == PACKET_MULTICAST) &&
+   idev->cnf.drop_unicast_in_l2_multicast)
+   goto err;
+
/* RFC4291 2.7
 * Nodes must not originate a packet to a multicast address whose scope
 * field contains the reserved value 0; if such a packet is received, it
-- 
2.7.0

[PATCH v3 1/4] ipv4: add option to drop unicast encapsulated in L2 multicast

[Johannes Berg]

From: Johannes Berg <johannes.b...@intel.com>

In order to solve a problem with 802.11, the so-called hole-196 attack,
add an option (sysctl) called "drop_unicast_in_l2_multicast" which, if
enabled, causes the stack to drop IPv4 unicast packets encapsulated in
link-layer multi- or broadcast frames. Such frames can (as an attack)
be created by any member of the same wireless network and transmitted
as valid encrypted frames since the symmetric key for broadcast frames
is shared between all stations.

Additionally, enabling this option provides compliance with a SHOULD
clause of RFC 1122.

Reviewed-by: Julian Anastasov <j...@ssi.bg>
Signed-off-by: Johannes Berg <johannes.b...@intel.com>
---
 Documentation/networking/ip-sysctl.txt |  7 +++
 include/uapi/linux/ip.h|  1 +
 net/ipv4/devinet.c |  2 ++
 net/ipv4/ip_input.c| 25 -
 4 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/Documentation/networking/ip-sysctl.txt 
b/Documentation/networking/ip-sysctl.txt
index 05915be86235..35c4c43dd8de 100644
--- a/Documentation/networking/ip-sysctl.txt
+++ b/Documentation/networking/ip-sysctl.txt
@@ -1208,6 +1208,13 @@ promote_secondaries - BOOLEAN
promote a corresponding secondary IP address instead of
removing all the corresponding secondary IP addresses.
 
+drop_unicast_in_l2_multicast - BOOLEAN
+   Drop any unicast IP packets that are received in link-layer
+   multicast (or broadcast) frames.
+   This behavior (for multicast) is actually a SHOULD in RFC
+   1122, but is disabled by default for compatibility reasons.
+   Default: off (0)
+
 
 tag - INTEGER
Allows you to write a number, which can be used as required.
diff --git a/include/uapi/linux/ip.h b/include/uapi/linux/ip.h
index 08f894d2ddbd..584834f7e95c 100644
--- a/include/uapi/linux/ip.h
+++ b/include/uapi/linux/ip.h
@@ -165,6 +165,7 @@ enum
IPV4_DEVCONF_IGMPV2_UNSOLICITED_REPORT_INTERVAL,
IPV4_DEVCONF_IGMPV3_UNSOLICITED_REPORT_INTERVAL,
IPV4_DEVCONF_IGNORE_ROUTES_WITH_LINKDOWN,
+   IPV4_DEVCONF_DROP_UNICAST_IN_L2_MULTICAST,
__IPV4_DEVCONF_MAX
 };
 
diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index cebd9d31e65a..dbbab28a52a4 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -2192,6 +2192,8 @@ static struct devinet_sysctl_table {
  "promote_secondaries"),
DEVINET_SYSCTL_FLUSHING_ENTRY(ROUTE_LOCALNET,
  "route_localnet"),
+   DEVINET_SYSCTL_FLUSHING_ENTRY(DROP_UNICAST_IN_L2_MULTICAST,
+ "drop_unicast_in_l2_multicast"),
},
 };
 
diff --git a/net/ipv4/ip_input.c b/net/ipv4/ip_input.c
index b1209b63381f..997ef64a1c0b 100644
--- a/net/ipv4/ip_input.c
+++ b/net/ipv4/ip_input.c
@@ -359,8 +359,31 @@ static int ip_rcv_finish(struct net *net, struct sock *sk, 
struct sk_buff *skb)
rt = skb_rtable(skb);
if (rt->rt_type == RTN_MULTICAST) {
IP_UPD_PO_STATS_BH(net, IPSTATS_MIB_INMCAST, skb->len);
-   } else if (rt->rt_type == RTN_BROADCAST)
+   } else if (rt->rt_type == RTN_BROADCAST) {
IP_UPD_PO_STATS_BH(net, IPSTATS_MIB_INBCAST, skb->len);
+   } else if (skb->pkt_type == PACKET_BROADCAST ||
+  skb->pkt_type == PACKET_MULTICAST) {
+   struct in_device *in_dev = __in_dev_get_rcu(skb->dev);
+
+   /* RFC 1122 3.3.6:
+*
+*   When a host sends a datagram to a link-layer broadcast
+*   address, the IP destination address MUST be a legal IP
+*   broadcast or IP multicast address.
+*
+*   A host SHOULD silently discard a datagram that is received
+*   via a link-layer broadcast (see Section 2.4) but does not
+*   specify an IP multicast or broadcast destination address.
+*
+* This doesn't explicitly say L2 *broadcast*, but broadcast is
+* in a way a form of multicast and the most common use case for
+* this is 802.11 protecting against cross-station spoofing (the
+* so-called "hole-196" attack) so do it for both.
+*/
+   if (in_dev &&
+   IN_DEV_ORCONF(in_dev, DROP_UNICAST_IN_L2_MULTICAST))
+   goto drop;
+   }
 
return dst_input(skb);
 
-- 
2.7.0

[PATCH v3 2/4] ipv4: add option to drop gratuitous ARP packets

[Johannes Berg]

From: Johannes Berg <johannes.b...@intel.com>

In certain 802.11 wireless deployments, there will be ARP proxies
that use knowledge of the network to correctly answer requests.
To prevent gratuitous ARP frames on the shared medium from being
a problem, on such deployments wireless needs to drop them.

Enable this by providing an option called "drop_gratuitous_arp".

Signed-off-by: Johannes Berg <johannes.b...@intel.com>
---
 Documentation/networking/ip-sysctl.txt | 6 ++
 include/uapi/linux/ip.h| 1 +
 net/ipv4/arp.c | 8 
 net/ipv4/devinet.c | 2 ++
 4 files changed, 17 insertions(+)

diff --git a/Documentation/networking/ip-sysctl.txt 
b/Documentation/networking/ip-sysctl.txt
index 35c4c43dd8de..56bb6dd881bd 100644
--- a/Documentation/networking/ip-sysctl.txt
+++ b/Documentation/networking/ip-sysctl.txt
@@ -1215,6 +1215,12 @@ drop_unicast_in_l2_multicast - BOOLEAN
1122, but is disabled by default for compatibility reasons.
Default: off (0)
 
+drop_gratuitous_arp - BOOLEAN
+   Drop all gratuitous ARP frames, for example if there's a known
+   good ARP proxy on the network and such frames need not be used
+   (or in the case of 802.11, must not be used to prevent attacks.)
+   Default: off (0)
+
 
 tag - INTEGER
Allows you to write a number, which can be used as required.
diff --git a/include/uapi/linux/ip.h b/include/uapi/linux/ip.h
index 584834f7e95c..f291569768dd 100644
--- a/include/uapi/linux/ip.h
+++ b/include/uapi/linux/ip.h
@@ -166,6 +166,7 @@ enum
IPV4_DEVCONF_IGMPV3_UNSOLICITED_REPORT_INTERVAL,
IPV4_DEVCONF_IGNORE_ROUTES_WITH_LINKDOWN,
IPV4_DEVCONF_DROP_UNICAST_IN_L2_MULTICAST,
+   IPV4_DEVCONF_DROP_GRATUITOUS_ARP,
__IPV4_DEVCONF_MAX
 };
 
diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
index 59b3e0e8fd51..c102eb5ac55c 100644
--- a/net/ipv4/arp.c
+++ b/net/ipv4/arp.c
@@ -735,6 +735,14 @@ static int arp_process(struct net *net, struct sock *sk, 
struct sk_buff *skb)
(!IN_DEV_ROUTE_LOCALNET(in_dev) && ipv4_is_loopback(tip)))
goto out;
 
+ /*
+  *For some 802.11 wireless deployments (and possibly other networks),
+  *there will be an ARP proxy and gratuitous ARP frames are attacks
+  *and thus should not be accepted.
+  */
+   if (sip == tip && IN_DEV_ORCONF(in_dev, DROP_GRATUITOUS_ARP))
+   goto out;
+
 /*
  * Special case: We must set Frame Relay source Q.922 address
  */
diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index dbbab28a52a4..3d835313575e 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -2185,6 +2185,8 @@ static struct devinet_sysctl_table {
"igmpv3_unsolicited_report_interval"),
DEVINET_SYSCTL_RW_ENTRY(IGNORE_ROUTES_WITH_LINKDOWN,
"ignore_routes_with_linkdown"),
+   DEVINET_SYSCTL_RW_ENTRY(DROP_GRATUITOUS_ARP,
+   "drop_gratuitous_arp"),
 
DEVINET_SYSCTL_FLUSHING_ENTRY(NOXFRM, "disable_xfrm"),
DEVINET_SYSCTL_FLUSHING_ENTRY(NOPOLICY, "disable_policy"),
-- 
2.7.0

Re: [PATCH 8/9] rfkill: Userspace control for airplane mode

[Johannes Berg]

On 2016-02-08 17:20, Marcel Holtmann wrote:


as explained in an earlier response, the concept Airplane Mode seems
to be imposing policy into the kernel. Do we really want have this as
a kernel exposed API.


This patch is the one that *solves* that problem ... please read it more 
carefully?


johannes

Re: WARNING at net/mac80211/rate.c:513 ieee80211_get_tx_rates [mac80211]

[Johannes Berg]

On Thu, 2016-01-28 at 19:54 -0600, Larry Finger wrote:
> 
> I have been running an RTL8821AE since kernel 3.18 without hitting
> this problem 
> using a TRENDnet AC1750 dual-band AP. The UniFi may be doing
> something that the 
> driver is not expecting.

Are you quite sure you're actually using VHT though, perhaps the AP
somehow turned it off? It seems unlikely that you could successfully
use it in any way given that RATE_INFO_FLAGS_VHT_MCS doesn't show up in
the driver or rate scaling at all.


johannes

Re: WARNING at net/mac80211/rate.c:513 ieee80211_get_tx_rates [mac80211]

[Johannes Berg]

On Fri, 2016-01-29 at 10:12 -0600, Larry Finger wrote:
> 
> Upon further inspection, my log has the line "rtl8821ae :02:00.0
> wlp2s0: disabling HT/VHT due to WEP/TKIP use". I need to fix that
> first.
> 
Likely TKIP; enable only WPA2 (CCMP) on the AP.

johannes

Re: [PATCH net-next 08/40] net: fec: move cbd_bufaddr assignment closer to the mapping function

[Johannes Berg]

On Thu, 2016-01-28 at 23:02 +0100, Arnd Bergmann wrote:
> On Thursday 28 January 2016 14:25:32 Troy Kisky wrote:
> > Signed-off-by: Troy Kisky 
> > ---
> >  drivers/net/ethernet/freescale/fec_main.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> [note: missing changelog?]
> 
> > diff --git a/drivers/net/ethernet/freescale/fec_main.c
> > b/drivers/net/ethernet/freescale/fec_main.c
> > index b87f80d..15a93f90 100644
> > --- a/drivers/net/ethernet/freescale/fec_main.c
> > +++ b/drivers/net/ethernet/freescale/fec_main.c
> > @@ -476,6 +476,8 @@ static int fec_enet_txq_submit_skb(struct
> > fec_enet_priv_tx_q *txq,
> >     estatus |= BD_ENET_TX_TS;
> >     }
> >     }
> > +   bdp->cbd_bufaddr = addr;
> > +   bdp->cbd_datlen = buflen;
> >  
> >     if (fep->bufdesc_ex) {
> >  
> > @@ -499,8 +501,6 @@ static int fec_enet_txq_submit_skb(struct
> > fec_enet_priv_tx_q *txq,
> >     /* Save skb pointer */
> >     txq->tx_skbuff[index] = skb;
> >  
> > -   bdp->cbd_datlen = buflen;
> > -   bdp->cbd_bufaddr = addr;
> >     /* Make sure the updates to rest of the descriptor are
> > performed before
> >      * transferring ownership.
> >      */
> > 
> 
> This patch and others in the series conflicts with the bugfix "net:
> fec: make driver endian-safe" that Johannes sent this week. Can you
> include his fix in your series and ensure that all descriptor
> accesses are done in an endian-safe way?
> 

My fixes are already in net.git, so you should base on a current
net.git or on net-next.git after net.git is merged there (which davem
usually does after Linus merges from him, IIRC)

johannes

Re: WARNING at net/mac80211/rate.c:513 ieee80211_get_tx_rates [mac80211]

[Johannes Berg]

On Thu, 2016-01-28 at 14:04 -0800, Linus Torvalds wrote:
> 
> Well, it "makes a difference" in the sense that the warning goes
> away.
> But it doesn't make things work. In fact, it might be making things
> worse.

Heh, ok.

> Because with that patch, the wireless still authenticates and
> associates, but then it doesn't even get an IP address, so now even
> dhcp doesn't work. Of course, I was surprised that it worked last
> time, and I'm not 100% sure it did work consistently. I'll re-test
> without the patch, just to make sure, but it doesn't really seem to
> improve on anything.
> 

It makes some sense, here's some speculation:

VHT rates are MCS 0-9. If the rate scaling decides to use only VHT
MCSes with a VHT-capable peer, then it stands to reason it might still
start at 0, but forget to set the VHT_MCS flag, so it would really use
rate index 0 from the table, which is 6 MBps. Then, it would see that
"working" (since it's not the right thing) and scale up until it hits
MCS 8 or 9, which is no longer a valid rate (those are only 0-7).

Since the suggested changes make it worse, we can assume that this is
not the only place where VHT is simply completely broken, and fixing
VHT here will instead uncover a bug elsewhere, that was previously not
happening because we never got to real VHT rates.

Your best workaround may just be to ignore VHT for now - clearly it's
broken so using "just" HT (which is likely not that much of a penalty
anyway since you're apparently not using 80 MHz) will be much better.

Go into

_rtl_init_hw_vht_capab()

and just remove or stub out the entire contents of that (or you could
just remove the "vht_supported=true" if you feel like it.)

That should get it to HT only, which is likely tested and working
better.

johannes

Re: WARNING at net/mac80211/rate.c:513 ieee80211_get_tx_rates [mac80211]

[Johannes Berg]

On Thu, 2016-01-28 at 11:01 -0800, Linus Torvalds wrote:
> 
> I used to have the basic original UniFi UAP. I've replaced them with
> the newer "AC Lite" version:
> 
> https://www.ubnt.com/unifi/unifi-ap-ac-lite/
> 
> so it's a fairly big jump from a 2.4GHz-only network to a dual-band
> one.
> 
> The old 2.4GHz-only AP's showed the problem with minstrel-ht
> incorrectly starting off at the highest rate (on a totally different
> machine). So the Unifi AP's have shown problems in the kernel
> wireless before, but so far it's always been the fault of the kernel
> wireless, not the AP.

Yeah; I wasn't trying to blame it on this change, I was just trying to
understand the change in the environment. Seems likely that it's simply
the switch to 5 GHz, which is strange, I'd have thought that even that
rtlwifi driver would've been tested with that :)

> > Could you print out the entire table there when the warning
> > happens?
> 
> This is the best I can come up with: printing out the index, and the
> rate and bitrate tables:
> 
>   rates[i].idx (9) >= sband->n_bitrates (8)
>   Rates:
>   0: idx 9 count 1 flags a0
>   1: idx 8 count 1 flags a0
>   2: idx 7 count 2 flags a0
>   3: idx 6 count 3 flags a0

Yeah, perfect. See, this is already evidently not making any sense:

flags a0 is
IEEE80211_TX_RC_40_MHZ_WIDTH | IEEE80211_TX_RC_SHORT_GI

both of those options *require* IEEE80211_TX_RC_MCS or
IEEE80211_TX_RC_VHT_MCS as well, so the flags really should be a8 or
1a0.

>   Bitrates:
>   0: flags 0002 bitrate 60 (hw: 0004 )
>   1: flags  bitrate 90 (hw: 0005 )
>   2: flags 0002 bitrate 120 (hw: 0006 )
>   3: flags  bitrate 180 (hw: 0007 )
>   4: flags 0002 bitrate 240 (hw: 0008 )
>   5: flags  bitrate 360 (hw: 0009 )
>   6: flags  bitrate 480 (hw: 000a )
>   7: flags  bitrate 540 (hw: 000b )
> 
> So it's the very first rate that has index 9, but the bitrate table
> only goes from 0-7.
> 
> So I suspect that once the first index has been marked invalid, it
> now will never even look at the later indices, so it has no transmit
> rates at all.  Or something.

Indeed.

> That bitrate table does seem to match:
> 
>    static struct ieee80211_rate rtl_ratetable_5g[] = {
> 
> in drivers/net/wireless/realtek/rtlwifi/base.c
> 

Yeah, it would, but it's irrelevant since the rate table isn't actually
used with MCS rates.

I'm not familiar with this code at all, but looking at it suggests that
perhaps the switch to 5 GHz wasn't at fault, but instead the switch to
VHT (802.11ac) - that's more plausible too, not testing with VHT seems
like something that could have happened for this driver.

And as I figured, the code in _rtl_rc_rate_set_series() is obviously
not handling VHT correctly: it has

                if (sgi_20 || sgi_40 || sgi_80)
rate->flags |= IEEE80211_TX_RC_SHORT_GI;
if (sta && sta->ht_cap.ht_supported &&
((wireless_mode == WIRELESS_MODE_N_5G) ||
 (wireless_mode == WIRELESS_MODE_N_24G)))
rate->flags |= IEEE80211_TX_RC_MCS;

but can never set IEEE80211_TX_RC_VHT_MCS. Seems like there should be
something like

                if (sta && sta->ht_cap.vht_supported &&
(wireless_mode == WIRELESS_MODE_AC_5G ||
                     wireless_mode == WIRELESS_MODE_AC_24G ||
                     wireless_mode == WIRELESS_MODE_AC_ONLY))
rate->flags |= IEEE80211_TX_RC_VHT_MCS;

just after/before the above block.

But I'm not familiar with this code at all, so that may not really be
the right fix or even work.

johannes

pull-request: mac80211 2016-01-26

[Johannes Berg]

Hi Dave,

After split up the patches from the previous pull request that was
lost in the process, here's a new one with fixes for the current
cycle (I'll follow up with a -next one later.)

I've also scripted my pull-request sending process a bit more, so
hopefully the emails will no longer have any unexpected UTF-8 contents
that tripped up patchwork :)

Let me know if there's any problem.

Thanks,
johannes



The following changes since commit 701a0fd5231866db08cebcd502894699f49cb960:

  hip04_eth: fix missing error handle for build_skb failed (2016-01-13 15:48:14 
-0500)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211.git 
tags/mac80211-for-davem-2016-01-26

for you to fetch changes up to 6736fde9672ff6717ac576e9bba2fd5f3dfec822:

  rfkill: fix rfkill_fop_read wait_event usage (2016-01-26 11:32:05 +0100)


Here's a first set of fixes for the 4.5-rc cycle:
 * make regulatory messages much less verbose by default
 * various remain-on-channel fixes
 * scheduled scanning fixes with hardware restart
 * a PS-Poll handling fix; was broken just recently
 * bugfix to avoid buffering non-bufferable MMPDUs
 * world regulatory domain data fix
 * a fix for scanning causing other work to get stuck
 * hwsim: revert an older problematic patch that caused some
   userspace tools to have issues - not that big a deal as
   it's a debug only driver though


Bob Copeland (1):
  Revert "mac80211_hwsim: support any address in userspace"

Dave Young (1):
  wireless: change cfg80211 regulatory domain info as debug messages

Eliad Peller (3):
  mac80211: avoid ROC during hw restart
  mac80211: clear local->sched_scan_req properly on reconfig
  mac80211: handle sched_scan_stopped vs. hw restart race

Emmanuel Grumbach (1):
  mac80211: fix PS-Poll handling

Helmut Schaa (1):
  mac80211: Don't buffer non-bufferable MMPDUs

Johannes Berg (4):
  mac80211: recalculate SW ROC only when needed
  mac80211: fix remain-on-channel cancellation
  regulatory: fix world regulatory domain data
  rfkill: fix rfkill_fop_read wait_event usage

Sachin Kulkarni (1):
  mac80211: Requeue work after scan complete for all VIF types.

 drivers/net/wireless/mac80211_hwsim.c |  5 ++--
 net/mac80211/ibss.c   |  1 -
 net/mac80211/main.c   |  6 +
 net/mac80211/mesh.c   | 11 -
 net/mac80211/mesh.h   |  4 
 net/mac80211/mlme.c   |  2 --
 net/mac80211/offchannel.c | 16 +++--
 net/mac80211/scan.c   | 20 +++-
 net/mac80211/sta_info.c   |  2 +-
 net/mac80211/status.c |  5 
 net/mac80211/util.c   | 16 ++---
 net/rfkill/core.c | 16 -
 net/wireless/reg.c| 45 +++
 13 files changed, 85 insertions(+), 64 deletions(-)

Re: WARNING at net/mac80211/rate.c:513 ieee80211_get_tx_rates [mac80211]

[Johannes Berg]

On Wed, 2016-01-27 at 21:34 -0800, Linus Torvalds wrote:

> .. except now I upgraded the nearest access point, and now wireless
> on that machine no longer works.

Can you describe the upgrade a bit more, just for background?

> Or rather, it actually *does* work in the sense that it
> authenticates, it associates, and it actually gets a DHCP lease etc.
> So the darn thing has an IP address and everything, but then nothing
> else seems to go through after that. Very odd. My guess is that the
> auth/assoc/dhcp thign happens at low rates, then it starts trying to
> up the rates, and things go to hell.

That's usually the case, yes. Auth/assoc/etc. management frames use low
rates anyway, and the first few data frames usually also do until it
scales up.

The code involved is drivers/net/wireless/realtek/rtlwifi/rc.c

> I do note that that rate_fixup_ratelist() function is a bit odd wrt
> those rate indexes: it has code to make sure that there are no valid
> rates following an invalid one:
> 
> /*
>  * make sure there's no valid rate following
>  * an invalid one, just in case drivers don't
>  * take the API seriously to stop at -1.
>  */
> if (inval) {
> rates[i].idx = -1;
> continue;
> }
> if (rates[i].idx < 0) {
> inval = true;
> continue;
> }
> 
> but then that "RC is busted" case that generates a warning will add
> one of those invalid rates in the middle anyway. So I get the feeling
> that if that warning ever triggers, it will basically be screwing up
> that whole rate table. I dunno.

This should be OK, it's more of a sanity check. The driver is supposed
to stop transmission attempts at the first -1 it seems, but the rate
control algorithm shouldn't generate useless attempts that will never
really get used, since that indicates a bug in the rate scaling.

> Is there anything sane I can do to help debug this case?

Could you print out the entire table there when the warning happens? Or
at least, it'd help to figure out at which index the invalid actually
happens. It seems that if that perhaps happens on the very first index,
the driver might get completely confused and perhaps not even send the
frame, which would lead to symptoms like the one you describe.

It seems plausible that there's a path somewhere in the rate scaling
code that forgets to set IEEE80211_TX_RC_MCS or so.

johannes

Re: [PATCH v3 1/4] ipv4: add option to drop unicast encapsulated in L2 multicast

[Johannes Berg]

Hi,

Is there anything I should do about these patches?

I see you marked them as "deferred" in patchwork, but I don't really
know how you're using that state.

johannes

Re: [PATCHv2 3/4] ARM: tegra: use build-in device properties withrfkill_gpio

[Johannes Berg]

On Mon, 2016-01-25 at 21:59 +0100, Marc Dietrich wrote:
> 
> seems to work fine. I wish we could instantiate this from device-tree 
> so we can finially get rid of this file.

That'd be nice - anyone want to propose rfkill DT bindings? :)

johannes

Re: [PATCHv2 3/4] ARM: tegra: use build-in device properties with rfkill_gpio

[Johannes Berg]

On Mon, 2016-01-25 at 13:18 +0100, Thierry Reding wrote:
> 
> Johannes, I assume that you'll want to take this through your tree
> because of the dependency? In that case:
> 
> Acked-by: Thierry Reding <tred...@nvidia.com>

I can, but I don't really care - perhaps you'd rather take the entire
series through your tree to get it into one place for Marc?

In which case, you have my

Acked-by: Johannes Berg <johan...@sipsolutions.net>

for the other 3 patches.

Let me know which you prefer.

johannes

Re: ibss.c backtrace when batman-adv adds wireless interface

[Johannes Berg]

On Wed, 2016-02-03 at 10:26 -0500, Josh Boyer wrote:
> On Wed, Feb 3, 2016 at 10:24 AM, Josh Boyer  g> wrote:
> > Hi All,
> > 
> > We've had a user report the backtrace below when loading batman-adv 
> > on
> > his machine.  It looks like the cfg80211 layer is complaining about
> > a
> > null bss returned, but I cannot tell if the rtlwifi driver or
> > batman-adv is in error here.
> > 
> > Thoughts?
> 
> Sorry, forgot to include the link to the actual bug report:
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1304428
> 
> Reporter says this is new with 4.3.y and did not happen on e.g.
> 4.2.8.
> 

AFAICT this should be a driver (or perhaps mac80211) issue, but I don't
see any information about the driver used.

johannes

Re: [PATCH] mac80211: minstrel_ht: fix out-of-bound in minstrel_ht_set_best_prob_rate

[Johannes Berg]

On Fri, 2016-01-29 at 11:35 +0300, Konstantin Khlebnikov wrote:
> Patch fixes this splat
> 
> BUG: KASAN: slab-out-of-bounds in
> minstrel_ht_update_stats.isra.7+0x6e1/0x9e0
> [mac80211] at addr 8800cee640f4 Read of size 4 by task
> swapper/3/0
> 
> Signed-off-by: Konstantin Khlebnikov 
> Link: http://lkml.kernel.org/r/CALYGNiNyJhSaVnE35qS6UCGaSb2Dx1_i5HcRa
> vuox14otz2...@mail.gmail.com
> 

Applied, thanks.

johannes

Re: custom ioctl-based interface to control LED in networking (was Re: [PATCHv2 09/10] rfkill: Userspace control for airplane mode)

[Johannes Berg]

On Wed, 2016-02-24 at 13:14 +0100, Pavel Machek wrote:
> 
> Why would it need to? It could look at default triggers for the led
> if it really wanted to.

And then it needs to change them; if anything goes wrong error recovery
is practically impossible since the trigger information is lost
forever.

> > I'm sure you'd also not like to see 2**7 triggers implemented in
> > rfkill
> > to cover all the possibilities.
> 
> Are all the possibilities useful? I assumed about 2 are. If you need
> 2**7 of them, LED triggers can have parameters.

It's not my position to decide which combinations some system
integrator or userspace developer might find useful.

Even when we add parameters to a trigger (I don't see a generic way to
do that, but please do enlighten me), you're now ignoring the issue of
the userspace controlled GSM modem...

> > Really what you have here is a concept of "airplane mode", and that
> > concept is specific to the rfkill subsystem. This happens to affect
> > mostly an LED trigger, today, but as a concept it's something that
> > *should* be managed within the rfkill subsystem.
> 
> How is that concept used outside the LEDs? What semantics does
> "airplane mode" have? You tried to explain "airplane mode" is not
> well defined up in this thread...

I'd say it's well-defined for any given set of system software, so if
e.g. NetworkManager decides to define it one way, and connman another
way, there's a definition but the kernel need not interfere with it.

> > > Besides, the series really should have been Cc-ed to LED
> > > people, too.
> > 
> > That's simply unreasonable, you're essentially saying that any user
> > of any kernel infrastructure should be Cc'ed to the implementer of
> > that infrastructure... 9/10 patches in this series aren't even LED
> > specific,
> 
> I'm not saying that. I'm saying that LED maintainers should be Cced,
> to keep the interfaces consistent.

I pretty much have to read it that way, since the LED API is in no way
impacted by these changes. Here's a new trigger, with some magic inner
working. No impact on the LED API.

johannes

Re: custom ioctl-based interface to control LED in networking (was Re: [PATCHv2 09/10] rfkill: Userspace control for airplane mode)

[Johannes Berg]

On Wed, 2016-02-24 at 11:46 +0100, Pavel Machek wrote:

> If you want different trigger, implement different trigger. If you
> want to indicate all but wifi, implement all but wifi, and then
> userspace can select it by writing trigger name. 

This is still mostly a strawman, since userspace cannot have a database
of LEDs that indicate airplane mode.

I'm sure you'd also not like to see 2**7 triggers implemented in rfkill
to cover all the possibilities.

> If you want complete
> userspace control, fine, but we have standard interface and it is not
> ioctl.

The "standard interface" is usable if you really just want to driver a
single LED and you know which one.

I think you're looking at this the wrong way, focusing too much on the
LED aspect.

Really what you have here is a concept of "airplane mode", and that
concept is specific to the rfkill subsystem. This happens to affect
mostly an LED trigger, today, but as a concept it's something that
*should* be managed within the rfkill subsystem.

> Besides, the series really should have been Cc-ed to LED
> people, too.

That's simply unreasonable, you're essentially saying that any user of
any kernel infrastructure should be Cc'ed to the implementer of that
infrastructure... 9/10 patches in this series aren't even LED specific,
only the *previous* patch, the one that tied the "airplane mode"
concept to an LED trigger in a very standard way had anything to do
with LED triggers at all.

johannes

Re: pull-request: mac80211-next 2016-02-23

[Johannes Berg]

On Tue, 2016-02-23 at 15:45 +0100, Johannes Berg wrote:
> Hi Dave,
> 
> Here's a (first, even? I didn't have much until before the
> conference...) pull request for net-next.
> 

Hm, need to withdraw this, sorry about that. There appears to be some
compilation issue that I'm not seeing and need to sort out.

johannes

Re: [PATCHv2 09/10] rfkill: Userspace control for airplane mode

[Johannes Berg]

On Tue, 2016-02-23 at 22:45 +0100, Pavel Machek wrote:

> Well "the airplane mode" is well defined. It means no intentional
> transmitting at radio frequencies.
> 
> The fact that you are allowed to use WIFI on certain flights does not
> change anything.

Nope, not that simple. Pick up any (contemporary) smartphone and watch
what happens with the airplane mode indicator (the little airplane
icon) when you enable wifi after enabling airplane mode. It stays
there.

Clearly the same logic could then apply to an actual LED (on a system
that's less size-constrained, e.g. a laptop or tablet.)

Thus, the display of "in airplane mode" *does* have policy, and clearly
there's precedent for not disabling the icon or LED when wifi is
enabled, but the kernel shouldn't really impose that. Now, the kernel
has a "safe" default which does what you thought was the "well defined"
airplane mode, but at the same time it's obviously not good enough.

And evidently, the Asus system that this was originally proposed for
has such an LED.

> I see that "airplane mode" trigger might be a tiny bit
> useful... dunno, for a LED near the airplane mode switch, when vendor
> replaced hardware toggle with a key. But policy should have nothing
> to do with that. If you argue additional "policy daemon" is needed
> for that... forget it, that's overdesigned.

See above.

> (Besides, finding all LEDs with given trigger is trivial
> operation. Besides... there should never be more than one).

That *might* actually work. But once a tool has detached the trigger
the information is gone; and tools would have to do that to control the
LED, making recovery from any kind of error difficult.

In any case, I've applied those patches already.

As far as the LED subsystem is concerned, all of this is just another
trigger anyway.

johannes

pull-request: mac80211 2016-02-23

[Johannes Berg]

Hi Dave,

Unfortunately I neglected to send you a pull request last week,
so it's here now. We gave a number of fairly important fixes
(two found by KASAN, the wext thing that for some reason people
have started seeing issues with) so it'd be good if these can
still go into 4.5.

Let me know if there's any problem.

Thanks,
johannes



The following changes since commit 6736fde9672ff6717ac576e9bba2fd5f3dfec822:

  rfkill: fix rfkill_fop_read wait_event usage (2016-01-26 11:32:05 +0100)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211.git 
tags/mac80211-for-davem-2016-02-23

for you to fetch changes up to b86071528f3261ab592fad5b9b1a02aea3dcabf3:

  cfg80211: stop critical protocol session upon disconnect event (2016-02-23 
10:41:24 +0100)


Another small set of fixes:
 * stop critical protocol session on disconnect to avoid
   it getting stuck
 * wext: fix two RTNL message ordering issues
 * fix an uninitialized value (found by KASAN)
 * fix an out-of-bounds access (also found by KASAN)
 * clear connection keys when freeing them in all cases
   (IBSS, all other places already did so)
 * fix expected throughput unit to get consistent values
 * set default TX aggregation timeout to 0 in minstrel
   to avoid (really just hide) issues and perform better


Arend van Spriel (1):
  cfg80211: stop critical protocol session upon disconnect event

Chris Bainbridge (1):
  mac80211: fix use of uninitialised values in RX aggregation

Felix Fietkau (1):
  mac80211: minstrel_ht: set default tx aggregation timeout to 0

Johannes Berg (2):
  wext: fix message delay/ordering
  cfg80211/wext: fix message ordering

Konstantin Khlebnikov (1):
  mac80211: minstrel_ht: fix out-of-bound in minstrel_ht_set_best_prob_rate

Ola Olsson (1):
  nl80211: Zero out the connection keys memory when freeing them.

Sven Eckelmann (1):
  mac80211: minstrel: Change expected throughput unit back to Kbps

 include/net/iw_handler.h   |  6 +
 net/mac80211/agg-rx.c  |  2 +-
 net/mac80211/rc80211_minstrel.c|  2 +-
 net/mac80211/rc80211_minstrel_ht.c | 14 +-
 net/wireless/core.c|  2 ++
 net/wireless/nl80211.c |  2 +-
 net/wireless/sme.c |  6 +
 net/wireless/wext-core.c   | 52 ++
 8 files changed, 66 insertions(+), 20 deletions(-)

Re: [PATCHv2 3/4] ARM: tegra: use build-in device properties with rfkill_gpio

[Johannes Berg]

On Fri, 2016-02-19 at 19:03 +0100, Thierry Reding wrote:
> On Thu, Feb 18, 2016 at 09:04:49PM +0100, Johannes Berg wrote:
> > On Tue, 2016-01-26 at 09:42 +0100, Johannes Berg wrote:
> > > On Mon, 2016-01-25 at 13:18 +0100, Thierry Reding wrote:
> > > >  
> > > > Johannes, I assume that you'll want to take this through your
> > > > tree
> > > > because of the dependency? In that case:
> > > > 
> > > > Acked-by: Thierry Reding <tred...@nvidia.com>
> > > 
> > > I can, but I don't really care - perhaps you'd rather take the
> > > entire
> > > series through your tree to get it into one place for Marc?
> > > 
> > > In which case, you have my
> > > 
> > > Acked-by: Johannes Berg <johan...@sipsolutions.net>
> > > 
> > > for the other 3 patches.
> > > 
> > > Let me know which you prefer.
> > > 
> > Did these patches get applied anywhere? Otherwise I'm willing to
> > pick
> > them up.
> 
> Might be easier for everyone if you took this one patch. My earlier
> Acked-by still stands:
> 
Yep, done. I've applied all 4 patches now.

johannes

Re: [PATCHv2 09/10] rfkill: Userspace control for airplane mode

[Johannes Berg]

On Tue, 2016-02-23 at 21:45 +0100, Pavel Machek wrote:

> So... you add LED trigger to display the state of the airplane
> mode. Ok, why not.

Yes. However, consider that "the airplane mode" isn't a well-defined
concept; some systems may want to light up that LED even when wifi is
still enabled, since you're nowadays quite likely to be allowed to use
wifi (but not enable e.g. LTE) while in-flight.

> But now you add an way to override it? Why? If someone wants to
> change
> the led state, he can just change trigger to none, and then control
> the LED manually...

Yes, but now you've forced every application that wants to deal with
this to know about every single LED that might be used with this
trigger... that won't work for some generic userland tool that might
want to implement an "airplane-mode policy".

> BTW what happens when the device contains both radios controlled by
> kernel (wifi, bluetooth) and radios controlled by userspace (GSM
> modem)?

You'd better have the userspace to control the LED :)

johannes

Re: [PATCHv2 3/4] ARM: tegra: use build-in device properties with rfkill_gpio

[Johannes Berg]

On Tue, 2016-02-23 at 16:06 +0100, Arnd Bergmann wrote:
> 
> Does rfkill always have a separate device in the Linux driver model?

Yes, the rfkill core code registers and adds one in the rfkill class.

> johannes

pull-request: mac80211-next 2016-02-26

[Johannes Berg]

Hi Dave,

Let's try this again. I backed out some of the rfkill changes
that are buggy and fixed some of that too. I also left out the
one that generated the big discussion, but I still think it's
the saner thing to do rather than requiring userspace to poke
around that much with sysfs when all it wants to do is tell
us what it thinks should be "airplane mode". Anyway, wanted to
get these things in before sorting that out.

Still the ARM patch in here - acked by the relevant people to
fit into the series (rfkill -> ARM -> rfkill with dependencies
that way); and, as Emmanuel reminded me, an iwlwifi patch that
has similar dependency issues and we decided to take through
my tree.

Let me know if there's any problem.

Thanks,
johannes



The following changes since commit 725da8dee445662beea77d3f42c3f4c79f7a7a0e:

  Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net (2016-01-13 
00:22:13 -0500)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next.git 
tags/mac80211-next-for-davem-2016-02-26

for you to fetch changes up to 50ee738d7271fe825e4024cdfa5c5301a871e2c2:

  rfkill: Add documentation about LED triggers (2016-02-24 09:13:12 +0100)


Here's another round of updates for -next:
 * big A-MSDU RX performance improvement (avoid linearize of paged RX)
 * rfkill changes: cleanups, documentation, platform properties
 * basic PBSS support in cfg80211
 * MU-MIMO action frame processing support
 * BlockAck reordering & duplicate detection offload support
 * various cleanups & little fixes


Arnd Bergmann (1):
  mac80211: avoid excessive stack usage in sta_info

Beni Lev (1):
  cfg80211: Add global RRM capability

Bjorn Andersson (1):
  mac80211: Make addr const in SET_IEEE80211_PERM_ADDR()

Bob Copeland (1):
  mac80211: mesh: drop constant field mean_chain_len

Eliad Peller (3):
  mac80211: move TKIP TX IVs to public part of key struct
  iwlwifi: mvm: move TX PN assignment for TKIP to the driver
  mac80211: remove ieee80211_get_key_tx_seq/ieee80211_set_key_tx_seq

Emmanuel Grumbach (1):
  mac80211: limit the A-MSDU Tx based on peer's capabilities

Felix Fietkau (5):
  mac80211: move A-MSDU skb_linearize call to ieee80211_amsdu_to_8023s
  cfg80211: add function for 802.3 conversion with separate output buffer
  cfg80211: add support for non-linear skbs in ieee80211_amsdu_to_8023s
  cfg80211: fix faulty variable initialization in ieee80211_amsdu_to_8023s
  cfg80211: reuse existing page fragments in A-MSDU rx

Geliang Tang (1):
  cfg80211/mac80211: use to_delayed_work

Grzegorz Bajorski (1):
  mac80211: allow drivers to report (non-)monitor frames

Heikki Krogerus (4):
  net: rfkill: add rfkill_find_type function
  net: rfkill: gpio: get the name and type from device property
  ARM: tegra: use build-in device properties with rfkill_gpio
  net: rfkill: gpio: remove rfkill_gpio_platform_data

Henning Rogge (3):
  mac80211: Remove MPP table entries with MPath
  mac80211: let unused MPP table entries timeout
  mac80211: Unify mesh and mpp path removal function

Ilan Peer (1):
  mac80211: Recalc min chandef when station is associated

Johannes Berg (8):
  cfg80211: remove CFG80211_REG_DEBUG
  mac80211: document status.freq restrictions
  mac80211: refactor HT/VHT to chandef code
  mac80211_hwsim: remove shadowing variable
  rfkill: disentangle polling pause and suspend
  mac80211: add RX_FLAG_MACTIME_PLCP_START
  mac80211: always print a message when disconnecting
  mac80211: change ieee80211_rx_reorder_ready() arguments

Jouni Malinen (1):
  mac80211: Interoperability workaround for 80+80 and 160 MHz channels

João Paulo Rechi Vita (10):
  rfkill: use variable instead of duplicating the expression
  rfkill: remove/inline __rfkill_set_hw_state
  rfkill: Remove obsolete "claim" sysfs interface
  rfkill: Update userspace API documentation
  rfkill: Improve documentation language
  rfkill: Remove extra blank line
  rfkill: Point to the correct deprecated doc location
  rfkill: Move "state" sysfs file back to stable
  rfkill: Factor rfkill_global_states[].cur assignments
  rfkill: Add documentation about LED triggers

Lior David (1):
  cfg80211: basic support for PBSS network type

Lorenzo Bianconi (2):
  mac80211: fix wiphy supported_band access
  cfg80211: add radiotap VHT info to rtap_namespace_sizes

Michal Kazior (3):
  mac80211: fix txq queue related crashes
  mac80211: fix unnecessary frame drops in mesh fwding
  mac80211: expose txq queue depth and size to drivers

Ola Olsson (2):
  cfg80211: add more warnings for inconsistent ops
  cfg80211: Fix some linguistics in Kconfig

Sara Sharo

pull-request: mac80211-next 2016-02-23

[Johannes Berg]

Hi Dave,

Here's a (first, even? I didn't have much until before the conference...)
pull request for net-next.

There's nothing out of the ordinary, I think, except that I have an ARM
Tegra patch in here. It's part of an rfkill series and needed to be merged
with those patches, it also has the relevant ACKs from Thierry and Arnd.

Let me know if there's any problem.

Thanks,
johannes



The following changes since commit 725da8dee445662beea77d3f42c3f4c79f7a7a0e:

  Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net (2016-01-13 
00:22:13 -0500)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next.git 
tags/mac80211-next-for-davem-2016-02-23

for you to fetch changes up to c4b29719e095c1aa42e1dec691d6178611c1096f:

  rfkill: Notify userspace of airplane-mode state changes (2016-02-23 11:53:56 
+0100)


Here's another round of updates for -next:
 * big A-MSDU RX performance improvement (avoid linearize of paged RX)
 * big rfkill changes: airplane mode indicator and cleanups
 * basic PBSS support in cfg80211
 * MU-MIMO action frame processing support
 * BlockAck reordering & duplicate detection offload support
 * various cleanups & little fixes


Arnd Bergmann (1):
  mac80211: avoid excessive stack usage in sta_info

Beni Lev (1):
  cfg80211: Add global RRM capability

Bjorn Andersson (1):
  mac80211: Make addr const in SET_IEEE80211_PERM_ADDR()

Bob Copeland (1):
  mac80211: mesh: drop constant field mean_chain_len

Eliad Peller (3):
  mac80211: move TKIP TX IVs to public part of key struct
  iwlwifi: mvm: move TX PN assignment for TKIP to the driver
  mac80211: remove ieee80211_get_key_tx_seq/ieee80211_set_key_tx_seq

Emmanuel Grumbach (1):
  mac80211: limit the A-MSDU Tx based on peer's capabilities

Felix Fietkau (5):
  mac80211: move A-MSDU skb_linearize call to ieee80211_amsdu_to_8023s
  cfg80211: add function for 802.3 conversion with separate output buffer
  cfg80211: add support for non-linear skbs in ieee80211_amsdu_to_8023s
  cfg80211: fix faulty variable initialization in ieee80211_amsdu_to_8023s
  cfg80211: reuse existing page fragments in A-MSDU rx

Geliang Tang (1):
  cfg80211/mac80211: use to_delayed_work

Grzegorz Bajorski (1):
  mac80211: allow drivers to report (non-)monitor frames

Heikki Krogerus (4):
  net: rfkill: add rfkill_find_type function
  net: rfkill: gpio: get the name and type from device property
  ARM: tegra: use build-in device properties with rfkill_gpio
  net: rfkill: gpio: remove rfkill_gpio_platform_data

Henning Rogge (3):
  mac80211: Remove MPP table entries with MPath
  mac80211: let unused MPP table entries timeout
  mac80211: Unify mesh and mpp path removal function

Ilan Peer (1):
  mac80211: Recalc min chandef when station is associated

Johannes Berg (8):
  cfg80211: remove CFG80211_REG_DEBUG
  mac80211: document status.freq restrictions
  mac80211: refactor HT/VHT to chandef code
  mac80211_hwsim: remove shadowing variable
  rfkill: disentangle polling pause and suspend
  mac80211: add RX_FLAG_MACTIME_PLCP_START
  mac80211: always print a message when disconnecting
  mac80211: change ieee80211_rx_reorder_ready() arguments

Jouni Malinen (1):
  mac80211: Interoperability workaround for 80+80 and 160 MHz channels

João Paulo Rechi Vita (14):
  rfkill: use variable instead of duplicating the expression
  rfkill: remove/inline __rfkill_set_hw_state
  rfkill: Remove obsolete "claim" sysfs interface
  rfkill: Update userspace API documentation
  rfkill: Improve documentation language
  rfkill: Remove extra blank line
  rfkill: Point to the correct deprecated doc location
  rfkill: Move "state" sysfs file back to stable
  rfkill: Factor rfkill_global_states[].cur assignments
  rfkill: Add documentation about LED triggers
  rfkill: Create "rfkill-airplane-mode" LED trigger
  rfkill: Use switch to demux userspace operations
  rfkill: Userspace control for airplane mode
  rfkill: Notify userspace of airplane-mode state changes

Lior David (1):
  cfg80211: basic support for PBSS network type

Lorenzo Bianconi (2):
  mac80211: fix wiphy supported_band access
  cfg80211: add radiotap VHT info to rtap_namespace_sizes

Michal Kazior (3):
  mac80211: fix txq queue related crashes
  mac80211: fix unnecessary frame drops in mesh fwding
  mac80211: expose txq queue depth and size to drivers

Ola Olsson (2):
  cfg80211: add more warnings for inconsistent ops
  cfg80211: Fix some linguistics in Kconfig

Sara Sharon (10):
  mac80211: process and save VHT MU-MIMO group frame
  mac80211: add flag for duplication check
  mac

[PATCH] net: fec: make driver endian-safe

[Johannes Berg]

The driver treats the device descriptors as CPU-endian, which is
probably right on both ARM (little-endian) and PowerPC (big-endian)
but wrong on big-endian ARM.

Add the correct annotations and byteswaps.

This gets it working on i.MX6 hummingboard booted in big-endian mode.

Signed-off-by: Johannes Berg <johan...@sipsolutions.net>
---
 drivers/net/ethernet/freescale/Makefile   |   2 +
 drivers/net/ethernet/freescale/fec.h  |  39 ++---
 drivers/net/ethernet/freescale/fec_main.c | 130 --
 3 files changed, 99 insertions(+), 72 deletions(-)

diff --git a/drivers/net/ethernet/freescale/Makefile 
b/drivers/net/ethernet/freescale/Makefile
index 71debd1c18c9..64ddc0bd2735 100644
--- a/drivers/net/ethernet/freescale/Makefile
+++ b/drivers/net/ethernet/freescale/Makefile
@@ -4,6 +4,8 @@
 
 obj-$(CONFIG_FEC) += fec.o
 fec-objs :=fec_main.o fec_ptp.o
+CFLAGS_fec_main.o := -D__CHECK_ENDIAN__
+
 obj-$(CONFIG_FEC_MPC52xx) += fec_mpc52xx.o
 ifeq ($(CONFIG_FEC_MPC52xx_MDIO),y)
obj-$(CONFIG_FEC_MPC52xx) += fec_mpc52xx_phy.o
diff --git a/drivers/net/ethernet/freescale/fec.h 
b/drivers/net/ethernet/freescale/fec.h
index 99d33e2d35e6..801dcc7fa6e6 100644
--- a/drivers/net/ethernet/freescale/fec.h
+++ b/drivers/net/ethernet/freescale/fec.h
@@ -191,27 +191,44 @@
 /*
  * Define the buffer descriptor structure.
  */
+/* buffer endianness appears to be a mess ... ARM is usually LE but can be BE 
*/
+#if defined(CONFIG_ARM) && defined(CONFIG_CPU_BIG_ENDIAN)
+#define fec32_to_cpu le32_to_cpu
+#define fec16_to_cpu le16_to_cpu
+#define cpu_to_fec32 cpu_to_le32
+#define cpu_to_fec16 cpu_to_le16
+#define __fec32 __le32
+#define __fec16 __le16
+#else
+#define fec32_to_cpu be32_to_cpu
+#define fec16_to_cpu be16_to_cpu
+#define cpu_to_fec32 cpu_to_be32
+#define cpu_to_fec16 cpu_to_be16
+#define __fec32 __be32
+#define __fec16 __be16
+#endif
+
 #if defined(CONFIG_ARCH_MXC) || defined(CONFIG_SOC_IMX28)
 struct bufdesc {
-   unsigned short cbd_datlen;  /* Data length */
-   unsigned short cbd_sc;  /* Control and status info */
-   unsigned long cbd_bufaddr;  /* Buffer address */
+   __fec16 cbd_datlen; /* Data length */
+   __fec16 cbd_sc; /* Control and status info */
+   __fec32 cbd_bufaddr;/* Buffer address */
 };
 #else
 struct bufdesc {
-   unsigned short  cbd_sc; /* Control and status info */
-   unsigned short  cbd_datlen; /* Data length */
-   unsigned long   cbd_bufaddr;/* Buffer address */
+   __fec16 cbd_sc; /* Control and status info */
+   __fec16 cbd_datlen; /* Data length */
+   __fec32 cbd_bufaddr;/* Buffer address */
 };
 #endif
 
 struct bufdesc_ex {
struct bufdesc desc;
-   unsigned long cbd_esc;
-   unsigned long cbd_prot;
-   unsigned long cbd_bdu;
-   unsigned long ts;
-   unsigned short res0[4];
+   __fec32 cbd_esc;
+   __fec32 cbd_prot;
+   __fec32 cbd_bdu;
+   __fec32 ts;
+   __fec16 res0[4];
 };
 
 /*
diff --git a/drivers/net/ethernet/freescale/fec_main.c 
b/drivers/net/ethernet/freescale/fec_main.c
index d2328fc5da57..8e81c4de1f41 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -331,11 +331,13 @@ static void fec_dump(struct net_device *ndev)
bdp = txq->tx_bd_base;
 
do {
-   pr_info("%3u %c%c 0x%04x 0x%08lx %4u %p\n",
+   pr_info("%3u %c%c 0x%04x 0x%08x %4u %p\n",
index,
bdp == txq->cur_tx ? 'S' : ' ',
bdp == txq->dirty_tx ? 'H' : ' ',
-   bdp->cbd_sc, bdp->cbd_bufaddr, bdp->cbd_datlen,
+   fec16_to_cpu(bdp->cbd_sc),
+   fec32_to_cpu(bdp->cbd_bufaddr),
+   fec16_to_cpu(bdp->cbd_datlen),
txq->tx_skbuff[index]);
bdp = fec_enet_get_nextdesc(bdp, fep, 0);
index++;
@@ -388,7 +390,7 @@ fec_enet_txq_submit_frag_skb(struct fec_enet_priv_tx_q *txq,
bdp = fec_enet_get_nextdesc(bdp, fep, queue);
ebdp = (struct bufdesc_ex *)bdp;
 
-   status = bdp->cbd_sc;
+   status = fec16_to_cpu(bdp->cbd_sc);
status &= ~BD_ENET_TX_STATS;
status |= (BD_ENET_TX_TC | BD_ENET_TX_READY);
frag_len = skb_shinfo(skb)->frags[frag].size;
@@ -410,7 +412,7 @@ fec_enet_txq_submit_frag_skb(struct fec_enet_priv_tx_q *txq,
if (skb->ip_summed == CHECKSUM_PARTIAL)
estatus |= BD_ENET_TX_PINS | BD_ENET_TX_IINS;
ebdp->cbd_bdu = 0;
-   ebdp->cbd_esc = estatus;
+   ebdp->cbd_esc = cpu_to_fec32(estatus);
}
 

Re: net: fec: make driver endian-safe

[Johannes Berg]

On Sat, 2016-01-23 at 23:09 +0100, Johannes Berg wrote:
> 
> +/* buffer endianness appears to be a mess ... ARM is usually LE but
> can be BE */
> +#if defined(CONFIG_ARM) && defined(CONFIG_CPU_BIG_ENDIAN)

Just realized that this is, of course, completely wrong. If the ARM CPU
is little endian, the device is of course *still* little endian, it
doesn't magically change itself with the CONFIG_CPU_BIG_ENDIAN
setting... :)

Ergo, the #if part should be taken, rather than the #else part, to
still define the descriptors as __le and just have no byteswapping
done.

Also, as Arnd had pointed out last night but I wasn't coherent enough
to fully understand, it's exceedingly likely that

>  #if defined(CONFIG_ARCH_MXC) || defined(CONFIG_SOC_IMX28)

this needs to be changed and that the device is simply "generated" as
little-endian for ARM SoCs. He also pointed out that there could be
multiple CONFIG_SOC or CONFIG_ARCH definitions (if I understood
correctly), so this would be wrong anyway.

It's tempting to combine both ifdefs and change them to just CONFIG_ARM
but I have no way of verifying that on anything other than i.MX6 (on
the hummingboard), nor do I even know which SoCs ship with this block.

johannes

Re: [PATCH v2] net: fec: make driver endian-safe

[Johannes Berg]

On Mon, 2016-01-25 at 10:52 +1000, Greg Ungerer wrote:
> 
> I tested this on a ColdFire (5208) target that uses this driver.
> Simple testing showed it working with no problems. The ColdFire
> SoC processors use a version of the FEC hardware module, and they
> always run big-endian.
> 

Great, thanks!

FWIW, I failed to mention this - I did test the new version also on
little-endian ARM.

johannes

[PATCH v2] net: fec: make driver endian-safe

[Johannes Berg]

The driver treats the device descriptors as CPU-endian, which appears
to be correct with the default endianness on both ARM (typically LE)
and PowerPC (typically BE) SoCs, indicating that the hardware block
is generated differently. Add endianness annotations and byteswaps as
necessary.

It's not clear that the ifdef there really is correct and shouldn't
just be #ifdef CONFIG_ARM, but I also can't test on anything but the
i.MX6 HummingBoard where this gets it working with a BE kernel.

Signed-off-by: Johannes Berg <johan...@sipsolutions.net>
---
 drivers/net/ethernet/freescale/Makefile   |   3 +
 drivers/net/ethernet/freescale/fec.h  |  40 ++---
 drivers/net/ethernet/freescale/fec_main.c | 130 --
 3 files changed, 101 insertions(+), 72 deletions(-)

diff --git a/drivers/net/ethernet/freescale/Makefile 
b/drivers/net/ethernet/freescale/Makefile
index 71debd1c18c9..632adc2d7562 100644
--- a/drivers/net/ethernet/freescale/Makefile
+++ b/drivers/net/ethernet/freescale/Makefile
@@ -4,6 +4,9 @@
 
 obj-$(CONFIG_FEC) += fec.o
 fec-objs :=fec_main.o fec_ptp.o
+CFLAGS_fec_main.o := -D__CHECK_ENDIAN__
+CFLAGS_fec_ptp.o := -D__CHECK_ENDIAN__
+
 obj-$(CONFIG_FEC_MPC52xx) += fec_mpc52xx.o
 ifeq ($(CONFIG_FEC_MPC52xx_MDIO),y)
obj-$(CONFIG_FEC_MPC52xx) += fec_mpc52xx_phy.o
diff --git a/drivers/net/ethernet/freescale/fec.h 
b/drivers/net/ethernet/freescale/fec.h
index 99d33e2d35e6..7dd47a0b8cfb 100644
--- a/drivers/net/ethernet/freescale/fec.h
+++ b/drivers/net/ethernet/freescale/fec.h
@@ -190,28 +190,46 @@
 
 /*
  * Define the buffer descriptor structure.
+ *
+ * Evidently, ARM SoCs have the FEC block generated in a
+ * little endian mode; or at least ARCH_MXC/SOC_IMX28 do,
+ * so adjust endianness accordingly.
  */
 #if defined(CONFIG_ARCH_MXC) || defined(CONFIG_SOC_IMX28)
+#define fec32_to_cpu le32_to_cpu
+#define fec16_to_cpu le16_to_cpu
+#define cpu_to_fec32 cpu_to_le32
+#define cpu_to_fec16 cpu_to_le16
+#define __fec32 __le32
+#define __fec16 __le16
+
 struct bufdesc {
-   unsigned short cbd_datlen;  /* Data length */
-   unsigned short cbd_sc;  /* Control and status info */
-   unsigned long cbd_bufaddr;  /* Buffer address */
+   __fec16 cbd_datlen; /* Data length */
+   __fec16 cbd_sc; /* Control and status info */
+   __fec32 cbd_bufaddr;/* Buffer address */
 };
 #else
+#define fec32_to_cpu be32_to_cpu
+#define fec16_to_cpu be16_to_cpu
+#define cpu_to_fec32 cpu_to_be32
+#define cpu_to_fec16 cpu_to_be16
+#define __fec32 __be32
+#define __fec16 __be16
+
 struct bufdesc {
-   unsigned short  cbd_sc; /* Control and status info */
-   unsigned short  cbd_datlen; /* Data length */
-   unsigned long   cbd_bufaddr;/* Buffer address */
+   __fec16 cbd_sc; /* Control and status info */
+   __fec16 cbd_datlen; /* Data length */
+   __fec32 cbd_bufaddr;/* Buffer address */
 };
 #endif
 
 struct bufdesc_ex {
struct bufdesc desc;
-   unsigned long cbd_esc;
-   unsigned long cbd_prot;
-   unsigned long cbd_bdu;
-   unsigned long ts;
-   unsigned short res0[4];
+   __fec32 cbd_esc;
+   __fec32 cbd_prot;
+   __fec32 cbd_bdu;
+   __fec32 ts;
+   __fec16 res0[4];
 };
 
 /*
diff --git a/drivers/net/ethernet/freescale/fec_main.c 
b/drivers/net/ethernet/freescale/fec_main.c
index b2a32209ffbf..a055fbad1253 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -331,11 +331,13 @@ static void fec_dump(struct net_device *ndev)
bdp = txq->tx_bd_base;
 
do {
-   pr_info("%3u %c%c 0x%04x 0x%08lx %4u %p\n",
+   pr_info("%3u %c%c 0x%04x 0x%08x %4u %p\n",
index,
bdp == txq->cur_tx ? 'S' : ' ',
bdp == txq->dirty_tx ? 'H' : ' ',
-   bdp->cbd_sc, bdp->cbd_bufaddr, bdp->cbd_datlen,
+   fec16_to_cpu(bdp->cbd_sc),
+   fec32_to_cpu(bdp->cbd_bufaddr),
+   fec16_to_cpu(bdp->cbd_datlen),
txq->tx_skbuff[index]);
bdp = fec_enet_get_nextdesc(bdp, fep, 0);
index++;
@@ -388,7 +390,7 @@ fec_enet_txq_submit_frag_skb(struct fec_enet_priv_tx_q *txq,
bdp = fec_enet_get_nextdesc(bdp, fep, queue);
ebdp = (struct bufdesc_ex *)bdp;
 
-   status = bdp->cbd_sc;
+   status = fec16_to_cpu(bdp->cbd_sc);
status &= ~BD_ENET_TX_STATS;
status |= (BD_ENET_TX_TC | BD_ENET_TX_READY);
frag_len = skb_shinfo(skb)->frags[frag].size;
@@ -410,7 +412,7 @@ fec_enet_txq_submit_frag_skb(struct fec_enet_priv_tx_q *txq,
if (skb->ip_summed == CHECKSUM_PARTIAL)

[PATCH v2] net: fec: use CONFIG_ARM instead of CONFIG_ARCH_MXC/SOC_IMX28

[Johannes Berg]

As Arnd Bergmann points out, using CONFIG_ARCH_MXC and/or SOC_IMX28
is wrong if some other ARM platform uses this device - the operation
of the driver would depend on an unrelated ARM platform that might
or might not be set for multi-platform kernels.

Prior to my previous patch, any other platforms using it would have
been broken already due to having the cbd_datlen/cbd_sc fields in
the wrong order, but byte ordering correctly, so no such platforms
can exist and work today.

In any case, it seems likely that only Freescale SoCs use this part,
and those are little-endian on ARM, so CONFIG_ARM is safe for them.

Signed-off-by: Johannes Berg <johan...@sipsolutions.net>
---
 drivers/net/ethernet/freescale/fec.h  | 8 +++-
 drivers/net/ethernet/freescale/fec_main.c | 3 +--
 2 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec.h 
b/drivers/net/ethernet/freescale/fec.h
index 7dd47a0b8cfb..2106d72c91dc 100644
--- a/drivers/net/ethernet/freescale/fec.h
+++ b/drivers/net/ethernet/freescale/fec.h
@@ -19,8 +19,7 @@
 #include 
 
 #if defined(CONFIG_M523x) || defined(CONFIG_M527x) || defined(CONFIG_M528x) || 
\
-defined(CONFIG_M520x) || defined(CONFIG_M532x) || \
-defined(CONFIG_ARCH_MXC) || defined(CONFIG_SOC_IMX28)
+defined(CONFIG_M520x) || defined(CONFIG_M532x) || defined(CONFIG_ARM)
 /*
  * Just figures, Motorola would have to change the offsets for
  * registers in the same peripheral device on different models
@@ -192,10 +191,9 @@
  * Define the buffer descriptor structure.
  *
  * Evidently, ARM SoCs have the FEC block generated in a
- * little endian mode; or at least ARCH_MXC/SOC_IMX28 do,
- * so adjust endianness accordingly.
+ * little endian mode so adjust endianness accordingly.
  */
-#if defined(CONFIG_ARCH_MXC) || defined(CONFIG_SOC_IMX28)
+#if defined(CONFIG_ARM)
 #define fec32_to_cpu le32_to_cpu
 #define fec16_to_cpu le16_to_cpu
 #define cpu_to_fec32 cpu_to_le32
diff --git a/drivers/net/ethernet/freescale/fec_main.c 
b/drivers/net/ethernet/freescale/fec_main.c
index a055fbad1253..dffb452b245f 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -2170,8 +2170,7 @@ static int fec_enet_get_regs_len(struct net_device *ndev)
 
 /* List of registers that can be safety be read to dump them with ethtool */
 #if defined(CONFIG_M523x) || defined(CONFIG_M527x) || defined(CONFIG_M528x) || 
\
-   defined(CONFIG_M520x) || defined(CONFIG_M532x) ||   \
-   defined(CONFIG_ARCH_MXC) || defined(CONFIG_SOC_IMX28)
+   defined(CONFIG_M520x) || defined(CONFIG_M532x) || defined(CONFIG_ARM)
 static u32 fec_enet_register_offset[] = {
FEC_IEVENT, FEC_IMASK, FEC_R_DES_ACTIVE_0, FEC_X_DES_ACTIVE_0,
FEC_ECNTRL, FEC_MII_DATA, FEC_MII_SPEED, FEC_MIB_CTRLSTAT, FEC_R_CNTRL,
-- 
2.6.2

[PATCH] net: fec: use CONFIG_ARM instead of CONFIG_ARCH_MXC/SOC_IMX28

[Johannes Berg]

From: Johannes Berg <johannes.b...@intel.com>

As Arnd Bergmann points out, using CONFIG_ARCH_MXC and/or SOC_IMX28
is wrong if some other ARM platform uses this device - the operation
of the driver would depend on an unrelated ARM platform that might
or might not be set for multi-platform kernels.

Prior to my previous patch, any other platforms using it would have
been broken already due to having the cbd_datlen/cbd_sc fields in
the wrong order, but byte ordering correctly, so no such platforms
can exist and work today.

In any case, it seems likely that only Freescale SoCs use this part,
and those are little-endian on ARM, so CONFIG_ARM is safe for them.

Signed-off-by: Johannes Berg <johannes.b...@intel.com>
---
 drivers/net/ethernet/freescale/fec.h  | 8 +++-
 drivers/net/ethernet/freescale/fec_main.c | 3 +--
 2 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec.h 
b/drivers/net/ethernet/freescale/fec.h
index 7dd47a0b8cfb..2106d72c91dc 100644
--- a/drivers/net/ethernet/freescale/fec.h
+++ b/drivers/net/ethernet/freescale/fec.h
@@ -19,8 +19,7 @@
 #include 
 
 #if defined(CONFIG_M523x) || defined(CONFIG_M527x) || defined(CONFIG_M528x) || 
\
-defined(CONFIG_M520x) || defined(CONFIG_M532x) || \
-defined(CONFIG_ARCH_MXC) || defined(CONFIG_SOC_IMX28)
+defined(CONFIG_M520x) || defined(CONFIG_M532x) || defined(CONFIG_ARM)
 /*
  * Just figures, Motorola would have to change the offsets for
  * registers in the same peripheral device on different models
@@ -192,10 +191,9 @@
  * Define the buffer descriptor structure.
  *
  * Evidently, ARM SoCs have the FEC block generated in a
- * little endian mode; or at least ARCH_MXC/SOC_IMX28 do,
- * so adjust endianness accordingly.
+ * little endian mode so adjust endianness accordingly.
  */
-#if defined(CONFIG_ARCH_MXC) || defined(CONFIG_SOC_IMX28)
+#if defined(CONFIG_ARM)
 #define fec32_to_cpu le32_to_cpu
 #define fec16_to_cpu le16_to_cpu
 #define cpu_to_fec32 cpu_to_le32
diff --git a/drivers/net/ethernet/freescale/fec_main.c 
b/drivers/net/ethernet/freescale/fec_main.c
index a055fbad1253..dffb452b245f 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -2170,8 +2170,7 @@ static int fec_enet_get_regs_len(struct net_device *ndev)
 
 /* List of registers that can be safety be read to dump them with ethtool */
 #if defined(CONFIG_M523x) || defined(CONFIG_M527x) || defined(CONFIG_M528x) || 
\
-   defined(CONFIG_M520x) || defined(CONFIG_M532x) ||   \
-   defined(CONFIG_ARCH_MXC) || defined(CONFIG_SOC_IMX28)
+   defined(CONFIG_M520x) || defined(CONFIG_M532x) || defined(CONFIG_ARM)
 static u32 fec_enet_register_offset[] = {
FEC_IEVENT, FEC_IMASK, FEC_R_DES_ACTIVE_0, FEC_X_DES_ACTIVE_0,
FEC_ECNTRL, FEC_MII_DATA, FEC_MII_SPEED, FEC_MIB_CTRLSTAT, FEC_R_CNTRL,
-- 
2.6.2

Re: net/rfkill: WARNING in rfkill_fop_read

[Johannes Berg]

On Tue, 2016-01-26 at 10:55 +0100, Dmitry Vyukov wrote:
> Hello,
> 
> The following program triggers WARNING message in rfkill_fop_read:
> 
> [ cut here ]
> WARNING: CPU: 2 PID: 6975 at kernel/sched/core.c:7663
> __might_sleep+0x138/0x1a0()
> do not call blocking ops when !TASK_RUNNING; state=1 set at
> [] prepare_to_wait_event+0x141/0x410
> kernel/sched/wait.c:210

>  [< inline >] rfkill_readable net/rfkill/core.c:1102

Yeah, this should not acquire the mutex ... but luckily it's not a
problem not to anyway, so it's easy to fix. Thanks! I'll send out a fix
once I've tested it.

> // autogenerated by syzkaller (http://github.com/google/syzkaller)

gotta love the raw mmap etc. :)

johannes

Re: [PATCH] net:mac80211:mesh_plink: remove redundant sta_info check

[Johannes Berg]

On Thu, 2016-01-21 at 11:06 +0530, Sunil Shahu wrote:
> Remove unnecessory "if" statement and club it with previos "if"
> block.
> 
Applied.

johannes

Re: [PATCH 0/8] General RFKill improvements

[Johannes Berg]

Hi,

On Tue, 2016-01-19 at 10:42 -0500, João Paulo Rechi Vita wrote:
> This series contains a few general improvements to the RFKill code,
> found while
> I was writing the rfkill-all / airplane mode LED trigger. All were
> points where
> I had to read twice or do some other kind of extra effort to fully
> understand
> it, so I think merging these changes will benefit someone trying to
> understand
> the RFKill subsystem in the future.

I applied some of this. I think we need to keep the "state" sysfs
interface, perhaps you want to document it more clearly. I also didn't
apply the first patch since I'd fixed the suspend/pause issue, please
see if any of your patches remain relevant and resend that.

Thanks,
johannes

[PATCH 2/2] cfg80211/wext: fix message ordering

[Johannes Berg]

From: Johannes Berg <johannes.b...@intel.com>

Since cfg80211 frequently takes actions from its netdev notifier
call, wireless extensions messages could still be ordered badly
since the wext netdev notifier, since wext is built into the
kernel, runs before the cfg80211 netdev notifier. For example,
the following can happen:

5: wlan1: <BROADCAST,MULTICAST> mtu 1500 qdisc mq state DOWN group default
link/ether 02:00:00:00:01:00 brd ff:ff:ff:ff:ff:ff
5: wlan1: <BROADCAST,MULTICAST,UP>
link/ether

when setting the interface down causes the wext message.

To also fix this, export the wireless_nlevent_flush() function
and also call it from the cfg80211 notifier.

Cc: sta...@vger.kernel.org
Signed-off-by: Johannes Berg <johannes.b...@intel.com>
---
 include/net/iw_handler.h | 6 ++
 net/wireless/core.c  | 2 ++
 net/wireless/wext-core.c | 3 ++-
 3 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/include/net/iw_handler.h b/include/net/iw_handler.h
index 8f81bbbc38fc..8a3ec3955f20 100644
--- a/include/net/iw_handler.h
+++ b/include/net/iw_handler.h
@@ -439,6 +439,12 @@ int dev_get_wireless_info(char *buffer, char **start, 
off_t offset, int length);
 /* Send a single event to user space */
 void wireless_send_event(struct net_device *dev, unsigned int cmd,
 union iwreq_data *wrqu, const char *extra);
+#ifdef CONFIG_WEXT_CORE
+/* flush all previous wext events - if work is done from netdev notifiers */
+void wireless_nlevent_flush(void);
+#else
+static void wireless_nlevent_flush(void) {}
+#endif
 
 /* We may need a function to send a stream of events to user space.
  * More on that later... */
diff --git a/net/wireless/core.c b/net/wireless/core.c
index b0915515640e..8f0bac7e03c4 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -1147,6 +1147,8 @@ static int cfg80211_netdev_notifier_call(struct 
notifier_block *nb,
return NOTIFY_DONE;
}
 
+   wireless_nlevent_flush();
+
return NOTIFY_OK;
 }
 
diff --git a/net/wireless/wext-core.c b/net/wireless/wext-core.c
index 5f429637efff..abdfcb5f3e48 100644
--- a/net/wireless/wext-core.c
+++ b/net/wireless/wext-core.c
@@ -342,7 +342,7 @@ static const int compat_event_type_size[] = {
 
 /* IW event code */
 
-static void wireless_nlevent_flush(void)
+void wireless_nlevent_flush(void)
 {
struct sk_buff *skb;
struct net *net;
@@ -355,6 +355,7 @@ static void wireless_nlevent_flush(void)
GFP_KERNEL);
}
 }
+EXPORT_SYMBOL_GPL(wireless_nlevent_flush);
 
 static int wext_netdev_notifier_call(struct notifier_block *nb,
 unsigned long state, void *ptr)
-- 
2.6.2

[PATCH 1/2] wext: fix message delay/ordering

[Johannes Berg]

From: Johannes Berg <johannes.b...@intel.com>

Beniamino reported that he was getting an RTM_NEWLINK message for a
given interface, after the RTM_DELLINK for it. It turns out that the
message is a wireless extensions message, which was sent because the
interface had been connected and disconnection while it was deleted
caused a wext message.

For its netlink messages, wext uses RTM_NEWLINK, but the message is
without all the regular rtnetlink attributes, so "ip monitor link"
prints just rudimentary information:

5: wlan1: <BROADCAST,MULTICAST> mtu 1500 qdisc mq state DOWN group default
link/ether 02:00:00:00:01:00 brd ff:ff:ff:ff:ff:ff
Deleted 5: wlan1: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN group 
default
link/ether 02:00:00:00:01:00 brd ff:ff:ff:ff:ff:ff
5: wlan1: <BROADCAST,MULTICAST,UP>
link/ether
(from my hwsim reproduction)

This can cause userspace to get confused since it doesn't expect an
RTM_NEWLINK message after RTM_DELLINK.

The reason for this is that wext schedules a worker to send out the
messages, and the scheduling delay can cause the messages to get out
to userspace in different order.

To fix this, have wext register a netdevice notifier and flush out
any pending messages when netdevice state changes. This fixes any
ordering whenever the original message wasn't sent by a notifier
itself.

Cc: sta...@vger.kernel.org
Reported-by: Beniamino Galvani <bgalv...@redhat.com>
Signed-off-by: Johannes Berg <johannes.b...@intel.com>
---
 net/wireless/wext-core.c | 49 ++--
 1 file changed, 39 insertions(+), 10 deletions(-)

diff --git a/net/wireless/wext-core.c b/net/wireless/wext-core.c
index c8717c1d082e..5f429637efff 100644
--- a/net/wireless/wext-core.c
+++ b/net/wireless/wext-core.c
@@ -342,6 +342,39 @@ static const int compat_event_type_size[] = {
 
 /* IW event code */
 
+static void wireless_nlevent_flush(void)
+{
+   struct sk_buff *skb;
+   struct net *net;
+
+   ASSERT_RTNL();
+
+   for_each_net(net) {
+   while ((skb = skb_dequeue(>wext_nlevents)))
+   rtnl_notify(skb, net, 0, RTNLGRP_LINK, NULL,
+   GFP_KERNEL);
+   }
+}
+
+static int wext_netdev_notifier_call(struct notifier_block *nb,
+unsigned long state, void *ptr)
+{
+   /*
+* When a netdev changes state in any way, flush all pending messages
+* to avoid them going out in a strange order, e.g. RTM_NEWLINK after
+* RTM_DELLINK, or with IFF_UP after without IFF_UP during dev_close()
+* or similar - all of which could otherwise happen due to delays from
+* schedule_work().
+*/
+   wireless_nlevent_flush();
+
+   return NOTIFY_OK;
+}
+
+static struct notifier_block wext_netdev_notifier = {
+   .notifier_call = wext_netdev_notifier_call,
+};
+
 static int __net_init wext_pernet_init(struct net *net)
 {
skb_queue_head_init(>wext_nlevents);
@@ -360,6 +393,11 @@ static struct pernet_operations wext_pernet_ops = {
 
 static int __init wireless_nlevent_init(void)
 {
+   int err = register_netdevice_notifier(_netdev_notifier);
+
+   if (err)
+   return err;
+
return register_pernet_subsys(_pernet_ops);
 }
 
@@ -368,17 +406,8 @@ subsys_initcall(wireless_nlevent_init);
 /* Process events generated by the wireless layer or the driver. */
 static void wireless_nlevent_process(struct work_struct *work)
 {
-   struct sk_buff *skb;
-   struct net *net;
-
rtnl_lock();
-
-   for_each_net(net) {
-   while ((skb = skb_dequeue(>wext_nlevents)))
-   rtnl_notify(skb, net, 0, RTNLGRP_LINK, NULL,
-   GFP_KERNEL);
-   }
-
+   wireless_nlevent_flush();
rtnl_unlock();
 }
 
-- 
2.6.2

Re: [PATCH v2] net: fec: make driver endian-safe

[Johannes Berg]

On Sun, 2016-01-24 at 17:49 +0100, Arnd Bergmann wrote:

> I'd argue that the "(CONFIG_ARCH_MXC) || defined(CONFIG_SOC_IMX28)"
> is definitely wrong, because if we ever get another ARM platform
> that uses this driver, it may or may not work depending on whether
> the ARCH_MXC is also set, and that is not a helpful behavior.
> 
> Better make it simply CONFIG_ARM to keep the behavior independent
> of config options. It won't change anything for now but any future
> platform will probably work independent of configuration or would
> require a bugfix at all.
> 

I agree, but I'm not really sure it's in the scope of this particular
patch? Might be better to just have a separate patch with an
appropriate commit message changing this ifdef.

johannes

Re: [PATCHv2 3/4] ARM: tegra: use build-in device properties with rfkill_gpio

[Johannes Berg]

On Tue, 2016-01-26 at 09:42 +0100, Johannes Berg wrote:
> On Mon, 2016-01-25 at 13:18 +0100, Thierry Reding wrote:
> >  
> > Johannes, I assume that you'll want to take this through your tree
> > because of the dependency? In that case:
> > 
> > Acked-by: Thierry Reding <tred...@nvidia.com>
> 
> I can, but I don't really care - perhaps you'd rather take the entire
> series through your tree to get it into one place for Marc?
> 
> In which case, you have my
> 
> Acked-by: Johannes Berg <johan...@sipsolutions.net>
> 
> for the other 3 patches.
> 
> Let me know which you prefer.
> 
Did these patches get applied anywhere? Otherwise I'm willing to pick
them up.

johannes

Re: [PATCH 7/9] rfkill: Create "rfkill-airplane_mode" LED trigger

[Johannes Berg]

On Mon, 2016-02-08 at 10:41 -0500, João Paulo Rechi Vita wrote:
> This creates a new LED trigger to be used by platform drivers as a
> default trigger for airplane-mode indicator LEDs.
> 
> By default this trigger will fire when RFKILL_OP_CHANGE_ALL is called
> for all types (RFKILL_TYPE_ALL), setting the LED brightness to
> LED_FULL
> when the changing the state to blocked, and to LED_OFF when the
> changing
> the state to unblocked. In the future there will be a mechanism for
> userspace to override the default policy, so it can implement its
> own.
> 
> This trigger will be used by the asus-wireless x86 platform driver.

Just one comment - I think you should be consistent with the _ vs - and
just use "rfkill-airplane-mode" as the name.

johannes

Re: [PATCH 8/9] rfkill: Userspace control for airplane mode

[Johannes Berg]

Hi,

Sorry for the delay reviewing this.



On Mon, 2016-02-08 at 10:41 -0500, João Paulo Rechi Vita wrote:
> Provide an interface for the airplane-mode indicator be controlled
> from
> userspace. User has to first acquire the control through
> RFKILL_OP_AIRPLANE_MODE_ACQUIRE and keep the fd open for the whole
> time
> it wants to be in control of the indicator. Closing the fd or using
> RFKILL_OP_AIRPLANE_MODE_RELEASE restores the default policy.

I've come to the conclusion that the new ops are probably the best
thing to do here.

> +Userspace can also override the default airplane-mode indicator
> policy through
> +/dev/rfkill. Control of the airplane mode indicator has to be
> acquired first,
> +using RFKILL_OP_AIRPLANE_MODE_ACQUIRE, and is only available for one
> userspace
> +application at a time. Closing the fd or using
> RFKILL_OP_AIRPLANE_MODE_RELEASE
> +reverts the airplane-mode indicator back to the default kernel
> policy and makes
> +it available for other applications to take control. Changes to the
> +airplane-mode indicator state can be made using
> RFKILL_OP_AIRPLANE_MODE_CHANGE,
> +passing the new value in the 'soft' field of 'struct rfkill_event'.

I don't really see any value in _RELEASE, since an application can just
close the fd? I'd prefer not having the duplicate functionality
and force us to exercise the single code path every time.

>  For further details consult Documentation/ABI/stable/sysfs-class-
> rfkill.
> diff --git a/include/uapi/linux/rfkill.h
> b/include/uapi/linux/rfkill.h
> index 2e00dce..9cb999b 100644
> --- a/include/uapi/linux/rfkill.h
> +++ b/include/uapi/linux/rfkill.h
> @@ -67,6 +67,9 @@ enum rfkill_operation {
>   RFKILL_OP_DEL,
>   RFKILL_OP_CHANGE,
>   RFKILL_OP_CHANGE_ALL,
> + RFKILL_OP_AIRPLANE_MODE_ACQUIRE,
> + RFKILL_OP_AIRPLANE_MODE_RELEASE,
> + RFKILL_OP_AIRPLANE_MODE_CHANGE,
>  };
 
> @@ -1199,7 +1202,7 @@ static ssize_t rfkill_fop_write(struct file
> *file, const char __user *buf,
>   if (copy_from_user(, buf, count))
>   return -EFAULT;
>  
> - if (ev.op != RFKILL_OP_CHANGE && ev.op !=
> RFKILL_OP_CHANGE_ALL)
> + if (ev.op < RFKILL_OP_CHANGE)
>   return -EINVAL;

You need to also reject invalid high values, like 27.

>   mutex_lock(_global_mutex);
>  
> + if (ev.op == RFKILL_OP_AIRPLANE_MODE_ACQUIRE) {
> + if (rfkill_apm_owned && !data->is_apm_owner) {
> + count = -EACCES;
> + } else {
> + rfkill_apm_owned = true;
> + data->is_apm_owner = true;
> + }
> + }
> +
> + if (ev.op == RFKILL_OP_AIRPLANE_MODE_RELEASE) {

It would probably be better to simply use "switch (ev.op)" and make the
default case do a reject.

>   if (ev.op == RFKILL_OP_CHANGE_ALL)
>   rfkill_update_global_state(ev.type, ev.soft);

Also moving the existing code inside the switch, of course.

johannes

Re: pull-request: mac80211-next 2016-02-26

[Johannes Berg]

On Tue, 2016-03-01 at 17:11 -0500, David Miller wrote:
> 
> There was a minor merge conflict with net/mac80211/debugfs.c, take
> a look and send me any fixups necessary.
> 

Ah yes, I'd wondered when that change was coming in, but missed that
you had it in net-next now.

Looks good, thanks!

johannes

Re: [PATCH net-next v2 3/3] macsec: introduce IEEE 802.1AE driver

[Johannes Berg]

Hi,

> +struct macsec_rx_sa_stats {
> + __u32 InPktsOK;
> + __u32 InPktsInvalid;
> + __u32 InPktsNotValid;
> + __u32 InPktsNotUsingSA;
> + __u32 InPktsUnusedSA;
> +};
> +
> +struct macsec_tx_sa_stats {
> + __u32 OutPktsProtected;
> + __u32 OutPktsEncrypted;
> +};

Just noticed this: is there a particular reason for using only __u32
here? The others all seem to use __u64.

> +static int macsec_dump_txsc(struct sk_buff *skb, struct
> netlink_callback *cb)
> +{
> + struct net *net = sock_net(skb->sk);
> + struct net_device *dev;
> + int dev_idx, d;
> +
> + dev_idx = cb->args[0];
> +
> + d = 0;
> + for_each_netdev(net, dev) {
> + struct macsec_secy *secy;
> +
> + if (d < dev_idx)
> + goto next;
> +
> + if (!netif_is_macsec(dev))
> + goto next;
> +
> + secy = _priv(dev)->secy;
> + if (dump_secy(secy, dev, skb, cb) < 0)
> + goto done;
> +next:
> + d++;
> + }
> +
> +done:
> + cb->args[0] = d;
> + return skb->len;
> +}

Maybe you should consider adding genl_dump_check_consistent() support
here, so userspace can figure out if the dump was really consistent, if
necessary.

To do this, you have to keep a global generation counter that changes
whenever this list changes (adding/removing macsec interfaces, I think)
and then set

cb->seq = macsec_generation_counter;

at the beginning of this function, and call
genl_dump_check_consistent() for each message in the loop.

Btw, aren't you missing locking here for for_each_netdev()?

johannes

Re: [PATCH net-next 1/3] uapi: add MACsec bits

[Johannes Berg]

On Wed, 2016-03-09 at 11:51 +0100, Sabrina Dubroca wrote:

> > > +#define DEFAULT_CIPHER_NAME "GCM-AES-128"
> > > +#define DEFAULT_CIPHER_ID   0x008002000101ULL
> > > +#define DEFAULT_CIPHER_ALT  0x0080C2000101ULL
> > 
> > > +enum macsec_attrs {
> > [...]
> > > + MACSEC_ATTR_CIPHER_SUITE,
> > 
> > should that be the ID, the alternate ID, or the string?
> 
> uh, the string is never actually used, I could get rid of it.

Heh, ok. I was actually wondering about the string but didn't look up
where/if it was used :)

> > > + MACSEC_ATTR_ICV_LEN,
> > > + MACSEC_TXSA_LIST,
> > > + MACSEC_RXSC_LIST,
> > > + MACSEC_TXSC_STATS,
> > > + MACSEC_SECY_STATS,
> > > + MACSEC_ATTR_PROTECT,
> > 
> > This seems a bit inconsistent, MACSEC_ATTR_* vs. MACSEC_*?
> 
> Only the MACSEC_ATTR_* can be set, the others are just for dumping.

Makes sense too.

I tend to prefer the names having a consistent prefix to indicate the
enum they're used in, which indicates the nesting level in nl80211 etc.
and makes it easier to figure out in the code that they're used
correctly (since accidentally mixing enums will give no warnings), but
that's just personal preference I guess.

> > > +enum macsec_sa_list_attrs {
> > > + MACSEC_SA_LIST_UNSPEC,
> > > + MACSEC_SA,
> > > + __MACSEC_ATTR_SA_LIST_MAX,
> > > + MACSEC_ATTR_SA_LIST_MAX = __MACSEC_ATTR_SA_LIST_MAX - 1,
> > > +};
> > 
> > Again, without documentation, it seems odd to have an enum with
> > just a
> > single useful entry? If you just wanted an array you don't need
> > this at
> > all? The netlink nesting properties could be specified somewhere.
> 
> Yes, in dump_secy(), I nest the TXSA_LIST, and then each SA
> underneath
> it.  I'm not sure how that would work without the list.  Can you have
> an array without the dummy level of nesting?


So, if I understand correctly, your message would be
[
   ..., /* e.g. IFINDEX, perhaps */
   TXSA_LIST -> [
       MACSEC_SA -> [
           MACSEC_ATTR_SA_AN -> ...,
           MACSEC_ATTR_SA_PN -> ...
       ],
       MACSEC_SA -> [...],
       MACSEC_SA -> [...],
       ...
   ],
]

right? That seems pretty odd to me, usually the same nesting level in
netlink shouldn't contain the same attribute multiple times, as I
understand it.

I *think* the way we do this in nl80211 is more customary, it would be
like this in your case (without defining the sa_list_attrs enum):

[
   ..., /* e.g. IFINDEX, perhaps */
   TXSA_LIST -> [
       1 -> [
           MACSEC_ATTR_SA_AN -> ...,
           MACSEC_ATTR_SA_PN -> ...
       ],
       2 -> [...],
       3 -> [...],
       ...
   ],
]

See, for example, nl80211_send_wowlan_patterns() which nests like this:

[
    NL80211_WOWLAN_TRIG_PKT_PATTERN -> [
        1 -> [
            NL80211_PKTPAT_MASK -> ...,
            NL80211_PKTPAT_PATTERN -> ...,
            NL80211_PKTPAT_OFFSET -> ...,
        ],
        2 -> [...],
        ...
    ]
]

> These stats are defined by the standard, but marked optional.
> A hardware device that doesn't implement some stat could just ignore
> it and export 0.

Fair enough. I tend to think there could be a difference between
knowing the value was 0 and knowing it wasn't provided, particularly
for the "exceptions" that you'd hope are mostly 0 under good operating
conditions, but I don't have a strong opinion about these or,
obviously, any idea about whether hardware might not be able to provide
them.

johannes

Re: [PATCH net-next 3/3] macsec: introduce IEEE 802.1AE driver

[Johannes Berg]

Hi,

Thanks for the comments.

> > > +struct gcm_iv {
> > > + union {
> > > + u8 secure_channel_id[8];
> > > + sci_t sci;
> > > + };
> > > + __be32 pn;
> > > +};
> > 
> > Should this be __packed?
> 
> I think that's not necessary here.

Yeah, there's probably no way a compiler could ever do something with
it that's not the same as packed, but it seems to me that just out of
convention structs that have some wire-format meaning should usually be
__packed. But it's your call. I'm not even entirely sure it has a wire-
format or similar meaning, although the name indicates it gets used
into the encryption and must be exactly these 12 octets.

> > That way, you have the same policy for everything and also don't
> > have
> > to play tricks with the aliasing since the top-level attributes
> > actually exist now, coming from the same namespace & policy.
> 
> That's a good idea, I'll have a look today.
> I don't really like the aliasing games I have to play, but I'd like
> to
> keep the RXSC attributes separate from the SA attributes, I think it
> looks cleaner (the first RFC had everything in the same policy:
> http://www.spinics.net/lists/netdev/msg358152.html).
> 

Ah, I see. Keeping it separate makes sense, but you can still achieve
that like this:

msg = [ifindex -> 7,
       rxsc -> [ sci -> 2, ...
               ],
       ...
      ]

with nested data. I'd also do the same for the stats and a whole bunch
of others too, I guess. I tend to imagine it as a kind of "dict of
dicts" (the message with nested).

Thanks,
johannes

Re: Question on rhashtable in worst-case scenario.

[Johannes Berg]

On Wed, 2016-03-30 at 09:52 -0700, Ben Greear wrote:

> If someone can fix rhashtable, then great.
> I read some earlier comments [1] back when someone else reported
> similar problems, and the comments seemed to indicate that rhashtable
> was broken in this manner on purpose to protect against hashing
> attacks.
> 
> If you are baking in this type of policy to what should be a basic
> data-type, then it is not useful for how it is being used in
> the mac80211 stack.
> 
> [1]  http://lkml.iu.edu/hypermail/linux/kernel/1512.2/01681.html
> 

That's not really saying it's purposely broken, that's more saying that
Herbert didn't see a point in fixing a case that has awful behaviour
already.

However, I'm confused now - we can much more easily live with
*insertion* failures, as the linked email indicates, than *deletion*
failures, which I think you had indicated originally. Are you really
seeing *deletion* failures?

If there are in fact *deletion* failures then I think we really need to
address those in rhashtable, no matter the worst-case behaviour of the
hashing or keys, since we should be able to delete entries in order to
get back to something reasonable. Looking at the code though, I don't
actually see that happening.

If you're seeing only *insertion* failures, which you indicated in the
root of this thread, then I think for the general case in mac80211 we
can live with that - we use a seeded jhash for the hash function, and
since in the general case we cannot accept entries with identical MAC
addresses to start with, it shouldn't be possible to run into this
problem under normal use cases.

In this case, I think perhaps you can just patch your local system with
the many interfaces connecting to the same AP to add the parameter
Herbert suggested (.insecure_elasticity = true in sta_rht_params). This
is, after all, very much a case that "normal" operation doesn't even
get close to.

johannes

Re: NETLINK_URELEASE non-bound socket problem (was: [PATCH] Fix local DoS in cfg80211 subsystem)

[Johannes Berg]

On Wed, 2016-04-06 at 11:20 +0300, Dmitrijs Ivanovs wrote:
> Hi Johannes!
> 
> I will prepare patch which does not send NETLINK_URELEASE for unbound
> sockets as you suggest. But I think protocol check in nl80211 is
> still needed because port_id is unique per-protocol.
> 

Yes, good point. Can you please send that as a separate patch? That one
should have a

Fixes: 026331c4d9b5 ("cfg80211/mac80211: allow registering for and sending 
action frames")

tag. I'll apply this one right away, but the other one should probably
go through Dave's tree.

Thanks,
johannes

[PATCH] netlink: don't send NETLINK_URELEASE for unbound sockets

[Johannes Berg]

From: Dmitry Ivanov <dmitrijs.ivan...@ubnt.com>

All existing users of NETLINK_URELEASE use it to clean up resources that
were previously allocated to a socket via some command. As a result, no
users require getting this notification for unbound sockets.

Sending it for unbound sockets, however, is a problem because any user
(including unprivileged users) can create a socket that uses the same ID
as an existing socket. Binding this new socket will fail, but if the
NETLINK_URELEASE notification is generated for such sockets, the users
thereof will be tricked into thinking the socket that they allocated the
resources for is closed.

In the nl80211 case, this will cause destruction of virtual interfaces
that still belong to an existing hostapd process; this is the case that
Dmitry noticed. In the NFC case, it will cause a poll abort. In the case
of netlink log/queue it will cause them to stop reporting events, as if
NFULNL_CFG_CMD_UNBIND/NFQNL_CFG_CMD_UNBIND had been called.

Fix this problem by checking that the socket is bound before generating
the NETLINK_URELEASE notification.

Cc: sta...@vger.kernel.org
Signed-off-by: Dmitry Ivanov <d...@ubnt.com>
Signed-off-by: Johannes Berg <johannes.b...@intel.com>
---
 net/netlink/af_netlink.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 215fc08c02ab..330ebd600f25 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -688,7 +688,7 @@ static int netlink_release(struct socket *sock)
 
skb_queue_purge(>sk_write_queue);
 
-   if (nlk->portid) {
+   if (nlk->portid && nlk->bound) {
struct netlink_notify n = {
.net = sock_net(sk),
.protocol = sk->sk_protocol,
-- 
2.7.0

Re: [PATCH v3] prism54: isl_38xx: Replace 'struct timeval'

[Johannes Berg]

> The patch was build-tested / debugged by removing the
> "if VERBOSE > SHOW_ERROR_MESSAGES" guards.

Stands to reason that we should just remove the (more or less) dead
code, since I don't think anyone really ever touches this driver any
more or will ever again ...

johannes

Re: [PATCH net-next WIP] ethtool: generic netlink policy

[Johannes Berg]

Hi,

> + [ETHTOOL_ATTR_FLAGS]= { .type = NLA_U32 },

I suppose this comes from the current API, but perhaps it'd be
worthwhile to make provision for more flags? Perhaps even using
NLA_BINARY and have "infinitely extensible" flags.

> + [ETHTOOL_ATTR_SSET_COUNT]   = { .type = NLA_U32

What do you need that for? Wouldn't it be sufficient to count the SSET
values returned? I can see how this would be useful for ioctl, but not
really for netlink messages?

> +static struct genl_ops ethtool_ops[] = {
> + {
> + .cmd = ETHTOOL_CMD_GET_SETTINGS,
> + .policy = ethtool_policy,
> + .doit = ethtool_get_settings,
> + },
[...]
> + {
> + .cmd = ETHTOOL_CMD_SET_MODULE_INFO,
> + .policy = ethtool_policy,
> + .doit = ethtool_set_module_info,
> + },
> +};

Shouldn't the ops have GENL_ADMIN_PERM as flags?

> +int ethtool_get_settings(struct net_device *dev, struct ethtool_cmd
> *cmd)
> +{
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(ethtool_get_settings);

I don't understand what kind of placeholder this was meant to be - but
why would it be exported? This part is called by the genl ops, so
doesn't really make sense?

It seems that instead these functions should be static, declared above
the ops, and call into the existing ethtool driver ops, based on
IFINDEX demultiplexing.

> +void ethtool_get_ethtool_stats(struct net_device *dev,
> +    struct ethtool_stats *stats,
> +    u64 *data)
> +{
> +
> + /* example the driver handler would do the below
> +  *
> + err = nla_put_u32(msg, PORT_ATTR_IFINDEX, ifindex);
> + if (err < 0)
> + goto err_out;
> +
> + err = nla_put_u32(msg, PORT_ATTR_FLAGS, flags);
> + if (err < 0)
> + goto err_out;
> +
> + err = nla_put_u32(msg, PORT_ATTR_SSET_COUNT,
> +   count);
> + if (err < 0)
> + goto err_out;
> +
> + nest = nla_nest_start(msg, PORT_ATTR_STATS);
> + for (i = 0; i < count; i++)
> + nla_put_u64(msg, PORT_ATTR_STAT, data[i]);
> + nla_nest_end(msg, nest);
> +
> +  */
> +}
> +EXPORT_SYMBOL_GPL(ethtool_get_ethtool_stats);

It seems possible that you could have a lot of ports, or a lot of
strings, or similar, so I think this should be a dumpit instead of a
doit handler.

Similar, perhaps, for the EEPROM thing, unless you provide API to query
the size first so the application can size it's recvmsg() buffer
appropriately - however doing so also requires a big message allocation
and more code in userspace, so I think having an "offset/length" type
of API combined with a dumpit rather than doit would be good for all of
the things that could get bigger or that might be extended in the
future.

johannes

Re: pull-request: mac80211-next 2016-04-06

[Johannes Berg]

On Wed, 2016-04-06 at 15:25 +0200, Johannes Berg wrote:
> Hi Dave,
> 
> For the 4.6 cycle, there's of course much more. The few things that
> 

Err, -next, so that's 4.7.

johannes

pull-request: mac80211-next 2016-04-06

[Johannes Berg]

Hi Dave,

For the 4.6 cycle, there's of course much more. The few things that
stand out are in the tag message below.

There's a pending patchset to add NAN (Neighbor Awareness Networking,
a WFA protocol) API support, but the API isn't ready yet. I might
have that later in time for 4.6.

Let me know if there's any problem.

Thanks,
johannes



The following changes since commit 05cf8077e54b20dddb756eaa26f3aeb5c38dd3cf:

  Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net (2016-04-01 
20:03:33 -0500)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next.git 
tags/mac80211-next-for-davem-2016-04-06

for you to fetch changes up to 4ce2bd9c4c1dfb416206ff1ad5283f6d24af4031:

  cfg80211: Allow reassociation to be requested with internal SME (2016-04-06 
15:09:28 +0200)


For the 4.6 cycle, we have a number of changes:
 * Bob's mesh mode rhashtable conversion, this includes
   the rhashtable API change for allocation flags
 * BSSID scan, connect() command reassoc support (Jouni)
 * fast (optimised data only) and support for RSS in mac80211 (myself)
 * various smaller changes


Akira Moroo (1):
  cfg80211: fix kernel-doc struct name

Arend van Spriel (1):
  nl80211: add feature for BSS selection support

Avraham Stern (1):
  ieee80211: support parsing Fine Timing Measurement action frame

Ayala Beker (2):
  cfg80211: allow userspace to specify client P2P PS support
  mac80211: track and tell driver about GO client P2P PS abilities

Bob Copeland (14):
  mac80211: mesh: move path tables into if_mesh
  mac80211: mesh: don't hash sdata in mpath tables
  mac80211: mesh: factor out common mesh path allocation code
  mac80211: mesh: embed known gates list in struct mesh_path
  rhashtable: accept GFP flags in rhashtable_walk_init
  mac80211: mesh: convert path table to rhashtable
  mac80211: mesh: fix crash in mesh_path_timer
  mac80211: mesh: handle failed alloc for rmc cache
  mac80211: mesh: use hlist for rmc cache
  mac80211: mesh: embed gates hlist head directly
  mac80211: mesh: reorder structure members
  mac80211: mesh: fix mesh path kerneldoc
  mac80211: mesh: fix cleanup for mesh pathtable
  mac80211: mesh: flush paths outside of plink lock

Emmanuel Grumbach (1):
  mac80211: Set global RRM capability

Felix Fietkau (4):
  mac80211: do not pass injected frames without a valid rate to the driver
  mac80211: minstrel_ht: improve sample rate skip logic
  mac80211: add A-MSDU tx support
  mac80211: minstrel_ht: set A-MSDU tx limits based on selected 
max_prob_rate

Johannes Berg (17):
  wext: unregister_pernet_subsys() on notifier registration failure
  mac80211: allow drivers to report CLOCK_BOOTTIME for scan results
  mac80211: remove sta_info debugfs sub-struct
  mac80211: don't start dynamic PS timer if not needed
  mac80211: clean up station flags debugfs
  mac80211: fix cipher scheme function name
  mac80211: avoid useless memory write on each frame RX
  mac80211: allow passing transmitter station on RX
  mac80211: count MSDUs in A-MSDU properly
  mac80211: move semicolon out of CALL_RXH macro
  mac80211: move averaged values out of rx_stats
  mac80211: remove rx_stats.last_rx update after sta alloc
  mac80211: add separate last_ack variable
  mac80211: fix last RX rate data consistency
  mac80211: fix RX u64 stats consistency on 32-bit platforms
  mac80211: add fast-rx path
  mac80211: enable collecting station statistics per-CPU

Jouni Malinen (5):
  cfg80211: Allow a scan request for a specific BSSID
  mac80211: Support a scan request for a specific BSSID
  mac80211_hwsim: Support a hw scan request for a specific BSSID
  cfg80211: Add option to specify previous BSSID for Connect command
  cfg80211: Allow reassociation to be requested with internal SME

João Paulo Rechi Vita (1):
  rfkill: Use switch to demux userspace operations

Lorenzo Bianconi (1):
  mac80211: parse VHT info in injected frames

Mohammed Shafi Shajakhan (1):
  mac80211: Remove unused variable in per STA debugfs struct

Sara Sharon (4):
  mac80211: allow not sending MIC up from driver for HW crypto
  mac80211: synchronize driver rx queues before removing a station
  mac80211: add NETIF_F_RXCSUM to features white list
  mac80211: enable starting BA session with custom timeout

Sven Eckelmann (2):
  mac80211: document only injected *_RADIOTAP_* flags
  mac80211: fix parsing of 40Mhz in injected radiotap header

 Documentation/networking/mac80211-injection.txt   |  17 +-
 drivers/net/wireless/ath/ath10k/htt_rx.c  |   2 +-
 drivers/net/wireless/ath/wcn36xx/txrx.c   |   2 +-
 drivers/net/wireless/intel/iwlwifi/dvm/rx.c

pull-request: mac80211 2016-04-06

[Johannes Berg]

Hi Dave,

First set of fixes for 4.6. Nothing really stands out.

Let me know if there's any problem.

Thanks,
johannes



The following changes since commit 05cf8077e54b20dddb756eaa26f3aeb5c38dd3cf:

  Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net (2016-04-01 
20:03:33 -0500)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211.git 
tags/mac80211-for-davem-2016-04-06

for you to fetch changes up to b4201cc4fc6e1c57d6d306b1f787865043d60129:

  mac80211: fix "warning: ‘target_metric’ may be used uninitialized" 
(2016-04-06 15:10:25 +0200)


For the current RC series, we have the following fixes:
 * TDLS fixes from Arik and Ilan
 * rhashtable fixes from Ben and myself
 * documentation fixes from Luis
 * U-APSD fixes from Emmanuel
 * a TXQ fix from Felix
 * and a compiler warning suppression from Jeff


Arik Nemtsov (3):
  mac80211: TDLS: always downgrade invalid chandefs
  mac80211: TDLS: change BW calculation for WIDER_BW peers
  mac80211: recalc min_def chanctx even when chandef is identical

Ben Greear (1):
  mac80211: ensure no limits on station rhashtable

Emmanuel Grumbach (2):
  mac80211: don't send deferred frames outside the SP
  mac80211: close the SP when we enqueue frames during the SP

Felix Fietkau (1):
  mac80211: fix AP buffered multicast frames with queue control and txq

Ilan Peer (1):
  mac80211: Fix BW upgrade for TDLS peers

Jeff Mahoney (1):
  mac80211: fix "warning: ‘target_metric’ may be used uninitialized"

Johannes Berg (1):
  mac80211: properly deal with station hashtable insert errors

Luis de Bethencourt (2):
  mac80211: add doc for RX_FLAG_DUP_VALIDATED flag
  mac80211: remove description of dropped member

 include/net/mac80211.h |  2 ++
 net/mac80211/chan.c|  4 +++-
 net/mac80211/ieee80211_i.h |  4 
 net/mac80211/mesh_hwmp.c   |  2 +-
 net/mac80211/sta_info.c| 14 +-
 net/mac80211/sta_info.h|  1 -
 net/mac80211/tdls.c| 43 +++
 net/mac80211/tx.c  | 13 +
 net/mac80211/vht.c | 30 +-
 9 files changed, 88 insertions(+), 25 deletions(-)

Re: [PATCH] mac80211: fix order of flag descriptions

[Johannes Berg]

On Fri, 2016-03-18 at 16:35 +, Luis de Bethencourt wrote:
> Fix order of mac80211_rx_flags description to match the enum.
> 
> Signed-off-by: Luis de Bethencourt 
> ---
> Hi,
> 
> I want ahead and fixed the order of the descriptions. checkpatch.pl
> was giving
> a warning to my previous patch and I had a hunch it was because the
> wrong order
> breaks the parser. Indeed it does and with this patch below
> checkpatch.pl does
> not complain about this flag descriptions anymore.
> 
Huh. I think we should fix checkpatch.pl instead. While the current
order isn't likely really good, I believe kernel-doc will output the
documentation in the order it's listed, and that can be useful for the
documentation output to group related things, even if their bits may be
further apart.

johannes

Re: [PATCH] iwlwifi: dvm: convert create_singlethread_workqueue() to alloc_workqueue()

[Johannes Berg]

On Thu, 2016-03-17 at 20:37 +0800, Eva Rachel Retuya wrote:
> Use alloc_workqueue() to allocate the workqueue instead of
> create_singlethread_workqueue() since the latter is deprecated and is
> scheduled for removal.

Scheduled where?

>  static void iwl_setup_deferred_work(struct iwl_priv *priv)
>  {
> - priv->workqueue = create_singlethread_workqueue(DRV_NAME);
> + priv->workqueue = alloc_workqueue(DRV_NAME, WQ_HIGHPRI |
> WQ_UNBOUND |
> +   WQ_MEM_RECLAIM, 1);

Seems like you should use alloc_ordered_workqueue() though? That also
gets you UNBOUND immediately, and the "1".

I'm not really sure HIGHPRI is needed either.

johannes

Re: [RFD] workqueue: WQ_MEM_RECLAIM usage in network drivers

[Johannes Berg]

On Sun, 2016-03-20 at 14:55 -0400, Tejun Heo wrote:
> If everything else is working, I'd be happy to throw in
> WQ_MEM_RECLAIM but I really don't want to add it if it doesn't
> actually achieve the goal.  Can a wireless person chime in here?
> 

I think for many wireless devices the workqueue, like for iwldvm that
was just changed, isn't in the packet path and thus less relevant.

For some, like SDIO based chips, it's more likely to be in the packet
path, so it would make sense to keep it.

Of course there's always a possibility of getting disconnected and then
not being able to re-establish the connection in memory pressure, but
that's not something we can control/predict (the disconnection). Can of
course also happen with wired if somebody pulls out the cable, but it's
less reliant on memory allocations to get back to working there, I
guess :)

johannes

Re: [iproute PATCH] libnetlink: Double the dump buffer size

[Johannes Berg]

On Fri, 2016-03-04 at 15:35 -0800, Stephen Hemminger wrote:
> 
> > There have been reports about 'ip addr' printing "Message
> > truncated" on
[...]
> I thought this was addressed in kernel by making the VF info
> optional.
> The netlink protocol is showing some strain, this is one of them.

I don't know how the dump is split here, but we had a similar issue
with nl80211 - originally each physical device info had to fit into a
single message (one message during dump for each device), but we fixed
that by having userspace to set a flag when it's able to understand a
multi-message single physical device info.

Before:
 msg1: phy1: A, B, C
 msg2: phy2: A, B, C

After:
 msg1: phy1: A
 msg2: phy1: B
 msg3: phy1: C
 msg4: phy1: D
 msg5: phy2: A
 [...]

For userspace not setting the flag, it only get partial info today for
compatibility (A, B, C, not D), but in our particular case this was
perfectly reasonable since it would be unaware of the new capabilities
anyway.

I don't know precisely enough what the issue at hand is to comment
whether such an approach will be feasible here, but it seems it could
be.

johannes

Re: [PATCH 2/2] mac80211: mesh: convert path table to rhashtable

[Johannes Berg]

On Wed, 2016-03-02 at 14:43 -0500, David Miller wrote:
> From: Bob Copeland 
> Date: Wed,  2 Mar 2016 10:09:20 -0500
> 
> > In the time since the mesh path table was implemented as an
> > RCU-traversable, dynamically growing hash table, a generic RCU
> > hashtable implementation was added to the kernel.
> > 
> > Switch the mesh path table over to rhashtable to remove some code
> > and also gain some features like automatic shrinking.
> > 
> > Cc: Thomas Graf 
> > Cc: netdev@vger.kernel.org
> > Signed-off-by: Bob Copeland 
> 
> Johannes, feel free to take both patches via your tree, thanks.

Will do, thanks.

johannes

Re: [PATCH 1/2] rhashtable: accept GFP flags in rhashtable_walk_init

[Johannes Berg]

On Wed, 2016-03-02 at 10:09 -0500, Bob Copeland wrote:
> In certain cases, the 802.11 mesh pathtable code wants to
> iterate over all of the entries in the forwarding table from
> the receive path, which is inside an RCU read-side critical
> section.  Enable walks inside atomic sections by allowing
> GFP_ATOMIC allocations for the walker state.
> 
> Change all existing callsites to pass in GFP_KERNEL.

Applied both. You missed 3 callsites, I've fixed those. I hope I got
them all :)

johannes

Re: [PATCH RFC 1/2] rhashtable: accept GFP flags in rhashtable_walk_init

[Johannes Berg]

On Sun, 2016-02-28 at 20:06 -0500, Bob Copeland wrote:
> In certain cases, the 802.11 mesh pathtable code wants to
> iterate over all of the entries in the forwarding table from
> the receive path, which is inside an RCU read-side critical
> section.  Enable walks inside atomic sections by allowing
> GFP_ATOMIC allocations for the walker state.
> 
> Change all existing callsites to pass in GFP_KERNEL.

Both look fine to me.

I see you have more patches for mesh, so this probably can't go through
net-next tree directly.

johannes

Re: [RFC/RFT] mac80211: implement fq_codel for software queuing

[Johannes Berg]

On Fri, 2016-02-26 at 14:09 +0100, Michal Kazior wrote:
> 
> +typedef u64 codel_time_t;

Do we really need this? And if yes, does it have to be in the public
header file? Why a typedef anyway?

> - * @txq_ac_max_pending: maximum number of frames per AC pending in
> all txq
> - *   entries for a vif.
> + * @txq_cparams: codel parameters to control tx queueing dropping
> behavior
> + * @txq_limit: maximum number of frames queuesd

typo - queued

> @@ -2133,7 +2155,8 @@ struct ieee80211_hw {
>   u8 uapsd_max_sp_len;
>   u8 n_cipher_schemes;
>   const struct ieee80211_cipher_scheme *cipher_schemes;
> - int txq_ac_max_pending;
> + struct codel_params txq_cparams;

Should this really be a parameter the driver determines?

> +static void ieee80211_if_setup_no_queue(struct net_device *dev)
> +{
> + ieee80211_if_setup(dev);
> + dev->priv_flags |= IFF_NO_QUEUE;
> + /* Note for backporters: use dev->tx_queue_len = 0 instead
> of IFF_ */

Heh. Remove that comment; we have an spatch in backports already :)

> --- a/net/mac80211/sta_info.h
> +++ b/net/mac80211/sta_info.h
> @@ -19,6 +19,7 @@
>  #include 
>  #include 
>  #include "key.h"
> +#include "codel_i.h"
>  
>  /**
>   * enum ieee80211_sta_info_flags - Stations flags
> @@ -327,6 +328,32 @@ struct mesh_sta {
>  
>  DECLARE_EWMA(signal, 1024, 8)
>  
> +struct txq_info;
> +
> +/**
> + * struct txq_flow - per traffic flow queue
> + *
> + * This structure is used to distinguish and queue different traffic
> flows
> + * separately for fair queueing/AQM purposes.
> + *
> + * @txqi: txq_info structure it is associated at given time
> + * @flowchain: can be linked to other flows for RR purposes
> + * @backlogchain: can be linked to other flows for backlog sorting
> purposes
> + * @queue: sk_buff queue
> + * @cvars: codel state vars
> + * @backlog: number of bytes pending in the queue
> + * @deficit: used for fair queueing balancing
> + */
> +struct txq_flow {
> + struct txq_info *txqi;
> + struct list_head flowchain;
> + struct list_head backlogchain;
> + struct sk_buff_head queue;
> + struct codel_vars cvars;
> + u32 backlog;
> + u32 deficit;
> +};
> +
>  /**
>   * struct sta_info - STA information
>   *
> diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
> index af584f7cdd63..f42f898cb8b5 100644
> --- a/net/mac80211/tx.c
> +++ b/net/mac80211/tx.c
> @@ -34,6 +34,7 @@
>  #include "wpa.h"
>  #include "wme.h"
>  #include "rate.h"
> +#include "codel.h"

> +static inline codel_time_t
> +custom_codel_get_enqueue_time(struct sk_buff *skb)

There are a lot of inlines here - first of all, do they all need to be
inline?

And secondly, perhaps it would make some sense to move this into
another file?

johannes

Re: [PATCHv2 08/10] rfkill: Use switch to demux userspace operations

[Johannes Berg]

On Tue, 2016-03-01 at 00:39 +0200, Jouni Malinen wrote:

> > I agree there is a difference in the logic here,

Gah. I thought I'd reviewed the logic and made sure there's no
difference ... :)

> >  thanks for taking the
> > time to point it out so clearly, and sorry for missing this. But AFAIU
> > userspace should not call RFKILL_OP_CHANGE with ev.type ==
> > RFKILL_TYPE_ALL, as RFKILL_OP_CHANGE is intended to be used to
> > block/unblock one RFKill switch, and it is not possible to create a
> > RFKill switch with type == RFKILL_TYPE_ALL (rfkill_alloc() would
> > return NULL).

> Interesting. Maybe Johannes can comment on that part since I think he
> wrote the code that interacts with kernel for the rfkill test cases.

So first of all, it seems that this argument is invalid since we can't break 
the ABI/API here; although perhaps if it's only a test case ...

Oh. It took me a while, but I see now. The original intent (I think)
was that with RFKILL_OP_CHANGE, the type would be ignored entirely. It
seems that the (my) original intent wouldn't have been to force
userspace to specify *both* the index and the type, but instead do

OP_CHANGE_ALL -> use type (possibly TYPE_ALL, ignoring idx)
OP_CHANGE     -> use idx (ignoring type)


The original code implemented it as follows:

                if (rfkill->idx != ev.idx && ev.op != RFKILL_OP_CHANGE_ALL)
continue;

-> check the idx only for OP_CHANGE

if (rfkill->type != ev.type && ev.type != RFKILL_TYPE_ALL)
continue;

-> check the type, allowing _ALL

Now, all userspace that I found sets the ev.type field to TYPE_ALL all
the time; and it had to given these checks.

e.g. from rfkill.py:

# idx, type, op, soft, hard
_event_struct = '@I'

[...]

    def block(self):
rfk = open('/dev/rfkill', 'w')
s = struct.pack(_event_struct, self.idx, TYPE_ALL, _OP_CHANGE, 1, 0)
rfk.write(s)
rfk.close()


This check, originally, probably should've been

                if (rfkill->type != ev.type && ev.type != RFKILL_TYPE_ALL &&
ev.op != RFKILL_OP_CHANGE)
continue;

to ignore the type entirely.

I'm fine with Jouni's change, preserving the original behaviour of
requiring TYPE_ALL or the correct type, but I'm tempted to simply
remove the type check entirely.

Thoughts?

johannes

pull-request: mac80211 2016-03-02

[Johannes Berg]

Hi Dave,

Here are a few more fixes for the current cycle; the MAINTAINERS patch
isn't really a fix for the tree, but helps the kbuild robot and clearly
can't be causing any regressions :)

Let me know if there's any problem.

Thanks,
johannes



The following changes since commit b86071528f3261ab592fad5b9b1a02aea3dcabf3:

  cfg80211: stop critical protocol session upon disconnect event (2016-02-23 
10:41:24 +0100)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211.git 
tags/mac80211-for-davem-2016-03-02

for you to fetch changes up to 2af8c4dc2e9c7038aefe9a822aa1df62e2b106b7:

  mac80211_hwsim: treat as part of mac80211 for MAINTAINERS (2016-03-02 
14:16:48 +0100)


Here are a few more fixes for the current cycle:
 * check GCMP encryption vs. fragmentation properly; we'd found
   this problem quite a while ago but waited for the 802.11 spec
   to be updated
 * fix RTS/CTS logic in minstrel_ht
 * fix RX of certain public action frames in AP mode
 * add mac80211_hwsim to MAC80211 in MAINTAINERS, this helps
   the kbuild robot pick up the right tree for it


Felix Fietkau (1):
  mac80211: minstrel_ht: fix a logic error in RTS/CTS handling

Johannes Berg (2):
  mac80211: check PN correctly for GCMP-encrypted fragmented MPDUs
  mac80211_hwsim: treat as part of mac80211 for MAINTAINERS

Jouni Malinen (1):
  mac80211: Fix Public Action frame RX in AP mode

 MAINTAINERS|  1 +
 net/mac80211/ieee80211_i.h |  2 +-
 net/mac80211/rc80211_minstrel_ht.c |  2 +-
 net/mac80211/rx.c  | 37 -
 4 files changed, 31 insertions(+), 11 deletions(-)

Re: [PATCH net-next 1/3] uapi: add MACsec bits

[Johannes Berg]

On Mon, 2016-03-07 at 18:12 +0100, Sabrina Dubroca wrote:

> +++ b/include/uapi/linux/if_macsec.h

Some bits of documentation in this file, regarding all the various
operations and attributes, might be nice. At least the netlink types?

E.g., given this:

> +#define DEFAULT_CIPHER_NAME "GCM-AES-128"
> +#define DEFAULT_CIPHER_ID   0x008002000101ULL
> +#define DEFAULT_CIPHER_ALT  0x0080C2000101ULL

> +enum macsec_attrs {
[...]
> + MACSEC_ATTR_CIPHER_SUITE,

should that be the ID, the alternate ID, or the string?

> + MACSEC_ATTR_ICV_LEN,
> + MACSEC_TXSA_LIST,
> + MACSEC_RXSC_LIST,
> + MACSEC_TXSC_STATS,
> + MACSEC_SECY_STATS,
> + MACSEC_ATTR_PROTECT,

This seems a bit inconsistent, MACSEC_ATTR_* vs. MACSEC_*?

> +enum macsec_sa_list_attrs {
> + MACSEC_SA_LIST_UNSPEC,
> + MACSEC_SA,
> + __MACSEC_ATTR_SA_LIST_MAX,
> + MACSEC_ATTR_SA_LIST_MAX = __MACSEC_ATTR_SA_LIST_MAX - 1,
> +};

Again, without documentation, it seems odd to have an enum with just a
single useful entry? If you just wanted an array you don't need this at
all? The netlink nesting properties could be specified somewhere.

> +enum macsec_rxsc_list_attrs {
> + MACSEC_RXSC_LIST_UNSPEC,
> + MACSEC_RXSC,

similarly here

> +enum macsec_rxsc_attrs {
> + MACSEC_ATTR_SC_UNSPEC,
> + /* use the same value to allow generic helper function, see
> +  * get_*_from_nl in drivers/net/macsec.c */
> + MACSEC_ATTR_SC_IFINDEX = MACSEC_ATTR_IFINDEX,
> + MACSEC_ATTR_SC_SCI = MACSEC_ATTR_SCI,

This seems odd, this must be nested inside the top-level attributes
since it's a single genl family, so why not use the top-level
attributes to start with?

If you need multiple you can use dump with multiple messages.

> +enum macsec_sa_attrs {
> + MACSEC_ATTR_SA_UNSPEC,
> + /* use the same value to allow generic helper function, see
> +  * get_*_from_nl in drivers/net/macsec.c */
> + MACSEC_ATTR_SA_IFINDEX = MACSEC_ATTR_IFINDEX,
> + MACSEC_ATTR_SA_SCI = MACSEC_ATTR_SCI,

likewise here

> +enum validation_type {
> + MACSEC_VALIDATE_DISABLED = 0,
> + MACSEC_VALIDATE_CHECK = 1,
> + MACSEC_VALIDATE_STRICT = 2,
> + __MACSEC_VALIDATE_MAX,
> +};
> +#define MACSEC_VALIDATE_MAX (__MACSEC_VALIDATE_MAX - 1)

everywhere else you put that into the enum, why not here?

> +struct macsec_rx_sc_stats {
> + __u64 InOctetsValidated;
> + __u64 InOctetsDecrypted;
> + __u64 InPktsUnchecked;
> + __u64 InPktsDelayed;
> + __u64 InPktsOK;
> + __u64 InPktsInvalid;
> + __u64 InPktsLate;
> + __u64 InPktsNotValid;
> + __u64 InPktsNotUsingSA;
> + __u64 InPktsUnusedSA;
> +};

It might be worth splitting those into attributes so that, if some
hardware offload can't provide all of the counters in the future, they
can just be left out of the netlink message?

johannes

Re: [PATCH net-next 3/3] macsec: introduce IEEE 802.1AE driver

[Johannes Berg]

On Mon, 2016-03-07 at 18:12 +0100, Sabrina Dubroca wrote:
> 
> +struct gcm_iv {
> + union {
> + u8 secure_channel_id[8];
> + sci_t sci;
> + };
> + __be32 pn;
> +};

Should this be __packed?

But the struct is confusing; sci_t is a host type (that depends on
endianness), and __be32 would seem to be a network type, how can they
both be in the same struct? Or does sci_t have to be __be64?

> +/**
> + * struct macsec_rx_sa - receive secure association
> + * @active
> + * @next_pn packet number expected for the next packet
> + * @lock protects next_pn manipulations
> + * @key key structure
> + * @stats per-SA stats
> + */
> +struct macsec_rx_sa {
> + bool active;
> + u32 next_pn;
> + spinlock_t lock;

If you put the spinlock first or at least next to active you can get
rid of some padding (on arches/configs where spinlock is small, at
least)

> +/**
> + * struct macsec_rx_sc - receive secure channel
> + * @sci secure channel identifier for this SC
> + * @active channel is active
> + * @sa array of secure associations
> + * @stats per-SC stats
> + */

Btw, all your kernel-doc comments are actually malformed, they're
missing a colon after the @member, e.g.

 @stats: per-SC stats

> +struct macsec_tx_sc {
> + bool active;
> + u8 encoding_sa;
> + bool encrypt;
> + bool send_sci;
> + bool end_station;
> + bool scb;
> + struct macsec_tx_sa __rcu *sa[4];


What's 4?

> +static sci_t make_sci(u8 *addr, __be16 port)
> +{
> + sci_t sci;
> +
> + memcpy(, addr, ETH_ALEN);
> + memcpy(((char *)) + ETH_ALEN, , sizeof(port));
> +
> + return sci;
> +}

Oh, maybe this explains my earlier question - looks like the sci_t
isn't really a 64-bit integer but some kind of structure.

Is there really much point in using a __bitwise u64 typedef, rather
than a small packed struct then?

> +/* minimum secure data length deemed "not short", see IEEE 802.1AE-
> 2006 9.7 */
> +#define MIN_NON_SHORT_LEN 48

I saw

> +#define MACSEC_SHORTLEN_THR 48

before, are they different? Seem very similar.

> + tx_sa->next_pn++;
> + if (tx_sa->next_pn == 0) {
> + pr_debug("PN wrapped, transitionning to !oper\n");

typo: transitioning

> +static const struct genl_ops macsec_genl_ops[] = {
> + {
> + .cmd = MACSEC_CMD_GET_TXSC,
> + .dumpit = macsec_dump_txsc,
> + .policy = macsec_genl_policy,
> + },
> + {
> + .cmd = MACSEC_CMD_ADD_RXSC,
> + .doit = macsec_add_rxsc,
> + .policy = macsec_genl_rxsc_policy,
> + .flags = GENL_ADMIN_PERM,

IMHO, having different policies for different operations is pretty
confusing. I now slowly start to understand why you had to do all this
aliasing with the IDs. However, perhaps it'd be better to define a top-
level attribute list with the ifindex etc., and make all the
*additional* data needed for RXSC operations for example go into a
nested attribute in the top-level.

That way, you have the same policy for everything and also don't have
to play tricks with the aliasing since the top-level attributes
actually exist now, coming from the same namespace & policy.

johannes

Re: [Odd commit author id merge via netdev]

[Johannes Berg]

On Fri, 2016-04-01 at 10:51 -0700, santosh shilimkar wrote:
> Hi Dave,
> 
> I noticed something odd while checking the recent
> commits of mine in kernel.org tree made it via netdev.
> 
> Don't know if its patchwork tool doing this.
> Usual author line in my git objects   :
>   Author: Santosh Shilimkar 
> 
> But the commits going via your tree seems to be like below..
>   Author: email-id 
> 
> Few more examples of the commits end of the email. Can this
> be fixed for future commits ? The git objects you pulled from
> my tree directly have right author format where as ones which
> are picked from patchworks seems to be odd.
> 

Patchwork does store this info somehow and re-use it, quite possibly
from the very first patch you ever sent. I think this bug was *just*
fixed in patchwork, but it'll probably be a while until that fix lands.

However, you can go and create a patchwork account with the real name,
associate it with all the email addresses you use and then I think
it'll pick it up. Not entirely sure though, you'll have to test it.

johannes

Re: Question on rhashtable in worst-case scenario.

[Johannes Berg]

On Fri, 2016-04-01 at 08:46 +0800, Herbert Xu wrote:
> On Thu, Mar 31, 2016 at 05:29:59PM +0200, Johannes Berg wrote:
> > 
> > 
> > Does removing this completely disable the "-EEXIST" error? I can't
> > say
> > I fully understand the elasticity stuff in
> > __rhashtable_insert_fast().
> What EEXIST error are you talking about? The only one that can be
> returned on insertion is if you're explicitly checking for dups
> which clearly can't be the case for you.

I was thinking about that one - it's not obvious to me from the code
how this "explicitly checking for dups" would be done or let's say how
rhashtable differentiates. But since it seems to work for Ben until
hitting a certain number of identical keys, surely that's just me not
understanding the code rather than anything else :)

johannes

Re: [RFC PATCH 4/5] mlx4: add support for fast rx drop bpf program

[Johannes Berg]

On Sat, 2016-04-02 at 23:38 -0700, Brenden Blanco wrote:
> 
> Having a common check makes sense. The tricky thing is that the type can
> only be checked after taking the reference, and I wanted to keep the
> scope of the prog brief in the case of errors. I would have to move the
> bpf_prog_get logic into dev_change_bpf_fd and pass a bpf_prog * into the
> ndo instead. Would that API look fine to you?

I can't really comment, I wasn't planning on using the API right now :)

However, what else is there that the driver could possibly do with the
FD, other than getting the bpf_prog?

> A possible extension of this is just to keep the bpf_prog * in the
> netdev itself and expose a feature flag from the driver rather than
> an ndo. But that would mean another 8 bytes in the netdev.

That also misses the signal to the driver when the program is
set/removed, so I don't think that works. I'd argue it's not really
desirable anyway though since I wouldn't expect a majority of drivers
to start supporting this.

johannes

Re: [RFC PATCH 0/5] Add driver bpf hook for early packet drop

[Johannes Berg]

On Sun, 2016-04-03 at 11:28 +0900, Lorenzo Colitti wrote:

> That said, getting BPF to the driver is part of the picture. On the
> chipsets we're targeting for APF, we're only seeing 2k-4k of memory
> (that's 256-512 BPF instructions) available for filtering code, which
> means that BPF might be too large.

That's true, but I think that as far as the userspace API is concerned
that shouldn't really be an issue. I think we can compile the BPF into
APF, similar to how BPF can be compiled into machine code today.
Additionally, I'm not sure we can realistically expect all devices to
really implement APF "natively", I think there's a good chance but
there's also a possibility of compiling to the native firmware
environment, for example.

johannes

Re: [PATCH] mac80211: add doc for RX_FLAG_DUP_VALIDATED flag

[Johannes Berg]

On Fri, 2016-03-18 at 16:09 +, Luis de Bethencourt wrote:
> Add documentation for the flag for duplication check.
> 
> Fixes the following warning when running make htmldocs:
> warning: Enum value 'RX_FLAG_DUP_VALIDATED' not described in enum
> 'mac80211_rx_flags'
> 
Applied, thanks.

johannes

NETLINK_URELEASE non-bound socket problem (was: [PATCH] Fix local DoS in cfg80211 subsystem)

[Johannes Berg]

Hi Dmitrijs,

Thanks for reporting this problem.

> The patch below corrects this problem in kernel space. 

I don't think that this is correct, there are four more users of
NETLINK_URELEASE (nfnetlink, NFC), and afaict all of them have the same
bug as nl80211.

Rather than fix all of them, I think we should simply not report
NETLINK_URELEASE for netlink sockets that weren't bound; if any user
comes up that requires them later we could add a new event instead.

I can't find what commit introduced this code, it goes back before git
history, so I don't have the commit log. Maybe it was done for
nfnetlink log/queue? Certainly both nl80211 and NFC are much newer.

> Also, it is
> recommended to ensure that user-space applications are not using
> user-supplied port_id for netlink sockets (which is default in
> libnl-tiny for example).

This I think we should remove from the commit log - it's misleading and
there's no point.

johannes

Re: [PATCH] mac80211: remove description of dropped member

[Johannes Berg]

On Fri, 2016-03-18 at 19:23 +, Luis de Bethencourt wrote:
> Commit 976bd9efdae6 ("mac80211: move beacon_loss_count into ifmgd")
> removed the member from the sta_info struct but the description
> stayed
> lingering. Remove it.
> 
Also applied.

johannes

Re: Question on rhashtable in worst-case scenario.

[Johannes Berg]

On Tue, 2016-03-29 at 09:16 -0700, Ben Greear wrote:
> Looks like rhashtable has too much policy in it to properly deal with
> cases where there are too many hash collisions, so I am going to work
> on reverting it's use in mac80211.

I'm not really all that happy with that approach - can't we fix the
rhashtable? It's a pretty rare corner case that many keys really are
identical and no kind of hash algorithm, but it seems much better to
still deal with it than to remove the rhashtable usage and go back to
hand-rolling something.

johannes

Re: Question on rhashtable in worst-case scenario.

[Johannes Berg]

On Wed, 2016-03-30 at 21:55 +0800, Herbert Xu wrote:

> Well to start with you should assess whether you really want to
> hash multiple objects with the same key.  In particular, can an
> adversary generate a large number of such objects?

No, the only reason this happens is local - if you take the single
hardware and connect it to the same AP many times. This is what Ben is
doing - he's creating virtual interfaces on top of the same physical
hardware, and then connection all of these to the same AP, mostly for
testing the AP.

> If your conclusion is that yes you really want to do this, then
> we have the parameter insecure_elasticity that you can use to
> disable the rehashing based on chain length.

But we really don't want that either - in the normal case where you
don't create all these virtual interfaces for testing, you have a
certain number of peers that can vary a lot (zero to hundreds, in
theory thousands) where you *don't* have the same key, so we still want
to have the rehashing if the chains get longer in that case.

It's really just the degenerate case that Ben is creating locally
that's causing a problem, afaict, though it's a bit disconcerting that
rhashtable in general can cause strange failures at delete time.

johannes

Re: Question on rhashtable in worst-case scenario.

[Johannes Berg]

On Thu, 2016-03-31 at 08:13 -0700, Ben Greear wrote:
> 
> I see insertion failure, and then later, if of course fails to delete
> as well since it was never inserted to begin with.  There is no good
> way to deal with insertion error, so just need to fix the hashtable.

Oh, that's an oversight in mac80211 - it should be dealing with
insertion failures properly. This isn't really a problem either,
although it will lead to errors in your particular case.

https://p.sipsolutions.net/cf77c78d69a231d4.txt

johannes

Re: Question on rhashtable in worst-case scenario.

[Johannes Berg]

On Thu, 2016-03-31 at 15:50 +0800, Herbert Xu wrote:
> On Thu, Mar 31, 2016 at 09:46:45AM +0200, Johannes Berg wrote:
> > 
> > 
> > In this case, I think perhaps you can just patch your local system
> > with
> > the many interfaces connecting to the same AP to add the parameter
> > Herbert suggested (.insecure_elasticity = true in sta_rht_params).
> > This
> > is, after all, very much a case that "normal" operation doesn't
> > even
> > get close to.
> I think you should just turn it on everywhere for mac80211.  Chain
> length checks simply don't make sense when you allow duplicate
> keys in the hash table.

Yes, that's a good point, and we can - in certain corner cases - end up
with duplicate keys even in normal operation.

Does removing this completely disable the "-EEXIST" error? I can't say
I fully understand the elasticity stuff in __rhashtable_insert_fast().

johannes

Re: [RFC PATCH 4/5] mlx4: add support for fast rx drop bpf program

[Johannes Berg]

On Fri, 2016-04-01 at 18:21 -0700, Brenden Blanco wrote:

> +static int mlx4_bpf_set(struct net_device *dev, int fd)
> +{
[...]
> + if (prog->type != BPF_PROG_TYPE_PHYS_DEV) {
> + bpf_prog_put(prog);
> + return -EINVAL;
> + }
> + }

Why wouldn't this check be done in the generic code that calls
ndo_bpf_set()?

johannes