API to regulatory control (1)

API to regulatory control (1)

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 4001 bytes --]

Ok since I'm having trouble communicating this via IRC and other people
may have input too, let me summarise how I think regulatory control
should be implemented. This is a high-level overview and maybe Luis's
stuff actually implements things this way, but I can't really tell
because the interesting information (i.e. the API) is buried in
thousands of lines of code of boring definitions. Not to dismiss the
effort to collect all this info, but I think it's rather unimportant at
this stage.

So let me first explain how I see a mac80211-based driver communicating
with mac80211/cfg80211 on regulatory info.

I think each driver should include a few arrays that define which
channels the hardware is capable of operating in. These arrays may be
statically or dynamically allocated depending on the needs. This
basically consists of:

struct ieee80211_channel {
	/* first part set by driver */
	u16 freq;		/* in MHz */
	int hardware_value;	/* arbitrary */
	enum ieee80211_modulation modulation; /* CCK, OFDM */

	u8 supp_dbm_power_max;	/* maximum TX power device supports */

	/* second part updated by regulatory agent */
	u32 flags;		/* disabled, active scan, passive scan, IBSS, DFS, ... */
	u8 perm_dbm_ptmp_power_max; /* permitted pmtp power */
	u8 perm_dbm_ptp_power_max;  /* permitted ptp power */
};

[NOTE: I think there's a restriction on OFDM somewhere so we must have a
channel struct for each modulation to be able to disable OFDM/force
lower power with OFDM]

These are aggregated into an array as such:
struct ieee80211_channels {
	struct ieee80211_channel *band2ghz;
	struct ieee80211_channel *band5ghz;
	int num_band2ghz, num_band5ghz;
};

It would not really be necessary to have a separate list for the two
bands currently in use but it is definitely a lot easier on the code
later.

Now, the driver (say b43) has defined a struct ieee80211_channels it may
call say b43_supported_channels.

Next, the driver needs to say which modulations/bitrates it supports:
struct ieee80211_rate {
	/* stuff from the driver */
	enum ieee80211_modulation modulation; /* OFDM, CCK, CCK_SHORTPRE */
	u32 rate; /* in units of 100 Kbps */
	/* internal stuff */
	TBD;
};

Again, it's stuffed together into an array:
struct ieee80211_rates {
	struct ieee80211_rate *rates;
	int num_rates;
};

so let's say we now have a b43_supported_rates variable although we
really have one for A, B, BG and ABG devices.

NOTE: All this rate stuff is *NOT* interesting for regulatory. *IFF*
there are regulatory restrictions on bitrates anywhere then we need to
make this part of the channels array!

Ok enough of the declarations.

Now, before the driver registers it's hardware struct, it puts both
&b43_supported_channels into the "struct wiphy". This is how both
mac80211 and cfg80211 get told about it. &b43_supported_rates is put
into struct ieee80211_hw, only mac80211 needs to know about it, fullmac
drivers never need to show this information to anything outside except
maybe for some cfg80211 *hooks*.

**For mac80211 drivers, this is it!**

Internally, mac80211 or fullmac drivers of course need to check the
channel structs internally when they are asked to move to a different
channel or change TX power.

At this point, however, the regulatory agent API only starts. First of
all, the channel list has already been registered with cfg80211 because
a pointer is part of struct wiphy. Then, when by any way possible, the
regulatory restrictions change (user setting, ...) the fields marked
"second part updated by regulatory agent" are updated for each wiphy and
a notifier chain is invoked that drivers may register on to get updates.
I'm not sure how important this notifier chain is but it can't hurt to
have it.

From what I can see, this finishes the channel restrictions API. Since
none of the information in the channels list is specific to some device,
it may be allocated statically.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 190 bytes --]

API to regulatory control (2)

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 1776 bytes --]

So in the first part I covered how information on channel restrictions
is communicated between the central regulatory control agent and drivers
and/or mac80211.

This second part is rather short, but important.

Devices that are capable of operating in a wide range of regulatory
restrictions usually also have country information encoded in their
EEPROM. I understand that Intel devices will need to make this EEPROM
information part of their channels list because the firmware rejects
tuning to other channels, but most devices fully enforce these
restrictions in software.

Hence, it is necessary that drivers are able to set the regulatory
domain they think they are operating in. It would probably be possible
to override by the user, but when the user has just booted it would be
good to be able to read the EEPROM of a wireless card to set the initial
operating domain.

For this, we need two exported functions:

void cfg80211_regulatory_set_alpha2(const char *alpha2);
void cfg80211_regulatory_set_domain(int ieee80211_domain);

These functions, when called, internally look up the value and, if
found, update each registered wiphy's channels list for the new
restrictions. They also set the internal domain to the value given
(well, I suppose the _set_domain function internally looks up an alpha2
value and calls the other one). Also, it will call the notifier chain I
talked about earlier so drivers can be notified of the change if
necessary.

Similarly, userspace can set the alpha2 value through some
nl80211/configfs API which will also cause the the same updates and
notifier chain calls.

I think this covers all the public API to regulatory control. Did I miss
anything? Everything else should be internal.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 190 bytes --]

Re: API to regulatory control (2)

[Luis R. Rodriguez]

On 9/22/07, Johannes Berg <johannes@sipsolutions.net> wrote:
> So in the first part I covered how information on channel restrictions
> is communicated between the central regulatory control agent and drivers
> and/or mac80211.
>
> This second part is rather short, but important.
>
> Devices that are capable of operating in a wide range of regulatory
> restrictions usually also have country information encoded in their
> EEPROM. I understand that Intel devices will need to make this EEPROM
> information part of their channels list because the firmware rejects
> tuning to other channels, but most devices fully enforce these
> restrictions in software.
>
> Hence, it is necessary that drivers are able to set the regulatory
> domain they think they are operating in. It would probably be possible
> to override by the user, but when the user has just booted it would be
> good to be able to read the EEPROM of a wireless card to set the initial
> operating domain.
>
> For this, we need two exported functions:
>
> void cfg80211_regulatory_set_alpha2(const char *alpha2);
> void cfg80211_regulatory_set_domain(int ieee80211_domain);
>
> These functions, when called, internally look up the value and, if
> found, update each registered wiphy's channels list for the new
> restrictions. They also set the internal domain to the value given
> (well, I suppose the _set_domain function internally looks up an alpha2
> value and calls the other one). Also, it will call the notifier chain I
> talked about earlier so drivers can be notified of the change if
> necessary.
>
> Similarly, userspace can set the alpha2 value through some
> nl80211/configfs API which will also cause the the same updates and
> notifier chain calls.
>
> I think this covers all the public API to regulatory control. Did I miss
> anything? Everything else should be internal.

