All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next v2 0/4] net/smc: fixes from 2018-05-17 - v2
@ 2018-04-19 13:56 Ursula Braun
  2018-04-19 13:56 ` [PATCH net-next v2 1/4] net/smc: fix structure size Ursula Braun
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Ursula Braun @ 2018-04-19 13:56 UTC (permalink / raw)
  To: davem; +Cc: netdev, linux-s390, schwidefsky, heiko.carstens, raspl, ubraun

From: Ursula Braun <ursula.braun@linux.vnet.ibm.com>

Dave,

thanks for your feedback and pointing out the kernel_setsockopt()
problem(s) in patch 2/4. Here is now version 2 of the patch series
with patches 2-4 implementing sockopts that require special
handling in SMC.

Version 2 changes of Patch 2/4 (and 3/4):
   * return error -EOPNOTSUPP for TCP_FASTOPEN sockopts
   * fix a kernel_setsockopt() usage bug by switching parameter
     variable from type "u8" to "int"
   * add return code validation when calling kernel_setsockopt()
   * propagate a setsockopt error on the internal CLC socket
     to the SMC socket.

Regards, Ursula

Karsten Graul (1):
  net/smc: fix structure size

Ursula Braun (3):
  net/smc: handle sockopt TCP_NODELAY
  net/smc: handle sockopt TCP_CORK
  net/smc: handle sockopt TCP_DEFER_ACCEPT

 net/smc/af_smc.c  | 193 ++++++++++++++++++++++++++++++++++++++++++++++++++++--
 net/smc/smc.h     |  12 ++++
 net/smc/smc_cdc.c |   2 +-
 net/smc/smc_cdc.h |   2 +-
 net/smc/smc_rx.c  |   2 +-
 net/smc/smc_rx.h  |   1 +
 net/smc/smc_tx.c  |  16 ++++-
 net/smc/smc_tx.h  |   8 +++
 8 files changed, 225 insertions(+), 11 deletions(-)

-- 
2.13.5

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

* [PATCH net-next v2 1/4] net/smc: fix structure size
  2018-04-19 13:56 [PATCH net-next v2 0/4] net/smc: fixes from 2018-05-17 - v2 Ursula Braun
@ 2018-04-19 13:56 ` Ursula Braun
  2018-04-19 13:56 ` [PATCH net-next v2 2/4] net/smc: handle sockopt TCP_NODELAY Ursula Braun
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Ursula Braun @ 2018-04-19 13:56 UTC (permalink / raw)
  To: davem; +Cc: netdev, linux-s390, schwidefsky, heiko.carstens, raspl, ubraun

From: Karsten Graul <kgraul@linux.vnet.ibm.com>

The struct smc_cdc_msg must be defined as packed so the
size is 44 bytes.
And change the structure size check so sizeof is checked.

Signed-off-by: Karsten Graul <kgraul@linux.vnet.ibm.com>
---
 net/smc/smc_cdc.c | 2 +-
 net/smc/smc_cdc.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/smc/smc_cdc.c b/net/smc/smc_cdc.c
index b42395d24cba..42ad57365eca 100644
--- a/net/smc/smc_cdc.c
+++ b/net/smc/smc_cdc.c
@@ -82,7 +82,7 @@ static inline void smc_cdc_add_pending_send(struct smc_connection *conn,
 		sizeof(struct smc_cdc_msg) > SMC_WR_BUF_SIZE,
 		"must increase SMC_WR_BUF_SIZE to at least sizeof(struct smc_cdc_msg)");
 	BUILD_BUG_ON_MSG(
-		offsetof(struct smc_cdc_msg, reserved) > SMC_WR_TX_SIZE,
+		sizeof(struct smc_cdc_msg) != SMC_WR_TX_SIZE,
 		"must adapt SMC_WR_TX_SIZE to sizeof(struct smc_cdc_msg); if not all smc_wr upper layer protocols use the same message size any more, must start to set link->wr_tx_sges[i].length on each individual smc_wr_tx_send()");
 	BUILD_BUG_ON_MSG(
 		sizeof(struct smc_cdc_tx_pend) > SMC_WR_TX_PEND_PRIV_SIZE,
diff --git a/net/smc/smc_cdc.h b/net/smc/smc_cdc.h
index ab240b37ad11..d2012fd22100 100644
--- a/net/smc/smc_cdc.h
+++ b/net/smc/smc_cdc.h
@@ -48,7 +48,7 @@ struct smc_cdc_msg {
 	struct smc_cdc_producer_flags	prod_flags;
 	struct smc_cdc_conn_state_flags	conn_state_flags;
 	u8				reserved[18];
-} __aligned(8);
+} __packed;					/* format defined in RFC7609 */
 
 static inline bool smc_cdc_rxed_any_close(struct smc_connection *conn)
 {
-- 
2.13.5

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

* [PATCH net-next v2 2/4] net/smc: handle sockopt TCP_NODELAY
  2018-04-19 13:56 [PATCH net-next v2 0/4] net/smc: fixes from 2018-05-17 - v2 Ursula Braun
  2018-04-19 13:56 ` [PATCH net-next v2 1/4] net/smc: fix structure size Ursula Braun
