Search Linux Wireless

Re: [PATCH 3/3] nl80211: New command for setting TX rate mask for rate control

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



 On 8/20/10 9:33 AM, Philip Prindeville wrote:
 On 8/20/10 2:20 AM, Johannes Berg wrote:
On Thu, 2010-08-19 at 14:26 -0700, Philip Prindeville wrote:

+    /*
+     * The nested attribute uses enum nl80211_band as the index. This maps
+     * directly to the enum ieee80211_band values used in cfg80211.
+     */
+    nla_for_each_nested(tx_rates, info->attrs[NL80211_ATTR_TX_RATES], rem)
+    {
+        enum ieee80211_band band = nla_type(tx_rates);
Can this even work?  The first entry in nl80211_band is NL80211_BAND_2GHZ, i.e. zero.

Yet looking at libnl-1.1/lib/attr.c there's:

int nla_parse(struct nlattr *tb[], int maxtype, struct nlattr *head, int len,
                struct nla_policy *policy)
{
...
          nla_for_each_attr(nla, head, len, rem) {
                  int type = nla_type(nla);

                  if (type == 0) {
Well, notice how you're quoting code that does the same thing
(for_each_attr), but the latter code does the extra non-zero check,
which we don't.

We don't use nla_parse for this, we have to use
nla_parse_nested/for_each_attr. Yeah, it's a little quirky, but still
works fine.

johannes


Well, I grepped through all the relevant source (i.e. libnl and iw) and the only place that the string "Illegal nla->nla_type == 0" gets printed was there, and I'm definitely seeing it.

So while the condition might be set in the driver, it's detected in "iw" here.

That was my point.


What I'm trying to say is that even though the driver handles the condition of nla_type == 0 when it generates the message, the condition isn't handled correct when the message finally gets passed up into user-space, because the above sequence in the netlink library gets hit both for "iw" and "hostapd".

So it's not enough to handle it correctly in the driver: it needs to be handled in libnl as well.

There are 3 potential solutions:

(1) go with a 1-based enum for the band, instead of 0-based;
(2) remove the test for nla_type == 0 in nla_parse() in libnl;
(3) use a way to convey the band that doesn't involve overloading nla_type;

What's the correct fix?

Thanks,

-Philip

--
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


[Index of Archives]     [Linux Host AP]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Linux Kernel]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]
  Powered by Linux