* [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 ` (2 more replies) 0 siblings, 3 replies; 17+ 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] 17+ messages in thread
* Re: [PATCH net-next] sctp: add support for SCTP_REUSE_PORT sockopt 2018-05-19 7:44 [PATCH net-next] sctp: add support for SCTP_REUSE_PORT sockopt Xin Long @ 2018-05-19 20:26 ` Marcelo Ricardo Leitner 2018-05-20 19:44 ` Tom Herbert 2018-05-21 0:50 ` Neil Horman 2 siblings, 0 replies; 17+ 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] 17+ messages in thread
* Re: [PATCH net-next] sctp: add support for SCTP_REUSE_PORT sockopt 2018-05-19 7:44 [PATCH net-next] sctp: add support for SCTP_REUSE_PORT sockopt Xin Long 2018-05-19 20:26 ` Marcelo Ricardo Leitner @ 2018-05-20 19:44 ` Tom Herbert 2018-05-21 0:50 ` Neil Horman 2 siblings, 0 replies; 17+ 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] 17+ messages in thread
* Re: [PATCH net-next] sctp: add support for SCTP_REUSE_PORT sockopt 2018-05-19 7:44 [PATCH net-next] sctp: add support for SCTP_REUSE_PORT sockopt Xin Long 2018-05-19 20:26 ` Marcelo Ricardo Leitner 2018-05-20 19:44 ` Tom Herbert @ 2018-05-21 0:50 ` Neil Horman 2018-05-21 1:54 ` Marcelo Ricardo Leitner 2 siblings, 1 reply; 17+ 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] 17+ 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 2018-05-21 3:59 ` Tom Herbert ` (2 more replies) 0 siblings, 3 replies; 17+ 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] 17+ 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 2018-05-21 7:46 ` Xin Long 2018-05-21 11:39 ` Neil Horman 2 siblings, 0 replies; 17+ 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] 17+ 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 @ 2018-05-21 7:46 ` Xin Long 2018-05-21 11:39 ` Neil Horman 2 siblings, 0 replies; 17+ 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] 17+ 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 2018-05-21 7:46 ` Xin Long @ 2018-05-21 11:39 ` Neil Horman 2018-05-21 12:16 ` Michael Tuexen 2 siblings, 1 reply; 17+ 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] 17+ 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 2018-05-21 13:48 ` Neil Horman 0 siblings, 1 reply; 17+ 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] 17+ 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 2018-05-21 14:09 ` Michael Tuexen 2018-05-22 7:07 ` Xin Long 0 siblings, 2 replies; 17+ 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] 17+ 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 2018-05-21 15:51 ` Marcelo Ricardo Leitner 2018-05-22 7:07 ` Xin Long 1 sibling, 1 reply; 17+ 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] 17+ 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 2018-05-22 15:36 ` David Laight 0 siblings, 1 reply; 17+ 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] 17+ 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 0 siblings, 0 replies; 17+ 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] 17+ 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 @ 2018-05-22 7:07 ` Xin Long 2018-05-22 11:51 ` Neil Horman 1 sibling, 1 reply; 17+ 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] 17+ 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 2018-05-23 7:04 ` Xin Long 0 siblings, 1 reply; 17+ 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] 17+ 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 2018-05-23 9:48 ` Neil Horman 0 siblings, 1 reply; 17+ 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] 17+ 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 0 siblings, 0 replies; 17+ 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] 17+ messages in thread
end of thread, other threads:[~2018-05-23 9:49 UTC | newest] Thread overview: 17+ 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 20:26 ` Marcelo Ricardo Leitner 2018-05-20 19:44 ` Tom Herbert 2018-05-21 0:50 ` Neil Horman 2018-05-21 1:54 ` Marcelo Ricardo Leitner 2018-05-21 3:59 ` Tom Herbert 2018-05-21 7:46 ` Xin Long 2018-05-21 11:39 ` Neil Horman 2018-05-21 12:16 ` Michael Tuexen 2018-05-21 13:48 ` Neil Horman 2018-05-21 14:09 ` Michael Tuexen 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 11:51 ` Neil Horman 2018-05-23 7:04 ` Xin Long 2018-05-23 9:48 ` Neil Horman
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).