Skip to contentSkip to conversation

Can we ignore frames with invalid BSSID in IBSS mode?

DISCUSSION Ben Greear 4 participants 11 messages Sep 25, 2015 – Sep 30, 2015 Qualcomm Atheros ATH10K wireless driver

lore
Share & tools
lore

Ben Greear

author
#0 lore raw

It seems that ath10k ar988X hardware has a bug where the BSSID for IBSS AMSDU frames is all zeros. The 'main' 636 ath10k firmware does not seem to use AMSDUs for IBSS, and when I enable it in my CT firmware, then I see the breakage. So, I suspect it is not just a simple software/firmware bug.

If I simply ignore the bssid_match check in ieee80211_accept_frame, then it seems everything runs fine.

So, I'm curious if anyone knows what sorts of bad things could happen if the bssid_match check is ignored? Maybe bcast/mcast frames could be accepted when they shouldn't be in certain cases?

Thanks,
Ben

Signature · 2 lines
Ben Greear <[email protected]>
Candela Technologies Inc  http://www.candelatech.com
#1 lore raw
Quoted 6 lines · Ben Greear

It seems that ath10k ar988X hardware has a bug where the BSSID for IBSS AMSDU frames is all zeros. The 'main' 636 ath10k firmware does not seem to use AMSDUs for IBSS, and when I enable it in my CT firmware, then I see the breakage. So, I suspect it is not just a simple software/firmware bug.

Quoted 7 lines

If I simply ignore the bssid_match check in ieee80211_accept_frame, then it seems everything runs fine.

So, I'm curious if anyone knows what sorts of bad things could happen if the bssid_match check is ignored? Maybe bcast/mcast frames could be accepted when they shouldn't be in certain cases?

You could end up accepting multicast frames from a different, overlapping, BSS? Seems like a bad idea.

johannes

author
#2 lore raw
Quoted 10 lines · Johannes Berg
Quoted 9 lines · Ben Greear

It seems that ath10k ar988X hardware has a bug where the BSSID for IBSS AMSDU frames is all zeros. The 'main' 636 ath10k firmware does not seem to use AMSDUs for IBSS, and when I enable it in my CT firmware, then I see the breakage. So, I suspect it is not just a simple software/firmware bug.

If I simply ignore the bssid_match check in ieee80211_accept_frame, then it seems everything runs fine.

Quoted 7 lines
Quoted 4 lines

So, I'm curious if anyone knows what sorts of bad things could happen if the bssid_match check is ignored? Maybe bcast/mcast frames could be accepted when they shouldn't be in certain cases?

You could end up accepting multicast frames from a different, overlapping, BSS? Seems like a bad idea.

It's definitely not a great idea.

In my testing, I always see the first frame of the AMPDU have a proper IBSS BSSID. Any idea if it would be OK (and even possible) for the driver or stack to detect this and save the BSSID aside for the subsequent frames?

Its not clear to me whether the rest of the AMPDU frames could somehow be interleaved with frames from a different BSSID?

Thanks,
Ben

Signature · 2 lines
Ben Greear <[email protected]>
Candela Technologies Inc  http://www.candelatech.com
#3 lore raw
Quoted 20 lines · Ben Greear
Quoted 10 lines · Johannes Berg
Quoted 9 lines · Ben Greear

It seems that ath10k ar988X hardware has a bug where the BSSID for IBSS AMSDU frames is all zeros. The 'main' 636 ath10k firmware does not seem to use AMSDUs for IBSS, and when I enable it in my CT firmware, then I see the breakage. So, I suspect it is not just a simple software/firmware bug.

If I simply ignore the bssid_match check in ieee80211_accept_frame, then it seems everything runs fine.

Quoted 7 lines
Quoted 4 lines

So, I'm curious if anyone knows what sorts of bad things could happen if the bssid_match check is ignored? Maybe bcast/mcast frames could be accepted when they shouldn't be in certain cases?

You could end up accepting multicast frames from a different, overlapping, BSS? Seems like a bad idea.

Quoted 6 lines

It's definitely not a great idea.

In my testing, I always see the first frame of the AMPDU have a proper IBSS BSSID. Any idea if it would be OK (and even possible) for the driver or stack to detect this and save the BSSID aside for the subsequent frames?

That seems reasonable.

Quoted 3 lines

Its not clear to me whether the rest of the AMPDU frames could somehow be interleaved with frames from a different BSSID?

They can't be, at least not without some very strange hacks on the transmitter.

