linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Eric Dumazet <eric.dumazet@gmail.com>
To: Craig Gallek <kraigatgoog@gmail.com>
Cc: Dmitry Vyukov <dvyukov@google.com>,
	"David S. Miller" <davem@davemloft.net>,
	netdev <netdev@vger.kernel.org>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: net: hang in ip_finish_output
Date: Mon, 18 Jan 2016 18:49:29 -0800	[thread overview]
Message-ID: <1453171769.1223.255.camel@edumazet-glaptop2.roam.corp.google.com> (raw)
In-Reply-To: <1453170024.1223.251.camel@edumazet-glaptop2.roam.corp.google.com>

On Mon, 2016-01-18 at 18:20 -0800, Eric Dumazet wrote:

> Same reason really.
> 
> Right after sk2=socket(), setsockopt(sk2,...,SO_REUSEPORT, on) and
> bind(sk2, ...), but _before_ the connect(sk2) is done, sk2 is added into
> the soreuseport array, with a score which is smaller than the score of
> first socket sk1 found in hash table (I am speaking of the regular UDP
> hash table), if sk1 had the connect() done, giving a +8 to its score.
> 
> So the bug has nothing to do with rcu or rcu_bh, it is just an infinite
> loop caused by different scores.
> 
> 
> hash bucket [X] -> sk1 -> sk2 -> NULL
> 
> sk1 score = 14  (because it did a connect())
> sk2 score = 6
> 
> I guess we should relax the test done after atomic_inc_not_zero_hint()
> to only test the base keys : 
> (net, ipv6_only_sock, inet->inet_rcv_saddr & inet->inet_num)

One way to fix the issue it to not call reuseport_select_sock() if loop
was restarted, and fallback to the old mechanism : If the optimized
version might have a problem, just fallback to the safe thing.

IPv4 only patch :

diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index dc45b538e237..8f1b2a9c90f8 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -500,6 +500,7 @@ static struct sock *udp4_lib_lookup2(struct net *net,
 	struct hlist_nulls_node *node;
 	int score, badness, matches = 0, reuseport = 0;
 	u32 hash = 0;
+	bool select_ok = true;
 
 begin:
 	result = NULL;
@@ -512,14 +513,18 @@ begin:
 			badness = score;
 			reuseport = sk->sk_reuseport;
 			if (reuseport) {
-				struct sock *sk2;
 				hash = udp_ehashfn(net, daddr, hnum,
 						   saddr, sport);
-				sk2 = reuseport_select_sock(sk, hash, skb,
+				if (select_ok) {
+					struct sock *sk2;
+
+					sk2 = reuseport_select_sock(sk, hash, skb,
 							    sizeof(struct udphdr));
-				if (sk2) {
-					result = sk2;
-					goto found;
+					if (sk2) {
+						result = sk2;
+						select_ok = false;
+						goto found;
+					}
 				}
 				matches = 1;
 			}
@@ -564,6 +569,7 @@ struct sock *__udp4_lib_lookup(struct net *net, __be32 saddr,
 	struct udp_hslot *hslot2, *hslot = &udptable->hash[slot];
 	int score, badness, matches = 0, reuseport = 0;
 	u32 hash = 0;
+	bool select_ok = true;
 
 	rcu_read_lock();
 	if (hslot->count > 10) {
@@ -601,14 +607,18 @@ begin:
 			badness = score;
 			reuseport = sk->sk_reuseport;
 			if (reuseport) {
-				struct sock *sk2;
 				hash = udp_ehashfn(net, daddr, hnum,
 						   saddr, sport);
-				sk2 = reuseport_select_sock(sk, hash, skb,
+				if (select_ok) {
+					struct sock *sk2;
+
+					sk2 = reuseport_select_sock(sk, hash, skb,
 							sizeof(struct udphdr));
-				if (sk2) {
-					result = sk2;
-					goto found;
+					if (sk2) {
+						select_ok = false;
+						result = sk2;
+						goto found;
+					}
 				}
 				matches = 1;
 			}

  reply	other threads:[~2016-01-19  2:49 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-18 18:33 net: hang in ip_finish_output Craig Gallek
2016-01-19  2:20 ` Eric Dumazet
2016-01-19  2:49   ` Eric Dumazet [this message]
2016-01-19 16:13     ` Craig Gallek
2016-01-19 16:36       ` [PATCH net] udp: fix potential infinite loop in SO_REUSEPORT logic Eric Dumazet
2016-01-19 17:15         ` Craig Gallek
2016-01-19 18:53         ` David Miller
  -- strict thread matches above, loose matches on Subject: below --
2016-01-15 17:57 net: hang in ip_finish_output Dmitry Vyukov
2016-01-16  0:20 ` Craig Gallek
2016-01-16  7:29   ` Eric Dumazet
2016-01-18  3:12     ` Eric Dumazet
2016-01-18 16:21       ` Eric Dumazet

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=1453171769.1223.255.camel@edumazet-glaptop2.roam.corp.google.com \
    --to=eric.dumazet@gmail.com \
    --cc=davem@davemloft.net \
    --cc=dvyukov@google.com \
    --cc=kraigatgoog@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    /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).