[RFC] Simulate medium behavior

[RFC] Simulate medium behavior

[Daniel Wagner]

Hi,

I'm still working improving the scan behavior of NM. Yes, I'm
terrible slow.  One thing which would help for this work is to
simulate the wireless medium.

Below is my humble try to get a knob for enabling/disabling 
tx/rx. The variable name 'medium' is not really good, though
I didn't come up with something better.

Writing a 0 to /debug/ieee80211/phy*/hwsim/medium enables rx/tx for
the specific phy. Writing anything but 0 disables rx/tx.  Not a very
good API. It could be made something more realistic like reception
quality is 80% associated some statistical dropping algorithm. Any
thoughts?

Is this the right direction I'm heading to?

thanks,
daniel

---
 drivers/net/wireless/mac80211_hwsim.c |   32 +++++++++++++++++++++++++++++++-
 1 files changed, 31 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c
index d4fdc8b..6373226 100644
--- a/drivers/net/wireless/mac80211_hwsim.c
+++ b/drivers/net/wireless/mac80211_hwsim.c
@@ -291,6 +291,9 @@ struct mac80211_hwsim_data {
 	bool ps_poll_pending;
 	struct dentry *debugfs;
 	struct dentry *debugfs_ps;
+
+	struct dentry *debugfs_medium;
+	unsigned int medium;
 };
 
 