That's the plan ;) For now I only have support to change regdomain
through configfs though, I have to add support for the driver changing
the regdomain though, which I think should just consists of a loop
over al wiphys and updating their channel/power info. The rare
exception we'd have to deal with is when the user is on a channel
already which is going to be disabled but like we spoke about this --
we can just disable it, I still think we should notify the user but
whatever, this is a pretty rare thing to deal with, we'll figure it
out.

  Luis

Re: API to regulatory control (2)

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 295 bytes --]

On Sat, 2007-09-22 at 12:33 -0400, Luis R. Rodriguez wrote:

> That's the plan ;) 

:)

> I still think we should notify the user but
> whatever, this is a pretty rare thing to deal with, we'll figure it
> out.

Yeah, I figured we could provide a notifier chain for that.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 190 bytes --]

Re: API to regulatory control (1)

[Luis R. Rodriguez]

On 9/22/07, Johannes Berg <johannes@sipsolutions.net> wrote:
> Ok since I'm having trouble communicating this via IRC and other people
> may have input too, let me summarise how I think regulatory control
> should be implemented. This is a high-level overview and maybe Luis's
> stuff actually implements things this way, but I can't really tell
> because the interesting information (i.e. the API) is buried in
> thousands of lines of code of boring definitions. Not to dismiss the
> effort to collect all this info, but I think it's rather unimportant at
> this stage.

In essence you are addressing here what would be series II of my
patchset which I haven't sent. This consists of the actual integration
of the regualtory work onto wireless drivers themselves.

> So let me first explain how I see a mac80211-based driver communicating
> with mac80211/cfg80211 on regulatory info.

See ;)

> I think each driver should include a few arrays that define which
> channels the hardware is capable of operating in. These arrays may be
> statically or dynamically allocated depending on the needs. This
> basically consists of:
>
> struct ieee80211_channel {
>         /* first part set by driver */
>         u16 freq;               /* in MHz */
>         int hardware_value;     /* arbitrary */
>         enum ieee80211_modulation modulation; /* CCK, OFDM */
>
>         u8 supp_dbm_power_max;  /* maximum TX power device supports */
>
>         /* second part updated by regulatory agent */
>         u32 flags;              /* disabled, active scan, passive scan, IBSS, DFS, ... */
>         u8 perm_dbm_ptmp_power_max; /* permitted pmtp power */
>         u8 perm_dbm_ptp_power_max;  /* permitted ptp power */
> };

ACK -- note here how you've changed ieee80211_channel struct. There's
a reason why I haven't done so although the regulatory work does
provide means to allocate and provide all this other information you
have entered here. I'll explain towards the end of this e-mail.

> [NOTE: I think there's a restriction on OFDM somewhere so we must have a
> channel struct for each modulation to be able to disable OFDM/force
> lower power with OFDM]

We'll need to elaborate on this later.

> These are aggregated into an array as such:
> struct ieee80211_channels {
>         struct ieee80211_channel *band2ghz;
>         struct ieee80211_channel *band5ghz;
>         int num_band2ghz, num_band5ghz;
> };
>
> It would not really be necessary to have a separate list for the two
> bands currently in use but it is definitely a lot easier on the code
> later.

ACK

> Now, the driver (say b43) has defined a struct ieee80211_channels it may
> call say b43_supported_channels.
>
> Next, the driver needs to say which modulations/bitrates it supports:
> struct ieee80211_rate {
>         /* stuff from the driver */
>         enum ieee80211_modulation modulation; /* OFDM, CCK, CCK_SHORTPRE */
>         u32 rate; /* in units of 100 Kbps */
>         /* internal stuff */
>         TBD;
> };
>
> Again, it's stuffed together into an array:
> struct ieee80211_rates {
>         struct ieee80211_rate *rates;
>         int num_rates;
> };

ACK -- however note that these are defined already by the IEEE "mode"
we operate in. The vendor rates and vendor modulations are the things
that will vary out of the norm I believe.

> so let's say we now have a b43_supported_rates variable although we
> really have one for A, B, BG and ABG devices.
>
> NOTE: All this rate stuff is *NOT* interesting for regulatory. *IFF*
> there are regulatory restrictions on bitrates anywhere then we need to
> make this part of the channels array!

I think it may have been Jouni who pointed this out for Japan at some
point. It was big news to me. If this is an exception we can treat it
as such just as with the FCC 3:1 rule between max IR and antenna_gain
for PtMP links, or Japan's rules for not allowing disabling active
scan on old 5GHz channels.

> Ok enough of the declarations.
>
> Now, before the driver registers it's hardware struct, it puts both
> &b43_supported_channels into the "struct wiphy".

Did you mean both &b43_supported_channels *and* &b43_supported_rates?
If not what did you mean by "both" here?

Anyway -- I now see what you were saying on IRC and I think its a
pretty good idea as the path I was following for series II of patches
was ab it different :-) But let me ask you this though -- is that
necessary? Since each mac80211 driver mode right now has a
ieee80211_channel array pointer can't we just have cfg80211 be aware
of this, at least for now?

> This is how both
> mac80211 and cfg80211 get told about it. &b43_supported_rates is put
> into struct ieee80211_hw, only mac80211 needs to know about it, fullmac
> drivers never need to show this information to anything outside except
> maybe for some cfg80211 *hooks*.

Hm, as I said above rate stuff seems pretty common amongst modes.
Can't we just assume if your card supports a certain IEEE-802.11 mode
that you'll have at least certain rates?

Then again -- its the vendor rates and vendor modulations which we can
implement exceptions for.

