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
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 <[email protected]>
---
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
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 <[email protected]>
>>> ---
>>> 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
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 <[email protected]>
---
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
Hi Marcel,
On Mon, Jul 11, 2016 at 5:47 AM, Marcel Holtmann <[email protected]> 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 <[email protected]>
>>>> ---
>>>> 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: [email protected]
Profile: http://www.google.com/profiles/julian.calaby/
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 <[email protected]>
> ---
> 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
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 <[email protected]>
>> ---
>> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
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 <[email protected]>
>>>>> ---
>>>>> 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
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 <[email protected]>
---
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
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 <[email protected]>
>>>>>> ---
>>>>>> 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
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