Request - Modification to Status Server to have Clients Stats in separate VSAs

[Peter Lambrechtsen]

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.freeradius

#
# 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