> **For mac80211 drivers, this is it!**

ACK

> Internally, mac80211 or fullmac drivers of course need to check the
> channel structs internally when they are asked to move to a different
> channel or change TX power.

Sure, cfg8021 can just do this. We just need to be sure cfg80211 won't
break anything.

> At this point, however, the regulatory agent API only starts. First of
> all, the channel list has already been registered with cfg80211 because
> a pointer is part of struct wiphy

If we take this approach, sure.

>  Then, when by any way possible, the
> regulatory restrictions change (user setting, ...) the fields marked
> "second part updated by regulatory agent" are updated for each wiphy

As of right now I think changing channel rules and power restrictions
(ir, eirp, and antenna gain) is all the regulatory agent should do.
Beyond that its news to me that regulatory laws say we have to change
other aspects of wireless communication.

> and
> a notifier chain is invoked that drivers may register on to get updates.
> I'm not sure how important this notifier chain is but it can't hurt to
> have it.

If all we have to do is update the channel flags and power
restrictions, why should drivers be informed about regulatory change?
I agree, it may be good to have, but I'm just wondering why we should
make this a priority right now if don't have a good reason for drivers
be informed about this.

I was thinking one simple way of doing this was to add to the wiphy
struct a reg timestamp or counter, and also have one for the central
reg agent. If the stamps don't match, well then we know the central
reg agent has changed the central regdomain and the driver can do
whatever work it needs to. A notifier seems much better than polling
though. But I just am not sure if we need this right now.

> From what I can see, this finishes the channel restrictions API. Since
> none of the information in the channels list is specific to some device,
> it may be allocated statically.

I don't quite get by what you mean in this last part. Can you
elaborate a bit further?

OK -- so now let me address why I haven't done some changes which you
have addressed yet. It's because I don't want to cause a headache to
driver developers and for introduction of this work it would mean A
LOT of work and changes. I rather introduce this work gently, only
modifying absolutely necessary things first.

If patchset series I work is embraced we can later push further
changes as cleanup patches for all drivers and such patches will be
easier work than trying to keep the work as a whole in synch with
current mac80211 changes and introducing a huge bomb patch which may
break some drivers.

  Luis

Re: API to regulatory control (1)

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 8472 bytes --]


> In essence you are addressing here what would be series II of my
> patchset which I haven't sent. This consists of the actual integration
> of the regualtory work onto wireless drivers themselves.

:)

> > [NOTE: I think there's a restriction on OFDM somewhere so we must have a
> > channel struct for each modulation to be able to disable OFDM/force
> > lower power with OFDM]
> 
> We'll need to elaborate on this later.

I'm not sure. I haven't looked up those restrictions but ISTR somebody
told me there were such restrictions. I'm happy to be proven wrong---it
can only simplify things.

> > Next, the driver needs to say which modulations/bitrates it supports:
> > struct ieee80211_rate {
> >         /* stuff from the driver */
> >         enum ieee80211_modulation modulation; /* OFDM, CCK, CCK_SHORTPRE */
> >         u32 rate; /* in units of 100 Kbps */
> >         /* internal stuff */
> >         TBD;
> > };
> >
> > Again, it's stuffed together into an array:
> > struct ieee80211_rates {
> >         struct ieee80211_rate *rates;
> >         int num_rates;
> > };
> 
> ACK -- however note that these are defined already by the IEEE "mode"
> we operate in. The vendor rates and vendor modulations are the things
> that will vary out of the norm I believe.

We don't really have a plan how to support vendor modulations because it
heavily influences calculations in the stack. Anybody have ideas? I'd
prefer if we'd defer that until later.

> > so let's say we now have a b43_supported_rates variable although we
> > really have one for A, B, BG and ABG devices.
> >
> > NOTE: All this rate stuff is *NOT* interesting for regulatory. *IFF*
> > there are regulatory restrictions on bitrates anywhere then we need to
> > make this part of the channels array!
> 
> I think it may have been Jouni who pointed this out for Japan at some
> point. It was big news to me. If this is an exception we can treat it
> as such just as with the FCC 3:1 rule between max IR and antenna_gain
> for PtMP links, or Japan's rules for not allowing disabling active
> scan on old 5GHz channels.

Ouch. Will have to change plans then.

Another thing I considered was to have a function to get "regulatory
OK", i.e. the only thing we'd have in regulatory code would be a few
functions like this:

int regulatory_check(freq, modulation, ...);
-> returns 0 or 1 (if allowed)

int regulatory_get_max_power(freq, modulation, ..., pmtp_or_ptp);

But I then thought we'd be calling this function a lot during scanning
so discarded this idea. However, with all the complexity here, maybe we
can revive it and provide some sort of "for each allowed channel" loop
or something too.

> > Now, before the driver registers it's hardware struct, it puts both
> > &b43_supported_channels into the "struct wiphy".
> 
> Did you mean both &b43_supported_channels *and* &b43_supported_rates?
> If not what did you mean by "both" here?

Sorry, scratch "both", I reworded the email last minute because I no
longer thought supported_rates was of any importance to regulatory.

> Anyway -- I now see what you were saying on IRC and I think its a
> pretty good idea as the path I was following for series II of patches
> was ab it different :-) 

I never meant to distract you from the ideas you have, I'm open for
other suggestions too. I didn't think long about this anyhow, just the
stuff I came up with after looking at it for a bit.

> But let me ask you this though -- is that
> necessary? Since each mac80211 driver mode right now has a
> ieee80211_channel array pointer can't we just have cfg80211 be aware
> of this, at least for now?

Do we actually need the modes? I'm a bit confused now.

> > This is how both
> > mac80211 and cfg80211 get told about it. &b43_supported_rates is put
> > into struct ieee80211_hw, only mac80211 needs to know about it, fullmac
> > drivers never need to show this information to anything outside except
> > maybe for some cfg80211 *hooks*.
> 
> Hm, as I said above rate stuff seems pretty common amongst modes.
> Can't we just assume if your card supports a certain IEEE-802.11 mode
> that you'll have at least certain rates?