johannes

author
#4 lore raw
Quoted 21 lines · Johannes Berg
Quoted 20 lines · Ben Greear
Quoted 17 lines · Johannes Berg
Quoted 6 lines · Ben Greear

It seems that ath10k ar988X hardware has a bug where the BSSID for IBSS AMSDU frames is all zeros. The 'main' 636 ath10k firmware does not seem to use AMSDUs for IBSS, and when I enable it in my CT firmware, then I see the breakage. So, I suspect it is not just a simple software/firmware bug.

Quoted 7 lines

If I simply ignore the bssid_match check in ieee80211_accept_frame, then it seems everything runs fine.

So, I'm curious if anyone knows what sorts of bad things could happen if the bssid_match check is ignored? Maybe bcast/mcast frames could be accepted when they shouldn't be in certain cases?

You could end up accepting multicast frames from a different, overlapping, BSS? Seems like a bad idea.

Quoted 8 lines
Quoted 6 lines

It's definitely not a great idea.

In my testing, I always see the first frame of the AMPDU have a proper IBSS BSSID. Any idea if it would be OK (and even possible) for the driver or stack to detect this and save the BSSID aside for the subsequent frames?

That seems reasonable.

Any idea how this could be done in the stack instead of the driver?

The problem is that this is a receiver-side issue, so even if I manage to hack the ath10k firmware or driver rx logic, it would not fix any other IBSS peer connected to ath10k peer.

Thanks,
Ben

Quoted 14 lines
Quoted 4 lines

Its not clear to me whether the rest of the AMPDU frames could somehow be interleaved with frames from a different BSSID?

They can't be, at least not without some very strange hacks on the transmitter.

johannes
--
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

Signature · 2 lines
Ben Greear <[email protected]>
Candela Technologies Inc  http://www.candelatech.com
#5 lore raw
Quoted 2 lines · Ben Greear

Any idea how this could be done in the stack instead of the driver?

I don't see why it should be?

Quoted 4 lines

The problem is that this is a receiver-side issue, so even if I manage to hack the ath10k firmware or driver rx logic, it would not fix any other IBSS peer connected to ath10k peer.

You mean it's a transmitter-side issue? In that case you should probably simply disable aggregation on the broken transmitter ...

johannes

author
#6 lore raw
Quoted 6 lines · Johannes Berg
Quoted 2 lines · Ben Greear

Any idea how this could be done in the stack instead of the driver?

I don't see why it should be?

Quoted 7 lines
Quoted 4 lines

The problem is that this is a receiver-side issue, so even if I manage to hack the ath10k firmware or driver rx logic, it would not fix any other IBSS peer connected to ath10k peer.

You mean it's a transmitter-side issue? In that case you should probably simply disable aggregation on the broken transmitter ...

Yes, it is a transmitter side problem, and A-MSDU on IBSS
is disabled by default in all ath10k firmware versions that I am aware of.

I was hoping there might be a way to allow A-MSDU + IBSS + ath10k to work in future kernels without applying out-of-tree kernel hacks. This would let people with appropriate firmware enable IBSS + A-MSDU for added performance in cases where they knew the peer could support the needed work-around.

I don't think it is worth a lot of effort, but if it were relatively simple to fix, then maybe it is worth it.

Thanks,
Ben

Signature · 2 lines
Ben Greear <[email protected]>
Candela Technologies Inc  http://www.candelatech.com
#7 lore raw
Quoted 3 lines · Ben Greear

Yes, it is a transmitter side problem, and A-MSDU on IBSS
is disabled by default in all ath10k firmware versions that I am aware of.

Right.

Quoted 9 lines

I was hoping there might be a way to allow A-MSDU + IBSS + ath10k to work in future kernels without applying out-of-tree kernel hacks. This would let people with appropriate firmware enable IBSS + A-MSDU for added performance in cases where they knew the peer could support the needed work-around.

I don't think it is worth a lot of effort, but if it were relatively simple to fix, then maybe it is worth it.

Had it been a receiver-side issue, then it'd seem reasonable to work around it. But it being a transmitter-side issue it doesn't really seem so - *every* possible peer would have to be adjusted, and some might not even be able to get adjusted (e.g. devices that have A-MSDU deaggregation in hardware/firmware) ...

So to do that properly you'd have to advertise some sort of quirk vendor IE, and all that, which seems excessive given the limited use.

johannes

author
#8 lore raw
Quoted 17 lines · Johannes Berg
Quoted 3 lines · Ben Greear