@@ -399,6 +402,11 @@ static bool mac80211_hwsim_tx_frame(struct ieee80211_hw *hw,
 	rx_status.rate_idx = info->control.rates[0].idx;
 	/* TODO: simulate signal strength (and optional packet drop) */
 
+	if (data->medium != 0) {
+		/* drop packet, no ack received */
+		return false;
+	}
+
 	if (data->ps != PS_DISABLED)
 		hdr->frame_control |= cpu_to_le16(IEEE80211_FCTL_PM);
 
@@ -412,7 +420,8 @@ static bool mac80211_hwsim_tx_frame(struct ieee80211_hw *hw,
 
 		if (!data2->started || !data2->radio_enabled ||
 		    !hwsim_ps_rx_ok(data2, skb) ||
-		    data->channel->center_freq != data2->channel->center_freq)
+		    data->channel->center_freq != data2->channel->center_freq ||
+		    data2->medium != 0)
 			continue;
 
 		nskb = skb_copy(skb, GFP_ATOMIC);
@@ -725,6 +734,7 @@ static void mac80211_hwsim_free(void)
 	spin_unlock_bh(&hwsim_radio_lock);
 
 	list_for_each_entry(data, &tmplist, list) {
+		debugfs_remove(data->debugfs_medium);
 		debugfs_remove(data->debugfs_ps);
 		debugfs_remove(data->debugfs);
 		ieee80211_unregister_hw(data->hw);
@@ -876,6 +886,22 @@ static int hwsim_fops_ps_write(void *dat, u64 val)
 DEFINE_SIMPLE_ATTRIBUTE(hwsim_fops_ps, hwsim_fops_ps_read, hwsim_fops_ps_write,
 			"%llu\n");
 
+static int hwsim_fops_medium_read(void *dat, u64 *val)
+{
+	struct mac80211_hwsim_data *data = dat;
+	*val = data->medium;
+	return 0;
+}
+
+static int hwsim_fops_medium_write(void *dat, u64 val)
+{
+	struct mac80211_hwsim_data *data = dat;
+	data->medium = val;
+	return 0;
+}
+
+DEFINE_SIMPLE_ATTRIBUTE(hwsim_fops_medium, hwsim_fops_medium_read, hwsim_fops_medium_write,
+			"%llu\n");
 
 static int __init init_mac80211_hwsim(void)
 {
@@ -1106,6 +1132,10 @@ static int __init init_mac80211_hwsim(void)
 						       data->debugfs, data,
 						       &hwsim_fops_ps);
 
+		data->debugfs_medium = debugfs_create_file("medium", 0666,
+							   data->debugfs, data,
+							   &hwsim_fops_medium);
+
 		setup_timer(&data->beacon_timer, mac80211_hwsim_beacon,
 			    (unsigned long) hw);
 
-- 
1.6.0.2.GIT

Re: [RFC] Simulate medium behavior

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 562 bytes --]

Hi,

> Writing a 0 to /debug/ieee80211/phy*/hwsim/medium enables rx/tx for
> the specific phy. Writing anything but 0 disables rx/tx.  Not a very
> good API. It could be made something more realistic like reception
> quality is 80% associated some statistical dropping algorithm. Any
> thoughts?
> 
> Is this the right direction I'm heading to?

This looks like a very simple knob, just a little more work could get
you much further -- have you seen this email/thread?

http://thread.gmane.org/gmane.linux.kernel.wireless.general/25001

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [RFC] Simulate medium behavior

[Daniel Wagner]

On Wed, Apr 22, 2009 at 11:38:08AM +0200, Johannes Berg wrote:
> Hi,
> 
> > Writing a 0 to /debug/ieee80211/phy*/hwsim/medium enables rx/tx for
> > the specific phy. Writing anything but 0 disables rx/tx.  Not a very
> > good API. It could be made something more realistic like reception
> > quality is 80% associated some statistical dropping algorithm. Any
> > thoughts?
> > 
> > Is this the right direction I'm heading to?
> 
> This looks like a very simple knob, just a little more work could get
> you much further -- have you seen this email/thread?
> 
> http://thread.gmane.org/gmane.linux.kernel.wireless.general/25001

No, I haven't. Yep, my solution is really too simple.  I like the
virtual air grouping. I'll implement it.

thanks for the tip!
daniel

Re: [RFC] Simulate medium behavior

[Daniel Wagner]

On Wed, Apr 22, 2009 at 01:35:56PM +0200, Daniel Wagner wrote:
> On Wed, Apr 22, 2009 at 11:38:08AM +0200, Johannes Berg wrote:
> > Hi,
> > 
> > > Writing a 0 to /debug/ieee80211/phy*/hwsim/medium enables rx/tx for
> > > the specific phy. Writing anything but 0 disables rx/tx.  Not a very
> > > good API. It could be made something more realistic like reception
> > > quality is 80% associated some statistical dropping algorithm. Any
> > > thoughts?
> > > 
> > > Is this the right direction I'm heading to?
> > 
> > This looks like a very simple knob, just a little more work could get
> > you much further -- have you seen this email/thread?
> > 
> > http://thread.gmane.org/gmane.linux.kernel.wireless.general/25001
> 
> No, I haven't. Yep, my solution is really too simple.  I like the
> virtual air grouping. I'll implement it.

After a bit studying how to use configfs (and getting a bit lost)
I found netconsole.c which uses configfs. So this version is a copy&past
thing. Of course, this needs more work. 

Is this what do did have in mind?

How it works:

# modprobe mac80211_hwsim
# mount -t configfs none /config

Make phy0 radio member of group 1: 
# mkdir /config/mac80211_hwsim/phy0
# echo "phy0" > /config/mac80211_hwsim/phy0/dev_name
# echo 1 > /config/mac80211_hwsim/phy0/group
# echo 1 > /config/mac80211_hwsim/phy0/enable

I have only tested it very short, no problems found. Works as expected :)
 
cheers,
daniel

---
 drivers/net/wireless/Kconfig          |    9 +
 drivers/net/wireless/mac80211_hwsim.c |  324 ++++++++++++++++++++++++++++++++-
 2 files changed, 332 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/Kconfig b/drivers/net/wireless/Kconfig
index 2d8434f..5316675 100644
--- a/drivers/net/wireless/Kconfig
+++ b/drivers/net/wireless/Kconfig
@@ -481,6 +481,15 @@ config MAC80211_HWSIM
 	  To compile this driver as a module, choose M here: the module will be
 	  called mac80211_hwsim.  If unsure, say N.
 
+config MAC80211_HWSIM_VGROUP
+	bool "enable virtual air grouping"
+	depends on MAC80211_HWSIM
+	default y
+	help
+	  This option enables virtual air grouping.
+
+	  If unsure, say N.
+
 config MWL8K
 	tristate "Marvell 88W8xxx PCI/PCIe Wireless support"
 	depends on MAC80211 && PCI && WLAN_80211 && EXPERIMENTAL
diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c
index d4fdc8b..bafe451 100644
--- a/drivers/net/wireless/mac80211_hwsim.c
+++ b/drivers/net/wireless/mac80211_hwsim.c
@@ -21,6 +21,7 @@
 #include <linux/rtnetlink.h>
 #include <linux/etherdevice.h>
 #include <linux/debugfs.h>
+#include <linux/configfs.h>
 
 MODULE_AUTHOR("Jouni Malinen");
 MODULE_DESCRIPTION("Software simulator of 802.11 radio(s) for mac80211");
@@ -291,6 +292,8 @@ struct mac80211_hwsim_data {
 	bool ps_poll_pending;
 	struct dentry *debugfs;
 	struct dentry *debugfs_ps;
+
+	unsigned int group;
 };
 
 
@@ -412,7 +415,8 @@ static bool mac80211_hwsim_tx_frame(struct ieee80211_hw *hw,
 
 		if (!data2->started || !data2->radio_enabled ||
 		    !hwsim_ps_rx_ok(data2, skb) ||
-		    data->channel->center_freq != data2->channel->center_freq)
+		    data->channel->center_freq != data2->channel->center_freq ||
+		    !(data->group & data2->group))
 			continue;
 
 		nskb = skb_copy(skb, GFP_ATOMIC);
@@ -877,6 +881,320 @@ DEFINE_SIMPLE_ATTRIBUTE(hwsim_fops_ps, hwsim_fops_ps_read, hwsim_fops_ps_write,
 			"%llu\n");
 
 