Ahh. I see what you mean. Well, what I'd do is provide an array in
mac80211 that is EXPORT_SYMBOL'ed so you can simply refer to that if you
don't have any special hardware-values for the stuff. And maybe provide
allocation functions for when you do need special hardware-values. Then
only drivers that need really special stuff need to do all the array
building themselves, but in principle can.

> > Internally, mac80211 or fullmac drivers of course need to check the
> > channel structs internally when they are asked to move to a different
> > channel or change TX power.
> 
> Sure, cfg8021 can just do this. We just need to be sure cfg80211 won't
> break anything.

Hm. cfg80211 could do it but I think it might be wise to have it in
drivers since scanning will require the driver to do it anyway.

> > At this point, however, the regulatory agent API only starts. First of
> > all, the channel list has already been registered with cfg80211 because
> > a pointer is part of struct wiphy
> 
> If we take this approach, sure.

No need to, of course, we could register it explicitly as well.

> >  Then, when by any way possible, the
> > regulatory restrictions change (user setting, ...) the fields marked
> > "second part updated by regulatory agent" are updated for each wiphy
> 
> As of right now I think changing channel rules and power restrictions
> (ir, eirp, and antenna gain) is all the regulatory agent should do.
> Beyond that its news to me that regulatory laws say we have to change
> other aspects of wireless communication.

Right. I'm not sure about modulations (see above) but I agree with this.

> If all we have to do is update the channel flags and power
> restrictions, why should drivers be informed about regulatory change?
> I agree, it may be good to have, but I'm just wondering why we should
> make this a priority right now if don't have a good reason for drivers
> be informed about this.

Well, I think it's not too important when you think of channels, but say
you have an AP open and for some reason move into a new regulatory
domain, the TX power might need to be adjusted. This isn't important,
but implementing the hook to allow (notifier chain) is trivial so I
figured we might as well do it.

> I was thinking one simple way of doing this was to add to the wiphy
> struct a reg timestamp or counter, and also have one for the central
> reg agent. If the stamps don't match, well then we know the central
> reg agent has changed the central regdomain and the driver can do
> whatever work it needs to. A notifier seems much better than polling
> though. But I just am not sure if we need this right now.

Yeah I agree about polling, that sucks since we're all in the kernel.
But see above as for why I think we should do it anyway.

> > From what I can see, this finishes the channel restrictions API. Since
> > none of the information in the channels list is specific to some device,
> > it may be allocated statically.
> 
> I don't quite get by what you mean in this last part. Can you
> elaborate a bit further?

Well, I think that there's no need to allocate the channels array
dynamically, each driver can have just a single one indicating what the
hardware is capable of and use it multiple times if you say plug in 20
zd1211 devices.

> OK -- so now let me address why I haven't done some changes which you
> have addressed yet. It's because I don't want to cause a headache to
> driver developers and for introduction of this work it would mean A
> LOT of work and changes. I rather introduce this work gently, only
> modifying absolutely necessary things first.

I'm not sure. I think I prefer getting the API right and having some
driver pain than trying to introduce it over a few weeks and having
longer pain ;) But I suppose that's just me, you may well prefer to do
it slower and I won't push it.

> If patchset series I work is embraced

I think the patches you posted are pretty much unusable as they stand,
you should at least clean out the number of lines you have in headers
and the ugly macros that declare arrays (why is that anyway? does
openbsd really ship such code?)

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 190 bytes --]

Re: API to regulatory control (1)

[Luis R. Rodriguez]

On 9/24/07, Johannes Berg <johannes@sipsolutions.net> wrote:

> > > [NOTE: I think there's a restriction on OFDM somewhere so we must have a
> > > channel struct for each modulation to be able to disable OFDM/force
> > > lower power with OFDM]
> >
> > We'll need to elaborate on this later.
>
> I'm not sure. I haven't looked up those restrictions but ISTR somebody
> told me there were such restrictions. I'm happy to be proven wrong---it
> can only simplify things.
>
> > > Next, the driver needs to say which modulations/bitrates it supports:
> > > struct ieee80211_rate {
> > >         /* stuff from the driver */
> > >         enum ieee80211_modulation modulation; /* OFDM, CCK, CCK_SHORTPRE */
> > >         u32 rate; /* in units of 100 Kbps */
> > >         /* internal stuff */
> > >         TBD;
> > > };
> > >
> > > Again, it's stuffed together into an array:
> > > struct ieee80211_rates {
> > >         struct ieee80211_rate *rates;
> > >         int num_rates;
> > > };
> >
> > ACK -- however note that these are defined already by the IEEE "mode"
> > we operate in. The vendor rates and vendor modulations are the things
> > that will vary out of the norm I believe.
>
> We don't really have a plan how to support vendor modulations because it
> heavily influences calculations in the stack. Anybody have ideas? I'd
> prefer if we'd defer that until later.
>
> > > so let's say we now have a b43_supported_rates variable although we
> > > really have one for A, B, BG and ABG devices.
> > >
> > > NOTE: All this rate stuff is *NOT* interesting for regulatory. *IFF*
> > > there are regulatory restrictions on bitrates anywhere then we need to
> > > make this part of the channels array!
> >
> > I think it may have been Jouni who pointed this out for Japan at some
> > point. It was big news to me. If this is an exception we can treat it
> > as such just as with the FCC 3:1 rule between max IR and antenna_gain
> > for PtMP links, or Japan's rules for not allowing disabling active
> > scan on old 5GHz channels.
>
> Ouch. Will have to change plans then.

With what I have we can treat these things as exceptions, since I
think they are exceptions. So fortunately with what I have, I don't
have to change anything.

> Another thing I considered was to have a function to get "regulatory
> OK", i.e. the only thing we'd have in regulatory code would be a few
> functions like this:
>
> int regulatory_check(freq, modulation, ...);
> -> returns 0 or 1 (if allowed)
>
> int regulatory_get_max_power(freq, modulation, ..., pmtp_or_ptp);
>
> But I then thought we'd be calling this function a lot during scanning
> so discarded this idea. However, with all the complexity here, maybe we
> can revive it and provide some sort of "for each allowed channel" loop
> or something too.

At scan time we can just check the channel flags, those are always
supposed to be up to date if cfg80211 is in charge of updating all
wiphys with the new rules upon regulatory domain change.

