Re: Autotools related problems in freeradius 1.1.6

2007-04-24 Thread Kostas Zorbadelos
On Mon, Apr 23, 2007 at 04:39:22PM +0200, Alan DeKok wrote:
 Kostas Zorbadelos wrote:
  If I do 
  
  ./configure --prefix=/opt/freeradius
  
  the build scripts presume that --enable-developer is true.
 
   That may be an issue only in 1.1.6.  You should be able to change it
 by doing --disable-developer.
 

This is exactly what I did. The reason I mention it is because I think
the default should be sane in future releases of freeradius (that is
developer options switched off by default).

  This has
  the effect that -DNDEBUG is not defined in CFLAGS during compilation,
  among other things, so the rad_assert() function can abort freeradius
  operation in production environments.
 
   Which is not necessarily a bad thing.  Yes, it's bad for your RADIUS
 server to go down.  It's arguably worse for the RADIUS server to keep
 running, and doing... something... after it notices that internal sanity
 checks have failed.
 

I disagree with you on this one Alan. I discovered all these issues I
mention the hard way, after our radius server stopped running in
random times (after a failure in rad_assert() in request_list.c around
the section 

... 
static int refresh_request(REQUEST *request, void *data)
...

/*
 *  If the request is marked as a delayed reject, AND it's
 *  time to send the reject, then do so now.
 */
if (request-finished 
((request-options  RAD_REQUEST_OPTION_DELAYED_REJECT) != 0)) {
   rad_assert(request-child_pid == NO_SUCH_CHILD_PID);
...)


In production environments the server should be able to at least
report the errors it encounters and continue operations. Service
availability is the most important.

In our case, after I recompiled freeradius with -DNDEBUG option set,
we noticed no further noticable problems in our radius service.  


  I believe that by default, --enable-developer should be false unless
  explicitly set during configure. 
  Let me know if you need anything else to trace the issue.
 
   It's just a couple of lines of shell scripting in configure.in.


As far as I can tell, the following minor patch should take care of the
issue of having developer flags switched off be default:

--- configure.in.orig   Tue Apr 24 12:02:13 2007
+++ configure.inTue Apr 24 12:02:40 2007
@@ -278,11 +278,11 @@
 AC_ARG_ENABLE(developer,
 [  --enable-developer   Enables features of interest to 
developers.],
 [ case $enableval in
-no)
-   developer=no
+yes)
+   developer=yes
;;
 *)
-   developer=yes
+   developer=no
   esac ]
 )


  Moreover, in a Solaris 9 environment
  --enable-developer or --disable-developer seem to be ignored and
  someone should define CFLAGS explicitly in the configure command to
  define -NDEBUG macro.
  
 
I didn't manage to undestand however why in a Solaris environment,
--disable-developer seems to be ignored. Even if I set
--disable-developer in configure, the -DNDEBUG macro is not passed in
compilation options.
Find attached (a gzipped) BUILD log in my environment.

Thanks,

Kostas Zorbadelos



   Alan DeKok.
 --
   http://deployingradius.com   - The web site of the book
   http://deployingradius.com/blog/ - The blog
 - 
 List info/subscribe/unsubscribe? See http://www.freeradius.org/list/users.html
 


BUILD.solaris-disable-developer.log.gz
Description: Binary data
- 
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/users.html

Re: Autotools related problems in freeradius 1.1.6

