Re: Setting single rate in ath10k broken by "reject/clear user rate mask if not usable"

[Ben Greear <greearb@candelatech.com>]

On 10/25/2017 08:17 AM, Johannes Berg wrote:
> Hi,
>
> So I fixed the part about the rate setting calling into the driver
> before rejecting.
>
>> If a user specifies a specific rate or rate-set, then I do not think we
>> should quietly change it out from under them.  So, we could check at the
>> time the rate-set is applied (all current peers).  We can reject the change
>> at that point if one of the peers does not support any common rates.
>> And, whenever a peer tries to be associated, we can check that there is some common rateset
>> in place.  If there is no common rateset, then reject the association
>> one way or another.
>
> It's not trivial to do in the kernel, but if we reject adding the
> station then hostapd will turn around and reject the association. This
> might not end up being done nicely, but I think it does still happen
> before the association response, so a negative one would result.
>
>>> We'll need to be a little careful what happens with client-mode
>>> interfaces, which is where we actually observed hitting the WARN_ON()
>>> about not having any rates at all, but I think I already put a reset of
>>> the rates there anyway if they're not compatible with the AP. At least
>>> that's easier because there's only one client.
>>
>> It would be easy to configure a station to do VHT MCS 8 only, and then
>> it would never be able to associate with an HT AP, for instance.  I don't
>> think this should be a WARN_ON event, just a failure.
>
> Well it resulted in a WARN_ON because if the AP didn't have those
> rates, we'd not find any usable rates while trying to transmit a frame,
> and then ended up warning and falling down to the lowest possible rate.
>
> I don't think I agree that configuring this should reject the
> association, and I've already implemented the behaviour of dropping the
> user's rate set in this case in the patch we're discussing.

So, as long as we are associating to a VHT AP, then we could still set the
tx-rateset to (only) VHT MCS 8 and allow association, even if there are no additional
rates set.  There is at least one common rate, so theoretically, the
station and AP can communicate.

If we tried to associate this same station to an HT AP, then your work-around
code could kick in and throw away the user's rateset configuration, preferably
with a fairly noticeable warning message since you are overriding the user's
preferred tx rateset?


>> It would be great
>> if we could get a useful error message back to the caller, but I am not
>> sure how feasible that is with the current netlink and mac80211 code.
>
> If we reject the user setting, then we can easily more useful return
> error messages now that we have generic netlink extack support. The
> more "interesting" case is when the user set this and then reconnects.
>
> This kind of problem is why I absolutely dislike out-of-band state that
> affects the connection, rather than giving it in the connection
> command(s) (connect, auth, associate, whatever). We're stuck with it
> now, and needed to redefine that this selection may be dropped if not
> usable.

You could start allowing the user to configure the full advertised and
transmit rateset for each of these actions (and probe too), and user-space can add the fields
to their netlink commands.  At least going forward, this might help
make the behaviour more as expected.  If you would like to implement at
least the basics in cfg/mac80211, I would be happy to work on the supplicant
end of things.

>
>> If your main concern is hitting a WARN_ON, maybe just change it to a
>> quieter error message or remove it entirely and just return a failure
>> code?
>
> No, the warning serves a useful purpose, it's not useful to fall back
> to the lowest rate, even if the user selected something completely
> unusable. Arguably we should simply reject transmitting in that case,
> but that's not really better either ...
>
> johannes


Thanks,
Ben


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

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k