linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC v2 0/3] Improve wireless netdev detection
@ 2016-07-09  6:16 Denis Kenzior
  2016-07-09  6:16 ` [RFC v2 1/3] nl80211: Add nl80211_notify_iface Denis Kenzior
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Denis Kenzior @ 2016-07-09  6:16 UTC (permalink / raw)
  To: linux-wireless; +Cc: Denis Kenzior

The current mechanism to detect hot-plug / unplug of wireless devices is
somewhat arcane.  One has to listen to NEW_WIPHY/DEL_WIPHY events over
nl80211 as well as RTM_NEWLINK / RTM_DELLINK events over rtnl, then
somehow find a correlation between these events.  This involves userspace
sending GET_INTERFACE or GET_WIPHY commands to the kernel, which incurs
additional roundtrips.

This patch series proposes that NEW_INTERFACE and DEL_INTERFACE events are
always emitted, regardless of whether a netdev was added/removed by the
driver or explicitly via NEW_INTERFACE/DEL_INTERFACE commands.

v2: Squished patches 2+3, 4+5.  DEL_INTERFACE event notification is now
sent inside cfg80211_unregister_wdev instead of nl80211_del_interface.

Denis Kenzior (3):
  nl80211: Add nl80211_notify_iface
  core: Always notify of new wireless netdevs
  core: Always notify when wireless netdev is removed

 net/wireless/core.c    |  6 +++++
 net/wireless/nl80211.c | 69 +++++++++++++++++++++++++++-----------------------
 net/wireless/nl80211.h |  3 +++
 3 files changed, 47 insertions(+), 31 deletions(-)

-- 
2.7.3


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [RFC v2 1/3] nl80211: Add nl80211_notify_iface
  2016-07-09  6:16 [RFC v2 0/3] Improve wireless netdev detection Denis Kenzior
@ 2016-07-09  6:16 ` Denis Kenzior
  2016-07-09 14:05   ` Marcel Holtmann
  2016-07-09  6:16 ` [RFC v2 2/3] core: Always notify of new wireless netdevs Denis Kenzior
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 11+ messages in thread
From: Denis Kenzior @ 2016-07-09  6:16 UTC (permalink / raw)
  To: linux-wireless; +Cc: Denis Kenzior

This function emits NL80211_CMD_NEW_INTERFACE or
NL80211_CMD_DEL_INTERFACE events.  This is meant to be used by the core
to notify userspace applications such as wpa_supplicant when a netdev
related to a wireless device has been added or removed.

Signed-off-by: Denis Kenzior <denkenz@gmail.com>
---
 net/wireless/nl80211.c | 28 ++++++++++++++++++++++++++++
 net/wireless/nl80211.h |  3 +++
 2 files changed, 31 insertions(+)

diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index f39fd4d..da03e17 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -11855,6 +11855,34 @@ void nl80211_notify_wiphy(struct cfg80211_registered_device *rdev,
 				NL80211_MCGRP_CONFIG, GFP_KERNEL);
 }
 