2007-04-24 Thread Alan DeKok
Kostas Zorbadelos wrote:
 This is exactly what I did. The reason I mention it is because I think
 the default should be sane in future releases of freeradius (that is
 developer options switched off by default).

  That's the intent, yes.

 I disagree with you on this one Alan. I discovered all these issues I
 mention the hard way, after our radius server stopped running in
 random times (after a failure in rad_assert() in request_list.c around
 the section 
...
 In production environments the server should be able to at least
 report the errors it encounters and continue operations. Service
 availability is the most important.

  My point was that it should continue doing *what*?  The assertions are
there to catch catastrophic failures in the code.  If the assertion
trips, it's doing so because the error is non-recoverable.

  If you disable the assertions, the server may look like it's still
running.  But there's no guarantee that it will do anything useful.  It
may crash randomly later, for reasons that are difficult to track down.
 The only *safe* thing to do is to revert to a known working state.
i.e. restart from scratch.

 As far as I can tell, the following minor patch should take care of the
 issue of having developer flags switched off be default:

  OK, thanks.

  Alan DeKok.
--
  http://deployingradius.com   - The web site of the book
  http://deployingradius.com/blog/ - The blog
- 
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/users.html


Re: Autotools related problems in freeradius 1.1.6

2007-04-24 Thread Kostas Zorbadelos
On Tue, Apr 24, 2007 at 01:12:26PM +0200, Alan DeKok wrote:
 Kostas Zorbadelos wrote:
  I disagree with you on this one Alan. I discovered all these issues I
  mention the hard way, after our radius server stopped running in
  random times (after a failure in rad_assert() in request_list.c around
  the section 
 ...
  In production environments the server should be able to at least
  report the errors it encounters and continue operations. Service
  availability is the most important.
 
   My point was that it should continue doing *what*?  The assertions are
 there to catch catastrophic failures in the code.  If the assertion
 trips, it's doing so because the error is non-recoverable.
 
   If you disable the assertions, the server may look like it's still
 running.  But there's no guarantee that it will do anything useful.  It
 may crash randomly later, for reasons that are difficult to track down.
  The only *safe* thing to do is to revert to a known working state.
 i.e. restart from scratch.


In the code snippet I sent, from what I can tell, nothing catastrophic
happens. The code checks to see if it is time to send a delayed reject
back to the client and asserts that there is no child thread that
works on that request. Anyway, if the developer flags are switched off
rad_assert() does nothing. This is the way it is defined:

#ifdef NDEBUG
#define rad_assert(expr) ((void) (0))
#else
#define rad_assert(expr) \
((void) ((expr) ? 0 : \
rad_assert_fail (__FILE__, __LINE__)))
#endif

So if someone compiles freeradius without developer flags he actually
de-activates all assertions :)

 
  As far as I can tell, the following minor patch should take care of the
  issue of having developer flags switched off be default:
 
   OK, thanks.
 

There is the Solaris issue however. I will try to track it down and
send a patch for this too if I can.


Kostas Zorbadelos

   Alan DeKok.
 --
   http://deployingradius.com   - The web site of the book
   http://deployingradius.com/blog/ - The blog
 - 
- 
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/users.html


Autotools related problems in freeradius 1.1.6

2007-04-23 Thread Kostas Zorbadelos
Greetings to all in the list.

I'd like to report an issue in the build scripts of freeradius. I
tried to build version 1.1.6 but the problem exists in earlier
versions too. 

If I do 

./configure --prefix=/opt/freeradius

the build scripts presume that --enable-developer is true. This has
the effect that -DNDEBUG is not defined in CFLAGS during compilation,
among other things, so the rad_assert() function can abort freeradius
operation in production environments.

I believe that by default, --enable-developer should be false unless
explicitly set during configure. Moreover, in a Solaris 9 environment
--enable-developer or --disable-developer seem to be ignored and
someone should define CFLAGS explicitly in the configure command to
define -NDEBUG macro.

Let me know if you need anything else to trace the issue.
Thanks,

Kostas Zorbadelos 
- 
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/users.html


Re: Autotools related problems in freeradius 1.1.6

2007-04-23 Thread Alan DeKok
Kostas Zorbadelos wrote:
 If I do 
 
 ./configure --prefix=/opt/freeradius
 
 the build scripts presume that --enable-developer is true.

  That may be an issue only in 1.1.6.  You should be able to change it
by doing --disable-developer.

 This has
 the effect that -DNDEBUG is not defined in CFLAGS during compilation,
 among other things, so the rad_assert() function can abort freeradius
 operation in production environments.

  Which is not necessarily a bad thing.  Yes, it's bad for your RADIUS
server to go down.  It's arguably worse for the RADIUS server to keep
running, and doing... something... after it notices that internal sanity
checks have failed.

 I believe that by default, --enable-developer should be false unless
 explicitly set during configure. Moreover, in a Solaris 9 environment
 --enable-developer or --disable-developer seem to be ignored and
 someone should define CFLAGS explicitly in the configure command to
 define -NDEBUG macro.
 
 Let me know if you need anything else to trace the issue.

  It's just a couple of lines of shell scripting in configure.in.

  Alan DeKok.
--
  http://deployingradius.com   - The web site of the book
  http://deployingradius.com/blog/ - The blog
- 
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/users.html