On Wed, 2010-05-26 at 18:46 +0200, ext Luis R. Rodriguez wrote:
> On Tue, May 25, 2010 at 10:08 PM, Juuso Oikarinen
> <juuso.oikarinen@xxxxxxxxx> wrote:
> > On Tue, 2010-05-25 at 21:02 +0200, ext Luis R. Rodriguez wrote:
> >> On Mon, May 24, 2010 at 11:50 PM, Juuso Oikarinen
> >> <juuso.oikarinen@xxxxxxxxx> wrote:
> >> > The userspace crda utility can fail to respond to kernel requests in (at least)
> >> > two scenarios: it is not runnable for any reason, or it is invoked with a
> >> > country code not in its database.
> >> >
> >> > When the userspace crda utility fails to respond to kernel requests (i.e. it
> >> > does not use NL80211_CMD_SET_REG to provide the kernel regulatory information
> >> > for the requested country) the kernel crda subsystem will stall. It will
> >> > refuse to process any further regulatory hints. This is easiest demonstrated
> >> > by using for instance the "iw" tool:
> >> >
> >> > iw reg set EU
> >> > iw reg set US
> >> >
> >> > "EU" is not a country code present in the database, so user space crda will
> >> > not respond. Attempting to define US after that will be silently ignored
> >> > (internally, an -EAGAIN is the result, as the "EU" request is still
> >> > "being processed".)
> >> >
> >> > To fix this issue, this patch implements timeout protection for the userspace
> >> > crda invocation. If there is no response for five seconds, the crda code will
> >> > force itself to the world regulatory domain for maximum safety.
> >> >
> >> > Signed-off-by: Juuso Oikarinen <juuso.oikarinen@xxxxxxxxx>
> >>
> >> This is a great idea, I really like it and appreciate you taking the
> >> time to work on this, however to do this it requires a little more
> >> work and will point out the notes below. So NACK for now but with some
> >> fixes I think this would be great.
> >>
> >> > ---
> >> > net/wireless/reg.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
> >> > 1 files changed, 44 insertions(+), 0 deletions(-)
> >> >
> >> > diff --git a/net/wireless/reg.c b/net/wireless/reg.c
> >> > index 8f0d97d..6c945f0 100644
> >> > --- a/net/wireless/reg.c
> >> > +++ b/net/wireless/reg.c
> >> > @@ -385,6 +385,40 @@ static inline void reg_regdb_query(const char *alpha2) {}
> >> > #endif /* CONFIG_CFG80211_INTERNAL_REGDB */
> >> >
> >> > /*
> >> > + * This gets invoked if crda in userspace is not responding (it's not getting
> >> > + * executed or the country code in the hint is not in the database.
> >> > + */
> >> > +
> >> > +static void call_crda_timeout_work(struct work_struct *work)
> >> > +{
> >> > + if (!last_request)
> >> > + return;
> >> > +
> >> > + printk(KERN_INFO "cfg80211: crda request timed out, reverting to 00\n");
> >> > +
> >> > + mutex_lock(&cfg80211_mutex);
> >> > +
> >> > + /*
> >> > + * As we are not getting data for the current country, force us back
> >> > + * to the world regdomain.
> >> > + */
> >> > + last_request->alpha2[0] = '0';
> >> > + last_request->alpha2[1] = '0';
> >> > + set_regdom(cfg80211_world_regdom);
> >> > + mutex_unlock(&cfg80211_mutex);
> >> > +}
> >>
> >>
> >> Actually you may want to consider using restore_regulatory_settings()
> >> instead as that would:
> >>
> >> * set the world regdom
> >> * then set the first user specified regulatory domain if they had one picked
> >>
> >> This would mean we would go into a loop here though if the user had
> >> specified 'EU' though so this routine first needs to be fixed to
> >> annotate a regulatory domain as invalid. Note that a regulatory cannot
> >> be invalid if CRDA just times out -- CRDA could time out if it was not
> >> present. So this is a bit tricky and not sure how to resolve it. John
> >> recently added support for building the regulatory database as part of
> >> the kernel but at the same time support letting CRDA still update the
> >> same info if it is present. Then for those setup it could be possible
> >> 'DE' exists on the core kernel regulatory database but if CRDA is not
> >> installed then CRDA will time out.
> >>
> >> Maybe what we can do is if both the internal kernel regdb *and* CRDA
> >> times out then mark the user specified request as invalid and restore
> >> to using the world regdom. This would allow the user to later install
> >> a CRDA with some newer regdb in userspace, for example. This allows
> >> for upgrading the db itself from userspace without making kernel
> >> changes. It allows for countries to be created.
> >
> > Sounds fair. Here's my initial thought:
> >
> > I will look into adding validation of the country code with the kernel
> > internal database if the db is enabled in the config - in which case we
> > proceed as you describe. In case the internal db is disabled in the
> > config, we proceed as the patch currently does?
> >
> > Would that sound like an acceptable approach?
>
> The first part yes, the second part no. You want to still do the
> restore of the regdomains, that routine already does most of the work
> you need to do except sanity checking in case the data the user
> submitted is bogus/invalid.
Ok.
As you say, currently the restore function will cause a loop, if the
user space CRDA is not available to run, as the function will invoke it
again.
Validating the country code is not easy without the kernel built-in
database, I'll need to think about another way of handling this
scenario.
I'll think about it and come back later.
-Juuso
>
> Luis
>
> Luis
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