> > Anyway -- I now see what you were saying on IRC and I think its a
> > pretty good idea as the path I was following for series II of patches
> > was ab it different :-)
>
> I never meant to distract you from the ideas you have, I'm open for
> other suggestions too. I didn't think long about this anyhow, just the
> stuff I came up with after looking at it for a bit.
>
> > But let me ask you this though -- is that
> > necessary? Since each mac80211 driver mode right now has a
> > ieee80211_channel array pointer can't we just have cfg80211 be aware
> > of this, at least for now?
>
> Do we actually need the modes? I'm a bit confused now.

No, you're right, It seems like we don't, yeah I don't think we do..
but I was just basing the model on the current mac80211 driver API,
which does have modes... if we are going to get rid of struct
ieee80211_hw_mode this is a pretty serious cleanup on the both
mac80211 and driver side. It'll be good, make driver code smaller but
it seems like a lot of work right now.

Hmm, it seems if we take this approach we'll need to stage this into
separate development parts to not cause too enforce too many changes
on drivers. Instead of the 2-series I suggested how about 3 series:

* I - Introduce new regdomain modules integrate with cfg80211
* II - Minimal mac80211 support with current driver infrastructure
* III - Start optimizing to remove all unnecessary materials

We may just want to go directly to series III but based on the amount
of complaints from current driver writers I think its best to use 3
stages for this... Comments?

> > > This is how both
> > > mac80211 and cfg80211 get told about it. &b43_supported_rates is put
> > > into struct ieee80211_hw, only mac80211 needs to know about it, fullmac
> > > drivers never need to show this information to anything outside except
> > > maybe for some cfg80211 *hooks*.
> >
> > Hm, as I said above rate stuff seems pretty common amongst modes.
> > Can't we just assume if your card supports a certain IEEE-802.11 mode
> > that you'll have at least certain rates?
>
> Ahh. I see what you mean. Well, what I'd do is provide an array in
> mac80211 that is EXPORT_SYMBOL'ed so you can simply refer to that if you
> don't have any special hardware-values for the stuff.

This seems like it would certainly optimize driver code. Same comment
as above applies here though.

> And maybe provide
> allocation functions for when you do need special hardware-values. Then
> only drivers that need really special stuff need to do all the array
> building themselves, but in principle can.

Hmm, yeah you know the nice thing about the current interface is that
we can support 'vendor' rates by using the current model.. Aha! How
about this...

We introduce very specific rates for each mode which drivers can use
for the minimal support from what we know IEEE-802.11 defines.. Then
we can simply use the current mode/rate work for vendor modes and
vendor rates? We have the part of the code already, we just need to
adjust it. This will make introduction a lot easier too. If the driver
doesn't use specify its using the 'stack' modes an rates, we can then
check its linked list of modes.

Comments?

> > > Internally, mac80211 or fullmac drivers of course need to check the
> > > channel structs internally when they are asked to move to a different
> > > channel or change TX power.
> >
> > Sure, cfg8021 can just do this. We just need to be sure cfg80211 won't
> > break anything.
>
> Hm. cfg80211 could do it but I think it might be wise to have it in
> drivers since scanning will require the driver to do it anyway.

Scanning won't require this update if we can assume a driver's channel
list flags are *always* up to date. Since cfg80211 is in charge of
updating the central regdomain it wil *know* when it gets updated and
as such can, for example, 'lock' the channel structs. This will assure
we don't allow channel change, scan, or any other nasty thing which
may break regulatory laws, during a regulatory change. Note that
regulatory domain change should be a rare thing though :-)

> > > At this point, however, the regulatory agent API only starts. First of
> > > all, the channel list has already been registered with cfg80211 because
> > > a pointer is part of struct wiphy
> >
> > If we take this approach, sure.
>
> No need to, of course, we could register it explicitly as well.

I think we'll need a writeup of what we want and agree on it as a path
to take so everyone is on the same page.

> > >  Then, when by any way possible, the
> > > regulatory restrictions change (user setting, ...) the fields marked
> > > "second part updated by regulatory agent" are updated for each wiphy
> >
> > As of right now I think changing channel rules and power restrictions
> > (ir, eirp, and antenna gain) is all the regulatory agent should do.
> > Beyond that its news to me that regulatory laws say we have to change
> > other aspects of wireless communication.
>
> Right. I'm not sure about modulations (see above) but I agree with this.

Jouni? Anyone?

> > If all we have to do is update the channel flags and power
> > restrictions, why should drivers be informed about regulatory change?
> > I agree, it may be good to have, but I'm just wondering why we should
> > make this a priority right now if don't have a good reason for drivers
> > be informed about this.
>
> Well, I think it's not too important when you think of channels, but say
> you have an AP open and for some reason move into a new regulatory
> domain, the TX power might need to be adjusted. This isn't important,
> but implementing the hook to allow (notifier chain) is trivial so I
> figured we might as well do it.

ACK -- but, since cfg80211 *can* be in charge of updating the, either
central channel struct, or each driver's channel struct, we can also
have it update power restrictions upon regulatory domain change. Same
as above. What do you think?

> > I was thinking one simple way of doing this was to add to the wiphy
> > struct a reg timestamp or counter, and also have one for the central
> > reg agent. If the stamps don't match, well then we know the central
> > reg agent has changed the central regdomain and the driver can do
> > whatever work it needs to. A notifier seems much better than polling
> > though. But I just am not sure if we need this right now.
>
> Yeah I agree about polling, that sucks since we're all in the kernel.
> But see above as for why I think we should do it anyway.

ACK but also see my above comments as to why I think we may not even
need notifiers, at least so far.

> > > From what I can see, this finishes the channel restrictions API. Since
> > > none of the information in the channels list is specific to some device,
> > > it may be allocated statically.
> >
> > I don't quite get by what you mean in this last part. Can you
> > elaborate a bit further?
>
> Well, I think that there's no need to allocate the channels array
> dynamically, each driver can have just a single one indicating what the
> hardware is capable of and use it multiple times if you say plug in 20
> zd1211 devices.

