[PATCH] cfg80211: modify lower retry limit to 0

[PATCH] cfg80211: modify lower retry limit to 0

[Robert Hodaszi]

From: Robert Hodaszi <robert.hodaszi@digi.com>

To disable the transmit retries on RT28xx driver, the retry limit should
be set to 0. The current lower limit (1) prevents it.

Signed-off-by: Robert Hodaszi <robert.hodaszi@digi.com>
---
 net/wireless/wext-compat.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/wireless/wext-compat.c b/net/wireless/wext-compat.c
index fff1bef..6e38320 100644
--- a/net/wireless/wext-compat.c
+++ b/net/wireless/wext-compat.c
@@ -370,7 +370,7 @@ static int cfg80211_wext_siwretry(struct net_device *dev,
 	u8 oshort = wdev->wiphy->retry_short;
 	int err;
 
-	if (retry->disabled || retry->value < 1 || retry->value > 255 ||
+	if (retry->disabled || retry->value < 0 || retry->value > 255 ||
 	    (retry->flags & IW_RETRY_TYPE) != IW_RETRY_LIMIT)
 		return -EINVAL;
 

Re: [PATCH] cfg80211: modify lower retry limit to 0

[Johannes Berg]

On Wed, 2015-06-03 at 14:46 +0200, Robert Hodaszi wrote:
> From: Robert Hodaszi <robert.hodaszi@digi.com>
> 
> To disable the transmit retries on RT28xx driver, the retry limit should
> be set to 0. The current lower limit (1) prevents it.

So actually - we have the same limit in nl80211... we should change both
at the same time.

However, I'm withdrawing my earlier comment. The dot11ShortRetryLimit
and dot11LongRetryLimit counters (which correspond to this) are defined
to have a range 1..255, and semantically are actually the number of
permissible transmission *attempts* (see their definition in
802.11-2012).

As a consequence, I'd say the driver is doing things wrong here and this
part is actually correct.

You could argue that this is userspace API that must never change,
but ... dunno. If you feel strongly about this I guess we could accept 0
and translate it to 1 in cfg80211 to get the correct semantics, but
you'd still have the driver bug then. I don't think we should accept 0
and pass it to the driver, since it need not expect such a value based
on the 802.11 spec.

johannes

Re: [PATCH] cfg80211: modify lower retry limit to 0

[Hodaszi, Robert]

VGhlIHByb2JsZW0gaXMgaWYgSSBjaGFuZ2UgdGhlIHJ0MjgwMCBkcml2ZXIgdG8gc2V0IDAgaWYg
aXQgZ2V0cyAxLA0KZXRjLiwgdGhhdCB3aWxsIGJyZWFrIGFsbCBleGlzdGluZyB1c2VyLXNwYWNl
IHByb2dyYW1zLg0KDQpBbmQgYWxzbyB3aXRoIHRoZSBjdXJyZW50IGxvd2VyIGxpbWl0LCBJIGNh
bm5vdCBkaXNhYmxlIHRyYW5zbWlzc2lvbg0KcmV0cmllcy4gVGhlIG1pbmltYWwgdHJhbnNtaXQg
YXR0ZW1wdHMgdmFsdWUgaXMgMiAocmV0cnkgaXMgMSkuDQoNClJvYmVydA0KDQoNCjIwMTUuIDA2
LiAzLCBzemVyZGEga2VsdGV6w6lzc2VsIDE1LjA1LWtvciBKb2hhbm5lcyBCZXJnIGV6dCDDrXJ0
YToNCj4gT24gV2VkLCAyMDE1LTA2LTAzIGF0IDE0OjQ2ICswMjAwLCBSb2JlcnQgSG9kYXN6aSB3
cm90ZToNCj4gPiBGcm9tOiBSb2JlcnQgSG9kYXN6aSA8cm9iZXJ0LmhvZGFzemlAZGlnaS5jb20+
DQo+ID4gDQo+ID4gVG8gZGlzYWJsZSB0aGUgdHJhbnNtaXQgcmV0cmllcyBvbiBSVDI4eHggZHJp
dmVyLCB0aGUgcmV0cnkgbGltaXQgc2hvdWxkDQo+ID4gYmUgc2V0IHRvIDAuIFRoZSBjdXJyZW50
IGxvd2VyIGxpbWl0ICgxKSBwcmV2ZW50cyBpdC4NCj4gDQo+IFNvIGFjdHVhbGx5IC0gd2UgaGF2
ZSB0aGUgc2FtZSBsaW1pdCBpbiBubDgwMjExLi4uIHdlIHNob3VsZCBjaGFuZ2UgYm90aA0KPiBh
dCB0aGUgc2FtZSB0aW1lLg0KPiANCj4gSG93ZXZlciwgSSdtIHdpdGhkcmF3aW5nIG15IGVhcmxp
ZXIgY29tbWVudC4gVGhlIGRvdDExU2hvcnRSZXRyeUxpbWl0DQo+IGFuZCBkb3QxMUxvbmdSZXRy
eUxpbWl0IGNvdW50ZXJzICh3aGljaCBjb3JyZXNwb25kIHRvIHRoaXMpIGFyZSBkZWZpbmVkDQo+
IHRvIGhhdmUgYSByYW5nZSAxLi4yNTUsIGFuZCBzZW1hbnRpY2FsbHkgYXJlIGFjdHVhbGx5IHRo
ZSBudW1iZXIgb2YNCj4gcGVybWlzc2libGUgdHJhbnNtaXNzaW9uICphdHRlbXB0cyogKHNlZSB0
aGVpciBkZWZpbml0aW9uIGluDQo+IDgwMi4xMS0yMDEyKS4NCj4gDQo+IEFzIGEgY29uc2VxdWVu
Y2UsIEknZCBzYXkgdGhlIGRyaXZlciBpcyBkb2luZyB0aGluZ3Mgd3JvbmcgaGVyZSBhbmQgdGhp
cw0KPiBwYXJ0IGlzIGFjdHVhbGx5IGNvcnJlY3QuDQo+IA0KPiBZb3UgY291bGQgYXJndWUgdGhh
dCB0aGlzIGlzIHVzZXJzcGFjZSBBUEkgdGhhdCBtdXN0IG5ldmVyIGNoYW5nZSwNCj4gYnV0IC4u
LiBkdW5uby4gSWYgeW91IGZlZWwgc3Ryb25nbHkgYWJvdXQgdGhpcyBJIGd1ZXNzIHdlIGNvdWxk
IGFjY2VwdCAwDQo+IGFuZCB0cmFuc2xhdGUgaXQgdG8gMSBpbiBjZmc4MDIxMSB0byBnZXQgdGhl
IGNvcnJlY3Qgc2VtYW50aWNzLCBidXQNCj4geW91J2Qgc3RpbGwgaGF2ZSB0aGUgZHJpdmVyIGJ1
ZyB0aGVuLiBJIGRvbid0IHRoaW5rIHdlIHNob3VsZCBhY2NlcHQgMA0KPiBhbmQgcGFzcyBpdCB0
byB0aGUgZHJpdmVyLCBzaW5jZSBpdCBuZWVkIG5vdCBleHBlY3Qgc3VjaCBhIHZhbHVlIGJhc2Vk
DQo+IG9uIHRoZSA4MDIuMTEgc3BlYy4NCj4gDQo+IGpvaGFubmVzDQo+IA0K

Re: [PATCH] cfg80211: modify lower retry limit to 0

[Johannes Berg]

On Wed, 2015-06-03 at 13:14 +0000, Hodaszi, Robert wrote:
> The problem is if I change the rt2800 driver to set 0 if it gets 1,
> etc., that will break all existing user-space programs.

Well, I guess it's a question of which userspace even uses it, and how
it'll break.

Perhaps we should modify the wext userspace API to allow 0-254, and add
1 to the value before passing it to the driver. That way, in wext it
would be "compatible" with your expected behaviour, but internally we'd
still use the 802.11 semantics. I'd accept a patch doing this, since
you're the only one to ever even admit to using it (and still using
wext) :-)

Of course we should also fix the driver to treat it as # of transmission
attempts rather than # of retries, since clearly that's required for the
nl80211 API which is documented to do it like the spec.

johannes