linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Johannes Berg <johannes@sipsolutions.net>
To: Matthias May <matthias.may@neratec.com>
Cc: linux-wireless@vger.kernel.org
Subject: Re: [PATCH] cfg80211: check correct maximum bandwidth for quarter and half rate.
Date: Wed, 10 Jun 2015 14:35:49 +0200	[thread overview]
Message-ID: <1433939749.3145.6.camel@sipsolutions.net> (raw)
In-Reply-To: <55782CE5.9010300@neratec.com>

On Wed, 2015-06-10 at 14:26 +0200, Matthias May wrote:

> I'm not sure what exactly you mean that the handle_channel_custom needs 
> to loop through 5/10/20.
> The freq_reg_info_regd sets the disabled flag on the channel at init.

That's kinda my point - this isn't true. freg_req_info_regd() *doesn't*
change any channel flags, it just returns some information, and through
freq_reg_info() this is even exported to drivers.

> This is not while trying to start operation on a channel, because the 
> channel is already disabled.
> Or do you mean to actually have different sets of flags for the 
> different bandwidths?

I mean basically what you did below, except handle it explicitly at the
outer level.

> I see that it's erroneously possible to run a 10/20MHz channel on 5175, 
> even though this shouldn't be allowed.
> However setting the proper flags bw_flags should fix this.
> 
> Expanding the patch with the patch below ensures that one can't start 
> operation on a channel which it's not allowed on.
> Or is there a better way?

I was thinking you should extend freq_reg_info_regd() to get the desired
bandwidth, in which case you'd have to call it three times (5, 10, 20).

However, you could also make the API so that freq_reg_info_regd() gets
the *minimum* bandwidth [that the caller is willing to handle].

That way, you could set the argument to 20 MHz (for now) in
freq_reg_info(), and to 5 MHz in the other callers that you're modifying
(with the proposed additional patch) to actually handle the returned
bandwidth lower than 20 MHz.



This is really the crux of the problem - with your patch,
freq_reg_info_regd() and thus freq_reg_info() can return something that
doesn't fit the requested channel. You're changing the semantics of this
function to no longer mean "return the rule that fits a 20MHz channel
around the given center frequency" but to mean "return a rule around the
given center frequency" - no matter how wide that rule can allow
channels to be.

If you change the function to pass in a "minimum required bandwidth"
argument *and* change the checking as suggested with the combination, I
think it'll be OK.

johannes


      reply	other threads:[~2015-06-10 12:35 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-09 15:27 [PATCH] cfg80211: check correct maximum bandwidth for quarter and half rate Matthias May
2015-06-09 20:29 ` Johannes Berg
2015-06-10 12:26   ` Matthias May
2015-06-10 12:35     ` Johannes Berg [this message]

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=1433939749.3145.6.camel@sipsolutions.net \
    --to=johannes@sipsolutions.net \
    --cc=linux-wireless@vger.kernel.org \
    --cc=matthias.may@neratec.com \
    /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).