2014-06-30 18:55:49

by Luis Chamberlain

[permalink] [raw]
Subject: Parser stable fix question for CFG80211_INTERNAL_REGDB

Folks,

John Walker reported an issue with 3.10 kernels not having the max
EIRP not set properly when CFG80211_INTERNAL_REGDB is used [0].
Krishna Chaitanya later confirmed this and annotated that an RFC I had
posted a while back, shortly after the change the created this issue,
is being carried over on OpenWrt as a fix for this [1]. The RFC was
for new kernels and it seems we just never got around to following up
and moving this upstream, so this change is certainly needed however
upon review on issues with older kernels I found an issue with
CFG80211_INTERNAL_REGDB which we hadn't considered before which we
should review in terms of strategy for newer kernels to avoid these
issues. Usage of CFG80211_INTERNAL_REGDB is popular for mobile and AP
devices. I'm Cc'ing the netfilter folks in case they might have some
ideas.

By design we have ensured that we make wireless-regdb backward
compatible with older versions of CRDA. We have done this well for a
while now by not requiring a binary format file change, but realizing
that at one point though we might have to upgrade the format and at
which point we may require two file formats on wireless-regdb for
different kernels. There's been some recent talks though over a new
strategy however which could always enable backward compatibility.
While those talks are underway we should consider that
CFG80211_INTERNAL_REGDB actually has a bit more different implications
on what we support on wireless-regdb for older kernels. The issue is
that CFG80211_INTERNAL_REGDB makes use of an awk script for parsing
the ASCII db.txt file, not the binary format. Commit e56cd9618e3e5
removed the antenna gain on wireless-regdb, upstream was not affected
for kernels using CRDA even when using older CRDA as the binary format
was not changed but since CFG80211_INTERNAL_REGDB relies on its own
ASCII parser older kernels are affected if this option is used. The
impact is that without an awk script update the max EIRP will be 0
when CFG80211_INTERNAL_REGDB is used with new wireless-regdb so
devices can't transmit. What this also means is that when
CFG80211_INTERNAL_REGDB is used you cannot use older versions of
wireless-regdb unless we update the awk script, this however gets
tricky and the awk script can become complicated quickly. The change
below is an example change to make the the awk script backward
compatible upstream but for stable kernels we'd have port this a bit
-- we'd have to remove usage of the new dfs_cac as that wasn't
available on older kernels.

We need to decide what we want to do for older kernels, right now, and
in the future in consideration for the parser requirements on the
ASCII file by CFG80211_INTERNAL_REGDB then. For the current kernel we
could intake the patch below, and I can port the change to no use
dfs_cac, that would enable older kernels to use new and older versions
of the ASCII database file. For future kernels I started looking at
ditching the awk script completely and instead using the new C parser
already on CRDA. Another option is to have a something that reads the
regulatory binary file in the kernel, that is, a built-in CRDA. We can
use the C parser but that would require the making it also backward
compatible, today it is not, and #ifdef'ing some upstream code with
__KERNEL if we wanted to share some headers. I started mucking with
this as an option but it does require a bit of change. Having an
internal CRDA is a rather bigger change but one at this point worth
considering. I saw that for example recently we moved firmware
uploading into the kernel and that udev will soon remove the firmware
loader option. This a bit different change but a bit similar -- the
only difference here is that we do rely on RSA digital signatures on
the binary file, and we allow usage of either gcrypt or openssl for
verifying the signature on the file. When CFG80211_INTERNAL_REGDB is
used though we don't rely on any signatures though given that the file
is read and parsed at build time once so there is no chance to corrupt
the file. We could upkeep that behavior but the reading of the file
then would be binary to txt transformation, and it'd be a one time
thing at build time. If we wanted instead to have
CFG80211_INTERNAL_REGDB to mean that CRDA is then built-in to read
files from the filesystem at run time we'd have to decide if we wanted
to upkeep parsing the RSA digitial signature at read time.

I should note that CRDA is licensed under the copyleft-next license, I
have checked with Richard on license compatibility and he's verified
that copyleft-next is GPLv2 compatible and would not implicate any new
things if we're to share copyleft-next code on the kernel. It'd be no
different than for example how we have ISC licensed code upstream.
This would enable us to share code between userspace and kernel
however we wish.

We need a fix strategy, and also a path forward. What are folks thoughts?

