with the current API the multicast rate parameter is specifiable at ibss and
mesh join only. With this new API multicast rate can instead be modified
whenever needed by the user (only if the VIF type is ADHOC or MESH_POINT).
This API is particularly useful in case of use of wpa_supplicant with IBSS/RSN
since it does not support mcast_rate as ibss join parameter (this lead to the
impossibility of changing the mcast_rate while using IBSS/RSN).
Signed-off-by: Antonio Quartulli <[email protected]>
---
include/linux/nl80211.h | 2 ++
include/net/cfg80211.h | 3 ++
net/mac80211/cfg.c | 27 ++++++++++++++++
net/wireless/core.c | 26 +++++++++++++++
net/wireless/core.h | 3 ++
net/wireless/nl80211.c | 81 +++++++++++++++++++++++++++++++----------------
6 files changed, 114 insertions(+), 28 deletions(-)
diff --git a/include/linux/nl80211.h b/include/linux/nl80211.h
index db961a5..f8c51af 100644
--- a/include/linux/nl80211.h
+++ b/include/linux/nl80211.h
@@ -708,6 +708,8 @@ enum nl80211_commands {
NL80211_CMD_CH_SWITCH_NOTIFY,
+ NL80211_CMD_SET_MCAST_RATE,
+
/* add new commands above here */
/* used to define NL80211_CMD_MAX below */
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 51f67a9..e55448d 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1722,6 +1722,9 @@ struct cfg80211_ops {
int (*set_wiphy_params)(struct wiphy *wiphy, u32 changed);
+ int (*set_mcast_rate)(struct wiphy *wiphy, struct net_device *dev,
+ int mcast_rate[IEEE80211_NUM_BANDS]);
+
int (*set_tx_power)(struct wiphy *wiphy,
enum nl80211_tx_power_setting type, int mbm);
int (*get_tx_power)(struct wiphy *wiphy, int *dbm);
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index ccbe241..43e3fbb 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -1919,6 +1919,32 @@ static int ieee80211_set_wiphy_params(struct wiphy *wiphy, u32 changed)
return 0;
}
+static int ieee80211_set_mcast_rate(struct wiphy *wiphy, struct net_device *dev,
+ int mcast_rate[IEEE80211_NUM_BANDS])
+{
+ struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
+ u32 basic_rates = sdata->vif.bss_conf.basic_rates;
+ int i;
+
+ /* check if the mcast_rates are also in basic_rates */
+ for (i = 0; i < IEEE80211_NUM_BANDS; i++)
+ if (!(basic_rates & (1 << (mcast_rate[i] - 1))))
+ return -EINVAL;
+
+ switch (sdata->vif.type) {
+ case NL80211_IFTYPE_ADHOC:
+ case NL80211_IFTYPE_MESH_POINT:
+ memcpy(sdata->vif.bss_conf.mcast_rate, mcast_rate,
+ sizeof(mcast_rate));
+ break;
+ default:
+ return -EOPNOTSUPP;
+ }
+
+ return 0;
+
+}
+
static int ieee80211_set_tx_power(struct wiphy *wiphy,
enum nl80211_tx_power_setting type, int mbm)
{
@@ -3041,6 +3067,7 @@ struct cfg80211_ops mac80211_config_ops = {
.join_ibss = ieee80211_join_ibss,
.leave_ibss = ieee80211_leave_ibss,
.set_wiphy_params = ieee80211_set_wiphy_params,
+ .set_mcast_rate = ieee80211_set_mcast_rate,
.set_tx_power = ieee80211_set_tx_power,
.get_tx_power = ieee80211_get_tx_power,
.set_wds_peer = ieee80211_set_wds_peer,
diff --git a/net/wireless/core.c b/net/wireless/core.c
index eb60410..fff29a1 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -786,6 +786,32 @@ void cfg80211_update_iface_num(struct cfg80211_registered_device *rdev,
}
}
+int cfg80211_set_mcast_rate(struct cfg80211_registered_device *rdev,
+ struct net_device *dev,
+ struct cfg80211_ibss_params *params)
+{
+ struct wireless_dev *wdev = dev->ieee80211_ptr;
+ int err;
+
+ if (wdev->iftype != NL80211_IFTYPE_ADHOC &&
+ wdev->iftype != NL80211_IFTYPE_MESH_POINT)
+ return -EOPNOTSUPP;
+
+ if (!rdev->ops->set_mcast_rate)
+ return -EOPNOTSUPP;
+
+ if (!wdev->current_bss)
+ return -EINVAL;
+
+ mutex_lock(&rdev->devlist_mtx);
+ wdev_lock(wdev);
+ err = rdev->ops->set_mcast_rate(&rdev->wiphy, dev, params->mcast_rate);
+ wdev_unlock(wdev);
+ mutex_unlock(&rdev->devlist_mtx);
+
+ return err;
+}
+
static int cfg80211_netdev_notifier_call(struct notifier_block *nb,
unsigned long state,
void *ndev)
diff --git a/net/wireless/core.h b/net/wireless/core.h
index 377dc39..2d88c5e 100644
--- a/net/wireless/core.h
+++ b/net/wireless/core.h
@@ -293,6 +293,9 @@ int cfg80211_leave_ibss(struct cfg80211_registered_device *rdev,
void __cfg80211_ibss_joined(struct net_device *dev, const u8 *bssid);
int cfg80211_ibss_wext_join(struct cfg80211_registered_device *rdev,
struct wireless_dev *wdev);
+int cfg80211_set_mcast_rate(struct cfg80211_registered_device *rdev,
+ struct net_device *dev,
+ struct cfg80211_ibss_params *params);
/* mesh */
extern const struct mesh_config default_mesh_config;
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 2a5cdb6..cdc0e05 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -1039,6 +1039,7 @@ static int nl80211_send_wiphy(struct sk_buff *msg, u32 pid, u32 seq, int flags,
if (nla_put_u32(msg, i, NL80211_CMD_REGISTER_BEACONS))
goto nla_put_failure;
}
+ CMD(set_mcast_rate, SET_MCAST_RATE);
#ifdef CONFIG_NL80211_TESTMODE
CMD(testmode_cmd, TESTMODE);
@@ -1668,6 +1669,50 @@ static int nl80211_set_wiphy(struct sk_buff *skb, struct genl_info *info)
return result;
}
+static bool
+nl80211_parse_mcast_rate(struct cfg80211_registered_device *rdev,
+ int mcast_rate[IEEE80211_NUM_BANDS],
+ int rateval)
+{
+ struct wiphy *wiphy = &rdev->wiphy;
+ bool found = false;
+ int band, i;
+
+ for (band = 0; band < IEEE80211_NUM_BANDS; band++) {
+ struct ieee80211_supported_band *sband;
+
+ sband = wiphy->bands[band];
+ if (!sband)
+ continue;
+
+ for (i = 0; i < sband->n_bitrates; i++) {
+ if (sband->bitrates[i].bitrate == rateval) {
+ mcast_rate[band] = i + 1;
+ found = true;
+ break;
+ }
+ }
+ }
+
+ return found;
+}
+
+static int nl80211_set_mcast_rate(struct sk_buff *skb, struct genl_info *info)
+{
+ struct cfg80211_registered_device *rdev = info->user_ptr[0];
+ struct net_device *dev = info->user_ptr[1];
+ struct cfg80211_ibss_params ibss;
+ uint32_t mcast_attr;
+
+ memset(&ibss, 0, sizeof(ibss));
+
+ mcast_attr = nla_get_u32(info->attrs[NL80211_ATTR_MCAST_RATE]);
+ if (info->attrs[NL80211_ATTR_MCAST_RATE] &&
+ !nl80211_parse_mcast_rate(rdev, ibss.mcast_rate, mcast_attr))
+ return -EINVAL;
+
+ return cfg80211_set_mcast_rate(rdev, dev, &ibss);
+}
static int nl80211_send_iface(struct sk_buff *msg, u32 pid, u32 seq, int flags,
struct cfg80211_registered_device *rdev,
@@ -5066,34 +5111,6 @@ static int nl80211_disassociate(struct sk_buff *skb, struct genl_info *info)
local_state_change);
}
-static bool
-nl80211_parse_mcast_rate(struct cfg80211_registered_device *rdev,
- int mcast_rate[IEEE80211_NUM_BANDS],
- int rateval)
-{
- struct wiphy *wiphy = &rdev->wiphy;
- bool found = false;
- int band, i;
-
- for (band = 0; band < IEEE80211_NUM_BANDS; band++) {
- struct ieee80211_supported_band *sband;
-
- sband = wiphy->bands[band];
- if (!sband)
- continue;
-
- for (i = 0; i < sband->n_bitrates; i++) {
- if (sband->bitrates[i].bitrate == rateval) {
- mcast_rate[band] = i + 1;
- found = true;
- break;
- }
- }
- }
-
- return found;
-}
-
static int nl80211_join_ibss(struct sk_buff *skb, struct genl_info *info)
{
struct cfg80211_registered_device *rdev = info->user_ptr[0];
@@ -6726,6 +6743,14 @@ static struct genl_ops nl80211_ops[] = {
.internal_flags = NL80211_FLAG_NEED_RTNL,
},
{
+ .cmd = NL80211_CMD_SET_MCAST_RATE,
+ .doit = nl80211_set_mcast_rate,
+ .policy = nl80211_policy,
+ .flags = GENL_ADMIN_PERM,
+ .internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
+ NL80211_FLAG_NEED_RTNL,
+ },
+ {
.cmd = NL80211_CMD_GET_INTERFACE,
.doit = nl80211_get_interface,
.dumpit = nl80211_dump_interface,
--
1.7.9.4
On Mon, 2012-07-09 at 15:05 +0200, Johannes Berg wrote:
> On Mon, 2012-07-09 at 01:51 +0200, Antonio Quartulli wrote:
>
> > +static int ieee80211_set_mcast_rate(struct wiphy *wiphy, struct net_device *dev,
> > + int mcast_rate[IEEE80211_NUM_BANDS])
> > +{
> > + struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
> > + u32 basic_rates = sdata->vif.bss_conf.basic_rates;
> > + int i;
> > +
> > + /* check if the mcast_rates are also in basic_rates */
> > + for (i = 0; i < IEEE80211_NUM_BANDS; i++)
> > + if (!(basic_rates & BIT(mcast_rate[i] - 1)))
> > + return -EINVAL;
>
> So this is kinda broken. In fact, the whole basic rate thing is broken
> it seems.
>
> The mcast rate is per band, as it should, since you could find the same
> BSSID on a 5 GHz channel and then jump to that channel if the TSF is
> higher...
>
> However, the basic rates aren't, which is wrong: the basic rates bitmap
> could be 1,2,6,9. If the driver is like most drivers, that translates to
> a bitmap of 0x33. But 0x33, for most drivers, if applied to the 5 GHz
> rates means 6,9,24,36. See why this is broken? A rate bitmap can't be
> siwtched around between bands and still make any sense.
Ok actually maybe it's not broken. We adopt the basic rates from the
network we join, I guess, and the original basic rates specified from
userspace are only used when we actually *create* the network?
Still though, comparing the bits for all bands against the same bitmap
like you did in the above code is just wrong :-)
Maybe we shouldn't bother at all. If we'd join another network, we might
have to downgrade the mcast rate, but then if some peer leaves or we
join another one ... it gets messy quickly.
Since all of this is probably for special cases anyway, maybe we should
just not check the mcast rate and if you set a stupid one you don't get
packets through to all peers? Could be hard to debug though ...
johannes
On Mon, Jul 09, 2012 at 03:12:31PM +0200, Johannes Berg wrote:
> On Mon, 2012-07-09 at 15:05 +0200, Johannes Berg wrote:
> > On Mon, 2012-07-09 at 01:51 +0200, Antonio Quartulli wrote:
> >
> > > +static int ieee80211_set_mcast_rate(struct wiphy *wiphy, struct net_device *dev,
> > > + int mcast_rate[IEEE80211_NUM_BANDS])
> > > +{
> > > + struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
> > > + u32 basic_rates = sdata->vif.bss_conf.basic_rates;
> > > + int i;
> > > +
> > > + /* check if the mcast_rates are also in basic_rates */
> > > + for (i = 0; i < IEEE80211_NUM_BANDS; i++)
> > > + if (!(basic_rates & BIT(mcast_rate[i] - 1)))
> > > + return -EINVAL;
> >
> > So this is kinda broken. In fact, the whole basic rate thing is broken
> > it seems.
> >
> > The mcast rate is per band, as it should, since you could find the same
> > BSSID on a 5 GHz channel and then jump to that channel if the TSF is
> > higher...
> >
> > However, the basic rates aren't, which is wrong: the basic rates bitmap
> > could be 1,2,6,9. If the driver is like most drivers, that translates to
> > a bitmap of 0x33. But 0x33, for most drivers, if applied to the 5 GHz
> > rates means 6,9,24,36. See why this is broken? A rate bitmap can't be
> > siwtched around between bands and still make any sense.
>
> Ok actually maybe it's not broken. We adopt the basic rates from the
> network we join, I guess, and the original basic rates specified from
> userspace are only used when we actually *create* the network?
mhmm.. I have to check
>
> Still though, comparing the bits for all bands against the same bitmap
> like you did in the above code is just wrong :-)
>
right, I should select the BAND which we are using and check that index only.
> Maybe we shouldn't bother at all. If we'd join another network, we might
> have to downgrade the mcast rate, but then if some peer leaves or we
> join another one ... it gets messy quickly.
>
it's adhoc! :-)
> Since all of this is probably for special cases anyway, maybe we should
> just not check the mcast rate and if you set a stupid one you don't get
> packets through to all peers? Could be hard to debug though ...
definitely hard.
we should keep the intersection of all the rates we have seen and chose the one
set by the user if possible or the highest mandatory we have in our set. Sounds
too easy :-)
--
Antonio Quartulli
..each of us alone is worth nothing..
Ernesto "Che" Guevara
On Mon, 2012-07-09 at 15:18 +0200, Antonio Quartulli wrote:
> > So this is kinda broken. In fact, the whole basic rate thing is broken
> > it seems.
> >
> > The mcast rate is per band, as it should, since you could find the same
> > BSSID on a 5 GHz channel and then jump to that channel if the TSF is
> > higher...
> >
> > However, the basic rates aren't, which is wrong: the basic rates bitmap
> > could be 1,2,6,9. If the driver is like most drivers, that translates to
> > a bitmap of 0x33. But 0x33, for most drivers, if applied to the 5 GHz
> > rates means 6,9,24,36. See why this is broken? A rate bitmap can't be
> > siwtched around between bands and still make any sense.
>
> I see, I wrongly thought that nl80211_parse_mcast_rate() was checking if the
> provided mcast_rate belongs to the basic_rate set of the band which we are now.
Well, ok, don't get confused -- there's no basic rate set "of the band",
it's a BSS property.
> But that's wrong! nl80211_parse_mcast_rate() only checks if the provided
> mcast_rate exists somewhere...
It checks that each rate exists in the band that it's for.
> > Oh, also, I'm not sure why you do BIT(... -1), but that's unrelated.
> > What kind of value is the mcast rate? A rate index, or a number?
> >
>
> it's an index and actually it's the index +1, as reported in mac80211.h:
>
> * @mcast_rate: per-band multicast rate index + 1 (0: disabled)
also in the cfg80211 API?
johannes
On Sun, 2012-07-08 at 11:31 +0200, Antonio Quartulli wrote:
> +int cfg80211_set_mcast_rate(struct cfg80211_registered_device *rdev,
> + struct net_device *dev,
> + struct cfg80211_ibss_params *params)
I'd say just inline the function into the nl80211.c handler itself, not
much value in having it separate.
> + mcast_attr = nla_get_u32(info->attrs[NL80211_ATTR_MCAST_RATE]);
> + if (info->attrs[NL80211_ATTR_MCAST_RATE] &&
this is wrong
johannes
On Sun, Jul 08, 2012 at 11:56:26AM +0200, Johannes Berg wrote:
> On Sun, 2012-07-08 at 11:31 +0200, Antonio Quartulli wrote:
>
> > +int cfg80211_set_mcast_rate(struct cfg80211_registered_device *rdev,
> > + struct net_device *dev,
> > + struct cfg80211_ibss_params *params)
>
> I'd say just inline the function into the nl80211.c handler itself, not
> much value in having it separate.
ok, will do.
>
>
> > + mcast_attr = nla_get_u32(info->attrs[NL80211_ATTR_MCAST_RATE]);
> > + if (info->attrs[NL80211_ATTR_MCAST_RATE] &&
>
> this is wrong
ops, you are right! I moved the nla_get_u32() at the line before to avoid having
a ugly statement, but I missed something :)
Thank you
--
Antonio Quartulli
..each of us alone is worth nothing..
Ernesto "Che" Guevara
with the current API the multicast rate parameter is specifiable at ibss and
mesh join only. With this new API multicast rate can instead be modified
whenever needed by the user (only if the VIF type is ADHOC or MESH_POINT).
This API is particularly useful in case of use of wpa_supplicant with IBSS/RSN
since it does not support mcast_rate as ibss join parameter (this lead to the
impossibility of changing the mcast_rate while using IBSS/RSN).
Signed-off-by: Antonio Quartulli <[email protected]>
---
v2: removed useless locking, use BIT() instead of 1<<
include/linux/nl80211.h | 2 +
include/net/cfg80211.h | 3 ++
net/mac80211/cfg.c | 27 ++++++++++++++
net/wireless/nl80211.c | 95 +++++++++++++++++++++++++++++++++--------------
4 files changed, 99 insertions(+), 28 deletions(-)
diff --git a/include/linux/nl80211.h b/include/linux/nl80211.h
index db961a5..f8c51af 100644
--- a/include/linux/nl80211.h
+++ b/include/linux/nl80211.h
@@ -708,6 +708,8 @@ enum nl80211_commands {
NL80211_CMD_CH_SWITCH_NOTIFY,
+ NL80211_CMD_SET_MCAST_RATE,
+
/* add new commands above here */
/* used to define NL80211_CMD_MAX below */
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 51f67a9..e55448d 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1722,6 +1722,9 @@ struct cfg80211_ops {
int (*set_wiphy_params)(struct wiphy *wiphy, u32 changed);
+ int (*set_mcast_rate)(struct wiphy *wiphy, struct net_device *dev,
+ int mcast_rate[IEEE80211_NUM_BANDS]);
+
int (*set_tx_power)(struct wiphy *wiphy,
enum nl80211_tx_power_setting type, int mbm);
int (*get_tx_power)(struct wiphy *wiphy, int *dbm);
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index ccbe241..b8f907d 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -1919,6 +1919,32 @@ static int ieee80211_set_wiphy_params(struct wiphy *wiphy, u32 changed)
return 0;
}
+static int ieee80211_set_mcast_rate(struct wiphy *wiphy, struct net_device *dev,
+ int mcast_rate[IEEE80211_NUM_BANDS])
+{
+ struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
+ u32 basic_rates = sdata->vif.bss_conf.basic_rates;
+ int i;
+
+ /* check if the mcast_rates are also in basic_rates */
+ for (i = 0; i < IEEE80211_NUM_BANDS; i++)
+ if (!(basic_rates & BIT(mcast_rate[i] - 1)))
+ return -EINVAL;
+
+ switch (sdata->vif.type) {
+ case NL80211_IFTYPE_ADHOC:
+ case NL80211_IFTYPE_MESH_POINT:
+ memcpy(sdata->vif.bss_conf.mcast_rate, mcast_rate,
+ sizeof(mcast_rate));
+ break;
+ default:
+ return -EOPNOTSUPP;
+ }
+
+ return 0;
+
+}
+
static int ieee80211_set_tx_power(struct wiphy *wiphy,
enum nl80211_tx_power_setting type, int mbm)
{
@@ -3041,6 +3067,7 @@ struct cfg80211_ops mac80211_config_ops = {
.join_ibss = ieee80211_join_ibss,
.leave_ibss = ieee80211_leave_ibss,
.set_wiphy_params = ieee80211_set_wiphy_params,
+ .set_mcast_rate = ieee80211_set_mcast_rate,
.set_tx_power = ieee80211_set_tx_power,
.get_tx_power = ieee80211_get_tx_power,
.set_wds_peer = ieee80211_set_wds_peer,
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 2a5cdb6..b0630b2 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -1039,6 +1039,7 @@ static int nl80211_send_wiphy(struct sk_buff *msg, u32 pid, u32 seq, int flags,
if (nla_put_u32(msg, i, NL80211_CMD_REGISTER_BEACONS))
goto nla_put_failure;
}
+ CMD(set_mcast_rate, SET_MCAST_RATE);
#ifdef CONFIG_NL80211_TESTMODE
CMD(testmode_cmd, TESTMODE);
@@ -1668,6 +1669,64 @@ static int nl80211_set_wiphy(struct sk_buff *skb, struct genl_info *info)
return result;
}
+static bool
+nl80211_parse_mcast_rate(struct cfg80211_registered_device *rdev,
+ int mcast_rate[IEEE80211_NUM_BANDS],
+ int rateval)
+{
+ struct wiphy *wiphy = &rdev->wiphy;
+ bool found = false;
+ int band, i;
+
+ for (band = 0; band < IEEE80211_NUM_BANDS; band++) {
+ struct ieee80211_supported_band *sband;
+
+ sband = wiphy->bands[band];
+ if (!sband)
+ continue;
+
+ for (i = 0; i < sband->n_bitrates; i++) {
+ if (sband->bitrates[i].bitrate == rateval) {
+ mcast_rate[band] = i + 1;
+ found = true;
+ break;
+ }
+ }
+ }
+
+ return found;
+}
+
+static int nl80211_set_mcast_rate(struct sk_buff *skb, struct genl_info *info)
+{
+ struct cfg80211_registered_device *rdev = info->user_ptr[0];
+ struct net_device *dev = info->user_ptr[1];
+ struct wireless_dev *wdev = dev->ieee80211_ptr;
+ u32 mcast_attr;
+ int mcast_rate[IEEE80211_NUM_BANDS];
+ int err;
+
+ if (!info->attrs[NL80211_ATTR_MCAST_RATE])
+ return -EINVAL;
+
+ mcast_attr = nla_get_u32(info->attrs[NL80211_ATTR_MCAST_RATE]);
+ if (!nl80211_parse_mcast_rate(rdev, mcast_rate, mcast_attr))
+ return -EINVAL;
+
+ if (wdev->iftype != NL80211_IFTYPE_ADHOC &&
+ wdev->iftype != NL80211_IFTYPE_MESH_POINT)
+ return -EOPNOTSUPP;
+
+ if (!rdev->ops->set_mcast_rate)
+ return -EOPNOTSUPP;
+
+ if (!wdev->current_bss)
+ return -EINVAL;
+
+ err = rdev->ops->set_mcast_rate(&rdev->wiphy, dev, mcast_rate);
+
+ return err;
+}
static int nl80211_send_iface(struct sk_buff *msg, u32 pid, u32 seq, int flags,
struct cfg80211_registered_device *rdev,
@@ -5066,34 +5125,6 @@ static int nl80211_disassociate(struct sk_buff *skb, struct genl_info *info)
local_state_change);
}
-static bool
-nl80211_parse_mcast_rate(struct cfg80211_registered_device *rdev,
- int mcast_rate[IEEE80211_NUM_BANDS],
- int rateval)
-{
- struct wiphy *wiphy = &rdev->wiphy;
- bool found = false;
- int band, i;
-
- for (band = 0; band < IEEE80211_NUM_BANDS; band++) {
- struct ieee80211_supported_band *sband;
-
- sband = wiphy->bands[band];
- if (!sband)
- continue;
-
- for (i = 0; i < sband->n_bitrates; i++) {
- if (sband->bitrates[i].bitrate == rateval) {
- mcast_rate[band] = i + 1;
- found = true;
- break;
- }
- }
- }
-
- return found;
-}
-
static int nl80211_join_ibss(struct sk_buff *skb, struct genl_info *info)
{
struct cfg80211_registered_device *rdev = info->user_ptr[0];
@@ -6726,6 +6757,14 @@ static struct genl_ops nl80211_ops[] = {
.internal_flags = NL80211_FLAG_NEED_RTNL,
},
{
+ .cmd = NL80211_CMD_SET_MCAST_RATE,
+ .doit = nl80211_set_mcast_rate,
+ .policy = nl80211_policy,
+ .flags = GENL_ADMIN_PERM,
+ .internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
+ NL80211_FLAG_NEED_RTNL,
+ },
+ {
.cmd = NL80211_CMD_GET_INTERFACE,
.doit = nl80211_get_interface,
.dumpit = nl80211_dump_interface,
--
1.7.9.4
On Mon, Jul 09, 2012 at 03:19:59PM +0200, Johannes Berg wrote:
> On Mon, 2012-07-09 at 15:18 +0200, Antonio Quartulli wrote:
>
> > > So this is kinda broken. In fact, the whole basic rate thing is broken
> > > it seems.
> > >
> > > The mcast rate is per band, as it should, since you could find the same
> > > BSSID on a 5 GHz channel and then jump to that channel if the TSF is
> > > higher...
> > >
> > > However, the basic rates aren't, which is wrong: the basic rates bitmap
> > > could be 1,2,6,9. If the driver is like most drivers, that translates to
> > > a bitmap of 0x33. But 0x33, for most drivers, if applied to the 5 GHz
> > > rates means 6,9,24,36. See why this is broken? A rate bitmap can't be
> > > siwtched around between bands and still make any sense.
> >
> > I see, I wrongly thought that nl80211_parse_mcast_rate() was checking if the
> > provided mcast_rate belongs to the basic_rate set of the band which we are now.
>
> Well, ok, don't get confused -- there's no basic rate set "of the band",
> it's a BSS property.
>
> > But that's wrong! nl80211_parse_mcast_rate() only checks if the provided
> > mcast_rate exists somewhere...
>
> It checks that each rate exists in the band that it's for.
>
> > > Oh, also, I'm not sure why you do BIT(... -1), but that's unrelated.
> > > What kind of value is the mcast rate? A rate index, or a number?
> > >
> >
> > it's an index and actually it's the index +1, as reported in mac80211.h:
> >
> > * @mcast_rate: per-band multicast rate index + 1 (0: disabled)
>
> also in the cfg80211 API?
yes
>
> johannes
--
Antonio Quartulli
..each of us alone is worth nothing..
Ernesto "Che" Guevara
On Mon, 2012-07-09 at 01:51 +0200, Antonio Quartulli wrote:
> +static int ieee80211_set_mcast_rate(struct wiphy *wiphy, struct net_device *dev,
> + int mcast_rate[IEEE80211_NUM_BANDS])
> +{
> + struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
> + u32 basic_rates = sdata->vif.bss_conf.basic_rates;
> + int i;
> +
> + /* check if the mcast_rates are also in basic_rates */
> + for (i = 0; i < IEEE80211_NUM_BANDS; i++)
> + if (!(basic_rates & BIT(mcast_rate[i] - 1)))
> + return -EINVAL;
So this is kinda broken. In fact, the whole basic rate thing is broken
it seems.
The mcast rate is per band, as it should, since you could find the same
BSSID on a 5 GHz channel and then jump to that channel if the TSF is
higher...
However, the basic rates aren't, which is wrong: the basic rates bitmap
could be 1,2,6,9. If the driver is like most drivers, that translates to
a bitmap of 0x33. But 0x33, for most drivers, if applied to the 5 GHz
rates means 6,9,24,36. See why this is broken? A rate bitmap can't be
siwtched around between bands and still make any sense.
Oh, also, I'm not sure why you do BIT(... -1), but that's unrelated.
What kind of value is the mcast rate? A rate index, or a number?
johannes
On Mon, Jul 09, 2012 at 03:05:46PM +0200, Johannes Berg wrote:
> On Mon, 2012-07-09 at 01:51 +0200, Antonio Quartulli wrote:
>
> > +static int ieee80211_set_mcast_rate(struct wiphy *wiphy, struct net_device *dev,
> > + int mcast_rate[IEEE80211_NUM_BANDS])
> > +{
> > + struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
> > + u32 basic_rates = sdata->vif.bss_conf.basic_rates;
> > + int i;
> > +
> > + /* check if the mcast_rates are also in basic_rates */
> > + for (i = 0; i < IEEE80211_NUM_BANDS; i++)
> > + if (!(basic_rates & BIT(mcast_rate[i] - 1)))
> > + return -EINVAL;
>
> So this is kinda broken. In fact, the whole basic rate thing is broken
> it seems.
>
> The mcast rate is per band, as it should, since you could find the same
> BSSID on a 5 GHz channel and then jump to that channel if the TSF is
> higher...
>
> However, the basic rates aren't, which is wrong: the basic rates bitmap
> could be 1,2,6,9. If the driver is like most drivers, that translates to
> a bitmap of 0x33. But 0x33, for most drivers, if applied to the 5 GHz
> rates means 6,9,24,36. See why this is broken? A rate bitmap can't be
> siwtched around between bands and still make any sense.
I see, I wrongly thought that nl80211_parse_mcast_rate() was checking if the
provided mcast_rate belongs to the basic_rate set of the band which we are now.
But that's wrong! nl80211_parse_mcast_rate() only checks if the provided
mcast_rate exists somewhere...
>
> Oh, also, I'm not sure why you do BIT(... -1), but that's unrelated.
> What kind of value is the mcast rate? A rate index, or a number?
>
it's an index and actually it's the index +1, as reported in mac80211.h:
* @mcast_rate: per-band multicast rate index + 1 (0: disabled)
> johannes
--
Antonio Quartulli
..each of us alone is worth nothing..
Ernesto "Che" Guevara