Request - Modification to Status Server to have Clients Stats in separate VSAs
I'm currently playing around with the Status Server for stats gathering and was hoping to have a minor change in the interface to support extracting the Client information under different VSAs rather than reusing the current "FreeRADIUS-Total-*" VSAs. It's not particularly logical in my view to have the same VSA returned twice in a Access-Accept, and then needing to determine if it's the client related one or the global server related one by it's sequence in the payload. It would make more sense in my mind to have the Client stats as separate VSAs. For this new VSAs for the FreeRADIUS-Client-* or FreeRADIUS-Stats-Client-* would be required since then the Total server values vs . I'm suggesting adding add them onto the current list numbering scheme or the other option is to add 100 to the current numbers. https://github.com/FreeRADIUS/freeradius-server/blob/master/share/dictionary... # # Client authentication statistics for packets received by the server. # ATTRIBUTE FreeRADIUS-Client-Access-Requests 186 integer ... ATTRIBUTE FreeRADIUS-Client-Auth-Unknown-Types 195 integer # # Client accounting statistics for packets received by the server. # ATTRIBUTE FreeRADIUS-Client-Accounting-Requests 196 integer ... ATTRIBUTE FreeRADIUS-Client-Acct-Unknown-Types 202 integer or # # Client authentication statistics for packets received by the server. # ATTRIBUTE FreeRADIUS-Client-Access-Requests 228 integer ... ATTRIBUTE FreeRADIUS-Client-Auth-Unknown-Types 237 integer # # Client accounting statistics for packets received by the server. # ATTRIBUTE FreeRADIUS-Client-Accounting-Requests 248 integer ... ATTRIBUTE FreeRADIUS-Client-Acct-Unknown-Types 254 integer Then update the stats to use those AVPs: https://github.com/FreeRADIUS/freeradius-server/blob/master/src/main/stats.c Line 406: static fr_stats2vp client_authvp[] = { { 186, offsetof(fr_stats_t, total_requests) }, ... { 195, offsetof(fr_stats_t, total_unknown_types) }, { 0, 0 } }; #ifdef WITH_ACCOUNTING static fr_stats2vp client_acctvp[] = { { 196, offsetof(fr_stats_t, total_requests) }, ... { 202, offsetof(fr_stats_t, total_unknown_types) }, { 0, 0 } }; #endif Which ever you believe is the more appropriate range to use either adding onto the sequential numbering scheme, or adding 100 (I suspect adding onto the existing range would be better) And also change around line 624 where it does the validation to see if you want client information or not: if ((flag->vp_integer & 0x01) != 0) { request_stats_addvp(request, client_authvp, &client->auth); } #ifdef WITH_ACCOUNTING if ((flag->vp_integer & 0x01) != 0) { request_stats_addvp(request, client_acctvp, &client->acct); } #endif I would just add the client VSA attributes in no matter what since if you are requesting client information with bit 0x20 you wanted client information, no point in having a check to see if the auth bit is turned on, so I would have it: request_stats_addvp(request, client_authvp, &client->auth); #ifdef WITH_ACCOUNTING request_stats_addvp(request, client_acctvp, &client->acct); #endif Another idiosyncrasy is if you query by "FreeRADIUS-Stats-Client-IP-Address" then that vp gets copied back as per line 590: pairadd(&request->reply->vps, paircopyvp(request->reply, vp)); But you have no way to know which client number was used in that response and if that client was a static clients.conf client, a dynamic client or one defined in the status-server . Since the other interesting thing I noticed was Clients that are defined in the Status Server site also add onto the total list of clients assigned to the server. So if your clients.conf has 8 clients defined, clients 0 -> 7 are from the clients.conf, client 8 is the first client defined in the status server config. So it would be nice if the "Client-Type" or "Client belongs to server base/dynamic/status/xxx" could be returned as an extra VSA. The other thing that would be nice would be to add IPv6 support into the Stats engine too, but that would require changes to client.c as well since the "client_find" function would need to be updated to include v6 lookup as well as the current v4. I know patches are welcome and I will work on getting a patch for the client to always return the client values and to have client specific VSAs, but thought I should ask first as the status server is part of the server core before I created something. Thoughts and comments? Cheers Peter
Peter Lambrechtsen wrote:
I'm currently playing around with the Status Server for stats gathering and was hoping to have a minor change in the interface to support extracting the Client information under different VSAs rather than reusing the current "FreeRADIUS-Total-*" VSAs. It's not particularly logical in my view to have the same VSA returned twice in a Access-Accept, and then needing to determine if it's the client related one or the global server related one by it's sequence in the payload. It would make more sense in my mind to have the Client stats as separate VSAs.
They should be separate VSAs. I'm not sure what crack I was smoking when I cam up with the current VSAs, but oh well. My inclination now is to leverage RFC 6929 and the extended attributes, and TLVs. We could create VSAs which mirror the SNMP information in RFC 4668..4671. Or, we could create our own TLVs in a sane format. e.g. have a "parent" TLV for home server, client, and "this" server. Inside of that parent, have the *same* list of child TLVs, in the same order, with the same meaning. I'm inclined to leave 2.x the way it is. It's imperfect, but it's workable. The new 3.0 code is preferred for new functionality. The nice thing about the 3.0 code is that if you're using the 3.0 radclient, you don't really care that the counters are buried inside of TLVs. They come out in "attr = value" form, just like anything else. Alan DeKok.
On Wed, Aug 21, 2013 at 2:04 AM, Alan DeKok <aland@deployingradius.com> wrote:
Peter Lambrechtsen wrote:
I'm currently playing around with the Status Server for stats gathering and was hoping to have a minor change in the interface to support extracting the Client information under different VSAs rather than reusing the current "FreeRADIUS-Total-*" VSAs. It's not particularly logical in my view to have the same VSA returned twice in a Access-Accept, and then needing to determine if it's the client related one or the global server related one by it's sequence in the payload. It would make more sense in my mind to have the Client stats as separate VSAs.
They should be separate VSAs. I'm not sure what crack I was smoking when I cam up with the current VSAs, but oh well.
My inclination now is to leverage RFC 6929 and the extended attributes, and TLVs. We could create VSAs which mirror the SNMP information in RFC 4668..4671.
Or, we could create our own TLVs in a sane format. e.g. have a "parent" TLV for home server, client, and "this" server. Inside of that parent, have the *same* list of child TLVs, in the same order, with the same meaning.
I'm inclined to leave 2.x the way it is. It's imperfect, but it's workable. The new 3.0 code is preferred for new functionality.
When looking through the master branch the suggestions I were making were to master, so the way that 3.0 is planning do things. I think the TLV approach is a valid one. One nice part of that is the ability to list all servers/proxies/clients could be listed in a tree structure way.
The nice thing about the 3.0 code is that if you're using the 3.0 radclient, you don't really care that the counters are buried inside of TLVs. They come out in "attr = value" form, just like anything else.
Nice, right now I have modified TinyRadius java client as that suits my needs quite nicely after adding the Status Message and support for Message-Authenticator as we have an exiting java monitoring tool that monitors all our other services across ldap/sql/http and a whole myriad of other systems for service and server availability and I have easily extended it to use the Status call via TinyRadius and that's working a treat. Currently wiring a bit more logic into it to handle peaks & dips so when one of the BNG(s)/NAS goes AWOL or decides to swap between primary/secondary FreeRadius instance which it doesn't alert from in the BNG Syslog which is a bit of a bother or for some reason gets a rapid increase in requests from a interface failure or other random things that can occur in the network we can send an alert to the relevant team to investigate. As that sort of stat analysis is what we are looking to achieve without building a massively complex solution that will be hard to maintain. If it's ok I will look to extend the current dictionary to include FreeRadius-Client-* using sequential numbers and those few minor changes in stats.c. And at least push that as a possible patch for 3.x and perhaps 2.x as it's pretty much the same code. Then when further thoughts have been done about the best way to do it long term has been decided then that can be supported too in 3. Cheers Peter
Peter Lambrechtsen wrote:
If it's ok I will look to extend the current dictionary to include FreeRadius-Client-* using sequential numbers and those few minor changes in stats.c. And at least push that as a possible patch for 3.x and perhaps 2.x as it's pretty much the same code. Then when further thoughts have been done about the best way to do it long term has been decided then that can be supported too in 3.
I'd rather just do it the right way for 3.x. There's no point in adding things which we know will be removed later. Alan DeKok.
participants (2)
-
Alan DeKok -
Peter Lambrechtsen