pull-request: mac80211-next 2019-07-31

pull-request: mac80211-next 2019-07-31

[Johannes Berg]

Hi Dave,

There's a fair number of changes here, so I thought I'd get them out.
I've included two Intel driver cleanups because Luca is on vacation,
I'm covering for him, and doing it all in one tree let me merge all
of the patches at once (including mac80211 that depends on that);
Kalle is aware.

Also, though this isn't very interesting yet, I've started looking at
weaning the wireless subsystem off the RTNL for all operations, as it
can cause significant lock contention, especially with slow USB devices.
The real patches for that are some way off, but one preparation here is
to use generic netlink's parallel_ops=true, to avoid trading one place
with contention for another in the future, and to avoid adding more
genl_family_attrbuf() usage (since that's no longer possible with the
parallel_ops setting).

Please pull and let me know if there's any problem.

Thanks,
johannes



The following changes since commit 00c33afbf9dd06f77a2f15117cd4bdc2a54b51d7:

  net: mvneta: use devm_platform_ioremap_resource() to simplify code (2019-07-25 17:28:11 -0700)

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-2019-07-31

for you to fetch changes up to f39b07fdfb688724fedabf5507e15eaf398f2500:

  mac80211: HE STA disassoc due to QOS NULL not sent (2019-07-31 13:26:41 +0200)

----------------------------------------------------------------
We have a reasonably large number of changes:
 * lots more HE (802.11ax) support, particularly things
   relevant for the the AP side, but also mesh support
 * debugfs cleanups from Greg
 * some more work on extended key ID
 * start using genl parallel_ops, as preparation for
   weaning ourselves off RTNL and getting parallelism
 * various other changes all over

----------------------------------------------------------------
Alexander Wetzel (3):
      mac80211_hwsim: Extended Key ID API update
      mac80211: Simplify Extended Key ID API
      mac80211: AMPDU handling for rekeys with Extended Key ID

Ard Biesheuvel (1):
      lib80211: use crypto API ccm(aes) transform for CCMP processing

Christophe JAILLET (1):
      mac80211_hwsim: Fix a typo in the name of function 'mac80211_hswim_he_capab()'

Colin Ian King (1):
      mac80211: add missing null return check from call to ieee80211_get_sband

Denis Kenzior (2):
      nl80211: document uapi for CMD_FRAME_WAIT_CANCEL
      nl80211: Include wiphy address setup in NEW_WIPHY

Emmanuel Grumbach (1):
      mac80211: pass the vif to cancel_remain_on_channel

Erik Stromdahl (1):
      mac80211: add tx dequeue function for process context

Greg Kroah-Hartman (4):
      iwlwifi: dvm: no need to check return value of debugfs_create functions
      iwlwifi: mvm: remove unused .remove_sta_debugfs callback
      mac80211: remove unused and unneeded remove_sta_debugfs callback
      cfg80211: no need to check return value of debugfs_create functions

Johannes Berg (6):
      cfg80211: clean up cfg80211_inform_single_bss_frame_data()
      cfg80211: don't parse MBSSID if transmitting BSS isn't created
      cfg80211: give all multi-BSSID BSS entries the same timestamp
      mac80211_hwsim: fill boottime_ns in netlink RX path
      cfg80211: use parallel_ops for genl
      nl80211: add strict start type

John Crispin (10):
      mac80211: add support for parsing ADDBA_EXT IEs
      mac80211: add xmit rate to struct ieee80211_tx_status
      mac80211: propagate struct ieee80211_tx_status into ieee80211_tx_monitor()
      mac80211: add struct ieee80211_tx_status support to ieee80211_add_tx_radiotap_header
      mac80211: HE: add Spatial Reuse element parsing support
      mac80211: fix ieee80211_he_oper_size() comment
      mac80211: propagate HE operation info into bss_conf
      mac80211: add support for the ADDBA extension element
      cfg80211: add support for parsing OBBS_PD attributes
      mac80211: allow setting spatial reuse parameters from bss_conf

