netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next 0/3] tun: timer cleanups
@ 2017-10-20 18:29 Eric Dumazet
  2017-10-20 18:29 ` [PATCH net-next 1/3] tun: do not block BH again in tun_flow_cleanup() Eric Dumazet
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Eric Dumazet @ 2017-10-20 18:29 UTC (permalink / raw)
  To: David S . Miller; +Cc: netdev, Eric Dumazet, Eric Dumazet, Jason Wang

While working on a syzkaller issue that might have been
fixed already by Cong Wang in commit 0ad646c81b21
("tun: call dev_get_valid_name() before register_netdevice()")
I made three small changes related to flow_gc_timer.

Eric Dumazet (3):
  tun: do not block BH again in tun_flow_cleanup()
  tun: avoid extra timer schedule in tun_flow_cleanup()
  tun: do not arm flow_gc_timer in tun_flow_init()

 drivers/net/tun.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

-- 
2.15.0.rc0.271.g36b669edcc-goog

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH net-next 1/3] tun: do not block BH again in tun_flow_cleanup()
  2017-10-20 18:29 [PATCH net-next 0/3] tun: timer cleanups Eric Dumazet
@ 2017-10-20 18:29 ` Eric Dumazet
  2017-10-20 18:29 ` [PATCH net-next 2/3] tun: avoid extra timer schedule " Eric Dumazet
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Eric Dumazet @ 2017-10-20 18:29 UTC (permalink / raw)
  To: David S . Miller; +Cc: netdev, Eric Dumazet, Eric Dumazet, Jason Wang

tun_flow_cleanup() being a timer callback, it is already
running in BH context.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 drivers/net/tun.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 3b41c36eae904fec5d11f51ed9fde0e060bba27e..f9541f7f9fb7592b50ac818702006f4f1042b448 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -454,7 +454,7 @@ static void tun_flow_cleanup(unsigned long data)
 
 	tun_debug(KERN_INFO, tun, "tun_flow_cleanup\n");
 
-	spin_lock_bh(&tun->lock);
+	spin_lock(&tun->lock);
 	for (i = 0; i < TUN_NUM_FLOW_ENTRIES; i++) {
 		struct tun_flow_entry *e;
 		struct hlist_node *n;
@@ -472,7 +472,7 @@ static void tun_flow_cleanup(unsigned long data)
 
 	if (count)
 		mod_timer(&tun->flow_gc_timer, round_jiffies_up(next_timer));
-	spin_unlock_bh(&tun->lock);
+	spin_unlock(&tun->lock);
 }
 
 static void tun_flow_update(struct tun_struct *tun, u32 rxhash,
-- 
2.15.0.rc0.271.g36b669edcc-goog

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH net-next 2/3] tun: avoid extra timer schedule in tun_flow_cleanup()
  2017-10-20 18:29 [PATCH net-next 0/3] tun: timer cleanups Eric Dumazet
  2017-10-20 18:29 ` [PATCH net-next 1/3] tun: do not block BH again in tun_flow_cleanup() Eric Dumazet