Yes, it is a transmitter side problem, and A-MSDU on IBSS
is disabled by default in all ath10k firmware versions that I am aware of.

Right.

Quoted 9 lines

I was hoping there might be a way to allow A-MSDU + IBSS + ath10k to work in future kernels without applying out-of-tree kernel hacks. This would let people with appropriate firmware enable IBSS + A-MSDU for added performance in cases where they knew the peer could support the needed work-around.

I don't think it is worth a lot of effort, but if it were relatively simple to fix, then maybe it is worth it.

Quoted 8 lines

Had it been a receiver-side issue, then it'd seem reasonable to work around it. But it being a transmitter-side issue it doesn't really seem so - *every* possible peer would have to be adjusted, and some might not even be able to get adjusted (e.g. devices that have A-MSDU deaggregation in hardware/firmware) ...

So to do that properly you'd have to advertise some sort of quirk vendor IE, and all that, which seems excessive given the limited use.

I was figuring the main users of this would be people rolling out IBSS mesh networks and such, and they might have good knowledge of exactly what peers will be used.

It is a small enough hack to the stack to just ignore the BSSID for adhoc, and since CT firmware related patches are not accepted upstream anyway, I guess anyone doing this is likely running custom patches already.

Thanks,
Ben

Signature · 2 lines
Ben Greear <[email protected]>
Candela Technologies Inc  http://www.candelatech.com
#9 lore raw
Quoted 27 lines · Ben Greear
Quoted 17 lines · Johannes Berg
Quoted 3 lines · Ben Greear

Yes, it is a transmitter side problem, and A-MSDU on IBSS
is disabled by default in all ath10k firmware versions that I am aware of.

Right.

Quoted 9 lines

I was hoping there might be a way to allow A-MSDU + IBSS + ath10k to work in future kernels without applying out-of-tree kernel hacks. This would let people with appropriate firmware enable IBSS + A-MSDU for added performance in cases where they knew the peer could support the needed work-around.

I don't think it is worth a lot of effort, but if it were relatively simple to fix, then maybe it is worth it.

Quoted 8 lines

Had it been a receiver-side issue, then it'd seem reasonable to work around it. But it being a transmitter-side issue it doesn't really seem so - *every* possible peer would have to be adjusted, and some might not even be able to get adjusted (e.g. devices that have A-MSDU deaggregation in hardware/firmware) ...

So to do that properly you'd have to advertise some sort of quirk vendor IE, and all that, which seems excessive given the limited use.

Quoted 8 lines

I was figuring the main users of this would be people rolling out IBSS mesh networks and such, and they might have good knowledge of exactly what peers will be used.

It is a small enough hack to the stack to just ignore the BSSID for adhoc, and since CT firmware related patches are not accepted upstream anyway, I guess anyone doing this is likely running custom patches already.

I think instead of making a bunch of assumptions about who is going to use this for what, you should just leave A-MSDU disabled for IBSS.

If you present this as a way to improve performance, users will probably mindlessly enable it without trying to understand why it wasn't enabled by default. Afterwards, they will create annoying and hard-to-debug bug reports for you and other people to waste time on.

- Felix

#10 lore raw
In reply to Ben Greear
Quoted 6 lines · Ben Greear

It seems that ath10k ar988X hardware has a bug where the BSSID for IBSS AMSDU frames is all zeros. The 'main' 636 ath10k firmware does not seem to use AMSDUs for IBSS, and when I enable it in my CT firmware, then I see the breakage. So, I suspect it is not just a simple software/firmware bug.

Quoted 6 lines

If I simply ignore the bssid_match check in ieee80211_accept_frame, then it seems everything runs fine.

So, I'm curious if anyone knows what sorts of bad things could happen if the bssid_match check is ignored? Maybe bcast/mcast frames could be accepted when they shouldn't be in certain cases?

Given that all it takes for an IBSS station to be added as a neighbor is to see a frame from an unknown station with the same BSSID (ieee80211_ibss_rx_no_sta(), just 10 lines below), your ath10k chip will consider all stations for neighboring IBSS to be part of this BSS.

If RSN is used, or any other protocol/program that watches the list of neighbors, then your station will try to communicate with them. Good (those who don't ignore bssid checks) neighbors will normally drop the frames. But if you deploy several machines ignoring the bssid check, then they cannot run two concurrent IBSS networks.

Also, if there is a limit on how much stations the ath10k hardware can handle, then that limit have a higher chance of being reached.