[patch 4] softmac: fix event sending

[patch 4] softmac: fix event sending

[Johannes Berg]

Softmac is sending custom events to userspace already, but it
should _really_ be sending the right WEXT events instead. This
patch fixes that.

Signed-off-by: Dan Williams <dcbw@redhat.com>
Signed-off-by: Johannes Berg <johannes@sipsolutions.net>

Index: wireless-2.6/include/net/ieee80211softmac.h
===================================================================
--- wireless-2.6.orig/include/net/ieee80211softmac.h	2006-04-13 02:35:54.686229309 +0200
+++ wireless-2.6/include/net/ieee80211softmac.h	2006-04-13 02:37:17.676229309 +0200
@@ -267,8 +267,9 @@
 #define IEEE80211SOFTMAC_EVENT_AUTH_FAILED		5
 #define IEEE80211SOFTMAC_EVENT_AUTH_TIMEOUT		6
 #define IEEE80211SOFTMAC_EVENT_ASSOCIATE_NET_NOT_FOUND	7
+#define IEEE80211SOFTMAC_EVENT_DISASSOCIATED		8
 /* keep this updated! */
-#define IEEE80211SOFTMAC_EVENT_LAST			7
+#define IEEE80211SOFTMAC_EVENT_LAST			8
 /*
  * If you want to be notified of certain events, you can call
  * ieee80211softmac_notify[_atomic] with
Index: wireless-2.6/net/ieee80211/softmac/ieee80211softmac_assoc.c
===================================================================
--- wireless-2.6.orig/net/ieee80211/softmac/ieee80211softmac_assoc.c	2006-04-13 02:35:54.686229309 +0200
+++ wireless-2.6/net/ieee80211/softmac/ieee80211softmac_assoc.c	2006-04-13 02:37:17.686229309 +0200
@@ -101,6 +101,7 @@
 	/* Do NOT clear bssvalid as that will break ieee80211softmac_assoc_work! */
 	mac->associated = 0;
 	mac->associnfo.associating = 0;
+	ieee80211softmac_call_events_locked(mac, IEEE80211SOFTMAC_EVENT_DISASSOCIATED, NULL);
 	spin_unlock_irqrestore(&mac->lock, flags);
 }
 
@@ -373,6 +374,7 @@
 	spin_lock_irqsave(&mac->lock, flags);
 	mac->associnfo.bssvalid = 0;
 	mac->associated = 0;
+	ieee80211softmac_call_events_locked(mac, IEEE80211SOFTMAC_EVENT_DISASSOCIATED, NULL);
 	schedule_work(&mac->associnfo.work);
 	spin_unlock_irqrestore(&mac->lock, flags);
 	
Index: wireless-2.6/net/ieee80211/softmac/ieee80211softmac_event.c
===================================================================
--- wireless-2.6.orig/net/ieee80211/softmac/ieee80211softmac_event.c	2006-04-13 02:35:54.686229309 +0200
+++ wireless-2.6/net/ieee80211/softmac/ieee80211softmac_event.c	2006-04-13 02:38:51.036229309 +0200
@@ -67,6 +67,7 @@
 	"authenticating failed",
 	"authenticating timed out",
 	"associating failed because no suitable network was found",
+	"disassociated",
 };
 
 