@ 2018-04-19 13:56 ` Ursula Braun
  2018-04-20 15:31   ` David Miller
  2018-04-19 13:56 ` [PATCH net-next v2 3/4] net/smc: handle sockopt TCP_CORK Ursula Braun
  2018-04-19 13:56 ` [PATCH net-next v2 4/4] net/smc: handle sockopt TCP_DEFER_ACCEPT Ursula Braun
  3 siblings, 1 reply; 6+ messages in thread
From: Ursula Braun @ 2018-04-19 13:56 UTC (permalink / raw)
  To: davem; +Cc: netdev, linux-s390, schwidefsky, heiko.carstens, raspl, ubraun

From: Ursula Braun <ubraun@linux.vnet.ibm.com>

TCP sockopts set on the SMC socket must not interfere with the
CLC handshake on the internal CLC socket. Therefore, we defer some
of them till the CLC handshake has completed, like resetting
TCP_NODELAY.

While touching setsockopt, the TCP_FASTOPEN sockopts are
ignored, because SMC currently does not support Fast Open.

Signed-off-by: Ursula Braun <ubraun@linux.vnet.ibm.com>
---
 net/smc/af_smc.c | 123 +++++++++++++++++++++++++++++++++++++++++++++++++++++--
 net/smc/smc.h    |   4 ++
 2 files changed, 123 insertions(+), 4 deletions(-)

diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
index 5f8046c62d90..297c2cb93b34 100644
--- a/net/smc/af_smc.c
+++ b/net/smc/af_smc.c
@@ -377,6 +377,24 @@ static void smc_link_save_peer_info(struct smc_link *link,
 	link->peer_mtu = clc->qp_mtu;
 }
 
