Re: [PATCH v2] Revert "cfg80211: fix processing world regdomain when non modular"
*From:* Johannes Berg *Sent:* Friday, June 14, 2019 4:01PM *To:* Hodaszi, Robert *Cc:* Linux-wireless *Subject:* Re: [PATCH v2] Revert "cfg80211: fix processing world regdomain when non modular" > On Fri, 2019-06-14 at 13:58 +0000, Hodaszi, Robert wrote: >> I didn't just resend that. I just realized, accidentally I forgot to fix >> the debug message printing function, that define doesn't exist anymore. >> Sorry for the confusion! > Oops. I looked too superficially then and didn't even see the > difference, sorry. > > I guess that's why Kalle always says you should have a patch changelog > :-) Shame on me, I was able to make a bug in a one-line change. :) > >> Under "original issue", you mean the issue, which commit >> 96cce12ff6e0bc9d9fcb2235e08b7fc150f96fd2 (cfg80211: fix processing world >> regdomain when non modular) supposed to fix? > Yes. > >> That still won't work, but >> that didn't work neither before I reverted the patch, because crda call >> timeout will just drop the last packet. Also, as it re-processed the >> last request, not just resent it, it caused undesired states. Like when >> I used 2 WiFi modules with US regulatory domains, after enumeration, my >> global regulator domain was set to "Country 98". >> >> To fix my issue, why I reverted the patch, and also fix the issue the >> reverted commit supposed to fix, I could imagine something like this. >> But I'm not sure, it doesn't have any side effect: > [snip] > > Ok, thanks. I guess I'll have to look at this in more detail. > > You don't happen to have a way to reproduce either issue with a hwsim > test case? > > johannes > To tell the truth, I never tried hwsim. But it's pretty trivial to repro it. You just need 2 WiFi modules, and put e.g. a "sleep 1" into the udev or mdev script, before it would call the "crda". That should trigger the issue immediately. After that change, I just did an "rmmod ath10k_pci; modprobe ath10k_pci", and bumm, "iw reg get" will should "Country 98". Robert
Re: [PATCH v2] Revert "cfg80211: fix processing world regdomain when non modular"
*From:* Johannes Berg
*Sent:* Friday, June 14, 2019 3:30PM
*To:* Hodaszi, Robert
*Cc:* Linux-wireless
*Subject:* Re: [PATCH v2] Revert "cfg80211: fix processing world
regdomain when non modular"
> On Fri, 2019-06-14 at 13:16 +0000, Hodaszi, Robert wrote:
>> This reverts commit 96cce12ff6e0bc9d9fcb2235e08b7fc150f96fd2.
>>
>> Re-triggering a reg_process_hint with the last request on all events,
>> can make the regulatory domain fail in case of multiple WiFi modules. On
>> slower boards (espacially with mdev), enumeration of the WiFi modules
>> can end up in an intersected regulatory domain, and user cannot set it
>> with 'iw reg set' anymore.
>>
>> This is happening, because:
>> - 1st module enumerates, queues up a regulatory request
>> - request gets processed by __reg_process_hint_driver():
>>- checks if previous was set by CORE -> yes
>> - checks if regulator domain changed -> yes, from '00' to e.g. 'US'
>>-> sends request to the 'crda'
>> - 2nd module enumerates, queues up a regulator request (which triggers
>> the reg_todo() work)
>> - reg_todo() -> reg_process_pending_hints() sees, that the last request
>> is not processed yet, so it tries to process it again.
>> __reg_process_hint driver() will run again, and:
>>- checks if the last request's initiator was the core -> no, it was
>> the driver (1st WiFi module)
>>- checks, if the previous initiator was the driver -> yes
>> - checks if the regulator domain changed -> yes, it was '00' (set by
>> core, and crda call did not return yet), and should be changed to 'US'
>>
>> --> __reg_process_hint_driver calls an intersect
>>
>> Besides, the reg_process_hint call with the last request is meaningless
>> since the crda call has a timeout work. If that timeout expires, the
>> first module's request will lost.
> It's pointless to resend when I still have the original patch pending,
> at least without any changes.
>
> That said, I looked at this today and I'm not sure how the code doesn't
> now have the original issue again?
>
> johannes
>
I didn't just resend that. I just realized, accidentally I forgot to fix
the debug message printing function, that define doesn't exist anymore.
Sorry for the confusion!
Under "original issue", you mean the issue, which commit
96cce12ff6e0bc9d9fcb2235e08b7fc150f96fd2 (cfg80211: fix processing world
regdomain when non modular) supposed to fix? That still won't work, but
that didn't work neither before I reverted the patch, because crda call
timeout will just drop the last packet. Also, as it re-processed the
last request, not just resent it, it caused undesired states. Like when
I used 2 WiFi modules with US regulatory domains, after enumeration, my
global regulator domain was set to "Country 98".
To fix my issue, why I reverted the patch, and also fix the issue the
reverted commit supposed to fix, I could imagine something like this.
But I'm not sure, it doesn't have any side effect:
diff --git a/linux/net/wireless/reg.c b/linux/net/wireless/reg.c
index 6fdb01b20b..13d564558d 100644
--- a/linux/net/wireless/reg.c
+++ b/linux/net/wireless/reg.c
@@ -2798,7 +2798,8 @@ static void reg_process_pending_hints(void)
diff --git a/linux/net/wireless/reg.c b/linux/net/wireless/reg.c
index 6fdb01b20b..13d564558d 100644
--- a/linux/net/wireless/reg.c
+++ b/linux/net/wireless/reg.c
@@ -2798,7 +2798,8 @@ static void reg_process_pending_hints(void)
/* When last_request->processed becomes true this will be
rescheduled */
if (lr && !lr->processed) {
- reg_process_hint(lr);
+ if (!reg_query_database(lr))
+ reg_free_request(lr);
return;
}
@@ -3175,6 +3176,7 @@ static void restore_regulatory_settings(bool
reset_user, bool cached)
struct reg_beacon *reg_beacon, *btmp;
LIST_HEAD(tmp_reg_req_list);
struct cfg80211_registered_device *rdev;
+ struct regulatory_request *lr;
ASSERT_RTNL();
@@ -3190,6 +3192,13 @@ static void restore_regulatory_settings(bool
reset_user, bool cached)
}
spin_unlock(®_indoor_lock);
+ /* If last request is pending, save it, will resubmit it */
+ lr = get_last_request();
+ if (lr && !lr->processed)
+ rcu_assign_pointer(last_request, NULL);
+ else
+ lr = NULL;
+
reset_regdomains(true, &world_regdom);
restore_alpha2(alpha2, reset_user);
@@ -3267,6 +3276,9 @@ static void restore_regulatory_settings(bool
reset_user, bool cached)
list_splice_tail_init(&tmp_reg_req_list, ®_requests_list);
spin_unlock(®_requests_lock);
+ if (lr != NULL)
+ rcu_assign_pointer(last_request, lr);
+
pr_debug("Kicking the queue\n");
schedule_work(®_work);
Best regards,
Robert Hodaszi
[PATCH v2] Revert "cfg80211: fix processing world regdomain when non modular"
This reverts commit 96cce12ff6e0bc9d9fcb2235e08b7fc150f96fd2.
Re-triggering a reg_process_hint with the last request on all events,
can make the regulatory domain fail in case of multiple WiFi modules. On
slower boards (espacially with mdev), enumeration of the WiFi modules
can end up in an intersected regulatory domain, and user cannot set it
with 'iw reg set' anymore.
This is happening, because:
- 1st module enumerates, queues up a regulatory request
- request gets processed by __reg_process_hint_driver():
- checks if previous was set by CORE -> yes
- checks if regulator domain changed -> yes, from '00' to e.g. 'US'
-> sends request to the 'crda'
- 2nd module enumerates, queues up a regulator request (which triggers
the reg_todo() work)
- reg_todo() -> reg_process_pending_hints() sees, that the last request
is not processed yet, so it tries to process it again.
__reg_process_hint driver() will run again, and:
- checks if the last request's initiator was the core -> no, it was
the driver (1st WiFi module)
- checks, if the previous initiator was the driver -> yes
- checks if the regulator domain changed -> yes, it was '00' (set by
core, and crda call did not return yet), and should be changed to 'US'
--> __reg_process_hint_driver calls an intersect
Besides, the reg_process_hint call with the last request is meaningless
since the crda call has a timeout work. If that timeout expires, the
first module's request will lost.
Signed-off-by: Robert Hodaszi
---
net/wireless/reg.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 4831ad745f91..327479ce69f5 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -2788,7 +2788,7 @@ static void reg_process_pending_hints(void)
/* When last_request->processed becomes true this will be rescheduled */
if (lr && !lr->processed) {
- reg_process_hint(lr);
+ pr_debug("Pending regulatory request, waiting for it to be
processed...\n");
return;
}
[PATCH] Revert "cfg80211: fix processing world regdomain when non modular"
This reverts commit 96cce12ff6e0bc9d9fcb2235e08b7fc150f96fd2.
Re-triggering a reg_process_hint with the last request on all events,
can make the regulatory domain fail in case of multiple WiFi modules. On
slower boards (espacially with mdev), enumeration of the WiFi modules
can end up in an intersected regulatory domain, and user cannot set it
with 'iw reg set' anymore.
This is happening, because:
- 1st module enumerates, queues up a regulatory request
- request gets processed by __reg_process_hint_driver():
- checks if previous was set by CORE -> yes
- checks if regulator domain changed -> yes, from '00' to e.g. 'US'
-> sends request to the 'crda'
- 2nd module enumerates, queues up a regulator request (which triggers
the reg_todo() work)
- reg_todo() -> reg_process_pending_hints() sees, that the last request
is not processed yet, so it tries to process it again.
__reg_process_hint driver() will run again, and:
- checks if the last request's initiator was the core -> no, it was
the driver (1st WiFi module)
- checks, if the previous initiator was the driver -> yes
- checks if the regulator domain changed -> yes, it was '00' (set by
core, and crda call did not return yet), and should be changed to 'US'
--> __reg_process_hint_driver calls an intersect
Besides, the reg_process_hint call with the last request is meaningless
since the crda call has a timeout work. If that timeout expires, the
first module's request will lost.
Signed-off-by: Robert Hodaszi
---
net/wireless/reg.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 4831ad745f91..b00ebf6fc696 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -2788,7 +2788,7 @@ static void reg_process_pending_hints(void)
/* When last_request->processed becomes true this will be rescheduled */
if (lr && !lr->processed) {
- reg_process_hint(lr);
+ REG_DBG_PRINT("Pending regulatory request, waiting for it to be
processed...\n");
return;
}
Re: [PATCH] cfg80211: modify lower retry limit to 0
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.
And also with the current lower limit, I cannot disable transmission
retries. The minimal transmit attempts value is 2 (retry is 1).
Robert
2015. 06. 3, szerda keltezéssel 15.05-kor Johannes Berg ezt írta:
> On Wed, 2015-06-03 at 14:46 +0200, Robert Hodaszi wrote:
> > From: Robert Hodaszi
> >
> > 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
>
N�r��yb�X��ǧv�^�){.n�+{��*ޕ�,�{ay� ʇڙ�,j��f���h���z� �w���
���j:+v���w�j�mzZ+�ݢj"��!�i
Cannot disable transmission retry
Hello, On kernel 3.13, I was able to disable the transmission retries on the wlan0 interface with 'iwconfig wlan0 retry 0'. Now I updated my kernel to 3.16, and it returns with 'Invalid argument' error. The cause is this commit: cfg80211: add sanity check for retry limit in wext-compat http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=f5aa0d21dd5ada040ff42b4d40357285e4ace441 --- 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 || + if (retry->disabled || retry->value < 1 || retry->value > 255 || (retry->flags & IW_RETRY_TYPE) != IW_RETRY_LIMIT) return -EINVAL; I'm wondering, is the 'retry->value < 1' is necessary? I would write 'retry->value < 0' instead, to make possible to disable the retries. Should I send a patch? Thanks! Robert Hodaszi
