* Re: [PATCHv3 net-next 4/4] sctp: implement sender-side procedures for Add Incoming/Outgoing Streams Request Parameter
2017-01-19 17:19 ` [PATCHv3 net-next 4/4] sctp: implement sender-side procedures for Add Incoming/Outgoing Streams Request Parameter Xin Long
@ 2017-01-19 20:17 ` Neil Horman
2017-01-19 22:18 ` Marcelo Ricardo Leitner
2017-01-19 21:47 ` Marcelo Ricardo Leitner
` (2 subsequent siblings)
3 siblings, 1 reply; 33+ messages in thread
From: Neil Horman @ 2017-01-19 20:17 UTC (permalink / raw)
To: Xin Long
Cc: network dev, linux-sctp, Marcelo Ricardo Leitner, Vlad Yasevich,
davem
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 <lucien.xin@gmail.com>
> ---
> 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)
> +{
you are going to need to provide some locking here or in sctp_send_add_streams.
By replacing the in/out streams pointer you run the risk of multiple callers
modifying the pointers in parallel, or in the sctp state machine reading it
while you do.
Neil
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [PATCHv3 net-next 4/4] sctp: implement sender-side procedures for Add Incoming/Outgoing Streams Request Parameter
2017-01-19 20:17 ` Neil Horman
@ 2017-01-19 22:18 ` Marcelo Ricardo Leitner
2017-01-23 14:50 ` Neil Horman
0 siblings, 1 reply; 33+ messages in thread
From: Marcelo Ricardo Leitner @ 2017-01-19 22:18 UTC (permalink / raw)
To: Neil Horman; +Cc: Xin Long, network dev, linux-sctp, Vlad Yasevich, davem
On Thu, Jan 19, 2017 at 03:17:18PM -0500, Neil Horman wrote:
> 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 <lucien.xin@gmail.com>
> > ---
> > 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)
> > +{
> you are going to need to provide some locking here or in sctp_send_add_streams.
> By replacing the in/out streams pointer you run the risk of multiple callers
> modifying the pointers in parallel, or in the sctp state machine reading it
> while you do.
There can't be multiple callers here because the socket is locked before
calling this function, in sctp_setsockopt().
I also don't see how a BH handler, timers or sleeping call (recvmsg)
could trip on this.
Marcelo
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [PATCHv3 net-next 4/4] sctp: implement sender-side procedures for Add Incoming/Outgoing Streams Request Parameter
2017-01-19 22:18 ` Marcelo Ricardo Leitner
@ 2017-01-23 14:50 ` Neil Horman
0 siblings, 0 replies; 33+ messages in thread
From: Neil Horman @ 2017-01-23 14:50 UTC (permalink / raw)
To: Marcelo Ricardo Leitner
Cc: Xin Long, network dev, linux-sctp, Vlad Yasevich, davem
On Thu, Jan 19, 2017 at 08:18:13PM -0200, Marcelo Ricardo Leitner wrote:
> On Thu, Jan 19, 2017 at 03:17:18PM -0500, Neil Horman wrote:
> > 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 <lucien.xin@gmail.com>
> > > ---
> > > 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)
> > > +{
> > you are going to need to provide some locking here or in sctp_send_add_streams.
> > By replacing the in/out streams pointer you run the risk of multiple callers
> > modifying the pointers in parallel, or in the sctp state machine reading it
> > while you do.
>
> There can't be multiple callers here because the socket is locked before
> calling this function, in sctp_setsockopt().
> I also don't see how a BH handler, timers or sleeping call (recvmsg)
> could trip on this.
You're right, I missed the socket lock in sctp_setsockopt, that covers us both
from multiple processes and bottom halves.
>
> Marcelo
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCHv3 net-next 4/4] sctp: implement sender-side procedures for Add Incoming/Outgoing Streams Request Parameter
2017-01-19 17:19 ` [PATCHv3 net-next 4/4] sctp: implement sender-side procedures for Add Incoming/Outgoing Streams Request Parameter Xin Long
2017-01-19 20:17 ` Neil Horman
@ 2017-01-19 21:47 ` Marcelo Ricardo Leitner
2017-01-20 8:56 ` Xin Long
2017-01-19 22:15 ` Marcelo Ricardo Leitner
2017-01-23 11:25 ` David Laight
3 siblings, 1 reply; 33+ messages in thread
From: Marcelo Ricardo Leitner @ 2017-01-19 21:47 UTC (permalink / raw)
To: Xin Long; +Cc: network dev, linux-sctp, Neil Horman, Vlad Yasevich, davem
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 <lucien.xin@gmail.com>
> ---
> 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(¶ms, optval, optlen)) {
> + retval = -EFAULT;
> + goto out;
> + }
> +
> + asoc = sctp_id2assoc(sk, params.sas_assoc_id);
> + if (!asoc)
> + goto out;
> +
> + retval = sctp_send_add_streams(asoc, ¶ms);
> +
> +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;
You should do these overflow checks before doing actual work,
preferreably at [@] mark above.
Here it's fine, but when [*] below run, it may be too late.
> +
> + /* 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;
> + }
> +
> +out:
> + return retval;
> +}
> --
> 2.1.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [PATCHv3 net-next 4/4] sctp: implement sender-side procedures for Add Incoming/Outgoing Streams Request Parameter
2017-01-19 21:47 ` Marcelo Ricardo Leitner
@ 2017-01-20 8:56 ` Xin Long
2017-01-20 11:43 ` Marcelo Ricardo Leitner
0 siblings, 1 reply; 33+ messages in thread
From: Xin Long @ 2017-01-20 8:56 UTC (permalink / raw)
To: Marcelo Ricardo Leitner
Cc: network dev, linux-sctp, Neil Horman, Vlad Yasevich, davem
On Fri, Jan 20, 2017 at 5:47 AM, Marcelo Ricardo Leitner
<marcelo.leitner@gmail.com> wrote:
> 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 <lucien.xin@gmail.com>
>> ---
>> 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(¶ms, optval, optlen)) {
>> + retval = -EFAULT;
>> + goto out;
>> + }
>> +
>> + asoc = sctp_id2assoc(sk, params.sas_assoc_id);
>> + if (!asoc)
>> + goto out;
>> +
>> + retval = sctp_send_add_streams(asoc, ¶ms);
>> +
>> +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;
>
> You should do these overflow checks before doing actual work,
> preferreably at [@] mark above.
> Here it's fine, but when [*] below run, it may be too late.
>
>> +
>> + /* 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;
>
> [*]
it will cause alloc unused memory, but no consistent issue if
we move outcnt/incnt after checking retval.
if we want overflow check to go ahead of the actual work, another
conditions if (in) {} if(out) {} have to be added.
>
>> +
>> + 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;
>> + }
>> +
>> +out:
>> + return retval;
>> +}
>> --
>> 2.1.0
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [PATCHv3 net-next 4/4] sctp: implement sender-side procedures for Add Incoming/Outgoing Streams Request Parameter
2017-01-20 8:56 ` Xin Long
@ 2017-01-20 11:43 ` Marcelo Ricardo Leitner
0 siblings, 0 replies; 33+ messages in thread
From: Marcelo Ricardo Leitner @ 2017-01-20 11:43 UTC (permalink / raw)
To: Xin Long; +Cc: network dev, linux-sctp, Neil Horman, Vlad Yasevich, davem
On Fri, Jan 20, 2017 at 04:56:30PM +0800, Xin Long wrote:
> On Fri, Jan 20, 2017 at 5:47 AM, Marcelo Ricardo Leitner
> <marcelo.leitner@gmail.com> wrote:
> > 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 <lucien.xin@gmail.com>
> >> ---
> >> 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(¶ms, optval, optlen)) {
> >> + retval = -EFAULT;
> >> + goto out;
> >> + }
> >> +
> >> + asoc = sctp_id2assoc(sk, params.sas_assoc_id);
> >> + if (!asoc)
> >> + goto out;
> >> +
> >> + retval = sctp_send_add_streams(asoc, ¶ms);
> >> +
> >> +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;
> >
> > You should do these overflow checks before doing actual work,
> > preferreably at [@] mark above.
> > Here it's fine, but when [*] below run, it may be too late.
> >
> >> +
> >> + /* 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;
> >
> > [*]
> it will cause alloc unused memory, but no consistent issue if
> we move outcnt/incnt after checking retval.
Ah yes, right, so it doesn't hurt actually.
>
> if we want overflow check to go ahead of the actual work, another
> conditions if (in) {} if(out) {} have to be added.
Not sure why? Seems
if (!in || !out || stream->incnt + in > SCTP_MAX_STREAM ||
stream->outcnt + out > SCTP_MAX_STREAM)
goto out;
would do it, no?
>
> >
> >> +
> >> + 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;
> >> + }
> >> +
> >> +out:
> >> + return retval;
> >> +}
> >> --
> >> 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
> >>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCHv3 net-next 4/4] sctp: implement sender-side procedures for Add Incoming/Outgoing Streams Request Parameter
2017-01-19 17:19 ` [PATCHv3 net-next 4/4] sctp: implement sender-side procedures for Add Incoming/Outgoing Streams Request Parameter Xin Long
2017-01-19 20:17 ` Neil Horman
2017-01-19 21:47 ` Marcelo Ricardo Leitner
@ 2017-01-19 22:15 ` Marcelo Ricardo Leitner
2017-01-20 8:51 ` Xin Long
2017-01-23 11:25 ` David Laight
3 siblings, 1 reply; 33+ messages in thread
From: Marcelo Ricardo Leitner @ 2017-01-19 22:15 UTC (permalink / raw)
To: Xin Long; +Cc: network dev, linux-sctp, Neil Horman, Vlad Yasevich, davem
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 <lucien.xin@gmail.com>
> ---
> 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(¶ms, optval, optlen)) {
> + retval = -EFAULT;
> + goto out;
> + }
> +
> + asoc = sctp_id2assoc(sk, params.sas_assoc_id);
> + if (!asoc)
> + goto out;
> +
> + retval = sctp_send_add_streams(asoc, ¶ms);
> +
> +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 majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [PATCHv3 net-next 4/4] sctp: implement sender-side procedures for Add Incoming/Outgoing Streams Request Parameter
2017-01-19 22:15 ` Marcelo Ricardo Leitner
@ 2017-01-20 8:51 ` Xin Long
0 siblings, 0 replies; 33+ messages in thread
From: Xin Long @ 2017-01-20 8:51 UTC (permalink / raw)
To: Marcelo Ricardo Leitner
Cc: network dev, linux-sctp, Neil Horman, Vlad Yasevich, davem
On Fri, Jan 20, 2017 at 6:15 AM, Marcelo Ricardo Leitner
<marcelo.leitner@gmail.com> wrote:
> 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 <lucien.xin@gmail.com>
>> ---
>> 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(¶ms, optval, optlen)) {
>> + retval = -EFAULT;
>> + goto out;
>> + }
>> +
>> + asoc = sctp_id2assoc(sk, params.sas_assoc_id);
>> + if (!asoc)
>> + goto out;
>> +
>> + retval = sctp_send_add_streams(asoc, ¶ms);
>> +
>> +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?
yeah, I will set incnt and outcnt only when retval is 0.
>
>> + }
>> +
>> +out:
>> + return retval;
>> +}
>> --
>> 2.1.0
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
^ permalink raw reply [flat|nested] 33+ messages in thread
* RE: [PATCHv3 net-next 4/4] sctp: implement sender-side procedures for Add Incoming/Outgoing Streams Request Parameter
2017-01-19 17:19 ` [PATCHv3 net-next 4/4] sctp: implement sender-side procedures for Add Incoming/Outgoing Streams Request Parameter Xin Long
` (2 preceding siblings ...)
2017-01-19 22:15 ` Marcelo Ricardo Leitner
@ 2017-01-23 11:25 ` David Laight
2017-01-23 14:53 ` Neil Horman
2017-01-23 18:47 ` Marcelo Ricardo Leitner
3 siblings, 2 replies; 33+ messages in thread
From: David Laight @ 2017-01-23 11:25 UTC (permalink / raw)
To: 'Xin Long', network dev, linux-sctp@vger.kernel.org
Cc: Marcelo Ricardo Leitner, Neil Horman, Vlad Yasevich,
davem@davemloft.net
From: Xin Long
> Sent: 19 January 2017 17:19
> 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.
...
> + out = params->sas_outstrms;
> + in = params->sas_instrms;
> +
> + if (!out && !in)
> + goto out;
> +
> + if (out) {
> + __u16 nums = stream->outcnt + out;
Make nums 'unsigned int', the code will be smaller and you can
use the value for the overflow check.
> + /* 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;
> + }
Does kcalloc() zero the entire area, or just the length you ask for?
If the latter you need to zero the rest here.
...
David
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [PATCHv3 net-next 4/4] sctp: implement sender-side procedures for Add Incoming/Outgoing Streams Request Parameter
2017-01-23 11:25 ` David Laight
@ 2017-01-23 14:53 ` Neil Horman
2017-01-23 16:02 ` Marcelo Ricardo Leitner
2017-01-23 18:47 ` Marcelo Ricardo Leitner
1 sibling, 1 reply; 33+ messages in thread
From: Neil Horman @ 2017-01-23 14:53 UTC (permalink / raw)
To: David Laight
Cc: 'Xin Long', network dev, linux-sctp@vger.kernel.org,
Marcelo Ricardo Leitner, Vlad Yasevich, davem@davemloft.net
On Mon, Jan 23, 2017 at 11:25:56AM +0000, David Laight wrote:
> From: Xin Long
> > Sent: 19 January 2017 17:19
> > 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.
> ...
> > + out = params->sas_outstrms;
> > + in = params->sas_instrms;
> > +
> > + if (!out && !in)
> > + goto out;
> > +
> > + if (out) {
> > + __u16 nums = stream->outcnt + out;
>
> Make nums 'unsigned int', the code will be smaller and you can
> use the value for the overflow check.
>
> > + /* 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;
> > + }
>
> Does kcalloc() zero the entire area, or just the length you ask for?
> If the latter you need to zero the rest here.
Better still, just use krealloc. You still need to zero out any space beyond
the old length, but it will make the code shorter, and avoid the need for
additional temporary variables.
Neil
> ...
>
> David
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [PATCHv3 net-next 4/4] sctp: implement sender-side procedures for Add Incoming/Outgoing Streams Request Parameter
2017-01-23 14:53 ` Neil Horman
@ 2017-01-23 16:02 ` Marcelo Ricardo Leitner
2017-01-24 12:35 ` David Laight
0 siblings, 1 reply; 33+ messages in thread
From: Marcelo Ricardo Leitner @ 2017-01-23 16:02 UTC (permalink / raw)
To: Neil Horman
Cc: David Laight, 'Xin Long', network dev,
linux-sctp@vger.kernel.org, Vlad Yasevich, davem@davemloft.net
On Mon, Jan 23, 2017 at 09:53:47AM -0500, Neil Horman wrote:
> On Mon, Jan 23, 2017 at 11:25:56AM +0000, David Laight wrote:
> > From: Xin Long
> > > Sent: 19 January 2017 17:19
> > > 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.
> > ...
> > > + out = params->sas_outstrms;
> > > + in = params->sas_instrms;
> > > +
> > > + if (!out && !in)
> > > + goto out;
> > > +
> > > + if (out) {
> > > + __u16 nums = stream->outcnt + out;
> >
> > Make nums 'unsigned int', the code will be smaller and you can
> > use the value for the overflow check.
> >
> > > + /* 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;
> > > + }
> >
> > Does kcalloc() zero the entire area, or just the length you ask for?
> > If the latter you need to zero the rest here.
> Better still, just use krealloc. You still need to zero out any space beyond
> the old length, but it will make the code shorter, and avoid the need for
> additional temporary variables.
Seems if we pass gfp | __GFP_ZERO to krealloc it will end up zeroing the
slab for us before doing the memcpy.
I didn't follow all paths but in slab_alloc_node it will end up calling:
if (unlikely(gfpflags & __GFP_ZERO) && object)
memset(object, 0, s->object_size);
So I would expect that other paths also do it.
Marcelo
>
> Neil
>
> > ...
> >
> > David
> >
> > --
> > 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
> >
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 33+ messages in thread* RE: [PATCHv3 net-next 4/4] sctp: implement sender-side procedures for Add Incoming/Outgoing Streams Request Parameter
2017-01-23 16:02 ` Marcelo Ricardo Leitner
@ 2017-01-24 12:35 ` David Laight
2017-01-24 13:08 ` 'Marcelo Ricardo Leitner'
0 siblings, 1 reply; 33+ messages in thread
From: David Laight @ 2017-01-24 12:35 UTC (permalink / raw)
To: 'Marcelo Ricardo Leitner', Neil Horman
Cc: 'Xin Long', network dev, linux-sctp@vger.kernel.org,
Vlad Yasevich, davem@davemloft.net
From: Marcelo Ricardo Leitner
> Sent: 23 January 2017 16:03
...
> > > Does kcalloc() zero the entire area, or just the length you ask for?
> > > If the latter you need to zero the rest here.
> > Better still, just use krealloc. You still need to zero out any space beyond
> > the old length, but it will make the code shorter, and avoid the need for
> > additional temporary variables.
>
> Seems if we pass gfp | __GFP_ZERO to krealloc it will end up zeroing the
> slab for us before doing the memcpy.
> I didn't follow all paths but in slab_alloc_node it will end up calling:
> if (unlikely(gfpflags & __GFP_ZERO) && object)
> memset(object, 0, s->object_size);
> So I would expect that other paths also do it.
You probably don't want krealloc() zeroing all of the new area.
David
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCHv3 net-next 4/4] sctp: implement sender-side procedures for Add Incoming/Outgoing Streams Request Parameter
2017-01-24 12:35 ` David Laight
@ 2017-01-24 13:08 ` 'Marcelo Ricardo Leitner'
0 siblings, 0 replies; 33+ messages in thread
From: 'Marcelo Ricardo Leitner' @ 2017-01-24 13:08 UTC (permalink / raw)
To: David Laight
Cc: Neil Horman, 'Xin Long', network dev,
linux-sctp@vger.kernel.org, Vlad Yasevich, davem@davemloft.net
On Tue, Jan 24, 2017 at 12:35:39PM +0000, David Laight wrote:
> From: Marcelo Ricardo Leitner
> > Sent: 23 January 2017 16:03
> ...
> > > > Does kcalloc() zero the entire area, or just the length you ask for?
> > > > If the latter you need to zero the rest here.
> > > Better still, just use krealloc. You still need to zero out any space beyond
> > > the old length, but it will make the code shorter, and avoid the need for
> > > additional temporary variables.
> >
> > Seems if we pass gfp | __GFP_ZERO to krealloc it will end up zeroing the
> > slab for us before doing the memcpy.
> > I didn't follow all paths but in slab_alloc_node it will end up calling:
> > if (unlikely(gfpflags & __GFP_ZERO) && object)
> > memset(object, 0, s->object_size);
> > So I would expect that other paths also do it.
>
> You probably don't want krealloc() zeroing all of the new area.
Yep, agreed.
Marcelo
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCHv3 net-next 4/4] sctp: implement sender-side procedures for Add Incoming/Outgoing Streams Request Parameter
2017-01-23 11:25 ` David Laight
2017-01-23 14:53 ` Neil Horman
@ 2017-01-23 18:47 ` Marcelo Ricardo Leitner
2017-01-24 12:34 ` David Laight
1 sibling, 1 reply; 33+ messages in thread
From: Marcelo Ricardo Leitner @ 2017-01-23 18:47 UTC (permalink / raw)
To: David Laight
Cc: 'Xin Long', network dev, linux-sctp@vger.kernel.org,
Neil Horman, Vlad Yasevich, davem@davemloft.net
On Mon, Jan 23, 2017 at 11:25:56AM +0000, David Laight wrote:
> From: Xin Long
> > Sent: 19 January 2017 17:19
> > 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.
> ...
> > + out = params->sas_outstrms;
> > + in = params->sas_instrms;
> > +
> > + if (!out && !in)
> > + goto out;
> > +
> > + if (out) {
> > + __u16 nums = stream->outcnt + out;
>
> Make nums 'unsigned int', the code will be smaller and you can
> use the value for the overflow check.
Smaller as in to avoid the sum below?
>
> > + /* 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;
> > + }
>
> Does kcalloc() zero the entire area, or just the length you ask for?
> If the latter you need to zero the rest here.
> ...
>
> David
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 33+ messages in thread* RE: [PATCHv3 net-next 4/4] sctp: implement sender-side procedures for Add Incoming/Outgoing Streams Request Parameter
2017-01-23 18:47 ` Marcelo Ricardo Leitner
@ 2017-01-24 12:34 ` David Laight
2017-01-24 13:10 ` 'Marcelo Ricardo Leitner'
0 siblings, 1 reply; 33+ messages in thread
From: David Laight @ 2017-01-24 12:34 UTC (permalink / raw)
To: 'Marcelo Ricardo Leitner'
Cc: 'Xin Long', network dev, linux-sctp@vger.kernel.org,
Neil Horman, Vlad Yasevich, davem@davemloft.net
From: Marcelo Ricardo Leitner
> Sent: 23 January 2017 18:48
> On Mon, Jan 23, 2017 at 11:25:56AM +0000, David Laight wrote:
> > From: Xin Long
> > > Sent: 19 January 2017 17:19
> > > 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.
> > ...
> > > + out = params->sas_outstrms;
> > > + in = params->sas_instrms;
> > > +
> > > + if (!out && !in)
> > > + goto out;
> > > +
> > > + if (out) {
> > > + __u16 nums = stream->outcnt + out;
> >
> > Make nums 'unsigned int', the code will be smaller and you can
> > use the value for the overflow check.
>
> Smaller as in to avoid the sum below?
>
> >
> > > + /* Check for overflow, can't use nums here */
> > > + if (stream->outcnt + out > SCTP_MAX_STREAM)
> > > + goto out;
No, smaller as in not requiring the compiler to add instructions
to mask (or worse sign extend) the result of the arithmetic expression
to less than the number of bits in an 'int' when the result of the
expression is to be kept in a register.
The x86 is about the only modern cpu that has 8 and 16 bit arithmetic.
For everything else you really don't want to do arithmetic on char
and short unless you really want the wrapping to happen.
David
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [PATCHv3 net-next 4/4] sctp: implement sender-side procedures for Add Incoming/Outgoing Streams Request Parameter
2017-01-24 12:34 ` David Laight
@ 2017-01-24 13:10 ` 'Marcelo Ricardo Leitner'
0 siblings, 0 replies; 33+ messages in thread
From: 'Marcelo Ricardo Leitner' @ 2017-01-24 13:10 UTC (permalink / raw)
To: David Laight
Cc: 'Xin Long', network dev, linux-sctp@vger.kernel.org,
Neil Horman, Vlad Yasevich, davem@davemloft.net
On Tue, Jan 24, 2017 at 12:34:06PM +0000, David Laight wrote:
> From: Marcelo Ricardo Leitner
> > Sent: 23 January 2017 18:48
> > On Mon, Jan 23, 2017 at 11:25:56AM +0000, David Laight wrote:
> > > From: Xin Long
> > > > Sent: 19 January 2017 17:19
> > > > 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.
> > > ...
> > > > + out = params->sas_outstrms;
> > > > + in = params->sas_instrms;
> > > > +
> > > > + if (!out && !in)
> > > > + goto out;
> > > > +
> > > > + if (out) {
> > > > + __u16 nums = stream->outcnt + out;
> > >
> > > Make nums 'unsigned int', the code will be smaller and you can
> > > use the value for the overflow check.
> >
> > Smaller as in to avoid the sum below?
> >
> > >
> > > > + /* Check for overflow, can't use nums here */
> > > > + if (stream->outcnt + out > SCTP_MAX_STREAM)
> > > > + goto out;
>
> No, smaller as in not requiring the compiler to add instructions
> to mask (or worse sign extend) the result of the arithmetic expression
> to less than the number of bits in an 'int' when the result of the
> expression is to be kept in a register.
>
> The x86 is about the only modern cpu that has 8 and 16 bit arithmetic.
> For everything else you really don't want to do arithmetic on char
> and short unless you really want the wrapping to happen.
Okay thanks.
Marcelo
^ permalink raw reply [flat|nested] 33+ messages in thread