+/* deferred setsockopt's not desired during clc handshake */
+static int smc_apply_deferred_sockopts(struct smc_sock *smc)
+{
+	struct smc_sock *opt_smc = smc;
+	int val, rc = 0;
+
+	if (smc->listen_smc)
+		opt_smc = smc->listen_smc;
+	if (opt_smc->deferred_nodelay_reset) {
+		val = 0;
+		rc = kernel_setsockopt(smc->clcsock, SOL_TCP, TCP_NODELAY,
+				       (char *)&val, sizeof(val));
+		if (!rc)
+			opt_smc->deferred_nodelay_reset = 0;
+	}
+	return rc;
+}
+
 /* setup for RDMA connection of client */
 static int smc_connect_rdma(struct smc_sock *smc)
 {
@@ -506,6 +524,7 @@ static int smc_connect_rdma(struct smc_sock *smc)
 	smc_tx_init(smc);
 
 out_connected:
+	rc = smc_apply_deferred_sockopts(smc);
 	smc_copy_sock_settings_to_clc(smc);
 	if (smc->sk.sk_state == SMC_INIT)
 		smc->sk.sk_state = SMC_ACTIVE;
@@ -908,6 +927,9 @@ static void smc_listen_work(struct work_struct *work)
 	mutex_unlock(&smc_create_lgr_pending);
 
 out_connected:
+	rc = smc_apply_deferred_sockopts(new_smc);
+	if (rc)
+		goto out_err;
 	sk_refcnt_debug_inc(newsmcsk);
 	if (newsmcsk->sk_state == SMC_INIT)
 		newsmcsk->sk_state = SMC_ACTIVE;
@@ -1280,23 +1302,111 @@ static int smc_setsockopt(struct socket *sock, int level, int optname,
 {
 	struct sock *sk = sock->sk;
 	struct smc_sock *smc;
+	int val, rc = 0;
 
 	smc = smc_sk(sk);
+	if (smc->use_fallback || level != SOL_TCP)
+		goto clcsock;
+
+	/* level SOL_TCP */
+	switch (optname) {
+	case TCP_CONGESTION:
+	case TCP_ULP:
+		/* sockopts without integer value; do not apply to SMC */
+		goto clcsock;
+	default:
+		break;
+	}
+
+	if (optlen < sizeof(int))
+		return -EINVAL;
+	if (get_user(val, (int __user *)optval))
+		return -EFAULT;
+
+	lock_sock(sk);
+	switch (optname) {
+	case TCP_NODELAY:
+		if (sk->sk_state != SMC_INIT && sk->sk_state != SMC_LISTEN) {
+			release_sock(sk);
+			goto clcsock;
+		}
+		/* for the CLC-handshake TCP_NODELAY is desired;
+		 * in case of fallback to TCP, a nodelay reset is
+		 * triggered afterwards.
+		 */
+		if (val)
+			smc->deferred_nodelay_reset = 0;
+		else
+			smc->deferred_nodelay_reset = 1;
+		break;
+	case TCP_FASTOPEN:
+	case TCP_FASTOPEN_CONNECT:
+	case TCP_FASTOPEN_KEY:
+	case TCP_FASTOPEN_NO_COOKIE:
+		/* ignore these options; 3-way handshake shouldn't be
+		 * bypassed with SMC
+		 */
+		rc = -EOPNOTSUPP;
+		break;
+	default:
+		/* apply option to the CLC socket */
+		release_sock(sk);
+		goto clcsock;
+	}
+	release_sock(sk);
+	return rc;
 
+clcsock:
 	/* generic setsockopts reaching us here always apply to the
 	 * CLC socket
 	 */
-	return smc->clcsock->ops->setsockopt(smc->clcsock, level, optname,
-					     optval, optlen);
+	rc = smc->clcsock->ops->setsockopt(smc->clcsock, level, optname,
+					   optval, optlen);
+	if (smc->clcsock->sk->sk_err) {
+		sk->sk_err = smc->clcsock->sk->sk_err;
+		sk->sk_error_report(sk);
+	}
+	return rc;
 }
 
 static int smc_getsockopt(struct socket *sock, int level, int optname,
 			  char __user *optval, int __user *optlen)
 {
+	struct sock *sk = sock->sk;
 	struct smc_sock *smc;
+	int val, len;
 
-	smc = smc_sk(sock->sk);
-	/* socket options apply to the CLC socket */
+	smc = smc_sk(sk);
+
+	if (smc->use_fallback || level != SOL_TCP)
+		goto clcsock;
+
+	if (get_user(len, optlen))
+		return -EFAULT;
+	len = min_t(unsigned int, len, sizeof(int));
+	if (len < 0)
+		return -EINVAL;
+
+	/* level SOL_TCP */
+	switch (optname) {
+	case TCP_NODELAY:
+		if (smc->deferred_nodelay_reset)
+			val = 0;
+		else
+			goto clcsock;
+		break;
+	default:
+		goto clcsock;
+	}
+
+	if (put_user(len, optlen))
+		return -EFAULT;
+	if (copy_to_user(optval, &val, len))
+		return -EFAULT;
+	return 0;
+
+clcsock:
+	/* socket options applying to the CLC socket */
 	return smc->clcsock->ops->getsockopt(smc->clcsock, level, optname,
 					     optval, optlen);
 }
@@ -1387,6 +1497,7 @@ static int smc_create(struct net *net, struct socket *sock, int protocol,
 	int family = (protocol == SMCPROTO_SMC6) ? PF_INET6 : PF_INET;
 	struct smc_sock *smc;
 	struct sock *sk;
+	int val = 1;
 	int rc;
 
 	rc = -ESOCKTNOSUPPORT;
@@ -1412,6 +1523,10 @@ static int smc_create(struct net *net, struct socket *sock, int protocol,
 		sk_common_release(sk);
 		goto out;
 	}
+	/* clc handshake should run with disabled Nagle algorithm */
+	rc = kernel_setsockopt(smc->clcsock, SOL_TCP, TCP_NODELAY, (char *)&val,
+			       sizeof(val));
+	smc->deferred_nodelay_reset = 1; /* TCP_NODELAY is not the default */
 	smc->sk.sk_sndbuf = max(smc->clcsock->sk->sk_sndbuf, SMC_BUF_MIN_SIZE);
 	smc->sk.sk_rcvbuf = max(smc->clcsock->sk->sk_rcvbuf, SMC_BUF_MIN_SIZE);
 
diff --git a/net/smc/smc.h b/net/smc/smc.h
index e4829a2f46ba..6dfc1c90bed2 100644
--- a/net/smc/smc.h
+++ b/net/smc/smc.h
@@ -185,6 +185,10 @@ struct smc_sock {				/* smc sock container */
 						 * started, waiting for unsent
 						 * data to be sent
 						 */
+	u8			deferred_nodelay_reset : 1;
+						/* defer Nagle after CLC
+						 * handshake
+						 */
 };
 
 static inline struct smc_sock *smc_sk(const struct sock *sk)
-- 
2.13.5

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

* [PATCH net-next v2 3/4] net/smc: handle sockopt TCP_CORK
  2018-04-19 13:56 [PATCH net-next v2 0/4] net/smc: fixes from 2018-05-17 - v2 Ursula Braun
  2018-04-19 13:56 ` [PATCH net-next v2 1/4] net/smc: fix structure size Ursula Braun
  2018-04-19 13:56 ` [PATCH net-next v2 2/4] net/smc: handle sockopt TCP_NODELAY Ursula Braun
@ 2018-04-19 13:56 ` Ursula Braun
  2018-04-19 13:56 ` [PATCH net-next v2 4/4] net/smc: handle sockopt TCP_DEFER_ACCEPT Ursula Braun
  3 siblings, 0 replies; 6+ messages in thread
From: Ursula Braun @ 2018-04-19 13:56 UTC (permalink / raw)
  To: davem; +Cc: netdev, linux-s390, schwidefsky, heiko.carstens, raspl, ubraun

From: Ursula Braun <ubraun@linux.vnet.ibm.com>

TCP sockopts must not interfere with the CLC handshake on the
CLC socket. Therefore, we defer some of them till the CLC
handshake has completed, like setting TCP_CORK.

For a corked SMC socket RDMA writes are deferred, if there is
still sufficient send buffer space available.

Signed-off-by: Ursula Braun <ubraun@linux.vnet.ibm.com>
---
 net/smc/af_smc.c | 36 +++++++++++++++++++++++++++++++++++-
 net/smc/smc.h    |  4 ++++
 net/smc/smc_tx.c | 16 +++++++++++++---
 net/smc/smc_tx.h |  8 ++++++++
 4 files changed, 60 insertions(+), 4 deletions(-)

diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
index 297c2cb93b34..27d3aa8d0181 100644
--- a/net/smc/af_smc.c
+++ b/net/smc/af_smc.c
@@ -389,8 +389,16 @@ static int smc_apply_deferred_sockopts(struct smc_sock *smc)
 		val = 0;
 		rc = kernel_setsockopt(smc->clcsock, SOL_TCP, TCP_NODELAY,
 				       (char *)&val, sizeof(val));
+		if (rc)
+			return rc;
+		opt_smc->deferred_nodelay_reset = 0;
+	}
+	if (opt_smc->deferred_cork_set) {
+		val = 1;
+		rc = kernel_setsockopt(smc->clcsock, SOL_TCP, TCP_CORK,
+				       (char *)&val, sizeof(val));
 		if (!rc)
-			opt_smc->deferred_nodelay_reset = 0;
+			opt_smc->deferred_cork_set = 0;
 	}
 	return rc;
 }
@@ -1327,6 +1335,9 @@ static int smc_setsockopt(struct socket *sock, int level, int optname,
 	switch (optname) {
 	case TCP_NODELAY:
 		if (sk->sk_state != SMC_INIT && sk->sk_state != SMC_LISTEN) {
+			if (val && smc_tx_is_corked(smc))
+				mod_delayed_work(system_wq, &smc->conn.tx_work,
+						 0);
 			release_sock(sk);
 			goto clcsock;
 		}
@@ -1339,6 +1350,23 @@ static int smc_setsockopt(struct socket *sock, int level, int optname,
 		else
 			smc->deferred_nodelay_reset = 1;
 		break;
+	case TCP_CORK:
+		if (sk->sk_state != SMC_INIT && sk->sk_state != SMC_LISTEN) {
+			if (!val)
+				mod_delayed_work(system_wq, &smc->conn.tx_work,
+						 0);
+			release_sock(sk);
+			goto clcsock;
+		}
+		/* for the CLC-handshake TCP_CORK is not desired;
+		 * in case of fallback to TCP, cork setting is
+		 * triggered afterwards.
+		 */
+		if (val)
+			smc->deferred_cork_set = 1;
+		else
+			smc->deferred_cork_set = 0;
+		break;
 	case TCP_FASTOPEN:
 	case TCP_FASTOPEN_CONNECT:
 	case TCP_FASTOPEN_KEY:
@@ -1395,6 +1423,12 @@ static int smc_getsockopt(struct socket *sock, int level, int optname,
 		else
 			goto clcsock;
 		break;
+	case TCP_CORK:
+		if (smc->deferred_cork_set)
+			val = 1;
+		else
+			goto clcsock;
+		break;
 	default:
 		goto clcsock;
 	}
diff --git a/net/smc/smc.h b/net/smc/smc.h
index 6dfc1c90bed2..38888da5a5ea 100644
--- a/net/smc/smc.h
+++ b/net/smc/smc.h
@@ -189,6 +189,10 @@ struct smc_sock {				/* smc sock container */
 						/* defer Nagle after CLC
 						 * handshake
 						 */
+	u8			deferred_cork_set : 1;
+						/* defer corking after CLC
+						 * handshake
+						 */
 };
 
 static inline struct smc_sock *smc_sk(const struct sock *sk)
diff --git a/net/smc/smc_tx.c b/net/smc/smc_tx.c
index 72f004c9c9b1..a31377bb400b 100644
--- a/net/smc/smc_tx.c
+++ b/net/smc/smc_tx.c
@@ -26,6 +26,7 @@
 #include "smc_tx.h"
 
 #define SMC_TX_WORK_DELAY	HZ
+#define SMC_TX_CORK_DELAY	(HZ >> 2)	/* 250 ms */
 
 /***************************** sndbuf producer *******************************/
 
@@ -209,7 +210,16 @@ int smc_tx_sendmsg(struct smc_sock *smc, struct msghdr *msg, size_t len)
 		/* since we just produced more new data into sndbuf,
 		 * trigger sndbuf consumer: RDMA write into peer RMBE and CDC
 		 */
-		smc_tx_sndbuf_nonempty(conn);
+		if ((msg->msg_flags & MSG_MORE || smc_tx_is_corked(smc)) &&
+		    (atomic_read(&conn->sndbuf_space) >
+						(conn->sndbuf_size >> 1)))
+			/* for a corked socket defer the RDMA writes if there
+			 * is still sufficient sndbuf_space available
+			 */
+			schedule_delayed_work(&conn->tx_work,
+					      SMC_TX_CORK_DELAY);
+		else
+			smc_tx_sndbuf_nonempty(conn);
 	} /* while (msg_data_left(msg)) */
 
 	return send_done;
@@ -409,8 +419,8 @@ int smc_tx_sndbuf_nonempty(struct smc_connection *conn)
 			}
 			rc = 0;
 			if (conn->alert_token_local) /* connection healthy */
-				schedule_delayed_work(&conn->tx_work,
-						      SMC_TX_WORK_DELAY);
+				mod_delayed_work(system_wq, &conn->tx_work,
+						 SMC_TX_WORK_DELAY);
 		}
 		goto out_unlock;
 	}
