[PATCH v2 0/3] mac80211: add cleanup path for MPP table entries

[PATCH v2 0/3] mac80211: add cleanup path for MPP table entries

[Henning Rogge]

Currently MPP table entries are only removed from memory when their
802.11s mesh interface goes down. This can make the kernel to remember
a growing list of proxied mac addresses which are not relevant or even
reachable anymore.

This patchset adds two additional cleanup paths for MPP table entries
to remove them when their corresponding mesh node is removed or when
the MPP table entry is not used for MESH_PATH_EXPIRE time.

Henning Rogge (3):
  mac80211: Remove MPP table entries with MPath
  mac80211: let unused MPP table entries timeout
  mac80211: Unify mesh and mpp path removal function

 net/mac80211/mesh_pathtbl.c | 132 +++++++++++++++++++++++++++++++++++---------
 net/mac80211/rx.c           |   1 +
 net/mac80211/tx.c           |   6 +-
 3 files changed, 112 insertions(+), 27 deletions(-)

-- 
2.5.0

[PATCH v2 1/3] mac80211: Remove MPP table entries with MPath

[Henning Rogge]

Make the mesh_path_del() function remove all mpp table entries
that are proxied by the removed mesh path.

Signed-off-by: Henning Rogge <henning.rogge@fkie.fraunhofer.de>
---
 net/mac80211/mesh_pathtbl.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/net/mac80211/mesh_pathtbl.c b/net/mac80211/mesh_pathtbl.c
index dadf8dc..d9424ea 100644
--- a/net/mac80211/mesh_pathtbl.c
+++ b/net/mac80211/mesh_pathtbl.c
@@ -835,6 +835,29 @@ void mesh_path_flush_by_nexthop(struct sta_info *sta)
 	rcu_read_unlock();
 }
 
