Commit report for master branch

Alan DeKok aland at deployingradius.com
Wed Apr 10 16:00:07 CEST 2013


Brian Candler wrote:
> Is this configurable, so that this preprocessing can be disabled for a
> particular module?

  Not right now.

> It's just that %{exec:...} and %{redis:...} expect to get a single un-xlat'd
> string, which they split on space, and then xlat the parts individually.

  Yes.  That can be fixed with simple changes.

  a) the module supplies an "escape" function which escapes spaces

  b) the "split on spaces" code converts escaped characters to the
non-escaped version... but doesn't split on escaped spaces

  One other downside of the new code is that it does a LOT of memory
allocation for each call to radius_xlat().  The old code allocated tons
of memory on the stack, which was faster.

  The goal is to both decrease the memory allocation, and also to do
less dynamic parsing of expansions.

  i.e. the "files" module should parse the xlat expansion once, and save
the parsed result.  It can then be interpreted dynamically multiple times.

  The configuration file parser should do that, too.  The conditions
should be parsed once, along with with the xlat expansions.  Then, the
parsed tree should be evaluated at run time.

  The end result will be less memory used, less CPU time used, and less
code.  For example, even with new support functions, the new xlat code
is ~400 LoC smaller than the old one.  And MUCH more understandable.

  The other benefit is good errors:

...
(0)     update reply {
(0) ERROR: %{expr: %{Stripped-User-Name} %X * 12}
(0) ERROR:                                ^ Invalid variable expansion
...

  Each parse error now has a unique error message.  The error message is
used to print the location where the error occurred.  This means it's
MUCH easier to understand what's gone wrong.

  The new errors will be used in the config files && users file.  So
mis-typing an xlat expansion means that the server will get a syntax
error on the config files, and refuse to use the bad data.

  Over all, this is much better.  The main cost is additional memory
allocation, which can be slow.  However, this is 2013.  If your RADIUS
server is CPU bound, you're probably doing something wrong.

  Alan DeKok.


More information about the Freeradius-Devel mailing list