diff --git a/net/smc/smc_tx.h b/net/smc/smc_tx.h
index 78255964fa4d..e5f4188b4bdb 100644
--- a/net/smc/smc_tx.h
+++ b/net/smc/smc_tx.h
@@ -14,6 +14,7 @@
 
 #include <linux/socket.h>
 #include <linux/types.h>
+#include <net/tcp.h>
 
 #include "smc.h"
 #include "smc_cdc.h"
@@ -27,6 +28,13 @@ static inline int smc_tx_prepared_sends(struct smc_connection *conn)
 	return smc_curs_diff(conn->sndbuf_size, &sent, &prep);
 }
 
+static inline bool smc_tx_is_corked(struct smc_sock *smc)
+{
+	struct tcp_sock *tp = tcp_sk(smc->clcsock->sk);
+
+	return (tp->nonagle & TCP_NAGLE_CORK) ? true : false;
+}
+
 void smc_tx_init(struct smc_sock *smc);
 int smc_tx_sendmsg(struct smc_sock *smc, struct msghdr *msg, size_t len);
 int smc_tx_sndbuf_nonempty(struct smc_connection *conn);
-- 
2.13.5

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

* [PATCH net-next v2 4/4] net/smc: handle sockopt TCP_DEFER_ACCEPT
  2018-04-19 13:56 [PATCH net-next v2 0/4] net/smc: fixes from 2018-05-17 - v2 Ursula Braun
                   ` (2 preceding siblings ...)
  2018-04-19 13:56 ` [PATCH net-next v2 3/4] net/smc: handle sockopt TCP_CORK Ursula Braun