Oh sure. Unless we want to be freaks and support different regulatory
domains for each device... This may apply later for Software Defined
Radios.. but not for now ;)

> > OK -- so now let me address why I haven't done some changes which you
> > have addressed yet. It's because I don't want to cause a headache to
> > driver developers and for introduction of this work it would mean A
> > LOT of work and changes. I rather introduce this work gently, only
> > modifying absolutely necessary things first.
>
> I'm not sure. I think I prefer getting the API right and having some
> driver pain than trying to introduce it over a few weeks and having
> longer pain ;) But I suppose that's just me, you may well prefer to do
> it slower and I won't push it.

Well I think we can come to a compromise. If we use the strategy above
of moving modes to 'vendor_modes' and add a central mode, with the
option of letting the driver specify it it wants to use the central
modes, we can move any driver slowly at any time to the new API
without breaking a single thing.

Introducing a central reg agent *now* with the API as it is would
require some minimal changes which I can make now. That needs some
work though too (series II). But this work could proceed to at least
start removing every driver's own regulatory domain implementation
while the backend gets support for generic modes and rates. Later on,
we can use let drivers ask to parse a 'vendor' mode through the
current regulatory domain, for that we can use what we'll provide in
the meantime to support the current mode allocation on mac80211.

> > If patchset series I work is embraced
>
> I think the patches you posted are pretty much unusable as they stand,
> you should at least clean out the number of lines you have in headers
> and the ugly macros that declare arrays (why is that anyway? does
> openbsd really ship such code?)

Yeap. I agree, I also have a list of things to flip around based on
your comments. I'll send v3 soon of series I soon.

  Luis

Re: API to regulatory control (1)

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 5519 bytes --]

On Mon, 2007-09-24 at 15:55 -0400, Luis R. Rodriguez wrote:

> > But I then thought we'd be calling this function a lot during scanning
> > so discarded this idea. However, with all the complexity here, maybe we
> > can revive it and provide some sort of "for each allowed channel" loop
> > or something too.
> 
> At scan time we can just check the channel flags, those are always
> supposed to be up to date if cfg80211 is in charge of updating all
> wiphys with the new rules upon regulatory domain change.

Right, that's why I figured we should have them, I was considering
discarding them to make the structure smaller.

> No, you're right, It seems like we don't, yeah I don't think we do..
> but I was just basing the model on the current mac80211 driver API,
> which does have modes... if we are going to get rid of struct
> ieee80211_hw_mode this is a pretty serious cleanup on the both
> mac80211 and driver side. It'll be good, make driver code smaller but
> it seems like a lot of work right now.
> 
> Hmm, it seems if we take this approach we'll need to stage this into
> separate development parts to not cause too enforce too many changes
> on drivers. Instead of the 2-series I suggested how about 3 series:
> 
> * I - Introduce new regdomain modules integrate with cfg80211
> * II - Minimal mac80211 support with current driver infrastructure
> * III - Start optimizing to remove all unnecessary materials
> 
> We may just want to go directly to series III but based on the amount
> of complaints from current driver writers I think its best to use 3
> stages for this... Comments?

I dunno. Did we get that many complaints? I think the filter flags
change went rather well. In any case, I don't really have a plan and I
don't see how we can change any of this without breaking drivers at
least once.

> We introduce very specific rates for each mode which drivers can use
> for the minimal support from what we know IEEE-802.11 defines.. Then
> we can simply use the current mode/rate work for vendor modes and
> vendor rates? We have the part of the code already, we just need to
> adjust it. This will make introduction a lot easier too. If the driver
> doesn't use specify its using the 'stack' modes an rates, we can then
> check its linked list of modes.

Not sure. Some drivers actually do need the hardware-specific value in
the struct there that was pass around when setting channel/rate. Also,
vendor-modes really has more a problem of the userspace API rather than
the in-kernel API.

> Scanning won't require this update if we can assume a driver's channel
> list flags are *always* up to date. Since cfg80211 is in charge of
> updating the central regdomain it wil *know* when it gets updated and
> as such can, for example, 'lock' the channel structs. This will assure
> we don't allow channel change, scan, or any other nasty thing which
> may break regulatory laws, during a regulatory change. Note that
> regulatory domain change should be a rare thing though :-)

Obviously.

> 
> > > > At this point, however, the regulatory agent API only starts. First of
> > > > all, the channel list has already been registered with cfg80211 because
> > > > a pointer is part of struct wiphy
> > >
> > > If we take this approach, sure.
> >
> > No need to, of course, we could register it explicitly as well.
> 
> I think we'll need a writeup of what we want and agree on it as a path
> to take so everyone is on the same page.

Well I see basically two options:
 * register it with each wiphy and document that you can re-use the same
   array if you wish because it's updated only in an idempotent fashion
   (i.e. two updates don't matter)
 * register the array structure itself

> > Well, I think it's not too important when you think of channels, but say
> > you have an AP open and for some reason move into a new regulatory
> > domain, the TX power might need to be adjusted. This isn't important,
> > but implementing the hook to allow (notifier chain) is trivial so I
> > figured we might as well do it.
> 
> ACK -- but, since cfg80211 *can* be in charge of updating the, either
> central channel struct, or each driver's channel struct, we can also
> have it update power restrictions upon regulatory domain change. Same
> as above. What do you think?

Well, but the driver must actually change the power level and hence it
must be notified of the change after it has occurred so it is able to
adjust. I don't think I want to call into some cfg80211 call for this, a
specific notifier seems more appropriate.

> Oh sure. Unless we want to be freaks and support different regulatory
> domains for each device... This may apply later for Software Defined
> Radios.. but not for now ;)

Why would that be a different domain? It'd be a different set of
restrictions but I don't think you can really manage to have a machine
in two countries at the same time, physically :) [corner cases with
directional antennas excluded]

> Well I think we can come to a compromise. If we use the strategy above
> of moving modes to 'vendor_modes' and add a central mode, with the
> option of letting the driver specify it it wants to use the central
> modes, we can move any driver slowly at any time to the new API
> without breaking a single thing.

Aha, but I don't think that strategy can work since there's this
hw-specific value in the structs. But I already said that above.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 190 bytes --]