@@ -128,13 +129,36 @@
 ieee80211softmac_call_events_locked(struct ieee80211softmac_device *mac, int event, void *event_ctx)
 {
 	struct ieee80211softmac_event *eventptr, *tmp;
-	union iwreq_data wrqu;
-	char *msg;
+	struct ieee80211softmac_network *network;
 	
 	if (event >= 0) {
-		msg = event_descriptions[event];
-		wrqu.data.length = strlen(msg);
-		wireless_send_event(mac->dev, IWEVCUSTOM, &wrqu, msg);
+		union iwreq_data wrqu;
+		int we_event;
+		char *msg = NULL;
+
+		switch(event) {
+		case IEEE80211SOFTMAC_EVENT_ASSOCIATED:
+			network = (struct ieee80211softmac_network *)event_ctx;
+			wrqu.data.length = 0;
+			wrqu.data.flags = 0;
+			memcpy(wrqu.ap_addr.sa_data, &network->bssid[0], ETH_ALEN);
+			wrqu.ap_addr.sa_family = ARPHRD_ETHER;
+			we_event = SIOCGIWAP;
+			break;
+		case IEEE80211SOFTMAC_EVENT_DISASSOCIATED:
+			wrqu.data.length = 0;
+			wrqu.data.flags = 0;
+			memset(&wrqu, '\0', sizeof (union iwreq_data));
+			wrqu.ap_addr.sa_family = ARPHRD_ETHER;
+			we_event = SIOCGIWAP;
+			break;
+		default:
+			msg = event_descriptions[event];
+			wrqu.data.length = strlen(msg);
+			we_event = IWEVCUSTOM;
+			break;
+		}
+		wireless_send_event(mac->dev, we_event, &wrqu, msg);
 	}
 
 	if (!list_empty(&mac->events))

Re: [patch 1/3] softmac: return -EAGAIN from getscan while scanning

[Pete Zaitcev]

On Tue, 11 Apr 2006 10:58:06 +0200, johannes@sipsolutions.net wrote:

> Below patch was developed after discussion with Daniel Drake who
> mentioned to me that wireless tools expect an EAGAIN return from getscan
> so that they can wait for the scan to finish before printing out the
> results.

This sounds completely wrong. Do you guys remember the Subject: string
of this discussion by any chance?

It's very likely that tools do indeed loop when they see EAGAIN, but
this is a workaround, not the main mode of operation. It seems obvious
to me that the get method should wait until the scan results are
available. Perhaps the discussion had a specific scenario where
EAGAIN would make sense, but I cannot imagine what it might be.

-- Pete

Re: [patch 1/3] softmac: return -EAGAIN from getscan while scanning

[Johannes Berg]

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

On Thu, 2006-04-13 at 02:00 -0700, Pete Zaitcev wrote:

> This sounds completely wrong. Do you guys remember the Subject: string
> of this discussion by any chance?

Discussed offline at the wireless summit, sorry.

> It's very likely that tools do indeed loop when they see EAGAIN, but
> this is a workaround, not the main mode of operation. It seems obvious
> to me that the get method should wait until the scan results are
> available. Perhaps the discussion had a specific scenario where
> EAGAIN would make sense, but I cannot imagine what it might be.

Right, they do loop, but they don't have a method to indicate
completion. Thus we'd have to actually wait for the completion in the
kernel. Jean, what is the intended use here?

johannes

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

Re: [patch 1/3] softmac: return -EAGAIN from getscan while scanning

[Johannes Berg]

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

On Thu, 2006-04-13 at 11:06 +0200, Johannes Berg wrote:

> > It's very likely that tools do indeed loop when they see EAGAIN, but
> > this is a workaround, not the main mode of operation. It seems obvious
> > to me that the get method should wait until the scan results are
> > available. Perhaps the discussion had a specific scenario where
> > EAGAIN would make sense, but I cannot imagine what it might be.
> 
> Right, they do loop, but they don't have a method to indicate
> completion. Thus we'd have to actually wait for the completion in the
> kernel. Jean, what is the intended use here?

Hah, never mind, I guess we're just supposed to send SIOCGIWSCAN. Will
send a patch in a minute.

johannes

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

[patch 5] softmac: report when scanning has finished

[Johannes Berg]

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

Make softmac report a scan event when scanning has finished, that way
userspace can wait for the event to happen instead of polling for the
results.

Signed-off-by: Johannes Berg <johannes@sipsolutions.net>

---
This patch depends on the previous patch 4. If you want, I can resend
the whole series with proper patch x/5 headers etc. I don't know if
these can still go in before .17, but it'd be very good if they could,
they're essentially bug fixes for things I either didn't know or just
plain bugs :)

