Unify mpp/mesh_path handling for Mac 802.11s
RFC PATCH v2 1/1 Unify mpp/mesh_path handling for Mac 802.11s
Hi,
I looked a little bit more into the 802.11s mesh code to see if there is still a chance for unifying some code without forcing the issue as the last patch did.
The result was the attached patch, which still needs one "if (mpp) ... else" block but unifies most of the code for mpp_path_add() and mesh_path_add().
If this is still too much I would suggest leaving the mesh_pathtbl code as it is.
While looking through the code I noticed that there seems to be no code-path that removes stall mpp_path entries (except for the removal of the mesh interface) ! Unless I overlooked something this would be a way to use up all existing memory of the kernel by sending it 802.11s packets with changing proxied addresses.
I have added an atomic counter to restrict the number of proxied paths (similar to the restriction of the mesh paths), but I am not convinced that this would be enough. There needs to be some timeout mechanism, but resetting the timeout of a MPP every times we receive an unicast from it sounds expensive.
Henning Rogge
net/mac80211/ieee80211_i.h +1 -0
@@ -580,6 +580,7 @@ struct ieee80211_if_mesh {/* Last used PREQ ID */ u32 preq_id;atomic_t mpaths; +atomic_t mpps;/* Timestamp of last SN update */ unsigned long last_sn_update;/* Time when it's ok to send next PERR */
net/mac80211/mesh.c +1 -0
@@ -1324,6 +1324,7 @@ void ieee80211_mesh_init_sdata(struct ieee80211_sub_if_data *sdata) ifmsh->accepting_plinks = true;atomic_set (&ifmsh->mpaths,0 ); +atomic_set (&ifmsh->mpps,0 );mesh_rmc_init (sdata); ifmsh->last_preq = jiffies; ifmsh->next_perr = jiffies;
net/mac80211/mesh.h +6 -1
@@ -200,6 +200,9 @@ struct mesh_rmc {/* Maximum number of paths per interface */ #define MESH_MAX_MPATHS 1024 +/* Maximum number of proxied paths per interface */ +#define MESH_MAX_MPPS 1024 +/* Number of frames buffered per destination for unresolved destinations */ #define MESH_FRAME_QUEUE_LEN 10
@@ -266,10 +269,12 @@ struct mesh_path *mesh_path_lookup(struct ieee80211_sub_if_data *sdata, const u8 *dst);struct mesh_path *mpp_path_lookup (struct ieee80211_sub_if_data *sdata,const u8 *dst); -int mpp_path_add (struct ieee80211_sub_if_data *sdata, +struct mesh_path *mpp_path_add (struct ieee80211_sub_if_data *sdata,const u8 *dst,const u8 *mpp);struct mesh_path *mesh_path_lookup_by_idx (struct ieee80211_sub_if_data *sdata,int idx); +struct mesh_path * +mpp_lookup_by_idx (struct ieee80211_sub_if_data *sdata,int idx);void mesh_path_fix_nexthop (struct mesh_path *mpath,struct sta_info *next_hop);void mesh_path_expire (struct ieee80211_sub_if_data *sdata); void mesh_rx_path_sel_frame(struct ieee80211_sub_if_data *sdata,
net/mac80211/mesh_pathtbl.c +74 -121
250 lines not on this page · raw message
Quoted 5 lines
While looking through the code I noticed that there seems to be no code-path that removes stall
mpp_pathentries (except for the removal of the mesh interface) ! Unless I overlooked something this would be a way to use up all existing memory of the kernel by sending it 802.11s packets with changing proxied addresses.
AFAIK, we don't really "manage" the mpp path and relying on incoming packet to update the mpp path table.
----
Chun-Yeow
Quoted 8 lines · Yeoh Chun-Yeow
Quoted 5 lines
While looking through the code I noticed that there seems to be no code-path that removes stall
mpp_pathentries (except for the removal of the mesh interface) ! Unless I overlooked something this would be a way to use up all existing memory of the kernel by sending it 802.11s packets with changing proxied addresses.AFAIK, we don't really "manage" the mpp path and relying on incoming packet to update the mpp path table.
Imagine a malicious or maybe misbehaving node that is sending you proxied traffic and is just counting up the source mac address. Every received packet would generate another entry in the mpp_path table.
Even without this, one usecase for 802.11s is to connect multiple access points and allow clients to roam between them and not loosing internet connectivity through a gateway. A scenario like this would have one mpp_path table entry for every client that has connected to the network... even if it has been years ago.
Henning Rogge
A message that is not in the archive (sent off-list or not yet received): CAEFj984PE5L2Yv4r6nshAy6Zuama5wsins27xeX9A1B4RGqmgg@mail.gmail.com
Quoted 2 lines · Yeoh Chun-Yeow
Agree. Maybe a timer to delete the expired mpp table and just let the incoming received packet to reinsert the table?
I will prepare a second cleanup patch for the 802.11s code.
Henning Rogge
Johannes Berg
maintainerQuoted 17 lines · Henning Rogge
Hi,
I looked a little bit more into the 802.11s mesh code to see if there is still a chance for unifying some code without forcing the issue as the last patch did.
The result was the attached patch, which still needs one "if (mpp) ... else" block but unifies most of the code for
mpp_path_add()andmesh_path_add().If this is still too much I would suggest leaving the
mesh_pathtblcode as it is.While looking through the code I noticed that there seems to be no code-path that removes stall
mpp_pathentries (except for the removal of the mesh interface) ! Unless I overlooked something this would be a way to use up all existing memory of the kernel by sending it 802.11s packets with changing proxied addresses.
Quoted 5 lines
I have added an atomic counter to restrict the number of proxied paths (similar to the restriction of the mesh paths), but I am not convinced that this would be enough. There needs to be some timeout mechanism, but resetting the timeout of a MPP every times we receive an unicast from it sounds expensive.
Can you not do everything in one patch? :)
Also - I think reporting the MPPs to userspace like the mesh paths is probably not a good idea? And I think that happens now since you didn't touch the code in cfg?
johannes
johannes
Quoted 5 lines · Johannes Berg
Can you not do everything in one patch? :)
Also - I think reporting the MPPs to userspace like the mesh paths is probably not a good idea? And I think that happens now since you didn't touch the code in cfg?
The current patch ONLY unifies the mpp_path_add() and mesh_path_add() function. But I could factor out the addition of the atomic counter for mpp_paths into a patch of its own.
I kept the "exporting mpp" thing for a followup patch, I first wanted to hear if the whole cleanup is worth the effort.
Henning Rogge
Johannes Berg
maintainerQuoted 11 lines · Henning Rogge
Quoted 5 lines · Johannes Berg
Can you not do everything in one patch? :)
Also - I think reporting the MPPs to userspace like the mesh paths is probably not a good idea? And I think that happens now since you didn't touch the code in cfg?
The current patch ONLY unifies the
mpp_path_add()andmesh_path_add()function. But I could factor out the addition of the atomic counter formpp_pathsinto a patch of its own.
Please.
Quoted 2 lines
I kept the "exporting mpp" thing for a followup patch, I first wanted to hear if the whole cleanup is worth the effort.
Ok. Also, please read CodingStyle.
johannes
Quoted 5 lines · Johannes Berg
Quoted 3 lines
The current patch ONLY unifies the
mpp_path_add()andmesh_path_add()function. But I could factor out the addition of the atomic counter formpp_pathsinto a patch of its own.Please.
*sigh* new version of the patch will be coming later...
the existing atomic counter thing in mesh_pathtbl.c is slightly broken and I have to look how to fix it before I can think about the unification thing again.
Maybe I will try to work on the nl80211 addition for mpp_dumps in parallel, but all the "tracing event" code looks still quite obscure to me.
Henning Rogge