+void nl80211_notify_iface(struct cfg80211_registered_device *rdev,
+				struct wireless_dev *wdev,
+				enum nl80211_commands cmd)
+{
+	struct sk_buff *msg;
+	bool removal;
+
+	WARN_ON(cmd != NL80211_CMD_NEW_INTERFACE &&
+		cmd != NL80211_CMD_DEL_INTERFACE);
+
+	if (cmd == NL80211_CMD_DEL_INTERFACE)
+		removal = true;
+	else
+		removal = false;
+
+	msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
+	if (!msg)
+		return;
+
+	if (nl80211_send_iface(msg, 0, 0, 0, rdev, wdev, removal) < 0) {
+		nlmsg_free(msg);
+		return;
+	}
+
+	genlmsg_multicast_netns(&nl80211_fam, wiphy_net(&rdev->wiphy), msg, 0,
+				NL80211_MCGRP_CONFIG, GFP_KERNEL);
+}
+
 static int nl80211_add_scan_req(struct sk_buff *msg,
 				struct cfg80211_registered_device *rdev)
 {
diff --git a/net/wireless/nl80211.h b/net/wireless/nl80211.h
index a63f402..6f6b73c 100644
--- a/net/wireless/nl80211.h
+++ b/net/wireless/nl80211.h
@@ -7,6 +7,9 @@ int nl80211_init(void);
 void nl80211_exit(void);
 void nl80211_notify_wiphy(struct cfg80211_registered_device *rdev,
 			  enum nl80211_commands cmd);
+void nl80211_notify_iface(struct cfg80211_registered_device *rdev,
+				struct wireless_dev *wdev,
+				enum nl80211_commands cmd);
 void nl80211_send_scan_start(struct cfg80211_registered_device *rdev,
 			     struct wireless_dev *wdev);
 struct sk_buff *nl80211_build_scan_msg(struct cfg80211_registered_device *rdev,
-- 
2.7.3


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [RFC v2 2/3] core: Always notify of new wireless netdevs
  2016-07-09  6:16 [RFC v2 0/3] Improve wireless netdev detection Denis Kenzior
  2016-07-09  6:16 ` [RFC v2 1/3] nl80211: Add nl80211_notify_iface Denis Kenzior
@ 2016-07-09  6:16 ` Denis Kenzior
  2016-07-09  6:16 ` [RFC v2 3/3] core: Always notify when wireless netdev is removed Denis Kenzior
  2016-08-03  6:59 ` [RFC v2 0/3] Improve wireless netdev detection Johannes Berg
  3 siblings, 0 replies; 11+ messages in thread
From: Denis Kenzior @ 2016-07-09  6:16 UTC (permalink / raw)
  To: linux-wireless; +Cc: Denis Kenzior

This change alters the semantics of NL80211_CMD_NEW_INTERFACE events
by always sending this event whenever a new net_device object
associated with a wdev is registered.  Prior to this change, this event
was only sent as a result of NL80211_CMD_NEW_INTERFACE command sent
from userspace.  This allows userspace to reliably detect new wireless
interfaces (e.g. due to hardware hot-plug events, etc).

For wdevs created without an associated net_device object (e.g.
NL80211_IFTYPE_P2P_DEVICE), the NL80211_CMD_NEW_INTERFACE event is
still generated inside the relevant nl80211 command handler.

Signed-off-by: Denis Kenzior <denkenz@gmail.com>
---
 net/wireless/core.c    |  2 ++
 net/wireless/nl80211.c | 23 +++++++++--------------
 2 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/net/wireless/core.c b/net/wireless/core.c
index 7645e97..7758c0f 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -1079,6 +1079,8 @@ static int cfg80211_netdev_notifier_call(struct notifier_block *nb,
 		     wdev->iftype == NL80211_IFTYPE_P2P_CLIENT ||
 		     wdev->iftype == NL80211_IFTYPE_ADHOC) && !wdev->use_4addr)
 			dev->priv_flags |= IFF_DONT_BRIDGE;
+
+		nl80211_notify_iface(rdev, wdev, NL80211_CMD_NEW_INTERFACE);
 		break;
 	case NETDEV_GOING_DOWN:
 		cfg80211_leave(rdev, wdev);
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index da03e17..ec8eb88 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -2787,7 +2787,7 @@ static int nl80211_new_interface(struct sk_buff *skb, struct genl_info *info)
 	struct cfg80211_registered_device *rdev = info->user_ptr[0];
 	struct vif_params params;
 	struct wireless_dev *wdev;
-	struct sk_buff *msg, *event;
+	struct sk_buff *msg;
 	int err;
 	enum nl80211_iftype type = NL80211_IFTYPE_UNSPECIFIED;
 	u32 flags;
@@ -2891,20 +2891,15 @@ static int nl80211_new_interface(struct sk_buff *skb, struct genl_info *info)
 		return -ENOBUFS;
 	}
 
-	event = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
-	if (event) {
-		if (nl80211_send_iface(event, 0, 0, 0,
-				       rdev, wdev, false) < 0) {
-			nlmsg_free(event);
-			goto out;
-		}
-
-		genlmsg_multicast_netns(&nl80211_fam, wiphy_net(&rdev->wiphy),
-					event, 0, NL80211_MCGRP_CONFIG,
-					GFP_KERNEL);
-	}
+	/*
+	 * For wdevs which have no associated netdev object (e.g. of type
+	 * NL80211_IFTYPE_P2P_DEVICE), emit the NEW_INTERFACE event here.
+	 * For all other types, the event will be generated from the
+	 * netdev notifier
+	 */
+	if (!wdev->netdev)
+		nl80211_notify_iface(rdev, wdev, NL80211_CMD_NEW_INTERFACE);
 
-out:
 	return genlmsg_reply(msg, info);
 }
 
