* [PATCH] wext: fix message delay/ordering
@ 2016-01-27 12:14 Johannes Berg
2016-01-27 12:17 ` Johannes Berg
0 siblings, 1 reply; 4+ messages in thread
From: Johannes Berg @ 2016-01-27 12:14 UTC (permalink / raw)
To: linux-wireless; +Cc: Beniamino Galvani, Johannes Berg
From: Johannes Berg <johannes.berg@intel.com>
Beniamino reported that he was getting an RTM_NEWLINK message for a
given interface, after the RTM_DELLINK for it. It turns out that the
message is a wireless extensions message, which was sent because the
interface had been connected and disconnection while it was deleted
caused a wext message.
It's strange that wext uses RTM_NEWLINK, and the message is somewhat
malformed without any proper attributes, causing "ip monitor link"
to print just rudimentary information:
5: wlan1: <BROADCAST,MULTICAST> mtu 1500 qdisc mq state DOWN group default
link/ether 02:00:00:00:01:00 brd ff:ff:ff:ff:ff:ff
Deleted 5: wlan1: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN group default
link/ether 02:00:00:00:01:00 brd ff:ff:ff:ff:ff:ff
5: wlan1: <BROADCAST,MULTICAST,UP>
link/ether
(from my hwsim reproduction)
The reason for this is that wext schedules a worker to send out the
messages, and the scheduling delay can cause the messages to get out
to userspace after the RTM_DELLINK.
To fix this, have wext register a netdevice notifier and flush out
any pending messages when a netdevice is unregistered.
Cc: stable@vger.kernel.org
Reported-by: Beniamino Galvani <bgalvani@redhat.com>
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
net/wireless/wext-core.c | 48 ++++++++++++++++++++++++++++++++++++++----------
1 file changed, 38 insertions(+), 10 deletions(-)
diff --git a/net/wireless/wext-core.c b/net/wireless/wext-core.c
index c8717c1d082e..4e4e1c5236cc 100644
--- a/net/wireless/wext-core.c
+++ b/net/wireless/wext-core.c
@@ -342,6 +342,38 @@ static const int compat_event_type_size[] = {
/* IW event code */
+static void wireless_nlevent_flush(void)
+{
+ struct sk_buff *skb;
+ struct net *net;
+
+ ASSERT_RTNL();
+
+ for_each_net(net) {
+ while ((skb = skb_dequeue(&net->wext_nlevents)))
+ rtnl_notify(skb, net, 0, RTNLGRP_LINK, NULL,
+ GFP_KERNEL);
+ }
+}
+
+static int wext_netdev_notifier_call(struct notifier_block *nb,
+ unsigned long state, void *ptr)
+{
+ /*
+ * When a netdev is unregistered, flush all pending messages
+ * to avoid them going out after the RTM_DELLINK, which can
+ * happen due to the schedule_work().
+ */
+ if (state == NETDEV_UNREGISTER)
+ wireless_nlevent_flush();
+
+ return NOTIFY_OK;
+}
+
+static struct notifier_block wext_netdev_notifier = {
+ .notifier_call = wext_netdev_notifier_call,
+};
+
static int __net_init wext_pernet_init(struct net *net)
{
skb_queue_head_init(&net->wext_nlevents);
@@ -360,6 +392,11 @@ static struct pernet_operations wext_pernet_ops = {
static int __init wireless_nlevent_init(void)
{
+ int err = register_netdevice_notifier(&wext_netdev_notifier);
+
+ if (err)
+ return err;
+
return register_pernet_subsys(&wext_pernet_ops);
}
@@ -368,17 +405,8 @@ subsys_initcall(wireless_nlevent_init);
/* Process events generated by the wireless layer or the driver. */
static void wireless_nlevent_process(struct work_struct *work)
{
- struct sk_buff *skb;
- struct net *net;
-
rtnl_lock();
-
- for_each_net(net) {
- while ((skb = skb_dequeue(&net->wext_nlevents)))
- rtnl_notify(skb, net, 0, RTNLGRP_LINK, NULL,
- GFP_KERNEL);
- }
-
+ wireless_nlevent_flush();
rtnl_unlock();
}
--
2.6.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] wext: fix message delay/ordering
2016-01-27 12:14 [PATCH] wext: fix message delay/ordering Johannes Berg
@ 2016-01-27 12:17 ` Johannes Berg
2016-01-27 12:18 ` Johannes Berg
0 siblings, 1 reply; 4+ messages in thread
From: Johannes Berg @ 2016-01-27 12:17 UTC (permalink / raw)
To: linux-wireless; +Cc: Beniamino Galvani
On Wed, 2016-01-27 at 13:14 +0100, Johannes Berg wrote:
>
> +static int wext_netdev_notifier_call(struct notifier_block *nb,
> + unsigned long state, void *ptr)
> +{
> + /*
> + * When a netdev is unregistered, flush all pending messages
> + * to avoid them going out after the RTM_DELLINK, which can
> + * happen due to the schedule_work().
> + */
> + if (state == NETDEV_UNREGISTER)
> + wireless_nlevent_flush();
>
It could be argued that the state check isn't really necessary and
should be removed to avoid ordering issues with up/down vs. wext, but
this fixes the really strange issue where you get an RTM_NEWLINK after
RTM_DELLINK (with the same ifidx), and I don't see how any software
would care much about the ordering otherwise.
johannes
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] wext: fix message delay/ordering
2016-01-27 12:17 ` Johannes Berg
@ 2016-01-27 12:18 ` Johannes Berg
2016-01-27 12:28 ` Johannes Berg
0 siblings, 1 reply; 4+ messages in thread
From: Johannes Berg @ 2016-01-27 12:18 UTC (permalink / raw)
To: linux-wireless; +Cc: Beniamino Galvani
> > + if (state == NETDEV_UNREGISTER)
> > + wireless_nlevent_flush();
> >
> It could be argued that the state check isn't really necessary and
> should be removed to avoid ordering issues with up/down vs. wext, but
> this fixes the really strange issue where you get an RTM_NEWLINK
> after
> RTM_DELLINK (with the same ifidx), and I don't see how any software
> would care much about the ordering otherwise.
>
Actually though, with the fix I still get:
5: wlan1: <BROADCAST,MULTICAST> mtu 1500 qdisc mq state DOWN group default
link/ether 02:00:00:00:01:00 brd ff:ff:ff:ff:ff:ff
5: wlan1: <BROADCAST,MULTICAST,UP>
link/ether
Deleted 5: wlan1: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN group default
link/ether 02:00:00:00:01:00 brd ff:ff:ff:ff:ff:ff
which is clearly odd (see the UP flag), so I'll change it.
johannes
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] wext: fix message delay/ordering
2016-01-27 12:18 ` Johannes Berg
@ 2016-01-27 12:28 ` Johannes Berg
0 siblings, 0 replies; 4+ messages in thread
From: Johannes Berg @ 2016-01-27 12:28 UTC (permalink / raw)
To: linux-wireless; +Cc: Beniamino Galvani
On Wed, 2016-01-27 at 13:18 +0100, Johannes Berg wrote:
> > > + if (state == NETDEV_UNREGISTER)
> > > + wireless_nlevent_flush();
> > >
> > It could be argued that the state check isn't really necessary and
> > should be removed to avoid ordering issues with up/down vs. wext,
> > but
> > this fixes the really strange issue where you get an RTM_NEWLINK
> > after
> > RTM_DELLINK (with the same ifidx), and I don't see how any software
> > would care much about the ordering otherwise.
> >
>
> Actually though, with the fix I still get:
>
> 5: wlan1: <BROADCAST,MULTICAST> mtu 1500 qdisc mq state DOWN group
> default
> link/ether 02:00:00:00:01:00 brd ff:ff:ff:ff:ff:ff
> 5: wlan1: <BROADCAST,MULTICAST,UP>
> link/ether
> Deleted 5: wlan1: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state
> DOWN group default
> link/ether 02:00:00:00:01:00 brd ff:ff:ff:ff:ff:ff
>
> which is clearly odd (see the UP flag), so I'll change it.
>
Doesn't help, since the wext netdev notifier is registered earlier and
thus runs earlier than the cfg80211 one that triggers the action ...
I'll fix that differently then.
johannes
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-01-27 12:28 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-27 12:14 [PATCH] wext: fix message delay/ordering Johannes Berg
2016-01-27 12:17 ` Johannes Berg
2016-01-27 12:18 ` Johannes Berg
2016-01-27 12:28 ` Johannes Berg
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.