[PATCH 1/4] mac80211-hwsim: notify user-space about channel change.

[PATCH 1/4] mac80211-hwsim: notify user-space about channel change.

[greearb]

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>
---
 drivers/net/wireless/mac80211_hwsim.c | 48 +++++++++++++++++++++++++++++++++++
 drivers/net/wireless/mac80211_hwsim.h |  6 +++++
 2 files changed, 54 insertions(+)

diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c
index 8908be6..d0e88b2 100644
--- a/drivers/net/wireless/mac80211_hwsim.c
+++ b/drivers/net/wireless/mac80211_hwsim.c
@@ -871,6 +871,52 @@ static bool hwsim_ps_rx_ok(struct mac80211_hwsim_data *data,
 	return true;
 }
 
+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 = ACCESS_ONCE(wmediumd_portid);
+
+	if (!_portid)
+		return;
+
+	skb = genlmsg_new(GENLMSG_DEFAULT_SIZE, GFP_ATOMIC);
+	if (skb == NULL)
+		goto err_print;
+
+	msg_head = genlmsg_put(skb, 0, 0, &hwsim_genl_family, 0,
+			       HWSIM_CMD_NOTIFY);
+	if (msg_head == NULL) {
+		printk(KERN_DEBUG "mac80211_hwsim: problem with msg_head, notify\n");
+		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;
+
+	genlmsg_end(skb, msg_head);
+	if (genlmsg_unicast(&init_net, skb, _portid))
+		goto err_print;
+
+	return;
+
+nla_put_failure:
+	nlmsg_free(skb);
+err_print:
+	printk(KERN_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)
@@ -1465,6 +1511,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 66e1c73..f0fc495c 100644
--- a/drivers/net/wireless/mac80211_hwsim.h
+++ b/drivers/net/wireless/mac80211_hwsim.h
@@ -71,6 +71,11 @@ 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: notify user-space about driver changes.  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_ADDR_TRANSMITTER # ID which radio we are notifying about.
  * @__HWSIM_CMD_MAX: enum limit
  */
 enum {
@@ -81,6 +86,7 @@ enum {
 	HWSIM_CMD_NEW_RADIO,
 	HWSIM_CMD_DEL_RADIO,
 	HWSIM_CMD_GET_RADIO,
+	HWSIM_CMD_NOTIFY,
 	__HWSIM_CMD_MAX,
 };
 #define HWSIM_CMD_MAX (_HWSIM_CMD_MAX - 1)
-- 
1.7.11.7

[PATCH 2/4] mac80211-hwsim: remove dmesg spam about get-survey.

[greearb]

From: Ben Greear <greearb@candelatech.com>

This message just fills up dmesg and/or kernel logs and does
not provide any useful information.

Signed-off-by: Ben Greear <greearb@candelatech.com>
---
 drivers/net/wireless/mac80211_hwsim.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c
index d0e88b2..5089169 100644
--- a/drivers/net/wireless/mac80211_hwsim.c
+++ b/drivers/net/wireless/mac80211_hwsim.c
@@ -1696,7 +1696,7 @@ static int mac80211_hwsim_get_survey(
 {
 	struct ieee80211_conf *conf = &hw->conf;
 
-	wiphy_debug(hw->wiphy, "%s (idx=%d)\n", __func__, idx);
+	/* wiphy_debug(hw->wiphy, "%s (idx=%d)\n", __func__, idx); */
 
 	if (idx != 0)
 		return -ENOENT;
-- 
1.7.11.7

[PATCH 3/4] mac80211-hwsim: Pass rate-ctrl flags and tx-power to user-space

[greearb]

From: Ben Greear <greearb@candelatech.com>

The flags field allows user-space to properly decode what the
rate->idx really means.  The tx-power field is useful as well,
in case user-space is interested in doing something clever with
path-loss calculations.

Signed-off-by: Ben Greear <greearb@candelatech.com>
---
 drivers/net/wireless/mac80211_hwsim.c | 21 ++++++++++++++++++++-
 drivers/net/wireless/mac80211_hwsim.h |  8 ++++++++
 2 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c
index 5089169..afb2139 100644
--- a/drivers/net/wireless/mac80211_hwsim.c
+++ b/drivers/net/wireless/mac80211_hwsim.c
@@ -521,6 +521,9 @@ static const struct nla_policy hwsim_genl_policy[HWSIM_ATTR_MAX + 1] = {
 	[HWSIM_ATTR_RADIO_NAME] = { .type = NLA_STRING },
 	[HWSIM_ATTR_NO_VIF] = { .type = NLA_FLAG },
 	[HWSIM_ATTR_FREQ] = { .type = NLA_U32 },
+	[HWSIM_ATTR_TX_INFO2] = { .type = NLA_UNSPEC,
+				  .len = IEEE80211_TX_MAX_RATES *
+				         sizeof(struct hwsim_tx_rate2)},
 };
 
 static void mac80211_hwsim_tx_frame(struct ieee80211_hw *hw,
@@ -929,6 +932,7 @@ static void mac80211_hwsim_tx_frame_nl(struct ieee80211_hw *hw,
 	unsigned int hwsim_flags = 0;
 	int i;
 	struct hwsim_tx_rate tx_attempts[IEEE80211_TX_MAX_RATES];
+	struct hwsim_tx_rate2 tx_attempts2[IEEE80211_TX_MAX_RATES];
 
 	if (data->ps != PS_DISABLED)
 		hdr->frame_control |= cpu_to_le16(IEEE80211_FCTL_PM);
@@ -980,6 +984,8 @@ static void mac80211_hwsim_tx_frame_nl(struct ieee80211_hw *hw,
 	for (i = 0; i < IEEE80211_TX_MAX_RATES; i++) {
 		tx_attempts[i].idx = info->status.rates[i].idx;
 		tx_attempts[i].count = info->status.rates[i].count;
+		tx_attempts2[i].rc_flags = info->status.rates[i].flags;
+		tx_attempts2[i].power_level = data->power_level;
 	}
 
 	if (nla_put(skb, HWSIM_ATTR_TX_INFO,
@@ -987,6 +993,11 @@ static void mac80211_hwsim_tx_frame_nl(struct ieee80211_hw *hw,
 		    tx_attempts))
 		goto nla_put_failure;
 
+	if (nla_put(skb, HWSIM_ATTR_TX_INFO2,
+		    sizeof(struct hwsim_tx_rate2)*IEEE80211_TX_MAX_RATES,
+		    tx_attempts2))
+		goto nla_put_failure;
+
 	/* We create a cookie to identify this skb */
 	if (nla_put_u64(skb, HWSIM_ATTR_COOKIE, (unsigned long) my_skb))
 		goto nla_put_failure;
@@ -2680,6 +2691,7 @@ static int hwsim_tx_info_frame_received_nl(struct sk_buff *skb_2,
 	struct mac80211_hwsim_data *data2;
 	struct ieee80211_tx_info *txi;
 	struct hwsim_tx_rate *tx_attempts;
+	struct hwsim_tx_rate2 *tx_attempts2;
 	unsigned long ret_skb_ptr;
 	struct sk_buff *skb, *tmp;
 	const u8 *src;
@@ -2723,6 +2735,12 @@ static int hwsim_tx_info_frame_received_nl(struct sk_buff *skb_2,
 	tx_attempts = (struct hwsim_tx_rate *)nla_data(
 		       info->attrs[HWSIM_ATTR_TX_INFO]);
 
+	if (info->attrs[HWSIM_ATTR_TX_INFO2])
+		tx_attempts2 = (struct hwsim_tx_rate2 *)nla_data(
+		       info->attrs[HWSIM_ATTR_TX_INFO2]);
+	else
+		tx_attempts2 = NULL;
+
 	/* now send back TX status */
 	txi = IEEE80211_SKB_CB(skb);
 
@@ -2731,7 +2749,8 @@ static int hwsim_tx_info_frame_received_nl(struct sk_buff *skb_2,
 	for (i = 0; i < IEEE80211_TX_MAX_RATES; i++) {
 		txi->status.rates[i].idx = tx_attempts[i].idx;
 		txi->status.rates[i].count = tx_attempts[i].count;
-		/*txi->status.rates[i].flags = 0;*/
+		if (tx_attempts2)
+			txi->status.rates[i].flags = tx_attempts2[i].rc_flags;
 	}
 
 	txi->status.ack_signal = nla_get_u32(info->attrs[HWSIM_ATTR_SIGNAL]);
diff --git a/drivers/net/wireless/mac80211_hwsim.h b/drivers/net/wireless/mac80211_hwsim.h
index f0fc495c..a2e2e11 100644
--- a/drivers/net/wireless/mac80211_hwsim.h
+++ b/drivers/net/wireless/mac80211_hwsim.h
@@ -129,6 +129,7 @@ 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_TX_INFO2: hwsim_tx_rate2 array
  * @__HWSIM_ATTR_MAX: enum limit
  */
 
@@ -154,6 +155,7 @@ enum {
 	HWSIM_ATTR_RADIO_NAME,
 	HWSIM_ATTR_NO_VIF,
 	HWSIM_ATTR_FREQ,
+	HWSIM_ATTR_TX_INFO2,
 	__HWSIM_ATTR_MAX,
 };
 #define HWSIM_ATTR_MAX (__HWSIM_ATTR_MAX - 1)
@@ -176,4 +178,10 @@ struct hwsim_tx_rate {
 	u8 count;
 } __packed;
 
+/* Auxilary info to allow user-space to better understand the rate */
+struct hwsim_tx_rate2 {
+	u16 rc_flags; /* rate-ctrl flags (see mac80211_rate_control_flags) */
+	s16 power_level;
+} __packed;
+
 #endif /* __MAC80211_HWSIM_H */
-- 
1.7.11.7

[PATCH 4/4] mac80211-hwsim: enable better rx-status when using netlink.

[greearb]

From: Ben Greear <greearb@candelatech.com>

This allows proper rx-status reporting for packets received
from the netlink api.

Signed-off-by: Ben Greear <greearb@candelatech.com>
---
 drivers/net/wireless/mac80211_hwsim.c | 10 ++++++++++
 drivers/net/wireless/mac80211_hwsim.h | 18 ++++++++++++++++++
 2 files changed, 28 insertions(+)

diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c
index afb2139..67c604b 100644
--- a/drivers/net/wireless/mac80211_hwsim.c
+++ b/drivers/net/wireless/mac80211_hwsim.c
@@ -2824,6 +2824,16 @@ static int hwsim_cloned_frame_received_nl(struct sk_buff *skb_2,
 	rx_status.rate_idx = nla_get_u32(info->attrs[HWSIM_ATTR_RX_RATE]);
 	rx_status.signal = nla_get_u32(info->attrs[HWSIM_ATTR_SIGNAL]);
 
+	if (info->attrs[HWSIM_ATTR_RX_INFO]) {
+		struct hwsim_rx_info *r;
+		r = (struct hwsim_rx_info *)nla_data(
+			info->attrs[HWSIM_ATTR_RX_INFO]);
+		rx_status.flag = r->rx_flags;
+		rx_status.vht_flag = r->vht_flags;
+		rx_status.vht_nss = r->vht_nss;
+		rx_status.ampdu_reference = r->ampdu_reference;
+	}
+
 	memcpy(IEEE80211_SKB_RXCB(skb), &rx_status, sizeof(rx_status));
 	data2->rx_pkts++;
 	data2->rx_bytes += skb->len;
diff --git a/drivers/net/wireless/mac80211_hwsim.h b/drivers/net/wireless/mac80211_hwsim.h
index a2e2e11..0e26c9f 100644
--- a/drivers/net/wireless/mac80211_hwsim.h
+++ b/drivers/net/wireless/mac80211_hwsim.h
@@ -130,6 +130,7 @@ enum {
  * @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_TX_INFO2: hwsim_tx_rate2 array
+ * @HWSIM_ATTR_RX_INFO: hwsim_rx_info
  * @__HWSIM_ATTR_MAX: enum limit
  */
 
@@ -156,6 +157,7 @@ enum {
 	HWSIM_ATTR_NO_VIF,
 	HWSIM_ATTR_FREQ,
 	HWSIM_ATTR_TX_INFO2,
+	HWSIM_ATTR_RX_INFO,
 	__HWSIM_ATTR_MAX,
 };
 #define HWSIM_ATTR_MAX (__HWSIM_ATTR_MAX - 1)
@@ -184,4 +186,20 @@ struct hwsim_tx_rate2 {
 	s16 power_level;
 } __packed;
 
+/**
+ * This relates to the ieee80211_rx_status struct in mac80211.h
+ * @rx_flags: %RX_FLAG_* (see  mac80211_rx_flags)
+ * @vht_flags: %RX_VHT_FLAG_*
+ * @vht_nss: number of streams (VHT only)
+ * @ampdu_reference: A-MPDU reference number, must be a different value for
+ *	each A-MPDU but the same for each subframe within one A-MPDU
+ */
+struct hwsim_rx_info {
+	u32 rx_flags;
+	u8 vht_flags;
+	u8 vht_nss;
+	u16 unused_pad; /* pad to 32-bits, and space for growth */
+	u32 ampdu_reference;
+} __packed;
+
 #endif /* __MAC80211_HWSIM_H */
-- 
1.7.11.7

Re: [PATCH 1/4] mac80211-hwsim: notify user-space about channel change.

[Sergei Shtylyov]

Hello.

On 4/4/2015 12:12 AM, greearb@candelatech.com wrote:

> 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>
> ---
>   drivers/net/wireless/mac80211_hwsim.c | 48 +++++++++++++++++++++++++++++++++++
>   drivers/net/wireless/mac80211_hwsim.h |  6 +++++
>   2 files changed, 54 insertions(+)

> diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c
> index 8908be6..d0e88b2 100644
> --- a/drivers/net/wireless/mac80211_hwsim.c
> +++ b/drivers/net/wireless/mac80211_hwsim.c
> @@ -871,6 +871,52 @@ static bool hwsim_ps_rx_ok(struct mac80211_hwsim_data *data,
>   	return true;
>   }
>
> +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 = ACCESS_ONCE(wmediumd_portid);
> +

    I don't think empty line is needed here...

> +	if (!_portid)
> +		return;
> +
> +	skb = genlmsg_new(GENLMSG_DEFAULT_SIZE, GFP_ATOMIC);
> +	if (skb == NULL)

    Hm, have you run this thru scripts/checkpatch.pl? It should have 
recommended using '!skb' instead (which would have been only consistent with 
your previous code).

> +		goto err_print;
> +
> +	msg_head = genlmsg_put(skb, 0, 0, &hwsim_genl_family, 0,
> +			       HWSIM_CMD_NOTIFY);
> +	if (msg_head == NULL) {

    Same here.

[...]

WBR, Sergei

Re: [PATCH 1/4] mac80211-hwsim: notify user-space about channel change.

[Johannes Berg]

> +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 = ACCESS_ONCE(wmediumd_portid);
> +
> +	if (!_portid)
> +		return;
> +
> +	skb = genlmsg_new(GENLMSG_DEFAULT_SIZE, GFP_ATOMIC);
> +	if (skb == NULL)
> +		goto err_print;
> +
> +	msg_head = genlmsg_put(skb, 0, 0, &hwsim_genl_family, 0,
> +			       HWSIM_CMD_NOTIFY);
> +	if (msg_head == NULL) {
> +		printk(KERN_DEBUG "mac80211_hwsim: problem with msg_head, notify\n");

None of this can really happen, so I don't think you should print
anything here. It's just a waste of space in all ways I think.

> +	if (nla_put(skb, HWSIM_ATTR_ADDR_TRANSMITTER,
> +		    ETH_ALEN, data->addresses[1].addr))

That doesn't seem right, Bob just fixed the code to not do this any
more.

> +	if (nla_put_u32(skb, HWSIM_ATTR_FREQ, center_freq))
> +		goto nla_put_failure;

You shouldn't unconditionally put this attribute but just leave it out
when the channel isn't known. However, see below.

> +nla_put_failure:
> +	nlmsg_free(skb);
> +err_print:
> +	printk(KERN_DEBUG "mac80211_hwsim: error occurred in %s\n", __func__);

ditto.

> @@ -1465,6 +1511,8 @@ static int mac80211_hwsim_config(struct ieee80211_hw *hw, u32 changed)
>  				      HRTIMER_MODE_REL);
>  	}
>  
> +	mac80211_hwsim_check_nl_notify(data);

Still this bothers me a bit, if you enable multi-channel in hwsim, you
don't actually get a data->channel, which renders this functionality
useless, you might never get this message.

Wouldn't it be better to have an API to wmediumd and similar userspace
that didn't break as soon as you enable multi-channel?

johannes

Re: [PATCH 2/4] mac80211-hwsim: remove dmesg spam about get-survey.

[Johannes Berg]

On Fri, 2015-04-03 at 14:12 -0700, greearb@candelatech.com wrote:
> From: Ben Greear <greearb@candelatech.com>
> 
> This message just fills up dmesg and/or kernel logs and does
> not provide any useful information.

I'm fine with removing it but don't see much point in commenting it
out...

johannes

Re: [PATCH 3/4] mac80211-hwsim: Pass rate-ctrl flags and tx-power to user-space

[Johannes Berg]

On Fri, 2015-04-03 at 14:12 -0700, greearb@candelatech.com wrote:

> +/* Auxilary info to allow user-space to better understand the rate */
> +struct hwsim_tx_rate2 {
> +	u16 rc_flags; /* rate-ctrl flags (see mac80211_rate_control_flags) */

I really don't like this - it ties internal API to userspace API. You
may argue that this is userspace that is for debug purposes only, but
I'm sure you'll also scratch your head very confused when I change the
rate control flags for any reason and your code breaks :)

johannes

Re: [PATCH 4/4] mac80211-hwsim: enable better rx-status when using netlink.

[Johannes Berg]

On Fri, 2015-04-03 at 14:12 -0700, greearb@candelatech.com wrote:

> +/**
> + * This relates to the ieee80211_rx_status struct in mac80211.h
> + * @rx_flags: %RX_FLAG_* (see  mac80211_rx_flags)
> + * @vht_flags: %RX_VHT_FLAG_*

Same here.

johannes

Re: [PATCH 3/4] mac80211-hwsim: Pass rate-ctrl flags and tx-power to user-space

[Ben Greear]

On 04/14/2015 01:15 AM, Johannes Berg wrote:
> On Fri, 2015-04-03 at 14:12 -0700, greearb@candelatech.com wrote:
> 
>> +/* Auxilary info to allow user-space to better understand the rate */
>> +struct hwsim_tx_rate2 {
>> +	u16 rc_flags; /* rate-ctrl flags (see mac80211_rate_control_flags) */
> 
> I really don't like this - it ties internal API to userspace API. You
> may argue that this is userspace that is for debug purposes only, but
> I'm sure you'll also scratch your head very confused when I change the
> rate control flags for any reason and your code breaks :)

Is this a documentation only issue (at this point, while code matches)?

Then later, we can add translation to keep user-space API the same
as needed?

And in that case, maybe I'll make it u32 to give room down the road?

Thanks,
Ben


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: [PATCH 3/4] mac80211-hwsim: Pass rate-ctrl flags and tx-power to user-space

[Johannes Berg]

On Wed, 2015-04-15 at 09:23 -0700, Ben Greear wrote:
> On 04/14/2015 01:15 AM, Johannes Berg wrote:
> > On Fri, 2015-04-03 at 14:12 -0700, greearb@candelatech.com wrote:
> > 
> >> +/* Auxilary info to allow user-space to better understand the rate */
> >> +struct hwsim_tx_rate2 {
> >> +	u16 rc_flags; /* rate-ctrl flags (see mac80211_rate_control_flags) */
> > 
> > I really don't like this - it ties internal API to userspace API. You
> > may argue that this is userspace that is for debug purposes only, but
> > I'm sure you'll also scratch your head very confused when I change the
> > rate control flags for any reason and your code breaks :)
> 
> Is this a documentation only issue (at this point, while code matches)?

Well, technically this could be done.

> Then later, we can add translation to keep user-space API the same
> as needed?

We'll almost certainly forget that though, so better to have a
translation layer now.

OTOH though, perhaps we don't even want to advertise all these flags. We
might want to change the details of how these work, so I think it'd be
good to actually analyse them and see which ones really are needed. That
really applies more to the rx-status flags then here perhaps.

> And in that case, maybe I'll make it u32 to give room down the road?

That makes sense - netlink attributes are even 4-byte aligned so there's
no space saving with u16 either :)

johannes