netfilter.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Eric Dumazet <eric.dumazet@gmail.com>
To: Evgeniy Polyakov <zbr@ioremap.net>
Cc: kapil dakhane <kdakhane@gmail.com>,
	"David S. Miller" <davem@davemloft.net>,
	netdev@vger.kernel.org, netfilter@vger.kernel.org
Subject: [PATCH] tcp: documents timewait refcnt tricks
Date: Mon, 07 Dec 2009 10:59:41 +0100	[thread overview]
Message-ID: <4B1CD20D.8030407@gmail.com> (raw)
In-Reply-To: <20091205212148.GA26270@ioremap.net>

Evgeniy Polyakov a écrit :
> Hi Eric.
> 
> On Fri, Dec 04, 2009 at 02:46:54PM +0100, Eric Dumazet (eric.dumazet@gmail.com) wrote:
>> First patch changes __inet_hash_nolisten() and __inet6_hash()
>> to get a timewait parameter to be able to unhash it from ehash
>> at same time the new socket is inserted in hash.
>>
>> This makes sure timewait socket wont be found by a concurrent
>> writer in __inet_check_established()
> 
> Both patches look good, although trick with returning reference counter
> may look like a hack especially when only viewing into ip code and not
> hashtable itself. Can you please cook up a documentation update for hash
> function that it is supposed to return refcnt when socket was in hash
> table.
> 

Sure, here it is :

Thanks !

[PATCH] tcp: documents timewait refcnt tricks 

Adds kerneldoc for inet_twsk_unhash() & inet_twsk_bind_unhash().

Suggested-by: Evgeniy Polyakov <zbr@ioremap.net>
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---

diff --git a/net/ipv4/inet_timewait_sock.c b/net/ipv4/inet_timewait_sock.c
index 1958cf5..cf719c2 100644
--- a/net/ipv4/inet_timewait_sock.c
+++ b/net/ipv4/inet_timewait_sock.c
@@ -15,9 +15,13 @@
 #include <net/ip.h>
 
 
-/*
- * unhash a timewait socket from established hash
- * lock must be hold by caller
+/**
+ *	inet_twsk_unhash - unhash a timewait socket from established hash
+ *	@tw: timewait socket
+ *
+ *	unhash a timewait socket from established hash, if hashed.
+ *	ehash lock must be hold by caller.
+ *	Returns 1 if caller should call inet_twsk_put() after lock release.
  */
 int inet_twsk_unhash(struct inet_timewait_sock *tw)
 {
@@ -26,12 +30,21 @@ int inet_twsk_unhash(struct inet_timewait_sock *tw)
 
 	hlist_nulls_del_rcu(&tw->tw_node);
 	sk_nulls_node_init(&tw->tw_node);
+	/*
+	 * We cannot call inet_twsk_put() ourself under lock,
+	 * caller must call it for us.
+	 */
 	return 1;
 }
 
-/*
- * unhash a timewait socket from bind hash
- * lock must be hold by caller
+/**
+ *	inet_twsk_bind_unhash - unhash a timewait socket from bind hash
+ *	@tw: timewait socket
+ *	@hashinfo: hashinfo pointer
+ *
+ *	unhash a timewait socket from bind hash, if hashed.
+ *	bind hash lock must be hold by caller.
+ *	Returns 1 if caller should call inet_twsk_put() after lock release.
  */
 int inet_twsk_bind_unhash(struct inet_timewait_sock *tw,
 			  struct inet_hashinfo *hashinfo)
@@ -44,6 +57,10 @@ int inet_twsk_bind_unhash(struct inet_timewait_sock *tw,
 	__hlist_del(&tw->tw_bind_node);
 	tw->tw_tb = NULL;
 	inet_bind_bucket_destroy(hashinfo->bind_bucket_cachep, tb);
+	/*
+	 * We cannot call inet_twsk_put() ourself under lock,
+	 * caller must call it for us.
+	 */
 	return 1;
 }
 
@@ -140,7 +157,7 @@ void __inet_twsk_hashdance(struct inet_timewait_sock *tw, struct sock *sk,
 
 	/*
 	 * Notes :
- 	 * - We initially set tw_refcnt to 0 in inet_twsk_alloc()
+	 * - We initially set tw_refcnt to 0 in inet_twsk_alloc()
 	 * - We add one reference for the bhash link
 	 * - We add one reference for the ehash link
 	 * - We want this refcnt update done before allowing other
@@ -150,7 +167,6 @@ void __inet_twsk_hashdance(struct inet_timewait_sock *tw, struct sock *sk,
 
 	spin_unlock(lock);
 }
-
 EXPORT_SYMBOL_GPL(__inet_twsk_hashdance);
 
 struct inet_timewait_sock *inet_twsk_alloc(const struct sock *sk, const int state)
@@ -191,7 +207,6 @@ struct inet_timewait_sock *inet_twsk_alloc(const struct sock *sk, const int stat
 
 	return tw;
 }
-
 EXPORT_SYMBOL_GPL(inet_twsk_alloc);
 
 /* Returns non-zero if quota exceeded.  */
@@ -270,7 +285,6 @@ void inet_twdr_hangman(unsigned long data)
 out:
 	spin_unlock(&twdr->death_lock);
 }
