Hi Shinoda, Thanks for the effort involved in these patches.
On Sun, Sep 2, 2012 at 1:20 PM, LEO Airwarosu Yoichi Shinoda <[email protected]> wrote: > > An companion kernel patch to go with the iw-3.3 patch > (Message-Id: <[email protected]>) > > The design of the patch is really ugly to avoid adding > additional cfg80211 calls; > the net/mac80211/mesh_pathtbl.c:mesh_path_lookup_by_idx() > was expanded to loop through mpp_paths and known_gates > lists in addition to the original mesh_paths. known_gates is just an optimization to avoid iterating over all mpaths when looking for all gates, so you can avoid treating them as separate mpath entries. The unattractive nature of this patch seems like a sign that we can simplify the mpath storage / semantics. mpath->dst would point to either a mesh destination, a mesh gate, or a proxied destination. We could then get rid of the mpp path table entirely. I guess we should also take care with how proxied paths are handled by the hwmp code. > Despite the effort, and quite ironically, the > include/net/cfg80211.h had to be changed to increase > the mpath_info.flag bits (sigh). > > --- shinoda > > --- > diff --git a/include/linux/nl80211.h b/include/linux/nl80211.h > index 2f38788..9e5a467 100644 > --- a/include/linux/nl80211.h > +++ b/include/linux/nl80211.h > @@ -1789,6 +1789,10 @@ enum nl80211_mpath_flags { > NL80211_MPATH_FLAG_SN_VALID = 1<<2, > NL80211_MPATH_FLAG_FIXED = 1<<3, > NL80211_MPATH_FLAG_RESOLVED = 1<<4, > + NL80211_MPATH_FLAG_IS_ROOT = 1<<5, > + NL80211_MPATH_FLAG_IS_GATE = 1<<6, > + NL80211_MPATH_FLAG_IS_MPPPATH = 1<<7, > + NL80211_MPATH_FLAG_IS_KNOWN_GATE = 1<<8, > }; > > /** > diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h > index 3d254e1..a79fe15 100644 > --- a/include/net/cfg80211.h > +++ b/include/net/cfg80211.h > @@ -780,7 +780,7 @@ struct mpath_info { > u32 exptime; > u32 discovery_timeout; > u8 discovery_retries; > - u8 flags; > + u32 flags; > > int generation; > }; > diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c > index d41974a..72d4317 100644 > --- a/net/mac80211/cfg.c > +++ b/net/mac80211/cfg.c > @@ -1373,11 +1373,15 @@ static void mpath_set_pinfo(struct mesh_path *mpath, > u8 *next_hop, > { > struct sta_info *next_hop_sta = rcu_dereference(mpath->next_hop); > > - if (next_hop_sta) > + if (mpath->is_mpppath) > + memcpy(next_hop, mpath->mpp, ETH_ALEN); > + else if (next_hop_sta) > memcpy(next_hop, next_hop_sta->sta.addr, ETH_ALEN); > else > memset(next_hop, 0, ETH_ALEN); > > + memset(pinfo, 0, sizeof(struct mpath_info)); > + > pinfo->generation = mesh_paths_generation; > > pinfo->filled = MPATH_INFO_FRAME_QLEN | > @@ -1396,7 +1400,6 @@ static void mpath_set_pinfo(struct mesh_path *mpath, u8 > *next_hop, > pinfo->discovery_timeout = > jiffies_to_msecs(mpath->discovery_timeout); > pinfo->discovery_retries = mpath->discovery_retries; > - pinfo->flags = 0; > if (mpath->flags & MESH_PATH_ACTIVE) > pinfo->flags |= NL80211_MPATH_FLAG_ACTIVE; > if (mpath->flags & MESH_PATH_RESOLVING) > @@ -1405,10 +1408,16 @@ static void mpath_set_pinfo(struct mesh_path *mpath, > u8 *next_hop, > pinfo->flags |= NL80211_MPATH_FLAG_SN_VALID; > if (mpath->flags & MESH_PATH_FIXED) > pinfo->flags |= NL80211_MPATH_FLAG_FIXED; > - if (mpath->flags & MESH_PATH_RESOLVING) > - pinfo->flags |= NL80211_MPATH_FLAG_RESOLVING; > - > - pinfo->flags = mpath->flags; > + if (mpath->flags & MESH_PATH_RESOLVED) > + pinfo->flags |= NL80211_MPATH_FLAG_RESOLVED; > + if (mpath->is_root) > + pinfo->flags |= NL80211_MPATH_FLAG_IS_ROOT; > + if (mpath->is_gate) > + pinfo->flags |= NL80211_MPATH_FLAG_IS_GATE; > + if (mpath->is_mpppath) > + pinfo->flags |= NL80211_MPATH_FLAG_IS_MPPPATH; > + if (mpath->is_known_gate) > + pinfo->flags |= NL80211_MPATH_FLAG_IS_KNOWN_GATE; > } > > static int ieee80211_get_mpath(struct wiphy *wiphy, struct net_device *dev, > @@ -1423,8 +1432,11 @@ static int ieee80211_get_mpath(struct wiphy *wiphy, > struct net_device *dev, > rcu_read_lock(); > mpath = mesh_path_lookup(dst, sdata); > if (!mpath) { > - rcu_read_unlock(); > - return -ENOENT; > + mpath = mpp_path_lookup(dst, sdata); > + if (!mpath) { > + rcu_read_unlock(); > + return -ENOENT; > + } > } > memcpy(dst, mpath->dst, ETH_ALEN); > mpath_set_pinfo(mpath, next_hop, pinfo); > diff --git a/net/mac80211/mesh.h b/net/mac80211/mesh.h > index 13fd5b5..fe68140 100644 > --- a/net/mac80211/mesh.h > +++ b/net/mac80211/mesh.h > @@ -135,6 +135,8 @@ struct mesh_path { > unsigned long last_preq_to_root; > bool is_root; > bool is_gate; > + bool is_mpppath; > + bool is_known_gate; I know there is legacy code using these is_* bools, but maybe these should just be mpath->flags bits? Also why is is_known_gate needed in addition to is_gate? > }; > > /** > diff --git a/net/mac80211/mesh_pathtbl.c b/net/mac80211/mesh_pathtbl.c > index 075bc53..56ad9e1 100644 > --- a/net/mac80211/mesh_pathtbl.c > +++ b/net/mac80211/mesh_pathtbl.c > @@ -399,10 +399,43 @@ struct mesh_path *mesh_path_lookup_by_idx(int idx, > struct ieee80211_sub_if_data > node->mpath->flags &= ~MESH_PATH_ACTIVE; > spin_unlock_bh(&node->mpath->state_lock); > } > + node->mpath->is_known_gate = false; // XXX Ugly... Yes. > return node->mpath; > } > } > > + tbl = rcu_dereference(mpp_paths); > + > + for_each_mesh_entry(tbl, p, node, i) { > + if (sdata && node->mpath->sdata != sdata) > + continue; > + if (j++ == idx) { > + // mpp paths do not have expiration? > + if (MPATH_EXPIRED(node->mpath)) { > + spin_lock_bh(&node->mpath->state_lock); > + node->mpath->flags &= ~MESH_PATH_ACTIVE; > + spin_unlock_bh(&node->mpath->state_lock); > + } > + node->mpath->is_known_gate = false; // XXX Ugly... > + return node->mpath; > + } > + } If mesh and proxy destinations were in the same table you wouldn't need this hunk at all. There are quite a few other places where we access both tables sequentially which could also be simplified. > + tbl = rcu_dereference(mesh_paths); > + > + { > + struct mpath_node *gate; > + struct hlist_node *n; > + > + hlist_for_each_entry_rcu(gate, n, tbl->known_gates, list) > + { > + if (j++ == idx) { > + gate->mpath->is_known_gate = true; > + return gate->mpath; > + } > + } > + } Don't need this hunk (or _shouldn't_ :)). > + > return NULL; > } > > @@ -535,6 +568,7 @@ int mesh_path_add(u8 *dst, struct ieee80211_sub_if_data > *sdata) > new_mpath->is_root = false; > new_mpath->sdata = sdata; > new_mpath->flags = 0; > + new_mpath->is_mpppath = false; > skb_queue_head_init(&new_mpath->frame_queue); > new_node->mpath = new_mpath; > new_mpath->timer.data = (unsigned long) new_mpath; > @@ -666,6 +700,7 @@ int mpp_path_add(u8 *dst, u8 *mpp, struct > ieee80211_sub_if_data *sdata) > memcpy(new_mpath->mpp, mpp, ETH_ALEN); > new_mpath->sdata = sdata; > new_mpath->flags = 0; > + new_mpath->is_mpppath = true; > skb_queue_head_init(&new_mpath->frame_queue); > new_node->mpath = new_mpath; > init_timer(&new_mpath->timer); > diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c > index 97026f3..1d0280d 100644 > --- a/net/wireless/nl80211.c > +++ b/net/wireless/nl80211.c > @@ -3314,7 +3314,7 @@ static int nl80211_send_mpath(struct sk_buff *msg, u32 > pid, u32 seq, > nla_put_u32(msg, NL80211_MPATH_INFO_EXPTIME, > pinfo->exptime)) || > ((pinfo->filled & MPATH_INFO_FLAGS) && > - nla_put_u8(msg, NL80211_MPATH_INFO_FLAGS, > + nla_put_u32(msg, NL80211_MPATH_INFO_FLAGS, > pinfo->flags)) || > ((pinfo->filled & MPATH_INFO_DISCOVERY_TIMEOUT) && > nla_put_u32(msg, NL80211_MPATH_INFO_DISCOVERY_TIMEOUT, > --- > > _______________________________________________ > Devel mailing list > [email protected] > http://lists.open80211s.org/cgi-bin/mailman/listinfo/devel _______________________________________________ Devel mailing list [email protected] http://lists.open80211s.org/cgi-bin/mailman/listinfo/devel
