[PATCH] mac80211: Restore rx.fc before every invocation of ieee80211_invoke_rx_handlers

[PATCH] mac80211: Restore rx.fc before every invocation of ieee80211_invoke_rx_handlers

[Helmut Schaa]

This patch fixes a problem with rx handling on multiple interfaces. Especially 
when using hardware-scanning and a wireless driver (i.e. iwlwifi) which is 
able to receive data while scanning.

The rx handlers can modify the skb and the frame control field (see 
ieee80211_rx_h_remove_qos_control) but since every interface gets its own 
copy of the skb each should get its own copy of rx.fc too.

In my case the wlan0-interface did not remove the qos-control from the frame 
because the corresponding flag in rx.fc was already removed while processing 
the frame on the master interface. Therefore somehow corrupted frames were 
passed to the userspace.

Signed-off-by: Helmut Schaa <hschaa@suse.de>
---
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 362e8e5..08a6905 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -1723,6 +1723,7 @@ void __ieee80211_rx(struct ieee80211_hw *hw, struct 
sk_buff *skb,
 				       prev->dev->name);
 			continue;
 		}
+		rx.fc = le16_to_cpu(hdr->frame_control);
 		rx.skb = skb_new;
 		rx.dev = prev->dev;
 		rx.sdata = prev;