+#ifdef CONFIG_MAC80211_HWSIM_VGROUP
+
+/* Linked list of all configured targets */
+static LIST_HEAD(target_list);
+
+/* XXX document the spinlock */
+static DEFINE_SPINLOCK(target_list_lock);
+
+/**
+ * struct mac80211_hwsim_target - Represents a configured mac80211_hwsim target.
+ * @list:	Links this target into the target_list.
+ * @item:	Links us into the configfs subsystem hierarchy.
+ * @enabled:	On / off knob to enable / disable target.
+ *		Visible from userspace (read-write).
+ *		Contains the other userspace visible parameters:
+ *		dev_name	(read-write)
+ *		local_port	(read-write)
+ *		group		(read-write)
+ */
+struct mac80211_hwsim_target {
+	struct list_head	list;
+	struct config_item	item;
+	int			enabled;
+	char		        dev_name[IFNAMSIZ];
+	long			group;
+};
+
+
+/*
+ * Our subsystem hierarchy is:
+ *
+ * /config/mac80211_hwsim/
+ *				|
+ *				<target>/
+ *				|	enabled
+ *				|	dev_name
+ *				|	group
+ *				|
+ *				<target>/...
+ */
+
+struct mac80211_hwsim_target_attr {
+	struct configfs_attribute	attr;
+	ssize_t				(*show)(struct mac80211_hwsim_target *nt,
+						char *buf);
+	ssize_t				(*store)(struct mac80211_hwsim_target *nt,
+						 const char *buf,
+						 size_t count);
+};
+
+static struct mac80211_hwsim_target *to_target(struct config_item *item)
+{
+	return item ?
+		container_of(item, struct mac80211_hwsim_target, item) :
+		NULL;
+}
+
+
+/*
+ * Attribute operations for mac80211_hwsim_target.
+ */
+
+static ssize_t show_enabled(struct mac80211_hwsim_target *nt, char *buf)
+{
+	return snprintf(buf, PAGE_SIZE, "%d\n", nt->enabled);
+}
+
+static ssize_t show_dev_name(struct mac80211_hwsim_target *nt, char *buf)
+{
+	return snprintf(buf, PAGE_SIZE, "%s\n", nt->dev_name);
+}
+
+static ssize_t show_group(struct mac80211_hwsim_target *nt, char *buf)
+{
+	return snprintf(buf, PAGE_SIZE, "%ld\n", nt->group);
+}
+
+
+static ssize_t store_enabled(struct mac80211_hwsim_target *nt,
+			     const char *buf,
+			     size_t count)
+{
+	char *p = (char *) buf;
+	int tmp;
+	struct mac80211_hwsim_data *data;
+
+	tmp = simple_strtoul(p, &p, 10);
+	if (!p || (*p && (*p != '\n')))
+		return -EINVAL;
+
+	if (tmp != 0 && tmp != 1)
+		return -ERANGE;
+
+	spin_lock(&hwsim_radio_lock);
+	list_for_each_entry(data, &hwsim_radios, list) {
+		if (strcmp(wiphy_name(data->hw->wiphy), nt->dev_name) == 0) {
+			data->group = nt->group;
+			break;
+		}
+	}
+	spin_unlock(&hwsim_radio_lock);
+
+	nt->enabled = tmp;
+
+	return strnlen(buf, count);
+}
+
+static ssize_t store_dev_name(struct mac80211_hwsim_target *nt,
+			      const char *buf,
+			      size_t count)
+{
+	size_t len;
+
+	if (nt->enabled) {
+		printk(KERN_ERR "mac80211_hwsim: target (%s) is enabled, "
+				"disable to update parameters\n",
+				config_item_name(&nt->item));
+		return -EINVAL;
+	}
+
+	strlcpy(nt->dev_name, buf, IFNAMSIZ);
+
+	/* Get rid of possible trailing newline from echo(1) */
+	len = strnlen(nt->dev_name, IFNAMSIZ);
+	if (nt->dev_name[len - 1] == '\n')
+		nt->dev_name[len - 1] = '\0';
+
+	return strnlen(buf, count);
+}
+
+static ssize_t store_group(struct mac80211_hwsim_target *nt,
+			   const char *buf,
+			   size_t count)
+{
+	char *p = (char *) buf;
+
+	if (nt->enabled) {
+		printk(KERN_ERR "mac80211_hwsim: target (%s) is enabled, "
+				"disable to update parameters\n",
+				config_item_name(&nt->item));
+		return -EINVAL;
+	}
+	
+	nt->group = simple_strtoul(p, &p, 16);
+
+	return strnlen(buf, count);
+}
+
+
+/*
+ * Attribute definitions for mac80211_hwsim_target.
+ */
+
+#define MAC80211_HWSIM_TARGET_ATTR_RO(_name)				\
+static struct mac80211_hwsim_target_attr mac80211_hwsim_target_##_name =	\
+	__CONFIGFS_ATTR(_name, S_IRUGO, show_##_name, NULL)
+
+#define MAC80211_HWSIM_TARGET_ATTR_RW(_name)				\
+static struct mac80211_hwsim_target_attr mac80211_hwsim_target_##_name =	\
+	__CONFIGFS_ATTR(_name, S_IRUGO | S_IWUSR, show_##_name, store_##_name)
+
+MAC80211_HWSIM_TARGET_ATTR_RW(enabled);
+MAC80211_HWSIM_TARGET_ATTR_RW(dev_name);
+MAC80211_HWSIM_TARGET_ATTR_RW(group);
+
+static struct configfs_attribute *mac80211_hwsim_target_attrs[] = {
+	&mac80211_hwsim_target_enabled.attr,
+	&mac80211_hwsim_target_dev_name.attr,
+	&mac80211_hwsim_target_group.attr,
+	NULL,
+};
+
+/*
+ * Item operations and type for mac80211_hwsim_target.
+ */
+
+static void mac80211_hwsim_target_release(struct config_item *item)
+{
+	kfree(to_target(item));
+}
+
+static ssize_t mac80211_hwsim_target_attr_show(struct config_item *item,
+					   struct configfs_attribute *attr,
+					   char *buf)
+{
+	ssize_t ret = -EINVAL;
+	struct mac80211_hwsim_target *nt = to_target(item);
+	struct mac80211_hwsim_target_attr *na =
+		container_of(attr, struct mac80211_hwsim_target_attr, attr);
+
+	if (na->show)
+		ret = na->show(nt, buf);
+
+	return ret;
+}
+
+static ssize_t mac80211_hwsim_target_attr_store(struct config_item *item,
+					    struct configfs_attribute *attr,
+					    const char *buf,
+					    size_t count)
+{
+	ssize_t ret = -EINVAL;
+	struct mac80211_hwsim_target *nt = to_target(item);
+	struct mac80211_hwsim_target_attr *na =
+		container_of(attr, struct mac80211_hwsim_target_attr, attr);
+
+	if (na->store)
+		ret = na->store(nt, buf, count);
+
+	return ret;
+}
+
+static struct configfs_item_operations mac80211_hwsim_target_item_ops = {
+	.release		= mac80211_hwsim_target_release,
+	.show_attribute		= mac80211_hwsim_target_attr_show,
+	.store_attribute	= mac80211_hwsim_target_attr_store,
+};
+
+static struct config_item_type mac80211_hwsim_target_type = {
+	.ct_attrs		= mac80211_hwsim_target_attrs,
+	.ct_item_ops		= &mac80211_hwsim_target_item_ops,
+	.ct_owner		= THIS_MODULE,
+};
+
+/*
+ * Group operations and type for mac80211_hwsim_subsys.
+ */
+
+static struct config_item *make_mac80211_hwsim_target(struct config_group *group,
+						  const char *name)
+{
+	unsigned long flags;
+	struct mac80211_hwsim_target *nt;
+
+	/*
+	 * Allocate and initialize with defaults.
+	 * Target is disabled at creation (enabled == 0).
+	 */
+	nt = kzalloc(sizeof(*nt), GFP_KERNEL);
+	if (!nt) {
+		printk(KERN_ERR "mac80211_hwsim: failed to allocate memory\n");
+		return ERR_PTR(-ENOMEM);
+	}
+
+	/* Initialize the config_item member */
+	config_item_init_type_name(&nt->item, name, &mac80211_hwsim_target_type);
+
+	/* Adding, but it is disabled */
+	spin_lock_irqsave(&target_list_lock, flags);
+	list_add(&nt->list, &target_list);
+	spin_unlock_irqrestore(&target_list_lock, flags);
+
+	return &nt->item;
+}
+
+static void drop_mac80211_hwsim_target(struct config_group *group,
+				   struct config_item *item)
+{
+	unsigned long flags;
+	struct mac80211_hwsim_target *nt = to_target(item);
+
+	spin_lock_irqsave(&target_list_lock, flags);
+	list_del(&nt->list);
+	spin_unlock_irqrestore(&target_list_lock, flags);
+
+
+	config_item_put(&nt->item);
+}
+
+static struct configfs_group_operations mac80211_hwsim_subsys_group_ops = {
+	.make_item	= make_mac80211_hwsim_target,
+	.drop_item	= drop_mac80211_hwsim_target,
+};
+
+static struct config_item_type mac80211_hwsim_subsys_type = {
+	.ct_group_ops	= &mac80211_hwsim_subsys_group_ops,
+	.ct_owner	= THIS_MODULE,
+};
+
+/* The mac80211_hwsim configfs subsystem */
+static struct configfs_subsystem mac80211_hwsim_subsys = {
+	.su_group	= {
+		.cg_item	= {
+			.ci_namebuf	= "mac80211_hwsim",
+			.ci_type	= &mac80211_hwsim_subsys_type,
+		},
+	},
+};
+
+static int __init configfs_mac80211_hwsim_init(void)
+{
+	config_group_init(&mac80211_hwsim_subsys.su_group);
+	mutex_init(&mac80211_hwsim_subsys.su_mutex);
+	return configfs_register_subsystem(&mac80211_hwsim_subsys);
+}
+
+static void __exit configfs_mac80211_hwsim_exit(void)
+{
+	configfs_unregister_subsystem(&mac80211_hwsim_subsys);
+}
+
+#else /* !CONFIg_MAC80211_HWSIM_VGROUP */
+
+static int __init configfs_mac80211_hwsim_init(void)
+{
+	return 0;
+}
+
+static void __exit configfs_mac80211_hwsim_exit(void)
+{
+}
+
+#endif /* CONFIG_MAC80211_HWSIM_VGROUP */
+
 static int __init init_mac80211_hwsim(void)
 {
 	int i, err = 0;
@@ -1122,6 +1440,9 @@ static int __init init_mac80211_hwsim(void)
 	if (err < 0)
 		goto failed_mon;
 
+	err = configfs_mac80211_hwsim_init();
+	if (err)
+		goto failed_mon;
 
 	err = register_netdevice(hwsim_mon);
 	if (err < 0)
@@ -1152,6 +1473,7 @@ static void __exit exit_mac80211_hwsim(void)
 	printk(KERN_DEBUG "mac80211_hwsim: unregister radios\n");
 
 	unregister_netdev(hwsim_mon);
+	configfs_mac80211_hwsim_exit();
 	mac80211_hwsim_free();
 }
 
-- 
1.6.2.4

Re: [RFC] Simulate medium behavior

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 1316 bytes --]

Hi Daniel,

> > > http://thread.gmane.org/gmane.linux.kernel.wireless.general/25001
> > 
> > No, I haven't. Yep, my solution is really too simple.  I like the
> > virtual air grouping. I'll implement it.
> 
> After a bit studying how to use configfs (and getting a bit lost)
> I found netconsole.c which uses configfs. So this version is a copy&past
> thing. Of course, this needs more work. 
> 
> Is this what do did have in mind?

Very cool. We had a patch at some point to add configfs support to hwsim
but it seems it got lost :(

But I don't think your configfs support is optimal since you need to
create those dirs for phy0 and put the name in etc. even when phy0
already exists. I'm sorry to have wasted your time with the configfs
suggestion, but I think with how hwsim _currently_ behaves, without
configfs support for creating new radios etc., putting this into debugfs
would be nicer, since debugfs already has a dir for this from
cfg80211/mac80211...

> +config MAC80211_HWSIM_VGROUP
> +	bool "enable virtual air grouping"
> +	depends on MAC80211_HWSIM
> +	default y
> +	help
> +	  This option enables virtual air grouping.
> +
> +	  If unsure, say N.

I also wouldn't make it configurable, just make it default to group == 1
so that it behaves as it was.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [RFC] Simulate medium behavior

[Daniel Wagner]

Hi Johannes

On Thu, May 07, 2009 at 09:02:52PM +0200, Johannes Berg wrote:
> Hi Daniel,
> 
> > > > http://thread.gmane.org/gmane.linux.kernel.wireless.general/25001
> > > 
> > > No, I haven't. Yep, my solution is really too simple.  I like the
> > > virtual air grouping. I'll implement it.
> > 
> > After a bit studying how to use configfs (and getting a bit lost)
> > I found netconsole.c which uses configfs. So this version is a copy&past
> > thing. Of course, this needs more work. 
> > 
> > Is this what do did have in mind?
> 
> Very cool. We had a patch at some point to add configfs support to hwsim
> but it seems it got lost :(
> 
> But I don't think your configfs support is optimal since you need to
> create those dirs for phy0 and put the name in etc. even when phy0
> already exists. 

Yes, that was also what I thought. If I understood it correctly that is
exactly the main difference between configfs and debugfs. In configfs 
the userspace has the owner ship of the 'objects' (mkdir, rmdir) whereas
in debugfs the kernel controls the lifetime of the 'objects'.

> I'm sorry to have wasted your time with the configfs
> suggestion, but I think with how hwsim _currently_ behaves, without
> configfs support for creating new radios etc., putting this into debugfs
> would be nicer, since debugfs already has a dir for this from
> cfg80211/mac80211...

No problem. I was able to learn something :) Ok, I'll cook up the next 
version. 

> > +config MAC80211_HWSIM_VGROUP
> > +	bool "enable virtual air grouping"
> > +	depends on MAC80211_HWSIM
> > +	default y
> > +	help
> > +	  This option enables virtual air grouping.
> > +
> > +	  If unsure, say N.
> 
> I also wouldn't make it configurable, just make it default to group == 1
> so that it behaves as it was.

Good point.

Thanks for your comments!

cheers, 
daniel

Re: [RFC] Simulate medium behavior

[Daniel Wagner]

Hi Johannes,

On Thu, May 07, 2009 at 09:02:52PM +0200, Johannes Berg wrote:
> But I don't think your configfs support is optimal since you need to
> create those dirs for phy0 and put the name in etc. even when phy0
> already exists. I'm sorry to have wasted your time with the configfs
> suggestion, but I think with how hwsim _currently_ behaves, without
> configfs support for creating new radios etc., putting this into debugfs
> would be nicer, since debugfs already has a dir for this from
> cfg80211/mac80211...

Here we go (slow as usual :)

Subject: [PATCH] mac80211_hwsim: Group radios

Each bit in /debug/ieee80211/phy*/hwsim/group represents one
group. By default all radios belong to the same group "1".
A radio can belong to several groups.

Signed-off-by: Daniel Wagner <wagi@monom.org>
---
 drivers/net/wireless/mac80211_hwsim.c |   30 +++++++++++++++++++++++++++++-
 1 files changed, 29 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c
index 61a4ad7..741f2e2 100644
--- a/drivers/net/wireless/mac80211_hwsim.c
+++ b/drivers/net/wireless/mac80211_hwsim.c
@@ -291,6 +291,9 @@ struct mac80211_hwsim_data {
 	bool ps_poll_pending;
 	struct dentry *debugfs;
 	struct dentry *debugfs_ps;
+
+	u64 group;
+	struct dentry *debugfs_group;
 };
 
 
@@ -412,7 +415,8 @@ static bool mac80211_hwsim_tx_frame(struct ieee80211_hw *hw,
 
 		if (!data2->started || !data2->radio_enabled ||
 		    !hwsim_ps_rx_ok(data2, skb) ||
-		    data->channel->center_freq != data2->channel->center_freq)
+		    data->channel->center_freq != data2->channel->center_freq ||
+		    !(data->group & data2->group))
 			continue;
 
 		nskb = skb_copy(skb, GFP_ATOMIC);
@@ -720,6 +724,7 @@ static void mac80211_hwsim_free(void)
 	spin_unlock_bh(&hwsim_radio_lock);
 
 	list_for_each_entry(data, &tmplist, list) {
+		debugfs_remove(data->debugfs_group);
 		debugfs_remove(data->debugfs_ps);
 		debugfs_remove(data->debugfs);
 		ieee80211_unregister_hw(data->hw);
@@ -872,6 +877,24 @@ DEFINE_SIMPLE_ATTRIBUTE(hwsim_fops_ps, hwsim_fops_ps_read, hwsim_fops_ps_write,
 			"%llu\n");
 
 
+static int hwsim_fops_group_read(void *dat, u64 *val)
+{
+	struct mac80211_hwsim_data *data = dat;
+	*val = data->group;
+	return 0;
+}
+
+static int hwsim_fops_group_write(void *dat, u64 val)
+{
+	struct mac80211_hwsim_data *data = dat;
+	data->group = val;
+	return 0;
+}
+
+DEFINE_SIMPLE_ATTRIBUTE(hwsim_fops_group,
+			hwsim_fops_group_read, hwsim_fops_group_write,
+			"%llu\n");
+
 static int __init init_mac80211_hwsim(void)
 {
 	int i, err = 0;
@@ -976,6 +999,8 @@ static int __init init_mac80211_hwsim(void)
 
 			hw->wiphy->bands[band] = sband;
 		}
+		/* By default all radios are belonging to the first group */
+		data->group = 1;
 
 		/* Work to be done prior to ieee80211_register_hw() */
 		switch (regtest) {
@@ -1100,6 +1125,9 @@ static int __init init_mac80211_hwsim(void)
 		data->debugfs_ps = debugfs_create_file("ps", 0666,
 						       data->debugfs, data,
 						       &hwsim_fops_ps);
+		data->debugfs_group = debugfs_create_file("group", 0666,
+							data->debugfs, data,
+							&hwsim_fops_group);
 
 		setup_timer(&data->beacon_timer, mac80211_hwsim_beacon,
 			    (unsigned long) hw);
-- 
1.6.2.4

Re: [RFC] Simulate medium behavior

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 1204 bytes --]

On Thu, 2009-05-14 at 20:45 +0200, Daniel Wagner wrote:

Looks good to me, couple of comments below.

> +static int hwsim_fops_group_read(void *dat, u64 *val)
> +{
> +	struct mac80211_hwsim_data *data = dat;
> +	*val = data->group;
> +	return 0;
> +}
> +
> +static int hwsim_fops_group_write(void *dat, u64 val)
> +{
> +	struct mac80211_hwsim_data *data = dat;
> +	data->group = val;
> +	return 0;
> +}
> +
> +DEFINE_SIMPLE_ATTRIBUTE(hwsim_fops_group,
> +			hwsim_fops_group_read, hwsim_fops_group_write,
> +			"%llu\n");

I think %llx would work better for this? At least I can do bitops better
in hex :)

>  		data->debugfs_ps = debugfs_create_file("ps", 0666,
>  						       data->debugfs, data,
>  						       &hwsim_fops_ps);
> +		data->debugfs_group = debugfs_create_file("group", 0666,
> +							data->debugfs, data,
> +							&hwsim_fops_group);

Good thing this is a test module only... debugfs files with user access
permissions are a security risk. (a user can open the file and leave the
fd open, if root then rmmods future reads/writes to the fd by the user
crash the kernel)

Anyway looks good, please submit properly with [PATCH] etc :)

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [RFC] Simulate medium behavior

[Daniel Wagner]

On Thu, May 14, 2009 at 09:12:49PM +0200, Johannes Berg wrote:
> On Thu, 2009-05-14 at 20:45 +0200, Daniel Wagner wrote:
> 
> Looks good to me, couple of comments below.
> 
> > +static int hwsim_fops_group_read(void *dat, u64 *val)
> > +{
> > +	struct mac80211_hwsim_data *data = dat;
> > +	*val = data->group;
> > +	return 0;
> > +}
> > +
> > +static int hwsim_fops_group_write(void *dat, u64 val)
> > +{
> > +	struct mac80211_hwsim_data *data = dat;
> > +	data->group = val;
> > +	return 0;
> > +}
> > +
> > +DEFINE_SIMPLE_ATTRIBUTE(hwsim_fops_group,
> > +			hwsim_fops_group_read, hwsim_fops_group_write,
> > +			"%llu\n");
> 
> I think %llx would work better for this? At least I can do bitops better
> in hex :)

