Re: [PATCH net-next] sctp: add support for SCTP_REUSE_PORT sockopt

[Xin Long <lucien.xin@gmail.com>]

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?