Karthikeyan Periyasamy (1):
      mac80211: reject zero MAC address in add station

Lorenzo Bianconi (1):
      mac80211: add IEEE80211_KEY_FLAG_GENERATE_MMIE to ieee80211_key_flags

Michael Vassernis (1):
      cfg80211: fix dfs channels remain DFS_AVAILABLE after ch_switch

Sergey Matyukevich (2):
      cfg80211: refactor cfg80211_bss_update
      cfg80211: fix duplicated scan entries after channel switch

Shay Bar (1):
      mac80211: HE STA disassoc due to QOS NULL not sent

Sven Eckelmann (1):
      mac80211: implement HE support for mesh

 drivers/net/wireless/ath/ath10k/mac.c             |   3 +-
 drivers/net/wireless/ath/ath9k/main.c             |   3 +-
 drivers/net/wireless/intel/iwlwifi/dvm/rs.c       |  29 +--
 drivers/net/wireless/intel/iwlwifi/dvm/rs.h       |   4 -
 drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c |   3 +-
 drivers/net/wireless/intel/iwlwifi/mvm/rs.c       |   5 -
 drivers/net/wireless/mac80211_hwsim.c             |  20 +-
 drivers/net/wireless/rsi/rsi_91x_mac80211.c       |   3 +-
 drivers/net/wireless/ti/wlcore/main.c             |   3 +-
 include/linux/ieee80211.h                         |  63 ++++-
 include/net/cfg80211.h                            |  15 ++
 include/net/mac80211.h                            |  53 ++++-
 include/uapi/linux/nl80211.h                      |  31 ++-
 net/mac80211/agg-rx.c                             |  72 +++++-
 net/mac80211/cfg.c                                |   7 +-
 net/mac80211/debugfs.c                            |   3 +-
 net/mac80211/driver-ops.h                         |   8 +-
 net/mac80211/he.c                                 |  39 ++++
 net/mac80211/ht.c                                 |   2 +-
 net/mac80211/ieee80211_i.h                        |  17 +-
 net/mac80211/key.c                                |  16 +-
 net/mac80211/main.c                               |  18 +-
 net/mac80211/mesh.c                               |  62 +++++
 net/mac80211/mesh.h                               |   4 +
 net/mac80211/mesh_plink.c                         |  12 +-
 net/mac80211/mlme.c                               |   7 +-
 net/mac80211/offchannel.c                         |   5 +-
 net/mac80211/rate.h                               |   9 -
 net/mac80211/sta_info.c                           |   1 -
 net/mac80211/status.c                             | 180 +++++++++++++--
 net/mac80211/trace.h                              |   7 +-
 net/mac80211/tx.c                                 |   5 +-
 net/mac80211/util.c                               |  60 +++++
 net/mac80211/wpa.c                                |   6 +-
 net/wireless/Kconfig                              |   2 +
 net/wireless/core.c                               |  17 +-
 net/wireless/core.h                               |   2 +
 net/wireless/lib80211_crypt_ccmp.c                | 197 +++++++---------
 net/wireless/nl80211.c                            | 182 ++++++++++++---
 net/wireless/scan.c                               | 269 ++++++++++++++--------
 40 files changed, 1070 insertions(+), 374 deletions(-)

Re: pull-request: mac80211-next 2019-07-31

[David Miller]

From: Johannes Berg <johannes@sipsolutions.net>
Date: Wed, 31 Jul 2019 17:50:56 +0200