-
 EXPORT_SYMBOL_GPL(inet_twdr_hangman);
 
 void inet_twdr_twkill_work(struct work_struct *work)
@@ -301,7 +315,6 @@ void inet_twdr_twkill_work(struct work_struct *work)
 		spin_unlock_bh(&twdr->death_lock);
 	}
 }
-
 EXPORT_SYMBOL_GPL(inet_twdr_twkill_work);
 
 /* These are always called from BH context.  See callers in
@@ -321,7 +334,6 @@ void inet_twsk_deschedule(struct inet_timewait_sock *tw,
 	spin_unlock(&twdr->death_lock);
 	__inet_twsk_kill(tw, twdr->hashinfo);
 }
-
 EXPORT_SYMBOL(inet_twsk_deschedule);
 
 void inet_twsk_schedule(struct inet_timewait_sock *tw,
@@ -402,7 +414,6 @@ void inet_twsk_schedule(struct inet_timewait_sock *tw,
 		mod_timer(&twdr->tw_timer, jiffies + twdr->period);
 	spin_unlock(&twdr->death_lock);
 }
-
 EXPORT_SYMBOL_GPL(inet_twsk_schedule);
 
 void inet_twdr_twcal_tick(unsigned long data)
@@ -463,7 +474,6 @@ out:
 #endif
 	spin_unlock(&twdr->death_lock);
 }
-
 EXPORT_SYMBOL_GPL(inet_twdr_twcal_tick);
 
 void inet_twsk_purge(struct inet_hashinfo *hashinfo,

  reply	other threads:[~2009-12-07  9:59 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-12-01  2:02 soft lockup in inet_csk_get_port kapil dakhane
2009-12-01  6:10 ` Eric Dumazet
2009-12-01 15:00 ` [PATCH] tcp: Fix a connect() race with timewait sockets Eric Dumazet
2009-12-02  8:59   ` David Miller
2009-12-02  9:23     ` Eric Dumazet
2009-12-02 10:33       ` Eric Dumazet
2009-12-02 11:32         ` Evgeniy Polyakov
2009-12-02 19:18           ` kapil dakhane
2009-12-03  2:43             ` kapil dakhane
2009-12-03 10:49               ` [PATCH] tcp: fix a timewait refcnt race Eric Dumazet
2009-12-04  0:19                 ` David Miller
2009-12-04  3:20                 ` kapil dakhane
2009-12-04  6:29                   ` Eric Dumazet
2009-12-04  6:39                     ` David Miller
2009-12-02 15:08         ` [PATCH net-next-2.6] tcp: connect() race with timewait reuse Eric Dumazet
2009-12-02 22:15           ` Evgeniy Polyakov
2009-12-03  6:44             ` Eric Dumazet
2009-12-03  8:31               ` Eric Dumazet
2009-12-03 23:22                 ` Evgeniy Polyakov
2009-12-04  0:18                 ` David Miller
2009-12-02 16:05     ` [PATCH] tcp: Fix a connect() race with timewait sockets Ashwani Wason
2009-12-03  6:38       ` David Miller
2009-12-04 13:45   ` [PATCH 0/2] tcp: Fix connect() races " Eric Dumazet
2009-12-04 13:46   ` [PATCH 1/2] tcp: Fix a connect() race " Eric Dumazet
2009-12-05 21:21     ` Evgeniy Polyakov
2009-12-07  9:59       ` Eric Dumazet [this message]
2009-12-07 16:06         ` [PATCH] tcp: documents timewait refcnt tricks Randy Dunlap
2009-12-09  4:20           ` David Miller
2009-12-09  4:18     ` [PATCH 1/2] tcp: Fix a connect() race with timewait sockets David Miller
2009-12-04 13:47   ` [PATCH 2/2] " Eric Dumazet
2009-12-09  4:19     ` David Miller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4B1CD20D.8030407@gmail.com \
    --to=eric.dumazet@gmail.com \
    --cc=davem@davemloft.net \
    --cc=kdakhane@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=netfilter@vger.kernel.org \
    --cc=zbr@ioremap.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).