Subject: Re: [PATCHv3 net-next 4/4] sctp: implement sender-side procedures for Add Incoming/Outgoing Streams Request Parameter

From: Marcelo Ricardo Leitner <[email-masked]>
Date: Thu, 19 Jan 2017 20:15:34 -0200

On Fri, Jan 20, 2017 at 01:19:14AM +0800, Xin Long wrote:
> This patch is to implement Sender-Side Procedures for the Add
> Outgoing and Incoming Streams Request Parameter described in
> rfc6525 section 5.1.5-5.1.6.
> 
> It is also to add sockopt SCTP_ADD_STREAMS in rfc6525 section
> 6.3.4 for users.
> 
> Signed-off-by: Xin Long <[email-masked]>
> ---
>  include/net/sctp/sctp.h   |  2 +
>  include/uapi/linux/sctp.h |  7 ++++
>  net/sctp/socket.c         | 29 ++++++++++++++
>  net/sctp/stream.c         | 99 +++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 137 insertions(+)
> 
> diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h
> index b93820f..68ee1a6 100644
> --- a/include/net/sctp/sctp.h
> +++ b/include/net/sctp/sctp.h
> @@ -199,6 +199,8 @@ int sctp_offload_init(void);
>  int sctp_send_reset_streams(struct sctp_association *asoc,
>  			    struct sctp_reset_streams *params);
>  int sctp_send_reset_assoc(struct sctp_association *asoc);
> +int sctp_send_add_streams(struct sctp_association *asoc,
> +			  struct sctp_add_streams *params);
>  
>  /*
>   * Module global variables
> diff --git a/include/uapi/linux/sctp.h b/include/uapi/linux/sctp.h
> index c0bd8c3..a91a9cc 100644
> --- a/include/uapi/linux/sctp.h
> +++ b/include/uapi/linux/sctp.h
> @@ -118,6 +118,7 @@ typedef __s32 sctp_assoc_t;
>  #define SCTP_ENABLE_STREAM_RESET	118
>  #define SCTP_RESET_STREAMS	119
>  #define SCTP_RESET_ASSOC	120
> +#define SCTP_ADD_STREAMS	121
>  
>  /* PR-SCTP policies */
>  #define SCTP_PR_SCTP_NONE	0x0000
> @@ -1027,4 +1028,10 @@ struct sctp_reset_streams {
>  	uint16_t srs_stream_list[];	/* list if srs_num_streams is not 0 */
>  };
>  
> +struct sctp_add_streams {
> +	sctp_assoc_t sas_assoc_id;
> +	uint16_t sas_instrms;
> +	uint16_t sas_outstrms;
> +};
> +
>  #endif /* _UAPI_SCTP_H */
> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> index 2c5c9ca..ae0a99e 100644
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@ -3838,6 +3838,32 @@ static int sctp_setsockopt_reset_assoc(struct sock *sk,
>  	return retval;
>  }
>  
> +static int sctp_setsockopt_add_streams(struct sock *sk,
> +				       char __user *optval,
> +				       unsigned int optlen)
> +{
> +	struct sctp_association *asoc;
> +	struct sctp_add_streams params;
> +	int retval = -EINVAL;
> +
> +	if (optlen != sizeof(params))
> +		goto out;
> +
> +	if (copy_from_user(&params, optval, optlen)) {
> +		retval = -EFAULT;
> +		goto out;
> +	}
> +
> +	asoc = sctp_id2assoc(sk, params.sas_assoc_id);
> +	if (!asoc)
> +		goto out;
> +
> +	retval = sctp_send_add_streams(asoc, &params);
> +
> +out:
> +	return retval;
> +}
> +
>  /* API 6.2 setsockopt(), getsockopt()
>   *
>   * Applications use setsockopt() and getsockopt() to set or retrieve
> @@ -4013,6 +4039,9 @@ static int sctp_setsockopt(struct sock *sk, int level, int optname,
>  	case SCTP_RESET_ASSOC:
>  		retval = sctp_setsockopt_reset_assoc(sk, optval, optlen);
>  		break;
> +	case SCTP_ADD_STREAMS:
> +		retval = sctp_setsockopt_add_streams(sk, optval, optlen);
> +		break;
>  	default:
>  		retval = -ENOPROTOOPT;
>  		break;
> diff --git a/net/sctp/stream.c b/net/sctp/stream.c
> index b368191..ba41837 100644
> --- a/net/sctp/stream.c
> +++ b/net/sctp/stream.c
> @@ -195,3 +195,102 @@ int sctp_send_reset_assoc(struct sctp_association *asoc)
>  
>  	return retval;
>  }
> +
> +int sctp_send_add_streams(struct sctp_association *asoc,
> +			  struct sctp_add_streams *params)
> +{
> +	struct sctp_stream *stream = asoc->stream;
> +	struct sctp_chunk *chunk = NULL;
> +	int retval = -EINVAL;
> +	__u16 out, in;
> +
> +	if (!asoc->peer.reconf_capable ||
> +	    !(asoc->strreset_enable & SCTP_ENABLE_CHANGE_ASSOC_REQ)) {
> +		retval = -ENOPROTOOPT;
> +		goto out;
> +	}
> +
> +	if (asoc->strreset_outstanding) {
> +		retval = -EINPROGRESS;
> +		goto out;
> +	}
> +
> +	out = params->sas_outstrms;
> +	in  = params->sas_instrms;
> +
> +	if (!out && !in)
> +		goto out;
> +
> +	if (out) {
> +		__u16 nums = stream->outcnt + out;
> +
> +		/* Check for overflow, can't use nums here */
> +		if (stream->outcnt + out > SCTP_MAX_STREAM)
> +			goto out;
> +
> +		/* Use ksize to check if stream array really needs to realloc */
> +		if (ksize(stream->out) / sizeof(*stream->out) < nums) {
> +			struct sctp_stream_out *streamout;
> +
> +			streamout = kcalloc(nums, sizeof(*streamout),
> +					    GFP_KERNEL);
> +			if (!streamout) {
> +				retval = -ENOMEM;
> +				goto out;
> +			}
> +
> +			memcpy(streamout, stream->out,
> +			       sizeof(*streamout) * stream->outcnt);
> +
> +			kfree(stream->out);
> +			stream->out = streamout;
> +		}
> +
> +		stream->outcnt = nums;
> +	}
> +
> +	if (in) {
> +		__u16 nums = stream->incnt + in;
> +
> +		if (stream->incnt + in > SCTP_MAX_STREAM)
> +			goto out;
> +
> +		if (ksize(stream->in) / sizeof(*stream->in) < nums) {
> +			struct sctp_stream_in *streamin;
> +
> +			streamin = kcalloc(nums, sizeof(*streamin),
> +					   GFP_KERNEL);
> +			if (!streamin) {
> +				retval = -ENOMEM;
> +				goto out;
> +			}
> +
> +			memcpy(streamin, stream->in,
> +			       sizeof(*streamin) * stream->incnt);
> +
> +			kfree(stream->in);
> +			stream->in = streamin;
> +		}
> +
> +		stream->incnt = nums;
> +	}
> +
> +	chunk = sctp_make_strreset_addstrm(asoc, out, in);
> +	if (!chunk) {
> +		retval = -ENOMEM;
> +		goto out;
> +	}
> +
> +	asoc->strreset_outstanding = !!out + !!in;
> +	asoc->strreset_chunk = chunk;
> +	sctp_chunk_hold(asoc->strreset_chunk);
> +
> +	retval = sctp_send_reconf(asoc, chunk);
> +	if (retval) {
> +		sctp_chunk_put(asoc->strreset_chunk);
> +		asoc->strreset_chunk = NULL;

Similar comment on recovery steps here. If we are assuming we failed to
send the request, we should also revert the changes above.
The memory reallocation seems fine as the streams defaults to closed,
but shouldn't incnt and outcnt be reset?

> +	}
> +
> +out:
> +	return retval;
> +}
> -- 
> 2.1.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to [email-masked]
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>