+static void mpp_flush_by_proxy(struct ieee80211_sub_if_data *sdata,
+			       const u8 *proxy)
+{
+	struct mesh_table *tbl;
+	struct mesh_path *mpp;
+	struct mpath_node *node;
+	int i;
+
+	rcu_read_lock();
+	read_lock_bh(&pathtbl_resize_lock);
+	tbl = resize_dereference_mpp_paths();
+	for_each_mesh_entry(tbl, node, i) {
+		mpp = node->mpath;
+		if (ether_addr_equal(mpp->mpp, proxy)) {
+			spin_lock(&tbl->hashwlock[i]);
+			__mesh_path_del(tbl, node);
+			spin_unlock(&tbl->hashwlock[i]);
+		}
+	}
+	read_unlock_bh(&pathtbl_resize_lock);
+	rcu_read_unlock();
+}
+
 static void table_flush_by_iface(struct mesh_table *tbl,
 				 struct ieee80211_sub_if_data *sdata)
 {
@@ -892,6 +915,9 @@ int mesh_path_del(struct ieee80211_sub_if_data *sdata, const u8 *addr)
 	int hash_idx;
 	int err = 0;
 
+	/* flush relevant mpp entries first */
+	mpp_flush_by_proxy(sdata, addr);
+
 	read_lock_bh(&pathtbl_resize_lock);
 	tbl = resize_dereference_mesh_paths();
 	hash_idx = mesh_table_hash(addr, sdata, tbl);
-- 
2.5.0

[PATCH v2 2/3] mac80211: let unused MPP table entries timeout

[Henning Rogge]

Remember the last time when a mpp table entry is used for
rx or tx and remove them after MESH_PATH_EXPIRE time.

Signed-off-by: Henning Rogge <henning.rogge@fkie.fraunhofer.de>
---
 net/mac80211/mesh_pathtbl.c | 54 +++++++++++++++++++++++++++++++++++++++++++++
 net/mac80211/rx.c           |  1 +
 net/mac80211/tx.c           |  6 ++++-
 3 files changed, 60 insertions(+), 1 deletion(-)

diff --git a/net/mac80211/mesh_pathtbl.c b/net/mac80211/mesh_pathtbl.c
index d9424ea..33bb1f25 100644
--- a/net/mac80211/mesh_pathtbl.c
+++ b/net/mac80211/mesh_pathtbl.c
@@ -942,6 +942,49 @@ enddel:
 }
 
 /**
+ * mpp_path_del - delete a mesh proxy path from the table
+ *
+ * @addr: addr address (ETH_ALEN length)
+ * @sdata: local subif
+ *
+ * Returns: 0 if successful
+ */
+static int mpp_path_del(struct ieee80211_sub_if_data *sdata, const u8 *addr)
+{
+	struct mesh_table *tbl;
+	struct mesh_path *mpath;
+	struct mpath_node *node;
+	struct hlist_head *bucket;
+	int hash_idx;
+	int err = 0;
+
+	/* flush relevant mpp entries first */
+	mpp_flush_by_proxy(sdata, addr);
+
+	read_lock_bh(&pathtbl_resize_lock);
+	tbl = resize_dereference_mpp_paths();
+	hash_idx = mesh_table_hash(addr, sdata, tbl);
+	bucket = &tbl->hash_buckets[hash_idx];
+
+	spin_lock(&tbl->hashwlock[hash_idx]);
+	hlist_for_each_entry(node, bucket, list) {
+		mpath = node->mpath;
+		if (mpath->sdata == sdata &&
+		    ether_addr_equal(addr, mpath->dst)) {
+			__mesh_path_del(tbl, node);
+			goto enddel;
+		}
+	}
+
+	err = -ENXIO;
+enddel:
+	mesh_paths_generation++;
+	spin_unlock(&tbl->hashwlock[hash_idx]);
+	read_unlock_bh(&pathtbl_resize_lock);
+	return err;
+}
+
+/**
  * mesh_path_tx_pending - sends pending frames in a mesh path queue
  *
  * @mpath: mesh path to activate
@@ -1157,6 +1200,17 @@ void mesh_path_expire(struct ieee80211_sub_if_data *sdata)
 		     time_after(jiffies, mpath->exp_time + MESH_PATH_EXPIRE))
 			mesh_path_del(mpath->sdata, mpath->dst);
 	}
+
+	tbl = rcu_dereference(mpp_paths);
+	for_each_mesh_entry(tbl, node, i) {
+		if (node->mpath->sdata != sdata)
+			continue;
+		mpath = node->mpath;
+		if ((!(mpath->flags & MESH_PATH_FIXED)) &&
+		    time_after(jiffies, mpath->exp_time + MESH_PATH_EXPIRE))
+			mpp_path_del(mpath->sdata, mpath->dst);
+	}
+
 	rcu_read_unlock();
 }
 
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index bc08185..d028581 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -2290,6 +2290,7 @@ ieee80211_rx_h_mesh_fwding(struct ieee80211_rx_data *rx)
 			spin_lock_bh(&mppath->state_lock);
 			if (!ether_addr_equal(mppath->mpp, mpp_addr))
 				memcpy(mppath->mpp, mpp_addr, ETH_ALEN);
+			mppath->exp_time = jiffies;
 			spin_unlock_bh(&mppath->state_lock);
 		}
 		rcu_read_unlock();
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 3311ce0..0236254 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -2099,8 +2099,12 @@ static struct sk_buff *ieee80211_build_hdr(struct ieee80211_sub_if_data *sdata,
 					mpp_lookup = true;
 			}
 
-			if (mpp_lookup)
+			if (mpp_lookup) {
 				mppath = mpp_path_lookup(sdata, skb->data);
+				if (mppath) {
+					mppath->exp_time = jiffies;
+				}
+			}
 
 			if (mppath && mpath)
 				mesh_path_del(mpath->sdata, mpath->dst);
-- 
2.5.0

[PATCH v2 3/3] mac80211: Unify mesh and mpp path removal function

[Henning Rogge]

mpp_path_del() and mesh_path_del() are mostly the same function.
Move common code into a new static function.

Signed-off-by: Henning Rogge <henning.rogge@fkie.fraunhofer.de>
---
 net/mac80211/mesh_pathtbl.c | 110 ++++++++++++++++++++++----------------------
 1 file changed, 55 insertions(+), 55 deletions(-)

diff --git a/net/mac80211/mesh_pathtbl.c b/net/mac80211/mesh_pathtbl.c
index 33bb1f25..86316ee 100644
--- a/net/mac80211/mesh_pathtbl.c
+++ b/net/mac80211/mesh_pathtbl.c
@@ -55,16 +55,20 @@ int mpp_paths_generation;
 static DEFINE_RWLOCK(pathtbl_resize_lock);
 
 
+static inline struct mesh_table *resize_dereference_paths(struct mesh_table *table)
+{
+  return rcu_dereference_protected(table,
+    lockdep_is_held(&pathtbl_resize_lock));
+}
+
 static inline struct mesh_table *resize_dereference_mesh_paths(void)
 {
-	return rcu_dereference_protected(mesh_paths,
-		lockdep_is_held(&pathtbl_resize_lock));
+	return resize_dereference_paths(mesh_paths);
 }
 
 static inline struct mesh_table *resize_dereference_mpp_paths(void)
 {
-	return rcu_dereference_protected(mpp_paths,
-		lockdep_is_held(&pathtbl_resize_lock));
+    return resize_dereference_paths(mpp_paths);
 }
 
 /*
@@ -899,6 +903,45 @@ void mesh_path_flush_by_iface(struct ieee80211_sub_if_data *sdata)
 }
 
 /**
+ * table_path_del - delete a path from the mesh or mpp table
+ *
+ * @tbl: mesh or mpp path table
+ * @sdata: local subif
+ * @addr: dst address (ETH_ALEN length)
+ *
+ * Returns: 0 if successful
+ */
+static int table_path_del(struct mesh_table *rcu_tbl,
+    struct ieee80211_sub_if_data *sdata, const u8 *addr)
+{
+  struct mesh_table *tbl;
+  struct mesh_path *mpath;
+  struct mpath_node *node;
+  struct hlist_head *bucket;
+  int hash_idx;
+  int err = 0;
+
+  tbl = resize_dereference_paths(rcu_tbl);
+  hash_idx = mesh_table_hash(addr, sdata, tbl);
+  bucket = &tbl->hash_buckets[hash_idx];
+
+  spin_lock(&tbl->hashwlock[hash_idx]);
+  hlist_for_each_entry(node, bucket, list) {
+    mpath = node->mpath;
+    if (mpath->sdata == sdata &&
+        ether_addr_equal(addr, mpath->dst)) {
+      __mesh_path_del(tbl, node);
+      goto enddel;
+    }
+  }
+
+  err = -ENXIO;
+enddel:
+  spin_unlock(&tbl->hashwlock[hash_idx]);
+  return err;
+}
+
+/**
  * mesh_path_del - delete a mesh path from the table
  *
  * @addr: dst address (ETH_ALEN length)
@@ -908,36 +951,16 @@ void mesh_path_flush_by_iface(struct ieee80211_sub_if_data *sdata)
  */
 int mesh_path_del(struct ieee80211_sub_if_data *sdata, const u8 *addr)
 {
-	struct mesh_table *tbl;
-	struct mesh_path *mpath;
-	struct mpath_node *node;
-	struct hlist_head *bucket;
-	int hash_idx;
 	int err = 0;
 
 	/* flush relevant mpp entries first */
 	mpp_flush_by_proxy(sdata, addr);
 
 	read_lock_bh(&pathtbl_resize_lock);
-	tbl = resize_dereference_mesh_paths();
-	hash_idx = mesh_table_hash(addr, sdata, tbl);
-	bucket = &tbl->hash_buckets[hash_idx];
-
-	spin_lock(&tbl->hashwlock[hash_idx]);
-	hlist_for_each_entry(node, bucket, list) {
-		mpath = node->mpath;
-		if (mpath->sdata == sdata &&
-		    ether_addr_equal(addr, mpath->dst)) {
-			__mesh_path_del(tbl, node);
-			goto enddel;
-		}
-	}
-
-	err = -ENXIO;
-enddel:
+	err = table_path_del(mesh_paths, sdata, addr);
 	mesh_paths_generation++;
-	spin_unlock(&tbl->hashwlock[hash_idx]);
 	read_unlock_bh(&pathtbl_resize_lock);
+
 	return err;
 }
 
