All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vladislav Yasevich <vladislav.yasevich@hp.com>
To: Xi Wang <xi.wang@gmail.com>
Cc: netdev@vger.kernel.org, linux-sctp@vger.kernel.org,
	Andrew Morton <akpm@linux-foundation.org>,
	Andrei Pelinescu-Onciul <andrei@iptel.org>,
	"David S. Miller" <davem@davemloft.net>
Subject: Re: [PATCH RESEND] sctp: fix incorrect overflow check on autoclose
Date: Mon, 12 Dec 2011 22:18:46 +0000	[thread overview]
Message-ID: <4EE67DC6.3000500@hp.com> (raw)
In-Reply-To: <8EEC521A-CA38-4CE9-BCFD-BA6FC9A85D18@gmail.com>

On 12/09/2011 01:04 PM, Xi Wang wrote:
> On Dec 9, 2011, at 12:38 PM, Vladislav Yasevich wrote:
>> I think this should be u32 since that's what sp->autoclose is.
> 
> Do you mean something like this?
> 
> 	min_t(u32, sp->autoclose, MAX_SCHEDULE_TIMEOUT / HZ)
> 
> On 64-bit platform this would limit autoclose for no good
> reason to
> 
> 	(u32)(MAX_SCHEDULE_TIMEOUT / HZ),
> 
> which is basically 0x978d4fdf (assuming HZ is 250).  I guess the
> intention was to allow autoclose to be any u32 on 64-bit platform.
> 

Hm..  this is a bit strange.  This makes it so that on 32 bit platforms
we have one upper bound for autoclose and on 64 we have another even though
the type is platform dependent.  This could be considered a regression by
applications.

In addition this would result in confusion to user since the values
between setsockopt() and getsockopt() for autoclose would be different.

Looking at the latest spec, it actually looks like we are completely
mis-interpreting autoclose.  It's supposed to be in seconds.

For now, I'd suggest to make this consistent between 32 and 64 bits.
Having inconsistent values seems strange.

As a next set the api needs to be fixed to accept seconds as
argument.

-vlad






WARNING: multiple messages have this Message-ID (diff)
From: Vladislav Yasevich <vladislav.yasevich@hp.com>
To: Xi Wang <xi.wang@gmail.com>
Cc: netdev@vger.kernel.org, linux-sctp@vger.kernel.org,
	Andrew Morton <akpm@linux-foundation.org>,
	Andrei Pelinescu-Onciul <andrei@iptel.org>,
	"David S. Miller" <davem@davemloft.net>
Subject: Re: [PATCH RESEND] sctp: fix incorrect overflow check on autoclose
Date: Mon, 12 Dec 2011 17:18:46 -0500	[thread overview]
Message-ID: <4EE67DC6.3000500@hp.com> (raw)
In-Reply-To: <8EEC521A-CA38-4CE9-BCFD-BA6FC9A85D18@gmail.com>

On 12/09/2011 01:04 PM, Xi Wang wrote:
> On Dec 9, 2011, at 12:38 PM, Vladislav Yasevich wrote:
>> I think this should be u32 since that's what sp->autoclose is.
> 
> Do you mean something like this?
> 
> 	min_t(u32, sp->autoclose, MAX_SCHEDULE_TIMEOUT / HZ)
> 
> On 64-bit platform this would limit autoclose for no good
> reason to
> 
> 	(u32)(MAX_SCHEDULE_TIMEOUT / HZ),
> 
> which is basically 0x978d4fdf (assuming HZ is 250).  I guess the
> intention was to allow autoclose to be any u32 on 64-bit platform.
> 

Hm..  this is a bit strange.  This makes it so that on 32 bit platforms
we have one upper bound for autoclose and on 64 we have another even though
the type is platform dependent.  This could be considered a regression by
applications.

In addition this would result in confusion to user since the values
between setsockopt() and getsockopt() for autoclose would be different.

Looking at the latest spec, it actually looks like we are completely
mis-interpreting autoclose.  It's supposed to be in seconds.

For now, I'd suggest to make this consistent between 32 and 64 bits.
Having inconsistent values seems strange.

As a next set the api needs to be fixed to accept seconds as
argument.

-vlad

  reply	other threads:[~2011-12-12 22:18 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-12-09  1:24 [PATCH RESEND] sctp: fix incorrect overflow check on autoclose Xi Wang
2011-12-09  1:24 ` Xi Wang
2011-12-09 17:38 ` Vladislav Yasevich
2011-12-09 17:38   ` Vladislav Yasevich
2011-12-09 18:04   ` Xi Wang
2011-12-09 18:04     ` Xi Wang
2011-12-12 22:18     ` Vladislav Yasevich [this message]
2011-12-12 22:18       ` Vladislav Yasevich
2011-12-13 22:00       ` Xi Wang
2011-12-13 22:00         ` Xi Wang
2011-12-13 22:15         ` Vladislav Yasevich
2011-12-13 22:15           ` Vladislav Yasevich
2011-12-14 21:35           ` Xi Wang
2011-12-14 21:35             ` Xi Wang
2011-12-14 21:48 ` [PATCH v2] " Xi Wang
2011-12-14 21:48   ` Xi Wang
2011-12-15 21:07   ` Vlad Yasevich
2011-12-15 21:07     ` Vlad Yasevich
2011-12-15 22:13     ` Xi Wang
2011-12-15 22:13       ` Xi Wang
2011-12-16 13:00       ` Vlad Yasevich
2011-12-16 13:00         ` Vlad Yasevich
2011-12-16 22:25         ` Xi Wang
2011-12-16 22:25           ` Xi Wang
2011-12-16 22:44 ` [PATCH v3] " Xi Wang
2011-12-16 22:44   ` Xi Wang
2012-01-03 15:52   ` Vladislav Yasevich
2012-01-03 15:52     ` Vladislav Yasevich

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4EE67DC6.3000500@hp.com \
    --to=vladislav.yasevich@hp.com \
    --cc=akpm@linux-foundation.org \
    --cc=andrei@iptel.org \
    --cc=davem@davemloft.net \
    --cc=linux-sctp@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=xi.wang@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.