NetworkManager and mac80211_hwsim

NetworkManager and mac80211_hwsim

[Daniel Wagner]

Hi

I'd like to setup a simple test scenario with mac80211_hwsim in
order to check some corner cases in the scanning behaviour
in NetworkManager. Though the problem is that NM doesn't see
the mac8011_hwsim devices. I found out that the problem is
that hald won't notifiy on dbus there is a new wireless device
available. After a bit debugging I can tell that the problem is
that there isn't a parent device available:

hal/hald/device.c:

hotplug_event_begin_add_dev (..., parent_dev = NULL, ...)
  net_add(..., parent_dev = NULL, ...)
  {
  	if (parent_dev == NULL)
		goto error;
  }

Before I go deeper into that thing, does someone has
an idea what could go wrong here with mac80211_hwsim?

thanks,
daniel

Re: NetworkManager and mac80211_hwsim

[Johannes Berg]

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

On Mon, 2009-02-16 at 17:00 +0100, Daniel Wagner wrote:

> hotplug_event_begin_add_dev (..., parent_dev = NULL, ...)
>   net_add(..., parent_dev = NULL, ...)
>   {
>   	if (parent_dev == NULL)
> 		goto error;
>   }
> 
> Before I go deeper into that thing, does someone has
> an idea what could go wrong here with mac80211_hwsim?

hwsim phys have no parents in sysfs since they're virtual.

johannes

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

Re: NetworkManager and mac80211_hwsim

[Daniel Wagner]

On Mon, Feb 16, 2009 at 05:04:44PM +0100, Johannes Berg wrote:
> On Mon, 2009-02-16 at 17:00 +0100, Daniel Wagner wrote:
> 
> > hotplug_event_begin_add_dev (..., parent_dev = NULL, ...)
> >   net_add(..., parent_dev = NULL, ...)
> >   {
> >   	if (parent_dev == NULL)
> > 		goto error;
> >   }
> > 
> > Before I go deeper into that thing, does someone has
> > an idea what could go wrong here with mac80211_hwsim?
> 
> hwsim phys have no parents in sysfs since they're virtual.

Do you propose to patch hald to handle this situation then?

daniel

Re: NetworkManager and mac80211_hwsim

[Dan Williams]

On Mon, 2009-02-16 at 18:25 +0100, Daniel Wagner wrote:
> On Mon, Feb 16, 2009 at 05:04:44PM +0100, Johannes Berg wrote:
> > On Mon, 2009-02-16 at 17:00 +0100, Daniel Wagner wrote:
> > 
> > > hotplug_event_begin_add_dev (..., parent_dev = NULL, ...)
> > >   net_add(..., parent_dev = NULL, ...)
> > >   {
> > >   	if (parent_dev == NULL)
> > > 		goto error;
> > >   }
> > > 
> > > Before I go deeper into that thing, does someone has
> > > an idea what could go wrong here with mac80211_hwsim?
> > 
> > hwsim phys have no parents in sysfs since they're virtual.
> 
> Do you propose to patch hald to handle this situation then?

The patch (for now) would have to go in hal, since obviously if HAL
can't see it, NM won't.  Until we port over to DeviceKit of course.

dan

Re: NetworkManager and mac80211_hwsim

[Daniel Wagner]

On Mon, Feb 16, 2009 at 01:00:51PM -0500, Dan Williams wrote:
> On Mon, 2009-02-16 at 18:25 +0100, Daniel Wagner wrote:
> > On Mon, Feb 16, 2009 at 05:04:44PM +0100, Johannes Berg wrote:
> > > On Mon, 2009-02-16 at 17:00 +0100, Daniel Wagner wrote:
> > > 
> > > > hotplug_event_begin_add_dev (..., parent_dev = NULL, ...)
> > > >   net_add(..., parent_dev = NULL, ...)
> > > >   {
> > > >   	if (parent_dev == NULL)
> > > > 		goto error;
> > > >   }
> > > > 
> > > > Before I go deeper into that thing, does someone has
> > > > an idea what could go wrong here with mac80211_hwsim?
> > > 
> > > hwsim phys have no parents in sysfs since they're virtual.
> > 
> > Do you propose to patch hald to handle this situation then?
> 
> The patch (for now) would have to go in hal, since obviously if HAL
> can't see it, NM won't.  Until we port over to DeviceKit of course.