-- 
2.7.3


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [RFC v2 3/3] core: Always notify when wireless netdev is removed
  2016-07-09  6:16 [RFC v2 0/3] Improve wireless netdev detection Denis Kenzior
  2016-07-09  6:16 ` [RFC v2 1/3] nl80211: Add nl80211_notify_iface Denis Kenzior
  2016-07-09  6:16 ` [RFC v2 2/3] core: Always notify of new wireless netdevs Denis Kenzior
@ 2016-07-09  6:16 ` Denis Kenzior
  2016-08-03  6:59 ` [RFC v2 0/3] Improve wireless netdev detection Johannes Berg
  3 siblings, 0 replies; 11+ messages in thread
From: Denis Kenzior @ 2016-07-09  6:16 UTC (permalink / raw)
  To: linux-wireless; +Cc: Denis Kenzior

This change alters the semantics of NL80211_CMD_DEL_INTERFACE events
by always sending this event whenever a net_device object associated
with a wdev is destroyed.  Prior to this change, this event was only
emitted as a result of NL80211_CMD_DEL_INTERFACE command sent from
userspace.  This allows userspace to reliably detect when wireless
interfaces have been removed, e.g. due to USB removal events, etc.

For wireless device objects without an associated net_device (e.g.
NL80211_IFTYPE_P2P_DEVICE), the NL80211_CMD_DEL_INTERFACE event is
now generated inside cfg80211_unregister_wdev.

Signed-off-by: Denis Kenzior <denkenz@gmail.com>
---
 net/wireless/core.c    |  4 ++++
 net/wireless/nl80211.c | 18 +-----------------
 2 files changed, 5 insertions(+), 17 deletions(-)

diff --git a/net/wireless/core.c b/net/wireless/core.c
index 7758c0f..fccead7 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -906,6 +906,8 @@ void cfg80211_unregister_wdev(struct wireless_dev *wdev)
 	if (WARN_ON(wdev->netdev))
 		return;
 
+	nl80211_notify_iface(rdev, wdev, NL80211_CMD_DEL_INTERFACE);
+
 	list_del_rcu(&wdev->list);
 	rdev->devlist_generation++;
 