Index: wireless-2.6/net/ieee80211/softmac/ieee80211softmac_event.c
===================================================================
--- wireless-2.6.orig/net/ieee80211/softmac/ieee80211softmac_event.c	2006-04-13 02:42:59.000000000 +0200
+++ wireless-2.6/net/ieee80211/softmac/ieee80211softmac_event.c	2006-04-13 11:30:59.896998787 +0200
@@ -152,6 +152,12 @@
 			wrqu.ap_addr.sa_family = ARPHRD_ETHER;
 			we_event = SIOCGIWAP;
 			break;
+		case IEEE80211SOFTMAC_EVENT_SCAN_FINISHED:
+			wrqu.data.length = 0;
+			wrqu.data.flags = 0;
+			memset(&wrqu, '\0', sizeof (union iwreq_data));
+			we_event = SIOCGIWSCAN;
+			break;
 		default:
 			msg = event_descriptions[event];
 			wrqu.data.length = strlen(msg);


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

Re: [patch 1/3] softmac: return -EAGAIN from getscan while scanning

[Dan Williams]

On Thu, 2006-04-13 at 02:00 -0700, Pete Zaitcev wrote:
> On Tue, 11 Apr 2006 10:58:06 +0200, johannes@sipsolutions.net wrote:
> 
> > Below patch was developed after discussion with Daniel Drake who
> > mentioned to me that wireless tools expect an EAGAIN return from getscan
> > so that they can wait for the scan to finish before printing out the
> > results.
> 
> This sounds completely wrong. Do you guys remember the Subject: string
> of this discussion by any chance?

Maybe it's wrong now, but it's how the CLI tools have operated for quite
a while AFAIK.

There are two options for tools: (a) request scan and block on GIWSCAN
until it doesn't return EAGAIN, or (b) request a scan, enter a loop,
wait for the GIWSCAN netlink message to come back.  The point here is
that if you have to write a tool with 100 lines of netlink message
processing code _just_ to get the "scan done!" message, that's a bitch.
More complicated programs can obviously do this, but simple tools don't
want or need to.

airo, atmel, and orinoco all do this.  ipw does not, and prism54 does
not because it does background scanning.  I believe that the patch for
softmac/bcm43xx EAGAIN is correct.

Dan

Re: [patch 1/3] softmac: return -EAGAIN from getscan while scanning

[Dan Williams]

On Thu, 2006-04-13 at 11:34 +0200, Johannes Berg wrote:
> On Thu, 2006-04-13 at 11:06 +0200, Johannes Berg wrote:
> 
> > > It's very likely that tools do indeed loop when they see EAGAIN, but
> > > this is a workaround, not the main mode of operation. It seems obvious
> > > to me that the get method should wait until the scan results are
> > > available. Perhaps the discussion had a specific scenario where
> > > EAGAIN would make sense, but I cannot imagine what it might be.
> > 
> > Right, they do loop, but they don't have a method to indicate
> > completion. Thus we'd have to actually wait for the completion in the
> > kernel. Jean, what is the intended use here?
> 
> Hah, never mind, I guess we're just supposed to send SIOCGIWSCAN. Will
> send a patch in a minute.

No, you should be doing both really.  If you don't do EAGAIN, then tools
like iwlist and other simple ones that don't want to spend 100 lines of
code in netlink message processing just to receive the wireless event
won't work very well.

Dan

Re: [patch 5] softmac: report when scanning has finished

[Dan Williams]

On Thu, 2006-04-13 at 11:41 +0200, Johannes Berg wrote:
> Make softmac report a scan event when scanning has finished, that way
> userspace can wait for the event to happen instead of polling for the
> results.
> 
> Signed-off-by: Johannes Berg <johannes@sipsolutions.net>

ACK, good catch.

Signed-off-by: Dan Williams <dcbw@redhat.com>

