[PATCH] Revert "wireless: Support of IFLA_INFO_KIND rtnl attribute"

[PATCH] Revert "wireless: Support of IFLA_INFO_KIND rtnl attribute"

[Johannes Berg]

From: Johannes Berg <johannes.berg@intel.com>

This reverts commit ba1debdfed974f25aa598c283567878657b292ee.

Oliver reported that it breaks network-manager, for some reason with
this patch NM decides that the device isn't wireless but "generic"
(ethernet), sees no carrier (as expected with wifi) and fails to do
anything else with it.

Revert this to unbreak userspace.

Reported-by: Oliver Hartkopp <socketcan@hartkopp.net>
Tested-by: Oliver Hartkopp <socketcan@hartkopp.net>
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/wireless/core.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/net/wireless/core.c b/net/wireless/core.c
index 456e4c38c279..3af0ecf1cc16 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -21,7 +21,6 @@
 #include <linux/sched.h>
 #include <net/genetlink.h>
 #include <net/cfg80211.h>
-#include <net/rtnetlink.h>
 #include "nl80211.h"
 #include "core.h"
 #include "sysfs.h"
@@ -964,10 +963,6 @@ void cfg80211_stop_iface(struct wiphy *wiphy, struct wireless_dev *wdev,
 }
 EXPORT_SYMBOL(cfg80211_stop_iface);
 
