preparing for 802.11ah channels

preparing for 802.11ah channels

[thomas pedersen]

Hello,

I'm working on defining new channels for S1G PHYs in mac80211. These
are in the 900MHz range and center frequency for the 1MHz channels are
on a half MHz, while the existing channel definitions are in units of
MHz.

In order to support the new channels we could change the internal
center frequency units to KHz and extend the nl80211 API so
NL80211_FREQUENCY_ATTR_FREQ and NL80211_ATTR_WIPHY_FREQ get _KHZ
variants while renaming the original attributes to _MHZ to reflect the
units.

From looking at the code it looks like this should be fairly
straightforward, if involving a lot of changes since the driver
declarations would have to change too. Am I missing something
obviously wrong, or does this sound reasonable?

Thanks,

Thomas

Re: preparing for 802.11ah channels

[Johannes Berg]

On Sat, 2019-02-16 at 15:45 -0800, thomas pedersen wrote:
> Hello,
> 
> I'm working on defining new channels for S1G PHYs in mac80211. These
> are in the 900MHz range and center frequency for the 1MHz channels are
> on a half MHz, while the existing channel definitions are in units of
> MHz.

Yay... :)

> In order to support the new channels we could change the internal
> center frequency units to KHz and extend the nl80211 API so
> NL80211_FREQUENCY_ATTR_FREQ and NL80211_ATTR_WIPHY_FREQ get _KHZ
> variants while renaming the original attributes to _MHZ to reflect the
> units.

The nl80211 API seems straightforward, yeah.

> From looking at the code it looks like this should be fairly
> straightforward, if involving a lot of changes since the driver
> declarations would have to change too. Am I missing something
> obviously wrong, or does this sound reasonable?

I'm not sure I agree about all the driver changes being straightforward.
In theory yes, but ... ;-)

I guess you'd define a new band (NL80211_BAND_S1G or something?), so
perhaps there's a way we can get away with doing it only on that band?

OTOH, I guess if you change center_freq1 to center_freq1_khz or
something then you compile-fail everywhere ...

johanne

Re: preparing for 802.11ah channels

[thomas]

On Fri, Mar 15, 2019 at 2:47 AM Johannes Berg <johannes@sipsolutions.net> wrote:
>
> On Sat, 2019-02-16 at 15:45 -0800, thomas pedersen wrote:
> > Hello,
> >
> > I'm working on defining new channels for S1G PHYs in mac80211. These
> > are in the 900MHz range and center frequency for the 1MHz channels are
> > on a half MHz, while the existing channel definitions are in units of
> > MHz.
>
> Yay... :)
>
> > In order to support the new channels we could change the internal
> > center frequency units to KHz and extend the nl80211 API so
> > NL80211_FREQUENCY_ATTR_FREQ and NL80211_ATTR_WIPHY_FREQ get _KHZ
> > variants while renaming the original attributes to _MHZ to reflect the
> > units.
>
> The nl80211 API seems straightforward, yeah.
>
> > From looking at the code it looks like this should be fairly
> > straightforward, if involving a lot of changes since the driver
> > declarations would have to change too. Am I missing something
> > obviously wrong, or does this sound reasonable?
>
> I'm not sure I agree about all the driver changes being straightforward.
> In theory yes, but ... ;-)
>
> I guess you'd define a new band (NL80211_BAND_S1G or something?), so
> perhaps there's a way we can get away with doing it only on that band?
>
> OTOH, I guess if you change center_freq1 to center_freq1_khz or
> something then you compile-fail everywhere ...

Yeah it gets ugly quick to have additional freq_khz members,
especially when doing comparisons etc.

The kernel part was not too bad, there are only 3 structs or so to
worry about. hostap on the other hand is incredibly hairy :)

Thomas

Re: preparing for 802.11ah channels

[Johannes Berg]

On Fri, 2019-03-22 at 13:40 -0700, thomas wrote:
> 
> > OTOH, I guess if you change center_freq1 to center_freq1_khz or
> > something then you compile-fail everywhere ...
> 
> Yeah it gets ugly quick to have additional freq_khz members,
> especially when doing comparisons etc.
> 
> The kernel part was not too bad, there are only 3 structs or so to
> worry about. hostap on the other hand is incredibly hairy :)