> ---
> This patch depends on the previous patch 4. If you want, I can resend
> the whole series with proper patch x/5 headers etc. I don't know if
> these can still go in before .17, but it'd be very good if they could,
> they're essentially bug fixes for things I either didn't know or just
> plain bugs :)
> 
> Index: wireless-2.6/net/ieee80211/softmac/ieee80211softmac_event.c
> ===================================================================
> --- wireless-2.6.orig/net/ieee80211/softmac/ieee80211softmac_event.c	2006-04-13 02:42:59.000000000 +0200
> +++ wireless-2.6/net/ieee80211/softmac/ieee80211softmac_event.c	2006-04-13 11:30:59.896998787 +0200
> @@ -152,6 +152,12 @@
>  			wrqu.ap_addr.sa_family = ARPHRD_ETHER;
>  			we_event = SIOCGIWAP;
>  			break;
> +		case IEEE80211SOFTMAC_EVENT_SCAN_FINISHED:
> +			wrqu.data.length = 0;
> +			wrqu.data.flags = 0;
> +			memset(&wrqu, '\0', sizeof (union iwreq_data));
> +			we_event = SIOCGIWSCAN;
> +			break;
>  		default:
>  			msg = event_descriptions[event];
>  			wrqu.data.length = strlen(msg);
> 

Re: [patch 1/3] softmac: return -EAGAIN from getscan while scanning

[Johannes Berg]

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

On Thu, 2006-04-13 at 08:12 -0400, Dan Williams wrote:

> Maybe it's wrong now, but it's how the CLI tools have operated for quite
> a while AFAIK.

Yes, and they don't seem to care about the netlink messages.

But the question is rather -- should we block the program inside the
kernel and only return from "get scan" after the scan is actually done,
rather than returning "we can't give you anything now, try later". We
could do that too, but then the program would be blocked inside the
kernel without any chance of saying "bah, this takes too long, I'll
ignore it".

> There are two options for tools: (a) request scan and block on GIWSCAN
> until it doesn't return EAGAIN, or (b) request a scan, enter a loop,
> wait for the GIWSCAN netlink message to come back.  The point here is
> that if you have to write a tool with 100 lines of netlink message
> processing code _just_ to get the "scan done!" message, that's a bitch.
> More complicated programs can obviously do this, but simple tools don't
> want or need to.

Yeah, I just implemented that in softmac too (patch 5 of my set), so you
can just request a scan and do nothing until you are notified that the
netlink message came in that scanning is done.

> airo, atmel, and orinoco all do this.  ipw does not, and prism54 does
> not because it does background scanning.  I believe that the patch for
> softmac/bcm43xx EAGAIN is correct.

Good. I hope that these can go in before 2.6.17.

johannes

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

Re: [patch 1/3] softmac: return -EAGAIN from getscan while scanning

[Jouni Malinen]

On Thu, Apr 13, 2006 at 02:19:26PM +0200, Johannes Berg wrote:

> But the question is rather -- should we block the program inside the
> kernel and only return from "get scan" after the scan is actually done,
> rather than returning "we can't give you anything now, try later". We
> could do that too, but then the program would be blocked inside the
> kernel without any chance of saying "bah, this takes too long, I'll
> ignore it".

Please don't. That could be blocking an ioctl call for couple of seconds
and would be quite horrible for single threaded programs. It is possible
to wait for scan completion events, but what if some other program were
to request a new scan between the completion event and the attempt to
read the previous scan results..

-- 
Jouni Malinen                                            PGP id EFC895FA

Re: [patch 1/3] softmac: return -EAGAIN from getscan while scanning

[Jean Tourrilhes]

On Thu, Apr 13, 2006 at 08:14:01AM -0400, Dan Williams wrote:
> On Thu, 2006-04-13 at 11:34 +0200, Johannes Berg wrote:
> > On Thu, 2006-04-13 at 11:06 +0200, Johannes Berg wrote:
> > 
> > > > It's very likely that tools do indeed loop when they see EAGAIN, but
> > > > this is a workaround, not the main mode of operation. It seems obvious
> > > > to me that the get method should wait until the scan results are
> > > > available. Perhaps the discussion had a specific scenario where
> > > > EAGAIN would make sense, but I cannot imagine what it might be.
> > > 
> > > Right, they do loop, but they don't have a method to indicate
> > > completion. Thus we'd have to actually wait for the completion in the
> > > kernel. Jean, what is the intended use here?
> > 
> > Hah, never mind, I guess we're just supposed to send SIOCGIWSCAN. Will
> > send a patch in a minute.
> 
> No, you should be doing both really.  If you don't do EAGAIN, then tools
> like iwlist and other simple ones that don't want to spend 100 lines of
> code in netlink message processing just to receive the wireless event
> won't work very well.
> 
> Dan

	Yes, Dan is 100% correct.

	Jean

