diff mbox

[160/306] mac80211-hwsim: add rate-limited debugging for rx-netlink

Message ID 1487896109-23851-5-git-send-email-greearb@candelatech.com (mailing list archive)
State Changes Requested
Delegated to: Johannes Berg
Headers show

Commit Message

Ben Greear Feb. 24, 2017, 12:28 a.m. UTC 
From: Ben Greear <greearb@candelatech.com>

This makes it easier to understand why wmediumd (or similar)
is getting errors when sending frames to the kernel.

Signed-off-by: Ben Greear <greearb@candelatech.com>
---
 drivers/net/wireless/mac80211_hwsim.c | 42 ++++++++++++++++++++++++++---------
 1 file changed, 32 insertions(+), 10 deletions(-)

Comments

Johannes Berg Feb. 24, 2017, 6:39 a.m. UTC | #1 
> +	    !info->attrs[HWSIM_ATTR_SIGNAL]) {
> +		if (net_ratelimit())
> +			printk(KERN_DEBUG " hwsim rx-nl: Missing
> required attribute\n");

I'm not convinced net_ratelimit() is a good idea, that's a global rate
limiter.

johannes
Ben Greear Feb. 24, 2017, 3:27 p.m. UTC | #2 
On 02/23/2017 10:39 PM, Johannes Berg wrote:
>
>> +	    !info->attrs[HWSIM_ATTR_SIGNAL]) {
>> +		if (net_ratelimit())
>> +			printk(KERN_DEBUG " hwsim rx-nl: Missing
>> required attribute\n");
>
> I'm not convinced net_ratelimit() is a good idea, that's a global rate
> limiter.

Is there a better rate-limiter w/out hand-crafting something?

Thanks,
Ben
Johannes Berg Feb. 27, 2017, 2:33 p.m. UTC | #3 
On Fri, 2017-02-24 at 07:27 -0800, Ben Greear wrote:
> 
> On 02/23/2017 10:39 PM, Johannes Berg wrote:
> > 
> > > +	    !info->attrs[HWSIM_ATTR_SIGNAL]) {
> > > +		if (net_ratelimit())
> > > +			printk(KERN_DEBUG " hwsim rx-nl: Missing
> > > required attribute\n");
> > 
> > I'm not convinced net_ratelimit() is a good idea, that's a global
> > rate limiter.
> 
> Is there a better rate-limiter w/out hand-crafting something?

You can use include/linux/ratelimit.h to do that?

johannes
diff mbox

Patch

diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c
index 8d494ac..aaba126 100644
--- a/drivers/net/wireless/mac80211_hwsim.c
+++ b/drivers/net/wireless/mac80211_hwsim.c
@@ -3010,8 +3010,11 @@  static int hwsim_cloned_frame_received_nl(struct sk_buff *skb_2,
 	if (!info->attrs[HWSIM_ATTR_ADDR_RECEIVER] ||
 	    !info->attrs[HWSIM_ATTR_FRAME] ||
 	    !info->attrs[HWSIM_ATTR_RX_RATE] ||
-	    !info->attrs[HWSIM_ATTR_SIGNAL])
+	    !info->attrs[HWSIM_ATTR_SIGNAL]) {
+		if (net_ratelimit())
+			printk(KERN_DEBUG " hwsim rx-nl: Missing required attribute\n");
 		goto out;
+	}
 
 	dst = (void *)nla_data(info->attrs[HWSIM_ATTR_ADDR_RECEIVER]);
 	frame_data_len = nla_len(info->attrs[HWSIM_ATTR_FRAME]);
@@ -3019,29 +3022,50 @@  static int hwsim_cloned_frame_received_nl(struct sk_buff *skb_2,
 
 	/* Allocate new skb here */
 	skb = alloc_skb(frame_data_len, GFP_KERNEL);
-	if (skb == NULL)
-		goto err;
+	if (skb == NULL) {
+		if (net_ratelimit())
+			printk(KERN_DEBUG " hwsim rx-nl: skb alloc failed, len: %d\n",
+			       frame_data_len);
+		goto out;
+	}
 
-	if (frame_data_len > IEEE80211_MAX_DATA_LEN)
-		goto err;
+	if (frame_data_len > IEEE80211_MAX_DATA_LEN) {
+		if (net_ratelimit())
+			printk(KERN_DEBUG " hwsim rx-nl: data lenth error: %d  max: %d\n",
+			       frame_data_len, IEEE80211_MAX_DATA_LEN);
+		goto out;
+	}
 
 	/* Copy the data */
 	memcpy(skb_put(skb, frame_data_len), frame_data, frame_data_len);
 
 	data2 = get_hwsim_data_ref_from_addr(dst);
-	if (!data2)
+
+	if (!data2) {
+		if (net_ratelimit())
+			printk(KERN_DEBUG " hwsim rx-nl: Cannot find radio %pM\n",
+			       dst);
 		goto out;
+	}
 
 	if (hwsim_net_get_netgroup(genl_info_net(info)) != data2->netgroup)
 		goto out;
 
-	if (info->snd_portid != data2->wmediumd)
+	if (info->snd_portid != data2->wmediumd) {
+		if (net_ratelimit())
+			printk(KERN_DEBUG " hwsim rx-nl: portid mismatch, snd_portid: %d  portid %d\n",
+			       info->snd_portid, data2->wmediumd);
 		goto out;
+	}
 
 	/* check if radio is configured properly */
 
-	if (data2->idle || !data2->started)
+	if (data2->idle || !data2->started) {
+		if (net_ratelimit())
+			printk(KERN_DEBUG " hwsim rx-nl: radio %pM idle: %d or not started: %d\n",
+			       dst, data2->idle, !data2->started);
 		goto out;
+	}
 
 	/* A frame is received from user space */
 	memset(&rx_status, 0, sizeof(rx_status));
@@ -3084,8 +3108,6 @@  static int hwsim_cloned_frame_received_nl(struct sk_buff *skb_2,
 	ieee80211_rx_irqsafe(data2->hw, skb);
 
 	return 0;
-err:
-	printk(KERN_DEBUG "mac80211_hwsim: error occurred in %s\n", __func__);
 out:
 	dev_kfree_skb(skb);
 	return -EINVAL;