@ 2017-10-20 18:29 ` Eric Dumazet
  2017-10-20 18:29 ` [PATCH net-next 3/3] tun: do not arm flow_gc_timer in tun_flow_init() Eric Dumazet
  2017-10-22  2:15 ` [PATCH net-next 0/3] tun: timer cleanups David Miller
  3 siblings, 0 replies; 5+ messages in thread
From: Eric Dumazet @ 2017-10-20 18:29 UTC (permalink / raw)
  To: David S . Miller; +Cc: netdev, Eric Dumazet, Eric Dumazet, Jason Wang

If tun_flow_cleanup() deleted all flows, no need to
arm the timer again. It will be armed next time
tun_flow_update() is called.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 drivers/net/tun.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index f9541f7f9fb7592b50ac818702006f4f1042b448..995887de5a98283382a631bcb8dfd874a4037ed3 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -461,11 +461,14 @@ static void tun_flow_cleanup(unsigned long data)
 
 		hlist_for_each_entry_safe(e, n, &tun->flows[i], hash_link) {
 			unsigned long this_timer;
-			count++;
+
 			this_timer = e->updated + delay;
-			if (time_before_eq(this_timer, jiffies))
+			if (time_before_eq(this_timer, jiffies)) {
 				tun_flow_delete(tun, e);
-			else if (time_before(this_timer, next_timer))
+				continue;
+			}
+			count++;
+			if (time_before(this_timer, next_timer))
 				next_timer = this_timer;
 		}
 	}
-- 
2.15.0.rc0.271.g36b669edcc-goog

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH net-next 3/3] tun: do not arm flow_gc_timer in tun_flow_init()
  2017-10-20 18:29 [PATCH net-next 0/3] tun: timer cleanups Eric Dumazet
  2017-10-20 18:29 ` [PATCH net-next 1/3] tun: do not block BH again in tun_flow_cleanup() Eric Dumazet
  2017-10-20 18:29 ` [PATCH net-next 2/3] tun: avoid extra timer schedule " Eric Dumazet
@ 2017-10-20 18:29 ` Eric Dumazet
  2017-10-22  2:15 ` [PATCH net-next 0/3] tun: timer cleanups David Miller
  3 siblings, 0 replies; 5+ messages in thread
From: Eric Dumazet @ 2017-10-20 18:29 UTC (permalink / raw)
  To: David S . Miller; +Cc: netdev, Eric Dumazet, Eric Dumazet, Jason Wang

Timer is properly armed on demand from tun_flow_update(),
so there is no need to arm it at tun init.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 drivers/net/tun.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 995887de5a98283382a631bcb8dfd874a4037ed3..2a2d058cdd403594e3f89c9a491ada17e3063506 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -1197,8 +1197,6 @@ static void tun_flow_init(struct tun_struct *tun)
 
 	tun->ageing_time = TUN_FLOW_EXPIRE;
 	setup_timer(&tun->flow_gc_timer, tun_flow_cleanup, (unsigned long)tun);
-	mod_timer(&tun->flow_gc_timer,
-		  round_jiffies_up(jiffies + tun->ageing_time));
 }
 
 static void tun_flow_uninit(struct tun_struct *tun)
-- 
2.15.0.rc0.271.g36b669edcc-goog

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH net-next 0/3] tun: timer cleanups
  2017-10-20 18:29 [PATCH net-next 0/3] tun: timer cleanups Eric Dumazet
                   ` (2 preceding siblings ...)
  2017-10-20 18:29 ` [PATCH net-next 3/3] tun: do not arm flow_gc_timer in tun_flow_init() Eric Dumazet
@ 2017-10-22  2:15 ` David Miller
  3 siblings, 0 replies; 5+ messages in thread
From: David Miller @ 2017-10-22  2:15 UTC (permalink / raw)
  To: edumazet; +Cc: netdev, eric.dumazet, jasowang

From: Eric Dumazet <edumazet@google.com>
Date: Fri, 20 Oct 2017 11:29:54 -0700

> While working on a syzkaller issue that might have been
> fixed already by Cong Wang in commit 0ad646c81b21
> ("tun: call dev_get_valid_name() before register_netdevice()")
> I made three small changes related to flow_gc_timer.

Series applied, thanks Eric.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2017-10-22  2:15 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-20 18:29 [PATCH net-next 0/3] tun: timer cleanups Eric Dumazet
2017-10-20 18:29 ` [PATCH net-next 1/3] tun: do not block BH again in tun_flow_cleanup() Eric Dumazet
2017-10-20 18:29 ` [PATCH net-next 2/3] tun: avoid extra timer schedule " Eric Dumazet
2017-10-20 18:29 ` [PATCH net-next 3/3] tun: do not arm flow_gc_timer in tun_flow_init() Eric Dumazet
2017-10-22  2:15 ` [PATCH net-next 0/3] tun: timer cleanups David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).