Yeah, I have to improve my copy&paste-foo :)

> 
> >  		data->debugfs_ps = debugfs_create_file("ps", 0666,
> >  						       data->debugfs, data,
> >  						       &hwsim_fops_ps);
> > +		data->debugfs_group = debugfs_create_file("group", 0666,
> > +							data->debugfs, data,
> > +							&hwsim_fops_group);
> 
> Good thing this is a test module only... debugfs files with user access
> permissions are a security risk. (a user can open the file and leave the
> fd open, if root then rmmods future reads/writes to the fd by the user
> crash the kernel)

There isn't much we can do about that, right? 

> Anyway looks good, please submit properly with [PATCH] etc :)

Done. 

Thanks for reviewing!

cheers,
daniel

Re: [RFC] Simulate medium behavior

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 823 bytes --]

On Thu, 2009-05-14 at 22:50 +0200, Daniel Wagner wrote:

> > >  		data->debugfs_ps = debugfs_create_file("ps", 0666,
> > >  						       data->debugfs, data,
> > >  						       &hwsim_fops_ps);
> > > +		data->debugfs_group = debugfs_create_file("group", 0666,
> > > +							data->debugfs, data,
> > > +							&hwsim_fops_group);
> > 
> > Good thing this is a test module only... debugfs files with user access
> > permissions are a security risk. (a user can open the file and leave the
> > fd open, if root then rmmods future reads/writes to the fd by the user
> > crash the kernel)
> 
> There isn't much we can do about that, right? 

I generally just use 0600 to not allow regular users to open the files.
Maybe at some point, with the revoke() discussion, we'll actually get a
solution.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]