Skip to contentSkip to conversation

net: tipc: Convert timers to use timer_setup()

PATCH In review Kees Cook 2 participants 5 messages Oct 24, 2017 – Oct 30, 2017

Subsystems (3)
lore
Share & tools
lore

Kees Cook

author
#0 lore raw

PATCH 1/1 net: tipc: Convert timers to use timer_setup()

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.

---
 net/tipc/discover.c |  6 +++---
 net/tipc/monitor.c  |  6 +++---
 net/tipc/node.c     |  8 ++++----
 net/tipc/socket.c   | 10 +++++-----
 net/tipc/subscr.c   |  6 +++---
 5 files changed, 18 insertions(+), 18 deletions(-)
net/tipc/discover.c +3 -3
index 02462d67d191..92e4828c6b09 100644
@@ -224,9 +224,9 @@ void tipc_disc_remove_dest(struct tipc_link_req *req)
  *
  * Called whenever a link setup request timer associated with a bearer expires.
  */
-static void disc_timeout(unsigned long data)
+static void disc_timeout(struct timer_list *t)
 {
-	struct tipc_link_req *req = (struct tipc_link_req *)data;
+	struct tipc_link_req *req = from_timer(req, t, timer);
 	struct sk_buff *skb;
 	int max_delay;
 
@@ -292,7 +292,7 @@ int tipc_disc_create(struct net *net, struct tipc_bearer *b,
 	req->num_nodes = 0;
 	req->timer_intv = TIPC_LINK_REQ_INIT;
 	spin_lock_init(&req->lock);
-	setup_timer(&req->timer, disc_timeout, (unsigned long)req);
+	timer_setup(&req->timer, disc_timeout, 0);
 	mod_timer(&req->timer, jiffies + req->timer_intv);
 	b->link_req = req;
 	*skb = skb_clone(req->buf, GFP_ATOMIC);
net/tipc/monitor.c +3 -3
index 9e109bb1a207..b9c32557d73c 100644
@@ -578,9 +578,9 @@ void tipc_mon_get_state(struct net *net, u32 addr,
 	read_unlock_bh(&mon->lock);
 }
 
-static void mon_timeout(unsigned long m)
+static void mon_timeout(struct timer_list *t)
 {
-	struct tipc_monitor *mon = (void *)m;
+	struct tipc_monitor *mon = from_timer(mon, t, timer);
 	struct tipc_peer *self;
 	int best_member_cnt = dom_size(mon->peer_cnt) - 1;
 
@@ -623,7 +623,7 @@ int tipc_mon_create(struct net *net, int bearer_id)
 	self->is_up = true;
 	self->is_head = true;
 	INIT_LIST_HEAD(&self->list);
-	setup_timer(&mon->timer, mon_timeout, (unsigned long)mon);
+	timer_setup(&mon->timer, mon_timeout, 0);
 	mon->timer_intv = msecs_to_jiffies(MON_TIMEOUT + (tn->random & 0xffff));
 	mod_timer(&mon->timer, jiffies + mon->timer_intv);
 	return 0;
net/tipc/node.c +4 -4
index 89f8ac73bf65..009a81631280 100644
@@ -153,7 +153,7 @@ static void tipc_node_link_down(struct tipc_node *n, int bearer_id,
 				bool delete);
 static void node_lost_contact(struct tipc_node *n, struct sk_buff_head *inputq);
 static void tipc_node_delete(struct tipc_node *node);
-static void tipc_node_timeout(unsigned long data);
+static void tipc_node_timeout(struct timer_list *t);
 static void tipc_node_fsm_evt(struct tipc_node *n, int evt);
 static struct tipc_node *tipc_node_find(struct net *net, u32 addr);
 static void tipc_node_put(struct tipc_node *node);
@@ -361,7 +361,7 @@ struct tipc_node *tipc_node_create(struct net *net, u32 addr, u16 capabilities)
 		goto exit;
 	}
 	tipc_node_get(n);
-	setup_timer(&n->timer, tipc_node_timeout, (unsigned long)n);
+	timer_setup(&n->timer, tipc_node_timeout, 0);
 	n->keepalive_intv = U32_MAX;
 	hlist_add_head_rcu(&n->hash, &tn->node_htable[tipc_hashfn(addr)]);
 	list_for_each_entry_rcu(temp_node, &tn->node_list, list) {
@@ -500,9 +500,9 @@ void tipc_node_remove_conn(struct net *net, u32 dnode, u32 port)
 
 /* tipc_node_timeout - handle expiration of node timer
  */
-static void tipc_node_timeout(unsigned long data)
+static void tipc_node_timeout(struct timer_list *t)
 {
-	struct tipc_node *n = (struct tipc_node *)data;
+	struct tipc_node *n = from_timer(n, t, timer);
 	struct tipc_link_entry *le;
 	struct sk_buff_head xmitq;
 	int bearer_id;
net/tipc/socket.c +5 -5
index b3b72d8e9543..6387839f643d 100644
@@ -125,7 +125,7 @@ static void tipc_sock_destruct(struct sock *sk);
 static int tipc_release(struct socket *sock);
 static int tipc_accept(struct socket *sock, struct socket *new_sock, int flags,
 		       bool kern);
-static void tipc_sk_timeout(unsigned long data);
+static void tipc_sk_timeout(struct timer_list *t);
 static int tipc_sk_publish(struct tipc_sock *tsk, uint scope,
 			   struct tipc_name_seq const *seq);
 static int tipc_sk_withdraw(struct tipc_sock *tsk, uint scope,
@@ -464,7 +464,7 @@ static int tipc_sk_create(struct net *net, struct socket *sock,
 		      NAMED_H_SIZE, 0);
 
 	msg_set_origport(msg, tsk->portid);
-	setup_timer(&sk->sk_timer, tipc_sk_timeout, (unsigned long)tsk);
+	timer_setup(&sk->sk_timer, tipc_sk_timeout, 0);
 	sk->sk_shutdown = 0;
 	sk->sk_backlog_rcv = tipc_sk_backlog_rcv;
 	sk->sk_rcvbuf = sysctl_tipc_rmem[1];
@@ -2530,14 +2530,14 @@ static int tipc_shutdown(struct socket *sock, int how)
 	return res;
 }
 
-static void tipc_sk_timeout(unsigned long data)
+static void tipc_sk_timeout(struct timer_list *t)
 {
-	struct tipc_sock *tsk = (struct tipc_sock *)data;
+	struct sock *sk = from_timer(sk, t, sk_timer);
+	struct tipc_sock *tsk = tipc_sk(sk);
 	u32 peer_port = tsk_peer_port(tsk);
 	u32 peer_node = tsk_peer_node(tsk);
 	u32 own_node = tsk_own_node(tsk);
 	u32 own_port = tsk->portid;
-	struct sock *sk = &tsk->sk;
 	struct net *net = sock_net(sk);
 	struct sk_buff *skb = NULL;
 
net/tipc/subscr.c +3 -3
index be3d9e3183dc..251065dfd8df 100644
@@ -133,9 +133,9 @@ void tipc_subscrp_report_overlap(struct tipc_subscription *sub, u32 found_lower,
 				node);
 }
 
-static void tipc_subscrp_timeout(unsigned long data)
+static void tipc_subscrp_timeout(struct timer_list *t)
 {
-	struct tipc_subscription *sub = (struct tipc_subscription *)data;
+	struct tipc_subscription *sub = from_timer(sub, t, timer);
 	struct tipc_subscriber *subscriber = sub->subscriber;
 
 	spin_lock_bh(&subscriber->lock);
@@ -303,7 +303,7 @@ static void tipc_subscrp_subscribe(struct net *net, struct tipc_subscr *s,
 	tipc_subscrb_get(subscriber);
 	spin_unlock_bh(&subscriber->lock);
 
-	setup_timer(&sub->timer, tipc_subscrp_timeout, (unsigned long)sub);
+	timer_setup(&sub->timer, tipc_subscrp_timeout, 0);
 	timeout = htohl(sub->evt.s.timeout, swap);
 
 	if (timeout != TIPC_WAIT_FOREVER)
Signature · 6 lines
2.7.4


-- 
Kees Cook
Pixel Security
#1 lore raw

NAK. It doesn't sound like a good idea to send this to net. Especially since one of these timers has already been refactored in net-next.

///jon

Quoted 194 lines

-----Original Message-----
From: Kees Cook [mailto:[email protected]]
Sent: Tuesday, October 24, 2017 10:47
To: David S. Miller <[email protected]>
Cc: Jon Maloy <[email protected]>; Ying Xue <[email protected]>; [email protected]; tipc- [email protected]; [email protected]
Subject: [PATCH] net: tipc: Convert timers to use timer_setup()

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.

---
 net/tipc/discover.c |  6 +++---
 net/tipc/monitor.c  |  6 +++---
 net/tipc/node.c     |  8 ++++----
 net/tipc/socket.c   | 10 +++++-----
 net/tipc/subscr.c   |  6 +++---
 5 files changed, 18 insertions(+), 18 deletions(-)
net/tipc/discover.c index renamed from net/tipc/discover.c +0 -0

02462d67d191..92e4828c6b09 100644

net/tipc/discover.c +0 -0
@@ -224,9 +224,9 @@ void tipc_disc_remove_dest(struct tipc_link_req
 *req)
  *
  * Called whenever a link setup request timer associated with a bearer

expires.
*/

net/tipc/discover.c +3 -3
-static void disc_timeout(unsigned long data)
+static void disc_timeout(struct timer_list *t)
 {
-	struct tipc_link_req *req = (struct tipc_link_req *)data;
+	struct tipc_link_req *req = from_timer(req, t, timer);
 	struct sk_buff *skb;
 	int max_delay;
@@ -292,7 +292,7 @@ int tipc_disc_create(struct net *net, struct
 tipc_bearer *b,
 	req->num_nodes = 0;
 	req->timer_intv = TIPC_LINK_REQ_INIT;
 	spin_lock_init(&req->lock);
-	setup_timer(&req->timer, disc_timeout, (unsigned long)req);
+	timer_setup(&req->timer, disc_timeout, 0);
 	mod_timer(&req->timer, jiffies + req->timer_intv);
 	b->link_req = req;

*skb = skb_clone(req->buf, GFP_ATOMIC); diff --git
a/net/tipc/monitor.c b/net/tipc/monitor.c index 9e109bb1a207..b9c32557d73c 100644

net/tipc/monitor.c +3 -3
@@ -578,9 +578,9 @@ void tipc_mon_get_state(struct net *net, u32 addr,
 	read_unlock_bh(&mon->lock);
 }
 
-static void mon_timeout(unsigned long m)
+static void mon_timeout(struct timer_list *t)
 {
-	struct tipc_monitor *mon = (void *)m;
+	struct tipc_monitor *mon = from_timer(mon, t, timer);
 	struct tipc_peer *self;
 	int best_member_cnt = dom_size(mon->peer_cnt) - 1;
 
@@ -623,7 +623,7 @@ int tipc_mon_create(struct net *net, int bearer_id)
 	self->is_up = true;
 	self->is_head = true;
 	INIT_LIST_HEAD(&self->list);
-	setup_timer(&mon->timer, mon_timeout, (unsigned long)mon);
+	timer_setup(&mon->timer, mon_timeout, 0);
 	mon->timer_intv = msecs_to_jiffies(MON_TIMEOUT + (tn->random
 & 0xffff));
 	mod_timer(&mon->timer, jiffies + mon->timer_intv);
net/tipc/monitor.c +0 -0
 	return 0;
net/tipc/node.c index renamed from net/tipc/node.c +0 -0

89f8ac73bf65..009a81631280 100644

net/tipc/node.c +1 -0
@@ -153,7 +153,7 @@ static void tipc_node_link_down(struct tipc_node *n,
 int bearer_id,
 				bool delete);
 static void node_lost_contact(struct tipc_node *n, struct sk_buff_head
 *inputq);  static void tipc_node_delete(struct tipc_node *node); -static void
 tipc_node_timeout(unsigned long data);
+static void tipc_node_timeout(struct timer_list *t);
 static void tipc_node_fsm_evt(struct tipc_node *n, int evt);  static struct

tipc_node *tipc_node_find(struct net *net, u32 addr); static void tipc_node_put(struct tipc_node *node); @@ -361,7 +361,7 @@ struct tipc_node *tipc_node_create(struct net *net, u32 addr, u16 capabilities)

net/tipc/node.c +1 -1
 		goto exit;
 	}
 	tipc_node_get(n);
-	setup_timer(&n->timer, tipc_node_timeout, (unsigned long)n);
+	timer_setup(&n->timer, tipc_node_timeout, 0);
 	n->keepalive_intv = U32_MAX;
 	hlist_add_head_rcu(&n->hash, &tn-
Quoted 1 line

node_htable[tipc_hashfn(addr)]);

list_for_each_entry_rcu(temp_node, &tn->node_list, list) { @@ -
500,9 +500,9 @@ void tipc_node_remove_conn(struct net *net, u32 dnode, u32 port)

/* tipc_node_timeout - handle expiration of node timer
*/

net/tipc/node.c +2 -2
-static void tipc_node_timeout(unsigned long data)
+static void tipc_node_timeout(struct timer_list *t)
 {
-	struct tipc_node *n = (struct tipc_node *)data;
+	struct tipc_node *n = from_timer(n, t, timer);
 	struct tipc_link_entry *le;
 	struct sk_buff_head xmitq;
 	int bearer_id;
net/tipc/socket.c index renamed from net/tipc/socket.c +0 -0

b3b72d8e9543..6387839f643d 100644

net/tipc/socket.c +0 -0
@@ -125,7 +125,7 @@ static void tipc_sock_destruct(struct sock *sk);  static

int tipc_release(struct socket *sock); static int tipc_accept(struct socket *sock, struct socket *new_sock, int flags,

net/tipc/socket.c +1 -1
 		       bool kern);
-static void tipc_sk_timeout(unsigned long data);
+static void tipc_sk_timeout(struct timer_list *t);
 static int tipc_sk_publish(struct tipc_sock *tsk, uint scope,
 			   struct tipc_name_seq const *seq);  static int

tipc_sk_withdraw(struct tipc_sock *tsk, uint scope, @@ -464,7 +464,7 @@ static int tipc_sk_create(struct net *net, struct socket *sock,

net/tipc/socket.c +4 -4
 		      NAMED_H_SIZE, 0);
 
 	msg_set_origport(msg, tsk->portid);
-	setup_timer(&sk->sk_timer, tipc_sk_timeout, (unsigned long)tsk);
+	timer_setup(&sk->sk_timer, tipc_sk_timeout, 0);
 	sk->sk_shutdown = 0;
 	sk->sk_backlog_rcv = tipc_sk_backlog_rcv;
 	sk->sk_rcvbuf = sysctl_tipc_rmem[1];
@@ -2530,14 +2530,14 @@ static int tipc_shutdown(struct socket *sock, int
 how)
 	return res;
 }
 
-static void tipc_sk_timeout(unsigned long data)
+static void tipc_sk_timeout(struct timer_list *t)
 {
-	struct tipc_sock *tsk = (struct tipc_sock *)data;
+	struct sock *sk = from_timer(sk, t, sk_timer);
+	struct tipc_sock *tsk = tipc_sk(sk);
 	u32 peer_port = tsk_peer_port(tsk);
 	u32 peer_node = tsk_peer_node(tsk);
 	u32 own_node = tsk_own_node(tsk);
 	u32 own_port = tsk->portid;
-	struct sock *sk = &tsk->sk;
 	struct net *net = sock_net(sk);
 	struct sk_buff *skb = NULL;
net/tipc/subscr.c index renamed from net/tipc/subscr.c +0 -0

be3d9e3183dc..251065dfd8df 100644

net/tipc/subscr.c +2 -2
@@ -133,9 +133,9 @@ void tipc_subscrp_report_overlap(struct
 tipc_subscription *sub, u32 found_lower,
 				node);
 }
 
-static void tipc_subscrp_timeout(unsigned long data)
+static void tipc_subscrp_timeout(struct timer_list *t)
 {
-	struct tipc_subscription *sub = (struct tipc_subscription *)data;
+	struct tipc_subscription *sub = from_timer(sub, t, timer);
 	struct tipc_subscriber *subscriber = sub->subscriber;
 
net/tipc/subscr.c +1 -1
 	spin_lock_bh(&subscriber->lock);
@@ -303,7 +303,7 @@ static void tipc_subscrp_subscribe(struct net *net,
 struct tipc_subscr *s,
 	tipc_subscrb_get(subscriber);
 	spin_unlock_bh(&subscriber->lock);
 
-	setup_timer(&sub->timer, tipc_subscrp_timeout, (unsigned
 long)sub);
+	timer_setup(&sub->timer, tipc_subscrp_timeout, 0);
 	timeout = htohl(sub->evt.s.timeout, swap);

if (timeout != TIPC_WAIT_FOREVER)
--
2.7.4

--
Kees Cook
Pixel Security

author
#2 lore raw
Quoted 1 line · Jon Maloy

NAK. It doesn't sound like a good idea to send this to net. Especially since one of these timers has already been refactored in net-next.

Hi! I'm not sure what you mean about the one timer issue. I don't see any use of timer_setup() in net/tipc (and no recent conversions to the older setup_timer() API). What's the preferred path for landing this API conversion in net/tipc/?

And, just to note, these changes are almost entirely mechanical. The only "special" case is in tipc_sk_timeout() where the argument needs to be slightly adjusted to fetch the tsk from the sk again.

Thanks!

-Kees

Signature · 2 lines
Kees Cook
Pixel Security
#3 lore raw
Quoted 17 lines

-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Kees Cook
Sent: Friday, October 27, 2017 06:58
To: Jon Maloy <[email protected]>
Cc: David S. Miller <[email protected]>; Ying Xue <[email protected]>; [email protected]; tipc- [email protected]; [email protected]
Subject: Re: [PATCH] net: tipc: Convert timers to use timer_setup()

Quoted 1 line · Jon Maloy

NAK. It doesn't sound like a good idea to send this to net. Especially since

one of these timers has already been refactored in net-next.

Quoted 8 lines

Hi! I'm not sure what you mean about the one timer issue. I don't see any use of timer_setup() in net/tipc (and no recent conversions to the older setup_timer() API). What's the preferred path for landing this API conversion in net/tipc/?

And, just to note, these changes are almost entirely mechanical. The only "special" case is in tipc_sk_timeout() where the argument needs to be slightly adjusted to fetch the tsk from the sk again.

I am referring to commit 0d5fcebf3c37 ("tipc: refactor tipc_sk_timeout() function") which is in "net-next" but not yet in "net".
I guess David M is the one who decides whether you deliver to net or net-next, but in the former case you will have to wait until David merges in the above commit, and then add your change again.

BR
///jon

Quoted 8 lines

Thanks!

-Kees

--
Kees Cook
Pixel Security

author
#4 lore raw
Quoted 22 lines · Jon Maloy
Quoted 22 lines

-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Kees Cook
Sent: Friday, October 27, 2017 06:58
To: Jon Maloy <[email protected]>
Cc: David S. Miller <[email protected]>; Ying Xue <[email protected]>; [email protected]; tipc- [email protected]; [email protected]
Subject: Re: [PATCH] net: tipc: Convert timers to use timer_setup()

Quoted 1 line · Jon Maloy

NAK. It doesn't sound like a good idea to send this to net. Especially since

one of these timers has already been refactored in net-next.

Hi! I'm not sure what you mean about the one timer issue. I don't see any use of timer_setup() in net/tipc (and no recent conversions to the older setup_timer() API). What's the preferred path for landing this API conversion in net/tipc/?

Quoted 6 lines
Quoted 3 lines

And, just to note, these changes are almost entirely mechanical. The only "special" case is in tipc_sk_timeout() where the argument needs to be slightly adjusted to fetch the tsk from the sk again.

I am referring to commit 0d5fcebf3c37 ("tipc: refactor tipc_sk_timeout() function") which is in "net-next" but not yet in "net".
I guess David M is the one who decides whether you deliver to net or net-next, but in the former case you will have to wait until David merges in the above commit, and then add your change again.

Ah, thanks, I see the change now. These patches passed in the night, it seems. I've sent a v2 now for net-next.

Thanks!

-Kees

Signature · 2 lines
Kees Cook
Pixel Security