Subject: Re: [PATCH v2] rhashtable: make walk safe from softirq context

From: Bob Copeland <[email-masked]>
Date: Tue, 12 Feb 2019 16:54:37 -0500

On Tue, Feb 12, 2019 at 08:03:17PM +0100, Johannes Berg wrote:
> commit 60854fd94573f0d3b80b55b40cf0140a0430f3ab
> Author: Bob Copeland <[email-masked]>
> Date:   Wed Mar 2 10:09:20 2016 -0500
> 
>     mac80211: mesh: convert path table to rhashtable
> 
> which is kinda old. Not sure why this didn't surface before, because the
> spinlock was introduced *before*, otherwise certainly the mutex would've
> caused us to not be able to do this code to start with (commit
> c6ff5268293 - rhashtable: Fix walker list corruption).
> 
> That commit also just converted an existing hashtable walk to
> rhashtable, so not sure that counts as having introduced the problem :-)

Yeah, I didn't change any of the contexts in which the iterations were
called, except making it possible to replace the previous mesh-specific
RCU hashtable with rhashtable by introducing this.  That said, it would
certainly be nice to not have to walk the table to find paths that
use a specific station as nexthop.

> But I guess we should also ask Bob first:
> 1) do you think it'd be easy to maintain a separate list or avoid the
>    iteration in some otherway, and make that a small enough patch to be
>    applicable for stable?

I'm not sure, it's been a while.  I guess most of the difficulties would
be around station removal?

> 2) or do you think maybe the mesh_plink_broken() call could just be
>    lifted into a workqueue instead?

As far as I know, no reason this can't wait until later; we might send
a few frames to the wrong destination but that can happen anyway.  But
there are a couple more walks in there like mesh_path_flush_by_nexthop().

-- 
Bob Copeland %% https://bobcopeland.com/