@@ -1159,6 +1161,8 @@ static int cfg80211_netdev_notifier_call(struct notifier_block *nb,
 		 * remove and clean it up.
 		 */
 		if (!list_empty(&wdev->list)) {
+			nl80211_notify_iface(rdev, wdev,
+						NL80211_CMD_DEL_INTERFACE);
 			sysfs_remove_link(&dev->dev.kobj, "phy80211");
 			list_del_rcu(&wdev->list);
 			rdev->devlist_generation++;
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index ec8eb88..bc45d8a 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -2907,18 +2907,10 @@ static int nl80211_del_interface(struct sk_buff *skb, struct genl_info *info)
 {
 	struct cfg80211_registered_device *rdev = info->user_ptr[0];
 	struct wireless_dev *wdev = info->user_ptr[1];
-	struct sk_buff *msg;
-	int status;
 
 	if (!rdev->ops->del_virtual_intf)
 		return -EOPNOTSUPP;
 
-	msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
-	if (msg && nl80211_send_iface(msg, 0, 0, 0, rdev, wdev, true) < 0) {
-		nlmsg_free(msg);
-		msg = NULL;
-	}
-
 	/*
 	 * If we remove a wireless device without a netdev then clear
 	 * user_ptr[1] so that nl80211_post_doit won't dereference it
@@ -2929,15 +2921,7 @@ static int nl80211_del_interface(struct sk_buff *skb, struct genl_info *info)
 	if (!wdev->netdev)
 		info->user_ptr[1] = NULL;
 
-	status = rdev_del_virtual_intf(rdev, wdev);
-	if (status >= 0 && msg)
-		genlmsg_multicast_netns(&nl80211_fam, wiphy_net(&rdev->wiphy),
-					msg, 0, NL80211_MCGRP_CONFIG,
-					GFP_KERNEL);
-	else
-		nlmsg_free(msg);
-
-	return status;
+	return rdev_del_virtual_intf(rdev, wdev);
 }
 
 static int nl80211_set_noack_map(struct sk_buff *skb, struct genl_info *info)
-- 
2.7.3


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [RFC v2 1/3] nl80211: Add nl80211_notify_iface
  2016-07-09  6:16 ` [RFC v2 1/3] nl80211: Add nl80211_notify_iface Denis Kenzior
@ 2016-07-09 14:05   ` Marcel Holtmann
  2016-07-10 18:42     ` Arend Van Spriel
  0 siblings, 1 reply; 11+ messages in thread
From: Marcel Holtmann @ 2016-07-09 14:05 UTC (permalink / raw)
  To: Denis Kenzior; +Cc: linux-wireless

Hi Denis,

> This function emits NL80211_CMD_NEW_INTERFACE or
> NL80211_CMD_DEL_INTERFACE events.  This is meant to be used by the core
> to notify userspace applications such as wpa_supplicant when a netdev
> related to a wireless device has been added or removed.
> 
> Signed-off-by: Denis Kenzior <denkenz@gmail.com>
> ---
> net/wireless/nl80211.c | 28 ++++++++++++++++++++++++++++
> net/wireless/nl80211.h |  3 +++
> 2 files changed, 31 insertions(+)
> 
> diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
> index f39fd4d..da03e17 100644
> --- a/net/wireless/nl80211.c
> +++ b/net/wireless/nl80211.c
> @@ -11855,6 +11855,34 @@ void nl80211_notify_wiphy(struct cfg80211_registered_device *rdev,
> 				NL80211_MCGRP_CONFIG, GFP_KERNEL);
> }
> 
> +void nl80211_notify_iface(struct cfg80211_registered_device *rdev,
> +				struct wireless_dev *wdev,
> +				enum nl80211_commands cmd)
> +{
> +	struct sk_buff *msg;
> +	bool removal;
> +
> +	WARN_ON(cmd != NL80211_CMD_NEW_INTERFACE &&
> +		cmd != NL80211_CMD_DEL_INTERFACE);

I would assume that BUILD_BUG_ON does a way job better here.

Regards

Marcel


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFC v2 1/3] nl80211: Add nl80211_notify_iface
  2016-07-09 14:05   ` Marcel Holtmann
@ 2016-07-10 18:42     ` Arend Van Spriel
  2016-07-10 19:47       ` Marcel Holtmann
  0 siblings, 1 reply; 11+ messages in thread
From: Arend Van Spriel @ 2016-07-10 18:42 UTC (permalink / raw)
  To: Marcel Holtmann, Denis Kenzior; +Cc: linux-wireless



On 9-7-2016 16:05, Marcel Holtmann wrote:
> Hi Denis,
> 
>> This function emits NL80211_CMD_NEW_INTERFACE or
>> NL80211_CMD_DEL_INTERFACE events.  This is meant to be used by the core
>> to notify userspace applications such as wpa_supplicant when a netdev
>> related to a wireless device has been added or removed.
>>
>> Signed-off-by: Denis Kenzior <denkenz@gmail.com>
>> ---
>> net/wireless/nl80211.c | 28 ++++++++++++++++++++++++++++
>> net/wireless/nl80211.h |  3 +++
>> 2 files changed, 31 insertions(+)
>>
>> diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
>> index f39fd4d..da03e17 100644
>> --- a/net/wireless/nl80211.c
>> +++ b/net/wireless/nl80211.c
>> @@ -11855,6 +11855,34 @@ void nl80211_notify_wiphy(struct cfg80211_registered_device *rdev,
>> 				NL80211_MCGRP_CONFIG, GFP_KERNEL);
>> }
>>
>> +void nl80211_notify_iface(struct cfg80211_registered_device *rdev,
>> +				struct wireless_dev *wdev,
>> +				enum nl80211_commands cmd)
>> +{
>> +	struct sk_buff *msg;
>> +	bool removal;
>> +
>> +	WARN_ON(cmd != NL80211_CMD_NEW_INTERFACE &&
>> +		cmd != NL80211_CMD_DEL_INTERFACE);
> 
> I would assume that BUILD_BUG_ON does a way job better here.

Why? I don't see why the condition would be compile-time constant as it
can be called from outside nl80211.c.

Regards,
Arend

> Regards
> 
> Marcel
> 
> --
> 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
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFC v2 1/3] nl80211: Add nl80211_notify_iface
  2016-07-10 18:42     ` Arend Van Spriel
@ 2016-07-10 19:47       ` Marcel Holtmann
  2016-07-10 23:49         ` Julian Calaby
  0 siblings, 1 reply; 11+ messages in thread
From: Marcel Holtmann @ 2016-07-10 19:47 UTC (permalink / raw)
  To: Arend Van Spriel; +Cc: Denis Kenzior, linux-wireless

Hi Arend,

>>> This function emits NL80211_CMD_NEW_INTERFACE or
>>> NL80211_CMD_DEL_INTERFACE events.  This is meant to be used by the core
>>> to notify userspace applications such as wpa_supplicant when a netdev
>>> related to a wireless device has been added or removed.
>>> 
>>> Signed-off-by: Denis Kenzior <denkenz@gmail.com>
>>> ---
>>> net/wireless/nl80211.c | 28 ++++++++++++++++++++++++++++
>>> net/wireless/nl80211.h |  3 +++
>>> 2 files changed, 31 insertions(+)
>>> 
>>> diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
>>> index f39fd4d..da03e17 100644
>>> --- a/net/wireless/nl80211.c
>>> +++ b/net/wireless/nl80211.c
>>> @@ -11855,6 +11855,34 @@ void nl80211_notify_wiphy(struct cfg80211_registered_device *rdev,
>>> 				NL80211_MCGRP_CONFIG, GFP_KERNEL);
>>> }
>>> 
>>> +void nl80211_notify_iface(struct cfg80211_registered_device *rdev,
>>> +				struct wireless_dev *wdev,
>>> +				enum nl80211_commands cmd)
>>> +{
>>> +	struct sk_buff *msg;
>>> +	bool removal;
>>> +
>>> +	WARN_ON(cmd != NL80211_CMD_NEW_INTERFACE &&
>>> +		cmd != NL80211_CMD_DEL_INTERFACE);
>> 
>> I would assume that BUILD_BUG_ON does a way job better here.
> 
> Why? I don't see why the condition would be compile-time constant as it
> can be called from outside nl80211.c.

what has that to do with it. No matter who calls it, the point is to ensure that it is called for new/del interface. And if a caller uses it for something else, then it should fail and it should fail loudly.

Regards

Marcel


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFC v2 1/3] nl80211: Add nl80211_notify_iface
  2016-07-10 19:47       ` Marcel Holtmann
@ 2016-07-10 23:49         ` Julian Calaby
  2016-07-11  7:39           ` Marcel Holtmann
  0 siblings, 1 reply; 11+ messages in thread
From: Julian Calaby @ 2016-07-10 23:49 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: Arend Van Spriel, Denis Kenzior, linux-wireless

Hi Marcel,

On Mon, Jul 11, 2016 at 5:47 AM, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Arend,
>
>>>> This function emits NL80211_CMD_NEW_INTERFACE or
>>>> NL80211_CMD_DEL_INTERFACE events.  This is meant to be used by the core
>>>> to notify userspace applications such as wpa_supplicant when a netdev
>>>> related to a wireless device has been added or removed.
>>>>
>>>> Signed-off-by: Denis Kenzior <denkenz@gmail.com>
>>>> ---
>>>> net/wireless/nl80211.c | 28 ++++++++++++++++++++++++++++
>>>> net/wireless/nl80211.h |  3 +++
>>>> 2 files changed, 31 insertions(+)
>>>>
>>>> diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
>>>> index f39fd4d..da03e17 100644
>>>> --- a/net/wireless/nl80211.c
>>>> +++ b/net/wireless/nl80211.c
>>>> @@ -11855,6 +11855,34 @@ void nl80211_notify_wiphy(struct cfg80211_registered_device *rdev,
>>>>                             NL80211_MCGRP_CONFIG, GFP_KERNEL);
>>>> }
>>>>
>>>> +void nl80211_notify_iface(struct cfg80211_registered_device *rdev,
>>>> +                           struct wireless_dev *wdev,
>>>> +                           enum nl80211_commands cmd)
>>>> +{
>>>> +   struct sk_buff *msg;
>>>> +   bool removal;
>>>> +
>>>> +   WARN_ON(cmd != NL80211_CMD_NEW_INTERFACE &&
>>>> +           cmd != NL80211_CMD_DEL_INTERFACE);
>>>
>>> I would assume that BUILD_BUG_ON does a way job better here.
>>
>> Why? I don't see why the condition would be compile-time constant as it
>> can be called from outside nl80211.c.
>
> what has that to do with it. No matter who calls it, the point is to ensure that it is called for new/del interface. And if a caller uses it for something else, then it should fail and it should fail loudly.

Let me ask this question another way:

How is BUILD_BUG_ON going to warn about a runtime error?

Thanks,

-- 
Julian Calaby

Email: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFC v2 1/3] nl80211: Add nl80211_notify_iface
  2016-07-10 23:49         ` Julian Calaby
@ 2016-07-11  7:39           ` Marcel Holtmann
  2016-07-11  9:34             ` Arend Van Spriel
  0 siblings, 1 reply; 11+ messages in thread
From: Marcel Holtmann @ 2016-07-11  7:39 UTC (permalink / raw)
  To: Julian Calaby; +Cc: Arend Van Spriel, Denis Kenzior, linux-wireless

Hi Julian,

>>>>> This function emits NL80211_CMD_NEW_INTERFACE or
>>>>> NL80211_CMD_DEL_INTERFACE events.  This is meant to be used by the core
>>>>> to notify userspace applications such as wpa_supplicant when a netdev
>>>>> related to a wireless device has been added or removed.
>>>>> 
>>>>> Signed-off-by: Denis Kenzior <denkenz@gmail.com>
>>>>> ---
>>>>> net/wireless/nl80211.c | 28 ++++++++++++++++++++++++++++
>>>>> net/wireless/nl80211.h |  3 +++
>>>>> 2 files changed, 31 insertions(+)
>>>>> 
>>>>> diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
>>>>> index f39fd4d..da03e17 100644
>>>>> --- a/net/wireless/nl80211.c
>>>>> +++ b/net/wireless/nl80211.c
>>>>> @@ -11855,6 +11855,34 @@ void nl80211_notify_wiphy(struct cfg80211_registered_device *rdev,
>>>>>                            NL80211_MCGRP_CONFIG, GFP_KERNEL);
>>>>> }
>>>>> 
>>>>> +void nl80211_notify_iface(struct cfg80211_registered_device *rdev,
>>>>> +                           struct wireless_dev *wdev,
>>>>> +                           enum nl80211_commands cmd)
>>>>> +{
>>>>> +   struct sk_buff *msg;
>>>>> +   bool removal;
>>>>> +
>>>>> +   WARN_ON(cmd != NL80211_CMD_NEW_INTERFACE &&
>>>>> +           cmd != NL80211_CMD_DEL_INTERFACE);
>>>> 
>>>> I would assume that BUILD_BUG_ON does a way job better here.
>>> 
>>> Why? I don't see why the condition would be compile-time constant as it
>>> can be called from outside nl80211.c.
>> 
>> what has that to do with it. No matter who calls it, the point is to ensure that it is called for new/del interface. And if a caller uses it for something else, then it should fail and it should fail loudly.
> 
> Let me ask this question another way:
> 
> How is BUILD_BUG_ON going to warn about a runtime error?

how is this a runtime error in the first place? If you call it with any other nl80211_commands than the two explicitly allowed, you are doing something wrong. If BUILD_BUG_ON has limitations, then fine, but please don't tell me this is a runtime error.

Regards

Marcel


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFC v2 1/3] nl80211: Add nl80211_notify_iface
  2016-07-11  7:39           ` Marcel Holtmann
@ 2016-07-11  9:34             ` Arend Van Spriel
  0 siblings, 0 replies; 11+ messages in thread
From: Arend Van Spriel @ 2016-07-11  9:34 UTC (permalink / raw)
  To: Marcel Holtmann, Julian Calaby; +Cc: Denis Kenzior, linux-wireless

On 11-7-2016 9:39, Marcel Holtmann wrote:
> Hi Julian,
> 
>>>>>> This function emits NL80211_CMD_NEW_INTERFACE or
>>>>>> NL80211_CMD_DEL_INTERFACE events.  This is meant to be used by the core
>>>>>> to notify userspace applications such as wpa_supplicant when a netdev
>>>>>> related to a wireless device has been added or removed.
>>>>>>
>>>>>> Signed-off-by: Denis Kenzior <denkenz@gmail.com>
>>>>>> ---
>>>>>> net/wireless/nl80211.c | 28 ++++++++++++++++++++++++++++
>>>>>> net/wireless/nl80211.h |  3 +++
>>>>>> 2 files changed, 31 insertions(+)
>>>>>>
>>>>>> diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
>>>>>> index f39fd4d..da03e17 100644
>>>>>> --- a/net/wireless/nl80211.c
>>>>>> +++ b/net/wireless/nl80211.c
>>>>>> @@ -11855,6 +11855,34 @@ void nl80211_notify_wiphy(struct cfg80211_registered_device *rdev,
>>>>>>                            NL80211_MCGRP_CONFIG, GFP_KERNEL);
>>>>>> }
>>>>>>
>>>>>> +void nl80211_notify_iface(struct cfg80211_registered_device *rdev,
>>>>>> +                           struct wireless_dev *wdev,
>>>>>> +                           enum nl80211_commands cmd)
>>>>>> +{
>>>>>> +   struct sk_buff *msg;
>>>>>> +   bool removal;
>>>>>> +
>>>>>> +   WARN_ON(cmd != NL80211_CMD_NEW_INTERFACE &&
>>>>>> +           cmd != NL80211_CMD_DEL_INTERFACE);
>>>>>
>>>>> I would assume that BUILD_BUG_ON does a way job better here.
>>>>
>>>> Why? I don't see why the condition would be compile-time constant as it
>>>> can be called from outside nl80211.c.
>>>
>>> what has that to do with it. No matter who calls it, the point is to ensure that it is called for new/del interface. And if a caller uses it for something else, then it should fail and it should fail loudly.
>>
>> Let me ask this question another way:
>>
>> How is BUILD_BUG_ON going to warn about a runtime error?
> 
> how is this a runtime error in the first place? If you call it with any other nl80211_commands than the two explicitly allowed, you are doing something wrong. If BUILD_BUG_ON has limitations, then fine, but please don't tell me this is a runtime error.

Hi Marcel,

Indeed it is not a run-time error. The limitation of BUILD_BUG_ON is
that the condition has to be compile-time constant. This means upon
compiling nl80211.c. BUILD_BUG_ON does not work at link-time so upon
linking nl80211.o with core.o where the function is called from (in
patches 2 and 3).


Regards,
Arend

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFC v2 0/3] Improve wireless netdev detection
  2016-07-09  6:16 [RFC v2 0/3] Improve wireless netdev detection Denis Kenzior
                   ` (2 preceding siblings ...)
  2016-07-09  6:16 ` [RFC v2 3/3] core: Always notify when wireless netdev is removed Denis Kenzior
@ 2016-08-03  6:59 ` Johannes Berg
  3 siblings, 0 replies; 11+ messages in thread
From: Johannes Berg @ 2016-08-03  6:59 UTC (permalink / raw)
  To: Denis Kenzior, linux-wireless

On Sat, 2016-07-09 at 01:16 -0500, Denis Kenzior wrote:
> The current mechanism to detect hot-plug / unplug of wireless devices
> is
> somewhat arcane.  One has to listen to NEW_WIPHY/DEL_WIPHY events
> over
> nl80211 as well as RTM_NEWLINK / RTM_DELLINK events over rtnl, then
> somehow find a correlation between these events.  This involves
> userspace
> sending GET_INTERFACE or GET_WIPHY commands to the kernel, which
> incurs
> additional roundtrips.
> 
> This patch series proposes that NEW_INTERFACE and DEL_INTERFACE
> events are
> always emitted, regardless of whether a netdev was added/removed by
> the
> driver or explicitly via NEW_INTERFACE/DEL_INTERFACE commands.
> 
> v2: Squished patches 2+3, 4+5.  DEL_INTERFACE event notification is
> now
> sent inside cfg80211_unregister_wdev instead of
> nl80211_del_interface.
> 

Looks fine to me, care to resend as [PATCH]?

johannes

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2016-08-03  8:01 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-07-09  6:16 [RFC v2 0/3] Improve wireless netdev detection Denis Kenzior
2016-07-09  6:16 ` [RFC v2 1/3] nl80211: Add nl80211_notify_iface Denis Kenzior
2016-07-09 14:05   ` Marcel Holtmann
2016-07-10 18:42     ` Arend Van Spriel
2016-07-10 19:47       ` Marcel Holtmann
2016-07-10 23:49         ` Julian Calaby
2016-07-11  7:39           ` Marcel Holtmann
2016-07-11  9:34             ` Arend Van Spriel
2016-07-09  6:16 ` [RFC v2 2/3] core: Always notify of new wireless netdevs Denis Kenzior
2016-07-09  6:16 ` [RFC v2 3/3] core: Always notify when wireless netdev is removed Denis Kenzior
2016-08-03  6:59 ` [RFC v2 0/3] Improve wireless netdev detection Johannes Berg

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).