@@ -1731,6 +1732,7 @@ void __ieee80211_rx(struct ieee80211_hw *hw, struct 
sk_buff *skb,
 		prev = sdata;
 	}
 	if (prev) {
+		rx.fc = le16_to_cpu(hdr->frame_control);
 		rx.skb = skb;
 		rx.dev = prev->dev;
 		rx.sdata = prev;

Re: [PATCH] mac80211: Restore rx.fc before every invocation of ieee80211_invoke_rx_handlers

[Johannes Berg]

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


On Fri, 2007-12-21 at 15:16 +0100, Helmut Schaa wrote:
> This patch fixes a problem with rx handling on multiple interfaces. Especially 
> when using hardware-scanning and a wireless driver (i.e. iwlwifi) which is 
> able to receive data while scanning.

Can you explain how you even got multiple interfaces with iwlwifi? The
patch looks (superficially) correct, but I'm a bit confused.

johannes

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

Re: [PATCH] mac80211: Restore rx.fc before every invocation of ieee80211_invoke_rx_handlers

[Helmut Schaa]

Am So 23 Dez 2007 10:24:39 CET schrieb Johannes Berg  
<johannes@sipsolutions.net>:

>
> On Fri, 2007-12-21 at 15:16 +0100, Helmut Schaa wrote:
>> This patch fixes a problem with rx handling on multiple interfaces.  
>>  Especially
>> when using hardware-scanning and a wireless driver (i.e. iwlwifi) which is
>> able to receive data while scanning.
>
> Can you explain how you even got multiple interfaces with iwlwifi? The
> patch looks (superficially) correct, but I'm a bit confused.

I was only referring to wmaster0 and wlan0 as two interfaces. While a scan
is in progress the master interface (wmaster0) processes all frames related
to scanning and it happened to me that data-frames got dropped on wmaster0
(which is obviously correct) and due to the frame control being modified
wlan0 was not able to process the frame correctly anymore.

Helmut

Re: [PATCH] mac80211: Restore rx.fc before every invocation of ieee80211_invoke_rx_handlers

[Johannes Berg]

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


> >> This patch fixes a problem with rx handling on multiple interfaces.  
> >>  Especially
> >> when using hardware-scanning and a wireless driver (i.e. iwlwifi) which is
> >> able to receive data while scanning.
> >
> > Can you explain how you even got multiple interfaces with iwlwifi? The
> > patch looks (superficially) correct, but I'm a bit confused.
> 
> I was only referring to wmaster0 and wlan0 as two interfaces. While a scan
> is in progress the master interface (wmaster0) processes all frames related
> to scanning and it happened to me that data-frames got dropped on wmaster0
> (which is obviously correct) and due to the frame control being modified
> wlan0 was not able to process the frame correctly anymore.

Ah, that must be because of the stuff Zhu Yi did wrt. hw scanning. Could
you take a look too please? The patch looks pretty much ok to me and I
think it'd be required with say VLAN interfaces as well.

johannes

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

Re: [PATCH] mac80211: Restore rx.fc before every invocation of ieee80211_invoke_rx_handlers

[Helmut Schaa]

Am So 23 Dez 2007 14:48:26 CET schrieb Johannes Berg  
<johannes@sipsolutions.net>:

>
>> >> This patch fixes a problem with rx handling on multiple interfaces.
>> >>  Especially
>> >> when using hardware-scanning and a wireless driver (i.e.   
>> iwlwifi) which is
>> >> able to receive data while scanning.
>> >
>> > Can you explain how you even got multiple interfaces with iwlwifi? The
>> > patch looks (superficially) correct, but I'm a bit confused.
>>
>> I was only referring to wmaster0 and wlan0 as two interfaces. While a scan
>> is in progress the master interface (wmaster0) processes all frames related
>> to scanning and it happened to me that data-frames got dropped on wmaster0
>> (which is obviously correct) and due to the frame control being modified
>> wlan0 was not able to process the frame correctly anymore.
>
> Ah, that must be because of the stuff Zhu Yi did wrt. hw scanning. Could

Exactly.

> you take a look too please? The patch looks pretty much ok to me and I
> think it'd be required with say VLAN interfaces as well.

The patch looks fine to me too and with the additional patch I was  
able to receive data while a scan is in progress (hw_scan only of  
course).

Helmut

Re: [PATCH] mac80211: Restore rx.fc before every invocation of ieee80211_invoke_rx_handlers

[Zhu Yi]

On Sun, 2007-12-23 at 14:48 +0100, Johannes Berg wrote:
> Ah, that must be because of the stuff Zhu Yi did wrt. hw scanning.
> Could you take a look too please? The patch looks pretty much ok to me
> and I think it'd be required with say VLAN interfaces as well.

The patch looks fine to me. Hardware scanning is a trigger that makes
the condition more likely to happen, the patch itself fixes the more
general problem for multiple interfaces.

An off-topic question: should wmaster0 continue to execute more rx
handlers (i.e ieee80211_rx_h_remove_qos_control) even if it knows it
will drop them finially?

Thanks,
-yi

Re: [PATCH] mac80211: Restore rx.fc before every invocation of ieee80211_invoke_rx_handlers

[Johannes Berg]

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


> The patch looks fine to me. Hardware scanning is a trigger that makes
> the condition more likely to happen, the patch itself fixes the more
> general problem for multiple interfaces.

Yeah, I think so too.

> An off-topic question: should wmaster0 continue to execute more rx
> handlers (i.e ieee80211_rx_h_remove_qos_control) even if it knows it
> will drop them finially?

I don't think it does, does it? In prepare_for_handlers frames aren't
sent to the master device while not scanning.

johannes

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

Re: [PATCH] mac80211: Restore rx.fc before every invocation of ieee80211_invoke_rx_handlers

[Helmut Schaa]

Am Mo 24 Dez 2007 11:23:51 CET schrieb Johannes Berg  
<johannes@sipsolutions.net>:

>
>> The patch looks fine to me. Hardware scanning is a trigger that makes
>> the condition more likely to happen, the patch itself fixes the more
>> general problem for multiple interfaces.
>
> Yeah, I think so too.
>
>> An off-topic question: should wmaster0 continue to execute more rx
>> handlers (i.e ieee80211_rx_h_remove_qos_control) even if it knows it
>> will drop them finially?
>
> I don't think it does, does it? In prepare_for_handlers frames aren't
> sent to the master device while not scanning.

It does so only during a scan. In that case the frame gets first
dropped in ieee80211_rx_h_data on the master device.

Helmut

Re: [PATCH] mac80211: Restore rx.fc before every invocation of ieee80211_invoke_rx_handlers

[Zhu Yi]

On Mon, 2007-12-24 at 11:23 +0100, Johannes Berg wrote:
> I don't think it does, does it? In prepare_for_handlers frames aren't
> sent to the master device while not scanning.

Should we also discard frames to mdev if ((rx->fc &
IEEE80211_FCTL_FTYPE) == IEEE80211_FTYPE_DATA)?

Thanks,
-yi

Re: [PATCH] mac80211: Restore rx.fc before every invocation of ieee80211_invoke_rx_handlers

[Johannes Berg]

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


On Tue, 2007-12-25 at 11:31 +0800, Zhu Yi wrote:
> On Mon, 2007-12-24 at 11:23 +0100, Johannes Berg wrote:
> > I don't think it does, does it? In prepare_for_handlers frames aren't
> > sent to the master device while not scanning.
> 
> Should we also discard frames to mdev if ((rx->fc &
> IEEE80211_FCTL_FTYPE) == IEEE80211_FTYPE_DATA)?

I guess we should, yeah. In fact, I'd like to make the scan code
short-cut and not go through the master device so we can completely
remove RX on the master.

johannes

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