> There's a fair number of changes here, so I thought I'd get them out.
> I've included two Intel driver cleanups because Luca is on vacation,
> I'm covering for him, and doing it all in one tree let me merge all
> of the patches at once (including mac80211 that depends on that);
> Kalle is aware.
> 
> Also, though this isn't very interesting yet, I've started looking at
> weaning the wireless subsystem off the RTNL for all operations, as it
> can cause significant lock contention, especially with slow USB devices.
> The real patches for that are some way off, but one preparation here is
> to use generic netlink's parallel_ops=true, to avoid trading one place
> with contention for another in the future, and to avoid adding more
> genl_family_attrbuf() usage (since that's no longer possible with the
> parallel_ops setting).
> 
> Please pull and let me know if there's any problem.

Pulled, thanks Johannes.

I'm sure people like Florian Westphal will also appreciate your RTNL
elimination work...

Re: pull-request: mac80211-next 2019-07-31

[Arnd Bergmann]

On Wed, Jul 31, 2019 at 5:53 PM Johannes Berg <johannes@sipsolutions.net> wrote:
> John Crispin (10):
>       mac80211: add support for parsing ADDBA_EXT IEs
>       mac80211: add xmit rate to struct ieee80211_tx_status
>       mac80211: propagate struct ieee80211_tx_status into ieee80211_tx_monitor()
>       mac80211: add struct ieee80211_tx_status support to ieee80211_add_tx_radiotap_header
>       mac80211: HE: add Spatial Reuse element parsing support

Hi Johannes and John,

It looks like one of the last additions pushed the stack usage over
the 1024 byte limit
for 32-bit architectures:

net/mac80211/mlme.c:4063:6: error: stack frame size of 1032 bytes in
function 'ieee80211_sta_rx_queued_mgmt' [-Werror,-Wframe-larger-than=]

struct ieee802_11_elems is fairly large, and just grew another two pointers.
When ieee80211_rx_mgmt_assoc_resp() and ieee80211_assoc_success()
are inlined into ieee80211_sta_rx_queued_mgmt(), there are three copies
of this structure, which is slightly too much.

Marking any of those functions as __noinline_for_stack would shut up the
warning but not fix the underlying issue. Silencing the warning might
be enough if there is a fairly short call chain leading up to
ieee80211_sta_rx_queued_mgmt(). Another option would be a dynamic
allocation.

Thoughts?

      Arnd

Re: pull-request: mac80211-next 2019-07-31

[Johannes Berg]

Hi Arnd,

> It looks like one of the last additions pushed the stack usage over
> the 1024 byte limit
> for 32-bit architectures:
> 
> net/mac80211/mlme.c:4063:6: error: stack frame size of 1032 bytes in
> function 'ieee80211_sta_rx_queued_mgmt' [-Werror,-Wframe-larger-than=]
> 
> struct ieee802_11_elems is fairly large, and just grew another two pointers.
> When ieee80211_rx_mgmt_assoc_resp() and ieee80211_assoc_success()
> are inlined into ieee80211_sta_rx_queued_mgmt(), there are three copies
> of this structure, which is slightly too much.

Hmm. I guess that means the compiler isn't smart enough to make the
copies from the inlined sub-functions alias each other? I mean, if I
have

fn1(...) { struct ... elems1; ... }
fn2(...) { struct ... elems2; ... }

fn(...)
{
  fn1();
  fn2();
}

then it could reasonably use the same stack memory for elems1 and
elems2, at least theoretically, but you're saying it doesn't do that I
guess?

It could even do that for different BBs, in theory ...

If it does, I'd have suggested to move the code from the outer function
inside the "case IEEE80211_STYPE_ACTION:" block into a new sub-function, 
but that won't work then.

I don't think dynamic allocation would be nice - but we could manually
do this by passing the elems pointer into the
ieee80211_rx_mgmt_assoc_resp() and ieee80211_assoc_success() functions.


Why do you say 32-bit btw, it should be *bigger* on 64-bit, but I didn't
see this ... hmm.

johannes

Re: pull-request: mac80211-next 2019-07-31

[Arnd Bergmann]

