All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next] sctp: add support for SCTP_REUSE_PORT sockopt
@ 2018-05-19  7:44 ` Xin Long
  0 siblings, 0 replies; 33+ messages in thread
From: Xin Long @ 2018-05-19  7:44 UTC (permalink / raw)
  To: network dev, linux-sctp; +Cc: Marcelo Ricardo Leitner, Neil Horman, davem

This feature is actually already supported by sk->sk_reuse which can be
set by SO_REUSEADDR. But it's not working exactly as RFC6458 demands in
section 8.1.27, like:

  - This option only supports one-to-one style SCTP sockets
  - This socket option must not be used after calling bind()
    or sctp_bindx().

Besides, SCTP_REUSE_PORT sockopt should be provided for user's programs.
Otherwise, the programs with SCTP_REUSE_PORT from other systems will not
work in linux.

This patch reuses sk->sk_reuse and works pretty much as SO_REUSEADDR,
just with some extra setup limitations that are neeeded when it is being
enabled.

"It should be noted that the behavior of the socket-level socket option
to reuse ports and/or addresses for SCTP sockets is unspecified", so it
leaves SO_REUSEADDR as is for the compatibility.

Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 include/uapi/linux/sctp.h |  1 +
 net/sctp/socket.c         | 48 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 49 insertions(+)

diff --git a/include/uapi/linux/sctp.h b/include/uapi/linux/sctp.h
index b64d583..c02986a 100644
--- a/include/uapi/linux/sctp.h
+++ b/include/uapi/linux/sctp.h
@@ -100,6 +100,7 @@ typedef __s32 sctp_assoc_t;
 #define SCTP_RECVNXTINFO	33
 #define SCTP_DEFAULT_SNDINFO	34
 #define SCTP_AUTH_DEACTIVATE_KEY	35
+#define SCTP_REUSE_PORT		36
 
 /* Internal Socket Options. Some of the sctp library functions are
  * implemented using these socket options.
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 1b4593b..8dfcc79 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -4170,6 +4170,28 @@ static int sctp_setsockopt_interleaving_supported(struct sock *sk,
 	return retval;
 }
 
+static int sctp_setsockopt_reuse_port(struct sock *sk, char __user *optval,
+				      unsigned int optlen)
+{
+	int val;
+
+	if (!sctp_style(sk, TCP))
+		return -EOPNOTSUPP;
+
+	if (sctp_sk(sk)->ep->base.bind_addr.port)
+		return -EFAULT;
+
+	if (optlen < sizeof(int))
+		return -EINVAL;
+
+	if (get_user(val, (int __user *)optval))
+		return -EFAULT;
+
+	sk->sk_reuse = val ? SK_CAN_REUSE : SK_NO_REUSE;
+
+	return 0;
+}
+
 /* API 6.2 setsockopt(), getsockopt()
  *
  * Applications use setsockopt() and getsockopt() to set or retrieve
@@ -4364,6 +4386,9 @@ static int sctp_setsockopt(struct sock *sk, int level, int optname,
 		retval = sctp_setsockopt_interleaving_supported(sk, optval,
 								optlen);
 		break;
+	case SCTP_REUSE_PORT:
+		retval = sctp_setsockopt_reuse_port(sk, optval, optlen);
+		break;
 	default:
 		retval = -ENOPROTOOPT;
 		break;
@@ -7175,6 +7200,26 @@ static int sctp_getsockopt_interleaving_supported(struct sock *sk, int len,
 	return retval;
 }
 
+static int sctp_getsockopt_reuse_port(struct sock *sk, int len,
+				      char __user *optval,
+				      int __user *optlen)
+{
+	int val = 0;
+
+	if (len < sizeof(int))
+		return -EINVAL;
+
+	len = sizeof(int);
+	if (sk->sk_reuse != SK_NO_REUSE)
+		val = 1;
+	if (put_user(len, optlen))
+		return -EFAULT;
+	if (copy_to_user(optval, &val, len))
+		return -EFAULT;
+
+	return 0;
+}
+
 static int sctp_getsockopt(struct sock *sk, int level, int optname,
 			   char __user *optval, int __user *optlen)
 {
@@ -7370,6 +7415,9 @@ static int sctp_getsockopt(struct sock *sk, int level, int optname,
 		retval = sctp_getsockopt_interleaving_supported(sk, len, optval,
 								optlen);
 		break;
+	case SCTP_REUSE_PORT:
+		retval = sctp_getsockopt_reuse_port(sk, len, optval, optlen);
+		break;
 	default:
 		retval = -ENOPROTOOPT;
 		break;
-- 
2.1.0

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

* [PATCH net-next] sctp: add support for SCTP_REUSE_PORT sockopt
@ 2018-05-19  7:44 ` Xin Long
  0 siblings, 0 replies; 33+ messages in thread
From: Xin Long @ 2018-05-19  7:44 UTC (permalink / raw)
  To: network dev, linux-sctp; +Cc: Marcelo Ricardo Leitner, Neil Horman, davem

This feature is actually already supported by sk->sk_reuse which can be
set by SO_REUSEADDR. But it's not working exactly as RFC6458 demands in
section 8.1.27, like:

  - This option only supports one-to-one style SCTP sockets
  - This socket option must not be used after calling bind()
    or sctp_bindx().

Besides, SCTP_REUSE_PORT sockopt should be provided for user's programs.
Otherwise, the programs with SCTP_REUSE_PORT from other systems will not
work in linux.

This patch reuses sk->sk_reuse and works pretty much as SO_REUSEADDR,
just with some extra setup limitations that are neeeded when it is being
enabled.

"It should be noted that the behavior of the socket-level socket option
to reuse ports and/or addresses for SCTP sockets is unspecified", so it
leaves SO_REUSEADDR as is for the compatibility.

Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 include/uapi/linux/sctp.h |  1 +
 net/sctp/socket.c         | 48 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 49 insertions(+)

diff --git a/include/uapi/linux/sctp.h b/include/uapi/linux/sctp.h
index b64d583..c02986a 100644
--- a/include/uapi/linux/sctp.h
+++ b/include/uapi/linux/sctp.h
@@ -100,6 +100,7 @@ typedef __s32 sctp_assoc_t;
 #define SCTP_RECVNXTINFO	33
 #define SCTP_DEFAULT_SNDINFO	34
 #define SCTP_AUTH_DEACTIVATE_KEY	35
