[PATCH] wireless: change cfg80211 regulatory domain info as debug messages

[PATCH] wireless: change cfg80211 regulatory domain info as debug messages

[Dave Young]

cfg80211 module prints a lot of messages like below. Actually printing
once is acceptable but sometimes it will print again and again, it looks
very annoying. It is better to change these detail messages to debugging
only.

cfg80211: World regulatory domain updated:
cfg80211:  DFS Master region: unset
cfg80211:   (start_freq - end_freq @ bandwidth), (max_antenna_gain, max_eirp), (dfs_cac_time)
cfg80211:   (2402000 KHz - 2472000 KHz @ 40000 KHz), (N/A, 2000 mBm), (N/A)
cfg80211:   (2457000 KHz - 2482000 KHz @ 40000 KHz), (N/A, 2000 mBm), (N/A)
cfg80211:   (2474000 KHz - 2494000 KHz @ 20000 KHz), (N/A, 2000 mBm), (N/A)
cfg80211:   (5170000 KHz - 5250000 KHz @ 80000 KHz, 160000 KHz AUTO), (N/A, 2000 mBm), (N/A)
cfg80211:   (5250000 KHz - 5330000 KHz @ 80000 KHz, 160000 KHz AUTO), (N/A, 2000 mBm), (0 s)
cfg80211:   (5490000 KHz - 5730000 KHz @ 160000 KHz), (N/A, 2000 mBm), (0 s)
cfg80211:   (5735000 KHz - 5835000 KHz @ 80000 KHz), (N/A, 2000 mBm), (N/A)
cfg80211:   (57240000 KHz - 63720000 KHz @ 2160000 KHz), (N/A, 0 mBm), (N/A)

The changes in this patch is to replace pr_info with pr_debug in function
print_rd_rules and print_regdomain_info

Signed-off-by: Dave Young <dyoung@redhat.com>
---
 net/wireless/reg.c |   24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

