Accounting multicast + unacknowledge mode and conflicting packet

[Alan DeKok]

On Apr 23, 2019, at 4:35 PM, François RAGUIN <fraguin at wanadoo.fr> wrote:
> We added Acct-Status-Type + Framed-IP-Address + Calling-Station-Id in the log "Received conflicting".
> This is to inform applications that consume downstream Radius accounting.
> And this while waiting to solve our punctual problems of slow database.

  OK.

> Below is the code for lines 1777 - 1781 of the process.c file before and after modification.
> I do not know the function fr_pair_find_by_num very well.
> I do not know if there is documentation, but if you have a few minutes, could you look at my code and tell me if there are flagrant errors.

> 
>                         char buffer[INET_ADDRSTRLEN];
>                         char const   *call_num, *acct_type = NULL;
>                         VALUE_PAIR   *vpcli, *vpip, *stype;
>                         
>                         
>                         stype = fr_pair_find_by_num(request->packet->vps, PW_ACCT_STATUS_TYPE, 0, TAG_ANY);
>                         vpip = fr_pair_find_by_num(request->packet->vps, PW_FRAMED_IP_ADDRESS, 0, TAG_ANY);
>                         vpcli = fr_pair_find_by_num(request->packet->vps, 0, PW_CALLING_STATION_ID, TAG_ANY);
>  
>  
>                         if (vpip && vpcli && stype ) {
>                         vp_prints_value(buffer, sizeof(buffer), vpip, '"');
>                         call_num = vpcli->vp_strvalue;
>                         acct_type = stype->vp_strvalue;

  You need to initialize the buffer if the "vpip"  isn't found.  And you need to check if the call_num and acct_type attributes have been found.  There is no guarantee that they will be in a packet.

  It's best to have a buffer for each one, and do something like:

	char vpip_buffer[128];
	char call_buffer[512];
	char acct_buffer[128];

	vpip_buffer[0] = 0;
	call_buffer[0] = 0;
	acct_buffer[0] = 0;

  	vpip = fr_pair_find_by_num(request->packet->vps, PW_FRAMED_IP_ADDRESS, 0, TAG_ANY)
	if (vpip) {
		char buffer[16];
		vp_prints_value(buffer, sizeof(buffer), vpip, '"');
		snprintf(vpip_buffer, sizeof(vpip_buffer), "IP-Client: %s ", buffer);
	}

  and do similar things for the other attributes.  Note that you *can't* look at acct_type->vp_strvalue, because it's an integer attribute.  You have to look at acct_type->vp_integer.

  You can then do:

                         ERROR("Received conflicting packet from "
                              "client %s port %d - ID: %u  %s%s%s due to "
                              "unfinished request in module %s.  Giving up on old request.",
                              client->shortname,
                              packet->src_port, packet->id, call_buffer, chip_buffer, acct_buffer,
                              old_module);

  That way your server won't crash if it receives packets without a Calling-Station-Id.

  Alan DeKok.