We/I thought about this some more now at the wifi summit, and I think
the better choice would be to just add a "u32 fractional;" part where
needed.

In effect, an existing

	u32 freq;

would become

	u32 freq, freq_fractional;

and we'd just treat the two as a 32.32 fixed point integer. Maybe even
just do "u8 freq_fractional" since a 32.8 fixed point integer would be
sufficient (we do need >64k MHz for WiGig, so doing 16.16 or 16.8 won't
work).

That would have the advantage of not requiring any changes from drivers
that don't need to support it. It would also cover some of the 4.9 GHz
stuff Jouni looked up - it as fractional center frequencies too (at
least expressed in MHz).

For userspace API I'm not really sure, maybe we should just move to KHz?
Or maybe just do the same thing? On the one hand, moving to KHz would be
cleaner in a sense, but on the other hand it would require doing things
like checking that the KHz and MHz attributes agree if both are given,
etc., so similarly adding a "fractional MHz" attribute might be nicer
since we can also assume it to be 0 if not present (in fact, we could
*reject* it if it's 0, thereby ensuring that it'll only ever be nonzero)

johannes

Re: preparing for 802.11ah channels

[Johannes Berg]

On Fri, 2019-03-22 at 21:46 +0100, Johannes Berg wrote:
> 
> In effect, an existing
> 
> 	u32 freq;
> 
> would become
> 
> 	u32 freq, freq_fractional;
> 
> and we'd just treat the two as a 32.32 fixed point integer.

Maybe easier would be to say

	u32 freq;
	u16 freq_partial_khz;

and basically say

	freq_khz = freq * 1000 + freq_partial_khz;

Same reasoning and similar logic, but no need to do any sort of fixed
point conversions.

johannes

Re: preparing for 802.11ah channels

[thomas]

On Fri, Mar 22, 2019 at 1:50 PM Johannes Berg <johannes@sipsolutions.net> wrote:
>
> On Fri, 2019-03-22 at 21:46 +0100, Johannes Berg wrote:
> >
> > In effect, an existing
> >
> >       u32 freq;
> >
> > would become
> >
> >       u32 freq, freq_fractional;
> >
> > and we'd just treat the two as a 32.32 fixed point integer.
>
> Maybe easier would be to say
>
>         u32 freq;
>         u16 freq_partial_khz;
>
> and basically say
>
>         freq_khz = freq * 1000 + freq_partial_khz;
>
> Same reasoning and similar logic, but no need to do any sort of fixed
> point conversions.

I guess it's a tradeoff between leaving MHz-only drivers alone and
wrapping existing assignments and comparisons in maybe a macro to
extract the final KHz value. The current set of changes is about 150
lines in net/mac80211/ and net/wireless/ [1] with drivers/ being about
the same.

Adding the KHz part to center_freq, center_freq1, and rx_status->freq
may be fewer overall changes in mac80211 and cfg80211, and should be
none for existing drivers.

I'll give this a shot at some point when it comes time to rebase on
upstream wireless-testing.

[1] https://paste.debian.net/1074289/

Re: preparing for 802.11ah channels

[Johannes Berg]

On Fri, 2019-03-22 at 15:29 -0700, thomas wrote:
> 
> I guess it's a tradeoff between leaving MHz-only drivers alone and
> wrapping existing assignments and comparisons in maybe a macro to
> extract the final KHz value. The current set of changes is about 150
> lines in net/mac80211/ and net/wireless/ [1] with drivers/ being about
> the same.
> 
> Adding the KHz part to center_freq, center_freq1, and rx_status->freq
> may be fewer overall changes in mac80211 and cfg80211, and should be
> none for existing drivers.

Yes. I was also thinking that perhaps keeping "MHz-mostly" would lead in
less overall new binary size, since you don't need to do "/1000" in that
many places. I can't see it any more, but IIRC your patch did add a
number of "KHZ_TO_MHZ()" or so?

johannes