@@ -951,37 +974,14 @@ enddel:
  */
 static int mpp_path_del(struct ieee80211_sub_if_data *sdata, const u8 *addr)
 {
-	struct mesh_table *tbl;
-	struct mesh_path *mpath;
-	struct mpath_node *node;
-	struct hlist_head *bucket;
-	int hash_idx;
-	int err = 0;
-
-	/* flush relevant mpp entries first */
-	mpp_flush_by_proxy(sdata, addr);
-
-	read_lock_bh(&pathtbl_resize_lock);
-	tbl = resize_dereference_mpp_paths();
-	hash_idx = mesh_table_hash(addr, sdata, tbl);
-	bucket = &tbl->hash_buckets[hash_idx];
+  int err = 0;
 
-	spin_lock(&tbl->hashwlock[hash_idx]);
-	hlist_for_each_entry(node, bucket, list) {
-		mpath = node->mpath;
-		if (mpath->sdata == sdata &&
-		    ether_addr_equal(addr, mpath->dst)) {
-			__mesh_path_del(tbl, node);
-			goto enddel;
-		}
-	}
+  read_lock_bh(&pathtbl_resize_lock);
+  err = table_path_del(mpp_paths, sdata, addr);
+  mpp_paths_generation++;
+  read_unlock_bh(&pathtbl_resize_lock);
 
-	err = -ENXIO;
-enddel:
-	mesh_paths_generation++;
-	spin_unlock(&tbl->hashwlock[hash_idx]);
-	read_unlock_bh(&pathtbl_resize_lock);
-	return err;
+  return err;
 }
 
 /**
-- 
2.5.0

Re: [PATCH v2 3/3] mac80211: Unify mesh and mpp path removal function

[kbuild test robot]

Hi Henning,

[auto build test WARNING on mac80211-next/master]
[also build test WARNING on v4.5-rc1 next-20160129]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Henning-Rogge/mac80211-add-cleanup-path-for-MPP-table-entries/20160129-181045
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next.git master
reproduce:
        # apt-get install sparse
        make ARCH=x86_64 allmodconfig
        make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

   include/linux/compiler.h:228:8: sparse: attribute 'no_sanitize_address': unknown attribute
>> net/mac80211/mesh_pathtbl.c:60:10: sparse: incompatible types in comparison expression (different address spaces)
>> net/mac80211/mesh_pathtbl.c:60:10: sparse: incompatible types in comparison expression (different address spaces)
>> net/mac80211/mesh_pathtbl.c:60:10: sparse: incompatible types in comparison expression (different address spaces)
>> net/mac80211/mesh_pathtbl.c:60:10: sparse: incompatible types in comparison expression (different address spaces)
>> net/mac80211/mesh_pathtbl.c:60:10: sparse: incompatible types in comparison expression (different address spaces)
>> net/mac80211/mesh_pathtbl.c:60:10: sparse: incompatible types in comparison expression (different address spaces)
>> net/mac80211/mesh_pathtbl.c:60:10: sparse: incompatible types in comparison expression (different address spaces)
>> net/mac80211/mesh_pathtbl.c:60:10: sparse: incompatible types in comparison expression (different address spaces)
>> net/mac80211/mesh_pathtbl.c:60:10: sparse: incompatible types in comparison expression (different address spaces)

vim +60 net/mac80211/mesh_pathtbl.c

    44	static struct mesh_table __rcu *mpp_paths; /* Store paths for MPP&MAP */
    45	
    46	int mesh_paths_generation;
    47	int mpp_paths_generation;
    48	
    49	/* This lock will have the grow table function as writer and add / delete nodes
    50	 * as readers. RCU provides sufficient protection only when reading the table
    51	 * (i.e. doing lookups).  Adding or adding or removing nodes requires we take
    52	 * the read lock or we risk operating on an old table.  The write lock is only
    53	 * needed when modifying the number of buckets a table.
    54	 */
    55	static DEFINE_RWLOCK(pathtbl_resize_lock);
    56	
    57	
    58	static inline struct mesh_table *resize_dereference_paths(struct mesh_table *table)
    59	{
  > 60	  return rcu_dereference_protected(table,
    61	    lockdep_is_held(&pathtbl_resize_lock));
    62	}
    63	
    64	static inline struct mesh_table *resize_dereference_mesh_paths(void)
    65	{
    66		return resize_dereference_paths(mesh_paths);
    67	}
    68	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

