All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ben Greear <greearb@candelatech.com>
To: Johannes Berg <johannes@sipsolutions.net>,
	Oleksij Rempel <linux@rempel-privat.de>,
	"linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>,
	ath10k <ath10k@lists.infradead.org>,
	kirtika@google.com
Subject: Re: Setting single rate in ath10k broken by "reject/clear user rate mask if not usable"
Date: Wed, 18 Oct 2017 14:30:38 -0700	[thread overview]
Message-ID: <0bfb289d-a9fa-03d9-f9c7-cc0d01b0bd43@candelatech.com> (raw)
In-Reply-To: <1508360527.2674.61.camel@sipsolutions.net>

On 10/18/2017 02:02 PM, Johannes Berg wrote:
> On Wed, 2017-10-18 at 13:51 -0700, Ben Greear wrote:
>> "
>> I guess you could implement this part? I.e. iterating the clients and
>> checking that they all support the rate that is set. However, then you
>> also need to implement that this gets reset when a new client that
>> doesn't support this rate connects.
>> "
>>
>> The first part seems OK, but the second seems like a pain.  Maybe just
>> keep a new client from being able to connect at all if it doesn't support
>> any available rates?
>
> I suppose if you reject the NEW_STATION command, then hostapd will
> reject the association though, so it's probably not hard to do.
> However, I'm not really sure why you'd want that? If you do want that
> then basically you're just implemented a very roundabout way of adding
> this rate to the basic rates, so you might as well just add it and work
> with the current basic rates check?

If a user specifies a specific rate or rate-set, then I do not think we
should quietly change it out from under them.  So, we could check at the
time the rate-set is applied (all current peers).  We can reject the change
at that point if one of the peers does not support any common rates.
And, whenever a peer tries to be associated, we can check that there is some common rateset
in place.  If there is no common rateset, then reject the association
one way or another.

> We'll need to be a little careful what happens with client-mode
> interfaces, which is where we actually observed hitting the WARN_ON()
> about not having any rates at all, but I think I already put a reset of
> the rates there anyway if they're not compatible with the AP. At least
> that's easier because there's only one client.

It would be easy to configure a station to do VHT MCS 8 only, and then
it would never be able to associate with an HT AP, for instance.  I don't
think this should be a WARN_ON event, just a failure.  It would be great
if we could get a useful error message back to the caller, but I am not
sure how feasible that is with the current netlink and mac80211 code.

If your main concern is hitting a WARN_ON, maybe just change it to a
quieter error message or remove it entirely and just return a failure
code?

Thanks,
Ben

>
> johannes
>


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

WARNING: multiple messages have this Message-ID (diff)
From: Ben Greear <greearb@candelatech.com>
To: Johannes Berg <johannes@sipsolutions.net>,
	Oleksij Rempel <linux@rempel-privat.de>,
	"linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>,
	ath10k <ath10k@lists.infradead.org>,
	kirtika@google.com
Subject: Re: Setting single rate in ath10k broken by "reject/clear user rate mask if not usable"
Date: Wed, 18 Oct 2017 14:30:38 -0700	[thread overview]
Message-ID: <0bfb289d-a9fa-03d9-f9c7-cc0d01b0bd43@candelatech.com> (raw)
In-Reply-To: <1508360527.2674.61.camel@sipsolutions.net>

On 10/18/2017 02:02 PM, Johannes Berg wrote:
> On Wed, 2017-10-18 at 13:51 -0700, Ben Greear wrote:
>> "
>> I guess you could implement this part? I.e. iterating the clients and
>> checking that they all support the rate that is set. However, then you
>> also need to implement that this gets reset when a new client that
>> doesn't support this rate connects.
>> "
>>
>> The first part seems OK, but the second seems like a pain.  Maybe just
>> keep a new client from being able to connect at all if it doesn't support
>> any available rates?
>
> I suppose if you reject the NEW_STATION command, then hostapd will
> reject the association though, so it's probably not hard to do.
> However, I'm not really sure why you'd want that? If you do want that
> then basically you're just implemented a very roundabout way of adding
> this rate to the basic rates, so you might as well just add it and work
> with the current basic rates check?

If a user specifies a specific rate or rate-set, then I do not think we
should quietly change it out from under them.  So, we could check at the
time the rate-set is applied (all current peers).  We can reject the change
at that point if one of the peers does not support any common rates.
And, whenever a peer tries to be associated, we can check that there is some common rateset
in place.  If there is no common rateset, then reject the association
one way or another.

> We'll need to be a little careful what happens with client-mode
> interfaces, which is where we actually observed hitting the WARN_ON()
> about not having any rates at all, but I think I already put a reset of
> the rates there anyway if they're not compatible with the AP. At least
> that's easier because there's only one client.

It would be easy to configure a station to do VHT MCS 8 only, and then
it would never be able to associate with an HT AP, for instance.  I don't
think this should be a WARN_ON event, just a failure.  It would be great
if we could get a useful error message back to the caller, but I am not
sure how feasible that is with the current netlink and mac80211 code.

If your main concern is hitting a WARN_ON, maybe just change it to a
quieter error message or remove it entirely and just return a failure
code?

Thanks,
Ben

>
> johannes
>


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

  reply	other threads:[~2017-10-18 21:30 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-10 20:54 Setting single rate in ath10k broken by "reject/clear user rate mask if not usable" Ben Greear
2017-10-10 20:54 ` Ben Greear
2017-10-11  4:37 ` Not able to set single rate in ath10k (backports-4.14-rc2-1) KAVITA MATHUR
2017-10-11  8:02 ` Setting single rate in ath10k broken by "reject/clear user rate mask if not usable" Johannes Berg
2017-10-11  8:02   ` Johannes Berg
2017-10-11  8:07   ` Johannes Berg
2017-10-11  8:07     ` Johannes Berg
2017-10-11 14:51   ` Ben Greear
2017-10-11 14:51     ` Ben Greear
2017-10-18  7:33     ` Johannes Berg
2017-10-18  7:33       ` Johannes Berg
2017-10-18 14:50       ` Ben Greear
2017-10-18 14:50         ` Ben Greear
2017-10-18 17:56         ` Oleksij Rempel
2017-10-18 17:56           ` Oleksij Rempel
2017-10-18 20:34           ` Johannes Berg
2017-10-18 20:34             ` Johannes Berg
2017-10-18 20:51             ` Ben Greear
2017-10-18 20:51               ` Ben Greear
2017-10-18 21:02               ` Johannes Berg
2017-10-18 21:02                 ` Johannes Berg
2017-10-18 21:30                 ` Ben Greear [this message]
2017-10-18 21:30                   ` Ben Greear
2017-10-25 15:17                   ` Johannes Berg
2017-10-25 15:17                     ` Johannes Berg
2017-10-25 16:13                     ` Ben Greear
2017-10-25 16:13                       ` Ben Greear
2017-10-27 20:15                       ` Johannes Berg
2017-10-27 20:15                         ` Johannes Berg
2017-10-27 20:41                         ` Ben Greear
2017-10-27 20:41                           ` Ben Greear
2017-11-13 10:09                           ` Johannes Berg
2017-11-13 10:09                             ` Johannes Berg
2017-11-13 17:05                             ` Ben Greear
2017-11-13 17:05                               ` Ben Greear

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=0bfb289d-a9fa-03d9-f9c7-cc0d01b0bd43@candelatech.com \
    --to=greearb@candelatech.com \
    --cc=ath10k@lists.infradead.org \
    --cc=johannes@sipsolutions.net \
    --cc=kirtika@google.com \
    --cc=linux-wireless@vger.kernel.org \
    --cc=linux@rempel-privat.de \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.