Removing attribute/vendor/type from valuepairs
Hi All, A number of commits have been pushed to master branch to remove the attribute, vendor, type and flags fields from the VLAUE_PAIR struct. When VALUE_PAIRs are created, the attribute number and vendor are resolved to a DICT_ATTR, the DICT_ATTR is then used as the source of type information for the VALUE_PAIR. For attributes not found in the FreeRADIUS dictionary, instances of DICT_ATTR are dynamically allocated, then freed when the VALUE_PAIR is freed. Alan has also rewritten the RADIUS protocol decoder to better support nested attributes, which should be fully supported before 3.0 is released. Next week work will begin on integrating talloc. talloc is a hierachical memory allocator which can dramatically simplify memory management for trees of dynamically allocated objects. This is in preparation for adding request chaining* and nested TLVs. As talloc is small (3K LoC) we plan to bundle it with the server, and integrate it into the new build system in a similar way to jlibtool. As always testing is appreciated. -Arran * Initially this will be for proxying between multiple virtual servers internally, but will make it easier to implement request trees, where a single incoming request spawns multiple child requests.
On 02/16/2013 06:04 PM, Arran Cudbard-Bell wrote:
As talloc is small (3K LoC) we plan to bundle it with the server, and integrate it into the new build system in a similar way to jlibtool.
I applaud the use of talloc, we use it in many of the projects we develop (i.e. sssd). But why bundle it? It's available as a installable package (libtalloc) on most distributions especially now that sssd is widely adopted. -- John Dennis <jdennis@redhat.com> Looking to carve out IT costs? www.redhat.com/carveoutcosts/
John Dennis wrote:
I applaud the use of talloc, we use it in many of the projects we develop (i.e. sssd). But why bundle it? It's available as a installable package (libtalloc) on most distributions especially now that sssd is widely adopted.
Well, FreeRADIUS is portable to many systems which don't have sssd. Maybe I'm old-school, but projects which don't build are annoying. i.e. download the tar file, configure.... fail. Download dependency library. re-configure. fail. Fight with configure's caching system. Download another dependency library. Continue. For nearly 15 years, FreeRADIUS has had a reputation as a project which "just works". I'm wary of changing that now. My inclination is to bundle talloc.[ch], and build them only if "configure" fails to detect a system talloc library. Unlike libtool/libltdl, the talloc API changes rarely. We can therefore depend on it "just working", and don't have to write tons of horrible configure checks. Alan DeKok.
Arran Cudbard-Bell wrote:
Alan has also rewritten the RADIUS protocol decoder to better support nested attributes, which should be fully supported before 3.0 is released.
I think that makes 4 implementations of the decoder in the past 14 years. Oh well, I'll get it right one of these days. :) For the record, the new decoder was written to support the WiMAX "continued" and Extended "long" attributes. It also came out of the RADIUS data type draft I wrote: http://tools.ietf.org/html/draft-dekok-radext-datatype So far, the code is again smaller, and easier to understand than the previous decoder.
Next week work will begin on integrating talloc. talloc is a hierachical memory allocator which can dramatically simplify memory management for trees of dynamically allocated objects. This is in preparation for adding request chaining* and nested TLVs.
As talloc is small (3K LoC) we plan to bundle it with the server, and integrate it into the new build system in a similar way to jlibtool.
As always testing is appreciated.
We're just in the process of running Coverity over the "master" branch. It hasn't been done for a while, so there are a number of issues. So far they seem to be mainly "leak memory on rare error condition" kind of problem. They're minor, but we're fixing them for sanity reasons.
* Initially this will be for proxying between multiple virtual servers internally, but will make it easier to implement request trees, where a single incoming request spawns multiple child requests.
The current REQUEST structure has fields explicitly for proxying. That is arguably wrong. Those fields should be removed, and replaced with parent/child relationships. In the short term, it makes code dealing with proxied packets a little more difficult. In the long term, it makes the server much more flexible. As Arran says, it allows for proxying between virtual servers. This will be near-zero-cost proxying. And will allow much more fine-grained division of policies. Alan DeKok.
participants (3)
-
Alan DeKok -
Arran Cudbard-Bell -
John Dennis