Re: [PATCH v2 3/3] mac80211: Unify mesh and mpp path removal function

[Bob Copeland]

On Fri, Jan 29, 2016 at 11:08:58AM +0100, Henning Rogge wrote:
> mpp_path_del() and mesh_path_del() are mostly the same function.
> Move common code into a new static function.
> 
> Signed-off-by: Henning Rogge <henning.rogge@fkie.fraunhofer.de>
> ---
>  net/mac80211/mesh_pathtbl.c | 110 ++++++++++++++++++++++----------------------
>  1 file changed, 55 insertions(+), 55 deletions(-)
> 
> diff --git a/net/mac80211/mesh_pathtbl.c b/net/mac80211/mesh_pathtbl.c
> index 33bb1f25..86316ee 100644
> --- a/net/mac80211/mesh_pathtbl.c
> +++ b/net/mac80211/mesh_pathtbl.c
> @@ -55,16 +55,20 @@ int mpp_paths_generation;
>  static DEFINE_RWLOCK(pathtbl_resize_lock);
>  
>  
> +static inline struct mesh_table *resize_dereference_paths(struct mesh_table *table)

0-day caught this -- the parameter should be "struct mesh_table __rcu *table"

You can install sparse, set CONFIG_SPARSE_RCU_POINTER=y and then check it
with something like:

        make C=2 M=net/mac80211