On Mon, Oct 28, 2019 at 10:52 AM Johannes Berg
<johannes@sipsolutions.net> wrote:
>
> > It looks like one of the last additions pushed the stack usage over
> > the 1024 byte limit
> > for 32-bit architectures:
> >
> > net/mac80211/mlme.c:4063:6: error: stack frame size of 1032 bytes in
> > function 'ieee80211_sta_rx_queued_mgmt' [-Werror,-Wframe-larger-than=]
> >
> > struct ieee802_11_elems is fairly large, and just grew another two pointers.
> > When ieee80211_rx_mgmt_assoc_resp() and ieee80211_assoc_success()
> > are inlined into ieee80211_sta_rx_queued_mgmt(), there are three copies
> > of this structure, which is slightly too much.
>
> Hmm. I guess that means the compiler isn't smart enough to make the
> copies from the inlined sub-functions alias each other? I mean, if I
> have
>
> fn1(...) { struct ... elems1; ... }
> fn2(...) { struct ... elems2; ... }
>
> fn(...)
> {
>   fn1();
>   fn2();
> }
>
> then it could reasonably use the same stack memory for elems1 and
> elems2, at least theoretically, but you're saying it doesn't do that I
> guess?

No, that's not the problem here (it can happen if the compiler is
unable to prove
the object lifetimes are non-overlapping).

What we have here are multiple functions that are in the same call chain:

fn1()
{
     struct ieee802_11_elems e ;
}

fn2()
{
   struct ieee802_11_elems e;
  ...
   fn1();
}

fn3()
{
   struct ieee802_11_elems e;
  ...
   fn2();
}

Here, the object lifetimes actually do overlap, so the compiler cannot easily
optimize that away.

> I don't think dynamic allocation would be nice - but we could manually
> do this by passing the elems pointer into the
> ieee80211_rx_mgmt_assoc_resp() and ieee80211_assoc_success() functions.

Ah, so you mean we can reuse the objects manually? I think that would
be great. I could not tell if that's possible when reading the source, but
if you can show that this works, that would be an easy solution.

> Why do you say 32-bit btw, it should be *bigger* on 64-bit, but I didn't
> see this ... hmm.

That is correct. For historic reasons, both the total amount of stack space
per thread and the warning limit on 64 bit are twice the amount that we
have on 32-bit kernels, so even though the problem is more serious on
64-bit architectures, we do not see a warning about it because we remain
well under the warning limit.

      Arnd

Re: pull-request: mac80211-next 2019-07-31

[Johannes Berg]

On Mon, 2019-10-28 at 11:53 +0100, Arnd Bergmann wrote:
> On Mon, Oct 28, 2019 at 10:52 AM Johannes Berg
> <johannes@sipsolutions.net> wrote:
> > > It looks like one of the last additions pushed the stack usage over
> > > the 1024 byte limit
> > > for 32-bit architectures:
> > > 
> > > net/mac80211/mlme.c:4063:6: error: stack frame size of 1032 bytes in
> > > function 'ieee80211_sta_rx_queued_mgmt' [-Werror,-Wframe-larger-than=]
> > > 
> > > struct ieee802_11_elems is fairly large, and just grew another two pointers.
> > > When ieee80211_rx_mgmt_assoc_resp() and ieee80211_assoc_success()
> > > are inlined into ieee80211_sta_rx_queued_mgmt(), there are three copies
> > > of this structure, which is slightly too much.
> > 
> > Hmm. I guess that means the compiler isn't smart enough to make the
> > copies from the inlined sub-functions alias each other? I mean, if I
> > have
> > 
> > fn1(...) { struct ... elems1; ... }
> > fn2(...) { struct ... elems2; ... }
> > 
> > fn(...)
> > {
> >   fn1();
> >   fn2();
> > }
> > 
> > then it could reasonably use the same stack memory for elems1 and
> > elems2, at least theoretically, but you're saying it doesn't do that I
> > guess?
> 
> No, that's not the problem here (it can happen if the compiler is
> unable to prove the object lifetimes are non-overlapping).
> 
> What we have here are multiple functions that are in the same call chain:
> 
> fn1()
> {
>      struct ieee802_11_elems e ;
> }
> 
> fn2()
> {
>    struct ieee802_11_elems e;
>   ...
>    fn1();
> }
> 
> fn3()
> {
>    struct ieee802_11_elems e;
>   ...
>    fn2();
> }
> 
> Here, the object lifetimes actually do overlap, so the compiler cannot easily
> optimize that away.