@ 2018-04-19 13:56 ` Ursula Braun
  3 siblings, 0 replies; 6+ messages in thread
From: Ursula Braun @ 2018-04-19 13:56 UTC (permalink / raw)
  To: davem; +Cc: netdev, linux-s390, schwidefsky, heiko.carstens, raspl, ubraun

From: Ursula Braun <ubraun@linux.vnet.ibm.com>

TCP sockopt setting of TCP_DEFER_ACCEPT should just be applied
to the SMC socket, but not to the internal CLC socket.

If set, the accept is delayed till data is available.

Signed-off-by: Ursula Braun <ubraun@linux.vnet.ibm.com>
---
 net/smc/af_smc.c | 36 +++++++++++++++++++++++++++++++++++-
 net/smc/smc.h    |  4 ++++
 net/smc/smc_rx.c |  2 +-
 net/smc/smc_rx.h |  1 +
 4 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
index 27d3aa8d0181..c5ece2b28179 100644
--- a/net/smc/af_smc.c
+++ b/net/smc/af_smc.c
@@ -1098,9 +1098,29 @@ static int smc_accept(struct socket *sock, struct socket *new_sock,
 
 	if (!rc)
 		rc = sock_error(nsk);
+	release_sock(sk);
+	if (rc)
+		goto out;
+
+	if (lsmc->sockopt_defer_accept && !(flags & O_NONBLOCK)) {
+		/* wait till data arrives on the socket */
+		timeo = msecs_to_jiffies(lsmc->sockopt_defer_accept *
+								MSEC_PER_SEC);
+		if (smc_sk(nsk)->use_fallback) {
+			struct sock *clcsk = smc_sk(nsk)->clcsock->sk;
+
+			lock_sock(clcsk);
+			if (skb_queue_empty(&clcsk->sk_receive_queue))
+				sk_wait_data(clcsk, &timeo, NULL);
+			release_sock(clcsk);
+		} else if (!atomic_read(&smc_sk(nsk)->conn.bytes_to_rcv)) {
+			lock_sock(nsk);
+			smc_rx_wait_data(smc_sk(nsk), &timeo);
+			release_sock(nsk);
+		}
+	}
 
 out:
-	release_sock(sk);
 	sock_put(sk); /* sock_hold above */
 	return rc;
 }
@@ -1367,6 +1387,14 @@ static int smc_setsockopt(struct socket *sock, int level, int optname,
 		else
 			smc->deferred_cork_set = 0;
 		break;
+	case TCP_DEFER_ACCEPT:
+		if (sk->sk_state != SMC_INIT && sk->sk_state != SMC_LISTEN) {
+			release_sock(sk);
+			goto clcsock;
+		}
+		/* for the CLC-handshake TCP_DEFER_ACCEPT is not desired */
+		smc->sockopt_defer_accept = val;
+		break;
 	case TCP_FASTOPEN:
 	case TCP_FASTOPEN_CONNECT:
 	case TCP_FASTOPEN_KEY:
@@ -1429,6 +1457,12 @@ static int smc_getsockopt(struct socket *sock, int level, int optname,
 		else
 			goto clcsock;
 		break;
+	case TCP_DEFER_ACCEPT:
+		if (smc->sockopt_defer_accept)
+			val = smc->sockopt_defer_accept;
+		else
+			goto clcsock;
+		break;
 	default:
 		goto clcsock;
 	}
diff --git a/net/smc/smc.h b/net/smc/smc.h
index 38888da5a5ea..11f869d1f28a 100644
--- a/net/smc/smc.h
+++ b/net/smc/smc.h
@@ -180,6 +180,10 @@ struct smc_sock {				/* smc sock container */
 	struct list_head	accept_q;	/* sockets to be accepted */
 	spinlock_t		accept_q_lock;	/* protects accept_q */
 	bool			use_fallback;	/* fallback to tcp */
+	int			sockopt_defer_accept;
+						/* sockopt TCP_DEFER_ACCEPT
+						 * value
+						 */
 	u8			wait_close_tx_prepared : 1;
 						/* shutdown wr or close
 						 * started, waiting for unsent
diff --git a/net/smc/smc_rx.c b/net/smc/smc_rx.c
index eff4e0d0bb31..af851d8df1f8 100644
--- a/net/smc/smc_rx.c
+++ b/net/smc/smc_rx.c
@@ -51,7 +51,7 @@ static void smc_rx_data_ready(struct sock *sk)
  * 1 if at least 1 byte available in rcvbuf or if socket error/shutdown.
  * 0 otherwise (nothing in rcvbuf nor timeout, e.g. interrupted).
  */
-static int smc_rx_wait_data(struct smc_sock *smc, long *timeo)
+int smc_rx_wait_data(struct smc_sock *smc, long *timeo)
 {
 	DEFINE_WAIT_FUNC(wait, woken_wake_function);
 	struct smc_connection *conn = &smc->conn;
diff --git a/net/smc/smc_rx.h b/net/smc/smc_rx.h
index 3a32b59bf06c..0b75a6b470e6 100644
--- a/net/smc/smc_rx.h
+++ b/net/smc/smc_rx.h
@@ -20,5 +20,6 @@
 void smc_rx_init(struct smc_sock *smc);
 int smc_rx_recvmsg(struct smc_sock *smc, struct msghdr *msg, size_t len,
 		   int flags);
+int smc_rx_wait_data(struct smc_sock *smc, long *timeo);
 
 #endif /* SMC_RX_H */
-- 
2.13.5

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

* Re: [PATCH net-next v2 2/4] net/smc: handle sockopt TCP_NODELAY
  2018-04-19 13:56 ` [PATCH net-next v2 2/4] net/smc: handle sockopt TCP_NODELAY Ursula Braun