Re: API to regulatory control (1)

[Luis R. Rodriguez]

On 9/24/07, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Mon, 2007-09-24 at 15:55 -0400, Luis R. Rodriguez wrote:
>
> > > But I then thought we'd be calling this function a lot during scanning
> > > so discarded this idea. However, with all the complexity here, maybe we
> > > can revive it and provide some sort of "for each allowed channel" loop
> > > or something too.
> >
> > At scan time we can just check the channel flags, those are always
> > supposed to be up to date if cfg80211 is in charge of updating all
> > wiphys with the new rules upon regulatory domain change.
>
> Right, that's why I figured we should have them, I was considering
> discarding them to make the structure smaller.

Yeah no it helps a lot to have them.

> > No, you're right, It seems like we don't, yeah I don't think we do..
> > but I was just basing the model on the current mac80211 driver API,
> > which does have modes... if we are going to get rid of struct
> > ieee80211_hw_mode this is a pretty serious cleanup on the both
> > mac80211 and driver side. It'll be good, make driver code smaller but
> > it seems like a lot of work right now.
> >
> > Hmm, it seems if we take this approach we'll need to stage this into
> > separate development parts to not cause too enforce too many changes
> > on drivers. Instead of the 2-series I suggested how about 3 series:
> >
> > * I - Introduce new regdomain modules integrate with cfg80211
> > * II - Minimal mac80211 support with current driver infrastructure
> > * III - Start optimizing to remove all unnecessary materials
> >
> > We may just want to go directly to series III but based on the amount
> > of complaints from current driver writers I think its best to use 3
> > stages for this... Comments?
>
> I dunno. Did we get that many complaints? I think the filter flags
> change went rather well. In any case, I don't really have a plan and I
> don't see how we can change any of this without breaking drivers at
> least once.

I saw complaints on the z1211 move thread to upstream about large
patches and driver developers not being able to keep up as mac80211 is
a 'moving target'.

> > We introduce very specific rates for each mode which drivers can use
> > for the minimal support from what we know IEEE-802.11 defines.. Then
> > we can simply use the current mode/rate work for vendor modes and
> > vendor rates? We have the part of the code already, we just need to
> > adjust it. This will make introduction a lot easier too. If the driver
> > doesn't use specify its using the 'stack' modes an rates, we can then
> > check its linked list of modes.
>
> Not sure. Some drivers actually do need the hardware-specific value in
> the struct there that was pass around when setting channel/rate.

Well channel mapping to hardware values requires less info than what
we currently use. But after a quick review of this you're right...
after we nuke min_rssi_ack and min_rssi_ack_delta (which I do in my
patches) from ieee80211_rate those structs are pretty optimized
already if all we want to provide is hw mapping values.

Another option here is just speak to the driver in a hardware-agnostic
sense and just let drivers map this out however they want. I guess it
provides for a cleaner API if we keep this in our own structs though.

> > ACK -- but, since cfg80211 *can* be in charge of updating the, either
> > central channel struct, or each driver's channel struct, we can also
> > have it update power restrictions upon regulatory domain change. Same
> > as above. What do you think?
>
> Well, but the driver must actually change the power level and hence it
> must be notified of the change after it has occurred so it is able to
> adjust.

Only if the new power restrictions makes he current one invalid.

> I don't think I want to call into some cfg80211 call for this, a
> specific notifier seems more appropriate.

Well what if cfg80211  generates a notification if a possible conflict
has been found? Is there a need to generate a notification on power
reg changes to the driver besides this?

> > Oh sure. Unless we want to be freaks and support different regulatory
> > domains for each device... This may apply later for Software Defined
> > Radios.. but not for now ;)
>
> Why would that be a different domain? It'd be a different set of
> restrictions but I don't think you can really manage to have a machine
> in two countries at the same time, physically :) [corner cases with
> directional antennas excluded]

ACK -- two different type of restrictions, I'm thinking ahead of myself :-P

> > Well I think we can come to a compromise. If we use the strategy above
> > of moving modes to 'vendor_modes' and add a central mode, with the
> > option of letting the driver specify it it wants to use the central
> > modes, we can move any driver slowly at any time to the new API
> > without breaking a single thing.
>
> Aha, but I don't think that strategy can work since there's this
> hw-specific value in the structs. But I already said that above.

Yeap... you're right.

  Luis

Re: API to regulatory control (1)

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 2090 bytes --]

On Mon, 2007-09-24 at 18:18 -0400, Luis R. Rodriguez wrote:

> I saw complaints on the z1211 move thread to upstream about large
> patches and driver developers not being able to keep up as mac80211 is
> a 'moving target'.

It'll always be. Everything in Linux is a moving target. That's the pain
associated with maintaining a driver out of tree. This change in
particular isn't even hard to follow and I'd happily help change all the
drivers that are in tree.

> Another option here is just speak to the driver in a hardware-agnostic
> sense and just let drivers map this out however they want. I guess it
> provides for a cleaner API if we keep this in our own structs though.

We could, but unless the hardware operates with frequencies like b43
it'll likely need a big switch() statement which doesn't seem too nice.

> > > ACK -- but, since cfg80211 *can* be in charge of updating the, either
> > > central channel struct, or each driver's channel struct, we can also
> > > have it update power restrictions upon regulatory domain change. Same
> > > as above. What do you think?
> >
> > Well, but the driver must actually change the power level and hence it
> > must be notified of the change after it has occurred so it is able to
> > adjust.
> 
> Only if the new power restrictions makes he current one invalid.

Right.

> > I don't think I want to call into some cfg80211 call for this, a
> > specific notifier seems more appropriate.
> 
> Well what if cfg80211  generates a notification if a possible conflict
> has been found? Is there a need to generate a notification on power
> reg changes to the driver besides this?

Well, that'd mean that cfg80211 would need to keep track of the current
power level. I've intentionally made cfg80211 keep track of as few
things as possible so that drivers are free to change things when
necessary without having synchronisation problems. I would prefer if it
stayed this way.

Also, just providing the hook is much simpler and we can leave it up to
drivers to implement this logic.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 190 bytes --]

Re: API to regulatory control (1)

