| Message ID | 1485561708-31559-3-git-send-email-rmanohar@qca.qualcomm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Johannes Berg |
| Headers | show |
Hi Rajkumar, Thanks this looks good, but.. On Fri, Jan 27, 2017 at 4:01 PM, Rajkumar Manoharan <rmanohar@qca.qualcomm.com> wrote: > Mesh moving average should be cleared, whenever mesh paths > to the given station are deactivated due to bad link. This will > give enough room to analysis more tx status than retaining the > current average. > > Signed-off-by: Rajkumar Manoharan <rmanohar@qca.qualcomm.com> > --- > net/mac80211/mesh_pathtbl.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/net/mac80211/mesh_pathtbl.c b/net/mac80211/mesh_pathtbl.c > index f0e6175a9821..208ad36c0a7f 100644 > --- a/net/mac80211/mesh_pathtbl.c > +++ b/net/mac80211/mesh_pathtbl.c > @@ -510,6 +510,7 @@ void mesh_plink_broken(struct sta_info *sta) > struct mesh_path *mpath; > struct rhashtable_iter iter; > int ret; > + bool paths_deactivated = false; > > ret = rhashtable_walk_init(&tbl->rhead, &iter, GFP_ATOMIC); > if (ret) > @@ -535,8 +536,11 @@ void mesh_plink_broken(struct sta_info *sta) > sdata->u.mesh.mshcfg.element_ttl, > mpath->dst, mpath->sn, > WLAN_REASON_MESH_PATH_DEST_UNREACHABLE, bcast); > + paths_deactivated = true; > } > } > + if (paths_deactivated) > + sta->mesh->fail_avg = 0; .. why this indirection? Just reset mesh->fail_avg unconditionally in this function?
On 2017-01-31 09:51, Thomas Pedersen wrote: > Hi Rajkumar, > > Thanks this looks good, but.. > > On Fri, Jan 27, 2017 at 4:01 PM, Rajkumar Manoharan > <rmanohar@qca.qualcomm.com> wrote: >> Mesh moving average should be cleared, whenever mesh paths >> to the given station are deactivated due to bad link. This will >> give enough room to analysis more tx status than retaining the >> current average. >> [...] >> ret = rhashtable_walk_init(&tbl->rhead, &iter, GFP_ATOMIC); >> if (ret) >> @@ -535,8 +536,11 @@ void mesh_plink_broken(struct sta_info *sta) >> sdata->u.mesh.mshcfg.element_ttl, >> mpath->dst, mpath->sn, >> >> WLAN_REASON_MESH_PATH_DEST_UNREACHABLE, bcast); >> + paths_deactivated = true; >> } >> } >> + if (paths_deactivated) >> + sta->mesh->fail_avg = 0; > > .. why this indirection? Just reset mesh->fail_avg unconditionally in > this function? > Hmm... As fixed paths are not affected, resetting fail_avg for fixed path may give wrong metric. no? -Rajkumar
On Tue, Jan 31, 2017 at 11:33 AM, Rajkumar Manoharan <rmanohar@codeaurora.org> wrote: > On 2017-01-31 09:51, Thomas Pedersen wrote: >> >> Hi Rajkumar, >> >> Thanks this looks good, but.. >> >> On Fri, Jan 27, 2017 at 4:01 PM, Rajkumar Manoharan >> <rmanohar@qca.qualcomm.com> wrote: >>> >>> Mesh moving average should be cleared, whenever mesh paths >>> to the given station are deactivated due to bad link. This will >>> give enough room to analysis more tx status than retaining the >>> current average. >>> > [...] >>> >>> ret = rhashtable_walk_init(&tbl->rhead, &iter, GFP_ATOMIC); >>> if (ret) >>> @@ -535,8 +536,11 @@ void mesh_plink_broken(struct sta_info *sta) >>> sdata->u.mesh.mshcfg.element_ttl, >>> mpath->dst, mpath->sn, >>> WLAN_REASON_MESH_PATH_DEST_UNREACHABLE, >>> bcast); >>> + paths_deactivated = true; >>> } >>> } >>> + if (paths_deactivated) >>> + sta->mesh->fail_avg = 0; >> >> >> .. why this indirection? Just reset mesh->fail_avg unconditionally in >> this function? >> > Hmm... As fixed paths are not affected, resetting fail_avg for fixed path > may give > wrong metric. no? I don't think setting fail_avg = 0 will affect existing fixed mpath code. Anyway metrics should not affect whether a fixed mpath is chosen.
On 2017-01-31 11:46, Thomas Pedersen wrote: > On Tue, Jan 31, 2017 at 11:33 AM, Rajkumar Manoharan wrote: >> On 2017-01-31 09:51, Thomas Pedersen wrote: >>> [...] >>>> + if (paths_deactivated) >>>> + sta->mesh->fail_avg = 0; >>> >>> >>> .. why this indirection? Just reset mesh->fail_avg unconditionally in >>> this function? >>> >> Hmm... As fixed paths are not affected, resetting fail_avg for fixed >> path >> may give >> wrong metric. no? > > I don't think setting fail_avg = 0 will affect existing fixed mpath > code. Anyway metrics should not > affect whether a fixed mpath is chosen. > Great. Will remove conditional check. Thanks for your feedback Thomas. -Rajkumar
diff --git a/net/mac80211/mesh_pathtbl.c b/net/mac80211/mesh_pathtbl.c index f0e6175a9821..208ad36c0a7f 100644 --- a/net/mac80211/mesh_pathtbl.c +++ b/net/mac80211/mesh_pathtbl.c @@ -510,6 +510,7 @@ void mesh_plink_broken(struct sta_info *sta) struct mesh_path *mpath; struct rhashtable_iter iter; int ret; + bool paths_deactivated = false; ret = rhashtable_walk_init(&tbl->rhead, &iter, GFP_ATOMIC); if (ret) @@ -535,8 +536,11 @@ void mesh_plink_broken(struct sta_info *sta) sdata->u.mesh.mshcfg.element_ttl, mpath->dst, mpath->sn, WLAN_REASON_MESH_PATH_DEST_UNREACHABLE, bcast); + paths_deactivated = true; } } + if (paths_deactivated) + sta->mesh->fail_avg = 0; out: rhashtable_walk_stop(&iter); rhashtable_walk_exit(&iter);
Mesh moving average should be cleared, whenever mesh paths to the given station are deactivated due to bad link. This will give enough room to analysis more tx status than retaining the current average. Signed-off-by: Rajkumar Manoharan <rmanohar@qca.qualcomm.com> --- net/mac80211/mesh_pathtbl.c | 4 ++++ 1 file changed, 4 insertions(+)