[0] http://marc.info/?t=140093492500001&r=1&w=3
[1] https://dev.openwrt.org/browser/trunk/package/kernel/mac80211/patches/005-make-genregdb.awk-skip-antenna-gain.patch?rev=40296

Luis

diff --git a/net/wireless/genregdb.awk b/net/wireless/genregdb.awk
index 40c37fc..8aa73b1 100644
--- a/net/wireless/genregdb.awI
+++ b/net/wireless/genregdb.awk
@@ -51,32 +51,48 @@ function parse_country_head() {

function parse_reg_rule()
{
+ flag_starts_at = 8
+
start = $1
sub(/\(/, "", start)
end = $3
bw = $5
sub(/\),/, "", bw)
+
+ # We have two ASCII formats, one without the antenna gain
gain = $6
- sub(/\(/, "", gain)
- sub(/,/, "", gain)
power = $7
- sub(/\)/, "", power)
+ units = $8
+ dfs_cac = 0
+
+ if (gain ~ /)/) {
+ flag_starts_at = 7
+
+ gain = 0
+ power = $6
+ units = $7
+ dfs_cac = $9
+ if (units != "mW")
+ dfs_cac = $8
+ } else {
+ sub(/\(/, "", gain)
+ sub(/,/, "", gain)
+ }
+
+ sub(/\(/, "", power)
sub(/,/, "", power)
+ sub(/\)/, "", power)
# power might be in mW...
- units = $8
sub(/\)/, "", units)
sub(/,/, "", units)
- dfs_cac = $9
if (units == "mW") {
power = 10 * log(power)/log(10)
- } else {
- dfs_cac = $8
}
sub(/,/, "", dfs_cac)
sub(/\(/, "", dfs_cac)
sub(/\)/, "", dfs_cac)
flagstr = ""
- for (i=8; i<=NF; i++)
+ for (i=flag_starts_at; i<=NF; i++)
flagstr = flagstr $i
split(flagstr, flagarray, ",")
flags = ""


2014-06-30 19:19:30

by Luis Chamberlain

[permalink] [raw]
Subject: Re: Parser stable fix question for CFG80211_INTERNAL_REGDB

On Mon, Jun 30, 2014 at 11:55 AM, Luis R. Rodriguez
<[email protected]> wrote:
> For the current kernel we
> could intake the patch below, and I can port the change to no use
> dfs_cac, that would enable older kernels to use new and older versions
> of the ASCII database file.

Personally I actually rather avoid us accept a patch upstream for a
userspace change. The problem here is that we failed to realize the
impact of CFG80211_INTERNAL_REGDB at build time with a userspace tool,
in this case the db.txt file wireless-regdb provides and its format.

If we wanted to avoid a stable patch we could require a match between
wireless-regdb input file used for a kernel when
CFG80211_INTERNAL_REGDB is used at build time. That would require
different ASCII files on wireless-regdb or having the users of
CFG80211_INTERNAL_REGDB do the conversion themselves. Upstream would
just follow the wireless-regdb latest format. This then would just
require upstream a Kconfig update to clarify the requirements.

This seems like a rather lazy option but also one that would be rather
more fair and honest for upstream, we could deal with a proper fix by
reconsidering the implementation of CFG80211_INTERNAL_REGDB completely
for future kernels.

Luis

2014-07-01 18:55:06

by Luis Chamberlain

[permalink] [raw]
Subject: Re: Parser stable fix question for CFG80211_INTERNAL_REGDB

On Tue, Jul 1, 2014 at 10:17 AM, Luis R. Rodriguez <[email protected]> wrote:
>> How is this any different from any other kernel support issue?
>
> This is a build issue that does not cause any errors, users go through with the install
> just fine and only at run time do they discover the issue. We never documented
> the expecations nor do I think we expected this before. This is also caused by an
> input file from userspace, I do consider the issue unexpected.

This is a bit more broken given that DFS CAC patches are upstream but
we haven't yet even provided the removal of antenna gain on the awk
script but for some reason it did add the DFS CAC parsing to the awk
script, so upstream has the awk DFS CAC parser options, we don't yet
even use it, and the antenna gain wasn't removed. I'll work on that
patch but parsing things in awk is getting a bit hairy for the
conditionals. We can use the C parser now that we have on but the DFS
CAC stuf was not yet ironed out for CRDA and wireless-regdb...

Luis

2014-07-02 07:33:30

by Krishna Chaitanya

[permalink] [raw]
Subject: Re: Parser stable fix question for CFG80211_INTERNAL_REGDB

On Tue, Jul 1, 2014 at 8:11 PM, John W. Linville <[email protected]> wrote:
>
> On Mon, Jun 30, 2014 at 12:19:08PM -0700, Luis R. Rodriguez wrote:
> > On Mon, Jun 30, 2014 at 11:55 AM, Luis R. Rodriguez
> > <[email protected]> wrote:
> > > For the current kernel we
> > > could intake the patch below, and I can port the change to no use
> > > dfs_cac, that would enable older kernels to use new and older versions
> > > of the ASCII database file.
> >
> > Personally I actually rather avoid us accept a patch upstream for a
> > userspace change. The problem here is that we failed to realize the
> > impact of CFG80211_INTERNAL_REGDB at build time with a userspace tool,
> > in this case the db.txt file wireless-regdb provides and its format.
> >
> > If we wanted to avoid a stable patch we could require a match between
> > wireless-regdb input file used for a kernel when
> > CFG80211_INTERNAL_REGDB is used at build time. That would require
> > different ASCII files on wireless-regdb or having the users of
> > CFG80211_INTERNAL_REGDB do the conversion themselves. Upstream would
> > just follow the wireless-regdb latest format. This then would just
> > require upstream a Kconfig update to clarify the requirements.
> >
> > This seems like a rather lazy option but also one that would be rather
> > more fair and honest for upstream, we could deal with a proper fix by
> > reconsidering the implementation of CFG80211_INTERNAL_REGDB completely
> > for future kernels.
>
> I'm shocked that anyone actually uses CFG80211_INTERNAL_REGDB...
>

Lot of embedded devices like ours avoid the pain of cross compiling
crda, udev...
and resort to internal_regdb as a easy way to get things done.


> Anyway, I don't see the big deal. We should keep
> CFG80211_INTERNAL_REGDB (whether implemented in awk or C) up-to-date
> with current kernels. Anyone wanting to use an old kernel with an
> updated wireless-regdb file is responsible for ensuring compatibility.
> If they can't do that, then they should seek support from a vendor.
> How is this any different from any other kernel support issue?

Sounds good to me.

2014-07-01 17:17:17

by Luis Chamberlain

[permalink] [raw]
Subject: Re: Parser stable fix question for CFG80211_INTERNAL_REGDB

On Tue, Jul 01, 2014 at 10:41:49AM -0400, John W. Linville wrote:
> On Mon, Jun 30, 2014 at 12:19:08PM -0700, Luis R. Rodriguez wrote:
> > On Mon, Jun 30, 2014 at 11:55 AM, Luis R. Rodriguez
> > <[email protected]> wrote:
> > > For the current kernel we
> > > could intake the patch below, and I can port the change to no use
> > > dfs_cac, that would enable older kernels to use new and older versions
> > > of the ASCII database file.
> >
> > Personally I actually rather avoid us accept a patch upstream for a
> > userspace change. The problem here is that we failed to realize the
> > impact of CFG80211_INTERNAL_REGDB at build time with a userspace tool,
> > in this case the db.txt file wireless-regdb provides and its format.
> >
> > If we wanted to avoid a stable patch we could require a match between
> > wireless-regdb input file used for a kernel when
> > CFG80211_INTERNAL_REGDB is used at build time. That would require
> > different ASCII files on wireless-regdb or having the users of
> > CFG80211_INTERNAL_REGDB do the conversion themselves. Upstream would
> > just follow the wireless-regdb latest format. This then would just
> > require upstream a Kconfig update to clarify the requirements.
> >
> > This seems like a rather lazy option but also one that would be rather
> > more fair and honest for upstream, we could deal with a proper fix by
> > reconsidering the implementation of CFG80211_INTERNAL_REGDB completely
> > for future kernels.
>
> I'm shocked that anyone actually uses CFG80211_INTERNAL_REGDB...

I believe all mobile and APs use it these days.

> Anyway, I don't see the big deal. We should keep
> CFG80211_INTERNAL_REGDB (whether implemented in awk or C) up-to-date
> with current kernels. Anyone wanting to use an old kernel with an
> updated wireless-regdb file is responsible for ensuring compatibility.
> If they can't do that, then they should seek support from a vendor.

Fine by me... I'll update the Kconfig.

> How is this any different from any other kernel support issue?

This is a build issue that does not cause any errors, users go through with the install
just fine and only at run time do they discover the issue. We never documented
the expecations nor do I think we expected this before. This is also caused by an
input file from userspace, I do consider the issue unexpected.

Luis

2014-07-01 20:30:14

by John W. Linville

[permalink] [raw]
Subject: Re: Parser stable fix question for CFG80211_INTERNAL_REGDB

On Tue, Jul 01, 2014 at 11:54:43AM -0700, Luis R. Rodriguez wrote:
> On Tue, Jul 1, 2014 at 10:17 AM, Luis R. Rodriguez <[email protected]> wrote:
> >> How is this any different from any other kernel support issue?
> >
> > This is a build issue that does not cause any errors, users go through with the install
> > just fine and only at run time do they discover the issue. We never documented
> > the expecations nor do I think we expected this before. This is also caused by an
> > input file from userspace, I do consider the issue unexpected.
>
> This is a bit more broken given that DFS CAC patches are upstream but
> we haven't yet even provided the removal of antenna gain on the awk
> script but for some reason it did add the DFS CAC parsing to the awk
> script, so upstream has the awk DFS CAC parser options, we don't yet
> even use it, and the antenna gain wasn't removed. I'll work on that
> patch but parsing things in awk is getting a bit hairy for the
> conditionals. We can use the C parser now that we have on but the DFS
> CAC stuf was not yet ironed out for CRDA and wireless-regdb...

I wrote that awk script years ago so that we could get rid of the
old hard-coded rulesets and so I could have something working in RHEL5.

I never merged the DFS CAC stuff in wireless-regdb, because of the
unresolved issue of supporting multiple formats. It looks to me like
the dfs_cac stuff in genregdb.awk is broken, since it doesn't seem
to account for flags.

I sent a patch that should account for the antenna gain issue.
Please give it a review.

Obviously, we need to talk more about this in Chicago if not before...

John
--
John W. Linville Someday the world will need a hero, and you
[email protected] might be all we have. Be ready.

2014-07-02 12:15:06

by Maxime Bizon

[permalink] [raw]
Subject: Re: Parser stable fix question for CFG80211_INTERNAL_REGDB


On Wed, 2014-07-02 at 13:03 +0530, Krishna Chaitanya wrote:

> Lot of embedded devices like ours avoid the pain of cross compiling
> crda, udev...
> and resort to internal_regdb as a easy way to get things done.

Count me in.

The whole crda stuff is crazy for embedded devices with fixed hardware.

--
Maxime



2014-07-01 14:45:14

by John W. Linville

[permalink] [raw]
Subject: Re: Parser stable fix question for CFG80211_INTERNAL_REGDB

On Mon, Jun 30, 2014 at 12:19:08PM -0700, Luis R. Rodriguez wrote:
> On Mon, Jun 30, 2014 at 11:55 AM, Luis R. Rodriguez
> <[email protected]> wrote:
> > For the current kernel we
> > could intake the patch below, and I can port the change to no use
> > dfs_cac, that would enable older kernels to use new and older versions
> > of the ASCII database file.
>
> Personally I actually rather avoid us accept a patch upstream for a
> userspace change. The problem here is that we failed to realize the
> impact of CFG80211_INTERNAL_REGDB at build time with a userspace tool,
> in this case the db.txt file wireless-regdb provides and its format.
>
> If we wanted to avoid a stable patch we could require a match between
> wireless-regdb input file used for a kernel when
> CFG80211_INTERNAL_REGDB is used at build time. That would require
> different ASCII files on wireless-regdb or having the users of
> CFG80211_INTERNAL_REGDB do the conversion themselves. Upstream would
> just follow the wireless-regdb latest format. This then would just
> require upstream a Kconfig update to clarify the requirements.
>
> This seems like a rather lazy option but also one that would be rather
> more fair and honest for upstream, we could deal with a proper fix by
> reconsidering the implementation of CFG80211_INTERNAL_REGDB completely
> for future kernels.

I'm shocked that anyone actually uses CFG80211_INTERNAL_REGDB...

Anyway, I don't see the big deal. We should keep
CFG80211_INTERNAL_REGDB (whether implemented in awk or C) up-to-date
with current kernels. Anyone wanting to use an old kernel with an
updated wireless-regdb file is responsible for ensuring compatibility.
If they can't do that, then they should seek support from a vendor.
How is this any different from any other kernel support issue?

John
--
John W. Linville Someday the world will need a hero, and you
[email protected] might be all we have. Be ready.