Re: [patch 1/3] softmac: return -EAGAIN from getscan while scanning

[Pete Zaitcev]

On Thu, 13 Apr 2006 08:12:55 -0400, Dan Williams <dcbw@redhat.com> wrote:

> > This sounds completely wrong. Do you guys remember the Subject: string
> > of this discussion by any chance?

> There are two options for tools: (a) request scan and block on GIWSCAN
> until it doesn't return EAGAIN, or (b) request a scan, enter a loop,
> wait for the GIWSCAN netlink message to come back.  The point here is
> that if you have to write a tool with 100 lines of netlink message
> processing code _just_ to get the "scan done!" message, that's a bitch.
> More complicated programs can obviously do this, but simple tools don't
> want or need to.

Do you realize that "block" means "enterering the kernel and calling
schedule()", which is exactly what is NOT happening in the patch?
I would not mind if the tools "blocked in GIWSCAN", only until
it returns success and not EGAIN.

With EAGAIN returned, you have to a) block elsewhere and not in
GIWSCAN (perhaps in sleep(2) or select(2)), or b) not block at all
and loop.

> I believe that the patch for softmac/bcm43xx EAGAIN is correct.

It is correct, no doubt. The discussion is about wether it is
desirable.

-- Pete

Re: [patch 1/3] softmac: return -EAGAIN from getscan while scanning

[Pete Zaitcev]

On Thu, 13 Apr 2006 09:00:51 -0700, "Jouni Malinen" <jkm@devicescape.com> wrote:

> That could be blocking an ioctl call for couple of seconds
> and would be quite horrible for single threaded programs.

I would say that waiting for couple of seconds in the kernel would
be quite wonderful for single threaded programs, when you consider
the alternative. I can guess now what your concern is, even though
you failed to articulate it: a single-threaded GUI application,
which cannot respond to events when blocked in getting scan results.
If that's the case, we should be looking at having both blocking
and non-blocking calls to fetch scan results.

> [...] but what if some other program were
> to request a new scan between the completion event and the attempt to
> read the previous scan results..

I do not see how this is relevant.

-- Pete

Re: [patch 1/3] softmac: return -EAGAIN from getscan while scanning

[Jouni Malinen]

On Thu, Apr 13, 2006 at 03:28:53PM -0700, Pete Zaitcev wrote:
> On Thu, 13 Apr 2006 09:00:51 -0700, "Jouni Malinen" <jkm@devicescape.com> wrote:
> 
> > That could be blocking an ioctl call for couple of seconds
> > and would be quite horrible for single threaded programs.
> 
> I would say that waiting for couple of seconds in the kernel would
> be quite wonderful for single threaded programs, when you consider
> the alternative. I can guess now what your concern is, even though
> you failed to articulate it: a single-threaded GUI application,
> which cannot respond to events when blocked in getting scan results.
> If that's the case, we should be looking at having both blocking
> and non-blocking calls to fetch scan results.

No, my main concern was single-threaded design in wpa_suppliant.. If the
ioctl call is blocking, I would need to create a new (well, the first
additional) thread just for this use. Without that, the blocking call
would also block all control interface commands (interaction with
external programs) and controlling of other interfaces (if more than one
is used).

> > [...] but what if some other program were
> > to request a new scan between the completion event and the attempt to
> > read the previous scan results..
> 
> I do not see how this is relevant.

That would make the application wait even if it properly waited for the
scan complete event before reading the scan result. If the get-results
call is blocking, the single-threaded application simply don't have any
easy way of getting the results while being able to do something else
while waiting for the scan to complete. Furthermore, blocking ioctl
handlers is not really something I would like to see in the kernel..
Aren't there some locks/semaphores/etc. kept for some cases?

-- 
Jouni Malinen                                            PGP id EFC895FA