Thanks for the information. After looking at HAL for a while and 
knowing that it will be replaced in the near future I decided 
not to write a proper patch. Instead I came up with a big hack. 
With this hack NM sees the mac80211_hwsim devices :)

daniel

---
 hald/linux/device.c |   23 +++++++++++++----------
 1 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/hald/linux/device.c b/hald/linux/device.c
index e48548e..699d26b 100644
--- a/hald/linux/device.c
+++ b/hald/linux/device.c
@@ -497,20 +497,22 @@ net_add (const gchar *sysfs_path, const gchar *device_file, HalDevice *parent_de
 	guint media_type;
 	gint flags;
 
-	d = NULL;
-
-	if (parent_dev == NULL)
-		goto error;
-
 	d = hal_device_new ();
 	hal_device_property_set_string (d, "linux.sysfs_path", sysfs_path);
-	hal_device_property_set_string (d, "info.parent", hal_device_get_udi (parent_dev));
+	if (parent_dev != NULL) {
+		hal_device_property_set_string (d, "info.parent", hal_device_get_udi (parent_dev));
+		hal_device_property_set_string (d, "net.originating_device", hal_device_get_udi (parent_dev));
+	} else {
+		hal_device_property_set_string (d, "info.parent", "/org/freedesktop/Hal/devices/computer");
+		hal_device_property_set_string (d, "net.originating_device", "/org/freedesktop/Hal/devices/computer");
+		hal_device_property_set_string (d, "info.linux.driver", "mac80211_hwsim");
+		parent_dev = hal_device_store_find (hald_get_gdl (), "/org/freedesktop/Hal/devices/computer");
+		hal_device_property_set_string (parent_dev, "info.linux.driver", "mac80211");
+	}
 
 	hal_device_property_set_string (d, "info.category", "net");
 	hal_device_add_capability (d, "net");
 
-	hal_device_property_set_string (d, "net.originating_device", hal_device_get_udi (parent_dev));
-
 	ifname = hal_util_get_last_element (sysfs_path);
 	hal_device_property_set_string (d, "net.interface", ifname);
 
@@ -530,7 +532,7 @@ net_add (const gchar *sysfs_path, const gchar *device_file, HalDevice *parent_de
 	media_type = hal_device_property_get_int (d, "net.arp_proto_hw_id");
 	if (media_type == ARPHRD_ETHER) {
 		const char *addr;
-		const char *parent_subsys;
+		const char *parent_subsys = 0;
 		char bridge_path[HAL_PATH_MAX];
 		char wireless_path[HAL_PATH_MAX];
 		char wiphy_path[HAL_PATH_MAX];
@@ -557,7 +559,8 @@ net_add (const gchar *sysfs_path, const gchar *device_file, HalDevice *parent_de
 		snprintf (wireless_path, HAL_PATH_MAX, "%s/wireless", sysfs_path);
 		/* wireless dscape stack e.g. from rt2500pci driver*/
 		snprintf (wiphy_path, HAL_PATH_MAX, "%s/wiphy", sysfs_path);
-		parent_subsys = hal_device_property_get_string (parent_dev, "info.subsystem");
+		if (parent_dev)
+			parent_subsys = hal_device_property_get_string (parent_dev, "info.subsystem");
 
 		if (parent_subsys && strcmp(parent_subsys, "bluetooth") == 0) {
 			hal_device_property_set_string (d, "info.product", "Bluetooth Interface");
-- 
1.6.0.2.GIT

Re: NetworkManager and mac80211_hwsim

[Johannes Berg]

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

On Wed, 2009-02-18 at 09:06 +0100, Daniel Wagner wrote:

> +	if (parent_dev != NULL) {
> +		hal_device_property_set_string (d, "info.parent", hal_device_get_udi (parent_dev));
> +		hal_device_property_set_string (d, "net.originating_device", hal_device_get_udi (parent_dev));
> +	} else {
> +		hal_device_property_set_string (d, "info.parent", "/org/freedesktop/Hal/devices/computer");
> +		hal_device_property_set_string (d, "net.originating_device", "/org/freedesktop/Hal/devices/computer");
> +		hal_device_property_set_string (d, "info.linux.driver", "mac80211_hwsim");
> +		parent_dev = hal_device_store_find (hald_get_gdl (), "/org/freedesktop/Hal/devices/computer");
> +		hal_device_property_set_string (parent_dev, "info.linux.driver", "mac80211");
> +	}

This is actually not _that_ bad an idea, because it makes sure everybody
else will properly link up their parent pointer in sysfs. An alternative
that doesn't guarantee that would be to check "is it in the ieee80211
class".

johannes

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

Re: NetworkManager and mac80211_hwsim

[Daniel Wagner]

On Wed, Feb 18, 2009 at 09:35:59AM +0100, Johannes Berg wrote:
> On Wed, 2009-02-18 at 09:06 +0100, Daniel Wagner wrote:
> 
> > +	if (parent_dev != NULL) {
> > +		hal_device_property_set_string (d, "info.parent", hal_device_get_udi (parent_dev));
> > +		hal_device_property_set_string (d, "net.originating_device", hal_device_get_udi (parent_dev));
> > +	} else {
> > +		hal_device_property_set_string (d, "info.parent", "/org/freedesktop/Hal/devices/computer");
> > +		hal_device_property_set_string (d, "net.originating_device", "/org/freedesktop/Hal/devices/computer");
> > +		hal_device_property_set_string (d, "info.linux.driver", "mac80211_hwsim");
> > +		parent_dev = hal_device_store_find (hald_get_gdl (), "/org/freedesktop/Hal/devices/computer");
> > +		hal_device_property_set_string (parent_dev, "info.linux.driver", "mac80211");
> > +	}
> 
> This is actually not _that_ bad an idea, because it makes sure everybody
> else will properly link up their parent pointer in sysfs. 

That was exactly the problem. HAL creates only a 'correct' dbus tree if the
sysfs parent pointers exists. The patch just fakes the parent 'computer'.
This 'computer' node holds all parent-less nodes (eg alsa_sequencer, alsa_timer).
One further problem is that the 'computer' node doesn't have a 'info.linux.driver'
property which NM wants to read. Setting this property to 'mac80211' is
not correct for all children of this node.

> An alternative
> that doesn't guarantee that would be to check "is it in the ieee80211
> class".

At the moment NM wants a proper parent->child construct to work. Adding something like
/org/freedesktop/Hal/devices/virtual/mac80211_hwsim as parent for the child
would propably be a solution. As I said I was not really ready for fixing
this up. But if this is something worth to fix I'll do it.

daniel

Re: NetworkManager and mac80211_hwsim

[Kevin Wilson]

Hello,

Can you please tell against which version of hal source you
made this patch ? I tried it against the hal sources in my
fedora distro and got errors when patching.
Rgs,
Kevin


On Wed, Feb 18, 2009 at 10:06 AM, Daniel Wagner <wagi@monom.org> wrote:
> On Mon, Feb 16, 2009 at 01:00:51PM -0500, Dan Williams wrote:
>> On Mon, 2009-02-16 at 18:25 +0100, Daniel Wagner wrote:
>> > On Mon, Feb 16, 2009 at 05:04:44PM +0100, Johannes Berg wrote:
>> > > On Mon, 2009-02-16 at 17:00 +0100, Daniel Wagner wrote:
>> > >
>> > > > hotplug_event_begin_add_dev (..., parent_dev = NULL, ...)
>> > > >   net_add(..., parent_dev = NULL, ...)
>> > > >   {
>> > > >         if (parent_dev == NULL)
>> > > >                 goto error;
>> > > >   }
>> > > >
>> > > > Before I go deeper into that thing, does someone has
>> > > > an idea what could go wrong here with mac80211_hwsim?
>> > >
>> > > hwsim phys have no parents in sysfs since they're virtual.
>> >
>> > Do you propose to patch hald to handle this situation then?
>>
>> The patch (for now) would have to go in hal, since obviously if HAL
>> can't see it, NM won't.  Until we port over to DeviceKit of course.
>
> Thanks for the information. After looking at HAL for a while and
> knowing that it will be replaced in the near future I decided
> not to write a proper patch. Instead I came up with a big hack.
> With this hack NM sees the mac80211_hwsim devices :)
>
> daniel
>
> ---
>  hald/linux/device.c |   23 +++++++++++++----------
>  1 files changed, 13 insertions(+), 10 deletions(-)
>
> diff --git a/hald/linux/device.c b/hald/linux/device.c
> index e48548e..699d26b 100644
> --- a/hald/linux/device.c
> +++ b/hald/linux/device.c
> @@ -497,20 +497,22 @@ net_add (const gchar *sysfs_path, const gchar *device_file, HalDevice *parent_de
>        guint media_type;
>        gint flags;
>
> -       d = NULL;
> -
> -       if (parent_dev == NULL)
> -               goto error;
> -
>        d = hal_device_new ();
>        hal_device_property_set_string (d, "linux.sysfs_path", sysfs_path);
> -       hal_device_property_set_string (d, "info.parent", hal_device_get_udi (parent_dev));
> +       if (parent_dev != NULL) {
> +               hal_device_property_set_string (d, "info.parent", hal_device_get_udi (parent_dev));
> +               hal_device_property_set_string (d, "net.originating_device", hal_device_get_udi (parent_dev));
> +       } else {
> +               hal_device_property_set_string (d, "info.parent", "/org/freedesktop/Hal/devices/computer");
> +               hal_device_property_set_string (d, "net.originating_device", "/org/freedesktop/Hal/devices/computer");
> +               hal_device_property_set_string (d, "info.linux.driver", "mac80211_hwsim");
> +               parent_dev = hal_device_store_find (hald_get_gdl (), "/org/freedesktop/Hal/devices/computer");
> +               hal_device_property_set_string (parent_dev, "info.linux.driver", "mac80211");
> +       }
>
>        hal_device_property_set_string (d, "info.category", "net");
>        hal_device_add_capability (d, "net");
>
> -       hal_device_property_set_string (d, "net.originating_device", hal_device_get_udi (parent_dev));
> -
>        ifname = hal_util_get_last_element (sysfs_path);
>        hal_device_property_set_string (d, "net.interface", ifname);
>
> @@ -530,7 +532,7 @@ net_add (const gchar *sysfs_path, const gchar *device_file, HalDevice *parent_de
>        media_type = hal_device_property_get_int (d, "net.arp_proto_hw_id");
>        if (media_type == ARPHRD_ETHER) {
>                const char *addr;
> -               const char *parent_subsys;
> +               const char *parent_subsys = 0;
>                char bridge_path[HAL_PATH_MAX];
>                char wireless_path[HAL_PATH_MAX];
>                char wiphy_path[HAL_PATH_MAX];
> @@ -557,7 +559,8 @@ net_add (const gchar *sysfs_path, const gchar *device_file, HalDevice *parent_de
>                snprintf (wireless_path, HAL_PATH_MAX, "%s/wireless", sysfs_path);
>                /* wireless dscape stack e.g. from rt2500pci driver*/
>                snprintf (wiphy_path, HAL_PATH_MAX, "%s/wiphy", sysfs_path);
> -               parent_subsys = hal_device_property_get_string (parent_dev, "info.subsystem");
> +               if (parent_dev)
> +                       parent_subsys = hal_device_property_get_string (parent_dev, "info.subsystem");
>
>                if (parent_subsys && strcmp(parent_subsys, "bluetooth") == 0) {
>                        hal_device_property_set_string (d, "info.product", "Bluetooth Interface");
> --
> 1.6.0.2.GIT
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: NetworkManager and mac80211_hwsim

[Daniel Wagner]

On Thu, Feb 19, 2009 at 04:17:06PM +0200, Kevin Wilson wrote:
> Hello,
> 
> Can you please tell against which version of hal source you
> made this patch ? I tried it against the hal sources in my
> fedora distro and got errors when patching.

Argh, I'm stupid. I created the patch against the last 0.5.11 release.
I didn't see there is a 0.5.12rc on the fly. Anyway here is a new
version against the head. I think it could be send upstream. It doesn`t
look that bad anymore. What do you think?

daniel

---

>From 14c76a6642a1ed4f93b6dfed96af6d7513102c88 Mon Sep 17 00:00:00 2001
From: Daniel Wagner <wagi@monom.org>
Date: Thu, 19 Feb 2009 18:14:09 +0100
Subject: [PATCH] Add info.linux.driver property for mac80211_hwsim devices

NetworkManager wants to read info.linux.driver for all devices.
Since a mac80211_hwsim device doesn't have a real parent the
property is not added. So let's add this property if there
is no proper parent and it is a 80211 device.

Signed-off-by: Daniel Wagner <wagi@monom.org>
---
 hald/linux/device.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/hald/linux/device.c b/hald/linux/device.c
index 21b9176..b68d3ef 100644
--- a/hald/linux/device.c
+++ b/hald/linux/device.c
@@ -1576,6 +1576,7 @@ net_add (const gchar *sysfs_path, const gchar *device_file, HalDevice *parent_de
 	guint media_type;
 	gint flags;
 	gint addr_len;
+	gboolean real_parent = TRUE;
 
 	d = NULL;
 	d = hal_device_new ();
@@ -1588,6 +1589,7 @@ net_add (const gchar *sysfs_path, const gchar *device_file, HalDevice *parent_de
 				HAL_ERROR (("Device '%s' has no parent and couldn't find computer root object."));
 				goto error;
 			}
+			real_parent = FALSE;
 		}
         }
 
@@ -1662,6 +1664,11 @@ net_add (const gchar *sysfs_path, const gchar *device_file, HalDevice *parent_de
 			hal_device_property_set_string (d, "info.category", "net.80211");
 			hal_device_add_capability (d, "net.80211");
 			hal_device_property_set_uint64 (d, "net.80211.mac_address", mac_address);
+
+			if (!real_parent) {
+				/* This is must be a mac80211_hwsim device */
+				hal_device_property_set_string (parent_dev, "info.linux.driver", "mac80211_hwsim");
+			}
 		} else if (stat (bridge_path, &s) == 0 && (s.st_mode & S_IFDIR)) {
 			hal_device_property_set_string (d, "info.product", "Bridge Interface");
 			hal_device_property_set_string (d, "info.category", "net.bridge");
-- 
1.6.0.2.GIT

Re: NetworkManager and mac80211_hwsim

[Daniel Wagner]

As it seems some more tweaking was needed to get it running
with the current HAL head. Does this work for you?

daniel

---

NetworkManager wants to read info.linux.driver for all devices.
Since a mac80211_hwsim device doesn't have a real parent the
property is not added. So let's add this property if there
is no proper parent and it is a 80211 device.

Signed-off-by: Daniel Wagner <wagi@monom.org>

---
v2: Set address for wmaster devices to nil. Otherwise the real
    networking device is overwritten.

 hald/linux/device.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/hald/linux/device.c b/hald/linux/device.c
index 21b9176..83b4a30 100644
--- a/hald/linux/device.c
+++ b/hald/linux/device.c
@@ -1576,6 +1576,7 @@ net_add (const gchar *sysfs_path, const gchar *device_file, HalDevice *parent_de
 	guint media_type;
 	gint flags;
 	gint addr_len;
+	gboolean real_parent = TRUE;
 
 	d = NULL;
 	d = hal_device_new ();
@@ -1588,6 +1589,7 @@ net_add (const gchar *sysfs_path, const gchar *device_file, HalDevice *parent_de
 				HAL_ERROR (("Device '%s' has no parent and couldn't find computer root object."));
 				goto error;
 			}
+			real_parent = FALSE;
 		}
         }
 
@@ -1662,6 +1664,11 @@ net_add (const gchar *sysfs_path, const gchar *device_file, HalDevice *parent_de
 			hal_device_property_set_string (d, "info.category", "net.80211");
 			hal_device_add_capability (d, "net.80211");
 			hal_device_property_set_uint64 (d, "net.80211.mac_address", mac_address);
+
+			if (!real_parent) {
+				/* This is must be a mac80211_hwsim device */
+				hal_device_property_set_string (parent_dev, "info.linux.driver", "mac80211_hwsim");
+			}
 		} else if (stat (bridge_path, &s) == 0 && (s.st_mode & S_IFDIR)) {
 			hal_device_property_set_string (d, "info.product", "Bridge Interface");
 			hal_device_property_set_string (d, "info.category", "net.bridge");
@@ -1691,6 +1698,7 @@ net_add (const gchar *sysfs_path, const gchar *device_file, HalDevice *parent_de
 		hal_device_property_set_string (d, "info.product", "Networking Wireless Control Interface");
 		hal_device_property_set_string (d, "info.category", "net.80211control");
 		hal_device_add_capability (d, "net.80211control");
+		hal_device_property_set_string (d, "net.address", "00:00:00:00:00:00");
 	}
 #else
 #warning ARPHRD_IEEE80211_RADIOTAP and/or ARPHRD_IEEE80211_PRISM not defined!
-- 
1.6.0.2.GIT