* [PATCH v2 1/4] mac80211-hwsim: notify user-space about channel change.
2017-02-27 21:56 [PATCH v2 2/4] mac80211-hwsim: remove dmesg spam about get-survey greearb
@ 2017-02-27 21:56 ` greearb
2017-03-06 12:58 ` Johannes Berg
2017-03-06 12:59 ` Johannes Berg
2017-02-27 21:56 ` [PATCH v2 3/4] mac80211-hwsim: add rate-limited debugging for rx-netlink greearb
2017-02-27 21:56 ` [PATCH v2 4/4] mac80211-hwsim: add length checks before allocating skb greearb
2 siblings, 2 replies; 7+ messages in thread
From: greearb @ 2017-02-27 21:56 UTC (permalink / raw)
To: linux-wireless; +Cc: Ben Greear
From: Ben Greear <greearb@candelatech.com>
The goal is to allow the user-space application to properly
filter packets before sending them down to the kernel. This
should more closely mimic what a real piece of hardware would
do.
Signed-off-by: Ben Greear <greearb@candelatech.com>
---
v2: Change IDs, add new ones for width, freq1, freq2.
drivers/net/wireless/mac80211_hwsim.c | 67 +++++++++++++++++++++++++++++++++++
drivers/net/wireless/mac80211_hwsim.h | 16 +++++++++
2 files changed, 83 insertions(+)
diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c
index 0150747..12e9333 100644
--- a/drivers/net/wireless/mac80211_hwsim.c
+++ b/drivers/net/wireless/mac80211_hwsim.c
@@ -527,6 +527,10 @@ struct mac80211_hwsim_data {
u8 scan_addr[ETH_ALEN];
struct ieee80211_channel *channel;
+ enum nl80211_chan_width ch_width;
+ u32 center_freq1;
+ u32 center_freq2;
+
u64 beacon_int /* beacon interval in us */;
unsigned int rx_filter;
bool started, idle, scanning;
@@ -998,6 +1002,64 @@ static int hwsim_unicast_netgroup(struct mac80211_hwsim_data *data,
return res;
}
+static void mac80211_hwsim_check_nl_notify(struct mac80211_hwsim_data *data)
+{
+ struct sk_buff *skb;
+ u32 center_freq = 0;
+ u32 _portid;
+ void *msg_head;
+
+ /* wmediumd mode check */
+ _portid = READ_ONCE(data->wmediumd);
+
+ if (!_portid)
+ return;
+
+ skb = genlmsg_new(GENLMSG_DEFAULT_SIZE, GFP_ATOMIC);
+ if (!skb)
+ goto err_print;
+
+ msg_head = genlmsg_put(skb, 0, 0, &hwsim_genl_family, 0,
+ HWSIM_CMD_NOTIFY_CH_CHANGE);
+ if (!msg_head) {
+ pr_debug("mac80211_hwsim: problem with msg_head, notify-ch-change\n");
+ goto nla_put_failure;
+ }
+
+ if (nla_put_u32(skb, HWSIM_ATTR_RADIO_ID, data->idx))
+ goto nla_put_failure;
+
+ if (nla_put(skb, HWSIM_ATTR_ADDR_TRANSMITTER,
+ ETH_ALEN, data->addresses[1].addr))
+ goto nla_put_failure;
+
+ if (data->channel)
+ center_freq = data->channel->center_freq;
+
+ if (nla_put_u32(skb, HWSIM_ATTR_FREQ, center_freq))
+ goto nla_put_failure;
+
+ if (nla_put_u32(skb, HWSIM_ATTR_CH_FREQ1, data->center_freq1))
+ goto nla_put_failure;
+
+ if (nla_put_u32(skb, HWSIM_ATTR_CH_FREQ2, data->center_freq2))
+ goto nla_put_failure;
+
+ if (nla_put_u32(skb, HWSIM_ATTR_CH_WIDTH, data->ch_width))
+ goto nla_put_failure;
+
+ genlmsg_end(skb, msg_head);
+ if (genlmsg_unicast(&init_net, skb, _portid))
+ goto err_print;
+
+ return;
+
+nla_put_failure:
+ nlmsg_free(skb);
+err_print:
+ pr_debug("mac80211_hwsim: error occurred in %s\n", __func__);
+}
+
static void mac80211_hwsim_tx_frame_nl(struct ieee80211_hw *hw,
struct sk_buff *my_skb,
int dst_portid)
@@ -1599,6 +1661,9 @@ static int mac80211_hwsim_config(struct ieee80211_hw *hw, u32 changed)
data->idle = !!(conf->flags & IEEE80211_CONF_IDLE);
data->channel = conf->chandef.chan;
+ data->ch_width = conf->chandef.width;
+ data->center_freq1 = conf->chandef.center_freq1;
+ data->center_freq2 = conf->chandef.center_freq2;
WARN_ON(data->channel && data->use_chanctx);
@@ -1615,6 +1680,8 @@ static int mac80211_hwsim_config(struct ieee80211_hw *hw, u32 changed)
HRTIMER_MODE_REL);
}
+ mac80211_hwsim_check_nl_notify(data);
+
return 0;
}
diff --git a/drivers/net/wireless/mac80211_hwsim.h b/drivers/net/wireless/mac80211_hwsim.h
index 39f2246..1251fdd 100644
--- a/drivers/net/wireless/mac80211_hwsim.h
+++ b/drivers/net/wireless/mac80211_hwsim.h
@@ -71,6 +71,15 @@ enum hwsim_tx_control_flags {
* @HWSIM_CMD_DEL_RADIO: destroy a radio, reply is multicasted
* @HWSIM_CMD_GET_RADIO: fetch information about existing radios, uses:
* %HWSIM_ATTR_RADIO_ID
+ * @HWSIM_CMD_NOTIFY_CH_CHANGE: notify user-space about channel-change. This is
+ * designed to help the user-space app better emulate radio hardware.
+ * This command uses:
+ * %HWSIM_ATTR_FREQ # Notify current operating center frequency.
+ * %HWSIM_ATTR_CH_FREQ1 # Configured center-freq-1.
+ * %HWSIM_ATTR_CH_FREQ2 # Configured center-freq-2.
+ * %HWSIM_ATTR_CH_WIDTH # Configured channel width.
+ * %HWSIM_ATTR_ADDR_TRANSMITTER # ID which radio we are notifying about.
+ * %HWSIM_ATTR_ADDR_ID # Numeric Radio ID.
* @__HWSIM_CMD_MAX: enum limit
*/
enum {
@@ -81,6 +90,7 @@ enum {
HWSIM_CMD_NEW_RADIO,
HWSIM_CMD_DEL_RADIO,
HWSIM_CMD_GET_RADIO,
+ HWSIM_CMD_NOTIFY_CH_CHANGE,
__HWSIM_CMD_MAX,
};
#define HWSIM_CMD_MAX (_HWSIM_CMD_MAX - 1)
@@ -123,6 +133,9 @@ enum {
* @HWSIM_ATTR_RADIO_NAME: Name of radio, e.g. phy666
* @HWSIM_ATTR_NO_VIF: Do not create vif (wlanX) when creating radio.
* @HWSIM_ATTR_FREQ: Frequency at which packet is transmitted or received.
+ * @HWSIM_ATTR_CH_FREQ1: Configured center-freq-1.
+ * @HWSIM_ATTR_CH_FREQ2: Configured center-freq-2.
+ * @HWSIM_ATTR_CH_WIDTH: Configured channel width.
* @__HWSIM_ATTR_MAX: enum limit
*/
@@ -149,6 +162,9 @@ enum {
HWSIM_ATTR_NO_VIF,
HWSIM_ATTR_FREQ,
HWSIM_ATTR_PAD,
+ HWSIM_ATTR_CH_FREQ1,
+ HWSIM_ATTR_CH_FREQ2,
+ HWSIM_ATTR_CH_WIDTH,
__HWSIM_ATTR_MAX,
};
#define HWSIM_ATTR_MAX (__HWSIM_ATTR_MAX - 1)
--
2.4.11
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH v2 3/4] mac80211-hwsim: add rate-limited debugging for rx-netlink
2017-02-27 21:56 [PATCH v2 2/4] mac80211-hwsim: remove dmesg spam about get-survey greearb
2017-02-27 21:56 ` [PATCH v2 1/4] mac80211-hwsim: notify user-space about channel change greearb
@ 2017-02-27 21:56 ` greearb
2017-02-28 6:31 ` Joe Perches
2017-02-27 21:56 ` [PATCH v2 4/4] mac80211-hwsim: add length checks before allocating skb greearb
2 siblings, 1 reply; 7+ messages in thread
From: greearb @ 2017-02-27 21:56 UTC (permalink / raw)
To: linux-wireless; +Cc: Ben Greear
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>
---
v2: Add and use hwsim_ratelimit() instead of net_ratelimit.
Squash two patches into this one.
drivers/net/wireless/mac80211_hwsim.c | 55 +++++++++++++++++++++++++++--------
1 file changed, 43 insertions(+), 12 deletions(-)
diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c
index 635ad03..c259b99 100644
--- a/drivers/net/wireless/mac80211_hwsim.c
+++ b/drivers/net/wireless/mac80211_hwsim.c
@@ -137,6 +137,12 @@ static int regtest = HWSIM_REGTEST_DISABLED;
module_param(regtest, int, 0444);
MODULE_PARM_DESC(regtest, "The type of regulatory test we want to run");
+DEFINE_RATELIMIT_STATE(hwsim_ratelimit_state, 5 * HZ, 10);
+int hwsim_ratelimit(void)
+{
+ return __ratelimit(&hwsim_ratelimit_state);
+}
+
static const char *hwsim_alpha2s[] = {
"FI",
"AL",
@@ -1641,7 +1647,7 @@ static int mac80211_hwsim_config(struct ieee80211_hw *hw, u32 changed)
if (conf->chandef.chan)
wiphy_debug(hw->wiphy,
- "%s (freq=%d(%d - %d)/%s idle=%d ps=%d smps=%s)\n",
+ "%s (chandef-chan freq=%d(%d - %d)/%s idle=%d ps=%d smps=%s)\n",
__func__,
conf->chandef.chan->center_freq,
conf->chandef.center_freq1,
@@ -1652,7 +1658,7 @@ static int mac80211_hwsim_config(struct ieee80211_hw *hw, u32 changed)
smps_modes[conf->smps_mode]);
else
wiphy_debug(hw->wiphy,
- "%s (freq=0 idle=%d ps=%d smps=%s)\n",
+ "%s (no-chandef-chan freq=0 idle=%d ps=%d smps=%s)\n",
__func__,
!!(conf->flags & IEEE80211_CONF_IDLE),
!!(conf->flags & IEEE80211_CONF_PS),
@@ -3018,8 +3024,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 (hwsim_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]);
@@ -3027,29 +3036,53 @@ 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 (hwsim_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 (hwsim_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 (hwsim_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 (hwsim_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) {
+ static unsigned int cnt = 0;
+ /* This is fairly common, and usually not a bug. So, print errors
+ rarely. */
+ if (((cnt++ & 0x3FF) == 0x3FF) && hwsim_ratelimit())
+ printk(KERN_DEBUG " hwsim rx-nl: radio %pM idle: %d or not started: %d cnt: %d\n",
+ dst, data2->idle, !data2->started, cnt);
goto out;
+ }
/* A frame is received from user space */
memset(&rx_status, 0, sizeof(rx_status));
@@ -3082,8 +3115,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;
--
2.4.11
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH v2 3/4] mac80211-hwsim: add rate-limited debugging for rx-netlink
2017-02-27 21:56 ` [PATCH v2 3/4] mac80211-hwsim: add rate-limited debugging for rx-netlink greearb
@ 2017-02-28 6:31 ` Joe Perches
0 siblings, 0 replies; 7+ messages in thread
From: Joe Perches @ 2017-02-28 6:31 UTC (permalink / raw)
To: greearb, linux-wireless
On Mon, 2017-02-27 at 13:56 -0800, greearb@candelatech.com wrote:
> 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>
> ---
>
> v2: Add and use hwsim_ratelimit() instead of net_ratelimit.
> Squash two patches into this one.
>
> drivers/net/wireless/mac80211_hwsim.c | 55 +++++++++++++++++++++++++++--------
> 1 file changed, 43 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c
[]
> @@ -137,6 +137,12 @@ static int regtest = HWSIM_REGTEST_DISABLED;
> module_param(regtest, int, 0444);
> MODULE_PARM_DESC(regtest, "The type of regulatory test we want to run");
>
> +DEFINE_RATELIMIT_STATE(hwsim_ratelimit_state, 5 * HZ, 10);
> +int hwsim_ratelimit(void)
> +{
> + return __ratelimit(&hwsim_ratelimit_state);
> +}
Maybe it'd be better to add a function like
__printf(1, 2)
static void hwsim_dbg_ratelimited(const char *fmt, ...)
{
struct va_format vaf;
va_list args;
if (!__ratelimit(&hwsim_ratelimit_state))
return;
va_start(args, fmt);
vaf.fmt = fmt;
vaf.va = &args;
printk(KERN_DEBUG "hwsim rx-nl: %pV", &vaf);
va_end(args);
}
[]
> @@ -3018,8 +3024,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 (hwsim_ratelimit())
> + printk(KERN_DEBUG " hwsim rx-nl: Missing required attribute\n");
so this becomes:
hwsim_dbg_ratelimited("Missing required attribute\n");
and why are these currently indented with a space?
> goto out;
> + }
>
> dst = (void *)nla_data(info->attrs[HWSIM_ATTR_ADDR_RECEIVER]);
> frame_data_len = nla_len(info->attrs[HWSIM_ATTR_FRAME]);
> @@ -3027,29 +3036,53 @@ 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 (hwsim_ratelimit())
> + printk(KERN_DEBUG " hwsim rx-nl: skb alloc failed, len: %d\n",
> + frame_data_len);
hwsim_dbg_ratelimited("skb alloc failed, len: %d\n",
frame_data_len);
etc...
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 4/4] mac80211-hwsim: add length checks before allocating skb.
2017-02-27 21:56 [PATCH v2 2/4] mac80211-hwsim: remove dmesg spam about get-survey greearb
2017-02-27 21:56 ` [PATCH v2 1/4] mac80211-hwsim: notify user-space about channel change greearb
2017-02-27 21:56 ` [PATCH v2 3/4] mac80211-hwsim: add rate-limited debugging for rx-netlink greearb
@ 2017-02-27 21:56 ` greearb
2 siblings, 0 replies; 7+ messages in thread
From: greearb @ 2017-02-27 21:56 UTC (permalink / raw)
To: linux-wireless; +Cc: Ben Greear
From: Ben Greear <greearb@candelatech.com>
Modify the receive-from-user-space logic to do length
and 'is-down' checks before trying to allocate an skb.
And, if we are going to ignore the pkt due to radio idle,
then do not return an error code to user-space. User-space
cannot reliably know exactly when a radio is idle or not.
Signed-off-by: Ben Greear <greearb@candelatech.com>
---
v2: Don't return success when radio is idle, but do return unique
error code (ENETDOWN) in hopes user-space can make a distinction.
drivers/net/wireless/mac80211_hwsim.c | 41 +++++++++++++++++++----------------
1 file changed, 22 insertions(+), 19 deletions(-)
diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c
index c259b99..73dc627 100644
--- a/drivers/net/wireless/mac80211_hwsim.c
+++ b/drivers/net/wireless/mac80211_hwsim.c
@@ -3020,6 +3020,7 @@ static int hwsim_cloned_frame_received_nl(struct sk_buff *skb_2,
int frame_data_len;
void *frame_data;
struct sk_buff *skb = NULL;
+ int rv = -EINVAL;
if (!info->attrs[HWSIM_ATTR_ADDR_RECEIVER] ||
!info->attrs[HWSIM_ATTR_FRAME] ||
@@ -3034,25 +3035,6 @@ static int hwsim_cloned_frame_received_nl(struct sk_buff *skb_2,
frame_data_len = nla_len(info->attrs[HWSIM_ATTR_FRAME]);
frame_data = (void *)nla_data(info->attrs[HWSIM_ATTR_FRAME]);
- /* Allocate new skb here */
- skb = alloc_skb(frame_data_len, GFP_KERNEL);
- if (skb == NULL) {
- if (hwsim_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) {
- if (hwsim_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) {
@@ -3081,9 +3063,30 @@ static int hwsim_cloned_frame_received_nl(struct sk_buff *skb_2,
if (((cnt++ & 0x3FF) == 0x3FF) && hwsim_ratelimit())
printk(KERN_DEBUG " hwsim rx-nl: radio %pM idle: %d or not started: %d cnt: %d\n",
dst, data2->idle, !data2->started, cnt);
+ rv = -ENETDOWN;
goto out;
}
+ if (frame_data_len > IEEE80211_MAX_DATA_LEN) {
+ if (hwsim_ratelimit())
+ printk(KERN_DEBUG " hwsim rx-nl: data lenth error: %d max: %d\n",
+ frame_data_len, IEEE80211_MAX_DATA_LEN);
+ goto out;
+ }
+
+
+ /* Allocate new skb here */
+ skb = alloc_skb(frame_data_len, GFP_KERNEL);
+ if (skb == NULL) {
+ if (hwsim_ratelimit())
+ printk(KERN_DEBUG " hwsim rx-nl: skb alloc failed, len: %d\n",
+ frame_data_len);
+ goto out;
+ }
+
+ /* Copy the data */
+ memcpy(skb_put(skb, frame_data_len), frame_data, frame_data_len);
+
/* A frame is received from user space */
memset(&rx_status, 0, sizeof(rx_status));
if (info->attrs[HWSIM_ATTR_FREQ]) {
--
2.4.11
^ permalink raw reply related [flat|nested] 7+ messages in thread