--- linux.orig/net/wireless/reg.c
+++ linux/net/wireless/reg.c
@@ -2763,7 +2763,7 @@ static void print_rd_rules(const struct
 	const struct ieee80211_power_rule *power_rule = NULL;
 	char bw[32], cac_time[32];
 
-	pr_info("  (start_freq - end_freq @ bandwidth), (max_antenna_gain, max_eirp), (dfs_cac_time)\n");
+	pr_debug("  (start_freq - end_freq @ bandwidth), (max_antenna_gain, max_eirp), (dfs_cac_time)\n");
 
 	for (i = 0; i < rd->n_reg_rules; i++) {
 		reg_rule = &rd->reg_rules[i];
@@ -2790,7 +2790,7 @@ static void print_rd_rules(const struct
 		 * in certain regions
 		 */
 		if (power_rule->max_antenna_gain)
-			pr_info("  (%d KHz - %d KHz @ %s), (%d mBi, %d mBm), (%s)\n",
+			pr_debug("  (%d KHz - %d KHz @ %s), (%d mBi, %d mBm), (%s)\n",
 				freq_range->start_freq_khz,
 				freq_range->end_freq_khz,
 				bw,
@@ -2798,7 +2798,7 @@ static void print_rd_rules(const struct
 				power_rule->max_eirp,
 				cac_time);
 		else
-			pr_info("  (%d KHz - %d KHz @ %s), (N/A, %d mBm), (%s)\n",
+			pr_debug("  (%d KHz - %d KHz @ %s), (N/A, %d mBm), (%s)\n",
 				freq_range->start_freq_khz,
 				freq_range->end_freq_khz,
 				bw,
@@ -2831,35 +2831,35 @@ static void print_regdomain(const struct
 			struct cfg80211_registered_device *rdev;
 			rdev = cfg80211_rdev_by_wiphy_idx(lr->wiphy_idx);
 			if (rdev) {
-				pr_info("Current regulatory domain updated by AP to: %c%c\n",
+				pr_debug("Current regulatory domain updated by AP to: %c%c\n",
 					rdev->country_ie_alpha2[0],
 					rdev->country_ie_alpha2[1]);
 			} else
-				pr_info("Current regulatory domain intersected:\n");
+				pr_debug("Current regulatory domain intersected:\n");
 		} else
-			pr_info("Current regulatory domain intersected:\n");
+			pr_debug("Current regulatory domain intersected:\n");
 	} else if (is_world_regdom(rd->alpha2)) {
-		pr_info("World regulatory domain updated:\n");
+		pr_debug("World regulatory domain updated:\n");
 	} else {
 		if (is_unknown_alpha2(rd->alpha2))
-			pr_info("Regulatory domain changed to driver built-in settings (unknown country)\n");
+			pr_debug("Regulatory domain changed to driver built-in settings (unknown country)\n");
 		else {
 			if (reg_request_cell_base(lr))
-				pr_info("Regulatory domain changed to country: %c%c by Cell Station\n",
+				pr_debug("Regulatory domain changed to country: %c%c by Cell Station\n",
 					rd->alpha2[0], rd->alpha2[1]);
 			else
-				pr_info("Regulatory domain changed to country: %c%c\n",
+				pr_debug("Regulatory domain changed to country: %c%c\n",
 					rd->alpha2[0], rd->alpha2[1]);
 		}
 	}
 
-	pr_info(" DFS Master region: %s", reg_dfs_region_str(rd->dfs_region));
+	pr_debug(" DFS Master region: %s", reg_dfs_region_str(rd->dfs_region));
 	print_rd_rules(rd);
 }
 
 static void print_regdomain_info(const struct ieee80211_regdomain *rd)
 {
-	pr_info("Regulatory domain: %c%c\n", rd->alpha2[0], rd->alpha2[1]);
+	pr_debug("Regulatory domain: %c%c\n", rd->alpha2[0], rd->alpha2[1]);
 	print_rd_rules(rd);
 }
 

Re: [PATCH] wireless: change cfg80211 regulatory domain info as debug messages

[Joe Perches]

On Sun, 2015-11-15 at 15:31 +0800, Dave Young wrote:
> cfg80211 module prints a lot of messages like below. Actually printing
> once is acceptable but sometimes it will print again and again, it looks
> very annoying. It is better to change these detail messages to debugging
> only.

So maybe add some wrapper that does a pr_info then
a pr_debug for the second and subsequent uses like:

#define pr_info_once_then_debug(fmt, ...)			\
({								\
	static bool __print_once __read_mostly;			\
								\
	if (!__print_once) {					\
		__print_once = true;				\
		pr_info(fmt, ##__VA_ARGS__);			\
	} else {						\
		pr_debug(fmt, ##__VA_ARGS__);			\
	}							\
})

Re: [PATCH] wireless: change cfg80211 regulatory domain info as debug messages

[Dave Young]

On 11/15/15 at 09:38am, Joe Perches wrote:
> On Sun, 2015-11-15 at 15:31 +0800, Dave Young wrote:
> > cfg80211 module prints a lot of messages like below. Actually printing
> > once is acceptable but sometimes it will print again and again, it looks
> > very annoying. It is better to change these detail messages to debugging
> > only.
> 
> So maybe add some wrapper that does a pr_info then
> a pr_debug for the second and subsequent uses like:
> 
> #define pr_info_once_then_debug(fmt, ...)			\
> ({								\
> 	static bool __print_once __read_mostly;			\
> 								\
> 	if (!__print_once) {					\
> 		__print_once = true;				\
> 		pr_info(fmt, ##__VA_ARGS__);			\
> 	} else {						\
> 		pr_debug(fmt, ##__VA_ARGS__);			\
> 	}							\
> })
> 

Hmm, it looks too much for this issue, I'm thinking about to add a cmdline
param to disable mute it.

Thanks
Dave

Re: [PATCH] wireless: change cfg80211 regulatory domain info as debug messages

[Johannes Berg]

> So maybe add some wrapper that does a pr_info then
> a pr_debug for the second and subsequent uses like:
> 

That seems like a bad idea - one might be tricked into think that one
saw the current data, but the actually current data is later hidden.

johannes

Re: [PATCH] wireless: change cfg80211 regulatory domain info as debug messages

[Johannes Berg]

> So maybe add some wrapper that does a pr_info then
> a pr_debug for the second and subsequent uses like:
> 

That seems like a bad idea - one might be tricked into think that one
saw the current data, but the actually current data is later hidden.

johannes
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] wireless: change cfg80211 regulatory domain info as debug messages

[Stefan Lippers-Hollmann]

Hi

On 2015-11-15, Dave Young wrote:
> cfg80211 module prints a lot of messages like below. Actually printing
> once is acceptable but sometimes it will print again and again, it looks
> very annoying. It is better to change these detail messages to debugging
> only.

It is a lot of info, easily repeated 3 times on boot, but it's also the
only real chance to determine why you ended up with the regulatory 
domain settings you got, rather than just the values itself. Given that
a lot (most?) of officially shipping wireless devices are misconfigured
(wrong EEPROM regdom settings for the region they're sold in) and 
considering that the limits can even change at runtime (IEEE 802.11d), 
it is imho quite important not just to be able what the current 
restrictions (iw reg get) are, but also why the kernel settled on those.

Regards
	Stefan Lippers-Hollmann

Re: [PATCH] wireless: change cfg80211 regulatory domain info as debug messages

[Dave Young]

Hi,

On 11/15/15 at 07:25pm, Stefan Lippers-Hollmann wrote:
> Hi
> 
> On 2015-11-15, Dave Young wrote:
> > cfg80211 module prints a lot of messages like below. Actually printing
> > once is acceptable but sometimes it will print again and again, it looks
> > very annoying. It is better to change these detail messages to debugging
> > only.
> 
> It is a lot of info, easily repeated 3 times on boot, but it's also the
> only real chance to determine why you ended up with the regulatory 
> domain settings you got, rather than just the values itself. Given that
> a lot (most?) of officially shipping wireless devices are misconfigured
> (wrong EEPROM regdom settings for the region they're sold in) and 
> considering that the limits can even change at runtime (IEEE 802.11d), 
> it is imho quite important not just to be able what the current 
> restrictions (iw reg get) are, but also why the kernel settled on those.

If it is really important then a kernel cmdline param to disable the logs
sounds better?

Thanks
Dave

Re: [PATCH] wireless: change cfg80211 regulatory domain info as debug messages

[Dave Young]

Hi,

On 11/15/15 at 07:25pm, Stefan Lippers-Hollmann wrote:
> Hi
> 
> On 2015-11-15, Dave Young wrote:
> > cfg80211 module prints a lot of messages like below. Actually printing
> > once is acceptable but sometimes it will print again and again, it looks
> > very annoying. It is better to change these detail messages to debugging
> > only.
> 
> It is a lot of info, easily repeated 3 times on boot, but it's also the
> only real chance to determine why you ended up with the regulatory 
> domain settings you got, rather than just the values itself. Given that
> a lot (most?) of officially shipping wireless devices are misconfigured
> (wrong EEPROM regdom settings for the region they're sold in) and 
> considering that the limits can even change at runtime (IEEE 802.11d), 
> it is imho quite important not just to be able what the current 
> restrictions (iw reg get) are, but also why the kernel settled on those.

If it is really important then a kernel cmdline param to disable the logs
sounds better?

Thanks
Dave
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] wireless: change cfg80211 regulatory domain info as debug messages

[Johannes Berg]

On Sun, 2015-11-15 at 19:25 +0100, Stefan Lippers-Hollmann wrote:
> Hi
> 
> On 2015-11-15, Dave Young wrote:
> > cfg80211 module prints a lot of messages like below. Actually
> > printing once is acceptable but sometimes it will print again and
> > again, it looks very annoying. It is better to change these detail
> > messages to debugging only.
> 
> It is a lot of info, easily repeated 3 times on boot, but it's also
> the only real chance to determine why you ended up with the
> regulatory domain settings you got, rather than just the values
> itself. Given that a lot (most?) of officially shipping wireless
> devices are misconfigured (wrong EEPROM regdom settings for the
> region they're sold in) and considering that the limits can even
> change at runtime (IEEE 802.11d), it is imho quite important not just
> to be able what the current restrictions (iw reg get) are, but also
> why the kernel settled on those.
> 

Hm. I kinda sympathize with both points of view here, not sure what to
do.

Maybe we could skip this for the world regdomain only? It doesn't
really change, and we typically don't care that much for it? That'd
probably get rid of most of the lines already.

Alternatively, perhaps the internal computations should be more
transparently visible through some other mechanism?

johannes

Re: [PATCH] wireless: change cfg80211 regulatory domain info as debug messages

[Dave Young]

On 11/20/15 at 12:55pm, Johannes Berg wrote:
> On Sun, 2015-11-15 at 19:25 +0100, Stefan Lippers-Hollmann wrote:
> > Hi
> > 
> > On 2015-11-15, Dave Young wrote:
> > > cfg80211 module prints a lot of messages like below. Actually
> > > printing once is acceptable but sometimes it will print again and
> > > again, it looks very annoying. It is better to change these detail
> > > messages to debugging only.
> > 
> > It is a lot of info, easily repeated 3 times on boot, but it's also
> > the only real chance to determine why you ended up with the
> > regulatory domain settings you got, rather than just the values
> > itself. Given that a lot (most?) of officially shipping wireless
> > devices are misconfigured (wrong EEPROM regdom settings for the
> > region they're sold in) and considering that the limits can even
> > change at runtime (IEEE 802.11d), it is imho quite important not just
> > to be able what the current restrictions (iw reg get) are, but also
> > why the kernel settled on those.
> > 
> 
> Hm. I kinda sympathize with both points of view here, not sure what to
> do.
> 
> Maybe we could skip this for the world regdomain only? It doesn't
> really change, and we typically don't care that much for it? That'd
> probably get rid of most of the lines already.
> 
> Alternatively, perhaps the internal computations should be more
> transparently visible through some other mechanism?
> 

If they are for debugging purpose I would like to see them as pr_debug
or something in debugfs. Especially for printks which will not only
being called on initialization phase.

Seems there're a lot of other wireless messages. Should we refactor 
them as well? I still did not get chance to see where is the code.
(My wireless driver being used is iwlwifi)

# uptime
 09:36:31 up 17 days, 19:17, 11 users,  load average: 0.26, 0.25, 0.17

#dmesg|grep wlp3s0|wc
   4868   54014  404187

# dmesg|grep "Limiting TX power"|wc
   4128   49600  360052

Thanks
Dave

Re: [PATCH] wireless: change cfg80211 regulatory domain info as debug messages

[Johannes Berg]

On Mon, 2015-11-23 at 09:37 +0800, Dave Young wrote:

> Seems there're a lot of other wireless messages. Should we refactor 
> them as well? I still did not get chance to see where is the code.
> (My wireless driver being used is iwlwifi)

Most are probably from net/mac80211/.

> # dmesg|grep "Limiting TX power"|wc
>    4128   49600  360052
> 

I fixed this one recently, due to a bug it was printed all the time
instead of just once when taking effect.

johannes

Re: [PATCH] wireless: change cfg80211 regulatory domain info as debug messages

[Johannes Berg]

On Mon, 2015-11-23 at 09:37 +0800, Dave Young wrote:

> Seems there're a lot of other wireless messages. Should we refactor 
> them as well? I still did not get chance to see where is the code.
> (My wireless driver being used is iwlwifi)

Most are probably from net/mac80211/.

> # dmesg|grep "Limiting TX power"|wc
>    4128   49600  360052
> 

I fixed this one recently, due to a bug it was printed all the time
instead of just once when taking effect.

johannes
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] wireless: change cfg80211 regulatory domain info as debug messages

[Dave Young]

Hi,

On 12/11/15 at 03:26pm, Johannes Berg wrote:
> On Mon, 2015-11-23 at 09:37 +0800, Dave Young wrote:
> 
> > Seems there're a lot of other wireless messages. Should we refactor 
> > them as well? I still did not get chance to see where is the code.
> > (My wireless driver being used is iwlwifi)
> 
> Most are probably from net/mac80211/.
> 
> > # dmesg|grep "Limiting TX power"|wc
> >    4128   49600  360052
> > 
> 
> I fixed this one recently, due to a bug it was printed all the time
> instead of just once when taking effect.

Cool, has the fix been in mainline or somewhere else?

Thanks
Dave

Re: [PATCH] wireless: change cfg80211 regulatory domain info as debug messages

[Johannes Berg]

On Thu, 2015-12-17 at 11:19 +0800, Dave Young wrote:
> 
> Cool, has the fix been in mainline or somewhere else?
> 

https://git.kernel.org/cgit/linux/kernel/git/davem/net.git/commit/?id=a87da0cbc42949cefc8282c39ab4cb8c460bd6ea

johannes

Re: [PATCH] wireless: change cfg80211 regulatory domain info as debug messages

[Johannes Berg]

On Sun, 2015-11-15 at 15:31 +0800, Dave Young wrote:
> cfg80211 module prints a lot of messages like below. Actually
> printing once is acceptable but sometimes it will print again and
> again, it looks very annoying. It is better to change these detail
> messages to debugging only.
> 

Despite the objections, I've applied this patch now.

I've made one change: keeping the alpha2 (e.g. "US") printed in some of
the pr_err() cases in this file.
I also got rid of CONFIG_CFG80211_REG_DEBUG in a separate patch.

I somewhat agree with the objections, but if the kernel is with
CONFIG_DYNAMIC_DEBUG then it's really simple to get the messages back
by enabling them for this file.

Where the messages were used as an indication of something having gone
awry at a different level (e.g. mac80211 disconnect) I don't really
quite agree - that then perhaps should have a more explicit (and less
noisy) message.

I also agree that the regulatory code is quite opaque, and the way it
arrives at certain conclusions is not always obvious. These messages
don't help all that much though since they don't contain the actual
input to the decisions. I think for that, we'd be much better served
with some kind of tracepoint or so that records all the information.

johannes

Re: [PATCH] wireless: change cfg80211 regulatory domain info as debug messages

[Dave Young]

Hi, Johannes

Sorry for late feedback, I was busying on other things.

On 12/11/15 at 03:38pm, Johannes Berg wrote:
> On Sun, 2015-11-15 at 15:31 +0800, Dave Young wrote:
> > cfg80211 module prints a lot of messages like below. Actually
> > printing once is acceptable but sometimes it will print again and
> > again, it looks very annoying. It is better to change these detail
> > messages to debugging only.
> > 
> 
> Despite the objections, I've applied this patch now.
> 

Thanks a lot.

> I've made one change: keeping the alpha2 (e.g. "US") printed in some of
> the pr_err() cases in this file.
> I also got rid of CONFIG_CFG80211_REG_DEBUG in a separate patch.
> 
> I somewhat agree with the objections, but if the kernel is with
> CONFIG_DYNAMIC_DEBUG then it's really simple to get the messages back
> by enabling them for this file.
> 
> Where the messages were used as an indication of something having gone
> awry at a different level (e.g. mac80211 disconnect) I don't really
> quite agree - that then perhaps should have a more explicit (and less
> noisy) message.
> 
> I also agree that the regulatory code is quite opaque, and the way it
> arrives at certain conclusions is not always obvious. These messages
> don't help all that much though since they don't contain the actual
> input to the decisions. I think for that, we'd be much better served
> with some kind of tracepoint or so that records all the information.

I think you guys are expert in this area, I will agree with all of
above. But I hope we can have some rate limited messages at least
especially for endless things.

Thanks
Dave