[PATCH v6 0/3] sctp: check the rto_min and rto_max

[PATCH v6 0/3] sctp: check the rto_min and rto_max

[Wang Weidong]

v5 -> v6
  -patch1: do rto_min/max socket option handling in its own patch, and
   fix the check of rto_min/max.
  -patch2: do rto_min/max sysctl handling in its own patch.
  -patch3: add Suggested-by Daniel.

Wang Weidong (3):
  sctp: check the rto_min and rto_max in setsockopt
  sctp: add check rto_min and rto_max in sysctl
  sctp: fix up a spacing

 net/sctp/socket.c | 32 +++++++++++++++--------
 net/sctp/sysctl.c | 76 ++++++++++++++++++++++++++++++++++++++++++++++++-------
 2 files changed, 89 insertions(+), 19 deletions(-)

-- 
1.7.12

[PATCH v6 1/3] sctp: check the rto_min and rto_max in setsockopt

[Wang Weidong]

When we set 0 to rto_min or rto_max, just not change the value. Also
we should check the rto_min > rto_max.

Suggested-by: Vlad Yasevich <vyasevich@gmail.com>
Signed-off-by: Wang Weidong <wangweidong1@huawei.com>
---
 net/sctp/socket.c | 32 ++++++++++++++++++++++----------
 1 file changed, 22 insertions(+), 10 deletions(-)

diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 72046b9..da09e59 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -2811,6 +2811,8 @@ static int sctp_setsockopt_rtoinfo(struct sock *sk, char __user *optval, unsigne
 {
 	struct sctp_rtoinfo rtoinfo;
 	struct sctp_association *asoc;
+	unsigned long rto_min, rto_max;
+	struct sctp_sock *sp = sctp_sk(sk);
 
 	if (optlen != sizeof (struct sctp_rtoinfo))
 		return -EINVAL;
@@ -2824,26 +2826,36 @@ static int sctp_setsockopt_rtoinfo(struct sock *sk, char __user *optval, unsigne
 	if (!asoc && rtoinfo.srto_assoc_id && sctp_style(sk, UDP))
 		return -EINVAL;
 
+	rto_max = rtoinfo.srto_max;
+	rto_min = rtoinfo.srto_min;
+
+	if (rto_max)
+		rto_max = asoc ? msecs_to_jiffies(rto_max) : rto_max;
+	else
+		rto_max = asoc ? asoc->rto_max : sp->rtoinfo.srto_max;
+
+	if (rto_min)
+		rto_min = asoc ? msecs_to_jiffies(rto_min) : rto_min;
+	else
+		rto_min = asoc ? asoc->rto_min : sp->rtoinfo.srto_min;
+
+	if (rto_min > rto_max)
+		return -EINVAL;
+
 	if (asoc) {
 		if (rtoinfo.srto_initial != 0)
 			asoc->rto_initial =
 				msecs_to_jiffies(rtoinfo.srto_initial);
-		if (rtoinfo.srto_max != 0)
-			asoc->rto_max = msecs_to_jiffies(rtoinfo.srto_max);
-		if (rtoinfo.srto_min != 0)
-			asoc->rto_min = msecs_to_jiffies(rtoinfo.srto_min);
+		asoc->rto_max = rto_max;
+		asoc->rto_min = rto_min;
 	} else {
 		/* If there is no association or the association-id = 0
 		 * set the values to the endpoint.
 		 */
-		struct sctp_sock *sp = sctp_sk(sk);
-
 		if (rtoinfo.srto_initial != 0)
 			sp->rtoinfo.srto_initial = rtoinfo.srto_initial;
-		if (rtoinfo.srto_max != 0)
-			sp->rtoinfo.srto_max = rtoinfo.srto_max;
-		if (rtoinfo.srto_min != 0)
-			sp->rtoinfo.srto_min = rtoinfo.srto_min;
+		sp->rtoinfo.srto_max = rto_max;
+		sp->rtoinfo.srto_min = rto_min;
 	}
 
 	return 0;
-- 
1.7.12

[PATCH v6 2/3] sctp: add check rto_min and rto_max in sysctl

[Wang Weidong]

rto_min should be smaller than rto_max while rto_max should be larger
than rto_min. Add two proc_handler for the checking.

Suggested-by: Vlad Yasevich <vyasevich@gmail.com>
Signed-off-by: Wang Weidong <wangweidong1@huawei.com>
---
 net/sctp/sysctl.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 65 insertions(+), 4 deletions(-)

diff --git a/net/sctp/sysctl.c b/net/sctp/sysctl.c
index 6b36561..ed4ec89 100644
--- a/net/sctp/sysctl.c
+++ b/net/sctp/sysctl.c
@@ -61,6 +61,13 @@ static int proc_sctp_do_hmac_alg(struct ctl_table *ctl,
 				void __user *buffer, size_t *lenp,
 
 				loff_t *ppos);
+static int proc_sctp_do_rto_min(struct ctl_table *ctl, int write,
+				void __user *buffer, size_t *lenp,
+				loff_t *ppos);
+static int proc_sctp_do_rto_max(struct ctl_table *ctl, int write,
+				void __user *buffer, size_t *lenp,
+				loff_t *ppos);
+
 static struct ctl_table sctp_table[] = {
 	{
 		.procname	= "sctp_mem",
@@ -102,17 +109,17 @@ static struct ctl_table sctp_net_table[] = {
 		.data		= &init_net.sctp.rto_min,
 		.maxlen		= sizeof(unsigned int),
 		.mode		= 0644,
-		.proc_handler	= proc_dointvec_minmax,
+		.proc_handler	= proc_sctp_do_rto_min,
 		.extra1         = &one,
-		.extra2         = &timer_max
+		.extra2         = &init_net.sctp.rto_max
 	},
 	{
 		.procname	= "rto_max",
 		.data		= &init_net.sctp.rto_max,
 		.maxlen		= sizeof(unsigned int),
 		.mode		= 0644,
-		.proc_handler	= proc_dointvec_minmax,
-		.extra1         = &one,
+		.proc_handler	= proc_sctp_do_rto_max,
+		.extra1         = &init_net.sctp.rto_min,
 		.extra2         = &timer_max
 	},
 	{
@@ -342,6 +349,60 @@ static int proc_sctp_do_hmac_alg(struct ctl_table *ctl,
 	return ret;
 }
 
+static int proc_sctp_do_rto_min(struct ctl_table *ctl, int write,
+				void __user*buffer, size_t *lenp,
+				loff_t *ppos)
+{
+	struct net *net = current->nsproxy->net_ns;
+	int new_value;
+	struct ctl_table tbl;
+	unsigned int min = *(unsigned int *) ctl->extra1;
+	unsigned int max = *(unsigned int *) ctl->extra2;
+	int ret;
+
+	memset(&tbl, 0, sizeof(struct ctl_table));
+	tbl.maxlen = sizeof(unsigned int);
+
+	if (write)
+		tbl.data = &new_value;
+	else
+		tbl.data = &net->sctp.rto_min;
+	ret = proc_dointvec(&tbl, write, buffer, lenp, ppos);
+	if (write) {
+		if (ret || new_value > max || new_value < min)
+			return -EINVAL;
+		net->sctp.rto_min = new_value;
+	}
+	return ret;
+}
+
+static int proc_sctp_do_rto_max(struct ctl_table *ctl, int write,
+				void __user*buffer, size_t *lenp,
+				loff_t *ppos)
+{
+	struct net *net = current->nsproxy->net_ns;
+	int new_value;
+	struct ctl_table tbl;
+	unsigned int min = *(unsigned int *) ctl->extra1;
+	unsigned int max = *(unsigned int *) ctl->extra2;
+	int ret;
+
+	memset(&tbl, 0, sizeof(struct ctl_table));
+	tbl.maxlen = sizeof(unsigned int);
+
+	if (write)
+		tbl.data = &new_value;
+	else
+		tbl.data = &net->sctp.rto_max;
+	ret = proc_dointvec(&tbl, write, buffer, lenp, ppos);
+	if (write) {
+                if (ret || new_value > max || new_value < min)
+			return -EINVAL;
+		net->sctp.rto_max = new_value;
+	}
+	return ret;
+}
+
 int sctp_sysctl_net_register(struct net *net)
 {
 	struct ctl_table *table;
-- 
1.7.12

Re: [PATCH v6 2/3] sctp: add check rto_min and rto_max in sysctl

[Vlad Yasevich]

On 12/09/2013 09:46 PM, Wang Weidong wrote:
> rto_min should be smaller than rto_max while rto_max should be larger
> than rto_min. Add two proc_handler for the checking.
> 
> Suggested-by: Vlad Yasevich <vyasevich@gmail.com>
> Signed-off-by: Wang Weidong <wangweidong1@huawei.com>
> ---
>  net/sctp/sysctl.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 65 insertions(+), 4 deletions(-)
> 
> diff --git a/net/sctp/sysctl.c b/net/sctp/sysctl.c
> index 6b36561..ed4ec89 100644
> --- a/net/sctp/sysctl.c
> +++ b/net/sctp/sysctl.c
> @@ -61,6 +61,13 @@ static int proc_sctp_do_hmac_alg(struct ctl_table *ctl,
>  				void __user *buffer, size_t *lenp,
>  
>  				loff_t *ppos);
> +static int proc_sctp_do_rto_min(struct ctl_table *ctl, int write,
> +				void __user *buffer, size_t *lenp,
> +				loff_t *ppos);
> +static int proc_sctp_do_rto_max(struct ctl_table *ctl, int write,
> +				void __user *buffer, size_t *lenp,
> +				loff_t *ppos);
> +
>  static struct ctl_table sctp_table[] = {
>  	{
>  		.procname	= "sctp_mem",
> @@ -102,17 +109,17 @@ static struct ctl_table sctp_net_table[] = {
>  		.data		= &init_net.sctp.rto_min,
>  		.maxlen		= sizeof(unsigned int),
>  		.mode		= 0644,
> -		.proc_handler	= proc_dointvec_minmax,
> +		.proc_handler	= proc_sctp_do_rto_min,
>  		.extra1         = &one,
> -		.extra2         = &timer_max
> +		.extra2         = &init_net.sctp.rto_max
>  	},
>  	{
>  		.procname	= "rto_max",
>  		.data		= &init_net.sctp.rto_max,
>  		.maxlen		= sizeof(unsigned int),
>  		.mode		= 0644,
> -		.proc_handler	= proc_dointvec_minmax,
> -		.extra1         = &one,
> +		.proc_handler	= proc_sctp_do_rto_max,
> +		.extra1         = &init_net.sctp.rto_min,
>  		.extra2         = &timer_max
>  	},
>  	{
> @@ -342,6 +349,60 @@ static int proc_sctp_do_hmac_alg(struct ctl_table *ctl,
>  	return ret;
>  }
>  
> +static int proc_sctp_do_rto_min(struct ctl_table *ctl, int write,
> +				void __user*buffer, size_t *lenp,
                                         ^^^^
Please put a space before the '*'.

> +				loff_t *ppos)
> +{
> +	struct net *net = current->nsproxy->net_ns;
> +	int new_value;
> +	struct ctl_table tbl;
> +	unsigned int min = *(unsigned int *) ctl->extra1;
> +	unsigned int max = *(unsigned int *) ctl->extra2;
> +	int ret;
> +
> +	memset(&tbl, 0, sizeof(struct ctl_table));
> +	tbl.maxlen = sizeof(unsigned int);
> +
> +	if (write)
> +		tbl.data = &new_value;
> +	else
> +		tbl.data = &net->sctp.rto_min;
> +	ret = proc_dointvec(&tbl, write, buffer, lenp, ppos);
> +	if (write) {
> +		if (ret || new_value > max || new_value < min)
> +			return -EINVAL;
> +		net->sctp.rto_min = new_value;
> +	}
> +	return ret;
> +}
> +
> +static int proc_sctp_do_rto_max(struct ctl_table *ctl, int write,
> +				void __user*buffer, size_t *lenp,
                                         ^^^^
Please put a space after the before the '*'.

Thanks
-vlad

> +				loff_t *ppos)
> +{
> +	struct net *net = current->nsproxy->net_ns;
> +	int new_value;
> +	struct ctl_table tbl;
> +	unsigned int min = *(unsigned int *) ctl->extra1;
> +	unsigned int max = *(unsigned int *) ctl->extra2;
> +	int ret;
> +
> +	memset(&tbl, 0, sizeof(struct ctl_table));
> +	tbl.maxlen = sizeof(unsigned int);
> +
> +	if (write)
> +		tbl.data = &new_value;
> +	else
> +		tbl.data = &net->sctp.rto_max;
> +	ret = proc_dointvec(&tbl, write, buffer, lenp, ppos);
> +	if (write) {
> +                if (ret || new_value > max || new_value < min)
> +			return -EINVAL;
> +		net->sctp.rto_max = new_value;
> +	}
> +	return ret;
> +}
> +
>  int sctp_sysctl_net_register(struct net *net)
>  {
>  	struct ctl_table *table;
> 

Re: [PATCH v6 2/3] sctp: add check rto_min and rto_max in sysctl

[Wang Weidong]

 From Wang Weidong <wangweidong1@huawei.com>

On 2013/12/10 22:34, Vlad Yasevich wrote:
> On 12/09/2013 09:46 PM, Wang Weidong wrote:
>> rto_min should be smaller than rto_max while rto_max should be larger
>> than rto_min. Add two proc_handler for the checking.
>>
>> Suggested-by: Vlad Yasevich <vyasevich@gmail.com>
>> Signed-off-by: Wang Weidong <wangweidong1@huawei.com>
>> ---
>>   net/sctp/sysctl.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++----
>>   1 file changed, 65 insertions(+), 4 deletions(-)
>>
>> diff --git a/net/sctp/sysctl.c b/net/sctp/sysctl.c
>> index 6b36561..ed4ec89 100644
>> --- a/net/sctp/sysctl.c
>> +++ b/net/sctp/sysctl.c
>> @@ -61,6 +61,13 @@ static int proc_sctp_do_hmac_alg(struct ctl_table *ctl,
>>   				void __user *buffer, size_t *lenp,
>>
>>   				loff_t *ppos);
>> +static int proc_sctp_do_rto_min(struct ctl_table *ctl, int write,
>> +				void __user *buffer, size_t *lenp,
>> +				loff_t *ppos);
>> +static int proc_sctp_do_rto_max(struct ctl_table *ctl, int write,
>> +				void __user *buffer, size_t *lenp,
>> +				loff_t *ppos);
>> +
>>   static struct ctl_table sctp_table[] = {
>>   	{
>>   		.procname	= "sctp_mem",
>> @@ -102,17 +109,17 @@ static struct ctl_table sctp_net_table[] = {
>>   		.data		= &init_net.sctp.rto_min,
>>   		.maxlen		= sizeof(unsigned int),
>>   		.mode		= 0644,
>> -		.proc_handler	= proc_dointvec_minmax,
>> +		.proc_handler	= proc_sctp_do_rto_min,
>>   		.extra1         = &one,
>> -		.extra2         = &timer_max
>> +		.extra2         = &init_net.sctp.rto_max
>>   	},
>>   	{
>>   		.procname	= "rto_max",
>>   		.data		= &init_net.sctp.rto_max,
>>   		.maxlen		= sizeof(unsigned int),
>>   		.mode		= 0644,
>> -		.proc_handler	= proc_dointvec_minmax,
>> -		.extra1         = &one,
>> +		.proc_handler	= proc_sctp_do_rto_max,
>> +		.extra1         = &init_net.sctp.rto_min,
>>   		.extra2         = &timer_max
>>   	},
>>   	{
>> @@ -342,6 +349,60 @@ static int proc_sctp_do_hmac_alg(struct ctl_table *ctl,
>>   	return ret;
>>   }
>>
>> +static int proc_sctp_do_rto_min(struct ctl_table *ctl, int write,
>> +				void __user*buffer, size_t *lenp,
>                                           ^^^^
> Please put a space before the '*'.
>
>> +				loff_t *ppos)
>> +{
>> +	struct net *net = current->nsproxy->net_ns;
>> +	int new_value;
>> +	struct ctl_table tbl;
>> +	unsigned int min = *(unsigned int *) ctl->extra1;
>> +	unsigned int max = *(unsigned int *) ctl->extra2;
>> +	int ret;
>> +
>> +	memset(&tbl, 0, sizeof(struct ctl_table));
>> +	tbl.maxlen = sizeof(unsigned int);
>> +
>> +	if (write)
>> +		tbl.data = &new_value;
>> +	else
>> +		tbl.data = &net->sctp.rto_min;
>> +	ret = proc_dointvec(&tbl, write, buffer, lenp, ppos);
>> +	if (write) {
>> +		if (ret || new_value > max || new_value < min)
>> +			return -EINVAL;
>> +		net->sctp.rto_min = new_value;
>> +	}
>> +	return ret;
>> +}
>> +
>> +static int proc_sctp_do_rto_max(struct ctl_table *ctl, int write,
>> +				void __user*buffer, size_t *lenp,
>                                           ^^^^
> Please put a space after the before the '*'.
>
Ok, I will fix them.
What Daniel point out is that? I no need to line up them?

Regards.
Wang

> Thanks
> -vlad
>
>> +				loff_t *ppos)
>> +{
>> +	struct net *net = current->nsproxy->net_ns;
>> +	int new_value;
>> +	struct ctl_table tbl;
>> +	unsigned int min = *(unsigned int *) ctl->extra1;
>> +	unsigned int max = *(unsigned int *) ctl->extra2;
>> +	int ret;
>> +
>> +	memset(&tbl, 0, sizeof(struct ctl_table));
>> +	tbl.maxlen = sizeof(unsigned int);
>> +
>> +	if (write)
>> +		tbl.data = &new_value;
>> +	else
>> +		tbl.data = &net->sctp.rto_max;
>> +	ret = proc_dointvec(&tbl, write, buffer, lenp, ppos);
>> +	if (write) {
>> +                if (ret || new_value > max || new_value < min)
>> +			return -EINVAL;
>> +		net->sctp.rto_max = new_value;
>> +	}
>> +	return ret;
>> +}
>> +
>>   int sctp_sysctl_net_register(struct net *net)
>>   {
>>   	struct ctl_table *table;
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH v6 2/3] sctp: add check rto_min and rto_max in sysctl

[Daniel Borkmann]

On 12/10/2013 04:18 PM, Wang Weidong wrote:
...
>>> +static int proc_sctp_do_rto_min(struct ctl_table *ctl, int write,
>>> +                                void __user*buffer, size_t *lenp,
>>                                           ^^^^
>> Please put a space before the '*'.
...
>>> +static int proc_sctp_do_rto_max(struct ctl_table *ctl, int write,
>>> +                                void __user*buffer, size_t *lenp,
>>                                           ^^^^
>> Please put a space after the before the '*'.
>>
> Ok, I will fix them.
> What Daniel point out is that? I no need to line up them?

Yes, what Vlad pointed out to you is what I meant in my reply to
your cover letter ...

Re: [PATCH v6 2/3] sctp: add check rto_min and rto_max in sysctl

[Wang Weidong]

On 2013/12/10 23:21, Daniel Borkmann wrote:
> On 12/10/2013 04:18 PM, Wang Weidong wrote:
> ...
>>>> +static int proc_sctp_do_rto_min(struct ctl_table *ctl, int write,
>>>> +                                void __user*buffer, size_t *lenp,
>>>                                           ^^^^
>>> Please put a space before the '*'.
> ...
>>>> +static int proc_sctp_do_rto_max(struct ctl_table *ctl, int write,
>>>> +                                void __user*buffer, size_t *lenp,
>>>                                           ^^^^
>>> Please put a space after the before the '*'.
>>>
>> Ok, I will fix them.
>> What Daniel point out is that? I no need to line up them?
>
> Yes, what Vlad pointed out to you is what I meant in my reply to
> your cover letter ...
Nice. I get it.

Thanks.

[PATCH v6 3/3] sctp: fix up a spacing

[Wang Weidong]

fix up spacing of proc_sctp_do_hmac_alg for according to the
proc_sctp_do_rto_min[max] in sysctl.c

Suggested-by: Daniel Borkmann <dborkman@redhat.com>
Signed-off-by: Wang Weidong <wangweidong1@huawei.com>
---
 net/sctp/sysctl.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/net/sctp/sysctl.c b/net/sctp/sysctl.c
index ed4ec89..9ae59a3 100644
--- a/net/sctp/sysctl.c
+++ b/net/sctp/sysctl.c
@@ -56,10 +56,8 @@ extern long sysctl_sctp_mem[3];
 extern int sysctl_sctp_rmem[3];
 extern int sysctl_sctp_wmem[3];
 
-static int proc_sctp_do_hmac_alg(struct ctl_table *ctl,
-				int write,
+static int proc_sctp_do_hmac_alg(struct ctl_table *ctl, int write,
 				void __user *buffer, size_t *lenp,
-
 				loff_t *ppos);
 static int proc_sctp_do_rto_min(struct ctl_table *ctl, int write,
 				void __user *buffer, size_t *lenp,
@@ -301,8 +299,7 @@ static struct ctl_table sctp_net_table[] = {
 	{ /* sentinel */ }
 };
 
-static int proc_sctp_do_hmac_alg(struct ctl_table *ctl,
-				int write,
+static int proc_sctp_do_hmac_alg(struct ctl_table *ctl, int write,
 				void __user *buffer, size_t *lenp,
 				loff_t *ppos)
 {
-- 
1.7.12

Re: [PATCH v6 0/3] sctp: check the rto_min and rto_max

[Daniel Borkmann]

On 12/10/2013 03:46 AM, Wang Weidong wrote:
> v5 -> v6
>    -patch1: do rto_min/max socket option handling in its own patch, and
>     fix the check of rto_min/max.
>    -patch2: do rto_min/max sysctl handling in its own patch.
>    -patch3: add Suggested-by Daniel.

Fyi, for future submission, please keep the full changelog.

I would have much rather liked seeing you with finishing the last
2 patches first, and then approach the newly introduced 1st one
in this series for now. You still haven't fully fixed whitespace
issues in your second patch in the function itself which I hoped
you would address, but fair enough ...

> Wang Weidong (3):
>    sctp: check the rto_min and rto_max in setsockopt
>    sctp: add check rto_min and rto_max in sysctl
>    sctp: fix up a spacing
>
>   net/sctp/socket.c | 32 +++++++++++++++--------
>   net/sctp/sysctl.c | 76 ++++++++++++++++++++++++++++++++++++++++++++++++-------
>   2 files changed, 89 insertions(+), 19 deletions(-)
>

Re: [PATCH v6 0/3] sctp: check the rto_min and rto_max

[Wang Weidong]

From: Wang Weidong <wangweidong1@huawei.com>

On 2013/12/10 20:51, Daniel Borkmann wrote:
> On 12/10/2013 03:46 AM, Wang Weidong wrote:
>> v5 -> v6
>>    -patch1: do rto_min/max socket option handling in its own patch, and
>>     fix the check of rto_min/max.
>>    -patch2: do rto_min/max sysctl handling in its own patch.
>>    -patch3: add Suggested-by Daniel.
>
> Fyi, for future submission, please keep the full changelog.
>

Yeah, Thanks! I will do this in future submission.

> I would have much rather liked seeing you with finishing the last
> 2 patches first, and then approach the newly introduced 1st one
> in this series for now. You still haven't fully fixed whitespace
> issues in your second patch in the function itself which I hoped
> you would address, but fair enough ...
>

As Vlad's suggestion, I split it to 2 patches. Should I combine these
2 patch to one?

The second patch, do you mean that?
I should do it from

+static int proc_sctp_do_rto_min(struct ctl_table *ctl, int write,
+				void __user *buffer, size_t *lenp,
+				loff_t *ppos);
+static int proc_sctp_do_rto_max(struct ctl_table *ctl, int write,
+				void __user *buffer, size_t *lenp,
+				loff_t *ppos);

to

+static int proc_sctp_do_rto_min(struct ctl_table *ctl, int write,
+				 void __user *buffer, size_t *lenp,
+				 loff_t *ppos);
+static int proc_sctp_do_rto_max(struct ctl_table *ctl, int write,
+				 void __user *buffer, size_t *lenp,
+				 loff_t *ppos);

Just line up to first line? And I should change the third patch as well.

Regards.
Wang

>> Wang Weidong (3):
>>    sctp: check the rto_min and rto_max in setsockopt
>>    sctp: add check rto_min and rto_max in sysctl
>>    sctp: fix up a spacing
>>
>>   net/sctp/socket.c | 32 +++++++++++++++--------
>>   net/sctp/sysctl.c | 76 ++++++++++++++++++++++++++++++++++++++++++++++++-------
>>   2 files changed, 89 insertions(+), 19 deletions(-)
>>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v6 0/3] sctp: check the rto_min and rto_max

[Vlad Yasevich]

On 12/10/2013 09:34 AM, Wang Weidong wrote:
> From: Wang Weidong <wangweidong1@huawei.com>
> 
> On 2013/12/10 20:51, Daniel Borkmann wrote:
>> On 12/10/2013 03:46 AM, Wang Weidong wrote:
>>> v5 -> v6
>>>    -patch1: do rto_min/max socket option handling in its own patch, and
>>>     fix the check of rto_min/max.
>>>    -patch2: do rto_min/max sysctl handling in its own patch.
>>>    -patch3: add Suggested-by Daniel.
>>
>> Fyi, for future submission, please keep the full changelog.
>>
> 
> Yeah, Thanks! I will do this in future submission.
> 
>> I would have much rather liked seeing you with finishing the last
>> 2 patches first, and then approach the newly introduced 1st one
>> in this series for now. You still haven't fully fixed whitespace
>> issues in your second patch in the function itself which I hoped
>> you would address, but fair enough ...
>>
> 
> As Vlad's suggestion, I split it to 2 patches. Should I combine these
> 2 patch to one?

No, they deserve a separate patch.  What Daniel meant was that you
should have addressed the sysctl control issues first.  It doesn't
really matter all that much here.  In fact the last whitespace patch
is the one that doesn't really fit in this series as it fixes something
quite unrelated to the reset of the series, but we are talking semantics.

I am fine with the series as long as you address the spacing issue
in the new proc handler functions.

> 
> The second patch, do you mean that?
> I should do it from
> 
> +static int proc_sctp_do_rto_min(struct ctl_table *ctl, int write,
> +                void __user *buffer, size_t *lenp,
> +                loff_t *ppos);
> +static int proc_sctp_do_rto_max(struct ctl_table *ctl, int write,
> +                void __user *buffer, size_t *lenp,
> +                loff_t *ppos);
> 
> to
> 
> +static int proc_sctp_do_rto_min(struct ctl_table *ctl, int write,
> +                 void __user *buffer, size_t *lenp,
> +                 loff_t *ppos);
> +static int proc_sctp_do_rto_max(struct ctl_table *ctl, int write,
> +                 void __user *buffer, size_t *lenp,
> +                 loff_t *ppos);
> 
> Just line up to first line? And I should change the third patch as well.

No, the prototypes are fine.  I sent a separate comment pointing out
the spacing issue.

-vlad

> 
> Regards.
> Wang
> 
>>> Wang Weidong (3):
>>>    sctp: check the rto_min and rto_max in setsockopt
>>>    sctp: add check rto_min and rto_max in sysctl
>>>    sctp: fix up a spacing
>>>
>>>   net/sctp/socket.c | 32 +++++++++++++++--------
>>>   net/sctp/sysctl.c | 76
>>> ++++++++++++++++++++++++++++++++++++++++++++++++-------
>>>   2 files changed, 89 insertions(+), 19 deletions(-)
>>>
>> -- 
>> To unsubscribe from this list: send the line "unsubscribe netdev" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v6 0/3] sctp: check the rto_min and rto_max

[Wang Weidong]

From: Wang Weidong <wangweidong1@huawei.com>

On 2013/12/10 22:38, Vlad Yasevich wrote:
> On 12/10/2013 09:34 AM, Wang Weidong wrote:
>> From: Wang Weidong <wangweidong1@huawei.com>
>>
>> On 2013/12/10 20:51, Daniel Borkmann wrote:
>>> On 12/10/2013 03:46 AM, Wang Weidong wrote:
>>>> v5 -> v6
>>>>     -patch1: do rto_min/max socket option handling in its own patch, and
>>>>      fix the check of rto_min/max.
>>>>     -patch2: do rto_min/max sysctl handling in its own patch.
>>>>     -patch3: add Suggested-by Daniel.
>>>
>>> Fyi, for future submission, please keep the full changelog.
>>>
>>
>> Yeah, Thanks! I will do this in future submission.
>>
>>> I would have much rather liked seeing you with finishing the last
>>> 2 patches first, and then approach the newly introduced 1st one
>>> in this series for now. You still haven't fully fixed whitespace
>>> issues in your second patch in the function itself which I hoped
>>> you would address, but fair enough ...
>>>
>>
>> As Vlad's suggestion, I split it to 2 patches. Should I combine these
>> 2 patch to one?
>
> No, they deserve a separate patch.  What Daniel meant was that you
> should have addressed the sysctl control issues first.  It doesn't
> really matter all that much here.  In fact the last whitespace patch
> is the one that doesn't really fit in this series as it fixes something
> quite unrelated to the reset of the series, but we are talking semantics.
>
> I am fine with the series as long as you address the spacing issue
> in the new proc handler functions.
>
Nice, I will fix them in v7.

Regards.
Wang

>>
>> The second patch, do you mean that?
>> I should do it from
>>
>> +static int proc_sctp_do_rto_min(struct ctl_table *ctl, int write,
>> +                void __user *buffer, size_t *lenp,
>> +                loff_t *ppos);
>> +static int proc_sctp_do_rto_max(struct ctl_table *ctl, int write,
>> +                void __user *buffer, size_t *lenp,
>> +                loff_t *ppos);
>>
>> to
>>
>> +static int proc_sctp_do_rto_min(struct ctl_table *ctl, int write,
>> +                 void __user *buffer, size_t *lenp,
>> +                 loff_t *ppos);
>> +static int proc_sctp_do_rto_max(struct ctl_table *ctl, int write,
>> +                 void __user *buffer, size_t *lenp,
>> +                 loff_t *ppos);
>>
>> Just line up to first line? And I should change the third patch as well.
>
> No, the prototypes are fine.  I sent a separate comment pointing out
> the spacing issue.
>
> -vlad
>
>>
>> Regards.
>> Wang
>>
>>>> Wang Weidong (3):
>>>>     sctp: check the rto_min and rto_max in setsockopt
>>>>     sctp: add check rto_min and rto_max in sysctl
>>>>     sctp: fix up a spacing
>>>>
>>>>    net/sctp/socket.c | 32 +++++++++++++++--------
>>>>    net/sctp/sysctl.c | 76
>>>> ++++++++++++++++++++++++++++++++++++++++++++++++-------
>>>>    2 files changed, 89 insertions(+), 19 deletions(-)
>>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe netdev" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>