[PATCH] nl80211: allow more operations for mesh and ad-hoc interfaces

[PATCH] nl80211: allow more operations for mesh and ad-hoc interfaces

[Markus Theil]

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

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

Re: [PATCH] nl80211: allow more operations for mesh and ad-hoc interfaces

[Johannes Berg]

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

Re: [PATCH] nl80211: allow more operations for mesh and ad-hoc interfaces

[Markus Theil]

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

Re: [PATCH] nl80211: allow more operations for mesh and ad-hoc interfaces

[Johannes Berg]

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

Re: [PATCH] nl80211: allow more operations for mesh and ad-hoc interfaces

[Markus Theil]

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