Re: Question about socket timeouts
On Thu, Aug 29, 2013 at 6:03 PM, John Baldwin j...@freebsd.org wrote: On Monday, August 26, 2013 3:05:06 pm John Baldwin wrote: On Monday, August 26, 2013 2:23:44 pm Davide Italiano wrote: Please consider the following patch: http://people.freebsd.org/~davide/review/socket_timeout.diff I've tested it and it works OK. I got a timeout which is ~= 25ms using the testcase provided by the user. The only doubt I have is about the range check, I've changed a bit because the 'integer' part of sbintime_t fits in 32-bits, but I'm not sure it's the best way of doing this. Nice! Bruce actually wants me to adjust the range check a bit (which will fit in well with your changes I think). Please let me get that fix in (so it can be part of the future MFC) and then you can commit this. Thanks! Actually, I think you still need to patch the sogetopt() case to work correctly (it is still doing a manual conversion from 'val' to a timeval assuming it is in 'hz' units). I'm done with my range check changes, please move forward with your change (though make sure you fix the sogetsockopt() case please). -- John Baldwin For the archives, this should be fixed in r255138. Thanks to both, -- Davide There are no solved problems; there are only problems that are more or less solved -- Henri Poincare ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: Question about socket timeouts
On Tue, Aug 27, 2013 at 7:10 AM, Vitja Makarov vitja.maka...@gmail.com wrote: On Fri, Aug 23, 2013 at 7:04 AM, Vitja Makarov vitja.maka...@gmail.com wrote: 2013/8/23 Davide Italiano dav...@freebsd.org: I think that for socket's timeouts it's ok to have a HZ-precision. It would be much more important to implement high-precision timeouts for select() and friends, if it's not done yet (sorry I'm running 9.1). JFYI, select()/usleep()/etc... are all fine grained right now in HEAD. That's cool! Does that mean that FreeBSD 10 would be a tickless system? -- vitja. FreeBSD will have a tickless callout(9) subsystem. There are still some kernel subsystems that depends on hardclock() even if the long term goal is that of moving away from it (when/if possible). A notable example is SCHED_ULE code which depends on hardclock() (sched_tick()) but it could be optimized to skip some calls even though CPU is active. -- Davide There are no solved problems; there are only problems that are more or less solved -- Henri Poincare ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: Question about socket timeouts
On Monday, August 26, 2013 3:05:06 pm John Baldwin wrote: On Monday, August 26, 2013 2:23:44 pm Davide Italiano wrote: Please consider the following patch: http://people.freebsd.org/~davide/review/socket_timeout.diff I've tested it and it works OK. I got a timeout which is ~= 25ms using the testcase provided by the user. The only doubt I have is about the range check, I've changed a bit because the 'integer' part of sbintime_t fits in 32-bits, but I'm not sure it's the best way of doing this. Nice! Bruce actually wants me to adjust the range check a bit (which will fit in well with your changes I think). Please let me get that fix in (so it can be part of the future MFC) and then you can commit this. Thanks! Actually, I think you still need to patch the sogetopt() case to work correctly (it is still doing a manual conversion from 'val' to a timeval assuming it is in 'hz' units). I'm done with my range check changes, please move forward with your change (though make sure you fix the sogetsockopt() case please). -- John Baldwin ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: Question about socket timeouts
2013/8/26 Davide Italiano dav...@freebsd.org: Please consider the following patch: http://people.freebsd.org/~davide/review/socket_timeout.diff I've tested it and it works OK. I got a timeout which is ~= 25ms using the testcase provided by the user. The only doubt I have is about the range check, I've changed a bit because the 'integer' part of sbintime_t fits in 32-bits, but I'm not sure it's the best way of doing this. Nice, that worked for me! I got ~26ms timeouts in average on my VM. On Fri, Aug 23, 2013 at 7:04 AM, Vitja Makarov vitja.maka...@gmail.com wrote: 2013/8/23 Davide Italiano dav...@freebsd.org: I think that for socket's timeouts it's ok to have a HZ-precision. It would be much more important to implement high-precision timeouts for select() and friends, if it's not done yet (sorry I'm running 9.1). JFYI, select()/usleep()/etc... are all fine grained right now in HEAD. That's cool! Does that mean that FreeBSD 10 would be a tickless system? -- vitja. ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: Question about socket timeouts
On Fri, Aug 23, 2013 at 5:29 PM, John Baldwin j...@freebsd.org wrote:
On Friday, August 23, 2013 9:53:12 am Davide Italiano wrote:
On Fri, Aug 23, 2013 at 3:45 PM, John Baldwin j...@freebsd.org wrote:
On Friday, August 23, 2013 2:27:58 am Vitja Makarov wrote:
2013/8/22 John Baldwin j...@freebsd.org:
On Thursday, August 22, 2013 12:18:48 am Vitja Makarov wrote:
2013/8/21 John Baldwin j...@freebsd.org:
On Monday, August 19, 2013 11:13:02 pm Daniel Eischen wrote:
On Mon, 19 Aug 2013, Adrian Chadd wrote:
Yes! Please file a PR!
This sorta implies that both are acceptable (although,
the Linux behavior seems more desirable).
http://austingroupbugs.net/view.php?id=369
No, that says round up, so it does mean that the requested timeout
should be the minimum amount slept. tvtohz() does this. Really odd
that the socket code is using its own version of this rather than
tvtohz().
Oh, I bet this just predates tvtohz(). Interesting that it keeps
getting
bug fixes in its history that simply using tvtohz() would have
solved.
Try this:
Index: uipc_socket.c
===
--- uipc_socket.c (revision 254570)
+++ uipc_socket.c (working copy)
@@ -2699,21 +2699,16 @@ sosetopt(struct socket *so, struct sockopt
*sopt)
if (error)
goto bad;
- /* assert(hz 0); */
if (tv.tv_sec 0 || tv.tv_sec INT_MAX /
hz ||
tv.tv_usec 0 || tv.tv_usec = 100)
{
error = EDOM;
goto bad;
}
- /* assert(tick 0); */
- /* assert(ULONG_MAX - INT_MAX = 100);
*/
- val = (u_long)(tv.tv_sec * hz) + tv.tv_usec
/ tick;
- if (val INT_MAX) {
+ val = tvtohz(tv);
+ if (val == INT_MAX) {
error = EDOM;
goto bad;
}
- if (val == 0 tv.tv_usec != 0)
- val = 1;
switch (sopt-sopt_name) {
case SO_SNDTIMEO:
That must help. But I want to see the issue solved in the next
release. I can't apply patch to the production system. Btw in
production environment we have kern.hz set to 1000 so it's not a
problem there.
Can you test this in some way in a test environment?
Ok, sorry for posting out of the list.
Simple test program is attached. Without your patch timeout expires in
about 20ms. With it it's ~40ms.
40 instead of 30 is beacuse of odd tick added by tvtohz().
Ok, thanks. tvtohz() will be good to MFC (and I will do that), but for
HEAD I think we can fix this to use a precise timeout. I've cc'd davide@
so he can take a look at that.
--
John Baldwin
Hi,
I think I can switch this code to new timeout KPI, but this will
require the timeout field of 'struct sockbuf' to be changed from 'int'
to 'sbintime_t' which breaks binary compatibility. Do you have any
strong objections about this? If any, I would like this to happen ABI
freeze, so it looks like this is the right moment.
This should be fine to change now, it just can't be MFC'd (which we can't
do anyway).
--
John Baldwin
Please consider the following patch:
http://people.freebsd.org/~davide/review/socket_timeout.diff
I've tested it and it works OK. I got a timeout which is ~= 25ms using
the testcase provided by the user.
The only doubt I have is about the range check, I've changed a bit
because the 'integer' part of sbintime_t fits in 32-bits, but I'm not
sure it's the best way of doing this.
Thanks,
--
Davide
There are no solved problems; there are only problems that are more
or less solved -- Henri Poincare
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: Question about socket timeouts
On Fri, Aug 23, 2013 at 7:04 AM, Vitja Makarov vitja.maka...@gmail.com wrote: 2013/8/23 Davide Italiano dav...@freebsd.org: I think that for socket's timeouts it's ok to have a HZ-precision. It would be much more important to implement high-precision timeouts for select() and friends, if it's not done yet (sorry I'm running 9.1). JFYI, select()/usleep()/etc... are all fine grained right now in HEAD. -- vitja. -- Davide There are no solved problems; there are only problems that are more or less solved -- Henri Poincare ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: Question about socket timeouts
On Monday, August 26, 2013 2:23:44 pm Davide Italiano wrote:
On Fri, Aug 23, 2013 at 5:29 PM, John Baldwin j...@freebsd.org wrote:
On Friday, August 23, 2013 9:53:12 am Davide Italiano wrote:
On Fri, Aug 23, 2013 at 3:45 PM, John Baldwin j...@freebsd.org wrote:
On Friday, August 23, 2013 2:27:58 am Vitja Makarov wrote:
2013/8/22 John Baldwin j...@freebsd.org:
On Thursday, August 22, 2013 12:18:48 am Vitja Makarov wrote:
2013/8/21 John Baldwin j...@freebsd.org:
On Monday, August 19, 2013 11:13:02 pm Daniel Eischen wrote:
On Mon, 19 Aug 2013, Adrian Chadd wrote:
Yes! Please file a PR!
This sorta implies that both are acceptable (although,
the Linux behavior seems more desirable).
http://austingroupbugs.net/view.php?id=369
No, that says round up, so it does mean that the requested
timeout
should be the minimum amount slept. tvtohz() does this. Really
odd
that the socket code is using its own version of this rather than
tvtohz().
Oh, I bet this just predates tvtohz(). Interesting that it keeps
getting
bug fixes in its history that simply using tvtohz() would have
solved.
Try this:
Index: uipc_socket.c
===
--- uipc_socket.c (revision 254570)
+++ uipc_socket.c (working copy)
@@ -2699,21 +2699,16 @@ sosetopt(struct socket *so, struct
sockopt
*sopt)
if (error)
goto bad;
- /* assert(hz 0); */
if (tv.tv_sec 0 || tv.tv_sec INT_MAX
/
hz ||
tv.tv_usec 0 || tv.tv_usec =
100)
{
error = EDOM;
goto bad;
}
- /* assert(tick 0); */
- /* assert(ULONG_MAX - INT_MAX =
100);
*/
- val = (u_long)(tv.tv_sec * hz) +
tv.tv_usec
/ tick;
- if (val INT_MAX) {
+ val = tvtohz(tv);
+ if (val == INT_MAX) {
error = EDOM;
goto bad;
}
- if (val == 0 tv.tv_usec != 0)
- val = 1;
switch (sopt-sopt_name) {
case SO_SNDTIMEO:
That must help. But I want to see the issue solved in the next
release. I can't apply patch to the production system. Btw in
production environment we have kern.hz set to 1000 so it's not a
problem there.
Can you test this in some way in a test environment?
Ok, sorry for posting out of the list.
Simple test program is attached. Without your patch timeout expires in
about 20ms. With it it's ~40ms.
40 instead of 30 is beacuse of odd tick added by tvtohz().
Ok, thanks. tvtohz() will be good to MFC (and I will do that), but for
HEAD I think we can fix this to use a precise timeout. I've cc'd
davide@
so he can take a look at that.
--
John Baldwin
Hi,
I think I can switch this code to new timeout KPI, but this will
require the timeout field of 'struct sockbuf' to be changed from 'int'
to 'sbintime_t' which breaks binary compatibility. Do you have any
strong objections about this? If any, I would like this to happen ABI
freeze, so it looks like this is the right moment.
This should be fine to change now, it just can't be MFC'd (which we can't
do anyway).
--
John Baldwin
Please consider the following patch:
http://people.freebsd.org/~davide/review/socket_timeout.diff
I've tested it and it works OK. I got a timeout which is ~= 25ms using
the testcase provided by the user.
The only doubt I have is about the range check, I've changed a bit
because the 'integer' part of sbintime_t fits in 32-bits, but I'm not
sure it's the best way of doing this.
Nice! Bruce actually wants me to adjust the range check a bit (which will
fit in well with your changes I think). Please let me get that fix in
(so it can be part of the future MFC) and then you can commit this. Thanks!
Actually, I think you still need to patch the sogetopt() case to work correctly
(it is still doing a manual conversion from 'val' to a timeval assuming it is
in 'hz' units).
--
John Baldwin
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: Question about socket timeouts
2013/8/22 John Baldwin j...@freebsd.org:
On Thursday, August 22, 2013 12:18:48 am Vitja Makarov wrote:
2013/8/21 John Baldwin j...@freebsd.org:
On Monday, August 19, 2013 11:13:02 pm Daniel Eischen wrote:
On Mon, 19 Aug 2013, Adrian Chadd wrote:
Yes! Please file a PR!
This sorta implies that both are acceptable (although,
the Linux behavior seems more desirable).
http://austingroupbugs.net/view.php?id=369
No, that says round up, so it does mean that the requested timeout
should be the minimum amount slept. tvtohz() does this. Really odd
that the socket code is using its own version of this rather than
tvtohz().
Oh, I bet this just predates tvtohz(). Interesting that it keeps getting
bug fixes in its history that simply using tvtohz() would have solved.
Try this:
Index: uipc_socket.c
===
--- uipc_socket.c (revision 254570)
+++ uipc_socket.c (working copy)
@@ -2699,21 +2699,16 @@ sosetopt(struct socket *so, struct sockopt *sopt)
if (error)
goto bad;
- /* assert(hz 0); */
if (tv.tv_sec 0 || tv.tv_sec INT_MAX / hz ||
tv.tv_usec 0 || tv.tv_usec = 100) {
error = EDOM;
goto bad;
}
- /* assert(tick 0); */
- /* assert(ULONG_MAX - INT_MAX = 100); */
- val = (u_long)(tv.tv_sec * hz) + tv.tv_usec / tick;
- if (val INT_MAX) {
+ val = tvtohz(tv);
+ if (val == INT_MAX) {
error = EDOM;
goto bad;
}
- if (val == 0 tv.tv_usec != 0)
- val = 1;
switch (sopt-sopt_name) {
case SO_SNDTIMEO:
That must help. But I want to see the issue solved in the next
release. I can't apply patch to the production system. Btw in
production environment we have kern.hz set to 1000 so it's not a
problem there.
Can you test this in some way in a test environment?
Ok, sorry for posting out of the list.
Simple test program is attached. Without your patch timeout expires in
about 20ms. With it it's ~40ms.
40 instead of 30 is beacuse of odd tick added by tvtohz().
--
vitja.
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: Question about socket timeouts
On Friday, August 23, 2013 2:27:58 am Vitja Makarov wrote:
2013/8/22 John Baldwin j...@freebsd.org:
On Thursday, August 22, 2013 12:18:48 am Vitja Makarov wrote:
2013/8/21 John Baldwin j...@freebsd.org:
On Monday, August 19, 2013 11:13:02 pm Daniel Eischen wrote:
On Mon, 19 Aug 2013, Adrian Chadd wrote:
Yes! Please file a PR!
This sorta implies that both are acceptable (although,
the Linux behavior seems more desirable).
http://austingroupbugs.net/view.php?id=369
No, that says round up, so it does mean that the requested timeout
should be the minimum amount slept. tvtohz() does this. Really odd
that the socket code is using its own version of this rather than
tvtohz().
Oh, I bet this just predates tvtohz(). Interesting that it keeps getting
bug fixes in its history that simply using tvtohz() would have solved.
Try this:
Index: uipc_socket.c
===
--- uipc_socket.c (revision 254570)
+++ uipc_socket.c (working copy)
@@ -2699,21 +2699,16 @@ sosetopt(struct socket *so, struct sockopt *sopt)
if (error)
goto bad;
- /* assert(hz 0); */
if (tv.tv_sec 0 || tv.tv_sec INT_MAX / hz ||
tv.tv_usec 0 || tv.tv_usec = 100) {
error = EDOM;
goto bad;
}
- /* assert(tick 0); */
- /* assert(ULONG_MAX - INT_MAX = 100); */
- val = (u_long)(tv.tv_sec * hz) + tv.tv_usec /
tick;
- if (val INT_MAX) {
+ val = tvtohz(tv);
+ if (val == INT_MAX) {
error = EDOM;
goto bad;
}
- if (val == 0 tv.tv_usec != 0)
- val = 1;
switch (sopt-sopt_name) {
case SO_SNDTIMEO:
That must help. But I want to see the issue solved in the next
release. I can't apply patch to the production system. Btw in
production environment we have kern.hz set to 1000 so it's not a
problem there.
Can you test this in some way in a test environment?
Ok, sorry for posting out of the list.
Simple test program is attached. Without your patch timeout expires in
about 20ms. With it it's ~40ms.
40 instead of 30 is beacuse of odd tick added by tvtohz().
Ok, thanks. tvtohz() will be good to MFC (and I will do that), but for
HEAD I think we can fix this to use a precise timeout. I've cc'd davide@
so he can take a look at that.
--
John Baldwin
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: Question about socket timeouts
On Fri, Aug 23, 2013 at 3:45 PM, John Baldwin j...@freebsd.org wrote:
On Friday, August 23, 2013 2:27:58 am Vitja Makarov wrote:
2013/8/22 John Baldwin j...@freebsd.org:
On Thursday, August 22, 2013 12:18:48 am Vitja Makarov wrote:
2013/8/21 John Baldwin j...@freebsd.org:
On Monday, August 19, 2013 11:13:02 pm Daniel Eischen wrote:
On Mon, 19 Aug 2013, Adrian Chadd wrote:
Yes! Please file a PR!
This sorta implies that both are acceptable (although,
the Linux behavior seems more desirable).
http://austingroupbugs.net/view.php?id=369
No, that says round up, so it does mean that the requested timeout
should be the minimum amount slept. tvtohz() does this. Really odd
that the socket code is using its own version of this rather than
tvtohz().
Oh, I bet this just predates tvtohz(). Interesting that it keeps
getting
bug fixes in its history that simply using tvtohz() would have solved.
Try this:
Index: uipc_socket.c
===
--- uipc_socket.c (revision 254570)
+++ uipc_socket.c (working copy)
@@ -2699,21 +2699,16 @@ sosetopt(struct socket *so, struct sockopt
*sopt)
if (error)
goto bad;
- /* assert(hz 0); */
if (tv.tv_sec 0 || tv.tv_sec INT_MAX / hz ||
tv.tv_usec 0 || tv.tv_usec = 100) {
error = EDOM;
goto bad;
}
- /* assert(tick 0); */
- /* assert(ULONG_MAX - INT_MAX = 100); */
- val = (u_long)(tv.tv_sec * hz) + tv.tv_usec /
tick;
- if (val INT_MAX) {
+ val = tvtohz(tv);
+ if (val == INT_MAX) {
error = EDOM;
goto bad;
}
- if (val == 0 tv.tv_usec != 0)
- val = 1;
switch (sopt-sopt_name) {
case SO_SNDTIMEO:
That must help. But I want to see the issue solved in the next
release. I can't apply patch to the production system. Btw in
production environment we have kern.hz set to 1000 so it's not a
problem there.
Can you test this in some way in a test environment?
Ok, sorry for posting out of the list.
Simple test program is attached. Without your patch timeout expires in
about 20ms. With it it's ~40ms.
40 instead of 30 is beacuse of odd tick added by tvtohz().
Ok, thanks. tvtohz() will be good to MFC (and I will do that), but for
HEAD I think we can fix this to use a precise timeout. I've cc'd davide@
so he can take a look at that.
--
John Baldwin
Hi,
I think I can switch this code to new timeout KPI, but this will
require the timeout field of 'struct sockbuf' to be changed from 'int'
to 'sbintime_t' which breaks binary compatibility. Do you have any
strong objections about this? If any, I would like this to happen ABI
freeze, so it looks like this is the right moment.
Thanks,
--
Davide
There are no solved problems; there are only problems that are more
or less solved -- Henri Poincare
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: Question about socket timeouts
2013/8/23 Davide Italiano dav...@freebsd.org:
On Fri, Aug 23, 2013 at 3:45 PM, John Baldwin j...@freebsd.org wrote:
On Friday, August 23, 2013 2:27:58 am Vitja Makarov wrote:
2013/8/22 John Baldwin j...@freebsd.org:
On Thursday, August 22, 2013 12:18:48 am Vitja Makarov wrote:
2013/8/21 John Baldwin j...@freebsd.org:
On Monday, August 19, 2013 11:13:02 pm Daniel Eischen wrote:
On Mon, 19 Aug 2013, Adrian Chadd wrote:
Yes! Please file a PR!
This sorta implies that both are acceptable (although,
the Linux behavior seems more desirable).
http://austingroupbugs.net/view.php?id=369
No, that says round up, so it does mean that the requested timeout
should be the minimum amount slept. tvtohz() does this. Really odd
that the socket code is using its own version of this rather than
tvtohz().
Oh, I bet this just predates tvtohz(). Interesting that it keeps
getting
bug fixes in its history that simply using tvtohz() would have solved.
Try this:
Index: uipc_socket.c
===
--- uipc_socket.c (revision 254570)
+++ uipc_socket.c (working copy)
@@ -2699,21 +2699,16 @@ sosetopt(struct socket *so, struct sockopt
*sopt)
if (error)
goto bad;
- /* assert(hz 0); */
if (tv.tv_sec 0 || tv.tv_sec INT_MAX / hz
||
tv.tv_usec 0 || tv.tv_usec = 100) {
error = EDOM;
goto bad;
}
- /* assert(tick 0); */
- /* assert(ULONG_MAX - INT_MAX = 100); */
- val = (u_long)(tv.tv_sec * hz) + tv.tv_usec /
tick;
- if (val INT_MAX) {
+ val = tvtohz(tv);
+ if (val == INT_MAX) {
error = EDOM;
goto bad;
}
- if (val == 0 tv.tv_usec != 0)
- val = 1;
switch (sopt-sopt_name) {
case SO_SNDTIMEO:
That must help. But I want to see the issue solved in the next
release. I can't apply patch to the production system. Btw in
production environment we have kern.hz set to 1000 so it's not a
problem there.
Can you test this in some way in a test environment?
Ok, sorry for posting out of the list.
Simple test program is attached. Without your patch timeout expires in
about 20ms. With it it's ~40ms.
40 instead of 30 is beacuse of odd tick added by tvtohz().
Ok, thanks. tvtohz() will be good to MFC (and I will do that), but for
HEAD I think we can fix this to use a precise timeout. I've cc'd davide@
so he can take a look at that.
--
John Baldwin
Hi,
I think I can switch this code to new timeout KPI, but this will
require the timeout field of 'struct sockbuf' to be changed from 'int'
to 'sbintime_t' which breaks binary compatibility. Do you have any
strong objections about this? If any, I would like this to happen ABI
freeze, so it looks like this is the right moment.
I think that for socket's timeouts it's ok to have a HZ-precision. It
would be much more important to implement high-precision timeouts for
select() and friends, if it's not done yet (sorry I'm running 9.1).
--
vitja.
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: Question about socket timeouts
On Friday, August 23, 2013 9:53:12 am Davide Italiano wrote:
On Fri, Aug 23, 2013 at 3:45 PM, John Baldwin j...@freebsd.org wrote:
On Friday, August 23, 2013 2:27:58 am Vitja Makarov wrote:
2013/8/22 John Baldwin j...@freebsd.org:
On Thursday, August 22, 2013 12:18:48 am Vitja Makarov wrote:
2013/8/21 John Baldwin j...@freebsd.org:
On Monday, August 19, 2013 11:13:02 pm Daniel Eischen wrote:
On Mon, 19 Aug 2013, Adrian Chadd wrote:
Yes! Please file a PR!
This sorta implies that both are acceptable (although,
the Linux behavior seems more desirable).
http://austingroupbugs.net/view.php?id=369
No, that says round up, so it does mean that the requested timeout
should be the minimum amount slept. tvtohz() does this. Really odd
that the socket code is using its own version of this rather than
tvtohz().
Oh, I bet this just predates tvtohz(). Interesting that it keeps
getting
bug fixes in its history that simply using tvtohz() would have
solved.
Try this:
Index: uipc_socket.c
===
--- uipc_socket.c (revision 254570)
+++ uipc_socket.c (working copy)
@@ -2699,21 +2699,16 @@ sosetopt(struct socket *so, struct sockopt
*sopt)
if (error)
goto bad;
- /* assert(hz 0); */
if (tv.tv_sec 0 || tv.tv_sec INT_MAX /
hz ||
tv.tv_usec 0 || tv.tv_usec = 100)
{
error = EDOM;
goto bad;
}
- /* assert(tick 0); */
- /* assert(ULONG_MAX - INT_MAX = 100);
*/
- val = (u_long)(tv.tv_sec * hz) + tv.tv_usec
/ tick;
- if (val INT_MAX) {
+ val = tvtohz(tv);
+ if (val == INT_MAX) {
error = EDOM;
goto bad;
}
- if (val == 0 tv.tv_usec != 0)
- val = 1;
switch (sopt-sopt_name) {
case SO_SNDTIMEO:
That must help. But I want to see the issue solved in the next
release. I can't apply patch to the production system. Btw in
production environment we have kern.hz set to 1000 so it's not a
problem there.
Can you test this in some way in a test environment?
Ok, sorry for posting out of the list.
Simple test program is attached. Without your patch timeout expires in
about 20ms. With it it's ~40ms.
40 instead of 30 is beacuse of odd tick added by tvtohz().
Ok, thanks. tvtohz() will be good to MFC (and I will do that), but for
HEAD I think we can fix this to use a precise timeout. I've cc'd davide@
so he can take a look at that.
--
John Baldwin
Hi,
I think I can switch this code to new timeout KPI, but this will
require the timeout field of 'struct sockbuf' to be changed from 'int'
to 'sbintime_t' which breaks binary compatibility. Do you have any
strong objections about this? If any, I would like this to happen ABI
freeze, so it looks like this is the right moment.
This should be fine to change now, it just can't be MFC'd (which we can't
do anyway).
--
John Baldwin
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: Question about socket timeouts
On Monday, August 19, 2013 11:13:02 pm Daniel Eischen wrote:
On Mon, 19 Aug 2013, Adrian Chadd wrote:
Yes! Please file a PR!
This sorta implies that both are acceptable (although,
the Linux behavior seems more desirable).
http://austingroupbugs.net/view.php?id=369
No, that says round up, so it does mean that the requested timeout
should be the minimum amount slept. tvtohz() does this. Really odd
that the socket code is using its own version of this rather than
tvtohz().
Oh, I bet this just predates tvtohz(). Interesting that it keeps getting
bug fixes in its history that simply using tvtohz() would have solved.
Try this:
Index: uipc_socket.c
===
--- uipc_socket.c (revision 254570)
+++ uipc_socket.c (working copy)
@@ -2699,21 +2699,16 @@ sosetopt(struct socket *so, struct sockopt *sopt)
if (error)
goto bad;
- /* assert(hz 0); */
if (tv.tv_sec 0 || tv.tv_sec INT_MAX / hz ||
tv.tv_usec 0 || tv.tv_usec = 100) {
error = EDOM;
goto bad;
}
- /* assert(tick 0); */
- /* assert(ULONG_MAX - INT_MAX = 100); */
- val = (u_long)(tv.tv_sec * hz) + tv.tv_usec / tick;
- if (val INT_MAX) {
+ val = tvtohz(tv);
+ if (val == INT_MAX) {
error = EDOM;
goto bad;
}
- if (val == 0 tv.tv_usec != 0)
- val = 1;
switch (sopt-sopt_name) {
case SO_SNDTIMEO:
--
John Baldwin
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: Question about socket timeouts
Yes! Please file a PR! -adrian On 19 August 2013 12:33, Vitja Makarov vitja.maka...@gmail.com wrote: Hi! Recently I was playing with small socket timeouts. setsockopt(2) SO_RCVTIMEO and found a problem with it: if timeout is small enough read(2) may return before timeout is actually expired. I was unable to reproduce this on linux box. I found that kernel uses a timer with 1/HZ precision so it converts time in microseconds to ticks that's ok linux does it as well. The problem is in details: freebsd uses floor() approach while linux uses ceil(): from FreeBSD's sys/kern/uipc_socket.c: val = (u_long)(tv.tv_sec * hz) + tv.tv_usec / tick; if (val == 0 tv.tv_usec != 0) val = 1; /* at least one tick if tv 0 */ from Linux's net/core/sock.c: *timeo_p = tv.tv_sec*HZ + (tv.tv_usec+(100/HZ-1))/(100/HZ); So, for instance, we have a freebsd system running with kern.hz set 100 and set receive timeout to 25ms that is converted to 2 ticks which is 20ms. In my test program read(2) returns with EAGAIN set in 0.019ms. So the question is: is that a problem or not? -- vitja. ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: Question about socket timeouts
On Mon, 19 Aug 2013, Adrian Chadd wrote: Yes! Please file a PR! This sorta implies that both are acceptable (although, the Linux behavior seems more desirable). http://austingroupbugs.net/view.php?id=369 On 19 August 2013 12:33, Vitja Makarov vitja.maka...@gmail.com wrote: Hi! Recently I was playing with small socket timeouts. setsockopt(2) SO_RCVTIMEO and found a problem with it: if timeout is small enough read(2) may return before timeout is actually expired. I was unable to reproduce this on linux box. I found that kernel uses a timer with 1/HZ precision so it converts time in microseconds to ticks that's ok linux does it as well. The problem is in details: freebsd uses floor() approach while linux uses ceil(): from FreeBSD's sys/kern/uipc_socket.c: val = (u_long)(tv.tv_sec * hz) + tv.tv_usec / tick; if (val == 0 tv.tv_usec != 0) val = 1; /* at least one tick if tv 0 */ from Linux's net/core/sock.c: *timeo_p = tv.tv_sec*HZ + (tv.tv_usec+(100/HZ-1))/(100/HZ); So, for instance, we have a freebsd system running with kern.hz set 100 and set receive timeout to 25ms that is converted to 2 ticks which is 20ms. In my test program read(2) returns with EAGAIN set in 0.019ms. So the question is: is that a problem or not? -- vitja. -- DE ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