+#define SCTP_REUSE_PORT		36
 
 /* Internal Socket Options. Some of the sctp library functions are
  * implemented using these socket options.
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 1b4593b..8dfcc79 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -4170,6 +4170,28 @@ static int sctp_setsockopt_interleaving_supported(struct sock *sk,
 	return retval;
 }
 
+static int sctp_setsockopt_reuse_port(struct sock *sk, char __user *optval,
+				      unsigned int optlen)
+{
+	int val;
+
+	if (!sctp_style(sk, TCP))
+		return -EOPNOTSUPP;
+
+	if (sctp_sk(sk)->ep->base.bind_addr.port)
+		return -EFAULT;
+
+	if (optlen < sizeof(int))
+		return -EINVAL;
+
+	if (get_user(val, (int __user *)optval))
+		return -EFAULT;
+
+	sk->sk_reuse = val ? SK_CAN_REUSE : SK_NO_REUSE;
+
+	return 0;
+}
+
 /* API 6.2 setsockopt(), getsockopt()
  *
  * Applications use setsockopt() and getsockopt() to set or retrieve
@@ -4364,6 +4386,9 @@ static int sctp_setsockopt(struct sock *sk, int level, int optname,
 		retval = sctp_setsockopt_interleaving_supported(sk, optval,
 								optlen);
 		break;
+	case SCTP_REUSE_PORT:
+		retval = sctp_setsockopt_reuse_port(sk, optval, optlen);
+		break;
 	default:
 		retval = -ENOPROTOOPT;
 		break;
@@ -7175,6 +7200,26 @@ static int sctp_getsockopt_interleaving_supported(struct sock *sk, int len,
 	return retval;
 }
 
+static int sctp_getsockopt_reuse_port(struct sock *sk, int len,
+				      char __user *optval,
+				      int __user *optlen)
+{
+	int val = 0;
+
+	if (len < sizeof(int))
+		return -EINVAL;
+
+	len = sizeof(int);
+	if (sk->sk_reuse != SK_NO_REUSE)
+		val = 1;
+	if (put_user(len, optlen))
+		return -EFAULT;
+	if (copy_to_user(optval, &val, len))
+		return -EFAULT;
+
+	return 0;
+}
+
 static int sctp_getsockopt(struct sock *sk, int level, int optname,
 			   char __user *optval, int __user *optlen)
 {
@@ -7370,6 +7415,9 @@ static int sctp_getsockopt(struct sock *sk, int level, int optname,
 		retval = sctp_getsockopt_interleaving_supported(sk, len, optval,
 								optlen);
 		break;
+	case SCTP_REUSE_PORT:
+		retval = sctp_getsockopt_reuse_port(sk, len, optval, optlen);
+		break;
 	default:
 		retval = -ENOPROTOOPT;
 		break;
-- 
2.1.0


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

* Re: [PATCH net-next] sctp: add support for SCTP_REUSE_PORT sockopt
  2018-05-19  7:44 ` Xin Long
@ 2018-05-19 20:26   ` Marcelo Ricardo Leitner
  -1 siblings, 0 replies; 33+ messages in thread
From: Marcelo Ricardo Leitner @ 2018-05-19 20:26 UTC (permalink / raw)
  To: Xin Long; +Cc: network dev, linux-sctp, Neil Horman, davem

On Sat, May 19, 2018 at 03:44:40PM +0800, Xin Long wrote:
> This feature is actually already supported by sk->sk_reuse which can be
> set by SO_REUSEADDR. But it's not working exactly as RFC6458 demands in
> section 8.1.27, like:
>
>   - This option only supports one-to-one style SCTP sockets
>   - This socket option must not be used after calling bind()
>     or sctp_bindx().
>
> Besides, SCTP_REUSE_PORT sockopt should be provided for user's programs.
> Otherwise, the programs with SCTP_REUSE_PORT from other systems will not
> work in linux.
>
> This patch reuses sk->sk_reuse and works pretty much as SO_REUSEADDR,
> just with some extra setup limitations that are neeeded when it is being
> enabled.
>
> "It should be noted that the behavior of the socket-level socket option
> to reuse ports and/or addresses for SCTP sockets is unspecified", so it
> leaves SO_REUSEADDR as is for the compatibility.
>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>

Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>

> ---
>  include/uapi/linux/sctp.h |  1 +
>  net/sctp/socket.c         | 48 +++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 49 insertions(+)
>
> diff --git a/include/uapi/linux/sctp.h b/include/uapi/linux/sctp.h
> index b64d583..c02986a 100644
> --- a/include/uapi/linux/sctp.h
> +++ b/include/uapi/linux/sctp.h
> @@ -100,6 +100,7 @@ typedef __s32 sctp_assoc_t;
>  #define SCTP_RECVNXTINFO	33
>  #define SCTP_DEFAULT_SNDINFO	34
>  #define SCTP_AUTH_DEACTIVATE_KEY	35
> +#define SCTP_REUSE_PORT		36
>
>  /* Internal Socket Options. Some of the sctp library functions are
>   * implemented using these socket options.
> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> index 1b4593b..8dfcc79 100644
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@ -4170,6 +4170,28 @@ static int sctp_setsockopt_interleaving_supported(struct sock *sk,
>  	return retval;
>  }
>
> +static int sctp_setsockopt_reuse_port(struct sock *sk, char __user *optval,
> +				      unsigned int optlen)
> +{
> +	int val;
> +
> +	if (!sctp_style(sk, TCP))
> +		return -EOPNOTSUPP;
> +
> +	if (sctp_sk(sk)->ep->base.bind_addr.port)
> +		return -EFAULT;
> +
> +	if (optlen < sizeof(int))
> +		return -EINVAL;
> +
> +	if (get_user(val, (int __user *)optval))
> +		return -EFAULT;
> +
> +	sk->sk_reuse = val ? SK_CAN_REUSE : SK_NO_REUSE;
> +
> +	return 0;
> +}
> +
>  /* API 6.2 setsockopt(), getsockopt()
>   *
>   * Applications use setsockopt() and getsockopt() to set or retrieve
> @@ -4364,6 +4386,9 @@ static int sctp_setsockopt(struct sock *sk, int level, int optname,
>  		retval = sctp_setsockopt_interleaving_supported(sk, optval,
>  								optlen);
>  		break;
> +	case SCTP_REUSE_PORT:
> +		retval = sctp_setsockopt_reuse_port(sk, optval, optlen);
> +		break;
>  	default:
>  		retval = -ENOPROTOOPT;
>  		break;
> @@ -7175,6 +7200,26 @@ static int sctp_getsockopt_interleaving_supported(struct sock *sk, int len,
>  	return retval;
>  }
>
> +static int sctp_getsockopt_reuse_port(struct sock *sk, int len,
> +				      char __user *optval,
> +				      int __user *optlen)
> +{
> +	int val = 0;
> +
> +	if (len < sizeof(int))
> +		return -EINVAL;
> +
> +	len = sizeof(int);
> +	if (sk->sk_reuse != SK_NO_REUSE)
> +		val = 1;
> +	if (put_user(len, optlen))
> +		return -EFAULT;
> +	if (copy_to_user(optval, &val, len))
> +		return -EFAULT;
> +
> +	return 0;
> +}
> +
>  static int sctp_getsockopt(struct sock *sk, int level, int optname,
>  			   char __user *optval, int __user *optlen)
>  {
> @@ -7370,6 +7415,9 @@ static int sctp_getsockopt(struct sock *sk, int level, int optname,
>  		retval = sctp_getsockopt_interleaving_supported(sk, len, optval,
>  								optlen);
>  		break;
> +	case SCTP_REUSE_PORT:
> +		retval = sctp_getsockopt_reuse_port(sk, len, optval, optlen);
> +		break;
>  	default:
>  		retval = -ENOPROTOOPT;
>  		break;
> --
> 2.1.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

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

* Re: [PATCH net-next] sctp: add support for SCTP_REUSE_PORT sockopt
@ 2018-05-19 20:26   ` Marcelo Ricardo Leitner
  0 siblings, 0 replies; 33+ messages in thread
From: Marcelo Ricardo Leitner @ 2018-05-19 20:26 UTC (permalink / raw)
  To: Xin Long; +Cc: network dev, linux-sctp, Neil Horman, davem

On Sat, May 19, 2018 at 03:44:40PM +0800, Xin Long wrote:
> This feature is actually already supported by sk->sk_reuse which can be
> set by SO_REUSEADDR. But it's not working exactly as RFC6458 demands in
> section 8.1.27, like:
>
>   - This option only supports one-to-one style SCTP sockets
>   - This socket option must not be used after calling bind()
>     or sctp_bindx().
>
> Besides, SCTP_REUSE_PORT sockopt should be provided for user's programs.
> Otherwise, the programs with SCTP_REUSE_PORT from other systems will not
> work in linux.
>
> This patch reuses sk->sk_reuse and works pretty much as SO_REUSEADDR,
> just with some extra setup limitations that are neeeded when it is being
> enabled.
>
> "It should be noted that the behavior of the socket-level socket option
> to reuse ports and/or addresses for SCTP sockets is unspecified", so it
> leaves SO_REUSEADDR as is for the compatibility.
>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>

Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>

> ---
>  include/uapi/linux/sctp.h |  1 +
>  net/sctp/socket.c         | 48 +++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 49 insertions(+)
>
> diff --git a/include/uapi/linux/sctp.h b/include/uapi/linux/sctp.h
> index b64d583..c02986a 100644
> --- a/include/uapi/linux/sctp.h
> +++ b/include/uapi/linux/sctp.h
> @@ -100,6 +100,7 @@ typedef __s32 sctp_assoc_t;
>  #define SCTP_RECVNXTINFO	33
>  #define SCTP_DEFAULT_SNDINFO	34
>  #define SCTP_AUTH_DEACTIVATE_KEY	35
> +#define SCTP_REUSE_PORT		36
>
>  /* Internal Socket Options. Some of the sctp library functions are
>   * implemented using these socket options.
> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> index 1b4593b..8dfcc79 100644
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@ -4170,6 +4170,28 @@ static int sctp_setsockopt_interleaving_supported(struct sock *sk,
>  	return retval;
>  }
>
> +static int sctp_setsockopt_reuse_port(struct sock *sk, char __user *optval,
> +				      unsigned int optlen)
> +{
> +	int val;
> +
> +	if (!sctp_style(sk, TCP))
> +		return -EOPNOTSUPP;
> +
> +	if (sctp_sk(sk)->ep->base.bind_addr.port)
> +		return -EFAULT;
> +
> +	if (optlen < sizeof(int))
> +		return -EINVAL;
> +
> +	if (get_user(val, (int __user *)optval))
> +		return -EFAULT;
> +
> +	sk->sk_reuse = val ? SK_CAN_REUSE : SK_NO_REUSE;
> +
> +	return 0;
> +}
> +
>  /* API 6.2 setsockopt(), getsockopt()
>   *
>   * Applications use setsockopt() and getsockopt() to set or retrieve
> @@ -4364,6 +4386,9 @@ static int sctp_setsockopt(struct sock *sk, int level, int optname,
>  		retval = sctp_setsockopt_interleaving_supported(sk, optval,
>  								optlen);
>  		break;
> +	case SCTP_REUSE_PORT:
> +		retval = sctp_setsockopt_reuse_port(sk, optval, optlen);
> +		break;
>  	default:
>  		retval = -ENOPROTOOPT;
>  		break;
> @@ -7175,6 +7200,26 @@ static int sctp_getsockopt_interleaving_supported(struct sock *sk, int len,
>  	return retval;
>  }
>
> +static int sctp_getsockopt_reuse_port(struct sock *sk, int len,
> +				      char __user *optval,
> +				      int __user *optlen)
> +{
> +	int val = 0;
> +
> +	if (len < sizeof(int))
> +		return -EINVAL;
> +
> +	len = sizeof(int);
> +	if (sk->sk_reuse != SK_NO_REUSE)
> +		val = 1;
> +	if (put_user(len, optlen))
> +		return -EFAULT;
> +	if (copy_to_user(optval, &val, len))
> +		return -EFAULT;
> +
> +	return 0;
> +}
> +
>  static int sctp_getsockopt(struct sock *sk, int level, int optname,
>  			   char __user *optval, int __user *optlen)
>  {
> @@ -7370,6 +7415,9 @@ static int sctp_getsockopt(struct sock *sk, int level, int optname,
>  		retval = sctp_getsockopt_interleaving_supported(sk, len, optval,
>  								optlen);
>  		break;
> +	case SCTP_REUSE_PORT:
> +		retval = sctp_getsockopt_reuse_port(sk, len, optval, optlen);
> +		break;
>  	default:
>  		retval = -ENOPROTOOPT;
>  		break;
> --
> 2.1.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

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

* Re: [PATCH net-next] sctp: add support for SCTP_REUSE_PORT sockopt
  2018-05-19  7:44 ` Xin Long
@ 2018-05-20 19:44   ` Tom Herbert
  -1 siblings, 0 replies; 33+ messages in thread
From: Tom Herbert @ 2018-05-20 19:44 UTC (permalink / raw)
  To: Xin Long
  Cc: network dev, linux-sctp @ vger . kernel . org,
	Marcelo Ricardo Leitner, Neil Horman, David S. Miller

On Sat, May 19, 2018 at 12:44 AM, Xin Long <lucien.xin@gmail.com> wrote:
> This feature is actually already supported by sk->sk_reuse which can be
> set by SO_REUSEADDR. But it's not working exactly as RFC6458 demands in
> section 8.1.27, like:
>
>   - This option only supports one-to-one style SCTP sockets
>   - This socket option must not be used after calling bind()
>     or sctp_bindx().
>
> Besides, SCTP_REUSE_PORT sockopt should be provided for user's programs.
> Otherwise, the programs with SCTP_REUSE_PORT from other systems will not
> work in linux.
>
How is this different than SO_REUSEPORT?

> This patch reuses sk->sk_reuse and works pretty much as SO_REUSEADDR,
> just with some extra setup limitations that are neeeded when it is being
> enabled.
>
> "It should be noted that the behavior of the socket-level socket option
> to reuse ports and/or addresses for SCTP sockets is unspecified", so it
> leaves SO_REUSEADDR as is for the compatibility.
>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> ---
>  include/uapi/linux/sctp.h |  1 +
>  net/sctp/socket.c         | 48 +++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 49 insertions(+)
>
> diff --git a/include/uapi/linux/sctp.h b/include/uapi/linux/sctp.h
> index b64d583..c02986a 100644
> --- a/include/uapi/linux/sctp.h
> +++ b/include/uapi/linux/sctp.h
> @@ -100,6 +100,7 @@ typedef __s32 sctp_assoc_t;
>  #define SCTP_RECVNXTINFO       33
>  #define SCTP_DEFAULT_SNDINFO   34
>  #define SCTP_AUTH_DEACTIVATE_KEY       35
> +#define SCTP_REUSE_PORT                36
>
>  /* Internal Socket Options. Some of the sctp library functions are
>   * implemented using these socket options.
> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> index 1b4593b..8dfcc79 100644
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@ -4170,6 +4170,28 @@ static int sctp_setsockopt_interleaving_supported(struct sock *sk,
>         return retval;
>  }
>
> +static int sctp_setsockopt_reuse_port(struct sock *sk, char __user *optval,
> +                                     unsigned int optlen)
> +{
> +       int val;
> +
> +       if (!sctp_style(sk, TCP))
> +               return -EOPNOTSUPP;
> +
> +       if (sctp_sk(sk)->ep->base.bind_addr.port)
> +               return -EFAULT;
> +
> +       if (optlen < sizeof(int))
> +               return -EINVAL;
> +
> +       if (get_user(val, (int __user *)optval))
> +               return -EFAULT;
> +
> +       sk->sk_reuse = val ? SK_CAN_REUSE : SK_NO_REUSE;
> +
> +       return 0;
> +}
> +
>  /* API 6.2 setsockopt(), getsockopt()
>   *
>   * Applications use setsockopt() and getsockopt() to set or retrieve
> @@ -4364,6 +4386,9 @@ static int sctp_setsockopt(struct sock *sk, int level, int optname,
>                 retval = sctp_setsockopt_interleaving_supported(sk, optval,
>                                                                 optlen);
>                 break;
> +       case SCTP_REUSE_PORT:
> +               retval = sctp_setsockopt_reuse_port(sk, optval, optlen);
> +               break;
>         default:
>                 retval = -ENOPROTOOPT;
>                 break;
> @@ -7175,6 +7200,26 @@ static int sctp_getsockopt_interleaving_supported(struct sock *sk, int len,
>         return retval;
>  }
>
> +static int sctp_getsockopt_reuse_port(struct sock *sk, int len,
> +                                     char __user *optval,
> +                                     int __user *optlen)
> +{
> +       int val = 0;
> +
> +       if (len < sizeof(int))
> +               return -EINVAL;
> +
> +       len = sizeof(int);
> +       if (sk->sk_reuse != SK_NO_REUSE)
> +               val = 1;
> +       if (put_user(len, optlen))
> +               return -EFAULT;
> +       if (copy_to_user(optval, &val, len))
> +               return -EFAULT;
> +
> +       return 0;
> +}
> +
>  static int sctp_getsockopt(struct sock *sk, int level, int optname,
>                            char __user *optval, int __user *optlen)
>  {
> @@ -7370,6 +7415,9 @@ static int sctp_getsockopt(struct sock *sk, int level, int optname,
>                 retval = sctp_getsockopt_interleaving_supported(sk, len, optval,
>                                                                 optlen);
>                 break;
> +       case SCTP_REUSE_PORT:
> +               retval = sctp_getsockopt_reuse_port(sk, len, optval, optlen);
> +               break;
>         default:
>                 retval = -ENOPROTOOPT;
>                 break;
> --
> 2.1.0
>

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

* Re: [PATCH net-next] sctp: add support for SCTP_REUSE_PORT sockopt
@ 2018-05-20 19:44   ` Tom Herbert
  0 siblings, 0 replies; 33+ messages in thread
From: Tom Herbert @ 2018-05-20 19:44 UTC (permalink / raw)
  To: Xin Long
  Cc: network dev, linux-sctp @ vger . kernel . org,
	Marcelo Ricardo Leitner, Neil Horman, David S. Miller

On Sat, May 19, 2018 at 12:44 AM, Xin Long <lucien.xin@gmail.com> wrote:
> This feature is actually already supported by sk->sk_reuse which can be
> set by SO_REUSEADDR. But it's not working exactly as RFC6458 demands in
> section 8.1.27, like:
>
>   - This option only supports one-to-one style SCTP sockets
>   - This socket option must not be used after calling bind()
>     or sctp_bindx().
>
> Besides, SCTP_REUSE_PORT sockopt should be provided for user's programs.
> Otherwise, the programs with SCTP_REUSE_PORT from other systems will not
> work in linux.
>
How is this different than SO_REUSEPORT?

> This patch reuses sk->sk_reuse and works pretty much as SO_REUSEADDR,
> just with some extra setup limitations that are neeeded when it is being
> enabled.
>
> "It should be noted that the behavior of the socket-level socket option
> to reuse ports and/or addresses for SCTP sockets is unspecified", so it
> leaves SO_REUSEADDR as is for the compatibility.
>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> ---
>  include/uapi/linux/sctp.h |  1 +
>  net/sctp/socket.c         | 48 +++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 49 insertions(+)
>
> diff --git a/include/uapi/linux/sctp.h b/include/uapi/linux/sctp.h
> index b64d583..c02986a 100644
> --- a/include/uapi/linux/sctp.h
> +++ b/include/uapi/linux/sctp.h
> @@ -100,6 +100,7 @@ typedef __s32 sctp_assoc_t;
>  #define SCTP_RECVNXTINFO       33
>  #define SCTP_DEFAULT_SNDINFO   34
>  #define SCTP_AUTH_DEACTIVATE_KEY       35
> +#define SCTP_REUSE_PORT                36
>
>  /* Internal Socket Options. Some of the sctp library functions are
>   * implemented using these socket options.
> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> index 1b4593b..8dfcc79 100644
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@ -4170,6 +4170,28 @@ static int sctp_setsockopt_interleaving_supported(struct sock *sk,
>         return retval;
>  }
>
> +static int sctp_setsockopt_reuse_port(struct sock *sk, char __user *optval,
> +                                     unsigned int optlen)
> +{
> +       int val;
> +
> +       if (!sctp_style(sk, TCP))
> +               return -EOPNOTSUPP;
> +
> +       if (sctp_sk(sk)->ep->base.bind_addr.port)
> +               return -EFAULT;
> +
> +       if (optlen < sizeof(int))
> +               return -EINVAL;
> +
> +       if (get_user(val, (int __user *)optval))
> +               return -EFAULT;
> +
> +       sk->sk_reuse = val ? SK_CAN_REUSE : SK_NO_REUSE;
> +
> +       return 0;
> +}
> +
>  /* API 6.2 setsockopt(), getsockopt()
>   *
>   * Applications use setsockopt() and getsockopt() to set or retrieve
> @@ -4364,6 +4386,9 @@ static int sctp_setsockopt(struct sock *sk, int level, int optname,
>                 retval = sctp_setsockopt_interleaving_supported(sk, optval,
>                                                                 optlen);
>                 break;
> +       case SCTP_REUSE_PORT:
> +               retval = sctp_setsockopt_reuse_port(sk, optval, optlen);
> +               break;
>         default:
>                 retval = -ENOPROTOOPT;
>                 break;
> @@ -7175,6 +7200,26 @@ static int sctp_getsockopt_interleaving_supported(struct sock *sk, int len,
>         return retval;
>  }
>
> +static int sctp_getsockopt_reuse_port(struct sock *sk, int len,
> +                                     char __user *optval,
> +                                     int __user *optlen)
> +{
> +       int val = 0;
> +
> +       if (len < sizeof(int))
> +               return -EINVAL;
> +
> +       len = sizeof(int);
> +       if (sk->sk_reuse != SK_NO_REUSE)
> +               val = 1;
> +       if (put_user(len, optlen))
> +               return -EFAULT;
> +       if (copy_to_user(optval, &val, len))
> +               return -EFAULT;
> +
> +       return 0;
> +}
> +
>  static int sctp_getsockopt(struct sock *sk, int level, int optname,
>                            char __user *optval, int __user *optlen)
>  {
> @@ -7370,6 +7415,9 @@ static int sctp_getsockopt(struct sock *sk, int level, int optname,
>                 retval = sctp_getsockopt_interleaving_supported(sk, len, optval,
>                                                                 optlen);
>                 break;
> +       case SCTP_REUSE_PORT:
> +               retval = sctp_getsockopt_reuse_port(sk, len, optval, optlen);
> +               break;
>         default:
>                 retval = -ENOPROTOOPT;
>                 break;
> --
> 2.1.0
>

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

* Re: [PATCH net-next] sctp: add support for SCTP_REUSE_PORT sockopt
  2018-05-19  7:44 ` Xin Long
@ 2018-05-21  0:50   ` Neil Horman
  -1 siblings, 0 replies; 33+ messages in thread
From: Neil Horman @ 2018-05-21  0:50 UTC (permalink / raw)
  To: Xin Long; +Cc: network dev, linux-sctp, Marcelo Ricardo Leitner, davem

On Sat, May 19, 2018 at 03:44:40PM +0800, Xin Long wrote:
> This feature is actually already supported by sk->sk_reuse which can be
> set by SO_REUSEADDR. But it's not working exactly as RFC6458 demands in
> section 8.1.27, like:
> 
>   - This option only supports one-to-one style SCTP sockets
>   - This socket option must not be used after calling bind()
>     or sctp_bindx().
> 
> Besides, SCTP_REUSE_PORT sockopt should be provided for user's programs.
> Otherwise, the programs with SCTP_REUSE_PORT from other systems will not
> work in linux.
> 
> This patch reuses sk->sk_reuse and works pretty much as SO_REUSEADDR,
> just with some extra setup limitations that are neeeded when it is being
> enabled.
> 
> "It should be noted that the behavior of the socket-level socket option
> to reuse ports and/or addresses for SCTP sockets is unspecified", so it
> leaves SO_REUSEADDR as is for the compatibility.
> 
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> ---
>  include/uapi/linux/sctp.h |  1 +
>  net/sctp/socket.c         | 48 +++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 49 insertions(+)
> 
A few things:

1) I agree with Tom, this feature is a complete duplication of the SK_REUSEPORT
socket option.  I understand that this is an implementation of the option in the
RFC, but its definately a duplication of a feature, which makes several things
really messy.

2) The overloading of the sk_reuse opeion is a bad idea, for several reasons.
Chief among them is the behavioral interference between this patch and the
SO_REUSEADDR socket level option, that also sets this feature.  If you set
sk_reuse via SO_REUSEADDR, you will set the SCTP port reuse feature regardless
of the bind or 1:1/1:m state of the socket.  Vice versa, if you set this socket
option via the SCTP_PORT_REUSE option you will inadvertently turn on address
reuse for the socket.  We can't do that.

Its a bit frustrating, since SO_REUSEPORT is widely available on multiple
operating systems, but isn't standard (AFAIK).  I would say however, given the
prevalence of the socket level option, we should likely advocate for the removal
of the sctp specific option, or at the least implement and document it as being
an alias for SO_REUSEPORT


As this stands however, its a NACK from me.

Neil

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

* Re: [PATCH net-next] sctp: add support for SCTP_REUSE_PORT sockopt
@ 2018-05-21  0:50   ` Neil Horman
  0 siblings, 0 replies; 33+ messages in thread
From: Neil Horman @ 2018-05-21  0:50 UTC (permalink / raw)
  To: Xin Long; +Cc: network dev, linux-sctp, Marcelo Ricardo Leitner, davem

On Sat, May 19, 2018 at 03:44:40PM +0800, Xin Long wrote:
> This feature is actually already supported by sk->sk_reuse which can be
> set by SO_REUSEADDR. But it's not working exactly as RFC6458 demands in
> section 8.1.27, like:
> 
>   - This option only supports one-to-one style SCTP sockets
>   - This socket option must not be used after calling bind()
>     or sctp_bindx().
> 
> Besides, SCTP_REUSE_PORT sockopt should be provided for user's programs.
> Otherwise, the programs with SCTP_REUSE_PORT from other systems will not
> work in linux.
> 
> This patch reuses sk->sk_reuse and works pretty much as SO_REUSEADDR,
> just with some extra setup limitations that are neeeded when it is being
> enabled.
> 
> "It should be noted that the behavior of the socket-level socket option
> to reuse ports and/or addresses for SCTP sockets is unspecified", so it
> leaves SO_REUSEADDR as is for the compatibility.
> 
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> ---
>  include/uapi/linux/sctp.h |  1 +
>  net/sctp/socket.c         | 48 +++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 49 insertions(+)
> 
A few things:

1) I agree with Tom, this feature is a complete duplication of the SK_REUSEPORT
socket option.  I understand that this is an implementation of the option in the
RFC, but its definately a duplication of a feature, which makes several things
really messy.

2) The overloading of the sk_reuse opeion is a bad idea, for several reasons.
Chief among them is the behavioral interference between this patch and the
SO_REUSEADDR socket level option, that also sets this feature.  If you set
sk_reuse via SO_REUSEADDR, you will set the SCTP port reuse feature regardless
of the bind or 1:1/1:m state of the socket.  Vice versa, if you set this socket
option via the SCTP_PORT_REUSE option you will inadvertently turn on address
reuse for the socket.  We can't do that.

Its a bit frustrating, since SO_REUSEPORT is widely available on multiple
operating systems, but isn't standard (AFAIK).  I would say however, given the
prevalence of the socket level option, we should likely advocate for the removal
of the sctp specific option, or at the least implement and document it as being
an alias for SO_REUSEPORT


As this stands however, its a NACK from me.

Neil


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

* Re: [PATCH net-next] sctp: add support for SCTP_REUSE_PORT sockopt
  2018-05-21  0:50   ` Neil Horman
@ 2018-05-21  1:54     ` Marcelo Ricardo Leitner
  -1 siblings, 0 replies; 33+ messages in thread
From: Marcelo Ricardo Leitner @ 2018-05-21  1:54 UTC (permalink / raw)
  To: Neil Horman; +Cc: Xin Long, network dev, linux-sctp, davem

On Sun, May 20, 2018 at 08:50:59PM -0400, Neil Horman wrote:
> On Sat, May 19, 2018 at 03:44:40PM +0800, Xin Long wrote:
> > This feature is actually already supported by sk->sk_reuse which can be
> > set by SO_REUSEADDR. But it's not working exactly as RFC6458 demands in
> > section 8.1.27, like:
> > 
> >   - This option only supports one-to-one style SCTP sockets
> >   - This socket option must not be used after calling bind()
> >     or sctp_bindx().
> > 
> > Besides, SCTP_REUSE_PORT sockopt should be provided for user's programs.
> > Otherwise, the programs with SCTP_REUSE_PORT from other systems will not
> > work in linux.
> > 
> > This patch reuses sk->sk_reuse and works pretty much as SO_REUSEADDR,
> > just with some extra setup limitations that are neeeded when it is being
> > enabled.
> > 
> > "It should be noted that the behavior of the socket-level socket option
> > to reuse ports and/or addresses for SCTP sockets is unspecified", so it
> > leaves SO_REUSEADDR as is for the compatibility.
> > 
> > Signed-off-by: Xin Long <lucien.xin@gmail.com>
> > ---
> >  include/uapi/linux/sctp.h |  1 +
> >  net/sctp/socket.c         | 48 +++++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 49 insertions(+)
> > 
> A few things:
> 
> 1) I agree with Tom, this feature is a complete duplication of the SK_REUSEPORT
> socket option.  I understand that this is an implementation of the option in the
> RFC, but its definately a duplication of a feature, which makes several things
> really messy.
> 
> 2) The overloading of the sk_reuse opeion is a bad idea, for several reasons.
> Chief among them is the behavioral interference between this patch and the
> SO_REUSEADDR socket level option, that also sets this feature.  If you set
> sk_reuse via SO_REUSEADDR, you will set the SCTP port reuse feature regardless
> of the bind or 1:1/1:m state of the socket.  Vice versa, if you set this socket
> option via the SCTP_PORT_REUSE option you will inadvertently turn on address
> reuse for the socket.  We can't do that.

Given your comments, going a bit further here, one other big
implication is that a port would never be able to be considered to
fully meet SCTP standards regarding reuse because a rogue application
may always abuse of the socket level opt to gain access to the port.

IOW, the patch allows the application to use such restrictions against
itself and nothing else, which undermines the patch idea.

I lack the knowledge on why the SCTP option was proposed in the RFC. I
guess they had a good reason to add the restriction on 1:1/1:m style.
Does the usage of the current imply in any risk to SCTP sockets? If
yes, that would give some grounds for going forward with the SCTP
option.

> 
> Its a bit frustrating, since SO_REUSEPORT is widely available on multiple
> operating systems, but isn't standard (AFAIK).  I would say however, given the
> prevalence of the socket level option, we should likely advocate for the removal
> of the sctp specific option, or at the least implement and document it as being

Is it possible, to remove/deprecate an option once it is published on a RFC?

> an alias for SO_REUSEPORT
> 
> 
> As this stands however, its a NACK from me.
> 
> Neil
> 

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

* Re: [PATCH net-next] sctp: add support for SCTP_REUSE_PORT sockopt
@ 2018-05-21  1:54     ` Marcelo Ricardo Leitner
  0 siblings, 0 replies; 33+ messages in thread
From: Marcelo Ricardo Leitner @ 2018-05-21  1:54 UTC (permalink / raw)
  To: Neil Horman; +Cc: Xin Long, network dev, linux-sctp, davem

On Sun, May 20, 2018 at 08:50:59PM -0400, Neil Horman wrote:
> On Sat, May 19, 2018 at 03:44:40PM +0800, Xin Long wrote:
> > This feature is actually already supported by sk->sk_reuse which can be
> > set by SO_REUSEADDR. But it's not working exactly as RFC6458 demands in
> > section 8.1.27, like:
> > 
> >   - This option only supports one-to-one style SCTP sockets
> >   - This socket option must not be used after calling bind()
> >     or sctp_bindx().
> > 
> > Besides, SCTP_REUSE_PORT sockopt should be provided for user's programs.
> > Otherwise, the programs with SCTP_REUSE_PORT from other systems will not
> > work in linux.
> > 
> > This patch reuses sk->sk_reuse and works pretty much as SO_REUSEADDR,
> > just with some extra setup limitations that are neeeded when it is being
> > enabled.
> > 
> > "It should be noted that the behavior of the socket-level socket option
> > to reuse ports and/or addresses for SCTP sockets is unspecified", so it
> > leaves SO_REUSEADDR as is for the compatibility.
> > 
> > Signed-off-by: Xin Long <lucien.xin@gmail.com>
> > ---
> >  include/uapi/linux/sctp.h |  1 +
> >  net/sctp/socket.c         | 48 +++++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 49 insertions(+)
> > 
> A few things:
> 
> 1) I agree with Tom, this feature is a complete duplication of the SK_REUSEPORT
> socket option.  I understand that this is an implementation of the option in the
> RFC, but its definately a duplication of a feature, which makes several things
> really messy.
> 
> 2) The overloading of the sk_reuse opeion is a bad idea, for several reasons.
> Chief among them is the behavioral interference between this patch and the
> SO_REUSEADDR socket level option, that also sets this feature.  If you set
> sk_reuse via SO_REUSEADDR, you will set the SCTP port reuse feature regardless
> of the bind or 1:1/1:m state of the socket.  Vice versa, if you set this socket
> option via the SCTP_PORT_REUSE option you will inadvertently turn on address
> reuse for the socket.  We can't do that.

Given your comments, going a bit further here, one other big
implication is that a port would never be able to be considered to
fully meet SCTP standards regarding reuse because a rogue application
may always abuse of the socket level opt to gain access to the port.

IOW, the patch allows the application to use such restrictions against
itself and nothing else, which undermines the patch idea.

I lack the knowledge on why the SCTP option was proposed in the RFC. I
guess they had a good reason to add the restriction on 1:1/1:m style.
Does the usage of the current imply in any risk to SCTP sockets? If
yes, that would give some grounds for going forward with the SCTP
option.

> 
> Its a bit frustrating, since SO_REUSEPORT is widely available on multiple
> operating systems, but isn't standard (AFAIK).  I would say however, given the
> prevalence of the socket level option, we should likely advocate for the removal
> of the sctp specific option, or at the least implement and document it as being

Is it possible, to remove/deprecate an option once it is published on a RFC?

> an alias for SO_REUSEPORT
> 
> 
> As this stands however, its a NACK from me.
> 
> Neil
> 

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

* Re: [PATCH net-next] sctp: add support for SCTP_REUSE_PORT sockopt
  2018-05-21  1:54     ` Marcelo Ricardo Leitner
@ 2018-05-21  3:59       ` Tom Herbert
  -1 siblings, 0 replies; 33+ messages in thread
From: Tom Herbert @ 2018-05-21  3:59 UTC (permalink / raw)
  To: Marcelo Ricardo Leitner
  Cc: Neil Horman, Xin Long, network dev,
	linux-sctp @ vger . kernel . org, David S. Miller

On Sun, May 20, 2018 at 6:54 PM, Marcelo Ricardo Leitner
<marcelo.leitner@gmail.com> wrote:
> On Sun, May 20, 2018 at 08:50:59PM -0400, Neil Horman wrote:
>> On Sat, May 19, 2018 at 03:44:40PM +0800, Xin Long wrote:
>> > This feature is actually already supported by sk->sk_reuse which can be
>> > set by SO_REUSEADDR. But it's not working exactly as RFC6458 demands in
>> > section 8.1.27, like:
>> >
>> >   - This option only supports one-to-one style SCTP sockets
>> >   - This socket option must not be used after calling bind()
>> >     or sctp_bindx().
>> >
>> > Besides, SCTP_REUSE_PORT sockopt should be provided for user's programs.
>> > Otherwise, the programs with SCTP_REUSE_PORT from other systems will not
>> > work in linux.
>> >
>> > This patch reuses sk->sk_reuse and works pretty much as SO_REUSEADDR,
>> > just with some extra setup limitations that are neeeded when it is being
>> > enabled.
>> >
>> > "It should be noted that the behavior of the socket-level socket option
>> > to reuse ports and/or addresses for SCTP sockets is unspecified", so it
>> > leaves SO_REUSEADDR as is for the compatibility.
>> >
>> > Signed-off-by: Xin Long <lucien.xin@gmail.com>
>> > ---
>> >  include/uapi/linux/sctp.h |  1 +
>> >  net/sctp/socket.c         | 48 +++++++++++++++++++++++++++++++++++++++++++++++
>> >  2 files changed, 49 insertions(+)
>> >
>> A few things:
>>
>> 1) I agree with Tom, this feature is a complete duplication of the SK_REUSEPORT
>> socket option.  I understand that this is an implementation of the option in the
>> RFC, but its definately a duplication of a feature, which makes several things
>> really messy.
>>
>> 2) The overloading of the sk_reuse opeion is a bad idea, for several reasons.
>> Chief among them is the behavioral interference between this patch and the
>> SO_REUSEADDR socket level option, that also sets this feature.  If you set
>> sk_reuse via SO_REUSEADDR, you will set the SCTP port reuse feature regardless
>> of the bind or 1:1/1:m state of the socket.  Vice versa, if you set this socket
>> option via the SCTP_PORT_REUSE option you will inadvertently turn on address
>> reuse for the socket.  We can't do that.
>
> Given your comments, going a bit further here, one other big
> implication is that a port would never be able to be considered to
> fully meet SCTP standards regarding reuse because a rogue application
> may always abuse of the socket level opt to gain access to the port.
>
There are mitigations in SO_REUSEPORT to prevent port hijacking. Don't
see why they can't be applied to SCTP.

Tom

> IOW, the patch allows the application to use such restrictions against
> itself and nothing else, which undermines the patch idea.
>
> I lack the knowledge on why the SCTP option was proposed in the RFC. I
> guess they had a good reason to add the restriction on 1:1/1:m style.
> Does the usage of the current imply in any risk to SCTP sockets? If
> yes, that would give some grounds for going forward with the SCTP
> option.
>
>>
>> Its a bit frustrating, since SO_REUSEPORT is widely available on multiple
>> operating systems, but isn't standard (AFAIK).  I would say however, given the
>> prevalence of the socket level option, we should likely advocate for the removal
>> of the sctp specific option, or at the least implement and document it as being
>
> Is it possible, to remove/deprecate an option once it is published on a RFC?
>
>> an alias for SO_REUSEPORT
>>
>>
>> As this stands however, its a NACK from me.
>>
>> Neil
>>

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

* Re: [PATCH net-next] sctp: add support for SCTP_REUSE_PORT sockopt
@ 2018-05-21  3:59       ` Tom Herbert
  0 siblings, 0 replies; 33+ messages in thread
From: Tom Herbert @ 2018-05-21  3:59 UTC (permalink / raw)
  To: Marcelo Ricardo Leitner
  Cc: Neil Horman, Xin Long, network dev,
	linux-sctp @ vger . kernel . org, David S. Miller

On Sun, May 20, 2018 at 6:54 PM, Marcelo Ricardo Leitner
<marcelo.leitner@gmail.com> wrote:
> On Sun, May 20, 2018 at 08:50:59PM -0400, Neil Horman wrote:
>> On Sat, May 19, 2018 at 03:44:40PM +0800, Xin Long wrote:
>> > This feature is actually already supported by sk->sk_reuse which can be
>> > set by SO_REUSEADDR. But it's not working exactly as RFC6458 demands in
>> > section 8.1.27, like:
>> >
>> >   - This option only supports one-to-one style SCTP sockets
>> >   - This socket option must not be used after calling bind()
>> >     or sctp_bindx().
>> >
>> > Besides, SCTP_REUSE_PORT sockopt should be provided for user's programs.
>> > Otherwise, the programs with SCTP_REUSE_PORT from other systems will not
>> > work in linux.
>> >
>> > This patch reuses sk->sk_reuse and works pretty much as SO_REUSEADDR,
>> > just with some extra setup limitations that are neeeded when it is being
>> > enabled.
>> >
>> > "It should be noted that the behavior of the socket-level socket option
>> > to reuse ports and/or addresses for SCTP sockets is unspecified", so it
>> > leaves SO_REUSEADDR as is for the compatibility.
>> >
>> > Signed-off-by: Xin Long <lucien.xin@gmail.com>
>> > ---
>> >  include/uapi/linux/sctp.h |  1 +
>> >  net/sctp/socket.c         | 48 +++++++++++++++++++++++++++++++++++++++++++++++
>> >  2 files changed, 49 insertions(+)
>> >
>> A few things:
>>
>> 1) I agree with Tom, this feature is a complete duplication of the SK_REUSEPORT
>> socket option.  I understand that this is an implementation of the option in the
>> RFC, but its definately a duplication of a feature, which makes several things
>> really messy.
>>
>> 2) The overloading of the sk_reuse opeion is a bad idea, for several reasons.
>> Chief among them is the behavioral interference between this patch and the
>> SO_REUSEADDR socket level option, that also sets this feature.  If you set
>> sk_reuse via SO_REUSEADDR, you will set the SCTP port reuse feature regardless
>> of the bind or 1:1/1:m state of the socket.  Vice versa, if you set this socket
>> option via the SCTP_PORT_REUSE option you will inadvertently turn on address
>> reuse for the socket.  We can't do that.
>
> Given your comments, going a bit further here, one other big
> implication is that a port would never be able to be considered to
> fully meet SCTP standards regarding reuse because a rogue application
> may always abuse of the socket level opt to gain access to the port.
>
There are mitigations in SO_REUSEPORT to prevent port hijacking. Don't
see why they can't be applied to SCTP.

Tom

> IOW, the patch allows the application to use such restrictions against
> itself and nothing else, which undermines the patch idea.
>
> I lack the knowledge on why the SCTP option was proposed in the RFC. I
> guess they had a good reason to add the restriction on 1:1/1:m style.
> Does the usage of the current imply in any risk to SCTP sockets? If
> yes, that would give some grounds for going forward with the SCTP
> option.
>
>>
>> Its a bit frustrating, since SO_REUSEPORT is widely available on multiple
>> operating systems, but isn't standard (AFAIK).  I would say however, given the
>> prevalence of the socket level option, we should likely advocate for the removal
>> of the sctp specific option, or at the least implement and document it as being
>
> Is it possible, to remove/deprecate an option once it is published on a RFC?
>
>> an alias for SO_REUSEPORT
>>
>>
>> As this stands however, its a NACK from me.
>>
>> Neil
>>

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

* Re: [PATCH net-next] sctp: add support for SCTP_REUSE_PORT sockopt
  2018-05-21  1:54     ` Marcelo Ricardo Leitner
@ 2018-05-21  7:46       ` Xin Long
  -1 siblings, 0 replies; 33+ messages in thread
From: Xin Long @ 2018-05-21  7:46 UTC (permalink / raw)
  To: Marcelo Ricardo Leitner, Michael Tuexen
  Cc: Neil Horman, network dev, linux-sctp, davem

On Mon, May 21, 2018 at 9:54 AM, Marcelo Ricardo Leitner
<marcelo.leitner@gmail.com> wrote:
> On Sun, May 20, 2018 at 08:50:59PM -0400, Neil Horman wrote:
>> On Sat, May 19, 2018 at 03:44:40PM +0800, Xin Long wrote:
>> > This feature is actually already supported by sk->sk_reuse which can be
>> > set by SO_REUSEADDR. But it's not working exactly as RFC6458 demands in
>> > section 8.1.27, like:
>> >
>> >   - This option only supports one-to-one style SCTP sockets
>> >   - This socket option must not be used after calling bind()
>> >     or sctp_bindx().
>> >
>> > Besides, SCTP_REUSE_PORT sockopt should be provided for user's programs.
>> > Otherwise, the programs with SCTP_REUSE_PORT from other systems will not
>> > work in linux.
>> >
>> > This patch reuses sk->sk_reuse and works pretty much as SO_REUSEADDR,
>> > just with some extra setup limitations that are neeeded when it is being
>> > enabled.
>> >
>> > "It should be noted that the behavior of the socket-level socket option
>> > to reuse ports and/or addresses for SCTP sockets is unspecified", so it
>> > leaves SO_REUSEADDR as is for the compatibility.
>> >
>> > Signed-off-by: Xin Long <lucien.xin@gmail.com>
>> > ---
>> >  include/uapi/linux/sctp.h |  1 +
>> >  net/sctp/socket.c         | 48 +++++++++++++++++++++++++++++++++++++++++++++++
>> >  2 files changed, 49 insertions(+)
>> >
>> A few things:
>>
>> 1) I agree with Tom, this feature is a complete duplication of the SK_REUSEPORT
>> socket option.  I understand that this is an implementation of the option in the
>> RFC, but its definately a duplication of a feature, which makes several things
>> really messy.
>>
>> 2) The overloading of the sk_reuse opeion is a bad idea, for several reasons.
>> Chief among them is the behavioral interference between this patch and the
>> SO_REUSEADDR socket level option, that also sets this feature.  If you set
>> sk_reuse via SO_REUSEADDR, you will set the SCTP port reuse feature regardless
>> of the bind or 1:1/1:m state of the socket.  Vice versa, if you set this socket
>> option via the SCTP_PORT_REUSE option you will inadvertently turn on address
>> reuse for the socket.  We can't do that.
>
> Given your comments, going a bit further here, one other big
> implication is that a port would never be able to be considered to
> fully meet SCTP standards regarding reuse because a rogue application
> may always abuse of the socket level opt to gain access to the port.
>
> IOW, the patch allows the application to use such restrictions against
> itself and nothing else, which undermines the patch idea.
>
> I lack the knowledge on why the SCTP option was proposed in the RFC. I
> guess they had a good reason to add the restriction on 1:1/1:m style.
> Does the usage of the current imply in any risk to SCTP sockets? If
> yes, that would give some grounds for going forward with the SCTP
> option.
It should go with SCTP option unless RFC6458 section 8.1.27 can really
be removed. Otherwise, sctp users will think linux sctp doesn't support
this feature.

Another thing is, when going with SCTP_PORT_REUSE, in this patch I didn't
add a new reuse_port member for sctp_sock, which causes the abuse of the
socket level opt to gain access to the port as you said. But if I do and ignore
the old sk->sk_reuse, there may be a compatibility problem with before.

>
>>
>> Its a bit frustrating, since SO_REUSEPORT is widely available on multiple
>> operating systems, but isn't standard (AFAIK).  I would say however, given the
>> prevalence of the socket level option, we should likely advocate for the removal
>> of the sctp specific option, or at the least implement and document it as being
>
> Is it possible, to remove/deprecate an option once it is published on a RFC?

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

* Re: [PATCH net-next] sctp: add support for SCTP_REUSE_PORT sockopt
@ 2018-05-21  7:46       ` Xin Long
  0 siblings, 0 replies; 33+ messages in thread
From: Xin Long @ 2018-05-21  7:46 UTC (permalink / raw)
  To: Marcelo Ricardo Leitner, Michael Tuexen
  Cc: Neil Horman, network dev, linux-sctp, davem

On Mon, May 21, 2018 at 9:54 AM, Marcelo Ricardo Leitner
<marcelo.leitner@gmail.com> wrote:
> On Sun, May 20, 2018 at 08:50:59PM -0400, Neil Horman wrote:
>> On Sat, May 19, 2018 at 03:44:40PM +0800, Xin Long wrote:
>> > This feature is actually already supported by sk->sk_reuse which can be
>> > set by SO_REUSEADDR. But it's not working exactly as RFC6458 demands in
>> > section 8.1.27, like:
>> >
>> >   - This option only supports one-to-one style SCTP sockets
>> >   - This socket option must not be used after calling bind()
>> >     or sctp_bindx().
>> >
>> > Besides, SCTP_REUSE_PORT sockopt should be provided for user's programs.
>> > Otherwise, the programs with SCTP_REUSE_PORT from other systems will not
>> > work in linux.
>> >
>> > This patch reuses sk->sk_reuse and works pretty much as SO_REUSEADDR,
>> > just with some extra setup limitations that are neeeded when it is being
>> > enabled.
>> >
>> > "It should be noted that the behavior of the socket-level socket option
>> > to reuse ports and/or addresses for SCTP sockets is unspecified", so it
>> > leaves SO_REUSEADDR as is for the compatibility.
>> >
>> > Signed-off-by: Xin Long <lucien.xin@gmail.com>
>> > ---
>> >  include/uapi/linux/sctp.h |  1 +
>> >  net/sctp/socket.c         | 48 +++++++++++++++++++++++++++++++++++++++++++++++
>> >  2 files changed, 49 insertions(+)
>> >
>> A few things:
>>
>> 1) I agree with Tom, this feature is a complete duplication of the SK_REUSEPORT
>> socket option.  I understand that this is an implementation of the option in the
>> RFC, but its definately a duplication of a feature, which makes several things
>> really messy.
>>
>> 2) The overloading of the sk_reuse opeion is a bad idea, for several reasons.
>> Chief among them is the behavioral interference between this patch and the
>> SO_REUSEADDR socket level option, that also sets this feature.  If you set
>> sk_reuse via SO_REUSEADDR, you will set the SCTP port reuse feature regardless
>> of the bind or 1:1/1:m state of the socket.  Vice versa, if you set this socket
>> option via the SCTP_PORT_REUSE option you will inadvertently turn on address
>> reuse for the socket.  We can't do that.
>
> Given your comments, going a bit further here, one other big
> implication is that a port would never be able to be considered to
> fully meet SCTP standards regarding reuse because a rogue application
> may always abuse of the socket level opt to gain access to the port.
>
> IOW, the patch allows the application to use such restrictions against
> itself and nothing else, which undermines the patch idea.
>
> I lack the knowledge on why the SCTP option was proposed in the RFC. I
> guess they had a good reason to add the restriction on 1:1/1:m style.
> Does the usage of the current imply in any risk to SCTP sockets? If
> yes, that would give some grounds for going forward with the SCTP
> option.
It should go with SCTP option unless RFC6458 section 8.1.27 can really
be removed. Otherwise, sctp users will think linux sctp doesn't support
this feature.

Another thing is, when going with SCTP_PORT_REUSE, in this patch I didn't
add a new reuse_port member for sctp_sock, which causes the abuse of the
socket level opt to gain access to the port as you said. But if I do and ignore
the old sk->sk_reuse, there may be a compatibility problem with before.

>
>>
>> Its a bit frustrating, since SO_REUSEPORT is widely available on multiple
>> operating systems, but isn't standard (AFAIK).  I would say however, given the
>> prevalence of the socket level option, we should likely advocate for the removal
>> of the sctp specific option, or at the least implement and document it as being
>
> Is it possible, to remove/deprecate an option once it is published on a RFC?

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

* Re: [PATCH net-next] sctp: add support for SCTP_REUSE_PORT sockopt
  2018-05-21  1:54     ` Marcelo Ricardo Leitner
@ 2018-05-21 11:39       ` Neil Horman
  -1 siblings, 0 replies; 33+ messages in thread
From: Neil Horman @ 2018-05-21 11:39 UTC (permalink / raw)
  To: Marcelo Ricardo Leitner; +Cc: Xin Long, network dev, linux-sctp, davem, tuexen

On Sun, May 20, 2018 at 10:54:04PM -0300, Marcelo Ricardo Leitner wrote:
> On Sun, May 20, 2018 at 08:50:59PM -0400, Neil Horman wrote:
> > On Sat, May 19, 2018 at 03:44:40PM +0800, Xin Long wrote:
> > > This feature is actually already supported by sk->sk_reuse which can be
> > > set by SO_REUSEADDR. But it's not working exactly as RFC6458 demands in
> > > section 8.1.27, like:
> > > 
> > >   - This option only supports one-to-one style SCTP sockets
> > >   - This socket option must not be used after calling bind()
> > >     or sctp_bindx().
> > > 
> > > Besides, SCTP_REUSE_PORT sockopt should be provided for user's programs.
> > > Otherwise, the programs with SCTP_REUSE_PORT from other systems will not
> > > work in linux.
> > > 
> > > This patch reuses sk->sk_reuse and works pretty much as SO_REUSEADDR,
> > > just with some extra setup limitations that are neeeded when it is being
> > > enabled.
> > > 
> > > "It should be noted that the behavior of the socket-level socket option
> > > to reuse ports and/or addresses for SCTP sockets is unspecified", so it
> > > leaves SO_REUSEADDR as is for the compatibility.
> > > 
> > > Signed-off-by: Xin Long <lucien.xin@gmail.com>
> > > ---
> > >  include/uapi/linux/sctp.h |  1 +
> > >  net/sctp/socket.c         | 48 +++++++++++++++++++++++++++++++++++++++++++++++
> > >  2 files changed, 49 insertions(+)
> > > 
> > A few things:
> > 
> > 1) I agree with Tom, this feature is a complete duplication of the SK_REUSEPORT
> > socket option.  I understand that this is an implementation of the option in the
> > RFC, but its definately a duplication of a feature, which makes several things
> > really messy.
> > 
> > 2) The overloading of the sk_reuse opeion is a bad idea, for several reasons.
> > Chief among them is the behavioral interference between this patch and the
> > SO_REUSEADDR socket level option, that also sets this feature.  If you set
> > sk_reuse via SO_REUSEADDR, you will set the SCTP port reuse feature regardless
> > of the bind or 1:1/1:m state of the socket.  Vice versa, if you set this socket
> > option via the SCTP_PORT_REUSE option you will inadvertently turn on address
> > reuse for the socket.  We can't do that.
> 
> Given your comments, going a bit further here, one other big
> implication is that a port would never be able to be considered to
> fully meet SCTP standards regarding reuse because a rogue application
> may always abuse of the socket level opt to gain access to the port.
> 
> IOW, the patch allows the application to use such restrictions against
> itself and nothing else, which undermines the patch idea.
> 
Agreed.

> I lack the knowledge on why the SCTP option was proposed in the RFC. I
> guess they had a good reason to add the restriction on 1:1/1:m style.
> Does the usage of the current imply in any risk to SCTP sockets? If
> yes, that would give some grounds for going forward with the SCTP
> option.
> 
I'm also not privy to why the sctp option was proposed, though I expect that the
lack of standardization of SO_REUSEPORT probably had something to do with it.
As for the reasoning behind restriction to only 1:1 sockets, if I had to guess,
I would say it likely because it creates ordering difficulty at the application
level.

CC-ing Michael Tuxen, who I believe had some input on this RFC.  Hopefully he
can shed some light on this.

Neil

> > 
> > Its a bit frustrating, since SO_REUSEPORT is widely available on multiple
> > operating systems, but isn't standard (AFAIK).  I would say however, given the
> > prevalence of the socket level option, we should likely advocate for the removal
> > of the sctp specific option, or at the least implement and document it as being
> 
> Is it possible, to remove/deprecate an option once it is published on a RFC?
> 
> > an alias for SO_REUSEPORT
> > 
> > 
> > As this stands however, its a NACK from me.
> > 
> > Neil
> > 
> 

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

* Re: [PATCH net-next] sctp: add support for SCTP_REUSE_PORT sockopt
@ 2018-05-21 11:39       ` Neil Horman
  0 siblings, 0 replies; 33+ messages in thread
From: Neil Horman @ 2018-05-21 11:39 UTC (permalink / raw)
  To: Marcelo Ricardo Leitner; +Cc: Xin Long, network dev, linux-sctp, davem, tuexen

On Sun, May 20, 2018 at 10:54:04PM -0300, Marcelo Ricardo Leitner wrote:
> On Sun, May 20, 2018 at 08:50:59PM -0400, Neil Horman wrote:
> > On Sat, May 19, 2018 at 03:44:40PM +0800, Xin Long wrote:
> > > This feature is actually already supported by sk->sk_reuse which can be
> > > set by SO_REUSEADDR. But it's not working exactly as RFC6458 demands in
> > > section 8.1.27, like:
> > > 
> > >   - This option only supports one-to-one style SCTP sockets
> > >   - This socket option must not be used after calling bind()
> > >     or sctp_bindx().
> > > 
> > > Besides, SCTP_REUSE_PORT sockopt should be provided for user's programs.
> > > Otherwise, the programs with SCTP_REUSE_PORT from other systems will not
> > > work in linux.
> > > 
> > > This patch reuses sk->sk_reuse and works pretty much as SO_REUSEADDR,
> > > just with some extra setup limitations that are neeeded when it is being
> > > enabled.
> > > 
> > > "It should be noted that the behavior of the socket-level socket option
> > > to reuse ports and/or addresses for SCTP sockets is unspecified", so it
> > > leaves SO_REUSEADDR as is for the compatibility.
> > > 
> > > Signed-off-by: Xin Long <lucien.xin@gmail.com>
> > > ---
> > >  include/uapi/linux/sctp.h |  1 +
> > >  net/sctp/socket.c         | 48 +++++++++++++++++++++++++++++++++++++++++++++++
> > >  2 files changed, 49 insertions(+)
> > > 
> > A few things:
> > 
> > 1) I agree with Tom, this feature is a complete duplication of the SK_REUSEPORT
> > socket option.  I understand that this is an implementation of the option in the
> > RFC, but its definately a duplication of a feature, which makes several things
> > really messy.
> > 
> > 2) The overloading of the sk_reuse opeion is a bad idea, for several reasons.
> > Chief among them is the behavioral interference between this patch and the
> > SO_REUSEADDR socket level option, that also sets this feature.  If you set
> > sk_reuse via SO_REUSEADDR, you will set the SCTP port reuse feature regardless
> > of the bind or 1:1/1:m state of the socket.  Vice versa, if you set this socket
> > option via the SCTP_PORT_REUSE option you will inadvertently turn on address
> > reuse for the socket.  We can't do that.
> 
> Given your comments, going a bit further here, one other big
> implication is that a port would never be able to be considered to
> fully meet SCTP standards regarding reuse because a rogue application
> may always abuse of the socket level opt to gain access to the port.
> 
> IOW, the patch allows the application to use such restrictions against
> itself and nothing else, which undermines the patch idea.
> 
Agreed.

> I lack the knowledge on why the SCTP option was proposed in the RFC. I
> guess they had a good reason to add the restriction on 1:1/1:m style.
> Does the usage of the current imply in any risk to SCTP sockets? If
> yes, that would give some grounds for going forward with the SCTP
> option.
> 
I'm also not privy to why the sctp option was proposed, though I expect that the
lack of standardization of SO_REUSEPORT probably had something to do with it.
As for the reasoning behind restriction to only 1:1 sockets, if I had to guess,
I would say it likely because it creates ordering difficulty at the application
level.

CC-ing Michael Tuxen, who I believe had some input on this RFC.  Hopefully he
can shed some light on this.

Neil

> > 
> > Its a bit frustrating, since SO_REUSEPORT is widely available on multiple
> > operating systems, but isn't standard (AFAIK).  I would say however, given the
> > prevalence of the socket level option, we should likely advocate for the removal
> > of the sctp specific option, or at the least implement and document it as being
> 
> Is it possible, to remove/deprecate an option once it is published on a RFC?
> 
> > an alias for SO_REUSEPORT
> > 
> > 
> > As this stands however, its a NACK from me.
> > 
> > Neil
> > 
> 

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

* Re: [PATCH net-next] sctp: add support for SCTP_REUSE_PORT sockopt
  2018-05-21 11:39       ` Neil Horman
@ 2018-05-21 12:16         ` Michael Tuexen
  -1 siblings, 0 replies; 33+ messages in thread
From: Michael Tuexen @ 2018-05-21 12:16 UTC (permalink / raw)
  To: Neil Horman
  Cc: Marcelo Ricardo Leitner, Xin Long, network dev, linux-sctp, davem

[-- Attachment #1: Type: text/plain, Size: 4927 bytes --]

> On 21. May 2018, at 13:39, Neil Horman <nhorman@tuxdriver.com> wrote:
> 
> On Sun, May 20, 2018 at 10:54:04PM -0300, Marcelo Ricardo Leitner wrote:
>> On Sun, May 20, 2018 at 08:50:59PM -0400, Neil Horman wrote:
>>> On Sat, May 19, 2018 at 03:44:40PM +0800, Xin Long wrote:
>>>> This feature is actually already supported by sk->sk_reuse which can be
>>>> set by SO_REUSEADDR. But it's not working exactly as RFC6458 demands in
>>>> section 8.1.27, like:
>>>> 
>>>>  - This option only supports one-to-one style SCTP sockets
>>>>  - This socket option must not be used after calling bind()
>>>>    or sctp_bindx().
>>>> 
>>>> Besides, SCTP_REUSE_PORT sockopt should be provided for user's programs.
>>>> Otherwise, the programs with SCTP_REUSE_PORT from other systems will not
>>>> work in linux.
>>>> 
>>>> This patch reuses sk->sk_reuse and works pretty much as SO_REUSEADDR,
>>>> just with some extra setup limitations that are neeeded when it is being
>>>> enabled.
>>>> 
>>>> "It should be noted that the behavior of the socket-level socket option
>>>> to reuse ports and/or addresses for SCTP sockets is unspecified", so it
>>>> leaves SO_REUSEADDR as is for the compatibility.
>>>> 
>>>> Signed-off-by: Xin Long <lucien.xin@gmail.com>
>>>> ---
>>>> include/uapi/linux/sctp.h |  1 +
>>>> net/sctp/socket.c         | 48 +++++++++++++++++++++++++++++++++++++++++++++++
>>>> 2 files changed, 49 insertions(+)
>>>> 
>>> A few things:
>>> 
>>> 1) I agree with Tom, this feature is a complete duplication of the SK_REUSEPORT
>>> socket option.  I understand that this is an implementation of the option in the
>>> RFC, but its definately a duplication of a feature, which makes several things
>>> really messy.
>>> 
>>> 2) The overloading of the sk_reuse opeion is a bad idea, for several reasons.
>>> Chief among them is the behavioral interference between this patch and the
>>> SO_REUSEADDR socket level option, that also sets this feature.  If you set
>>> sk_reuse via SO_REUSEADDR, you will set the SCTP port reuse feature regardless
>>> of the bind or 1:1/1:m state of the socket.  Vice versa, if you set this socket
>>> option via the SCTP_PORT_REUSE option you will inadvertently turn on address
>>> reuse for the socket.  We can't do that.
>> 
>> Given your comments, going a bit further here, one other big
>> implication is that a port would never be able to be considered to
>> fully meet SCTP standards regarding reuse because a rogue application
>> may always abuse of the socket level opt to gain access to the port.
>> 
>> IOW, the patch allows the application to use such restrictions against
>> itself and nothing else, which undermines the patch idea.
>> 
> Agreed.
> 
>> I lack the knowledge on why the SCTP option was proposed in the RFC. I
>> guess they had a good reason to add the restriction on 1:1/1:m style.
>> Does the usage of the current imply in any risk to SCTP sockets? If
>> yes, that would give some grounds for going forward with the SCTP
>> option.
>> 
> I'm also not privy to why the sctp option was proposed, though I expect that the
> lack of standardization of SO_REUSEPORT probably had something to do with it.
> As for the reasoning behind restriction to only 1:1 sockets, if I had to guess,
> I would say it likely because it creates ordering difficulty at the application
> level.
> 
> CC-ing Michael Tuxen, who I believe had some input on this RFC.  Hopefully he
> can shed some light on this.
Dear all,

the reason this was added is to have a specified way to allow a system to
behave like a client and server making use of the INIT collision.

For 1-to-many style sockets you can do this by creating a socket, binding it,
calling listen on it and trying to connect to the peer.

For 1-to-1 style sockets you need two sockets for it. One listener and one
you use to connect (and close it in case of failure, open a new one...).

It was not clear if one can achieve this with SO_REUSEPORT and/or SO_REUSEADDR
on all platforms. We left that unspecified.

I hope this makes the intention clearer.

Best regards
Michael
> 
> Neil
> 
>>> 
>>> Its a bit frustrating, since SO_REUSEPORT is widely available on multiple
>>> operating systems, but isn't standard (AFAIK).  I would say however, given the
>>> prevalence of the socket level option, we should likely advocate for the removal
>>> of the sctp specific option, or at the least implement and document it as being
>> 
>> Is it possible, to remove/deprecate an option once it is published on a RFC?
>> 
>>> an alias for SO_REUSEPORT
>>> 
>>> 
>>> As this stands however, its a NACK from me.
>>> 
>>> Neil
>>> 
>> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5367 bytes --]

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

* Re: [PATCH net-next] sctp: add support for SCTP_REUSE_PORT sockopt
@ 2018-05-21 12:16         ` Michael Tuexen
  0 siblings, 0 replies; 33+ messages in thread
From: Michael Tuexen @ 2018-05-21 12:16 UTC (permalink / raw)
  To: Neil Horman
  Cc: Marcelo Ricardo Leitner, Xin Long, network dev, linux-sctp, davem

[-- Attachment #1: Type: text/plain, Size: 4927 bytes --]

> On 21. May 2018, at 13:39, Neil Horman <nhorman@tuxdriver.com> wrote:
> 
> On Sun, May 20, 2018 at 10:54:04PM -0300, Marcelo Ricardo Leitner wrote:
>> On Sun, May 20, 2018 at 08:50:59PM -0400, Neil Horman wrote:
>>> On Sat, May 19, 2018 at 03:44:40PM +0800, Xin Long wrote:
>>>> This feature is actually already supported by sk->sk_reuse which can be
>>>> set by SO_REUSEADDR. But it's not working exactly as RFC6458 demands in
>>>> section 8.1.27, like:
>>>> 
>>>>  - This option only supports one-to-one style SCTP sockets
>>>>  - This socket option must not be used after calling bind()
>>>>    or sctp_bindx().
>>>> 
>>>> Besides, SCTP_REUSE_PORT sockopt should be provided for user's programs.
>>>> Otherwise, the programs with SCTP_REUSE_PORT from other systems will not
>>>> work in linux.
>>>> 
>>>> This patch reuses sk->sk_reuse and works pretty much as SO_REUSEADDR,
>>>> just with some extra setup limitations that are neeeded when it is being
>>>> enabled.
>>>> 
>>>> "It should be noted that the behavior of the socket-level socket option
>>>> to reuse ports and/or addresses for SCTP sockets is unspecified", so it
>>>> leaves SO_REUSEADDR as is for the compatibility.
>>>> 
>>>> Signed-off-by: Xin Long <lucien.xin@gmail.com>
>>>> ---
>>>> include/uapi/linux/sctp.h |  1 +
>>>> net/sctp/socket.c         | 48 +++++++++++++++++++++++++++++++++++++++++++++++
>>>> 2 files changed, 49 insertions(+)
>>>> 
>>> A few things:
>>> 
>>> 1) I agree with Tom, this feature is a complete duplication of the SK_REUSEPORT
>>> socket option.  I understand that this is an implementation of the option in the
>>> RFC, but its definately a duplication of a feature, which makes several things
>>> really messy.
>>> 
>>> 2) The overloading of the sk_reuse opeion is a bad idea, for several reasons.
>>> Chief among them is the behavioral interference between this patch and the
>>> SO_REUSEADDR socket level option, that also sets this feature.  If you set
>>> sk_reuse via SO_REUSEADDR, you will set the SCTP port reuse feature regardless
>>> of the bind or 1:1/1:m state of the socket.  Vice versa, if you set this socket
>>> option via the SCTP_PORT_REUSE option you will inadvertently turn on address
>>> reuse for the socket.  We can't do that.
>> 
>> Given your comments, going a bit further here, one other big
>> implication is that a port would never be able to be considered to
>> fully meet SCTP standards regarding reuse because a rogue application
>> may always abuse of the socket level opt to gain access to the port.
>> 
>> IOW, the patch allows the application to use such restrictions against
>> itself and nothing else, which undermines the patch idea.
>> 
> Agreed.
> 
>> I lack the knowledge on why the SCTP option was proposed in the RFC. I
>> guess they had a good reason to add the restriction on 1:1/1:m style.
>> Does the usage of the current imply in any risk to SCTP sockets? If
>> yes, that would give some grounds for going forward with the SCTP
>> option.
>> 
> I'm also not privy to why the sctp option was proposed, though I expect that the
> lack of standardization of SO_REUSEPORT probably had something to do with it.
> As for the reasoning behind restriction to only 1:1 sockets, if I had to guess,
> I would say it likely because it creates ordering difficulty at the application
> level.
> 
> CC-ing Michael Tuxen, who I believe had some input on this RFC.  Hopefully he
> can shed some light on this.
Dear all,

the reason this was added is to have a specified way to allow a system to
behave like a client and server making use of the INIT collision.

For 1-to-many style sockets you can do this by creating a socket, binding it,
calling listen on it and trying to connect to the peer.

For 1-to-1 style sockets you need two sockets for it. One listener and one
you use to connect (and close it in case of failure, open a new one...).

It was not clear if one can achieve this with SO_REUSEPORT and/or SO_REUSEADDR
on all platforms. We left that unspecified.

I hope this makes the intention clearer.

Best regards
Michael
> 
> Neil
> 
>>> 
>>> Its a bit frustrating, since SO_REUSEPORT is widely available on multiple
>>> operating systems, but isn't standard (AFAIK).  I would say however, given the
>>> prevalence of the socket level option, we should likely advocate for the removal
>>> of the sctp specific option, or at the least implement and document it as being
>> 
>> Is it possible, to remove/deprecate an option once it is published on a RFC?
>> 
>>> an alias for SO_REUSEPORT
>>> 
>>> 
>>> As this stands however, its a NACK from me.
>>> 
>>> Neil
>>> 
>> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5367 bytes --]

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

* Re: [PATCH net-next] sctp: add support for SCTP_REUSE_PORT sockopt
  2018-05-21 12:16         ` Michael Tuexen
@ 2018-05-21 13:48           ` Neil Horman
  -1 siblings, 0 replies; 33+ messages in thread
From: Neil Horman @ 2018-05-21 13:48 UTC (permalink / raw)
  To: Michael Tuexen
  Cc: Marcelo Ricardo Leitner, Xin Long, network dev, linux-sctp, davem

On Mon, May 21, 2018 at 02:16:56PM +0200, Michael Tuexen wrote:
> > On 21. May 2018, at 13:39, Neil Horman <nhorman@tuxdriver.com> wrote:
> > 
> > On Sun, May 20, 2018 at 10:54:04PM -0300, Marcelo Ricardo Leitner wrote:
> >> On Sun, May 20, 2018 at 08:50:59PM -0400, Neil Horman wrote:
> >>> On Sat, May 19, 2018 at 03:44:40PM +0800, Xin Long wrote:
> >>>> This feature is actually already supported by sk->sk_reuse which can be
> >>>> set by SO_REUSEADDR. But it's not working exactly as RFC6458 demands in
> >>>> section 8.1.27, like:
> >>>> 
> >>>>  - This option only supports one-to-one style SCTP sockets
> >>>>  - This socket option must not be used after calling bind()
> >>>>    or sctp_bindx().
> >>>> 
> >>>> Besides, SCTP_REUSE_PORT sockopt should be provided for user's programs.
> >>>> Otherwise, the programs with SCTP_REUSE_PORT from other systems will not
> >>>> work in linux.
> >>>> 
> >>>> This patch reuses sk->sk_reuse and works pretty much as SO_REUSEADDR,
> >>>> just with some extra setup limitations that are neeeded when it is being
> >>>> enabled.
> >>>> 
> >>>> "It should be noted that the behavior of the socket-level socket option
> >>>> to reuse ports and/or addresses for SCTP sockets is unspecified", so it
> >>>> leaves SO_REUSEADDR as is for the compatibility.
> >>>> 
> >>>> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> >>>> ---
> >>>> include/uapi/linux/sctp.h |  1 +
> >>>> net/sctp/socket.c         | 48 +++++++++++++++++++++++++++++++++++++++++++++++
> >>>> 2 files changed, 49 insertions(+)
> >>>> 
> >>> A few things:
> >>> 
> >>> 1) I agree with Tom, this feature is a complete duplication of the SK_REUSEPORT
> >>> socket option.  I understand that this is an implementation of the option in the
> >>> RFC, but its definately a duplication of a feature, which makes several things
> >>> really messy.
> >>> 
> >>> 2) The overloading of the sk_reuse opeion is a bad idea, for several reasons.
> >>> Chief among them is the behavioral interference between this patch and the
> >>> SO_REUSEADDR socket level option, that also sets this feature.  If you set
> >>> sk_reuse via SO_REUSEADDR, you will set the SCTP port reuse feature regardless
> >>> of the bind or 1:1/1:m state of the socket.  Vice versa, if you set this socket
> >>> option via the SCTP_PORT_REUSE option you will inadvertently turn on address
> >>> reuse for the socket.  We can't do that.
> >> 
> >> Given your comments, going a bit further here, one other big
> >> implication is that a port would never be able to be considered to
> >> fully meet SCTP standards regarding reuse because a rogue application
> >> may always abuse of the socket level opt to gain access to the port.
> >> 
> >> IOW, the patch allows the application to use such restrictions against
> >> itself and nothing else, which undermines the patch idea.
> >> 
> > Agreed.
> > 
> >> I lack the knowledge on why the SCTP option was proposed in the RFC. I
> >> guess they had a good reason to add the restriction on 1:1/1:m style.
> >> Does the usage of the current imply in any risk to SCTP sockets? If
> >> yes, that would give some grounds for going forward with the SCTP
> >> option.
> >> 
> > I'm also not privy to why the sctp option was proposed, though I expect that the
> > lack of standardization of SO_REUSEPORT probably had something to do with it.
> > As for the reasoning behind restriction to only 1:1 sockets, if I had to guess,
> > I would say it likely because it creates ordering difficulty at the application
> > level.
> > 
> > CC-ing Michael Tuxen, who I believe had some input on this RFC.  Hopefully he
> > can shed some light on this.
> Dear all,
> 
> the reason this was added is to have a specified way to allow a system to
> behave like a client and server making use of the INIT collision.
> 
> For 1-to-many style sockets you can do this by creating a socket, binding it,
> calling listen on it and trying to connect to the peer.
> 
> For 1-to-1 style sockets you need two sockets for it. One listener and one
> you use to connect (and close it in case of failure, open a new one...).
> 
> It was not clear if one can achieve this with SO_REUSEPORT and/or SO_REUSEADDR
> on all platforms. We left that unspecified.
> 
> I hope this makes the intention clearer.
> 
I think it makes the intention clearer yes, but it unfortunately does nothing in
my mind to clarify how the implementation should best handle the potential
overlap in functionality.  What I see here is that we have two functional paths
(the SO_REUSEPORT path and the SCTP_PORT_REUSE path), which may or may not
(depending on the OS implementation achieve the same functional goal (allowing
multiple sockets to share a port while allowing one socket to listen and the
other connect to a remote peer).  If both implementations do the same thing on a
given platform, we can either just alias one to another and be done, but if they
don't then we either have to implement both paths, and ensure that the
SO_REUSEPORT path is a no-op/error return for SCTP sockets, or that each path
implements a distinct feature set that is cleaarly documented.

That said, I think we may be in luck.  Looking at the connect and listen paths,
it appears to me that:

1) Sockets ignore SO_REUSEPORT in the connect and listen paths (save for any
autobinding) so it would appear that the intent of the SCTP rfc can be honored
via SO_REUSEPORT on linux.  

2) SO_REUSEPORT prevents changing state after a bind has occured, so we can honr
that part of the SCTP RFC.

The only missing part is the restriction that SCTP_REUSE_PORT has which is
unaccounted for is that 1:M sockets aren't allowed to enable port reuse.
However, I think the implication from Michaels description above is that port
reuse on a 1:M socket is implicit because a single socket can connect and listen
in that use case, rather than there being a danger to doing so.

As such, I would propose that we implement this socket option by simply setting
the sk->sk_reuseport field in the sock structure, and document the fact that
linux does not restrict port reuse from 1:M sockets.

Thoughts?
Neil

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

* Re: [PATCH net-next] sctp: add support for SCTP_REUSE_PORT sockopt
@ 2018-05-21 13:48           ` Neil Horman
  0 siblings, 0 replies; 33+ messages in thread
From: Neil Horman @ 2018-05-21 13:48 UTC (permalink / raw)
  To: Michael Tuexen
  Cc: Marcelo Ricardo Leitner, Xin Long, network dev, linux-sctp, davem

On Mon, May 21, 2018 at 02:16:56PM +0200, Michael Tuexen wrote:
> > On 21. May 2018, at 13:39, Neil Horman <nhorman@tuxdriver.com> wrote:
> > 
> > On Sun, May 20, 2018 at 10:54:04PM -0300, Marcelo Ricardo Leitner wrote:
> >> On Sun, May 20, 2018 at 08:50:59PM -0400, Neil Horman wrote:
> >>> On Sat, May 19, 2018 at 03:44:40PM +0800, Xin Long wrote:
> >>>> This feature is actually already supported by sk->sk_reuse which can be
> >>>> set by SO_REUSEADDR. But it's not working exactly as RFC6458 demands in
> >>>> section 8.1.27, like:
> >>>> 
> >>>>  - This option only supports one-to-one style SCTP sockets
> >>>>  - This socket option must not be used after calling bind()
> >>>>    or sctp_bindx().
> >>>> 
> >>>> Besides, SCTP_REUSE_PORT sockopt should be provided for user's programs.
> >>>> Otherwise, the programs with SCTP_REUSE_PORT from other systems will not
> >>>> work in linux.
> >>>> 
> >>>> This patch reuses sk->sk_reuse and works pretty much as SO_REUSEADDR,
> >>>> just with some extra setup limitations that are neeeded when it is being
> >>>> enabled.
> >>>> 
> >>>> "It should be noted that the behavior of the socket-level socket option
> >>>> to reuse ports and/or addresses for SCTP sockets is unspecified", so it
> >>>> leaves SO_REUSEADDR as is for the compatibility.
> >>>> 
> >>>> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> >>>> ---
> >>>> include/uapi/linux/sctp.h |  1 +
> >>>> net/sctp/socket.c         | 48 +++++++++++++++++++++++++++++++++++++++++++++++
> >>>> 2 files changed, 49 insertions(+)
> >>>> 
> >>> A few things:
> >>> 
> >>> 1) I agree with Tom, this feature is a complete duplication of the SK_REUSEPORT
> >>> socket option.  I understand that this is an implementation of the option in the
> >>> RFC, but its definately a duplication of a feature, which makes several things
> >>> really messy.
> >>> 
> >>> 2) The overloading of the sk_reuse opeion is a bad idea, for several reasons.
> >>> Chief among them is the behavioral interference between this patch and the
> >>> SO_REUSEADDR socket level option, that also sets this feature.  If you set
> >>> sk_reuse via SO_REUSEADDR, you will set the SCTP port reuse feature regardless
> >>> of the bind or 1:1/1:m state of the socket.  Vice versa, if you set this socket
> >>> option via the SCTP_PORT_REUSE option you will inadvertently turn on address
> >>> reuse for the socket.  We can't do that.
> >> 
> >> Given your comments, going a bit further here, one other big
> >> implication is that a port would never be able to be considered to
> >> fully meet SCTP standards regarding reuse because a rogue application
> >> may always abuse of the socket level opt to gain access to the port.
> >> 
> >> IOW, the patch allows the application to use such restrictions against
> >> itself and nothing else, which undermines the patch idea.
> >> 
> > Agreed.
> > 
> >> I lack the knowledge on why the SCTP option was proposed in the RFC. I
> >> guess they had a good reason to add the restriction on 1:1/1:m style.
> >> Does the usage of the current imply in any risk to SCTP sockets? If
> >> yes, that would give some grounds for going forward with the SCTP
> >> option.
> >> 
> > I'm also not privy to why the sctp option was proposed, though I expect that the
> > lack of standardization of SO_REUSEPORT probably had something to do with it.
> > As for the reasoning behind restriction to only 1:1 sockets, if I had to guess,
> > I would say it likely because it creates ordering difficulty at the application
> > level.
> > 
> > CC-ing Michael Tuxen, who I believe had some input on this RFC.  Hopefully he
> > can shed some light on this.
> Dear all,
> 
> the reason this was added is to have a specified way to allow a system to
> behave like a client and server making use of the INIT collision.
> 
> For 1-to-many style sockets you can do this by creating a socket, binding it,
> calling listen on it and trying to connect to the peer.
> 
> For 1-to-1 style sockets you need two sockets for it. One listener and one
> you use to connect (and close it in case of failure, open a new one...).
> 
> It was not clear if one can achieve this with SO_REUSEPORT and/or SO_REUSEADDR
> on all platforms. We left that unspecified.
> 
> I hope this makes the intention clearer.
> 
I think it makes the intention clearer yes, but it unfortunately does nothing in
my mind to clarify how the implementation should best handle the potential
overlap in functionality.  What I see here is that we have two functional paths
(the SO_REUSEPORT path and the SCTP_PORT_REUSE path), which may or may not
(depending on the OS implementation achieve the same functional goal (allowing
multiple sockets to share a port while allowing one socket to listen and the
other connect to a remote peer).  If both implementations do the same thing on a
given platform, we can either just alias one to another and be done, but if they
don't then we either have to implement both paths, and ensure that the
SO_REUSEPORT path is a no-op/error return for SCTP sockets, or that each path
implements a distinct feature set that is cleaarly documented.

That said, I think we may be in luck.  Looking at the connect and listen paths,
it appears to me that:

1) Sockets ignore SO_REUSEPORT in the connect and listen paths (save for any
autobinding) so it would appear that the intent of the SCTP rfc can be honored
via SO_REUSEPORT on linux.  

2) SO_REUSEPORT prevents changing state after a bind has occured, so we can honr
that part of the SCTP RFC.

The only missing part is the restriction that SCTP_REUSE_PORT has which is
unaccounted for is that 1:M sockets aren't allowed to enable port reuse.
However, I think the implication from Michaels description above is that port
reuse on a 1:M socket is implicit because a single socket can connect and listen
in that use case, rather than there being a danger to doing so.

As such, I would propose that we implement this socket option by simply setting
the sk->sk_reuseport field in the sock structure, and document the fact that
linux does not restrict port reuse from 1:M sockets.

Thoughts?
Neil


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

* Re: [PATCH net-next] sctp: add support for SCTP_REUSE_PORT sockopt
  2018-05-21 13:48           ` Neil Horman
@ 2018-05-21 14:09             ` Michael Tuexen
  -1 siblings, 0 replies; 33+ messages in thread
From: Michael Tuexen @ 2018-05-21 14:09 UTC (permalink / raw)
  To: Neil Horman
  Cc: Marcelo Ricardo Leitner, Xin Long, network dev, linux-sctp, davem

[-- Attachment #1: Type: text/plain, Size: 6459 bytes --]

> On 21. May 2018, at 15:48, Neil Horman <nhorman@tuxdriver.com> wrote:
> 
> On Mon, May 21, 2018 at 02:16:56PM +0200, Michael Tuexen wrote:
>>> On 21. May 2018, at 13:39, Neil Horman <nhorman@tuxdriver.com> wrote:
>>> 
>>> On Sun, May 20, 2018 at 10:54:04PM -0300, Marcelo Ricardo Leitner wrote:
>>>> On Sun, May 20, 2018 at 08:50:59PM -0400, Neil Horman wrote:
>>>>> On Sat, May 19, 2018 at 03:44:40PM +0800, Xin Long wrote:
>>>>>> This feature is actually already supported by sk->sk_reuse which can be
>>>>>> set by SO_REUSEADDR. But it's not working exactly as RFC6458 demands in
>>>>>> section 8.1.27, like:
>>>>>> 
>>>>>> - This option only supports one-to-one style SCTP sockets
>>>>>> - This socket option must not be used after calling bind()
>>>>>>   or sctp_bindx().
>>>>>> 
>>>>>> Besides, SCTP_REUSE_PORT sockopt should be provided for user's programs.
>>>>>> Otherwise, the programs with SCTP_REUSE_PORT from other systems will not
>>>>>> work in linux.
>>>>>> 
>>>>>> This patch reuses sk->sk_reuse and works pretty much as SO_REUSEADDR,
>>>>>> just with some extra setup limitations that are neeeded when it is being
>>>>>> enabled.
>>>>>> 
>>>>>> "It should be noted that the behavior of the socket-level socket option
>>>>>> to reuse ports and/or addresses for SCTP sockets is unspecified", so it
>>>>>> leaves SO_REUSEADDR as is for the compatibility.
>>>>>> 
>>>>>> Signed-off-by: Xin Long <lucien.xin@gmail.com>
>>>>>> ---
>>>>>> include/uapi/linux/sctp.h |  1 +
>>>>>> net/sctp/socket.c         | 48 +++++++++++++++++++++++++++++++++++++++++++++++
>>>>>> 2 files changed, 49 insertions(+)
>>>>>> 
>>>>> A few things:
>>>>> 
>>>>> 1) I agree with Tom, this feature is a complete duplication of the SK_REUSEPORT
>>>>> socket option.  I understand that this is an implementation of the option in the
>>>>> RFC, but its definately a duplication of a feature, which makes several things
>>>>> really messy.
>>>>> 
>>>>> 2) The overloading of the sk_reuse opeion is a bad idea, for several reasons.
>>>>> Chief among them is the behavioral interference between this patch and the
>>>>> SO_REUSEADDR socket level option, that also sets this feature.  If you set
>>>>> sk_reuse via SO_REUSEADDR, you will set the SCTP port reuse feature regardless
>>>>> of the bind or 1:1/1:m state of the socket.  Vice versa, if you set this socket
>>>>> option via the SCTP_PORT_REUSE option you will inadvertently turn on address
>>>>> reuse for the socket.  We can't do that.
>>>> 
>>>> Given your comments, going a bit further here, one other big
>>>> implication is that a port would never be able to be considered to
>>>> fully meet SCTP standards regarding reuse because a rogue application
>>>> may always abuse of the socket level opt to gain access to the port.
>>>> 
>>>> IOW, the patch allows the application to use such restrictions against
>>>> itself and nothing else, which undermines the patch idea.
>>>> 
>>> Agreed.
>>> 
>>>> I lack the knowledge on why the SCTP option was proposed in the RFC. I
>>>> guess they had a good reason to add the restriction on 1:1/1:m style.
>>>> Does the usage of the current imply in any risk to SCTP sockets? If
>>>> yes, that would give some grounds for going forward with the SCTP
>>>> option.
>>>> 
>>> I'm also not privy to why the sctp option was proposed, though I expect that the
>>> lack of standardization of SO_REUSEPORT probably had something to do with it.
>>> As for the reasoning behind restriction to only 1:1 sockets, if I had to guess,
>>> I would say it likely because it creates ordering difficulty at the application
>>> level.
>>> 
>>> CC-ing Michael Tuxen, who I believe had some input on this RFC.  Hopefully he
>>> can shed some light on this.
>> Dear all,
>> 
>> the reason this was added is to have a specified way to allow a system to
>> behave like a client and server making use of the INIT collision.
>> 
>> For 1-to-many style sockets you can do this by creating a socket, binding it,
>> calling listen on it and trying to connect to the peer.
>> 
>> For 1-to-1 style sockets you need two sockets for it. One listener and one
>> you use to connect (and close it in case of failure, open a new one...).
>> 
>> It was not clear if one can achieve this with SO_REUSEPORT and/or SO_REUSEADDR
>> on all platforms. We left that unspecified.
>> 
>> I hope this makes the intention clearer.
>> 
> I think it makes the intention clearer yes, but it unfortunately does nothing in
> my mind to clarify how the implementation should best handle the potential
> overlap in functionality.  What I see here is that we have two functional paths
> (the SO_REUSEPORT path and the SCTP_PORT_REUSE path), which may or may not
> (depending on the OS implementation achieve the same functional goal (allowing
> multiple sockets to share a port while allowing one socket to listen and the
> other connect to a remote peer).  If both implementations do the same thing on a
> given platform, we can either just alias one to another and be done, but if they
> don't then we either have to implement both paths, and ensure that the
> SO_REUSEPORT path is a no-op/error return for SCTP sockets, or that each path
> implements a distinct feature set that is cleaarly documented.
> 
> That said, I think we may be in luck.  Looking at the connect and listen paths,
> it appears to me that:
> 
> 1) Sockets ignore SO_REUSEPORT in the connect and listen paths (save for any
> autobinding) so it would appear that the intent of the SCTP rfc can be honored
> via SO_REUSEPORT on linux.  
> 
> 2) SO_REUSEPORT prevents changing state after a bind has occured, so we can honr
> that part of the SCTP RFC.
> 
> The only missing part is the restriction that SCTP_REUSE_PORT has which is
> unaccounted for is that 1:M sockets aren't allowed to enable port reuse.
> However, I think the implication from Michaels description above is that port
> reuse on a 1:M socket is implicit because a single socket can connect and listen
> in that use case, rather than there being a danger to doing so.
> 
> As such, I would propose that we implement this socket option by simply setting
> the sk->sk_reuseport field in the sock structure, and document the fact that
> linux does not restrict port reuse from 1:M sockets.
> 
> Thoughts?
Sounds acceptable to me...

Best regards
Michael
> Neil
> 


[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5367 bytes --]

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

* Re: [PATCH net-next] sctp: add support for SCTP_REUSE_PORT sockopt
@ 2018-05-21 14:09             ` Michael Tuexen
  0 siblings, 0 replies; 33+ messages in thread
From: Michael Tuexen @ 2018-05-21 14:09 UTC (permalink / raw)
  To: Neil Horman
  Cc: Marcelo Ricardo Leitner, Xin Long, network dev, linux-sctp, davem

[-- Attachment #1: Type: text/plain, Size: 6459 bytes --]

> On 21. May 2018, at 15:48, Neil Horman <nhorman@tuxdriver.com> wrote:
> 
> On Mon, May 21, 2018 at 02:16:56PM +0200, Michael Tuexen wrote:
>>> On 21. May 2018, at 13:39, Neil Horman <nhorman@tuxdriver.com> wrote:
>>> 
>>> On Sun, May 20, 2018 at 10:54:04PM -0300, Marcelo Ricardo Leitner wrote:
>>>> On Sun, May 20, 2018 at 08:50:59PM -0400, Neil Horman wrote:
>>>>> On Sat, May 19, 2018 at 03:44:40PM +0800, Xin Long wrote:
>>>>>> This feature is actually already supported by sk->sk_reuse which can be
>>>>>> set by SO_REUSEADDR. But it's not working exactly as RFC6458 demands in
>>>>>> section 8.1.27, like:
>>>>>> 
>>>>>> - This option only supports one-to-one style SCTP sockets
>>>>>> - This socket option must not be used after calling bind()
>>>>>>   or sctp_bindx().
>>>>>> 
>>>>>> Besides, SCTP_REUSE_PORT sockopt should be provided for user's programs.
>>>>>> Otherwise, the programs with SCTP_REUSE_PORT from other systems will not
>>>>>> work in linux.
>>>>>> 
>>>>>> This patch reuses sk->sk_reuse and works pretty much as SO_REUSEADDR,
>>>>>> just with some extra setup limitations that are neeeded when it is being
>>>>>> enabled.
>>>>>> 
>>>>>> "It should be noted that the behavior of the socket-level socket option
>>>>>> to reuse ports and/or addresses for SCTP sockets is unspecified", so it
>>>>>> leaves SO_REUSEADDR as is for the compatibility.
>>>>>> 
>>>>>> Signed-off-by: Xin Long <lucien.xin@gmail.com>
>>>>>> ---
>>>>>> include/uapi/linux/sctp.h |  1 +
>>>>>> net/sctp/socket.c         | 48 +++++++++++++++++++++++++++++++++++++++++++++++
>>>>>> 2 files changed, 49 insertions(+)
>>>>>> 
>>>>> A few things:
>>>>> 
>>>>> 1) I agree with Tom, this feature is a complete duplication of the SK_REUSEPORT
>>>>> socket option.  I understand that this is an implementation of the option in the
>>>>> RFC, but its definately a duplication of a feature, which makes several things
>>>>> really messy.
>>>>> 
>>>>> 2) The overloading of the sk_reuse opeion is a bad idea, for several reasons.
>>>>> Chief among them is the behavioral interference between this patch and the
>>>>> SO_REUSEADDR socket level option, that also sets this feature.  If you set
>>>>> sk_reuse via SO_REUSEADDR, you will set the SCTP port reuse feature regardless
>>>>> of the bind or 1:1/1:m state of the socket.  Vice versa, if you set this socket
>>>>> option via the SCTP_PORT_REUSE option you will inadvertently turn on address
>>>>> reuse for the socket.  We can't do that.
>>>> 
>>>> Given your comments, going a bit further here, one other big
>>>> implication is that a port would never be able to be considered to
>>>> fully meet SCTP standards regarding reuse because a rogue application
>>>> may always abuse of the socket level opt to gain access to the port.
>>>> 
>>>> IOW, the patch allows the application to use such restrictions against
>>>> itself and nothing else, which undermines the patch idea.
>>>> 
>>> Agreed.
>>> 
>>>> I lack the knowledge on why the SCTP option was proposed in the RFC. I
>>>> guess they had a good reason to add the restriction on 1:1/1:m style.
>>>> Does the usage of the current imply in any risk to SCTP sockets? If
>>>> yes, that would give some grounds for going forward with the SCTP
>>>> option.
>>>> 
>>> I'm also not privy to why the sctp option was proposed, though I expect that the
>>> lack of standardization of SO_REUSEPORT probably had something to do with it.
>>> As for the reasoning behind restriction to only 1:1 sockets, if I had to guess,
>>> I would say it likely because it creates ordering difficulty at the application
>>> level.
>>> 
>>> CC-ing Michael Tuxen, who I believe had some input on this RFC.  Hopefully he
>>> can shed some light on this.
>> Dear all,
>> 
>> the reason this was added is to have a specified way to allow a system to
>> behave like a client and server making use of the INIT collision.
>> 
>> For 1-to-many style sockets you can do this by creating a socket, binding it,
>> calling listen on it and trying to connect to the peer.
>> 
>> For 1-to-1 style sockets you need two sockets for it. One listener and one
>> you use to connect (and close it in case of failure, open a new one...).
>> 
>> It was not clear if one can achieve this with SO_REUSEPORT and/or SO_REUSEADDR
>> on all platforms. We left that unspecified.
>> 
>> I hope this makes the intention clearer.
>> 
> I think it makes the intention clearer yes, but it unfortunately does nothing in
> my mind to clarify how the implementation should best handle the potential
> overlap in functionality.  What I see here is that we have two functional paths
> (the SO_REUSEPORT path and the SCTP_PORT_REUSE path), which may or may not
> (depending on the OS implementation achieve the same functional goal (allowing
> multiple sockets to share a port while allowing one socket to listen and the
> other connect to a remote peer).  If both implementations do the same thing on a
> given platform, we can either just alias one to another and be done, but if they
> don't then we either have to implement both paths, and ensure that the
> SO_REUSEPORT path is a no-op/error return for SCTP sockets, or that each path
> implements a distinct feature set that is cleaarly documented.
> 
> That said, I think we may be in luck.  Looking at the connect and listen paths,
> it appears to me that:
> 
> 1) Sockets ignore SO_REUSEPORT in the connect and listen paths (save for any
> autobinding) so it would appear that the intent of the SCTP rfc can be honored
> via SO_REUSEPORT on linux.  
> 
> 2) SO_REUSEPORT prevents changing state after a bind has occured, so we can honr
> that part of the SCTP RFC.
> 
> The only missing part is the restriction that SCTP_REUSE_PORT has which is
> unaccounted for is that 1:M sockets aren't allowed to enable port reuse.
> However, I think the implication from Michaels description above is that port
> reuse on a 1:M socket is implicit because a single socket can connect and listen
> in that use case, rather than there being a danger to doing so.
> 
> As such, I would propose that we implement this socket option by simply setting
> the sk->sk_reuseport field in the sock structure, and document the fact that
> linux does not restrict port reuse from 1:M sockets.
> 
> Thoughts?
Sounds acceptable to me...

Best regards
Michael
> Neil
> 


[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5367 bytes --]

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

* Re: [PATCH net-next] sctp: add support for SCTP_REUSE_PORT sockopt
  2018-05-21 14:09             ` Michael Tuexen
@ 2018-05-21 15:51               ` Marcelo Ricardo Leitner
  -1 siblings, 0 replies; 33+ messages in thread
From: Marcelo Ricardo Leitner @ 2018-05-21 15:51 UTC (permalink / raw)
  To: Michael Tuexen; +Cc: Neil Horman, Xin Long, network dev, linux-sctp, davem

On Mon, May 21, 2018 at 04:09:31PM +0200, Michael Tuexen wrote:
> > On 21. May 2018, at 15:48, Neil Horman <nhorman@tuxdriver.com> wrote:
> > 
> > On Mon, May 21, 2018 at 02:16:56PM +0200, Michael Tuexen wrote:
> >>> On 21. May 2018, at 13:39, Neil Horman <nhorman@tuxdriver.com> wrote:
> >>> 
> >>> On Sun, May 20, 2018 at 10:54:04PM -0300, Marcelo Ricardo Leitner wrote:
> >>>> On Sun, May 20, 2018 at 08:50:59PM -0400, Neil Horman wrote:
> >>>>> On Sat, May 19, 2018 at 03:44:40PM +0800, Xin Long wrote:
> >>>>>> This feature is actually already supported by sk->sk_reuse which can be
> >>>>>> set by SO_REUSEADDR. But it's not working exactly as RFC6458 demands in
> >>>>>> section 8.1.27, like:
> >>>>>> 
> >>>>>> - This option only supports one-to-one style SCTP sockets
> >>>>>> - This socket option must not be used after calling bind()
> >>>>>>   or sctp_bindx().
> >>>>>> 
> >>>>>> Besides, SCTP_REUSE_PORT sockopt should be provided for user's programs.
> >>>>>> Otherwise, the programs with SCTP_REUSE_PORT from other systems will not
> >>>>>> work in linux.
> >>>>>> 
> >>>>>> This patch reuses sk->sk_reuse and works pretty much as SO_REUSEADDR,
> >>>>>> just with some extra setup limitations that are neeeded when it is being
> >>>>>> enabled.
> >>>>>> 
> >>>>>> "It should be noted that the behavior of the socket-level socket option
> >>>>>> to reuse ports and/or addresses for SCTP sockets is unspecified", so it
> >>>>>> leaves SO_REUSEADDR as is for the compatibility.
> >>>>>> 
> >>>>>> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> >>>>>> ---
> >>>>>> include/uapi/linux/sctp.h |  1 +
> >>>>>> net/sctp/socket.c         | 48 +++++++++++++++++++++++++++++++++++++++++++++++
> >>>>>> 2 files changed, 49 insertions(+)
> >>>>>> 
> >>>>> A few things:
> >>>>> 
> >>>>> 1) I agree with Tom, this feature is a complete duplication of the SK_REUSEPORT
> >>>>> socket option.  I understand that this is an implementation of the option in the
> >>>>> RFC, but its definately a duplication of a feature, which makes several things
> >>>>> really messy.
> >>>>> 
> >>>>> 2) The overloading of the sk_reuse opeion is a bad idea, for several reasons.
> >>>>> Chief among them is the behavioral interference between this patch and the
> >>>>> SO_REUSEADDR socket level option, that also sets this feature.  If you set
> >>>>> sk_reuse via SO_REUSEADDR, you will set the SCTP port reuse feature regardless
> >>>>> of the bind or 1:1/1:m state of the socket.  Vice versa, if you set this socket
> >>>>> option via the SCTP_PORT_REUSE option you will inadvertently turn on address
> >>>>> reuse for the socket.  We can't do that.
> >>>> 
> >>>> Given your comments, going a bit further here, one other big
> >>>> implication is that a port would never be able to be considered to
> >>>> fully meet SCTP standards regarding reuse because a rogue application
> >>>> may always abuse of the socket level opt to gain access to the port.
> >>>> 
> >>>> IOW, the patch allows the application to use such restrictions against
> >>>> itself and nothing else, which undermines the patch idea.
> >>>> 
> >>> Agreed.
> >>> 
> >>>> I lack the knowledge on why the SCTP option was proposed in the RFC. I
> >>>> guess they had a good reason to add the restriction on 1:1/1:m style.
> >>>> Does the usage of the current imply in any risk to SCTP sockets? If
> >>>> yes, that would give some grounds for going forward with the SCTP
> >>>> option.
> >>>> 
> >>> I'm also not privy to why the sctp option was proposed, though I expect that the
> >>> lack of standardization of SO_REUSEPORT probably had something to do with it.
> >>> As for the reasoning behind restriction to only 1:1 sockets, if I had to guess,
> >>> I would say it likely because it creates ordering difficulty at the application
> >>> level.
> >>> 
> >>> CC-ing Michael Tuxen, who I believe had some input on this RFC.  Hopefully he
> >>> can shed some light on this.
> >> Dear all,
> >> 
> >> the reason this was added is to have a specified way to allow a system to
> >> behave like a client and server making use of the INIT collision.
> >> 
> >> For 1-to-many style sockets you can do this by creating a socket, binding it,
> >> calling listen on it and trying to connect to the peer.
> >> 
> >> For 1-to-1 style sockets you need two sockets for it. One listener and one
> >> you use to connect (and close it in case of failure, open a new one...).
> >> 
> >> It was not clear if one can achieve this with SO_REUSEPORT and/or SO_REUSEADDR
> >> on all platforms. We left that unspecified.
> >> 
> >> I hope this makes the intention clearer.
> >> 
> > I think it makes the intention clearer yes, but it unfortunately does nothing in
> > my mind to clarify how the implementation should best handle the potential
> > overlap in functionality.  What I see here is that we have two functional paths
> > (the SO_REUSEPORT path and the SCTP_PORT_REUSE path), which may or may not
> > (depending on the OS implementation achieve the same functional goal (allowing
> > multiple sockets to share a port while allowing one socket to listen and the
> > other connect to a remote peer).  If both implementations do the same thing on a
> > given platform, we can either just alias one to another and be done, but if they
> > don't then we either have to implement both paths, and ensure that the
> > SO_REUSEPORT path is a no-op/error return for SCTP sockets, or that each path
> > implements a distinct feature set that is cleaarly documented.
> > 
> > That said, I think we may be in luck.  Looking at the connect and listen paths,
> > it appears to me that:
> > 
> > 1) Sockets ignore SO_REUSEPORT in the connect and listen paths (save for any
> > autobinding) so it would appear that the intent of the SCTP rfc can be honored
> > via SO_REUSEPORT on linux.  
> > 
> > 2) SO_REUSEPORT prevents changing state after a bind has occured, so we can honr
> > that part of the SCTP RFC.
> > 
> > The only missing part is the restriction that SCTP_REUSE_PORT has which is
> > unaccounted for is that 1:M sockets aren't allowed to enable port reuse.
> > However, I think the implication from Michaels description above is that port
> > reuse on a 1:M socket is implicit because a single socket can connect and listen
> > in that use case, rather than there being a danger to doing so.
> > 
> > As such, I would propose that we implement this socket option by simply setting
> > the sk->sk_reuseport field in the sock structure, and document the fact that
> > linux does not restrict port reuse from 1:M sockets.
> > 
> > Thoughts?
> Sounds acceptable to me...

+1

> 
> Best regards
> Michael
> > Neil
> > 
> 

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

* Re: [PATCH net-next] sctp: add support for SCTP_REUSE_PORT sockopt
@ 2018-05-21 15:51               ` Marcelo Ricardo Leitner
  0 siblings, 0 replies; 33+ messages in thread
From: Marcelo Ricardo Leitner @ 2018-05-21 15:51 UTC (permalink / raw)
  To: Michael Tuexen; +Cc: Neil Horman, Xin Long, network dev, linux-sctp, davem

On Mon, May 21, 2018 at 04:09:31PM +0200, Michael Tuexen wrote:
> > On 21. May 2018, at 15:48, Neil Horman <nhorman@tuxdriver.com> wrote:
> > 
> > On Mon, May 21, 2018 at 02:16:56PM +0200, Michael Tuexen wrote:
> >>> On 21. May 2018, at 13:39, Neil Horman <nhorman@tuxdriver.com> wrote:
> >>> 
> >>> On Sun, May 20, 2018 at 10:54:04PM -0300, Marcelo Ricardo Leitner wrote:
> >>>> On Sun, May 20, 2018 at 08:50:59PM -0400, Neil Horman wrote:
> >>>>> On Sat, May 19, 2018 at 03:44:40PM +0800, Xin Long wrote:
> >>>>>> This feature is actually already supported by sk->sk_reuse which can be
> >>>>>> set by SO_REUSEADDR. But it's not working exactly as RFC6458 demands in
> >>>>>> section 8.1.27, like:
> >>>>>> 
> >>>>>> - This option only supports one-to-one style SCTP sockets
> >>>>>> - This socket option must not be used after calling bind()
> >>>>>>   or sctp_bindx().
> >>>>>> 
> >>>>>> Besides, SCTP_REUSE_PORT sockopt should be provided for user's programs.
> >>>>>> Otherwise, the programs with SCTP_REUSE_PORT from other systems will not
> >>>>>> work in linux.
> >>>>>> 
> >>>>>> This patch reuses sk->sk_reuse and works pretty much as SO_REUSEADDR,
> >>>>>> just with some extra setup limitations that are neeeded when it is being
> >>>>>> enabled.
> >>>>>> 
> >>>>>> "It should be noted that the behavior of the socket-level socket option
> >>>>>> to reuse ports and/or addresses for SCTP sockets is unspecified", so it
> >>>>>> leaves SO_REUSEADDR as is for the compatibility.
> >>>>>> 
> >>>>>> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> >>>>>> ---
> >>>>>> include/uapi/linux/sctp.h |  1 +
> >>>>>> net/sctp/socket.c         | 48 +++++++++++++++++++++++++++++++++++++++++++++++
> >>>>>> 2 files changed, 49 insertions(+)
> >>>>>> 
> >>>>> A few things:
> >>>>> 
> >>>>> 1) I agree with Tom, this feature is a complete duplication of the SK_REUSEPORT
> >>>>> socket option.  I understand that this is an implementation of the option in the
> >>>>> RFC, but its definately a duplication of a feature, which makes several things
> >>>>> really messy.
> >>>>> 
> >>>>> 2) The overloading of the sk_reuse opeion is a bad idea, for several reasons.
> >>>>> Chief among them is the behavioral interference between this patch and the
> >>>>> SO_REUSEADDR socket level option, that also sets this feature.  If you set
> >>>>> sk_reuse via SO_REUSEADDR, you will set the SCTP port reuse feature regardless
> >>>>> of the bind or 1:1/1:m state of the socket.  Vice versa, if you set this socket
> >>>>> option via the SCTP_PORT_REUSE option you will inadvertently turn on address
> >>>>> reuse for the socket.  We can't do that.
> >>>> 
> >>>> Given your comments, going a bit further here, one other big
> >>>> implication is that a port would never be able to be considered to
> >>>> fully meet SCTP standards regarding reuse because a rogue application
> >>>> may always abuse of the socket level opt to gain access to the port.
> >>>> 
> >>>> IOW, the patch allows the application to use such restrictions against
> >>>> itself and nothing else, which undermines the patch idea.
> >>>> 
> >>> Agreed.
> >>> 
> >>>> I lack the knowledge on why the SCTP option was proposed in the RFC. I
> >>>> guess they had a good reason to add the restriction on 1:1/1:m style.
> >>>> Does the usage of the current imply in any risk to SCTP sockets? If
> >>>> yes, that would give some grounds for going forward with the SCTP
> >>>> option.
> >>>> 
> >>> I'm also not privy to why the sctp option was proposed, though I expect that the
> >>> lack of standardization of SO_REUSEPORT probably had something to do with it.
> >>> As for the reasoning behind restriction to only 1:1 sockets, if I had to guess,
> >>> I would say it likely because it creates ordering difficulty at the application
> >>> level.
> >>> 
> >>> CC-ing Michael Tuxen, who I believe had some input on this RFC.  Hopefully he
> >>> can shed some light on this.
> >> Dear all,
> >> 
> >> the reason this was added is to have a specified way to allow a system to
> >> behave like a client and server making use of the INIT collision.
> >> 
> >> For 1-to-many style sockets you can do this by creating a socket, binding it,
> >> calling listen on it and trying to connect to the peer.
> >> 
> >> For 1-to-1 style sockets you need two sockets for it. One listener and one
> >> you use to connect (and close it in case of failure, open a new one...).
> >> 
> >> It was not clear if one can achieve this with SO_REUSEPORT and/or SO_REUSEADDR
> >> on all platforms. We left that unspecified.
> >> 
> >> I hope this makes the intention clearer.
> >> 
> > I think it makes the intention clearer yes, but it unfortunately does nothing in
> > my mind to clarify how the implementation should best handle the potential
> > overlap in functionality.  What I see here is that we have two functional paths
> > (the SO_REUSEPORT path and the SCTP_PORT_REUSE path), which may or may not
> > (depending on the OS implementation achieve the same functional goal (allowing
> > multiple sockets to share a port while allowing one socket to listen and the
> > other connect to a remote peer).  If both implementations do the same thing on a
> > given platform, we can either just alias one to another and be done, but if they
> > don't then we either have to implement both paths, and ensure that the
> > SO_REUSEPORT path is a no-op/error return for SCTP sockets, or that each path
> > implements a distinct feature set that is cleaarly documented.
> > 
> > That said, I think we may be in luck.  Looking at the connect and listen paths,
> > it appears to me that:
> > 
> > 1) Sockets ignore SO_REUSEPORT in the connect and listen paths (save for any
> > autobinding) so it would appear that the intent of the SCTP rfc can be honored
> > via SO_REUSEPORT on linux.  
> > 
> > 2) SO_REUSEPORT prevents changing state after a bind has occured, so we can honr
> > that part of the SCTP RFC.
> > 
> > The only missing part is the restriction that SCTP_REUSE_PORT has which is
> > unaccounted for is that 1:M sockets aren't allowed to enable port reuse.
> > However, I think the implication from Michaels description above is that port
> > reuse on a 1:M socket is implicit because a single socket can connect and listen
> > in that use case, rather than there being a danger to doing so.
> > 
> > As such, I would propose that we implement this socket option by simply setting
> > the sk->sk_reuseport field in the sock structure, and document the fact that
> > linux does not restrict port reuse from 1:M sockets.
> > 
> > Thoughts?
> Sounds acceptable to me...

+1

> 
> Best regards
> Michael
> > Neil
> > 
> 



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

* Re: [PATCH net-next] sctp: add support for SCTP_REUSE_PORT sockopt
  2018-05-21 13:48           ` Neil Horman
@ 2018-05-22  7:07             ` Xin Long
  -1 siblings, 0 replies; 33+ messages in thread
From: Xin Long @ 2018-05-22  7:07 UTC (permalink / raw)
  To: Neil Horman
  Cc: Michael Tuexen, Marcelo Ricardo Leitner, network dev, linux-sctp, davem

On Mon, May 21, 2018 at 9:48 PM, Neil Horman <nhorman@tuxdriver.com> wrote:
> On Mon, May 21, 2018 at 02:16:56PM +0200, Michael Tuexen wrote:
>> > On 21. May 2018, at 13:39, Neil Horman <nhorman@tuxdriver.com> wrote:
>> >
>> > On Sun, May 20, 2018 at 10:54:04PM -0300, Marcelo Ricardo Leitner wrote:
>> >> On Sun, May 20, 2018 at 08:50:59PM -0400, Neil Horman wrote:
>> >>> On Sat, May 19, 2018 at 03:44:40PM +0800, Xin Long wrote:
>> >>>> This feature is actually already supported by sk->sk_reuse which can be
>> >>>> set by SO_REUSEADDR. But it's not working exactly as RFC6458 demands in
>> >>>> section 8.1.27, like:
>> >>>>
>> >>>>  - This option only supports one-to-one style SCTP sockets
>> >>>>  - This socket option must not be used after calling bind()
>> >>>>    or sctp_bindx().
>> >>>>
>> >>>> Besides, SCTP_REUSE_PORT sockopt should be provided for user's programs.
>> >>>> Otherwise, the programs with SCTP_REUSE_PORT from other systems will not
>> >>>> work in linux.
>> >>>>
>> >>>> This patch reuses sk->sk_reuse and works pretty much as SO_REUSEADDR,
>> >>>> just with some extra setup limitations that are neeeded when it is being
>> >>>> enabled.
>> >>>>
>> >>>> "It should be noted that the behavior of the socket-level socket option
>> >>>> to reuse ports and/or addresses for SCTP sockets is unspecified", so it
>> >>>> leaves SO_REUSEADDR as is for the compatibility.
>> >>>>
>> >>>> Signed-off-by: Xin Long <lucien.xin@gmail.com>
>> >>>> ---
>> >>>> include/uapi/linux/sctp.h |  1 +
>> >>>> net/sctp/socket.c         | 48 +++++++++++++++++++++++++++++++++++++++++++++++
>> >>>> 2 files changed, 49 insertions(+)
>> >>>>
>> >>> A few things:
>> >>>
>> >>> 1) I agree with Tom, this feature is a complete duplication of the SK_REUSEPORT
>> >>> socket option.  I understand that this is an implementation of the option in the
>> >>> RFC, but its definately a duplication of a feature, which makes several things
>> >>> really messy.
>> >>>
>> >>> 2) The overloading of the sk_reuse opeion is a bad idea, for several reasons.
>> >>> Chief among them is the behavioral interference between this patch and the
>> >>> SO_REUSEADDR socket level option, that also sets this feature.  If you set
>> >>> sk_reuse via SO_REUSEADDR, you will set the SCTP port reuse feature regardless
>> >>> of the bind or 1:1/1:m state of the socket.  Vice versa, if you set this socket
>> >>> option via the SCTP_PORT_REUSE option you will inadvertently turn on address
>> >>> reuse for the socket.  We can't do that.
>> >>
>> >> Given your comments, going a bit further here, one other big
>> >> implication is that a port would never be able to be considered to
>> >> fully meet SCTP standards regarding reuse because a rogue application
>> >> may always abuse of the socket level opt to gain access to the port.
>> >>
>> >> IOW, the patch allows the application to use such restrictions against
>> >> itself and nothing else, which undermines the patch idea.
>> >>
>> > Agreed.
>> >
>> >> I lack the knowledge on why the SCTP option was proposed in the RFC. I
>> >> guess they had a good reason to add the restriction on 1:1/1:m style.
>> >> Does the usage of the current imply in any risk to SCTP sockets? If
>> >> yes, that would give some grounds for going forward with the SCTP
>> >> option.
>> >>
>> > I'm also not privy to why the sctp option was proposed, though I expect that the
>> > lack of standardization of SO_REUSEPORT probably had something to do with it.
>> > As for the reasoning behind restriction to only 1:1 sockets, if I had to guess,
>> > I would say it likely because it creates ordering difficulty at the application
>> > level.
>> >
>> > CC-ing Michael Tuxen, who I believe had some input on this RFC.  Hopefully he
>> > can shed some light on this.
>> Dear all,
>>
>> the reason this was added is to have a specified way to allow a system to
>> behave like a client and server making use of the INIT collision.
>>
>> For 1-to-many style sockets you can do this by creating a socket, binding it,
>> calling listen on it and trying to connect to the peer.
>>
>> For 1-to-1 style sockets you need two sockets for it. One listener and one
>> you use to connect (and close it in case of failure, open a new one...).
>>
>> It was not clear if one can achieve this with SO_REUSEPORT and/or SO_REUSEADDR
>> on all platforms. We left that unspecified.
>>
>> I hope this makes the intention clearer.
>>
> I think it makes the intention clearer yes, but it unfortunately does nothing in
> my mind to clarify how the implementation should best handle the potential
> overlap in functionality.  What I see here is that we have two functional paths
> (the SO_REUSEPORT path and the SCTP_PORT_REUSE path), which may or may not
> (depending on the OS implementation achieve the same functional goal (allowing
> multiple sockets to share a port while allowing one socket to listen and the
> other connect to a remote peer).  If both implementations do the same thing on a
> given platform, we can either just alias one to another and be done, but if they
> don't then we either have to implement both paths, and ensure that the
> SO_REUSEPORT path is a no-op/error return for SCTP sockets, or that each path
> implements a distinct feature set that is cleaarly documented.
>
> That said, I think we may be in luck.  Looking at the connect and listen paths,
> it appears to me that:
>
> 1) Sockets ignore SO_REUSEPORT in the connect and listen paths (save for any
> autobinding) so it would appear that the intent of the SCTP rfc can be honored
> via SO_REUSEPORT on linux.
>
> 2) SO_REUSEPORT prevents changing state after a bind has occured, so we can honr
> that part of the SCTP RFC.
>
> The only missing part is the restriction that SCTP_REUSE_PORT has which is
> unaccounted for is that 1:M sockets aren't allowed to enable port reuse.
> However, I think the implication from Michaels description above is that port
> reuse on a 1:M socket is implicit because a single socket can connect and listen
> in that use case, rather than there being a danger to doing so.
>
> As such, I would propose that we implement this socket option by simply setting
> the sk->sk_reuseport field in the sock structure, and document the fact that
> linux does not restrict port reuse from 1:M sockets.
Note that, sk->sk_reuseport is not affecting linux SCTP socket at all now.
linux SCTP socket doesn't really have SO_REUSEADDR (sk->sk_reuse)
support, but use sk->sk_reuse as REUSE_PORT, (yes, it is confusing).
Pls refer to sctp_get_port_local().

So I'm not sure using sk->sk_reuseport here means we will drop sk->sk_reuse
use in linux SCTP but use sk->sk_reuseport instead, or we will think that socket
enables 'port reuse' when either of them is set.

Note some users may be already using SO_REUSEADDR to enable the 'port
reuse' in linux sctp socket. If we're changing to sk->sk_reuseport, we may face
a compatibility problem.


>
> Thoughts?
> Neil
>

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

* Re: [PATCH net-next] sctp: add support for SCTP_REUSE_PORT sockopt
@ 2018-05-22  7:07             ` Xin Long
  0 siblings, 0 replies; 33+ messages in thread
From: Xin Long @ 2018-05-22  7:07 UTC (permalink / raw)
  To: Neil Horman
  Cc: Michael Tuexen, Marcelo Ricardo Leitner, network dev, linux-sctp, davem

On Mon, May 21, 2018 at 9:48 PM, Neil Horman <nhorman@tuxdriver.com> wrote:
> On Mon, May 21, 2018 at 02:16:56PM +0200, Michael Tuexen wrote:
>> > On 21. May 2018, at 13:39, Neil Horman <nhorman@tuxdriver.com> wrote:
>> >
>> > On Sun, May 20, 2018 at 10:54:04PM -0300, Marcelo Ricardo Leitner wrote:
>> >> On Sun, May 20, 2018 at 08:50:59PM -0400, Neil Horman wrote:
>> >>> On Sat, May 19, 2018 at 03:44:40PM +0800, Xin Long wrote:
>> >>>> This feature is actually already supported by sk->sk_reuse which can be
>> >>>> set by SO_REUSEADDR. But it's not working exactly as RFC6458 demands in
>> >>>> section 8.1.27, like:
>> >>>>
>> >>>>  - This option only supports one-to-one style SCTP sockets
>> >>>>  - This socket option must not be used after calling bind()
>> >>>>    or sctp_bindx().
>> >>>>
>> >>>> Besides, SCTP_REUSE_PORT sockopt should be provided for user's programs.
>> >>>> Otherwise, the programs with SCTP_REUSE_PORT from other systems will not
>> >>>> work in linux.
>> >>>>
>> >>>> This patch reuses sk->sk_reuse and works pretty much as SO_REUSEADDR,
>> >>>> just with some extra setup limitations that are neeeded when it is being
>> >>>> enabled.
>> >>>>
>> >>>> "It should be noted that the behavior of the socket-level socket option
>> >>>> to reuse ports and/or addresses for SCTP sockets is unspecified", so it
>> >>>> leaves SO_REUSEADDR as is for the compatibility.
>> >>>>
>> >>>> Signed-off-by: Xin Long <lucien.xin@gmail.com>
>> >>>> ---
>> >>>> include/uapi/linux/sctp.h |  1 +
>> >>>> net/sctp/socket.c         | 48 +++++++++++++++++++++++++++++++++++++++++++++++
>> >>>> 2 files changed, 49 insertions(+)
>> >>>>
>> >>> A few things:
>> >>>
>> >>> 1) I agree with Tom, this feature is a complete duplication of the SK_REUSEPORT
>> >>> socket option.  I understand that this is an implementation of the option in the
>> >>> RFC, but its definately a duplication of a feature, which makes several things
>> >>> really messy.
>> >>>
>> >>> 2) The overloading of the sk_reuse opeion is a bad idea, for several reasons.
>> >>> Chief among them is the behavioral interference between this patch and the
>> >>> SO_REUSEADDR socket level option, that also sets this feature.  If you set
>> >>> sk_reuse via SO_REUSEADDR, you will set the SCTP port reuse feature regardless
>> >>> of the bind or 1:1/1:m state of the socket.  Vice versa, if you set this socket
>> >>> option via the SCTP_PORT_REUSE option you will inadvertently turn on address
>> >>> reuse for the socket.  We can't do that.
>> >>
>> >> Given your comments, going a bit further here, one other big
>> >> implication is that a port would never be able to be considered to
>> >> fully meet SCTP standards regarding reuse because a rogue application
>> >> may always abuse of the socket level opt to gain access to the port.
>> >>
>> >> IOW, the patch allows the application to use such restrictions against
>> >> itself and nothing else, which undermines the patch idea.
>> >>
>> > Agreed.
>> >
>> >> I lack the knowledge on why the SCTP option was proposed in the RFC. I
>> >> guess they had a good reason to add the restriction on 1:1/1:m style.
>> >> Does the usage of the current imply in any risk to SCTP sockets? If
>> >> yes, that would give some grounds for going forward with the SCTP
>> >> option.
>> >>
>> > I'm also not privy to why the sctp option was proposed, though I expect that the
>> > lack of standardization of SO_REUSEPORT probably had something to do with it.
>> > As for the reasoning behind restriction to only 1:1 sockets, if I had to guess,
>> > I would say it likely because it creates ordering difficulty at the application
>> > level.
>> >
>> > CC-ing Michael Tuxen, who I believe had some input on this RFC.  Hopefully he
>> > can shed some light on this.
>> Dear all,
>>
>> the reason this was added is to have a specified way to allow a system to
>> behave like a client and server making use of the INIT collision.
>>
>> For 1-to-many style sockets you can do this by creating a socket, binding it,
>> calling listen on it and trying to connect to the peer.
>>
>> For 1-to-1 style sockets you need two sockets for it. One listener and one
>> you use to connect (and close it in case of failure, open a new one...).
>>
>> It was not clear if one can achieve this with SO_REUSEPORT and/or SO_REUSEADDR
>> on all platforms. We left that unspecified.
>>
>> I hope this makes the intention clearer.
>>
> I think it makes the intention clearer yes, but it unfortunately does nothing in
> my mind to clarify how the implementation should best handle the potential
> overlap in functionality.  What I see here is that we have two functional paths
> (the SO_REUSEPORT path and the SCTP_PORT_REUSE path), which may or may not
> (depending on the OS implementation achieve the same functional goal (allowing
> multiple sockets to share a port while allowing one socket to listen and the
> other connect to a remote peer).  If both implementations do the same thing on a
> given platform, we can either just alias one to another and be done, but if they
> don't then we either have to implement both paths, and ensure that the
> SO_REUSEPORT path is a no-op/error return for SCTP sockets, or that each path
> implements a distinct feature set that is cleaarly documented.
>
> That said, I think we may be in luck.  Looking at the connect and listen paths,
> it appears to me that:
>
> 1) Sockets ignore SO_REUSEPORT in the connect and listen paths (save for any
> autobinding) so it would appear that the intent of the SCTP rfc can be honored
> via SO_REUSEPORT on linux.
>
> 2) SO_REUSEPORT prevents changing state after a bind has occured, so we can honr
> that part of the SCTP RFC.
>
> The only missing part is the restriction that SCTP_REUSE_PORT has which is
> unaccounted for is that 1:M sockets aren't allowed to enable port reuse.
> However, I think the implication from Michaels description above is that port
> reuse on a 1:M socket is implicit because a single socket can connect and listen
> in that use case, rather than there being a danger to doing so.
>
> As such, I would propose that we implement this socket option by simply setting
> the sk->sk_reuseport field in the sock structure, and document the fact that
> linux does not restrict port reuse from 1:M sockets.
Note that, sk->sk_reuseport is not affecting linux SCTP socket at all now.
linux SCTP socket doesn't really have SO_REUSEADDR (sk->sk_reuse)
support, but use sk->sk_reuse as REUSE_PORT, (yes, it is confusing).
Pls refer to sctp_get_port_local().

So I'm not sure using sk->sk_reuseport here means we will drop sk->sk_reuse
use in linux SCTP but use sk->sk_reuseport instead, or we will think that socket
enables 'port reuse' when either of them is set.

Note some users may be already using SO_REUSEADDR to enable the 'port
reuse' in linux sctp socket. If we're changing to sk->sk_reuseport, we may face
a compatibility problem.


>
> Thoughts?
> Neil
>

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

* Re: [PATCH net-next] sctp: add support for SCTP_REUSE_PORT sockopt
  2018-05-22  7:07             ` Xin Long
@ 2018-05-22 11:51               ` Neil Horman
  -1 siblings, 0 replies; 33+ messages in thread
From: Neil Horman @ 2018-05-22 11:51 UTC (permalink / raw)
  To: Xin Long
  Cc: Michael Tuexen, Marcelo Ricardo Leitner, network dev, linux-sctp, davem

On Tue, May 22, 2018 at 03:07:57PM +0800, Xin Long wrote:
> On Mon, May 21, 2018 at 9:48 PM, Neil Horman <nhorman@tuxdriver.com> wrote:
> > On Mon, May 21, 2018 at 02:16:56PM +0200, Michael Tuexen wrote:
> >> > On 21. May 2018, at 13:39, Neil Horman <nhorman@tuxdriver.com> wrote:
> >> >
> >> > On Sun, May 20, 2018 at 10:54:04PM -0300, Marcelo Ricardo Leitner wrote:
> >> >> On Sun, May 20, 2018 at 08:50:59PM -0400, Neil Horman wrote:
> >> >>> On Sat, May 19, 2018 at 03:44:40PM +0800, Xin Long wrote:
> >> >>>> This feature is actually already supported by sk->sk_reuse which can be
> >> >>>> set by SO_REUSEADDR. But it's not working exactly as RFC6458 demands in
> >> >>>> section 8.1.27, like:
> >> >>>>
> >> >>>>  - This option only supports one-to-one style SCTP sockets
> >> >>>>  - This socket option must not be used after calling bind()
> >> >>>>    or sctp_bindx().
> >> >>>>
> >> >>>> Besides, SCTP_REUSE_PORT sockopt should be provided for user's programs.
> >> >>>> Otherwise, the programs with SCTP_REUSE_PORT from other systems will not
> >> >>>> work in linux.
> >> >>>>
> >> >>>> This patch reuses sk->sk_reuse and works pretty much as SO_REUSEADDR,
> >> >>>> just with some extra setup limitations that are neeeded when it is being
> >> >>>> enabled.
> >> >>>>
> >> >>>> "It should be noted that the behavior of the socket-level socket option
> >> >>>> to reuse ports and/or addresses for SCTP sockets is unspecified", so it
> >> >>>> leaves SO_REUSEADDR as is for the compatibility.
> >> >>>>
> >> >>>> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> >> >>>> ---
> >> >>>> include/uapi/linux/sctp.h |  1 +
> >> >>>> net/sctp/socket.c         | 48 +++++++++++++++++++++++++++++++++++++++++++++++
> >> >>>> 2 files changed, 49 insertions(+)
> >> >>>>
> >> >>> A few things:
> >> >>>
> >> >>> 1) I agree with Tom, this feature is a complete duplication of the SK_REUSEPORT
> >> >>> socket option.  I understand that this is an implementation of the option in the
> >> >>> RFC, but its definately a duplication of a feature, which makes several things
> >> >>> really messy.
> >> >>>
> >> >>> 2) The overloading of the sk_reuse opeion is a bad idea, for several reasons.
> >> >>> Chief among them is the behavioral interference between this patch and the
> >> >>> SO_REUSEADDR socket level option, that also sets this feature.  If you set
> >> >>> sk_reuse via SO_REUSEADDR, you will set the SCTP port reuse feature regardless
> >> >>> of the bind or 1:1/1:m state of the socket.  Vice versa, if you set this socket
> >> >>> option via the SCTP_PORT_REUSE option you will inadvertently turn on address
> >> >>> reuse for the socket.  We can't do that.
> >> >>
> >> >> Given your comments, going a bit further here, one other big
> >> >> implication is that a port would never be able to be considered to
> >> >> fully meet SCTP standards regarding reuse because a rogue application
> >> >> may always abuse of the socket level opt to gain access to the port.
> >> >>
> >> >> IOW, the patch allows the application to use such restrictions against
> >> >> itself and nothing else, which undermines the patch idea.
> >> >>
> >> > Agreed.
> >> >
> >> >> I lack the knowledge on why the SCTP option was proposed in the RFC. I
> >> >> guess they had a good reason to add the restriction on 1:1/1:m style.
> >> >> Does the usage of the current imply in any risk to SCTP sockets? If
> >> >> yes, that would give some grounds for going forward with the SCTP
> >> >> option.
> >> >>
> >> > I'm also not privy to why the sctp option was proposed, though I expect that the
> >> > lack of standardization of SO_REUSEPORT probably had something to do with it.
> >> > As for the reasoning behind restriction to only 1:1 sockets, if I had to guess,
> >> > I would say it likely because it creates ordering difficulty at the application
> >> > level.
> >> >
> >> > CC-ing Michael Tuxen, who I believe had some input on this RFC.  Hopefully he
> >> > can shed some light on this.
> >> Dear all,
> >>
> >> the reason this was added is to have a specified way to allow a system to
> >> behave like a client and server making use of the INIT collision.
> >>
> >> For 1-to-many style sockets you can do this by creating a socket, binding it,
> >> calling listen on it and trying to connect to the peer.
> >>
> >> For 1-to-1 style sockets you need two sockets for it. One listener and one
> >> you use to connect (and close it in case of failure, open a new one...).
> >>
> >> It was not clear if one can achieve this with SO_REUSEPORT and/or SO_REUSEADDR
> >> on all platforms. We left that unspecified.
> >>
> >> I hope this makes the intention clearer.
> >>
> > I think it makes the intention clearer yes, but it unfortunately does nothing in
> > my mind to clarify how the implementation should best handle the potential
> > overlap in functionality.  What I see here is that we have two functional paths
> > (the SO_REUSEPORT path and the SCTP_PORT_REUSE path), which may or may not
> > (depending on the OS implementation achieve the same functional goal (allowing
> > multiple sockets to share a port while allowing one socket to listen and the
> > other connect to a remote peer).  If both implementations do the same thing on a
> > given platform, we can either just alias one to another and be done, but if they
> > don't then we either have to implement both paths, and ensure that the
> > SO_REUSEPORT path is a no-op/error return for SCTP sockets, or that each path
> > implements a distinct feature set that is cleaarly documented.
> >
> > That said, I think we may be in luck.  Looking at the connect and listen paths,
> > it appears to me that:
> >
> > 1) Sockets ignore SO_REUSEPORT in the connect and listen paths (save for any
> > autobinding) so it would appear that the intent of the SCTP rfc can be honored
> > via SO_REUSEPORT on linux.
> >
> > 2) SO_REUSEPORT prevents changing state after a bind has occured, so we can honr
> > that part of the SCTP RFC.
> >
> > The only missing part is the restriction that SCTP_REUSE_PORT has which is
> > unaccounted for is that 1:M sockets aren't allowed to enable port reuse.
> > However, I think the implication from Michaels description above is that port
> > reuse on a 1:M socket is implicit because a single socket can connect and listen
> > in that use case, rather than there being a danger to doing so.
> >
> > As such, I would propose that we implement this socket option by simply setting
> > the sk->sk_reuseport field in the sock structure, and document the fact that
> > linux does not restrict port reuse from 1:M sockets.
> Note that, sk->sk_reuseport is not affecting linux SCTP socket at all now.
> linux SCTP socket doesn't really have SO_REUSEADDR (sk->sk_reuse)
> support, but use sk->sk_reuse as REUSE_PORT, (yes, it is confusing).
> Pls refer to sctp_get_port_local().
> 
No, its not used now, but if you do use it to do something specific to SCTP (via
the SCTP_REUSE_PORT socket option), you risk aliasing SO_REUSEPORT behavior to
it, and if it doesn't match what the RFC behavior mandates, thats a problem.

> So I'm not sure using sk->sk_reuseport here means we will drop sk->sk_reuse
> use in linux SCTP but use sk->sk_reuseport instead, or we will think that socket
> enables 'port reuse' when either of them is set.
> 
I don't think we would drop the behavior of sk_reuse here, why would we?  As far
as I can see, the behavior of SO_REUSEADDR (not SO_REUSEPORT), isn't in
question, is it?

> Note some users may be already using SO_REUSEADDR to enable the 'port
> reuse' in linux sctp socket. If we're changing to sk->sk_reuseport, we may face
> a compatibility problem.
> 
I don't see how the behavior of SO_REUSEADDR is in question here.  All I'm
suggesting is that you simplify this patch so that the SCTP_REUSE_PORT socket
option set sk_reuseport, as that option to my eyes conforms to the sctp rfc
requirements.  Or am I' missing something?

Neil

> 
> >
> > Thoughts?
> > Neil
> >
> 

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

* Re: [PATCH net-next] sctp: add support for SCTP_REUSE_PORT sockopt
@ 2018-05-22 11:51               ` Neil Horman
  0 siblings, 0 replies; 33+ messages in thread
From: Neil Horman @ 2018-05-22 11:51 UTC (permalink / raw)
  To: Xin Long
  Cc: Michael Tuexen, Marcelo Ricardo Leitner, network dev, linux-sctp, davem

On Tue, May 22, 2018 at 03:07:57PM +0800, Xin Long wrote:
> On Mon, May 21, 2018 at 9:48 PM, Neil Horman <nhorman@tuxdriver.com> wrote:
> > On Mon, May 21, 2018 at 02:16:56PM +0200, Michael Tuexen wrote:
> >> > On 21. May 2018, at 13:39, Neil Horman <nhorman@tuxdriver.com> wrote:
> >> >
> >> > On Sun, May 20, 2018 at 10:54:04PM -0300, Marcelo Ricardo Leitner wrote:
> >> >> On Sun, May 20, 2018 at 08:50:59PM -0400, Neil Horman wrote:
> >> >>> On Sat, May 19, 2018 at 03:44:40PM +0800, Xin Long wrote:
> >> >>>> This feature is actually already supported by sk->sk_reuse which can be
> >> >>>> set by SO_REUSEADDR. But it's not working exactly as RFC6458 demands in
> >> >>>> section 8.1.27, like:
> >> >>>>
> >> >>>>  - This option only supports one-to-one style SCTP sockets
> >> >>>>  - This socket option must not be used after calling bind()
> >> >>>>    or sctp_bindx().
> >> >>>>
> >> >>>> Besides, SCTP_REUSE_PORT sockopt should be provided for user's programs.
> >> >>>> Otherwise, the programs with SCTP_REUSE_PORT from other systems will not
> >> >>>> work in linux.
> >> >>>>
> >> >>>> This patch reuses sk->sk_reuse and works pretty much as SO_REUSEADDR,
> >> >>>> just with some extra setup limitations that are neeeded when it is being
> >> >>>> enabled.
> >> >>>>
> >> >>>> "It should be noted that the behavior of the socket-level socket option
> >> >>>> to reuse ports and/or addresses for SCTP sockets is unspecified", so it
> >> >>>> leaves SO_REUSEADDR as is for the compatibility.
> >> >>>>
> >> >>>> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> >> >>>> ---
> >> >>>> include/uapi/linux/sctp.h |  1 +
> >> >>>> net/sctp/socket.c         | 48 +++++++++++++++++++++++++++++++++++++++++++++++
> >> >>>> 2 files changed, 49 insertions(+)
> >> >>>>
> >> >>> A few things:
> >> >>>
> >> >>> 1) I agree with Tom, this feature is a complete duplication of the SK_REUSEPORT
> >> >>> socket option.  I understand that this is an implementation of the option in the
> >> >>> RFC, but its definately a duplication of a feature, which makes several things
> >> >>> really messy.
> >> >>>
> >> >>> 2) The overloading of the sk_reuse opeion is a bad idea, for several reasons.
> >> >>> Chief among them is the behavioral interference between this patch and the
> >> >>> SO_REUSEADDR socket level option, that also sets this feature.  If you set
> >> >>> sk_reuse via SO_REUSEADDR, you will set the SCTP port reuse feature regardless
> >> >>> of the bind or 1:1/1:m state of the socket.  Vice versa, if you set this socket
> >> >>> option via the SCTP_PORT_REUSE option you will inadvertently turn on address
> >> >>> reuse for the socket.  We can't do that.
> >> >>
> >> >> Given your comments, going a bit further here, one other big
> >> >> implication is that a port would never be able to be considered to
> >> >> fully meet SCTP standards regarding reuse because a rogue application
> >> >> may always abuse of the socket level opt to gain access to the port.
> >> >>
> >> >> IOW, the patch allows the application to use such restrictions against
> >> >> itself and nothing else, which undermines the patch idea.
> >> >>
> >> > Agreed.
> >> >
> >> >> I lack the knowledge on why the SCTP option was proposed in the RFC. I
> >> >> guess they had a good reason to add the restriction on 1:1/1:m style.
> >> >> Does the usage of the current imply in any risk to SCTP sockets? If
> >> >> yes, that would give some grounds for going forward with the SCTP
> >> >> option.
> >> >>
> >> > I'm also not privy to why the sctp option was proposed, though I expect that the
> >> > lack of standardization of SO_REUSEPORT probably had something to do with it.
> >> > As for the reasoning behind restriction to only 1:1 sockets, if I had to guess,
> >> > I would say it likely because it creates ordering difficulty at the application
> >> > level.
> >> >
> >> > CC-ing Michael Tuxen, who I believe had some input on this RFC.  Hopefully he
> >> > can shed some light on this.
> >> Dear all,
> >>
> >> the reason this was added is to have a specified way to allow a system to
> >> behave like a client and server making use of the INIT collision.
> >>
> >> For 1-to-many style sockets you can do this by creating a socket, binding it,
> >> calling listen on it and trying to connect to the peer.
> >>
> >> For 1-to-1 style sockets you need two sockets for it. One listener and one
> >> you use to connect (and close it in case of failure, open a new one...).
> >>
> >> It was not clear if one can achieve this with SO_REUSEPORT and/or SO_REUSEADDR
> >> on all platforms. We left that unspecified.
> >>
> >> I hope this makes the intention clearer.
> >>
> > I think it makes the intention clearer yes, but it unfortunately does nothing in
> > my mind to clarify how the implementation should best handle the potential
> > overlap in functionality.  What I see here is that we have two functional paths
> > (the SO_REUSEPORT path and the SCTP_PORT_REUSE path), which may or may not
> > (depending on the OS implementation achieve the same functional goal (allowing
> > multiple sockets to share a port while allowing one socket to listen and the
> > other connect to a remote peer).  If both implementations do the same thing on a
> > given platform, we can either just alias one to another and be done, but if they
> > don't then we either have to implement both paths, and ensure that the
> > SO_REUSEPORT path is a no-op/error return for SCTP sockets, or that each path
> > implements a distinct feature set that is cleaarly documented.
> >
> > That said, I think we may be in luck.  Looking at the connect and listen paths,
> > it appears to me that:
> >
> > 1) Sockets ignore SO_REUSEPORT in the connect and listen paths (save for any
> > autobinding) so it would appear that the intent of the SCTP rfc can be honored
> > via SO_REUSEPORT on linux.
> >
> > 2) SO_REUSEPORT prevents changing state after a bind has occured, so we can honr
> > that part of the SCTP RFC.
> >
> > The only missing part is the restriction that SCTP_REUSE_PORT has which is
> > unaccounted for is that 1:M sockets aren't allowed to enable port reuse.
> > However, I think the implication from Michaels description above is that port
> > reuse on a 1:M socket is implicit because a single socket can connect and listen
> > in that use case, rather than there being a danger to doing so.
> >
> > As such, I would propose that we implement this socket option by simply setting
> > the sk->sk_reuseport field in the sock structure, and document the fact that
> > linux does not restrict port reuse from 1:M sockets.
> Note that, sk->sk_reuseport is not affecting linux SCTP socket at all now.
> linux SCTP socket doesn't really have SO_REUSEADDR (sk->sk_reuse)
> support, but use sk->sk_reuse as REUSE_PORT, (yes, it is confusing).
> Pls refer to sctp_get_port_local().
> 
No, its not used now, but if you do use it to do something specific to SCTP (via
the SCTP_REUSE_PORT socket option), you risk aliasing SO_REUSEPORT behavior to
it, and if it doesn't match what the RFC behavior mandates, thats a problem.

> So I'm not sure using sk->sk_reuseport here means we will drop sk->sk_reuse
> use in linux SCTP but use sk->sk_reuseport instead, or we will think that socket
> enables 'port reuse' when either of them is set.
> 
I don't think we would drop the behavior of sk_reuse here, why would we?  As far
as I can see, the behavior of SO_REUSEADDR (not SO_REUSEPORT), isn't in
question, is it?

> Note some users may be already using SO_REUSEADDR to enable the 'port
> reuse' in linux sctp socket. If we're changing to sk->sk_reuseport, we may face
> a compatibility problem.
> 
I don't see how the behavior of SO_REUSEADDR is in question here.  All I'm
suggesting is that you simplify this patch so that the SCTP_REUSE_PORT socket
option set sk_reuseport, as that option to my eyes conforms to the sctp rfc
requirements.  Or am I' missing something?

Neil

> 
> >
> > Thoughts?
> > Neil
> >
> 

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

* RE: [PATCH net-next] sctp: add support for SCTP_REUSE_PORT sockopt
  2018-05-21 15:51               ` Marcelo Ricardo Leitner
  (?)
@ 2018-05-22 15:36               ` David Laight
  -1 siblings, 0 replies; 33+ messages in thread
From: David Laight @ 2018-05-22 15:36 UTC (permalink / raw)
  To: 'Marcelo Ricardo Leitner', Michael Tuexen
  Cc: Neil Horman, Xin Long, network dev, linux-sctp, davem

...
> > >> the reason this was added is to have a specified way to allow a system to
> > >> behave like a client and server making use of the INIT collision.
> > >>
> > >> For 1-to-many style sockets you can do this by creating a socket, binding it,
> > >> calling listen on it and trying to connect to the peer.
> > >>
> > >> For 1-to-1 style sockets you need two sockets for it. One listener and one
> > >> you use to connect (and close it in case of failure, open a new one...).
> > >>
> > >> It was not clear if one can achieve this with SO_REUSEPORT and/or SO_REUSEADDR
> > >> on all platforms. We left that unspecified.
> > >>
> > >> I hope this makes the intention clearer.

That really doesn't help for 1-1 sockets.
You need a way of accepting connections that come from a specific remote host.

Otherwise the application has to verify that the remove address on the incoming
connection is the one it is expecting.

It is also a problem if two different applications (instances) want to
generate 'INIT collisions' for the same local addresses but different remote
ones.

The only way 'INIT collisions' are going to work is with a different
option that stops the receipt of an ABORT chunk erroring a connect.

	David

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

* Re: [PATCH net-next] sctp: add support for SCTP_REUSE_PORT sockopt
  2018-05-22 11:51               ` Neil Horman
@ 2018-05-23  7:04                 ` Xin Long
  -1 siblings, 0 replies; 33+ messages in thread
From: Xin Long @ 2018-05-23  7:04 UTC (permalink / raw)
  To: Neil Horman
  Cc: Michael Tuexen, Marcelo Ricardo Leitner, network dev, linux-sctp, davem

On Tue, May 22, 2018 at 7:51 PM, Neil Horman <nhorman@tuxdriver.com> wrote:
> On Tue, May 22, 2018 at 03:07:57PM +0800, Xin Long wrote:
>> On Mon, May 21, 2018 at 9:48 PM, Neil Horman <nhorman@tuxdriver.com> wrote:
>> > On Mon, May 21, 2018 at 02:16:56PM +0200, Michael Tuexen wrote:
>> >> > On 21. May 2018, at 13:39, Neil Horman <nhorman@tuxdriver.com> wrote:
>> >> >
>> >> > On Sun, May 20, 2018 at 10:54:04PM -0300, Marcelo Ricardo Leitner wrote:
>> >> >> On Sun, May 20, 2018 at 08:50:59PM -0400, Neil Horman wrote:
>> >> >>> On Sat, May 19, 2018 at 03:44:40PM +0800, Xin Long wrote:
>> >> >>>> This feature is actually already supported by sk->sk_reuse which can be
>> >> >>>> set by SO_REUSEADDR. But it's not working exactly as RFC6458 demands in
>> >> >>>> section 8.1.27, like:
>> >> >>>>
>> >> >>>>  - This option only supports one-to-one style SCTP sockets
>> >> >>>>  - This socket option must not be used after calling bind()
>> >> >>>>    or sctp_bindx().
>> >> >>>>
>> >> >>>> Besides, SCTP_REUSE_PORT sockopt should be provided for user's programs.
>> >> >>>> Otherwise, the programs with SCTP_REUSE_PORT from other systems will not
>> >> >>>> work in linux.
>> >> >>>>
>> >> >>>> This patch reuses sk->sk_reuse and works pretty much as SO_REUSEADDR,
>> >> >>>> just with some extra setup limitations that are neeeded when it is being
>> >> >>>> enabled.
>> >> >>>>
>> >> >>>> "It should be noted that the behavior of the socket-level socket option
>> >> >>>> to reuse ports and/or addresses for SCTP sockets is unspecified", so it
>> >> >>>> leaves SO_REUSEADDR as is for the compatibility.
>> >> >>>>
>> >> >>>> Signed-off-by: Xin Long <lucien.xin@gmail.com>
>> >> >>>> ---
>> >> >>>> include/uapi/linux/sctp.h |  1 +
>> >> >>>> net/sctp/socket.c         | 48 +++++++++++++++++++++++++++++++++++++++++++++++
>> >> >>>> 2 files changed, 49 insertions(+)
>> >> >>>>
>> >> >>> A few things:
>> >> >>>
>> >> >>> 1) I agree with Tom, this feature is a complete duplication of the SK_REUSEPORT
>> >> >>> socket option.  I understand that this is an implementation of the option in the
>> >> >>> RFC, but its definately a duplication of a feature, which makes several things
>> >> >>> really messy.
>> >> >>>
>> >> >>> 2) The overloading of the sk_reuse opeion is a bad idea, for several reasons.
>> >> >>> Chief among them is the behavioral interference between this patch and the
>> >> >>> SO_REUSEADDR socket level option, that also sets this feature.  If you set
>> >> >>> sk_reuse via SO_REUSEADDR, you will set the SCTP port reuse feature regardless
>> >> >>> of the bind or 1:1/1:m state of the socket.  Vice versa, if you set this socket
>> >> >>> option via the SCTP_PORT_REUSE option you will inadvertently turn on address
>> >> >>> reuse for the socket.  We can't do that.
>> >> >>
>> >> >> Given your comments, going a bit further here, one other big
>> >> >> implication is that a port would never be able to be considered to
>> >> >> fully meet SCTP standards regarding reuse because a rogue application
>> >> >> may always abuse of the socket level opt to gain access to the port.
>> >> >>
>> >> >> IOW, the patch allows the application to use such restrictions against
>> >> >> itself and nothing else, which undermines the patch idea.
>> >> >>
>> >> > Agreed.
>> >> >
>> >> >> I lack the knowledge on why the SCTP option was proposed in the RFC. I
>> >> >> guess they had a good reason to add the restriction on 1:1/1:m style.
>> >> >> Does the usage of the current imply in any risk to SCTP sockets? If
>> >> >> yes, that would give some grounds for going forward with the SCTP
>> >> >> option.
>> >> >>
>> >> > I'm also not privy to why the sctp option was proposed, though I expect that the
>> >> > lack of standardization of SO_REUSEPORT probably had something to do with it.
>> >> > As for the reasoning behind restriction to only 1:1 sockets, if I had to guess,
>> >> > I would say it likely because it creates ordering difficulty at the application
>> >> > level.
>> >> >
>> >> > CC-ing Michael Tuxen, who I believe had some input on this RFC.  Hopefully he
>> >> > can shed some light on this.
>> >> Dear all,
>> >>
>> >> the reason this was added is to have a specified way to allow a system to
>> >> behave like a client and server making use of the INIT collision.
>> >>
>> >> For 1-to-many style sockets you can do this by creating a socket, binding it,
>> >> calling listen on it and trying to connect to the peer.
>> >>
>> >> For 1-to-1 style sockets you need two sockets for it. One listener and one
>> >> you use to connect (and close it in case of failure, open a new one...).
>> >>
>> >> It was not clear if one can achieve this with SO_REUSEPORT and/or SO_REUSEADDR
>> >> on all platforms. We left that unspecified.
>> >>
>> >> I hope this makes the intention clearer.
>> >>
>> > I think it makes the intention clearer yes, but it unfortunately does nothing in
>> > my mind to clarify how the implementation should best handle the potential
>> > overlap in functionality.  What I see here is that we have two functional paths
>> > (the SO_REUSEPORT path and the SCTP_PORT_REUSE path), which may or may not
>> > (depending on the OS implementation achieve the same functional goal (allowing
>> > multiple sockets to share a port while allowing one socket to listen and the
>> > other connect to a remote peer).  If both implementations do the same thing on a
>> > given platform, we can either just alias one to another and be done, but if they
>> > don't then we either have to implement both paths, and ensure that the
>> > SO_REUSEPORT path is a no-op/error return for SCTP sockets, or that each path
>> > implements a distinct feature set that is cleaarly documented.
>> >
>> > That said, I think we may be in luck.  Looking at the connect and listen paths,
>> > it appears to me that:
>> >
>> > 1) Sockets ignore SO_REUSEPORT in the connect and listen paths (save for any
>> > autobinding) so it would appear that the intent of the SCTP rfc can be honored
>> > via SO_REUSEPORT on linux.
>> >
>> > 2) SO_REUSEPORT prevents changing state after a bind has occured, so we can honr
>> > that part of the SCTP RFC.
>> >
>> > The only missing part is the restriction that SCTP_REUSE_PORT has which is
>> > unaccounted for is that 1:M sockets aren't allowed to enable port reuse.
>> > However, I think the implication from Michaels description above is that port
>> > reuse on a 1:M socket is implicit because a single socket can connect and listen
>> > in that use case, rather than there being a danger to doing so.
>> >
>> > As such, I would propose that we implement this socket option by simply setting
>> > the sk->sk_reuseport field in the sock structure, and document the fact that
>> > linux does not restrict port reuse from 1:M sockets.
>> Note that, sk->sk_reuseport is not affecting linux SCTP socket at all now.
>> linux SCTP socket doesn't really have SO_REUSEADDR (sk->sk_reuse)
>> support, but use sk->sk_reuse as REUSE_PORT, (yes, it is confusing).
>> Pls refer to sctp_get_port_local().
>>
> No, its not used now, but if you do use it to do something specific to SCTP (via
> the SCTP_REUSE_PORT socket option), you risk aliasing SO_REUSEPORT behavior to
> it, and if it doesn't match what the RFC behavior mandates, thats a problem.
>
>> So I'm not sure using sk->sk_reuseport here means we will drop sk->sk_reuse
>> use in linux SCTP but use sk->sk_reuseport instead, or we will think that socket
>> enables 'port reuse' when either of them is set.
>>
> I don't think we would drop the behavior of sk_reuse here, why would we?  As far
> as I can see, the behavior of SO_REUSEADDR (not SO_REUSEPORT), isn't in
> question, is it?
>
>> Note some users may be already using SO_REUSEADDR to enable the 'port
>> reuse' in linux sctp socket. If we're changing to sk->sk_reuseport, we may face
>> a compatibility problem.
>>
> I don't see how the behavior of SO_REUSEADDR is in question here.  All I'm
> suggesting is that you simplify this patch so that the SCTP_REUSE_PORT socket
> option set sk_reuseport, as that option to my eyes conforms to the sctp rfc
> requirements.  Or am I' missing something?
No, I am :)
sk_reuseport seems more complicated than sk_reuse. I kind of mixed them.
I need to check more beofore continuing. Thanks.

>
> Neil
>
>>
>> >
>> > Thoughts?
>> > Neil
>> >
>>

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

* Re: [PATCH net-next] sctp: add support for SCTP_REUSE_PORT sockopt
@ 2018-05-23  7:04                 ` Xin Long
  0 siblings, 0 replies; 33+ messages in thread
From: Xin Long @ 2018-05-23  7:04 UTC (permalink / raw)
  To: Neil Horman
  Cc: Michael Tuexen, Marcelo Ricardo Leitner, network dev, linux-sctp, davem

On Tue, May 22, 2018 at 7:51 PM, Neil Horman <nhorman@tuxdriver.com> wrote:
> On Tue, May 22, 2018 at 03:07:57PM +0800, Xin Long wrote:
>> On Mon, May 21, 2018 at 9:48 PM, Neil Horman <nhorman@tuxdriver.com> wrote:
>> > On Mon, May 21, 2018 at 02:16:56PM +0200, Michael Tuexen wrote:
>> >> > On 21. May 2018, at 13:39, Neil Horman <nhorman@tuxdriver.com> wrote:
>> >> >
>> >> > On Sun, May 20, 2018 at 10:54:04PM -0300, Marcelo Ricardo Leitner wrote:
>> >> >> On Sun, May 20, 2018 at 08:50:59PM -0400, Neil Horman wrote:
>> >> >>> On Sat, May 19, 2018 at 03:44:40PM +0800, Xin Long wrote:
>> >> >>>> This feature is actually already supported by sk->sk_reuse which can be
>> >> >>>> set by SO_REUSEADDR. But it's not working exactly as RFC6458 demands in
>> >> >>>> section 8.1.27, like:
>> >> >>>>
>> >> >>>>  - This option only supports one-to-one style SCTP sockets
>> >> >>>>  - This socket option must not be used after calling bind()
>> >> >>>>    or sctp_bindx().
>> >> >>>>
>> >> >>>> Besides, SCTP_REUSE_PORT sockopt should be provided for user's programs.
>> >> >>>> Otherwise, the programs with SCTP_REUSE_PORT from other systems will not
>> >> >>>> work in linux.
>> >> >>>>
>> >> >>>> This patch reuses sk->sk_reuse and works pretty much as SO_REUSEADDR,
>> >> >>>> just with some extra setup limitations that are neeeded when it is being
>> >> >>>> enabled.
>> >> >>>>
>> >> >>>> "It should be noted that the behavior of the socket-level socket option
>> >> >>>> to reuse ports and/or addresses for SCTP sockets is unspecified", so it
>> >> >>>> leaves SO_REUSEADDR as is for the compatibility.
>> >> >>>>
>> >> >>>> Signed-off-by: Xin Long <lucien.xin@gmail.com>
>> >> >>>> ---
>> >> >>>> include/uapi/linux/sctp.h |  1 +
>> >> >>>> net/sctp/socket.c         | 48 +++++++++++++++++++++++++++++++++++++++++++++++
>> >> >>>> 2 files changed, 49 insertions(+)
>> >> >>>>
>> >> >>> A few things:
>> >> >>>
>> >> >>> 1) I agree with Tom, this feature is a complete duplication of the SK_REUSEPORT
>> >> >>> socket option.  I understand that this is an implementation of the option in the
>> >> >>> RFC, but its definately a duplication of a feature, which makes several things
>> >> >>> really messy.
>> >> >>>
>> >> >>> 2) The overloading of the sk_reuse opeion is a bad idea, for several reasons.
>> >> >>> Chief among them is the behavioral interference between this patch and the
>> >> >>> SO_REUSEADDR socket level option, that also sets this feature.  If you set
>> >> >>> sk_reuse via SO_REUSEADDR, you will set the SCTP port reuse feature regardless
>> >> >>> of the bind or 1:1/1:m state of the socket.  Vice versa, if you set this socket
>> >> >>> option via the SCTP_PORT_REUSE option you will inadvertently turn on address
>> >> >>> reuse for the socket.  We can't do that.
>> >> >>
>> >> >> Given your comments, going a bit further here, one other big
>> >> >> implication is that a port would never be able to be considered to
>> >> >> fully meet SCTP standards regarding reuse because a rogue application
>> >> >> may always abuse of the socket level opt to gain access to the port.
>> >> >>
>> >> >> IOW, the patch allows the application to use such restrictions against
>> >> >> itself and nothing else, which undermines the patch idea.
>> >> >>
>> >> > Agreed.
>> >> >
>> >> >> I lack the knowledge on why the SCTP option was proposed in the RFC. I
>> >> >> guess they had a good reason to add the restriction on 1:1/1:m style.
>> >> >> Does the usage of the current imply in any risk to SCTP sockets? If
>> >> >> yes, that would give some grounds for going forward with the SCTP
>> >> >> option.
>> >> >>
>> >> > I'm also not privy to why the sctp option was proposed, though I expect that the
>> >> > lack of standardization of SO_REUSEPORT probably had something to do with it.
>> >> > As for the reasoning behind restriction to only 1:1 sockets, if I had to guess,
>> >> > I would say it likely because it creates ordering difficulty at the application
>> >> > level.
>> >> >
>> >> > CC-ing Michael Tuxen, who I believe had some input on this RFC.  Hopefully he
>> >> > can shed some light on this.
>> >> Dear all,
>> >>
>> >> the reason this was added is to have a specified way to allow a system to
>> >> behave like a client and server making use of the INIT collision.
>> >>
>> >> For 1-to-many style sockets you can do this by creating a socket, binding it,
>> >> calling listen on it and trying to connect to the peer.
>> >>
>> >> For 1-to-1 style sockets you need two sockets for it. One listener and one
>> >> you use to connect (and close it in case of failure, open a new one...).
>> >>
>> >> It was not clear if one can achieve this with SO_REUSEPORT and/or SO_REUSEADDR
>> >> on all platforms. We left that unspecified.
>> >>
>> >> I hope this makes the intention clearer.
>> >>
>> > I think it makes the intention clearer yes, but it unfortunately does nothing in
>> > my mind to clarify how the implementation should best handle the potential
>> > overlap in functionality.  What I see here is that we have two functional paths
>> > (the SO_REUSEPORT path and the SCTP_PORT_REUSE path), which may or may not
>> > (depending on the OS implementation achieve the same functional goal (allowing
>> > multiple sockets to share a port while allowing one socket to listen and the
>> > other connect to a remote peer).  If both implementations do the same thing on a
>> > given platform, we can either just alias one to another and be done, but if they
>> > don't then we either have to implement both paths, and ensure that the
>> > SO_REUSEPORT path is a no-op/error return for SCTP sockets, or that each path
>> > implements a distinct feature set that is cleaarly documented.
>> >
>> > That said, I think we may be in luck.  Looking at the connect and listen paths,
>> > it appears to me that:
>> >
>> > 1) Sockets ignore SO_REUSEPORT in the connect and listen paths (save for any
>> > autobinding) so it would appear that the intent of the SCTP rfc can be honored
>> > via SO_REUSEPORT on linux.
>> >
>> > 2) SO_REUSEPORT prevents changing state after a bind has occured, so we can honr
>> > that part of the SCTP RFC.
>> >
>> > The only missing part is the restriction that SCTP_REUSE_PORT has which is
>> > unaccounted for is that 1:M sockets aren't allowed to enable port reuse.
>> > However, I think the implication from Michaels description above is that port
>> > reuse on a 1:M socket is implicit because a single socket can connect and listen
>> > in that use case, rather than there being a danger to doing so.
>> >
>> > As such, I would propose that we implement this socket option by simply setting
>> > the sk->sk_reuseport field in the sock structure, and document the fact that
>> > linux does not restrict port reuse from 1:M sockets.
>> Note that, sk->sk_reuseport is not affecting linux SCTP socket at all now.
>> linux SCTP socket doesn't really have SO_REUSEADDR (sk->sk_reuse)
>> support, but use sk->sk_reuse as REUSE_PORT, (yes, it is confusing).
>> Pls refer to sctp_get_port_local().
>>
> No, its not used now, but if you do use it to do something specific to SCTP (via
> the SCTP_REUSE_PORT socket option), you risk aliasing SO_REUSEPORT behavior to
> it, and if it doesn't match what the RFC behavior mandates, thats a problem.
>
>> So I'm not sure using sk->sk_reuseport here means we will drop sk->sk_reuse
>> use in linux SCTP but use sk->sk_reuseport instead, or we will think that socket
>> enables 'port reuse' when either of them is set.
>>
> I don't think we would drop the behavior of sk_reuse here, why would we?  As far
> as I can see, the behavior of SO_REUSEADDR (not SO_REUSEPORT), isn't in
> question, is it?
>
>> Note some users may be already using SO_REUSEADDR to enable the 'port
>> reuse' in linux sctp socket. If we're changing to sk->sk_reuseport, we may face
>> a compatibility problem.
>>
> I don't see how the behavior of SO_REUSEADDR is in question here.  All I'm
> suggesting is that you simplify this patch so that the SCTP_REUSE_PORT socket
> option set sk_reuseport, as that option to my eyes conforms to the sctp rfc
> requirements.  Or am I' missing something?
No, I am :)
sk_reuseport seems more complicated than sk_reuse. I kind of mixed them.
I need to check more beofore continuing. Thanks.

>
> Neil
>
>>
>> >
>> > Thoughts?
>> > Neil
>> >
>>

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

* Re: [PATCH net-next] sctp: add support for SCTP_REUSE_PORT sockopt
  2018-05-23  7:04                 ` Xin Long
@ 2018-05-23  9:48                   ` Neil Horman
  -1 siblings, 0 replies; 33+ messages in thread
From: Neil Horman @ 2018-05-23  9:48 UTC (permalink / raw)
  To: Xin Long
  Cc: Michael Tuexen, Marcelo Ricardo Leitner, network dev, linux-sctp, davem

On Wed, May 23, 2018 at 03:04:53PM +0800, Xin Long wrote:
> On Tue, May 22, 2018 at 7:51 PM, Neil Horman <nhorman@tuxdriver.com> wrote:
> > On Tue, May 22, 2018 at 03:07:57PM +0800, Xin Long wrote:
> >> On Mon, May 21, 2018 at 9:48 PM, Neil Horman <nhorman@tuxdriver.com> wrote:
> >> > On Mon, May 21, 2018 at 02:16:56PM +0200, Michael Tuexen wrote:
> >> >> > On 21. May 2018, at 13:39, Neil Horman <nhorman@tuxdriver.com> wrote:
> >> >> >
> >> >> > On Sun, May 20, 2018 at 10:54:04PM -0300, Marcelo Ricardo Leitner wrote:
> >> >> >> On Sun, May 20, 2018 at 08:50:59PM -0400, Neil Horman wrote:
> >> >> >>> On Sat, May 19, 2018 at 03:44:40PM +0800, Xin Long wrote:
> >> >> >>>> This feature is actually already supported by sk->sk_reuse which can be
> >> >> >>>> set by SO_REUSEADDR. But it's not working exactly as RFC6458 demands in
> >> >> >>>> section 8.1.27, like:
> >> >> >>>>
> >> >> >>>>  - This option only supports one-to-one style SCTP sockets
> >> >> >>>>  - This socket option must not be used after calling bind()
> >> >> >>>>    or sctp_bindx().
> >> >> >>>>
> >> >> >>>> Besides, SCTP_REUSE_PORT sockopt should be provided for user's programs.
> >> >> >>>> Otherwise, the programs with SCTP_REUSE_PORT from other systems will not
> >> >> >>>> work in linux.
> >> >> >>>>
> >> >> >>>> This patch reuses sk->sk_reuse and works pretty much as SO_REUSEADDR,
> >> >> >>>> just with some extra setup limitations that are neeeded when it is being
> >> >> >>>> enabled.
> >> >> >>>>
> >> >> >>>> "It should be noted that the behavior of the socket-level socket option
> >> >> >>>> to reuse ports and/or addresses for SCTP sockets is unspecified", so it
> >> >> >>>> leaves SO_REUSEADDR as is for the compatibility.
> >> >> >>>>
> >> >> >>>> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> >> >> >>>> ---
> >> >> >>>> include/uapi/linux/sctp.h |  1 +
> >> >> >>>> net/sctp/socket.c         | 48 +++++++++++++++++++++++++++++++++++++++++++++++
> >> >> >>>> 2 files changed, 49 insertions(+)
> >> >> >>>>
> >> >> >>> A few things:
> >> >> >>>
> >> >> >>> 1) I agree with Tom, this feature is a complete duplication of the SK_REUSEPORT
> >> >> >>> socket option.  I understand that this is an implementation of the option in the
> >> >> >>> RFC, but its definately a duplication of a feature, which makes several things
> >> >> >>> really messy.
> >> >> >>>
> >> >> >>> 2) The overloading of the sk_reuse opeion is a bad idea, for several reasons.
> >> >> >>> Chief among them is the behavioral interference between this patch and the
> >> >> >>> SO_REUSEADDR socket level option, that also sets this feature.  If you set
> >> >> >>> sk_reuse via SO_REUSEADDR, you will set the SCTP port reuse feature regardless
> >> >> >>> of the bind or 1:1/1:m state of the socket.  Vice versa, if you set this socket
> >> >> >>> option via the SCTP_PORT_REUSE option you will inadvertently turn on address
> >> >> >>> reuse for the socket.  We can't do that.
> >> >> >>
> >> >> >> Given your comments, going a bit further here, one other big
> >> >> >> implication is that a port would never be able to be considered to
> >> >> >> fully meet SCTP standards regarding reuse because a rogue application
> >> >> >> may always abuse of the socket level opt to gain access to the port.
> >> >> >>
> >> >> >> IOW, the patch allows the application to use such restrictions against
> >> >> >> itself and nothing else, which undermines the patch idea.
> >> >> >>
> >> >> > Agreed.
> >> >> >
> >> >> >> I lack the knowledge on why the SCTP option was proposed in the RFC. I
> >> >> >> guess they had a good reason to add the restriction on 1:1/1:m style.
> >> >> >> Does the usage of the current imply in any risk to SCTP sockets? If
> >> >> >> yes, that would give some grounds for going forward with the SCTP
> >> >> >> option.
> >> >> >>
> >> >> > I'm also not privy to why the sctp option was proposed, though I expect that the
> >> >> > lack of standardization of SO_REUSEPORT probably had something to do with it.
> >> >> > As for the reasoning behind restriction to only 1:1 sockets, if I had to guess,
> >> >> > I would say it likely because it creates ordering difficulty at the application
> >> >> > level.
> >> >> >
> >> >> > CC-ing Michael Tuxen, who I believe had some input on this RFC.  Hopefully he
> >> >> > can shed some light on this.
> >> >> Dear all,
> >> >>
> >> >> the reason this was added is to have a specified way to allow a system to
> >> >> behave like a client and server making use of the INIT collision.
> >> >>
> >> >> For 1-to-many style sockets you can do this by creating a socket, binding it,
> >> >> calling listen on it and trying to connect to the peer.
> >> >>
> >> >> For 1-to-1 style sockets you need two sockets for it. One listener and one
> >> >> you use to connect (and close it in case of failure, open a new one...).
> >> >>
> >> >> It was not clear if one can achieve this with SO_REUSEPORT and/or SO_REUSEADDR
> >> >> on all platforms. We left that unspecified.
> >> >>
> >> >> I hope this makes the intention clearer.
> >> >>
> >> > I think it makes the intention clearer yes, but it unfortunately does nothing in
> >> > my mind to clarify how the implementation should best handle the potential
> >> > overlap in functionality.  What I see here is that we have two functional paths
> >> > (the SO_REUSEPORT path and the SCTP_PORT_REUSE path), which may or may not
> >> > (depending on the OS implementation achieve the same functional goal (allowing
> >> > multiple sockets to share a port while allowing one socket to listen and the
> >> > other connect to a remote peer).  If both implementations do the same thing on a
> >> > given platform, we can either just alias one to another and be done, but if they
> >> > don't then we either have to implement both paths, and ensure that the
> >> > SO_REUSEPORT path is a no-op/error return for SCTP sockets, or that each path
> >> > implements a distinct feature set that is cleaarly documented.
> >> >
> >> > That said, I think we may be in luck.  Looking at the connect and listen paths,
> >> > it appears to me that:
> >> >
> >> > 1) Sockets ignore SO_REUSEPORT in the connect and listen paths (save for any
> >> > autobinding) so it would appear that the intent of the SCTP rfc can be honored
> >> > via SO_REUSEPORT on linux.
> >> >
> >> > 2) SO_REUSEPORT prevents changing state after a bind has occured, so we can honr
> >> > that part of the SCTP RFC.
> >> >
> >> > The only missing part is the restriction that SCTP_REUSE_PORT has which is
> >> > unaccounted for is that 1:M sockets aren't allowed to enable port reuse.
> >> > However, I think the implication from Michaels description above is that port
> >> > reuse on a 1:M socket is implicit because a single socket can connect and listen
> >> > in that use case, rather than there being a danger to doing so.
> >> >
> >> > As such, I would propose that we implement this socket option by simply setting
> >> > the sk->sk_reuseport field in the sock structure, and document the fact that
> >> > linux does not restrict port reuse from 1:M sockets.
> >> Note that, sk->sk_reuseport is not affecting linux SCTP socket at all now.
> >> linux SCTP socket doesn't really have SO_REUSEADDR (sk->sk_reuse)
> >> support, but use sk->sk_reuse as REUSE_PORT, (yes, it is confusing).
> >> Pls refer to sctp_get_port_local().
> >>
> > No, its not used now, but if you do use it to do something specific to SCTP (via
> > the SCTP_REUSE_PORT socket option), you risk aliasing SO_REUSEPORT behavior to
> > it, and if it doesn't match what the RFC behavior mandates, thats a problem.
> >
> >> So I'm not sure using sk->sk_reuseport here means we will drop sk->sk_reuse
> >> use in linux SCTP but use sk->sk_reuseport instead, or we will think that socket
> >> enables 'port reuse' when either of them is set.
> >>
> > I don't think we would drop the behavior of sk_reuse here, why would we?  As far
> > as I can see, the behavior of SO_REUSEADDR (not SO_REUSEPORT), isn't in
> > question, is it?
> >
> >> Note some users may be already using SO_REUSEADDR to enable the 'port
> >> reuse' in linux sctp socket. If we're changing to sk->sk_reuseport, we may face
> >> a compatibility problem.
> >>
> > I don't see how the behavior of SO_REUSEADDR is in question here.  All I'm
> > suggesting is that you simplify this patch so that the SCTP_REUSE_PORT socket
> > option set sk_reuseport, as that option to my eyes conforms to the sctp rfc
> > requirements.  Or am I' missing something?
> No, I am :)
> sk_reuseport seems more complicated than sk_reuse. I kind of mixed them.
> I need to check more beofore continuing. Thanks.
> 
Thank you!
Neil

> >
> > Neil
> >
> >>
> >> >
> >> > Thoughts?
> >> > Neil
> >> >
> >>
> 

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

* Re: [PATCH net-next] sctp: add support for SCTP_REUSE_PORT sockopt
@ 2018-05-23  9:48                   ` Neil Horman
  0 siblings, 0 replies; 33+ messages in thread
From: Neil Horman @ 2018-05-23  9:48 UTC (permalink / raw)
  To: Xin Long
  Cc: Michael Tuexen, Marcelo Ricardo Leitner, network dev, linux-sctp, davem

On Wed, May 23, 2018 at 03:04:53PM +0800, Xin Long wrote:
> On Tue, May 22, 2018 at 7:51 PM, Neil Horman <nhorman@tuxdriver.com> wrote:
> > On Tue, May 22, 2018 at 03:07:57PM +0800, Xin Long wrote:
> >> On Mon, May 21, 2018 at 9:48 PM, Neil Horman <nhorman@tuxdriver.com> wrote:
> >> > On Mon, May 21, 2018 at 02:16:56PM +0200, Michael Tuexen wrote:
> >> >> > On 21. May 2018, at 13:39, Neil Horman <nhorman@tuxdriver.com> wrote:
> >> >> >
> >> >> > On Sun, May 20, 2018 at 10:54:04PM -0300, Marcelo Ricardo Leitner wrote:
> >> >> >> On Sun, May 20, 2018 at 08:50:59PM -0400, Neil Horman wrote:
> >> >> >>> On Sat, May 19, 2018 at 03:44:40PM +0800, Xin Long wrote:
> >> >> >>>> This feature is actually already supported by sk->sk_reuse which can be
> >> >> >>>> set by SO_REUSEADDR. But it's not working exactly as RFC6458 demands in
> >> >> >>>> section 8.1.27, like:
> >> >> >>>>
> >> >> >>>>  - This option only supports one-to-one style SCTP sockets
> >> >> >>>>  - This socket option must not be used after calling bind()
> >> >> >>>>    or sctp_bindx().
> >> >> >>>>
> >> >> >>>> Besides, SCTP_REUSE_PORT sockopt should be provided for user's programs.
> >> >> >>>> Otherwise, the programs with SCTP_REUSE_PORT from other systems will not
> >> >> >>>> work in linux.
> >> >> >>>>
> >> >> >>>> This patch reuses sk->sk_reuse and works pretty much as SO_REUSEADDR,
> >> >> >>>> just with some extra setup limitations that are neeeded when it is being
> >> >> >>>> enabled.
> >> >> >>>>
> >> >> >>>> "It should be noted that the behavior of the socket-level socket option
> >> >> >>>> to reuse ports and/or addresses for SCTP sockets is unspecified", so it
> >> >> >>>> leaves SO_REUSEADDR as is for the compatibility.
> >> >> >>>>
> >> >> >>>> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> >> >> >>>> ---
> >> >> >>>> include/uapi/linux/sctp.h |  1 +
> >> >> >>>> net/sctp/socket.c         | 48 +++++++++++++++++++++++++++++++++++++++++++++++
> >> >> >>>> 2 files changed, 49 insertions(+)
> >> >> >>>>
> >> >> >>> A few things:
> >> >> >>>
> >> >> >>> 1) I agree with Tom, this feature is a complete duplication of the SK_REUSEPORT
> >> >> >>> socket option.  I understand that this is an implementation of the option in the
> >> >> >>> RFC, but its definately a duplication of a feature, which makes several things
> >> >> >>> really messy.
> >> >> >>>
> >> >> >>> 2) The overloading of the sk_reuse opeion is a bad idea, for several reasons.
> >> >> >>> Chief among them is the behavioral interference between this patch and the
> >> >> >>> SO_REUSEADDR socket level option, that also sets this feature.  If you set
> >> >> >>> sk_reuse via SO_REUSEADDR, you will set the SCTP port reuse feature regardless
> >> >> >>> of the bind or 1:1/1:m state of the socket.  Vice versa, if you set this socket
> >> >> >>> option via the SCTP_PORT_REUSE option you will inadvertently turn on address
> >> >> >>> reuse for the socket.  We can't do that.
> >> >> >>
> >> >> >> Given your comments, going a bit further here, one other big
> >> >> >> implication is that a port would never be able to be considered to
> >> >> >> fully meet SCTP standards regarding reuse because a rogue application
> >> >> >> may always abuse of the socket level opt to gain access to the port.
> >> >> >>
> >> >> >> IOW, the patch allows the application to use such restrictions against
> >> >> >> itself and nothing else, which undermines the patch idea.
> >> >> >>
> >> >> > Agreed.
> >> >> >
> >> >> >> I lack the knowledge on why the SCTP option was proposed in the RFC. I
> >> >> >> guess they had a good reason to add the restriction on 1:1/1:m style.
> >> >> >> Does the usage of the current imply in any risk to SCTP sockets? If
> >> >> >> yes, that would give some grounds for going forward with the SCTP
> >> >> >> option.
> >> >> >>
> >> >> > I'm also not privy to why the sctp option was proposed, though I expect that the
> >> >> > lack of standardization of SO_REUSEPORT probably had something to do with it.
> >> >> > As for the reasoning behind restriction to only 1:1 sockets, if I had to guess,
> >> >> > I would say it likely because it creates ordering difficulty at the application
> >> >> > level.
> >> >> >
> >> >> > CC-ing Michael Tuxen, who I believe had some input on this RFC.  Hopefully he
> >> >> > can shed some light on this.
> >> >> Dear all,
> >> >>
> >> >> the reason this was added is to have a specified way to allow a system to
> >> >> behave like a client and server making use of the INIT collision.
> >> >>
> >> >> For 1-to-many style sockets you can do this by creating a socket, binding it,
> >> >> calling listen on it and trying to connect to the peer.
> >> >>
> >> >> For 1-to-1 style sockets you need two sockets for it. One listener and one
> >> >> you use to connect (and close it in case of failure, open a new one...).
> >> >>
> >> >> It was not clear if one can achieve this with SO_REUSEPORT and/or SO_REUSEADDR
> >> >> on all platforms. We left that unspecified.
> >> >>
> >> >> I hope this makes the intention clearer.
> >> >>
> >> > I think it makes the intention clearer yes, but it unfortunately does nothing in
> >> > my mind to clarify how the implementation should best handle the potential
> >> > overlap in functionality.  What I see here is that we have two functional paths
> >> > (the SO_REUSEPORT path and the SCTP_PORT_REUSE path), which may or may not
> >> > (depending on the OS implementation achieve the same functional goal (allowing
> >> > multiple sockets to share a port while allowing one socket to listen and the
> >> > other connect to a remote peer).  If both implementations do the same thing on a
> >> > given platform, we can either just alias one to another and be done, but if they
> >> > don't then we either have to implement both paths, and ensure that the
> >> > SO_REUSEPORT path is a no-op/error return for SCTP sockets, or that each path
> >> > implements a distinct feature set that is cleaarly documented.
> >> >
> >> > That said, I think we may be in luck.  Looking at the connect and listen paths,
> >> > it appears to me that:
> >> >
> >> > 1) Sockets ignore SO_REUSEPORT in the connect and listen paths (save for any
> >> > autobinding) so it would appear that the intent of the SCTP rfc can be honored
> >> > via SO_REUSEPORT on linux.
> >> >
> >> > 2) SO_REUSEPORT prevents changing state after a bind has occured, so we can honr
> >> > that part of the SCTP RFC.
> >> >
> >> > The only missing part is the restriction that SCTP_REUSE_PORT has which is
> >> > unaccounted for is that 1:M sockets aren't allowed to enable port reuse.
> >> > However, I think the implication from Michaels description above is that port
> >> > reuse on a 1:M socket is implicit because a single socket can connect and listen
> >> > in that use case, rather than there being a danger to doing so.
> >> >
> >> > As such, I would propose that we implement this socket option by simply setting
> >> > the sk->sk_reuseport field in the sock structure, and document the fact that
> >> > linux does not restrict port reuse from 1:M sockets.
> >> Note that, sk->sk_reuseport is not affecting linux SCTP socket at all now.
> >> linux SCTP socket doesn't really have SO_REUSEADDR (sk->sk_reuse)
> >> support, but use sk->sk_reuse as REUSE_PORT, (yes, it is confusing).
> >> Pls refer to sctp_get_port_local().
> >>
> > No, its not used now, but if you do use it to do something specific to SCTP (via
> > the SCTP_REUSE_PORT socket option), you risk aliasing SO_REUSEPORT behavior to
> > it, and if it doesn't match what the RFC behavior mandates, thats a problem.
> >
> >> So I'm not sure using sk->sk_reuseport here means we will drop sk->sk_reuse
> >> use in linux SCTP but use sk->sk_reuseport instead, or we will think that socket
> >> enables 'port reuse' when either of them is set.
> >>
> > I don't think we would drop the behavior of sk_reuse here, why would we?  As far
> > as I can see, the behavior of SO_REUSEADDR (not SO_REUSEPORT), isn't in
> > question, is it?
> >
> >> Note some users may be already using SO_REUSEADDR to enable the 'port
> >> reuse' in linux sctp socket. If we're changing to sk->sk_reuseport, we may face
> >> a compatibility problem.
> >>
> > I don't see how the behavior of SO_REUSEADDR is in question here.  All I'm
> > suggesting is that you simplify this patch so that the SCTP_REUSE_PORT socket
> > option set sk_reuseport, as that option to my eyes conforms to the sctp rfc
> > requirements.  Or am I' missing something?
> No, I am :)
> sk_reuseport seems more complicated than sk_reuse. I kind of mixed them.
> I need to check more beofore continuing. Thanks.
> 
Thank you!
Neil

> >
> > Neil
> >
> >>
> >> >
> >> > Thoughts?
> >> > Neil
> >> >
> >>
> 

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

end of thread, other threads:[~2018-05-23  9:49 UTC | newest]

Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-19  7:44 [PATCH net-next] sctp: add support for SCTP_REUSE_PORT sockopt Xin Long
2018-05-19  7:44 ` Xin Long
2018-05-19 20:26 ` Marcelo Ricardo Leitner
2018-05-19 20:26   ` Marcelo Ricardo Leitner
2018-05-20 19:44 ` Tom Herbert
2018-05-20 19:44   ` Tom Herbert
2018-05-21  0:50 ` Neil Horman
2018-05-21  0:50   ` Neil Horman
2018-05-21  1:54   ` Marcelo Ricardo Leitner
2018-05-21  1:54     ` Marcelo Ricardo Leitner
2018-05-21  3:59     ` Tom Herbert
2018-05-21  3:59       ` Tom Herbert
2018-05-21  7:46     ` Xin Long
2018-05-21  7:46       ` Xin Long
2018-05-21 11:39     ` Neil Horman
2018-05-21 11:39       ` Neil Horman
2018-05-21 12:16       ` Michael Tuexen
2018-05-21 12:16         ` Michael Tuexen
2018-05-21 13:48         ` Neil Horman
2018-05-21 13:48           ` Neil Horman
2018-05-21 14:09           ` Michael Tuexen
2018-05-21 14:09             ` Michael Tuexen
2018-05-21 15:51             ` Marcelo Ricardo Leitner
2018-05-21 15:51               ` Marcelo Ricardo Leitner
2018-05-22 15:36               ` David Laight
2018-05-22  7:07           ` Xin Long
2018-05-22  7:07             ` Xin Long
2018-05-22 11:51             ` Neil Horman
2018-05-22 11:51               ` Neil Horman
2018-05-23  7:04               ` Xin Long
2018-05-23  7:04                 ` Xin Long
2018-05-23  9:48                 ` Neil Horman
2018-05-23  9:48                   ` Neil Horman

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.