LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Sowmini Varadhan <sowmini.varadhan@oracle.com>
To: netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Cc: chien.yen@oracle.com, davem@davemloft.net,
	rds-devel@oss.oracle.com, agrover@redhat.com, clm@fb.com,
	zab@zabbo.net, ajaykumar.hotchandani@oracle.com,
	Sowmini Varadhan <sowmini.varadhan@oracle.com>
Subject: [PATCH 2/6] RDS: only use passive connections when addresses match
Date: Thu,  2 Apr 2015 09:50:45 -0400	[thread overview]
Message-ID: <25b5d99baee278ade82d725bb80d0a382b2bf654.1427936719.git.sowmini.varadhan@oracle.com> (raw)
In-Reply-To: <cover.1427936719.git.sowmini.varadhan@oracle.com>
In-Reply-To: <cover.1427936719.git.sowmini.varadhan@oracle.com>

Passive connections were added for the case where one loopback IB
connection between identical addresses needs another connection to store
the second QP.  Unfortunately, they were also created in the case where
the addesses differ and we already have both QPs.

This lead to a message reordering bug.

- two different IB interfaces and addresses on a machine: A B
- traffic is sent from A to B
- connection from A-B is created, connect request sent
- listening accepts connect request, B-A is created
- traffic flows, next_rx is incremented
- unacked messages exist on the retrans list
- connection A-B is shut down, new connect request sent
- listen sees existing loopback B-A, creates new passive B-A
- retrans messages are sent and delivered because of 0 next_rx

The problem is that the second connection request saw the previously
existing parent connection.  Instead of using it, and using the existing
next_rx_seq state for the traffic between those IPs, it mistakenly
thought that it had to create a passive connection.

We fix this by only using passive connections in the special case where
laddr and faddr match.  In this case we'll only ever have one parent
sending connection requests and one passive connection created as the
listening path sees the existing parent connection which initiated the
request.

Original patch by Zach Brown

Signed-off-by: Sowmini Varadhan <sowmini.varadhan@oracle.com>
Reviewed-by: Ajaykumar Hotchandani <ajaykumar.hotchandani@oracle.com>
---
 net/rds/connection.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/rds/connection.c b/net/rds/connection.c
index 378c3a6..7952a5b 100644
--- a/net/rds/connection.c
+++ b/net/rds/connection.c
@@ -130,7 +130,7 @@ static struct rds_connection *__rds_conn_create(__be32 laddr, __be32 faddr,
 	rcu_read_lock();
 	conn = rds_conn_lookup(head, laddr, faddr, trans);
 	if (conn && conn->c_loopback && conn->c_trans != &rds_loop_transport &&
-	    !is_outgoing) {
+	    laddr == faddr && !is_outgoing) {
 		/* This is a looped back IB connection, and we're
 		 * called by the code handling the incoming connect.
 		 * We need a second connection object into which we
-- 
1.7.1


  parent reply	other threads:[~2015-04-02 13:52 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-02 13:50 [PATCH 0/6] RDS: RDS core bug fixes Sowmini Varadhan
2015-04-02 13:50 ` [PATCH 1/6] RDS: Re-add pf/sol access via sysctl Sowmini Varadhan
2015-04-02 13:50 ` Sowmini Varadhan [this message]
2015-04-02 13:50 ` [PATCH 3/6] rds: check for excessive looping in rds_send_xmit Sowmini Varadhan
2015-04-02 13:50 ` [PATCH 4/6] RDS: make sure not to loop forever inside rds_send_xmit Sowmini Varadhan
2015-04-02 13:50 ` [PATCH 5/6] RDS: rds_send_xmit is called under a spinlock, lets not do a cond_resched() Sowmini Varadhan
2015-04-02 13:50 ` [PATCH 6/6] RDS: don't trust the LL_SEND_FULL bit Sowmini Varadhan
2015-04-02 14:06   ` Sergei Shtylyov

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=25b5d99baee278ade82d725bb80d0a382b2bf654.1427936719.git.sowmini.varadhan@oracle.com \
    --to=sowmini.varadhan@oracle.com \
    --cc=agrover@redhat.com \
    --cc=ajaykumar.hotchandani@oracle.com \
    --cc=chien.yen@oracle.com \
    --cc=clm@fb.com \
    --cc=davem@davemloft.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=rds-devel@oss.oracle.com \
    --cc=zab@zabbo.net \
    --subject='Re: [PATCH 2/6] RDS: only use passive connections when addresses match' \
    /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

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).