Ah, yes, you're right. I didn't look closely enough, sorry.

> > I don't think dynamic allocation would be nice - but we could manually
> > do this by passing the elems pointer into the
> > ieee80211_rx_mgmt_assoc_resp() and ieee80211_assoc_success() functions.
> 
> Ah, so you mean we can reuse the objects manually? I think that would
> be great. I could not tell if that's possible when reading the source, but
> if you can show that this works, that would be an easy solution.

Now that I look more closely, I'm not even sure why we parse it again in
ieee80211_assoc_success() after already having done it in
_rx_mgmt_assoc_resp(), they're doing exactly the same thing:

        ieee802_11_parse_elems(pos, len - (pos - (u8 *)mgmt), false, &elems,
                               mgmt->bssid, assoc_data->bss->bssid);

(need to track the variables a bit more closely, but ...)

So I think we can even just avoid duplicate work.

And for the third copy - it's in a different switch case. Do you think
we could rely on the compiler being able to prove non-overlapping
lifetime? Or better to just pass a pointer down to
_rx_mgmt_assoc_resp()?

> > Why do you say 32-bit btw, it should be *bigger* on 64-bit, but I didn't
> > see this ... hmm.
> 
> That is correct. For historic reasons, both the total amount of stack space
> per thread and the warning limit on 64 bit are twice the amount that we
> have on 32-bit kernels, so even though the problem is more serious on
> 64-bit architectures, we do not see a warning about it because we remain
> well under the warning limit.

Ah, ok, thanks.

johannes

Re: pull-request: mac80211-next 2019-07-31

[Johannes Berg]

On Mon, 2019-10-28 at 11:53 +0100, Arnd Bergmann wrote:
> 
> > Why do you say 32-bit btw, it should be *bigger* on 64-bit, but I didn't
> > see this ... hmm.
> 
> That is correct. For historic reasons, both the total amount of stack space
> per thread and the warning limit on 64 bit are twice the amount that we
> have on 32-bit kernels, so even though the problem is more serious on
> 64-bit architectures, we do not see a warning about it because we remain
> well under the warning limit.

Hmm, but I have:

CONFIG_FRAME_WARN=1024

in my compilation?

Maybe I do in fact have merging of the storage space, and you don't? I
see another copy of it that shouldn't be merged ("bss_elems"), but ...

Hmm.

johannes

Re: pull-request: mac80211-next 2019-07-31

[Johannes Berg]

On Mon, 2019-10-28 at 12:08 +0100, Johannes Berg wrote:
> On Mon, 2019-10-28 at 11:53 +0100, Arnd Bergmann wrote:
> > > Why do you say 32-bit btw, it should be *bigger* on 64-bit, but I didn't
> > > see this ... hmm.
> > 
> > That is correct. For historic reasons, both the total amount of stack space
> > per thread and the warning limit on 64 bit are twice the amount that we
> > have on 32-bit kernels, so even though the problem is more serious on
> > 64-bit architectures, we do not see a warning about it because we remain
> > well under the warning limit.
> 
> Hmm, but I have:
> 
> CONFIG_FRAME_WARN=1024
> 
> in my compilation

But the compiler decides not to inline it, for whatever reason. Only if
I mark it as __always_inline, does it actually inline it.

But it does seem to merge the storage, if I inline only assoc_success()
I get 888 bytes (after the fix), if I inline also
ieee80211_rx_mgmt_assoc_resp() then I get 904 bytes.

johannes