[Luis R. Rodriguez]

On 9/25/07, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Mon, 2007-09-24 at 18:18 -0400, Luis R. Rodriguez wrote:
>
> > I saw complaints on the z1211 move thread to upstream about large
> > patches and driver developers not being able to keep up as mac80211 is
> > a 'moving target'.
>
> It'll always be. Everything in Linux is a moving target. That's the pain
> associated with maintaining a driver out of tree. This change in
> particular isn't even hard to follow and I'd happily help change all the
> drivers that are in tree.

OK fine, lets just move forward but I'll take you up on the offer ;)

> > > I don't think I want to call into some cfg80211 call for this, a
> > > specific notifier seems more appropriate.
> >
> > Well what if cfg80211  generates a notification if a possible conflict
> > has been found? Is there a need to generate a notification on power
> > reg changes to the driver besides this?
>
> Well, that'd mean that cfg80211 would need to keep track of the current
> power level. I've intentionally made cfg80211 keep track of as few
> things as possible so that drivers are free to change things when
> necessary without having synchronisation problems. I would prefer if it
> stayed this way.

I wasn't thinking of that, but  instead, of the 'loop' cfg80211 would
do upon regulatory change, on all wiphys.

> Also, just providing the hook is much simpler and we can leave it up to
> drivers to implement this logic.

Hm, the drivers will still call a cfg80211 helper anyway to update its
channel flags/power limits, why not let cfg80211 just do it then in a
loop for all wiphys upon reg change? At the end of the loop for each
wiphy we can call a driver-specific ops->verify_reg().

  Luis

Re: API to regulatory control (1)

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 418 bytes --]

On Tue, 2007-09-25 at 12:13 -0400, Luis R. Rodriguez wrote:

> Hm, the drivers will still call a cfg80211 helper anyway to update its
> channel flags/power limits, why not let cfg80211 just do it then in a
> loop for all wiphys upon reg change? At the end of the loop for each
> wiphy we can call a driver-specific ops->verify_reg().

Oh ok, yeah, sure, we can do that instead of a notifier chain.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 190 bytes --]

Re: API to regulatory control (1)

[Pavel Roskin]

On Sat, 2007-09-22 at 12:22 +0200, Johannes Berg wrote:

> struct ieee80211_channel {
> 	/* first part set by driver */
> 	u16 freq;		/* in MHz */
> 	int hardware_value;	/* arbitrary */

Please consider using "long" for hardware_value.  Drivers should be able
to use this value as a pointer to some driver-specific structure.

-- 
Regards,
Pavel Roskin

Re: API to regulatory control (1)

[Christoph Hellwig]

On Mon, Sep 24, 2007 at 08:59:35AM -0400, Pavel Roskin wrote:
> On Sat, 2007-09-22 at 12:22 +0200, Johannes Berg wrote:
> 
> > struct ieee80211_channel {
> > 	/* first part set by driver */
> > 	u16 freq;		/* in MHz */
> > 	int hardware_value;	/* arbitrary */
> 
> Please consider using "long" for hardware_value.  Drivers should be able
> to use this value as a pointer to some driver-specific structure.

If you want to use it as pointer better make it a pointer..

Re: API to regulatory control (1)

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 782 bytes --]

On Mon, 2007-09-24 at 14:07 +0100, Christoph Hellwig wrote:
> On Mon, Sep 24, 2007 at 08:59:35AM -0400, Pavel Roskin wrote:
> > On Sat, 2007-09-22 at 12:22 +0200, Johannes Berg wrote:
> > 
> > > struct ieee80211_channel {
> > > 	/* first part set by driver */
> > > 	u16 freq;		/* in MHz */
> > > 	int hardware_value;	/* arbitrary */
> > 
> > Please consider using "long" for hardware_value.  Drivers should be able
> > to use this value as a pointer to some driver-specific structure.
> 
> If you want to use it as pointer better make it a pointer..

Then we'd have to have two values. I don't see a need for having a
pointer at all though. Pavel, can you show a driver that really benefits
from that? And explain why it can't use an array index here?

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 190 bytes --]

Re: API to regulatory control (1)

[Pavel Roskin]

On Mon, 2007-09-24 at 15:11 +0200, Johannes Berg wrote:

> > > Please consider using "long" for hardware_value.  Drivers should be able
> > > to use this value as a pointer to some driver-specific structure.
> > 
> > If you want to use it as pointer better make it a pointer..
> 
> Then we'd have to have two values. I don't see a need for having a
> pointer at all though. Pavel, can you show a driver that really benefits
> from that? And explain why it can't use an array index here?

No, it's just a "general sanity" thing.  If we want the driver to keep
some data there without specifying what data it can be, it's better to
provide a way to keep a pointer if needed.

OK, if my suggestion doesn't sound interesting, we can forget it for
now.  After all, it's easy to change Linux code at any time, as long as
the external interfaces are not involved.

-- 
Regards,
Pavel Roskin

Re: API to regulatory control (1)

[Joerg Pommnitz]

 > From what I can see, this finishes the channel restrictions API. Since
 > none of the information in the channels list is specific to some device,
 > it may be allocated statically.

Well, I beg to differ. I'm working on a system were we have regulatory
approval to use a TX power of up to 4W in certain channels. This means
that the regulatory settings (e.g. allowed channels) are different for
cards with amplifier and those without, even on the same machine.
--  Regards
 
       Joerg
 






      __________________________________  
Yahoo! Clever: Sie haben Fragen? Yahoo! Nutzer antworten Ihnen. www.yahoo.de/clever

Re: API to regulatory control (1)

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 596 bytes --]

On Tue, 2007-09-25 at 07:43 -0700, Joerg Pommnitz wrote:
> > From what I can see, this finishes the channel restrictions API. Since
>  > none of the information in the channels list is specific to some device,
>  > it may be allocated statically.
> 
> Well, I beg to differ. I'm working on a system were we have regulatory
> approval to use a TX power of up to 4W in certain channels. This means
> that the regulatory settings (e.g. allowed channels) are different for
> cards with amplifier and those without, even on the same machine.

Thanks for your offer for patches.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 190 bytes --]