Can we ignore frames with invalid BSSID in IBSS mode?

Can we ignore frames with invalid BSSID in IBSS mode?

[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.

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

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: Can we ignore frames with invalid BSSID in IBSS mode?

[Johannes Berg]

On Fri, 2015-09-25 at 16:00 -0700, Ben Greear wrote:
> 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?
> 

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

johannes

Re: Can we ignore frames with invalid BSSID in IBSS mode?

[Ben Greear]

On 09/29/2015 11:46 PM, Johannes Berg wrote:
> On Fri, 2015-09-25 at 16:00 -0700, Ben Greear wrote:
>> 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?
>>
>
> 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


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: Can we ignore frames with invalid BSSID in IBSS mode?

[Johannes Berg]

On Wed, 2015-09-30 at 08:07 -0700, Ben Greear wrote:
> 
> On 09/29/2015 11:46 PM, Johannes Berg wrote:
> > On Fri, 2015-09-25 at 16:00 -0700, Ben Greear wrote:
> > > 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?
> > > 
> > 
> > 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?

That seems reasonable.

> 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

Re: Can we ignore frames with invalid BSSID in IBSS mode?

[Ben Greear]

On 09/30/2015 08:17 AM, Johannes Berg wrote:
> On Wed, 2015-09-30 at 08:07 -0700, Ben Greear wrote:
>>
>> On 09/29/2015 11:46 PM, Johannes Berg wrote:
>>> On Fri, 2015-09-25 at 16:00 -0700, Ben Greear wrote:
>>>> 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?
>>>>
>>>
>>> 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?
>
> 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

>
>> 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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: Can we ignore frames with invalid BSSID in IBSS mode?

[Johannes Berg]

On Wed, 2015-09-30 at 08:44 -0700, Ben Greear wrote:
> 
> Any idea how this could be done in the stack instead of the driver?

I don't see why it should be?

> 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

Re: Can we ignore frames with invalid BSSID in IBSS mode?

[Ben Greear]

On 09/30/2015 10:14 AM, Johannes Berg wrote:
> On Wed, 2015-09-30 at 08:44 -0700, Ben Greear wrote:
>>
>> Any idea how this could be done in the stack instead of the driver?
>
> I don't see why it should be?
>
>> 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

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: Can we ignore frames with invalid BSSID in IBSS mode?

[Johannes Berg]

On Wed, 2015-09-30 at 10:20 -0700, Ben Greear wrote:
> 
> 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.

> 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

Re: Can we ignore frames with invalid BSSID in IBSS mode?

[Ben Greear]

On 09/30/2015 11:30 AM, Johannes Berg wrote:
> On Wed, 2015-09-30 at 10:20 -0700, Ben Greear wrote:
>>
>> 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.
>
>> 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.

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


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: Can we ignore frames with invalid BSSID in IBSS mode?

[Felix Fietkau]

On 2015-09-30 20:34, Ben Greear wrote:
> On 09/30/2015 11:30 AM, Johannes Berg wrote:
>> On Wed, 2015-09-30 at 10:20 -0700, Ben Greear wrote:
>>>
>>> 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.
>>
>>> 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.
> 
> 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

Re: Can we ignore frames with invalid BSSID in IBSS mode?

[Nicolas Cavallari]

On 26/09/2015 01:00, Ben Greear wrote:
> 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?

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.