linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Johannes Berg <johannes@sipsolutions.net>
To: Mihai Moldovan <ionic@ionic.de>
Cc: linux-wireless@vger.kernel.org
Subject: Re: mac80211_hwsim: enable Beacon protection [a483e29ca07fb4eee2d7c7ee67c919d352fa4091]
Date: Thu, 30 Jul 2020 16:12:17 +0200	[thread overview]
Message-ID: <c0591d784c1593e6df02e6f5932ba05cee59d08b.camel@sipsolutions.net> (raw)
In-Reply-To: <c6de3886-1dbe-180d-d37b-ebccfba83933@ionic.de>

+list

> As mentioned in the commit message, you've pulled Jouni's patch, but applied it
> to hwsim only.

Yes, intentionally.

> Unfortunately, that leaves beacon protection for any "real" driver disabled, so
> this new feature will be pretty much unusable and (apart from hwsim for testing)
> dead code (for now).

Yes, intentionally. It's just barely done with interop testing, to some
extent...

> I understand the reasoning that it's not clear that drivers can handle this
> correctly (i.e., not modify data after it has been signed), but isn't that a bit
> too conservative?

It's not that "it's not clear". We know for a fact that some drivers
(e.g. iwlwifi) cannot handle this correctly.

So the only thing we can do is have the drivers advertise when they can
do it, which is exactly what all these commits do.

> After all, BIGTK/BEACON_PROT will only be used if explicitly turned on, mostly
> by hostapd, or any other AP software. In the worst case, i.e., if drivers update
> data after the fact, the connection just wouldn't work. That might be
> unfortunate,

I wouldn't really say "that's unfortunate". That'd be a bug!

Also, the intent is that at least wpa_supplicant would automatically
enable this if available, once the feature is more mature across various
implementations.

> but I can at least confirm that ath9k seems to handle this well.

Then you should submit a patch to ath9k similar like the one for hwsim
to enable it.

> I've been using protected management frames with that driver and a patch set
> backported to 5.6.x for a few months now without obvious hiccups, so there seem
> to at least be some drivers that *do* work.

:)

> Crucially, though, I don't see any potential for regressions here. If it's
> disabled in the AP software, all of this stuff just won't be used, just like it
> always has been the case in the past. Otherwise, it *might* be buggy, but you'd
> never know. Unless people patch their kernel to enable this feature, they won't
> be able to enable and test it in the first place.

This will change...

> Would you reconsider your decision and enable the feature in net/mac80211/main.c
> as well?

No.

johannes


           reply	other threads:[~2020-07-30 14:12 UTC|newest]

Thread overview: expand[flat|nested]  mbox.gz  Atom feed
 [parent not found: <c6de3886-1dbe-180d-d37b-ebccfba83933@ionic.de>]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=c0591d784c1593e6df02e6f5932ba05cee59d08b.camel@sipsolutions.net \
    --to=johannes@sipsolutions.net \
    --cc=ionic@ionic.de \
    --cc=linux-wireless@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).