Re: [PATCH] ath10k: transmit queued frames after waking queues

[Bob Copeland]

On Fri, May 25, 2018 at 02:36:56PM +0200, Niklas Cassel wrote:
> A spin lock does have the advantage of ordering: memory operations issued
> before the spin_unlock_bh() will be completed before the spin_unlock_bh()
> operation has completed.
> 
> However, ath10k_htt_tx_dec_pending() was called earlier in the same function,
> which decreases htt->num_pending_tx, so that write will be completed before
> our read. That is the only ordering we care about here (if we should call
> ath10k_mac_tx_push_pending() or not).

Sure.  I also understand that reading inside a lock and operating on the
value outside the lock isn't really the definition of synchronization
(doesn't really matter in this case though).

I was just suggesting that the implicit memory barrier in the spin unlock
that we are already paying for would be sufficient here too, and it matches
the semantic of "tx fields under tx_lock."  On the other hand, maybe it's
just me, but I tend to look askance at just-in-case READ_ONCEs sprinkled
about.

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

Re: [PATCH] ath10k: transmit queued frames after waking queues

[Bob Copeland]

On Mon, May 21, 2018 at 10:37:01PM +0200, Niklas Cassel wrote:
> On Thu, May 17, 2018 at 03:26:25PM -0700, Adrian Chadd wrote:
> > On Thu, 17 May 2018 at 16:16, Niklas Cassel <niklas.cas...@linaro.org>
> > wrote:
> > 
> > > diff --git a/drivers/net/wireless/ath/ath10k/txrx.c
> > b/drivers/net/wireless/ath/ath10k/txrx.c
> > > index cda164f6e9f6..1d3b2d2c3fee 100644
> > > --- a/drivers/net/wireless/ath/ath10k/txrx.c
> > > +++ b/drivers/net/wireless/ath/ath10k/txrx.c
> > > @@ -95,6 +95,9 @@ int ath10k_txrx_tx_unref(struct ath10k_htt *htt,
> > >  wake_up(>empty_tx_wq);
> > >  spin_unlock_bh(>tx_lock);
> > 
> > > +   if (htt->num_pending_tx <= 3 && !list_empty(>txqs))
> > > +   ath10k_mac_tx_push_pending(ar);
> > > +
> > 
> > Just sanity checking - what's protecting htt->num_pending_tx? or is it
> > serialised some other way?
[...]
> I can't see that any of the examples applies, but let's add READ_ONCE(),
> to make sure that the compiler doesn't try to optimize this.

Couldn't you just move the num_pending_tx read inside tx_lock which is 2 lines
above?  I think all the other manipulations are protected by tx_lock.

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

Re: [wireless-testsing2:master 1/4] drivers/net/netdevsim/bpf.c:130:14: sparse: incompatible types for 'case' statement

[Bob Copeland]

On Wed, Jan 03, 2018 at 05:02:37PM -0800, Jakub Kicinski wrote:
> >drivers/net/netdevsim/bpf.c: In function 'nsim_bpf_setup_tc_block_cb':
> > >> drivers/net/netdevsim/bpf.c:130:7: error: 'TC_CLSBPF_REPLACE' undeclared 
> > >> (first use in this function); did you mean 'TC_RED_REPLACE'?  
> >  case TC_CLSBPF_REPLACE:
> >   ^
> >   TC_RED_REPLACE
> 
> FWIW looks like the tree contains old net-next code and latest net
> (linux/master) code.  Pulling from net-next will solve this.
>

Thanks, yes, sorry about the noise.  This tree doesn't always pull net-next,
but I'll do so and compile-test netdevsim in next build.

> > :: TO: Jakub Kicinski <jakub.kicin...@netronome.com>
> > :: CC: Daniel Borkmann <dan...@iogearbox.net>
> 
> Interestingly Daniel and I were not CCed on the report, is this
> intentional?

One for kbuildbot team I guess, not sure how it determines whom to email.
Personally I would be ok if only myself and maybe linux-wireless got the
emails for this tree since most build failures are similar noise.

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

Re: [wireless-testsing2:master 4/5] drivers/net/ethernet/marvell/mvpp2.c:7618:49: error: passing argument 4 of 'mvpp2_port_copy_mac_addr' from incompatible pointer type

[Bob Copeland]

My fault, mismerged a driver conflict -- will fix in next w-t build.

On Tue, Sep 05, 2017 at 09:16:48PM +0800, kbuild test robot wrote:
> tree:   
> https://git.kernel.org/pub/scm/linux/kernel/git/wireless/wireless-testing.git 
> master
> head:   d17be7f7503bf9492198a242779a68af93fd92de
> commit: 96c03618031bae5e9068b16f9e437b79f98f6482 [4/5] Merge remote-tracking 
> branch 'mac80211-next/master'
> config: ia64-allyesconfig (attached as .config)
> compiler: ia64-linux-gcc (GCC) 6.2.0
> reproduce:
> wget 
> https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
> ~/bin/make.cross
> chmod +x ~/bin/make.cross
> git checkout 96c03618031bae5e9068b16f9e437b79f98f6482
> # save the attached .config to linux build tree
> make.cross ARCH=ia64 
> 
> All errors (new ones prefixed by >>):
> 
>drivers/net/ethernet/marvell/mvpp2.c: In function 'mvpp2_port_probe':
> >> drivers/net/ethernet/marvell/mvpp2.c:7618:49: error: passing argument 4 of 
> >> 'mvpp2_port_copy_mac_addr' from incompatible pointer type 
> >> [-Werror=incompatible-pointer-types]
>  mvpp2_port_copy_mac_addr(dev, priv, port_node, _from);

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

Re: [PATCH v2 2/3] mac80211: mesh: improve path resolving time

[Bob Copeland]

On Wed, Jul 13, 2016 at 02:45:25PM +0300, Yaniv Machani wrote:
> When a packet is received for transmission,
> a PREQ frame is sent to resolve the appropriate path to the desired 
> destination.
> After path was established, any sequential PREQ will be sent only after
> dot11MeshHWMPpreqMinInterval, which usually set to few seconds.
> 
> This implementation has an impact in cases where we would like to
> resolve the path quickly.
> A clear example is when a peer was disconnected from us,
> while he acted as a hop to our destination.
> Although the path table will be cleared, the next PREQ frame will be sent 
> only after reaching the MinInterval.
> This will cause unwanted delay, possibly of few seconds until the traffic 
> will resume.
> 
>   if (!(mpath->flags & MESH_PATH_RESOLVING))
> - mesh_queue_preq(mpath, PREQ_Q_F_START);
> + mesh_queue_preq(mpath, PREQ_Q_F_START, true);

What about something like this here instead:

if (!(mpath->flags & MESH_PATH_RESOLVING)) {
/* force next preq to be sent without delay */
ifmsh->last_preq = jiffies - min_preq_int_jiff(sdata) - 1;
mesh_queue_preq(mpath, PREQ_Q_F_START);
}

Maybe a little more magic, but it has a comment explaining it and doesn't
add a bool parameter everywhere.  Or, maybe even just do it inside
mesh_queue_preq() based on having PREQ_Q_F_START && !PREQ_Q_F_REFRESH (if
those are the only cases where "true" is passed).

Generally I try to avoid bool parameters where possible because when you
look at a callsite, you don't know immediately what "true" and "false"
mean, and also each one you add doubles the code paths through a given
function.

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

Re: [PATCH 3/3] mac80211: mesh: fixed HT ies in beacon template

[Bob Copeland]

On Wed, Jul 13, 2016 at 02:45:40PM +0300, Yaniv Machani wrote:
> The HT capab info field inside the HT capab IE of the mesh beacon
> is incorrect (in the case of 20MHz channel width).
> To fix this driver will check configuration from cfg and
> will build it accordingly.

> +/* determine capability flags */
> + cap = sband->ht_cap.cap;
> +
> +/* if channel width is 20MHz - configure HT capab accordingly*/
> + if (sdata->vif.bss_conf.chandef.width == NL80211_CHAN_WIDTH_20) {
> + cap &= ~IEEE80211_HT_CAP_SUP_WIDTH_20_40;
> + cap &= ~IEEE80211_HT_CAP_DSSSCCK40;
> + }

Is it required that HT capability match the HT operation in this case?

> diff --git a/net/mac80211/util.c b/net/mac80211/util.c
> index 42bf0b6..5375a82 100644
> --- a/net/mac80211/util.c
> +++ b/net/mac80211/util.c
> @@ -2349,10 +2349,7 @@ u8 *ieee80211_ie_build_ht_oper(u8 *pos, struct 
> ieee80211_sta_ht_cap *ht_cap,
>   ht_oper->operation_mode = cpu_to_le16(prot_mode);
>   ht_oper->stbc_param = 0x;
>  
> - /* It seems that Basic MCS set and Supported MCS set
> -are identical for the first 10 bytes */
>   memset(_oper->basic_set, 0, 16);
> - memcpy(_oper->basic_set, _cap->mcs, 10);

This change doesn't look right (basic mcs set will be all zeroes) but
then, neither does the original code.  Basic MCS set for a mesh STA should
be the mandatory MCSes according to 9.7.4.

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

Re: [PATCH 1/4] mac80211: mesh: flush stations before beacons are stopped

[Bob Copeland]

On Wed, Jul 13, 2016 at 10:11:25AM +, Machani, Yaniv wrote:
> > > Some drivers (e.g. wl18xx) expect that the last stage in the 
> > > de-initialization process will be stopping the beacons, similar to ap.
> > > Update ieee80211_stop_mesh() flow accordingly.
> > >
> > How well have you tested that with other drivers?
> > 
> 
> Sorry for the delayed response (I've been out) and thanks for your comments,
> I have tested it with RT3572 as well, and didn't see any issue.
> I'll update the comment to reflect that.

I'll give this a test on ath10k and wcn36xx as they are the ones most
likely to care about ordering.

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

Re: [PATCH 3/4] mac80211: mesh: fixed HT ies in beacon template

[Bob Copeland]

On Tue, Jun 28, 2016 at 02:13:06PM +0300, Yaniv Machani wrote:
> From: Meirav Kama <meir...@ti.com>
> 
> There are several values in HT info elements of mesh beacon (built by the
> mac80211) that are incorrect.

Would be good to enumerate the problems here.

> To fix them:
> 1. mac80211 will check configuration from cfg and will build accordingly.
> 2. changes made in mesh default values.

What is wrong with the defaults?

>   sband = local->hw.wiphy->bands[band];
>   if (!sband->ht_cap.ht_supported ||
> @@ -431,11 +433,40 @@ int mesh_add_ht_cap_ie(struct ieee80211_sub_if_data 
> *sdata,
>   sdata->vif.bss_conf.chandef.width == NL80211_CHAN_WIDTH_10)
>   return 0;
>  
> +/* determine capability flags */
> + cap = sband->ht_cap.cap;

There is some weird whitespace here (space instead of tabs for the
comment).

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

Re: [PATCH 4/4] mac80211: sta_info: max_peers reached falsely

[Bob Copeland]

On Tue, Jun 28, 2016 at 02:13:07PM +0300, Yaniv Machani wrote:
> From: Meirav Kama <meir...@ti.com>
> 
> Issue happened when receiving delete_sta command without
> changing plink_state from ESTAB to HOLDING before.
> When receiving delete_sta command for mesh interface
> verify plink_state is not ESTAB and if so, decrease
> plink count and update beacon.

This should be fixed already (and properly) by the patch
"mac80211: Fix mesh estab links counting" -- please let us
know if you have a case that's still broken with that fix.

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

Re: [PATCH 2/4] mac80211/cfg: mesh: fix healing time when a mesh peer is disconnecting

[Bob Copeland]

On Tue, Jun 28, 2016 at 02:13:05PM +0300, Yaniv Machani wrote:
> From: Maital Hahn <mait...@ti.com>
> 
> Once receiving a CLOSE action frame from the disconnecting peer,
> flush all entries in the path table which has this peer as the
> next hop.

Please address the user-visible behavior in your commit messages.
Does it crash?  Does it send frames to an invalid peer?  Do
frames get dropped?

> In addition, upon receiving a packet, if next hop is not found,
> trigger PERQ immidiatly, instead of just putting it in the queue.

"PREQ"

Please split this into a separate patch that we can review
separately (and also give the "why" in the commit log).

> Signed-off-by: Maital Hahn <mait...@ti.com>
> Acked-by: Yaniv Machani <yani...@ti.com>
> ---
>  net/mac80211/cfg.c   |  1 +
>  net/mac80211/mesh.c  |  3 ++-
>  net/mac80211/mesh_hwmp.c | 42 +-
>  3 files changed, 28 insertions(+), 18 deletions(-)
> 
> diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
> index 0c12e40..f876ef7 100644
> --- a/net/mac80211/cfg.c
> +++ b/net/mac80211/cfg.c
> @@ -1011,6 +1011,7 @@ static void sta_apply_mesh_params(struct 
> ieee80211_local *local,
>   if (sta->mesh->plink_state == NL80211_PLINK_ESTAB)
>   changed = mesh_plink_dec_estab_count(sdata);
>   sta->mesh->plink_state = params->plink_state;
> + mesh_path_flush_by_nexthop(sta);

This isn't necessary, caller should already be doing
mesh_path_flush_by_nexthop() in every case I could see.  Besides it
cannot be done under plink lock.

> +++ b/net/mac80211/mesh.c
> @@ -159,7 +159,8 @@ void mesh_sta_cleanup(struct sta_info *sta)
>   if (!sdata->u.mesh.user_mpm) {
>   changed |= mesh_plink_deactivate(sta);
>   del_timer_sync(>mesh->plink_timer);
> - }
> + } else
> + mesh_path_flush_by_nexthop(sta);

And this is already fixed in mac80211-next.

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

Re: [PATCH 0/3] *** Mesh Path Selection Metric Calculation ***

[Bob Copeland]

On Mon, Jun 20, 2016 at 04:00:19PM +0300, Maxim Altshul wrote:
> 2. Implements the opcode and the mechanism that reports the rates
> in TI driver.

Does this mean TI driver will support mesh at some point?

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

Re: [RFCv2 0/3] mac80211: implement fq codel

[Bob Copeland]

On Wed, Mar 16, 2016 at 11:36:31AM -0700, Dave Taht wrote:
> That is the sanest 802.11e queue behavior I have ever seen!  (at both
> 6 and 300mbit! in the ath10k patched mac test)

Out of curiosity, why does BE have larger latency than BK in that chart?
I'd have expected the opposite.

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

Re: [RFCv2 0/3] mac80211: implement fq codel

[Bob Copeland]

On Thu, Mar 17, 2016 at 09:55:03AM +0100, Michal Kazior wrote:
> If you consider Wi-Fi is half-duplex and latency in the entire stack
> (for processing ICMP and UDP_RR) is greater than 11e contention window
> timings you can get your BE flow responses with extra delay (since
> other queues might have responses ready quicker).

Got it, that makes sense.  Thanks for the explanation!

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

Re: [RFC/RFT] mac80211: implement fq_codel for software queuing

[Bob Copeland]

On Tue, Mar 08, 2016 at 08:12:21AM +0100, Michal Kazior wrote:
> However other drivers (e.g. ath10k) have offloaded rate control on
> device. There's currently no way of doing this calculation. I was
> thinking of drivers exporting tx_rate to mac80211 in some way - either
> via a simple sta->tx_rate scalar that the driver is responsible for
> updating, or a EWMA that driver updates (hopefully) periodically and
> often enough. This should in theory at least allow an estimate how
> much data on average you can fit into given time frame (e.g. txop, or
> hardcoded 1ms).

What about implementing ops->get_expected_throughput?  This would be
useful for mesh (both 11s and batman) as well since they need to
estimate link quality to pick a path.

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

Re: [PATCH 1/2] rhashtable: accept GFP flags in rhashtable_walk_init

[Bob Copeland]

On Thu, Mar 03, 2016 at 04:26:19PM +0100, Johannes Berg wrote:
> On Wed, 2016-03-02 at 10:09 -0500, Bob Copeland wrote:
> > In certain cases, the 802.11 mesh pathtable code wants to
> > iterate over all of the entries in the forwarding table from
> > the receive path, which is inside an RCU read-side critical
> > section.  Enable walks inside atomic sections by allowing
> > GFP_ATOMIC allocations for the walker state.
> > 
> > Change all existing callsites to pass in GFP_KERNEL.
> 
> Applied both. You missed 3 callsites, I've fixed those. I hope I got
> them all :)

Ouch, thanks for the catch, I must've grepped only in net.
And now I added those files to my build config. :)

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

[PATCH 1/2] rhashtable: accept GFP flags in rhashtable_walk_init

[Bob Copeland]

In certain cases, the 802.11 mesh pathtable code wants to
iterate over all of the entries in the forwarding table from
the receive path, which is inside an RCU read-side critical
section.  Enable walks inside atomic sections by allowing
GFP_ATOMIC allocations for the walker state.

Change all existing callsites to pass in GFP_KERNEL.

Cc: Thomas Graf <tg...@suug.ch>
Cc: netdev@vger.kernel.org
Acked-by: Thomas Graf <tg...@suug.ch>
Signed-off-by: Bob Copeland <m...@bobcopeland.com>
---
(-RFC, +Thomas's Ack)

 include/linux/rhashtable.h | 3 ++-
 lib/rhashtable.c   | 6 --
 net/ipv6/ila/ila_xlat.c| 3 ++-
 net/netfilter/nft_hash.c   | 4 ++--
 net/netlink/af_netlink.c   | 3 ++-
 net/sctp/proc.c| 3 ++-
 6 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/include/linux/rhashtable.h b/include/linux/rhashtable.h
index 63bd7601b6de..3eef0802a0cd 100644
--- a/include/linux/rhashtable.h
+++ b/include/linux/rhashtable.h
@@ -346,7 +346,8 @@ struct bucket_table *rhashtable_insert_slow(struct 
rhashtable *ht,
struct bucket_table *old_tbl);
 int rhashtable_insert_rehash(struct rhashtable *ht, struct bucket_table *tbl);
 
-int rhashtable_walk_init(struct rhashtable *ht, struct rhashtable_iter *iter);
+int rhashtable_walk_init(struct rhashtable *ht, struct rhashtable_iter *iter,
+gfp_t gfp);
 void rhashtable_walk_exit(struct rhashtable_iter *iter);
 int rhashtable_walk_start(struct rhashtable_iter *iter) __acquires(RCU);
 void *rhashtable_walk_next(struct rhashtable_iter *iter);
diff --git a/lib/rhashtable.c b/lib/rhashtable.c
index cc808707d1cf..5d845ffd7982 100644
--- a/lib/rhashtable.c
+++ b/lib/rhashtable.c
@@ -487,6 +487,7 @@ EXPORT_SYMBOL_GPL(rhashtable_insert_slow);
  * rhashtable_walk_init - Initialise an iterator
  * @ht:Table to walk over
  * @iter:  Hash table Iterator
+ * @gfp:   GFP flags for allocations
  *
  * This function prepares a hash table walk.
  *
@@ -504,14 +505,15 @@ EXPORT_SYMBOL_GPL(rhashtable_insert_slow);
  * You must call rhashtable_walk_exit if this function returns
  * successfully.
  */
-int rhashtable_walk_init(struct rhashtable *ht, struct rhashtable_iter *iter)
+int rhashtable_walk_init(struct rhashtable *ht, struct rhashtable_iter *iter,
+gfp_t gfp)
 {
iter->ht = ht;
iter->p = NULL;
iter->slot = 0;
iter->skip = 0;
 
-   iter->walker = kmalloc(sizeof(*iter->walker), GFP_KERNEL);
+   iter->walker = kmalloc(sizeof(*iter->walker), gfp);
if (!iter->walker)
return -ENOMEM;
 
diff --git a/net/ipv6/ila/ila_xlat.c b/net/ipv6/ila/ila_xlat.c
index 295ca29a23c3..0b03533453e4 100644
--- a/net/ipv6/ila/ila_xlat.c
+++ b/net/ipv6/ila/ila_xlat.c
@@ -501,7 +501,8 @@ static int ila_nl_dump_start(struct netlink_callback *cb)
struct ila_net *ilan = net_generic(net, ila_net_id);
struct ila_dump_iter *iter = (struct ila_dump_iter *)cb->args;
 
-   return rhashtable_walk_init(>rhash_table, >rhiter);
+   return rhashtable_walk_init(>rhash_table, >rhiter,
+   GFP_KERNEL);
 }
 
 static int ila_nl_dump_done(struct netlink_callback *cb)
diff --git a/net/netfilter/nft_hash.c b/net/netfilter/nft_hash.c
index 3f9d45d3d9b7..6fa016564f90 100644
--- a/net/netfilter/nft_hash.c
+++ b/net/netfilter/nft_hash.c
@@ -192,7 +192,7 @@ static void nft_hash_walk(const struct nft_ctx *ctx, const 
struct nft_set *set,
u8 genmask = nft_genmask_cur(read_pnet(>pnet));
int err;
 
-   err = rhashtable_walk_init(>ht, );
+   err = rhashtable_walk_init(>ht, , GFP_KERNEL);
iter->err = err;
if (err)
return;
@@ -248,7 +248,7 @@ static void nft_hash_gc(struct work_struct *work)
priv = container_of(work, struct nft_hash, gc_work.work);
set  = nft_set_container_of(priv);
 
-   err = rhashtable_walk_init(>ht, );
+   err = rhashtable_walk_init(>ht, , GFP_KERNEL);
if (err)
goto schedule;
 
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index c8416792cce0..6e0cbdeb21d3 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -2335,7 +2335,8 @@ static int netlink_walk_start(struct nl_seq_iter *iter)
 {
int err;
 
-   err = rhashtable_walk_init(_table[iter->link].hash, >hti);
+   err = rhashtable_walk_init(_table[iter->link].hash, >hti,
+  GFP_KERNEL);
if (err) {
iter->link = MAX_LINKS;
return err;
diff --git a/net/sctp/proc.c b/net/sctp/proc.c
index cfc3c7101a38..c5991e5e5daf 100644
--- a/net/sctp/proc.c
+++ b/net/sctp/proc.c
@@ -319,7 +319,8 @@ static int sctp_transport_walk_start(struct seq_file *seq)
struct sctp_ht_iter *iter = seq->private

[PATCH 2/2] mac80211: mesh: convert path table to rhashtable

[Bob Copeland]

In the time since the mesh path table was implemented as an
RCU-traversable, dynamically growing hash table, a generic RCU
hashtable implementation was added to the kernel.

Switch the mesh path table over to rhashtable to remove some code
and also gain some features like automatic shrinking.

Cc: Thomas Graf <tg...@suug.ch>
Cc: netdev@vger.kernel.org
Signed-off-by: Bob Copeland <m...@bobcopeland.com>
---
 net/mac80211/ieee80211_i.h  |  11 +-
 net/mac80211/mesh.c |   6 -
 net/mac80211/mesh.h |  31 +-
 net/mac80211/mesh_pathtbl.c | 786 ++--
 4 files changed, 259 insertions(+), 575 deletions(-)

diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index db7f0dbebc4b..c8945e2d8a86 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -697,17 +697,10 @@ struct ieee80211_if_mesh {
/* offset from skb->data while building IE */
int meshconf_offset;
 
-   struct mesh_table __rcu *mesh_paths;
-   struct mesh_table __rcu *mpp_paths; /* Store paths for MPP */
+   struct mesh_table *mesh_paths;
+   struct mesh_table *mpp_paths; /* Store paths for MPP */
int mesh_paths_generation;
int mpp_paths_generation;
-
-   /* Protects assignment of the mesh_paths/mpp_paths table
-* pointer for resize against reading it for add/delete
-* of individual paths.  Pure readers (lookups) just use
-* RCU.
-*/
-   rwlock_t pathtbl_resize_lock;
 };
 
 #ifdef CONFIG_MAC80211_MESH
diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c
index c92af2a7714d..a216c439b6f2 100644
--- a/net/mac80211/mesh.c
+++ b/net/mac80211/mesh.c
@@ -1347,12 +1347,6 @@ void ieee80211_mesh_work(struct ieee80211_sub_if_data 
*sdata)
   ifmsh->last_preq + 
msecs_to_jiffies(ifmsh->mshcfg.dot11MeshHWMPpreqMinInterval)))
mesh_path_start_discovery(sdata);
 
-   if (test_and_clear_bit(MESH_WORK_GROW_MPATH_TABLE, >wrkq_flags))
-   mesh_mpath_table_grow(sdata);
-
-   if (test_and_clear_bit(MESH_WORK_GROW_MPP_TABLE, >wrkq_flags))
-   mesh_mpp_table_grow(sdata);
-
if (test_and_clear_bit(MESH_WORK_HOUSEKEEPING, >wrkq_flags))
ieee80211_mesh_housekeeping(sdata);
 
diff --git a/net/mac80211/mesh.h b/net/mac80211/mesh.h
index f3cc3917e048..cc6854db156e 100644
--- a/net/mac80211/mesh.h
+++ b/net/mac80211/mesh.h
@@ -51,10 +51,6 @@ enum mesh_path_flags {
  *
  *
  * @MESH_WORK_HOUSEKEEPING: run the periodic mesh housekeeping tasks
- * @MESH_WORK_GROW_MPATH_TABLE: the mesh path table is full and needs
- * to grow.
- * @MESH_WORK_GROW_MPP_TABLE: the mesh portals table is full and needs to
- * grow
  * @MESH_WORK_ROOT: the mesh root station needs to send a frame
  * @MESH_WORK_DRIFT_ADJUST: time to compensate for clock drift relative to 
other
  * mesh nodes
@@ -62,8 +58,6 @@ enum mesh_path_flags {
  */
 enum mesh_deferred_task_flags {
MESH_WORK_HOUSEKEEPING,
-   MESH_WORK_GROW_MPATH_TABLE,
-   MESH_WORK_GROW_MPP_TABLE,
MESH_WORK_ROOT,
MESH_WORK_DRIFT_ADJUST,
MESH_WORK_MBSS_CHANGED,
@@ -105,6 +99,7 @@ enum mesh_deferred_task_flags {
 struct mesh_path {
u8 dst[ETH_ALEN];
u8 mpp[ETH_ALEN];   /* used for MPP or MAP */
+   struct rhash_head rhash;
struct hlist_node gate_list;
struct ieee80211_sub_if_data *sdata;
struct sta_info __rcu *next_hop;
@@ -129,34 +124,17 @@ struct mesh_path {
 /**
  * struct mesh_table
  *
- * @hash_buckets: array of hash buckets of the table
- * @hashwlock: array of locks to protect write operations, one per bucket
- * @hash_mask: 2^size_order - 1, used to compute hash idx
- * @hash_rnd: random value used for hash computations
  * @entries: number of entries in the table
- * @free_node: function to free nodes of the table
- * @copy_node: function to copy nodes of the table
- * @size_order: determines size of the table, there will be 2^size_order hash
- * buckets
  * @known_gates: list of known mesh gates and their mpaths by the station. The
  * gate's mpath may or may not be resolved and active.
- *
- * rcu_head: RCU head to free the table
+ * @rhash: the rhashtable containing struct mesh_paths, keyed by dest addr
  */
 struct mesh_table {
-   /* Number of buckets will be 2^N */
-   struct hlist_head *hash_buckets;
-   spinlock_t *hashwlock;  /* One per bucket, for add/del */
-   unsigned int hash_mask; /* (2^size_order) - 1 */
-   __u32 hash_rnd; /* Used for hash generation */
atomic_t entries;   /* Up to MAX_MESH_NEIGHBOURS */
-   void (*free_node) (struct hlist_node *p, bool free_leafs);
-   int (*copy_node) (struct hlist_node *p, struct mesh_table *newtbl);
-   int size_order;
struct hlist_head *known_gates;
spinlock_t gates_lock;
 
-   struct rcu_head rcu_head;
+

Re: [PATCH RFC 1/2] rhashtable: accept GFP flags in rhashtable_walk_init

[Bob Copeland]

On Tue, Mar 01, 2016 at 02:54:16PM +0100, Johannes Berg wrote:
> On Sun, 2016-02-28 at 20:06 -0500, Bob Copeland wrote:
> > In certain cases, the 802.11 mesh pathtable code wants to
> > iterate over all of the entries in the forwarding table from
> > the receive path, which is inside an RCU read-side critical
> > section.  Enable walks inside atomic sections by allowing
> > GFP_ATOMIC allocations for the walker state.
> > 
> > Change all existing callsites to pass in GFP_KERNEL.
> 
> Both look fine to me.
> 
> I see you have more patches for mesh, so this probably can't go through
> net-next tree directly.

Yeah, these two are on top of the other mesh path table changes.

So either both of these could go via mac80211-next or we could do the
1/2 in net-next and wait to apply 2/2 in mac80211-next.  Whatever works
for you guys.

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

[PATCH RFC 1/2] rhashtable: accept GFP flags in rhashtable_walk_init

[Bob Copeland]

In certain cases, the 802.11 mesh pathtable code wants to
iterate over all of the entries in the forwarding table from
the receive path, which is inside an RCU read-side critical
section.  Enable walks inside atomic sections by allowing
GFP_ATOMIC allocations for the walker state.

Change all existing callsites to pass in GFP_KERNEL.

Cc: Thomas Graf <tg...@suug.ch>
Cc: netdev@vger.kernel.org
Signed-off-by: Bob Copeland <m...@bobcopeland.com>
---
 include/linux/rhashtable.h | 3 ++-
 lib/rhashtable.c   | 6 --
 net/ipv6/ila/ila_xlat.c| 3 ++-
 net/netfilter/nft_hash.c   | 4 ++--
 net/netlink/af_netlink.c   | 3 ++-
 net/sctp/proc.c| 3 ++-
 6 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/include/linux/rhashtable.h b/include/linux/rhashtable.h
index 63bd7601b6de..3eef0802a0cd 100644
--- a/include/linux/rhashtable.h
+++ b/include/linux/rhashtable.h
@@ -346,7 +346,8 @@ struct bucket_table *rhashtable_insert_slow(struct 
rhashtable *ht,
struct bucket_table *old_tbl);
 int rhashtable_insert_rehash(struct rhashtable *ht, struct bucket_table *tbl);
 
-int rhashtable_walk_init(struct rhashtable *ht, struct rhashtable_iter *iter);
+int rhashtable_walk_init(struct rhashtable *ht, struct rhashtable_iter *iter,
+gfp_t gfp);
 void rhashtable_walk_exit(struct rhashtable_iter *iter);
 int rhashtable_walk_start(struct rhashtable_iter *iter) __acquires(RCU);
 void *rhashtable_walk_next(struct rhashtable_iter *iter);
diff --git a/lib/rhashtable.c b/lib/rhashtable.c
index cc808707d1cf..5d845ffd7982 100644
--- a/lib/rhashtable.c
+++ b/lib/rhashtable.c
@@ -487,6 +487,7 @@ EXPORT_SYMBOL_GPL(rhashtable_insert_slow);
  * rhashtable_walk_init - Initialise an iterator
  * @ht:Table to walk over
  * @iter:  Hash table Iterator
+ * @gfp:   GFP flags for allocations
  *
  * This function prepares a hash table walk.
  *
@@ -504,14 +505,15 @@ EXPORT_SYMBOL_GPL(rhashtable_insert_slow);
  * You must call rhashtable_walk_exit if this function returns
  * successfully.
  */
-int rhashtable_walk_init(struct rhashtable *ht, struct rhashtable_iter *iter)
+int rhashtable_walk_init(struct rhashtable *ht, struct rhashtable_iter *iter,
+gfp_t gfp)
 {
iter->ht = ht;
iter->p = NULL;
iter->slot = 0;
iter->skip = 0;
 
-   iter->walker = kmalloc(sizeof(*iter->walker), GFP_KERNEL);
+   iter->walker = kmalloc(sizeof(*iter->walker), gfp);
if (!iter->walker)
return -ENOMEM;
 
diff --git a/net/ipv6/ila/ila_xlat.c b/net/ipv6/ila/ila_xlat.c
index 295ca29a23c3..0b03533453e4 100644
--- a/net/ipv6/ila/ila_xlat.c
+++ b/net/ipv6/ila/ila_xlat.c
@@ -501,7 +501,8 @@ static int ila_nl_dump_start(struct netlink_callback *cb)
struct ila_net *ilan = net_generic(net, ila_net_id);
struct ila_dump_iter *iter = (struct ila_dump_iter *)cb->args;
 
-   return rhashtable_walk_init(>rhash_table, >rhiter);
+   return rhashtable_walk_init(>rhash_table, >rhiter,
+   GFP_KERNEL);
 }
 
 static int ila_nl_dump_done(struct netlink_callback *cb)
diff --git a/net/netfilter/nft_hash.c b/net/netfilter/nft_hash.c
index 3f9d45d3d9b7..6fa016564f90 100644
--- a/net/netfilter/nft_hash.c
+++ b/net/netfilter/nft_hash.c
@@ -192,7 +192,7 @@ static void nft_hash_walk(const struct nft_ctx *ctx, const 
struct nft_set *set,
u8 genmask = nft_genmask_cur(read_pnet(>pnet));
int err;
 
-   err = rhashtable_walk_init(>ht, );
+   err = rhashtable_walk_init(>ht, , GFP_KERNEL);
iter->err = err;
if (err)
return;
@@ -248,7 +248,7 @@ static void nft_hash_gc(struct work_struct *work)
priv = container_of(work, struct nft_hash, gc_work.work);
set  = nft_set_container_of(priv);
 
-   err = rhashtable_walk_init(>ht, );
+   err = rhashtable_walk_init(>ht, , GFP_KERNEL);
if (err)
goto schedule;
 
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index c8416792cce0..6e0cbdeb21d3 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -2335,7 +2335,8 @@ static int netlink_walk_start(struct nl_seq_iter *iter)
 {
int err;
 
-   err = rhashtable_walk_init(_table[iter->link].hash, >hti);
+   err = rhashtable_walk_init(_table[iter->link].hash, >hti,
+  GFP_KERNEL);
if (err) {
iter->link = MAX_LINKS;
return err;
diff --git a/net/sctp/proc.c b/net/sctp/proc.c
index cfc3c7101a38..c5991e5e5daf 100644
--- a/net/sctp/proc.c
+++ b/net/sctp/proc.c
@@ -319,7 +319,8 @@ static int sctp_transport_walk_start(struct seq_file *seq)
struct sctp_ht_iter *iter = seq->private;
int err;
 
-   err = rhashtable_walk_init(

[PATCH RFC 2/2] mac80211: mesh: convert path table to rhashtable

[Bob Copeland]

In the time since the mesh path table was implemented as an
RCU-traversable, dynamically growing hash table, a generic RCU
hashtable implementation was added to the kernel.

Switch the mesh path table over to rhashtable to remove some code
and also gain some features like automatic shrinking.

Cc: Thomas Graf <tg...@suug.ch>
Cc: netdev@vger.kernel.org
Signed-off-by: Bob Copeland <m...@bobcopeland.com>
---
 net/mac80211/ieee80211_i.h  |  11 +-
 net/mac80211/mesh.c |   6 -
 net/mac80211/mesh.h |  31 +-
 net/mac80211/mesh_pathtbl.c | 786 ++--
 4 files changed, 259 insertions(+), 575 deletions(-)

diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 597f1b6260f4..a2857cd6d479 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -697,17 +697,10 @@ struct ieee80211_if_mesh {
/* offset from skb->data while building IE */
int meshconf_offset;
 
-   struct mesh_table __rcu *mesh_paths;
-   struct mesh_table __rcu *mpp_paths; /* Store paths for MPP */
+   struct mesh_table *mesh_paths;
+   struct mesh_table *mpp_paths; /* Store paths for MPP */
int mesh_paths_generation;
int mpp_paths_generation;
-
-   /* Protects assignment of the mesh_paths/mpp_paths table
-* pointer for resize against reading it for add/delete
-* of individual paths.  Pure readers (lookups) just use
-* RCU.
-*/
-   rwlock_t pathtbl_resize_lock;
 };
 
 #ifdef CONFIG_MAC80211_MESH
diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c
index c92af2a7714d..a216c439b6f2 100644
--- a/net/mac80211/mesh.c
+++ b/net/mac80211/mesh.c
@@ -1347,12 +1347,6 @@ void ieee80211_mesh_work(struct ieee80211_sub_if_data 
*sdata)
   ifmsh->last_preq + 
msecs_to_jiffies(ifmsh->mshcfg.dot11MeshHWMPpreqMinInterval)))
mesh_path_start_discovery(sdata);
 
-   if (test_and_clear_bit(MESH_WORK_GROW_MPATH_TABLE, >wrkq_flags))
-   mesh_mpath_table_grow(sdata);
-
-   if (test_and_clear_bit(MESH_WORK_GROW_MPP_TABLE, >wrkq_flags))
-   mesh_mpp_table_grow(sdata);
-
if (test_and_clear_bit(MESH_WORK_HOUSEKEEPING, >wrkq_flags))
ieee80211_mesh_housekeeping(sdata);
 
diff --git a/net/mac80211/mesh.h b/net/mac80211/mesh.h
index f3cc3917e048..cc6854db156e 100644
--- a/net/mac80211/mesh.h
+++ b/net/mac80211/mesh.h
@@ -51,10 +51,6 @@ enum mesh_path_flags {
  *
  *
  * @MESH_WORK_HOUSEKEEPING: run the periodic mesh housekeeping tasks
- * @MESH_WORK_GROW_MPATH_TABLE: the mesh path table is full and needs
- * to grow.
- * @MESH_WORK_GROW_MPP_TABLE: the mesh portals table is full and needs to
- * grow
  * @MESH_WORK_ROOT: the mesh root station needs to send a frame
  * @MESH_WORK_DRIFT_ADJUST: time to compensate for clock drift relative to 
other
  * mesh nodes
@@ -62,8 +58,6 @@ enum mesh_path_flags {
  */
 enum mesh_deferred_task_flags {
MESH_WORK_HOUSEKEEPING,
-   MESH_WORK_GROW_MPATH_TABLE,
-   MESH_WORK_GROW_MPP_TABLE,
MESH_WORK_ROOT,
MESH_WORK_DRIFT_ADJUST,
MESH_WORK_MBSS_CHANGED,
@@ -105,6 +99,7 @@ enum mesh_deferred_task_flags {
 struct mesh_path {
u8 dst[ETH_ALEN];
u8 mpp[ETH_ALEN];   /* used for MPP or MAP */
+   struct rhash_head rhash;
struct hlist_node gate_list;
struct ieee80211_sub_if_data *sdata;
struct sta_info __rcu *next_hop;
@@ -129,34 +124,17 @@ struct mesh_path {
 /**
  * struct mesh_table
  *
- * @hash_buckets: array of hash buckets of the table
- * @hashwlock: array of locks to protect write operations, one per bucket
- * @hash_mask: 2^size_order - 1, used to compute hash idx
- * @hash_rnd: random value used for hash computations
  * @entries: number of entries in the table
- * @free_node: function to free nodes of the table
- * @copy_node: function to copy nodes of the table
- * @size_order: determines size of the table, there will be 2^size_order hash
- * buckets
  * @known_gates: list of known mesh gates and their mpaths by the station. The
  * gate's mpath may or may not be resolved and active.
- *
- * rcu_head: RCU head to free the table
+ * @rhash: the rhashtable containing struct mesh_paths, keyed by dest addr
  */
 struct mesh_table {
-   /* Number of buckets will be 2^N */
-   struct hlist_head *hash_buckets;
-   spinlock_t *hashwlock;  /* One per bucket, for add/del */
-   unsigned int hash_mask; /* (2^size_order) - 1 */
-   __u32 hash_rnd; /* Used for hash generation */
atomic_t entries;   /* Up to MAX_MESH_NEIGHBOURS */
-   void (*free_node) (struct hlist_node *p, bool free_leafs);
-   int (*copy_node) (struct hlist_node *p, struct mesh_table *newtbl);
-   int size_order;
struct hlist_head *known_gates;
spinlock_t gates_lock;
 
-   struct rcu_head rcu_head;
+