Re: [PATCH 4.18 093/150] rds: RDS (tcp) hangs on sendto() to unresponding address
On 11/3/18 2:34 AM, Greg Kroah-Hartman wrote:
4.18-stable review patch. If anyone has any objections, please let me know.
This patch has a problem. I am working on a fix for that. Please
do not include it in the stable release. Thanks.
--
[ Upstream commit 9a4890bd6d6325a1c88564a20ab310b2d56f6094 ]
In rds_send_mprds_hash(), if the calculated hash value is non-zero and
the MPRDS connections are not yet up, it will wait. But it should not
wait if the send is non-blocking. In this case, it should just use the
base c_path for sending the message.
Signed-off-by: Ka-Cheong Poon
Acked-by: Santosh Shilimkar
Signed-off-by: David S. Miller
Signed-off-by: Sasha Levin
---
net/rds/send.c | 13 ++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/net/rds/send.c b/net/rds/send.c
index 59f17a2335f4..0e54ca0f4e9e 100644
--- a/net/rds/send.c
+++ b/net/rds/send.c
@@ -1006,7 +1006,8 @@ static int rds_cmsg_send(struct rds_sock *rs, struct
rds_message *rm,
return ret;
}
-static int rds_send_mprds_hash(struct rds_sock *rs, struct rds_connection *conn)
+static int rds_send_mprds_hash(struct rds_sock *rs,
+ struct rds_connection *conn, int nonblock)
{
int hash;
@@ -1022,10 +1023,16 @@ static int rds_send_mprds_hash(struct rds_sock *rs, struct rds_connection *conn)
* used. But if we are interrupted, we have to use the zero
* c_path in case the connection ends up being non-MP capable.
*/
- if (conn->c_npaths == 0)
+ if (conn->c_npaths == 0) {
+ /* Cannot wait for the connection be made, so just use
+* the base c_path.
+*/
+ if (nonblock)
+ return 0;
if (wait_event_interruptible(conn->c_hs_waitq,
conn->c_npaths != 0))
hash = 0;
+ }
if (conn->c_npaths == 1)
hash = 0;
}
@@ -1170,7 +1177,7 @@ int rds_sendmsg(struct socket *sock, struct msghdr *msg,
size_t payload_len)
}
if (conn->c_trans->t_mp_capable)
- cpath = >c_path[rds_send_mprds_hash(rs, conn)];
+ cpath = >c_path[rds_send_mprds_hash(rs, conn, nonblock)];
else
cpath = >c_path[0];
--
K. Poon
ka-cheong.p...@oracle.com
Re: [PATCH 4.18 093/150] rds: RDS (tcp) hangs on sendto() to unresponding address
On 11/3/18 2:34 AM, Greg Kroah-Hartman wrote:
4.18-stable review patch. If anyone has any objections, please let me know.
This patch has a problem. I am working on a fix for that. Please
do not include it in the stable release. Thanks.
--
[ Upstream commit 9a4890bd6d6325a1c88564a20ab310b2d56f6094 ]
In rds_send_mprds_hash(), if the calculated hash value is non-zero and
the MPRDS connections are not yet up, it will wait. But it should not
wait if the send is non-blocking. In this case, it should just use the
base c_path for sending the message.
Signed-off-by: Ka-Cheong Poon
Acked-by: Santosh Shilimkar
Signed-off-by: David S. Miller
Signed-off-by: Sasha Levin
---
net/rds/send.c | 13 ++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/net/rds/send.c b/net/rds/send.c
index 59f17a2335f4..0e54ca0f4e9e 100644
--- a/net/rds/send.c
+++ b/net/rds/send.c
@@ -1006,7 +1006,8 @@ static int rds_cmsg_send(struct rds_sock *rs, struct
rds_message *rm,
return ret;
}
-static int rds_send_mprds_hash(struct rds_sock *rs, struct rds_connection *conn)
+static int rds_send_mprds_hash(struct rds_sock *rs,
+ struct rds_connection *conn, int nonblock)
{
int hash;
@@ -1022,10 +1023,16 @@ static int rds_send_mprds_hash(struct rds_sock *rs, struct rds_connection *conn)
* used. But if we are interrupted, we have to use the zero
* c_path in case the connection ends up being non-MP capable.
*/
- if (conn->c_npaths == 0)
+ if (conn->c_npaths == 0) {
+ /* Cannot wait for the connection be made, so just use
+* the base c_path.
+*/
+ if (nonblock)
+ return 0;
if (wait_event_interruptible(conn->c_hs_waitq,
conn->c_npaths != 0))
hash = 0;
+ }
if (conn->c_npaths == 1)
hash = 0;
}
@@ -1170,7 +1177,7 @@ int rds_sendmsg(struct socket *sock, struct msghdr *msg,
size_t payload_len)
}
if (conn->c_trans->t_mp_capable)
- cpath = >c_path[rds_send_mprds_hash(rs, conn)];
+ cpath = >c_path[rds_send_mprds_hash(rs, conn, nonblock)];
else
cpath = >c_path[0];
--
K. Poon
ka-cheong.p...@oracle.com
Re: [PATCH] net/rds/Kconfig: RDS should depend on IPV6
On 07/26/2018 06:36 AM, Santosh Shilimkar wrote:
On 7/25/2018 3:20 PM, Anders Roxell wrote:
Build error, implicit declaration of function __inet6_ehashfn shows up
When RDS is enabled but not IPV6.
net/rds/connection.c: In function ‘rds_conn_bucket’:
net/rds/connection.c:67:9: error: implicit declaration of function
‘__inet6_ehashfn’; did you mean ‘__inet_ehashfn’?
[-Werror=implicit-function-declaration]
hash = __inet6_ehashfn(lhash, 0, fhash, 0, rds_hash_secret);
^~~
__inet_ehashfn
Current code adds IPV6 as a depends on in config RDS.
Fixes: eee2fa6ab322 ("rds: Changing IP address internal representation
to struct in6_addr")
Signed-off-by: Anders Roxell
---
net/rds/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/rds/Kconfig b/net/rds/Kconfig
index 41f75563b54b..607128f10bcd 100644
--- a/net/rds/Kconfig
+++ b/net/rds/Kconfig
@@ -1,7 +1,7 @@
config RDS
tristate "The RDS Protocol"
- depends on INET
+ depends on INET && CONFIG_IPV6
This should build without CONFIG_IPV6 too.
Hi Ka-cheong,
Can you please loot at it ? I know you modified
lookup function to take always in6_addr now, but
probably hashing with '__inet_ehashfn' should
work too for non IPV6 address(s).
I guess for now, let's add this dependency first. I
will do a follow up patch to remove this dependency.
Thanks.
--
K. Poon
ka-cheong.p...@oracle.com
Re: [PATCH] net/rds/Kconfig: RDS should depend on IPV6
On 07/26/2018 06:36 AM, Santosh Shilimkar wrote:
On 7/25/2018 3:20 PM, Anders Roxell wrote:
Build error, implicit declaration of function __inet6_ehashfn shows up
When RDS is enabled but not IPV6.
net/rds/connection.c: In function ‘rds_conn_bucket’:
net/rds/connection.c:67:9: error: implicit declaration of function
‘__inet6_ehashfn’; did you mean ‘__inet_ehashfn’?
[-Werror=implicit-function-declaration]
hash = __inet6_ehashfn(lhash, 0, fhash, 0, rds_hash_secret);
^~~
__inet_ehashfn
Current code adds IPV6 as a depends on in config RDS.
Fixes: eee2fa6ab322 ("rds: Changing IP address internal representation
to struct in6_addr")
Signed-off-by: Anders Roxell
---
net/rds/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/rds/Kconfig b/net/rds/Kconfig
index 41f75563b54b..607128f10bcd 100644
--- a/net/rds/Kconfig
+++ b/net/rds/Kconfig
@@ -1,7 +1,7 @@
config RDS
tristate "The RDS Protocol"
- depends on INET
+ depends on INET && CONFIG_IPV6
This should build without CONFIG_IPV6 too.
Hi Ka-cheong,
Can you please loot at it ? I know you modified
lookup function to take always in6_addr now, but
probably hashing with '__inet_ehashfn' should
work too for non IPV6 address(s).
I guess for now, let's add this dependency first. I
will do a follow up patch to remove this dependency.
Thanks.
--
K. Poon
ka-cheong.p...@oracle.com
Re: [PATCH v2 net-next 1/2] tcp: add TCP_ZEROCOPY_RECEIVE support for zerocopy receive
On 04/26/2018 05:43 AM, Eric Dumazet wrote: When adding tcp mmap() implementation, I forgot that socket lock had to be taken before current->mm->mmap_sem. syzbot eventually caught the bug. Since we can not lock the socket in tcp mmap() handler we have to split the operation in two phases. 1) mmap() on a tcp socket simply reserves VMA space, and nothing else. This operation does not involve any TCP locking. 2) setsockopt(fd, IPPROTO_TCP, TCP_ZEROCOPY_RECEIVE, ...) implements the transfert of pages from skbs to one VMA. This operation only uses down_read(>mm->mmap_sem) after holding TCP lock, thus solving the lockdep issue. A quick question. Is it a normal practice to return a result in setsockopt() given that the optval parameter is supposed to be a const void *? -- K. Poon ka-cheong.p...@oracle.com
Re: [PATCH v2 net-next 1/2] tcp: add TCP_ZEROCOPY_RECEIVE support for zerocopy receive
On 04/26/2018 05:43 AM, Eric Dumazet wrote: When adding tcp mmap() implementation, I forgot that socket lock had to be taken before current->mm->mmap_sem. syzbot eventually caught the bug. Since we can not lock the socket in tcp mmap() handler we have to split the operation in two phases. 1) mmap() on a tcp socket simply reserves VMA space, and nothing else. This operation does not involve any TCP locking. 2) setsockopt(fd, IPPROTO_TCP, TCP_ZEROCOPY_RECEIVE, ...) implements the transfert of pages from skbs to one VMA. This operation only uses down_read(>mm->mmap_sem) after holding TCP lock, thus solving the lockdep issue. A quick question. Is it a normal practice to return a result in setsockopt() given that the optval parameter is supposed to be a const void *? -- K. Poon ka-cheong.p...@oracle.com
