* [PATCH] net/mac80211/mesh_plink: Convert timers to use
@ 2017-10-05 0:49 Kees Cook
2017-10-05 6:47 ` Johannes Berg
0 siblings, 1 reply; 3+ messages in thread
From: Kees Cook @ 2017-10-05 0:49 UTC (permalink / raw)
To: linux-kernel
Cc: Johannes Berg, David S. Miller, linux-wireless, netdev, Thomas Gleixner
In preparation for unconditionally passing the struct timer_list pointer to
all timer callbacks, switch to using the new timer_setup() and from_timer()
to pass the timer pointer explicitly. This requires adding a pointer back
to the sta_info since container_of() can't resolve the sta_info.
Cc: Johannes Berg <johannes@sipsolutions.net>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: linux-wireless@vger.kernel.org
Cc: netdev@vger.kernel.org
Cc: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
This requires commit 686fef928bba ("timer: Prepare to change timer
callback argument type") in v4.14-rc3, but should be otherwise
stand-alone.
---
net/mac80211/mesh_plink.c | 9 +++++----
net/mac80211/sta_info.c | 2 +-
net/mac80211/sta_info.h | 2 ++
3 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/net/mac80211/mesh_plink.c b/net/mac80211/mesh_plink.c
index f69c6c38ca43..fcc02beaee6d 100644
--- a/net/mac80211/mesh_plink.c
+++ b/net/mac80211/mesh_plink.c
@@ -604,8 +604,9 @@ void mesh_neighbour_update(struct ieee80211_sub_if_data *sdata,
ieee80211_mbss_info_change_notify(sdata, changed);
}
-static void mesh_plink_timer(unsigned long data)
+static void mesh_plink_timer(struct timer_list *t)
{
+ struct mesh_sta *mesh = from_timer(mesh, t, plink_timer);
struct sta_info *sta;
u16 reason = 0;
struct ieee80211_sub_if_data *sdata;
@@ -617,7 +618,7 @@ static void mesh_plink_timer(unsigned long data)
* del_timer_sync() this timer after having made sure
* it cannot be readded (by deleting the plink.)
*/
- sta = (struct sta_info *) data;
+ sta = mesh->plink_sta;
if (sta->sdata->local->quiescing)
return;
@@ -698,8 +699,8 @@ static void mesh_plink_timer(unsigned long data)
static inline void mesh_plink_timer_set(struct sta_info *sta, u32 timeout)
{
sta->mesh->plink_timer.expires = jiffies + msecs_to_jiffies(timeout);
- sta->mesh->plink_timer.data = (unsigned long) sta;
- sta->mesh->plink_timer.function = mesh_plink_timer;
+ sta->mesh->plink_sta = sta;
+ sta->mesh->plink_timer.function = (TIMER_FUNC_TYPE)mesh_plink_timer;
sta->mesh->plink_timeout = timeout;
add_timer(&sta->mesh->plink_timer);
}
diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index 69615016d5bf..5e5de9455e4e 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -332,7 +332,7 @@ struct sta_info *sta_info_alloc(struct ieee80211_sub_if_data *sdata,
spin_lock_init(&sta->mesh->plink_lock);
if (ieee80211_vif_is_mesh(&sdata->vif) &&
!sdata->u.mesh.user_mpm)
- init_timer(&sta->mesh->plink_timer);
+ timer_setup(&sta->mesh->plink_timer, NULL, 0);
sta->mesh->nonpeer_pm = NL80211_MESH_POWER_ACTIVE;
}
#endif
diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h
index 3acbdfa9f649..21d9760ce5c3 100644
--- a/net/mac80211/sta_info.h
+++ b/net/mac80211/sta_info.h
@@ -344,6 +344,7 @@ DECLARE_EWMA(mesh_fail_avg, 20, 8)
* @plink_state: peer link state
* @plink_timeout: timeout of peer link
* @plink_timer: peer link watch timer
+ * @plink_sta: peer link watch timer's sta_info
* @t_offset: timing offset relative to this host
* @t_offset_setpoint: reference timing offset of this sta to be used when
* calculating clockdrift
@@ -356,6 +357,7 @@ DECLARE_EWMA(mesh_fail_avg, 20, 8)
*/
struct mesh_sta {
struct timer_list plink_timer;
+ struct sta_info *plink_sta;
s64 t_offset;
s64 t_offset_setpoint;
--
2.7.4
--
Kees Cook
Pixel Security
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] net/mac80211/mesh_plink: Convert timers to use
2017-10-05 0:49 [PATCH] net/mac80211/mesh_plink: Convert timers to use Kees Cook
@ 2017-10-05 6:47 ` Johannes Berg
2017-10-05 17:27 ` Kees Cook
0 siblings, 1 reply; 3+ messages in thread
From: Johannes Berg @ 2017-10-05 6:47 UTC (permalink / raw)
To: Kees Cook, linux-kernel
Cc: David S. Miller, linux-wireless, netdev, Thomas Gleixner
On Wed, 2017-10-04 at 17:49 -0700, Kees Cook wrote:
> In preparation for unconditionally passing the struct timer_list
> pointer to all timer callbacks, switch to using the new timer_setup()
> and from_timer() to pass the timer pointer explicitly. This requires
> adding a pointer back to the sta_info since container_of() can't
> resolve the sta_info.
The subject seems to be lacking something ... :-)
> This requires commit 686fef928bba ("timer: Prepare to change timer
> callback argument type") in v4.14-rc3, but should be otherwise
> stand-alone.
I still can't apply that because that's not in net-next right now.
> static inline void mesh_plink_timer_set(struct sta_info *sta, u32
> timeout)
> {
> sta->mesh->plink_timer.expires = jiffies +
> msecs_to_jiffies(timeout);
> - sta->mesh->plink_timer.data = (unsigned long) sta;
> - sta->mesh->plink_timer.function = mesh_plink_timer;
> + sta->mesh->plink_sta = sta;
> + sta->mesh->plink_timer.function =
> (TIMER_FUNC_TYPE)mesh_plink_timer;
> sta->mesh->plink_timeout = timeout;
> add_timer(&sta->mesh->plink_timer);
Wouldn't it be better to convert this to timer_setup() now?
That add_timer() should probably also be mod_timer() anyway?
> diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
> index 69615016d5bf..5e5de9455e4e 100644
> --- a/net/mac80211/sta_info.c
> +++ b/net/mac80211/sta_info.c
> @@ -332,7 +332,7 @@ struct sta_info *sta_info_alloc(struct
> ieee80211_sub_if_data *sdata,
> spin_lock_init(&sta->mesh->plink_lock);
> if (ieee80211_vif_is_mesh(&sdata->vif) &&
> !sdata->u.mesh.user_mpm)
> - init_timer(&sta->mesh->plink_timer);
> + timer_setup(&sta->mesh->plink_timer, NULL,
> 0);
> sta->mesh->nonpeer_pm = NL80211_MESH_POWER_ACTIVE;
> }
You just have to make mesh_plink_timer() non-static, put a prototype
into mesh.h and then you can use the proper timer_setup() here with the
function?
Also, the sta->mesh->plink_sta assignment should be here I'd say, no
point rewriting it all the time.
I guess you were shooting for minimal, and I suppose we can do the
cleanups later too ...
johannes
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] net/mac80211/mesh_plink: Convert timers to use
2017-10-05 6:47 ` Johannes Berg
@ 2017-10-05 17:27 ` Kees Cook
0 siblings, 0 replies; 3+ messages in thread
From: Kees Cook @ 2017-10-05 17:27 UTC (permalink / raw)
To: Johannes Berg
Cc: LKML, David S. Miller, linux-wireless, Network Development,
Thomas Gleixner
On Wed, Oct 4, 2017 at 11:47 PM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> On Wed, 2017-10-04 at 17:49 -0700, Kees Cook wrote:
>> In preparation for unconditionally passing the struct timer_list
>> pointer to all timer callbacks, switch to using the new timer_setup()
>> and from_timer() to pass the timer pointer explicitly. This requires
>> adding a pointer back to the sta_info since container_of() can't
>> resolve the sta_info.
>
> The subject seems to be lacking something ... :-)
Oh wonderful, all the subjects are cut off. *sigh* I wonder which
piece of my workflow broke that...
>> This requires commit 686fef928bba ("timer: Prepare to change timer
>> callback argument type") in v4.14-rc3, but should be otherwise
>> stand-alone.
>
> I still can't apply that because that's not in net-next right now.
Okay, I'll see if Dave brings that into net-next and resend after that.
>> static inline void mesh_plink_timer_set(struct sta_info *sta, u32
>> timeout)
>> {
>> sta->mesh->plink_timer.expires = jiffies +
>> msecs_to_jiffies(timeout);
>> - sta->mesh->plink_timer.data = (unsigned long) sta;
>> - sta->mesh->plink_timer.function = mesh_plink_timer;
>> + sta->mesh->plink_sta = sta;
>> + sta->mesh->plink_timer.function =
>> (TIMER_FUNC_TYPE)mesh_plink_timer;
>> sta->mesh->plink_timeout = timeout;
>> add_timer(&sta->mesh->plink_timer);
>
> Wouldn't it be better to convert this to timer_setup() now?
The problem is that plink_timer is used in several places, and it's
originally initialized in net/mac80211/sta_info.c. The call to
mesh_plink_timer_set() does an update of the function field, so it
didn't look like it could get merged with the timer_setup(), but in
looking again, it seems that this is the _only_ update to
plink_timer.function, so it could probably get collapsed into the
timer_setup() call.
> That add_timer() should probably also be mod_timer() anyway?
Agreed. I'd avoided making those changes in most places, but I can do it here.
>> diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
>> index 69615016d5bf..5e5de9455e4e 100644
>> --- a/net/mac80211/sta_info.c
>> +++ b/net/mac80211/sta_info.c
>> @@ -332,7 +332,7 @@ struct sta_info *sta_info_alloc(struct
>> ieee80211_sub_if_data *sdata,
>> spin_lock_init(&sta->mesh->plink_lock);
>> if (ieee80211_vif_is_mesh(&sdata->vif) &&
>> !sdata->u.mesh.user_mpm)
>> - init_timer(&sta->mesh->plink_timer);
>> + timer_setup(&sta->mesh->plink_timer, NULL,
>> 0);
>> sta->mesh->nonpeer_pm = NL80211_MESH_POWER_ACTIVE;
>> }
>
> You just have to make mesh_plink_timer() non-static, put a prototype
> into mesh.h and then you can use the proper timer_setup() here with the
> function?
>
> Also, the sta->mesh->plink_sta assignment should be here I'd say, no
> point rewriting it all the time.
Sounds good. I'll get it cleaned up.
-Kees
--
Kees Cook
Pixel Security
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-10-05 17:27 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-05 0:49 [PATCH] net/mac80211/mesh_plink: Convert timers to use Kees Cook
2017-10-05 6:47 ` Johannes Berg
2017-10-05 17:27 ` Kees Cook
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.