The code is ok but the annotation catches cases where a dereference
would happen without the right lock -- in this case pathtable resize
lock is held.

> +{
> +  return rcu_dereference_protected(table,
> +    lockdep_is_held(&pathtbl_resize_lock));
> +}
> +

Besides that though, something is wrong with whitespace throughout
the patch, looks like lots of 2-space indents instead of tabs.

Approach looks fine though.  I might've moved the resize lock into
table_path_del, but it doesn't really matter as I plan to replace
with rhashtable anyway.

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

Re: [PATCH v2 3/3] mac80211: Unify mesh and mpp path removal function

[Bob Copeland]

On Fri, Jan 29, 2016 at 11:08:58AM +0100, Henning Rogge wrote:
> @@ -951,37 +974,14 @@ enddel:
>   */
>  static int mpp_path_del(struct ieee80211_sub_if_data *sdata, const u8 *addr)
>  {
> -	struct mesh_table *tbl;
> -	struct mesh_path *mpath;
> -	struct mpath_node *node;
> -	struct hlist_head *bucket;
> -	int hash_idx;
> -	int err = 0;
> -
> -	/* flush relevant mpp entries first */
> -	mpp_flush_by_proxy(sdata, addr);
> -

Is it intentional that mpp_path_del no longer calls mpp_flush_by_proxy()
while mesh_path_del does?

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

Re: [PATCH v2 3/3] mac80211: Unify mesh and mpp path removal function

[Henning Rogge]

On Sun, Jan 31, 2016 at 12:56 AM, Bob Copeland <me@bobcopeland.com> wrote:
> On Fri, Jan 29, 2016 at 11:08:58AM +0100, Henning Rogge wrote:
>> @@ -951,37 +974,14 @@ enddel:
>>   */
>>  static int mpp_path_del(struct ieee80211_sub_if_data *sdata, const u8 *addr)
>>  {
>> -     struct mesh_table *tbl;
>> -     struct mesh_path *mpath;
>> -     struct mpath_node *node;
>> -     struct hlist_head *bucket;
>> -     int hash_idx;
>> -     int err = 0;
>> -
>> -     /* flush relevant mpp entries first */
>> -     mpp_flush_by_proxy(sdata, addr);
>> -
>
> Is it intentional that mpp_path_del no longer calls mpp_flush_by_proxy()
> while mesh_path_del does?

You just found a bug in the original patch... the "mpp_flush_by_proxy"
is the function that erase all MPP entries going through a certain
MPath entry... so it is wrong in the mpp_path_del() function.

I will respin the series tomorrow.

Henning