| Message ID | 20191029115602.78990-1-markus.theil@tu-ilmenau.de (mailing list archive) |
|---|---|
| State | Rejected |
| Delegated to: | Johannes Berg |
| Headers | show |
| Series | nl80211: allow more operations for mesh and ad-hoc interfaces | expand |
On Tue, 2019-10-29 at 12:56 +0100, Markus Theil wrote: > This change allows mesh and ad-hoc interfaces to change beacons and > tx queue settings. The direct change of these settings should be ok > for these kind of interfaces and should maybe only forbidden for > station-like interface types. "should maybe"? :-) Not really, both of the changes are wrong. johannes
On 30.10.19 10:03, Johannes Berg wrote: > On Tue, 2019-10-29 at 12:56 +0100, Markus Theil wrote: >> This change allows mesh and ad-hoc interfaces to change beacons and >> tx queue settings. The direct change of these settings should be ok >> for these kind of interfaces and should maybe only forbidden for >> station-like interface types. > "should maybe"? :-) > > Not really, both of the changes are wrong. > > johannes > Mesh interfaces are allowed to perform EDCA according to the standard 802.11-2016. Of course, they should not implement HCF with HCCA as in 10.2.4.1: "The HCF shall be implemented in all QoS STAs except mesh STAs". But the MCF section (10.23.2 MCF contention based channel access) says: "MCF implements the same EDCA (see 10.22.2) as does HCF." QoS IBSS are also allowed in the standard: e.g. 4.7: "A QoS IBSS supports operation under the HCF using TXOPs gained through the EDCA mechanism." Of course, mesh STAs cannot send the EDCA parameter set in their beacons according to Table 9-27: "... is present if dot11QosOptionImplemented is true, and dot11MeshActivated is false, ...". Changing beacons on the fly from user-space in these modes is only useful, if vendor-specific elements are used, which can change over time. All in all I can nevertheless understand your point, that these changes could be "wrong" from a pragmatic point of view. Markus
On Wed, 2019-10-30 at 14:40 +0100, Markus Theil wrote: > > Mesh interfaces are allowed to perform EDCA according to the standard > 802.11-2016. Well, they *have to* in a sense :-) [...] > > Changing beacons on the fly from user-space in these modes is only > useful, if vendor-specific elements are used, which can change over time. > > All in all I can nevertheless understand your point, that these changes > could be "wrong" from a pragmatic point of view. No no, that's not even it. The problem is that you're focusing too much on the standard without understanding how the stack works. Take the QoS parameters again for example. Setting them from userspace is wrong because that data will immediately be forgotten and killed again by the call to ieee80211_set_wmm_default() in that code. Or look at how the change_beacon call is handled - the data you set here will never even be used for IBSS or mesh because in mac80211 ieee80211_change_beacon() will quite possibly even crash when you call it for a non-AP interface since it accesses sdata->u.ap.beacon without any other checks. So while the *idea* of being able to change beacons or WMM parameters *might* be correct, this kind of implementation is (fairly obviously) completely wrong. johannes
On 30.10.19 15:04, Johannes Berg wrote: > On Wed, 2019-10-30 at 14:40 +0100, Markus Theil wrote: >> Mesh interfaces are allowed to perform EDCA according to the standard >> 802.11-2016. > Well, they *have to* in a sense :-) > > [...] >> Changing beacons on the fly from user-space in these modes is only >> useful, if vendor-specific elements are used, which can change over time. >> >> All in all I can nevertheless understand your point, that these changes >> could be "wrong" from a pragmatic point of view. > No no, that's not even it. The problem is that you're focusing too much > on the standard without understanding how the stack works. > > Take the QoS parameters again for example. Setting them from userspace > is wrong because that data will immediately be forgotten and killed > again by the call to ieee80211_set_wmm_default() in that code. > > Or look at how the change_beacon call is handled - the data you set here > will never even be used for IBSS or mesh because in mac80211 > ieee80211_change_beacon() will quite possibly even crash when you call > it for a non-AP interface since it accesses sdata->u.ap.beacon without > any other checks. > > So while the *idea* of being able to change beacons or WMM parameters > *might* be correct, this kind of implementation is (fairly obviously) > completely wrong. > > johannes > Ok, thank you for the clarification :) Markus
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c index d1451e731bb8..c4ff8c2033af 100644 --- a/net/wireless/nl80211.c +++ b/net/wireless/nl80211.c @@ -2923,7 +2923,9 @@ static int nl80211_set_wiphy(struct sk_buff *skb, struct genl_info *info) return -EINVAL; if (netdev->ieee80211_ptr->iftype != NL80211_IFTYPE_AP && - netdev->ieee80211_ptr->iftype != NL80211_IFTYPE_P2P_GO) + netdev->ieee80211_ptr->iftype != NL80211_IFTYPE_P2P_GO && + netdev->ieee80211_ptr->iftype != NL80211_IFTYPE_MESH_POINT && + netdev->ieee80211_ptr->iftype != NL80211_IFTYPE_ADHOC) return -EINVAL; if (!netif_running(netdev)) @@ -4831,7 +4833,9 @@ static int nl80211_set_beacon(struct sk_buff *skb, struct genl_info *info) int err; if (dev->ieee80211_ptr->iftype != NL80211_IFTYPE_AP && - dev->ieee80211_ptr->iftype != NL80211_IFTYPE_P2P_GO) + dev->ieee80211_ptr->iftype != NL80211_IFTYPE_P2P_GO && + dev->ieee80211_ptr->iftype != NL80211_IFTYPE_MESH_POINT && + dev->ieee80211_ptr->iftype != NL80211_IFTYPE_ADHOC) return -EOPNOTSUPP; if (!rdev->ops->change_beacon)
This change allows mesh and ad-hoc interfaces to change beacons and tx queue settings. The direct change of these settings should be ok for these kind of interfaces and should maybe only forbidden for station-like interface types. Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de> --- net/wireless/nl80211.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)