-static const struct rtnl_link_ops wireless_link_ops = {
-	.kind = "wlan",
-};
-
 static int cfg80211_netdev_notifier_call(struct notifier_block *nb,
 					 unsigned long state, void *ptr)
 {
@@ -986,7 +981,6 @@ static int cfg80211_netdev_notifier_call(struct notifier_block *nb,
 	switch (state) {
 	case NETDEV_POST_INIT:
 		SET_NETDEV_DEVTYPE(dev, &wiphy_type);
-		dev->rtnl_link_ops = &wireless_link_ops;
 		break;
 	case NETDEV_REGISTER:
 		/*
-- 
2.1.4

Re: [PATCH] Revert "wireless: Support of IFLA_INFO_KIND rtnl attribute"

[Johannes Berg]

On Mon, 2015-01-19 at 18:52 +0100, Johannes Berg wrote:
> From: Johannes Berg <johannes.berg@intel.com>
> 
> This reverts commit ba1debdfed974f25aa598c283567878657b292ee.
> 
> Oliver reported that it breaks network-manager, for some reason with
> this patch NM decides that the device isn't wireless but "generic"
> (ethernet), sees no carrier (as expected with wifi) and fails to do
> anything else with it.
> 
> Revert this to unbreak userspace.

git-email didn't Cc Vadim or Marcel ...

Looks like we can't do this since it will break userspace - please don't
write any tools depending on it :)

johannes

Re: [PATCH] Revert "wireless: Support of IFLA_INFO_KIND rtnl attribute"

[Vadim Kochan]

On Mon, Jan 19, 2015 at 07:02:52PM +0100, Johannes Berg wrote:
> On Mon, 2015-01-19 at 18:52 +0100, Johannes Berg wrote:
> > From: Johannes Berg <johannes.berg@intel.com>
> > 
> > This reverts commit ba1debdfed974f25aa598c283567878657b292ee.
> > 
> > Oliver reported that it breaks network-manager, for some reason with
> > this patch NM decides that the device isn't wireless but "generic"
> > (ethernet), sees no carrier (as expected with wifi) and fails to do
> > anything else with it.
> > 
> > Revert this to unbreak userspace.
> 
> git-email didn't Cc Vadim or Marcel ...
> 
> Looks like we can't do this since it will break userspace - please don't
> write any tools depending on it :)
> 
> johannes
> 
I am really surprised, will dig into NM ...

Re: [PATCH] Revert "wireless: Support of IFLA_INFO_KIND rtnl attribute"

[Dan Williams]

On Mon, 2015-01-19 at 18:52 +0100, Johannes Berg wrote:
> From: Johannes Berg <johannes.berg@intel.com>
> 
> This reverts commit ba1debdfed974f25aa598c283567878657b292ee.
> 
> Oliver reported that it breaks network-manager, for some reason with
> this patch NM decides that the device isn't wireless but "generic"
> (ethernet), sees no carrier (as expected with wifi) and fails to do
> anything else with it.

Hmm, which NM version?  NM uses either DEVTYPE (from 'uevent' in sysfs),
the phy80211 link in sysfs, or (if enabled) WEXT to figure out whether a
network interface with arptype=1 is WiFi or not.  I can't think of why
IFLA_INFO_KIND would break that...  what are the userspace visible
effects of the patch before reversion?

Dan

> Revert this to unbreak userspace.
> 
> Reported-by: Oliver Hartkopp <socketcan@hartkopp.net>
> Tested-by: Oliver Hartkopp <socketcan@hartkopp.net>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
> ---
>  net/wireless/core.c | 6 ------
>  1 file changed, 6 deletions(-)
> 
> diff --git a/net/wireless/core.c b/net/wireless/core.c
> index 456e4c38c279..3af0ecf1cc16 100644
> --- a/net/wireless/core.c
> +++ b/net/wireless/core.c
> @@ -21,7 +21,6 @@
>  #include <linux/sched.h>
>  #include <net/genetlink.h>
>  #include <net/cfg80211.h>
> -#include <net/rtnetlink.h>
>  #include "nl80211.h"
>  #include "core.h"
>  #include "sysfs.h"
> @@ -964,10 +963,6 @@ void cfg80211_stop_iface(struct wiphy *wiphy, struct wireless_dev *wdev,
>  }
>  EXPORT_SYMBOL(cfg80211_stop_iface);
>  
> -static const struct rtnl_link_ops wireless_link_ops = {
> -	.kind = "wlan",
> -};
> -
>  static int cfg80211_netdev_notifier_call(struct notifier_block *nb,
>  					 unsigned long state, void *ptr)
>  {
> @@ -986,7 +981,6 @@ static int cfg80211_netdev_notifier_call(struct notifier_block *nb,
>  	switch (state) {
>  	case NETDEV_POST_INIT:
>  		SET_NETDEV_DEVTYPE(dev, &wiphy_type);
> -		dev->rtnl_link_ops = &wireless_link_ops;
>  		break;
>  	case NETDEV_REGISTER:
>  		/*

Re: [PATCH] Revert "wireless: Support of IFLA_INFO_KIND rtnl attribute"

[Dan Williams]

On Mon, 2015-01-19 at 13:22 -0600, Dan Williams wrote:
> On Mon, 2015-01-19 at 18:52 +0100, Johannes Berg wrote:
> > From: Johannes Berg <johannes.berg@intel.com>
> > 
> > This reverts commit ba1debdfed974f25aa598c283567878657b292ee.
> > 
> > Oliver reported that it breaks network-manager, for some reason with
> > this patch NM decides that the device isn't wireless but "generic"
> > (ethernet), sees no carrier (as expected with wifi) and fails to do
> > anything else with it.
> 
> Hmm, which NM version?  NM uses either DEVTYPE (from 'uevent' in sysfs),
> the phy80211 link in sysfs, or (if enabled) WEXT to figure out whether a
> network interface with arptype=1 is WiFi or not.  I can't think of why
> IFLA_INFO_KIND would break that...  what are the userspace visible
> effects of the patch before reversion?

So with the patch, libnl will return 'wlan' from rtnl_link_get_type().
NetworkManager was relying on the fact that this would return NULL for
WiFi devices and was not falling back to alternate WiFi detection
mechanisms (sysfs, DEVTYPE, etc) when an unknown IFLA_INFO_KIND was
found.

NM 0.9.10+ have this bug.

I'll submit a patch upstream for review, which will get backported to
0.9.10 and later when approved.

Dan

> Dan
> 
> > Revert this to unbreak userspace.
> > 
> > Reported-by: Oliver Hartkopp <socketcan@hartkopp.net>
> > Tested-by: Oliver Hartkopp <socketcan@hartkopp.net>
> > Signed-off-by: Johannes Berg <johannes.berg@intel.com>
> > ---
> >  net/wireless/core.c | 6 ------
> >  1 file changed, 6 deletions(-)
> > 
> > diff --git a/net/wireless/core.c b/net/wireless/core.c
> > index 456e4c38c279..3af0ecf1cc16 100644
> > --- a/net/wireless/core.c
> > +++ b/net/wireless/core.c
> > @@ -21,7 +21,6 @@
> >  #include <linux/sched.h>
> >  #include <net/genetlink.h>
> >  #include <net/cfg80211.h>
> > -#include <net/rtnetlink.h>
> >  #include "nl80211.h"
> >  #include "core.h"
> >  #include "sysfs.h"
> > @@ -964,10 +963,6 @@ void cfg80211_stop_iface(struct wiphy *wiphy, struct wireless_dev *wdev,
> >  }
> >  EXPORT_SYMBOL(cfg80211_stop_iface);
> >  
> > -static const struct rtnl_link_ops wireless_link_ops = {
> > -	.kind = "wlan",
> > -};
> > -
> >  static int cfg80211_netdev_notifier_call(struct notifier_block *nb,
> >  					 unsigned long state, void *ptr)
> >  {
> > @@ -986,7 +981,6 @@ static int cfg80211_netdev_notifier_call(struct notifier_block *nb,
> >  	switch (state) {
> >  	case NETDEV_POST_INIT:
> >  		SET_NETDEV_DEVTYPE(dev, &wiphy_type);
> > -		dev->rtnl_link_ops = &wireless_link_ops;
> >  		break;
> >  	case NETDEV_REGISTER:
> >  		/*
> 
> 
> --
> 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: [PATCH] Revert "wireless: Support of IFLA_INFO_KIND rtnl attribute"

[Dan Williams]

On Mon, 2015-01-19 at 15:09 -0600, Dan Williams wrote:
> On Mon, 2015-01-19 at 13:22 -0600, Dan Williams wrote:
> > On Mon, 2015-01-19 at 18:52 +0100, Johannes Berg wrote:
> > > From: Johannes Berg <johannes.berg@intel.com>
> > > 
> > > This reverts commit ba1debdfed974f25aa598c283567878657b292ee.
> > > 
> > > Oliver reported that it breaks network-manager, for some reason with
> > > this patch NM decides that the device isn't wireless but "generic"
> > > (ethernet), sees no carrier (as expected with wifi) and fails to do
> > > anything else with it.
> > 
> > Hmm, which NM version?  NM uses either DEVTYPE (from 'uevent' in sysfs),
> > the phy80211 link in sysfs, or (if enabled) WEXT to figure out whether a
> > network interface with arptype=1 is WiFi or not.  I can't think of why
> > IFLA_INFO_KIND would break that...  what are the userspace visible
> > effects of the patch before reversion?
> 
> So with the patch, libnl will return 'wlan' from rtnl_link_get_type().
> NetworkManager was relying on the fact that this would return NULL for
> WiFi devices and was not falling back to alternate WiFi detection
> mechanisms (sysfs, DEVTYPE, etc) when an unknown IFLA_INFO_KIND was
> found.
> 
> NM 0.9.10+ have this bug.
> 
> I'll submit a patch upstream for review, which will get backported to
> 0.9.10 and later when approved.

https://bugzilla.gnome.org/show_bug.cgi?id=743209

Dan

> Dan
> 
> > Dan
> > 
> > > Revert this to unbreak userspace.
> > > 
> > > Reported-by: Oliver Hartkopp <socketcan@hartkopp.net>
> > > Tested-by: Oliver Hartkopp <socketcan@hartkopp.net>
> > > Signed-off-by: Johannes Berg <johannes.berg@intel.com>
> > > ---
> > >  net/wireless/core.c | 6 ------
> > >  1 file changed, 6 deletions(-)
> > > 
> > > diff --git a/net/wireless/core.c b/net/wireless/core.c
> > > index 456e4c38c279..3af0ecf1cc16 100644
> > > --- a/net/wireless/core.c
> > > +++ b/net/wireless/core.c
> > > @@ -21,7 +21,6 @@
> > >  #include <linux/sched.h>
> > >  #include <net/genetlink.h>
> > >  #include <net/cfg80211.h>
> > > -#include <net/rtnetlink.h>
> > >  #include "nl80211.h"
> > >  #include "core.h"
> > >  #include "sysfs.h"
> > > @@ -964,10 +963,6 @@ void cfg80211_stop_iface(struct wiphy *wiphy, struct wireless_dev *wdev,
> > >  }
> > >  EXPORT_SYMBOL(cfg80211_stop_iface);
> > >  
> > > -static const struct rtnl_link_ops wireless_link_ops = {
> > > -	.kind = "wlan",
> > > -};
> > > -
> > >  static int cfg80211_netdev_notifier_call(struct notifier_block *nb,
> > >  					 unsigned long state, void *ptr)
> > >  {
> > > @@ -986,7 +981,6 @@ static int cfg80211_netdev_notifier_call(struct notifier_block *nb,
> > >  	switch (state) {
> > >  	case NETDEV_POST_INIT:
> > >  		SET_NETDEV_DEVTYPE(dev, &wiphy_type);
> > > -		dev->rtnl_link_ops = &wireless_link_ops;
> > >  		break;
> > >  	case NETDEV_REGISTER:
> > >  		/*
> > 
> > 
> > --
> > 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
> 
> 
> --
> 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