On 03/31/2016 12:46 AM, Johannes Berg wrote:
On Wed, 2016-03-30 at 09:52 -0700, Ben Greear wrote:
If someone can fix rhashtable, then great.
I read some earlier comments [1] back when someone else reported
similar problems, and the comments seemed to indicate that rhashtable
was broken in this manner on purpose to protect against hashing
attacks.
If you are baking in this type of policy to what should be a basic
data-type, then it is not useful for how it is being used in
the mac80211 stack.
[1] http://lkml.iu.edu/hypermail/linux/kernel/1512.2/01681.html
That's not really saying it's purposely broken, that's more saying that
Herbert didn't see a point in fixing a case that has awful behaviour
already.
However, I'm confused now - we can much more easily live with
*insertion* failures, as the linked email indicates, than *deletion*
failures, which I think you had indicated originally. Are you really
seeing *deletion* failures?
If there are in fact *deletion* failures then I think we really need to
address those in rhashtable, no matter the worst-case behaviour of the
hashing or keys, since we should be able to delete entries in order to
get back to something reasonable. Looking at the code though, I don't
actually see that happening.
If you're seeing only *insertion* failures, which you indicated in the
root of this thread, then I think for the general case in mac80211 we
can live with that - we use a seeded jhash for the hash function, and
since in the general case we cannot accept entries with identical MAC
addresses to start with, it shouldn't be possible to run into this
problem under normal use cases.
I see insertion failure, and then later, if of course fails to delete
as well since it was never inserted to begin with. There is no good
way to deal with insertion error, so just need to fix the hashtable.
In this case, I think perhaps you can just patch your local system with
the many interfaces connecting to the same AP to add the parameter
Herbert suggested (.insecure_elasticity = true in sta_rht_params). This
is, after all, very much a case that "normal" operation doesn't even
get close to.
Old code, even stock kernels, could deal with this properly, so I think it
should be fixed by default. I'll put rhash back in my tree and try that
insecure
option and see if it works.
Thanks,
Ben
johannes
--
Ben Greear <gree...@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