Re: [patch 1/3] softmac: return -EAGAIN from getscan while scanning

[Stephen Hemminger]

On Thu, 13 Apr 2006 15:45:22 -0700
"Jouni Malinen" <jkm@devicescape.com> wrote:

> On Thu, Apr 13, 2006 at 03:28:53PM -0700, Pete Zaitcev wrote:
> > On Thu, 13 Apr 2006 09:00:51 -0700, "Jouni Malinen" <jkm@devicescape.com> wrote:
> > 
> > > That could be blocking an ioctl call for couple of seconds
> > > and would be quite horrible for single threaded programs.
> > 
> > I would say that waiting for couple of seconds in the kernel would
> > be quite wonderful for single threaded programs, when you consider
> > the alternative. I can guess now what your concern is, even though
> > you failed to articulate it: a single-threaded GUI application,
> > which cannot respond to events when blocked in getting scan results.
> > If that's the case, we should be looking at having both blocking
> > and non-blocking calls to fetch scan results.
> 
> No, my main concern was single-threaded design in wpa_suppliant.. If the
> ioctl call is blocking, I would need to create a new (well, the first
> additional) thread just for this use. Without that, the blocking call
> would also block all control interface commands (interaction with
> external programs) and controlling of other interfaces (if more than one
> is used).
> 
> > > [...] but what if some other program were
> > > to request a new scan between the completion event and the attempt to
> > > read the previous scan results..
> > 
> > I do not see how this is relevant.
> 
> That would make the application wait even if it properly waited for the
> scan complete event before reading the scan result. If the get-results
> call is blocking, the single-threaded application simply don't have any
> easy way of getting the results while being able to do something else
> while waiting for the scan to complete. Furthermore, blocking ioctl
> handlers is not really something I would like to see in the kernel..
> Aren't there some locks/semaphores/etc. kept for some cases?
> 

Sounds like you want a message interface like netlink, not ioctl's.

Re: [patch 1/3] softmac: return -EAGAIN from getscan while scanning

[Pete Zaitcev]

On Thu, 13 Apr 2006 15:45:22 -0700, "Jouni Malinen" <jkm@devicescape.com> wrote:

> > [...] I can guess now what your concern is, even though
> > you failed to articulate it: a single-threaded GUI application,
> > which cannot respond to events when blocked in getting scan results.
> > If that's the case, we should be looking at having both blocking
> > and non-blocking calls to fetch scan results.
> 
> No, my main concern was single-threaded design in wpa_suppliant.. If the
> ioctl call is blocking, I would need to create a new (well, the first
> additional) thread just for this use. Without that, the blocking call
> would also block all control interface commands (interaction with
> external programs) and controlling of other interfaces (if more than one
> is used).

I see... Thanks for explainig it.

> [...] Furthermore, blocking ioctl
> handlers is not really something I would like to see in the kernel..
> Aren't there some locks/semaphores/etc. kept for some cases?

I think you are taking this concept too far. Consider things like
an ioctl to seek a magnetic tape N files forward, or an ioctl to
flush characters to a terminal. They seem to work fine in blocking
mode. And no, we do not have a generic API to expose semaphores
(I suppose you did not mean SYSV IPC above :-).

-- Pete

Re: [patch 1/3] softmac: return -EAGAIN from getscan while scanning

[Johannes Berg]

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

On Thu, 2006-04-13 at 09:13 -0700, Jean Tourrilhes wrote:

> 	Yes, Dan is 100% correct.

Right, slightly later I posted a patch to do it that way. After everyone
else has discussed, do we agree to leave the patches as they are now?

johannes

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

Re: [patch 1/3] softmac: return -EAGAIN from getscan while scanning

[Dan Williams]

On Sat, 2006-04-15 at 21:24 +0200, Johannes Berg wrote:
> On Thu, 2006-04-13 at 09:13 -0700, Jean Tourrilhes wrote:
> 
> > 	Yes, Dan is 100% correct.
> 
> Right, slightly later I posted a patch to do it that way. After everyone
> else has discussed, do we agree to leave the patches as they are now?

Current patch set looks good to me.

Dan