@ 2018-04-20 15:31   ` David Miller
  0 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2018-04-20 15:31 UTC (permalink / raw)
  To: ubraun; +Cc: netdev, linux-s390, schwidefsky, heiko.carstens, raspl, ubraun

From: Ursula Braun <ubraun@linux.ibm.com>
Date: Thu, 19 Apr 2018 15:56:53 +0200

> @@ -1412,6 +1523,10 @@ static int smc_create(struct net *net, struct socket *sock, int protocol,
>  		sk_common_release(sk);
>  		goto out;
>  	}
> +	/* clc handshake should run with disabled Nagle algorithm */
> +	rc = kernel_setsockopt(smc->clcsock, SOL_TCP, TCP_NODELAY, (char *)&val,
> +			       sizeof(val));
> +	smc->deferred_nodelay_reset = 1; /* TCP_NODELAY is not the default */
>  	smc->sk.sk_sndbuf = max(smc->clcsock->sk->sk_sndbuf, SMC_BUF_MIN_SIZE);
>  	smc->sk.sk_rcvbuf = max(smc->clcsock->sk->sk_rcvbuf, SMC_BUF_MIN_SIZE);

This is not what I asked for.

If the user asked for the socket option, you are unconditionally returning success
to the original user.

If it fails here during the smc create, it's too late to handle the error properly.

This is the problem you must solve before I can take these changes properly.

You aren't even really failing the smc_create() here, because if you were you
would kill and free up the clcsock and release 'sk'.

As it stands now you are returning an error, and not releasing resources, if
the kernel_setsockopt() fails.

But more fundamentally these semantics are terrible.  You must not ever create
a situation where you tell the user his setsockopt succeeded by in the end
not honoring that reqeust fully.  That is what your current changes allow to
happen.

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

end of thread, other threads:[~2018-04-20 15:31 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-19 13:56 [PATCH net-next v2 0/4] net/smc: fixes from 2018-05-17 - v2 Ursula Braun
2018-04-19 13:56 ` [PATCH net-next v2 1/4] net/smc: fix structure size Ursula Braun
2018-04-19 13:56 ` [PATCH net-next v2 2/4] net/smc: handle sockopt TCP_NODELAY Ursula Braun
2018-04-20 15:31   ` David Miller
2018-04-19 13:56 ` [PATCH net-next v2 3/4] net/smc: handle sockopt TCP_CORK Ursula Braun
2018-04-19 13:56 ` [PATCH net-next v2 4/4] net/smc: handle sockopt TCP_DEFER_ACCEPT Ursula Braun

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.