From: Alan T. DeKok Date: Tue, 22 Nov 2011 16:49:02 +0000 (+0100) Subject: Shrink the size of the VALUE_PAIR structure X-Git-Tag: release_3_0_0_beta0~485 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=b14fc9d3d978118617dfce7689f2ea154883c993;p=thirdparty%2Ffreeradius-server.git Shrink the size of the VALUE_PAIR structure Now that we're not writing strings to integer attributes, we can dynamically change the size of the VALUE_PAIR. It should be large enough to contain it's necessary fields, and *only* enough of the VALUE_PAIR_DATA structure to contain the type-specific data. This means we save 250 bytes of memory for every integer / date / ipaddr VALUE_PAIR. --- diff --git a/src/lib/radius.c b/src/lib/radius.c index ae458e0063d..3453ff9897a 100644 --- a/src/lib/radius.c +++ b/src/lib/radius.c @@ -2814,6 +2814,7 @@ static ssize_t data2vp_any(const RADIUS_PACKET *packet, int data_offset = 0; DICT_ATTR *da; VALUE_PAIR *vp = NULL; + uint8_t buffer[256]; if (length == 0) { /* @@ -2912,8 +2913,8 @@ static ssize_t data2vp_any(const RADIUS_PACKET *packet, /* * Copy the data to be decrypted */ - vp->length = length - data_offset; - memcpy(&vp->vp_octets[0], data + data_offset, vp->length); + vp->length = length - data_offset; + memcpy(buffer, data + data_offset, vp->length); /* * Decrypt the attribute. @@ -2924,16 +2925,17 @@ static ssize_t data2vp_any(const RADIUS_PACKET *packet, */ case FLAG_ENCRYPT_USER_PASSWORD: if (original) { - rad_pwdecode(vp->vp_strvalue, + rad_pwdecode(buffer, vp->length, secret, original->vector); } else { - rad_pwdecode(vp->vp_strvalue, + rad_pwdecode(buffer, vp->length, secret, packet->vector); } + buffer[253] = '\0'; if (vp->attribute == PW_USER_PASSWORD) { - vp->length = strlen(vp->vp_strvalue); + vp->length = strlen(buffer); } break; @@ -2944,7 +2946,7 @@ static ssize_t data2vp_any(const RADIUS_PACKET *packet, case FLAG_ENCRYPT_TUNNEL_PASSWORD: if (!original) goto raw; - if (rad_tunnel_pwdecode(vp->vp_octets, &vp->length, + if (rad_tunnel_pwdecode(buffer, &vp->length, secret, original->vector) < 0) { goto raw; } @@ -2962,10 +2964,10 @@ static ssize_t data2vp_any(const RADIUS_PACKET *packet, make_secret(my_digest, original->vector, secret, data); - memcpy(vp->vp_strvalue, my_digest, + memcpy(buffer, my_digest, AUTH_VECTOR_LEN ); - vp->vp_strvalue[AUTH_VECTOR_LEN] = '\0'; - vp->length = strlen(vp->vp_strvalue); + buffer[AUTH_VECTOR_LEN] = '\0'; + vp->length = strlen(buffer); } break; @@ -2976,58 +2978,49 @@ static ssize_t data2vp_any(const RADIUS_PACKET *packet, switch (vp->type) { case PW_TYPE_STRING: + memcpy(vp->vp_strvalue, buffer, vp->length); + vp->vp_strvalue[vp->length] = '\0'; + break; + case PW_TYPE_OCTETS: case PW_TYPE_ABINARY: - /* nothing more to do */ + memcpy(vp->vp_octets, buffer, vp->length); break; case PW_TYPE_BYTE: if (vp->length != 1) goto raw; - vp->vp_integer = vp->vp_octets[0]; + vp->vp_integer = buffer[0]; break; case PW_TYPE_SHORT: if (vp->length != 2) goto raw; - vp->vp_integer = (vp->vp_octets[0] << 8) | vp->vp_octets[1]; + vp->vp_integer = (buffer[0] << 8) | buffer[1]; break; case PW_TYPE_INTEGER: if (vp->length != 4) goto raw; - memcpy(&vp->vp_integer, vp->vp_octets, 4); + memcpy(&vp->vp_integer, buffer, 4); vp->vp_integer = ntohl(vp->vp_integer); if (vp->flags.has_tag) vp->vp_integer &= 0x00ffffff; - - /* - * Try to get named VALUEs - */ - { - DICT_VALUE *dval; - dval = dict_valbyattr(vp->attribute, vp->vendor, - vp->vp_integer); - if (dval) { - strlcpy(vp->vp_strvalue, - dval->name, - sizeof(vp->vp_strvalue)); - } - } break; case PW_TYPE_INTEGER64: if (vp->length != 8) goto raw; /* vp_integer64 is a union with vp_octets */ + memcpy(&vp->vp_integer64, buffer, 8); vp->vp_integer64 = ntohll(vp->vp_integer64); break; case PW_TYPE_DATE: if (vp->length != 4) goto raw; - memcpy(&vp->vp_date, vp->vp_octets, 4); + memcpy(&vp->vp_date, buffer, 4); vp->vp_date = ntohl(vp->vp_date); break; @@ -3035,7 +3028,7 @@ static ssize_t data2vp_any(const RADIUS_PACKET *packet, case PW_TYPE_IPADDR: if (vp->length != 4) goto raw; - memcpy(&vp->vp_ipaddr, vp->vp_octets, 4); + memcpy(&vp->vp_ipaddr, buffer, 4); break; /* @@ -3043,7 +3036,7 @@ static ssize_t data2vp_any(const RADIUS_PACKET *packet, */ case PW_TYPE_IFID: if (vp->length != 8) goto raw; - /* vp->vp_ifid == vp->vp_octets */ + memcpy(&vp->vp_ifid, buffer, 8); break; /* @@ -3051,7 +3044,7 @@ static ssize_t data2vp_any(const RADIUS_PACKET *packet, */ case PW_TYPE_IPV6ADDR: if (vp->length != 16) goto raw; - /* vp->vp_ipv6addr == vp->vp_octets */ + memcpy(&vp->vp_ipv6addr, buffer, 16); break; /* @@ -3065,14 +3058,15 @@ static ssize_t data2vp_any(const RADIUS_PACKET *packet, */ case PW_TYPE_IPV6PREFIX: if (vp->length < 2 || vp->length > 18) goto raw; - if (vp->vp_octets[1] > 128) goto raw; + if (buffer[1] > 128) goto raw; /* * FIXME: double-check that * (vp->vp_octets[1] >> 3) matches vp->length + 2 */ + memcpy(&vp->vp_ipv6prefix, buffer, vp->length); if (vp->length < 18) { - memset(vp->vp_octets + vp->length, 0, + memset(((uint8_t *)vp->vp_ipv6prefix) + vp->length, 0, 18 - vp->length); } break; @@ -3084,9 +3078,8 @@ static ssize_t data2vp_any(const RADIUS_PACKET *packet, * Overload vp_integer for ntohl, which takes * uint32_t, not int32_t */ - memcpy(&vp->vp_integer, vp->vp_octets, 4); + memcpy(&vp->vp_integer, buffer, 4); vp->vp_integer = ntohl(vp->vp_integer); - memcpy(&vp->vp_signed, &vp->vp_integer, 4); break; case PW_TYPE_TLV: @@ -3097,12 +3090,12 @@ static ssize_t data2vp_any(const RADIUS_PACKET *packet, case PW_TYPE_COMBO_IP: if (vp->length == 4) { vp->type = PW_TYPE_IPADDR; - memcpy(&vp->vp_ipaddr, vp->vp_octets, 4); + memcpy(&vp->vp_ipaddr, buffer, 4); break; } else if (vp->length == 16) { vp->type = PW_TYPE_IPV6ADDR; - /* vp->vp_ipv6addr == vp->vp_octets */ + memcpy(&vp->vp_ipv6addr, buffer, 16); break; } diff --git a/src/lib/valuepair.c b/src/lib/valuepair.c index bf69f3a8f9d..d32d0550610 100644 --- a/src/lib/valuepair.c +++ b/src/lib/valuepair.c @@ -62,23 +62,54 @@ static const char *months[] = { #define FR_VP_NAME_PAD (32) #define FR_VP_NAME_LEN (30) +static const int typesize[] = { + sizeof(((VALUE_PAIR *)0)->vp_strvalue), /* string */ + 4, /* integer */ + 4, /* ipaddr */ + 4, /* date */ + sizeof(((VALUE_PAIR *)0)->data), /* abinary */ + sizeof(((VALUE_PAIR *)0)->vp_octets), /* octets */ + sizeof(((VALUE_PAIR *)0)->vp_ifid), + sizeof(((VALUE_PAIR *)0)->vp_ipv6addr), + sizeof(((VALUE_PAIR *)0)->vp_ipv6prefix), + 4, /* byte (integer internally) */ + 4, /* short (integer internally) */ + sizeof(((VALUE_PAIR *)0)->vp_ether), + 4, /* signed */ + sizeof(((VALUE_PAIR *)0)->data), /* combo IP */ + sizeof(((VALUE_PAIR *)0)->vp_tlv), + sizeof(((VALUE_PAIR *)0)->data), /* extended */ + sizeof(((VALUE_PAIR *)0)->data), /* extended-flags */ + sizeof(((VALUE_PAIR *)0)->data), /* evs */ + 8 /* integer64 */ +}; + VALUE_PAIR *pairalloc(DICT_ATTR *da) { size_t name_len = 0; + size_t dsize; VALUE_PAIR *vp; /* * Not in the dictionary: the name is allocated AFTER * the VALUE_PAIR struct. */ - if (!da) name_len = FR_VP_NAME_PAD; + if (da) { + dsize = typesize[da->type]; + name_len = 0; + } else { + dsize = sizeof(vp->data); + name_len = FR_VP_NAME_PAD; + } - vp = malloc(sizeof(*vp) + name_len); + dsize += sizeof(*vp) - sizeof(vp->data) + name_len; + + vp = malloc(dsize); if (!vp) { fr_strerror_printf("Out of memory"); return NULL; } - memset(vp, 0, sizeof(*vp)); + memset(vp, 0, dsize); if (da) { vp->attribute = da->attr; @@ -204,7 +235,7 @@ void pairbasicfree(VALUE_PAIR *pair) { if (pair->type == PW_TYPE_TLV) free(pair->vp_tlv); /* clear the memory here */ - memset(pair, 0, sizeof(*pair)); + memset(pair, 0, sizeof(*pair) - sizeof(pair->data)); free(pair); } @@ -338,22 +369,24 @@ void pairreplace(VALUE_PAIR **first, VALUE_PAIR *replace) */ VALUE_PAIR *paircopyvp(const VALUE_PAIR *vp) { - size_t name_len; + size_t dsize, name_len; VALUE_PAIR *n; if (!vp) return NULL; - + if (!vp->flags.unknown_attr) { + dsize = typesize[vp->type]; name_len = 0; } else { + dsize = sizeof(vp->data); name_len = FR_VP_NAME_PAD; } - - if ((n = malloc(sizeof(*n) + name_len)) == NULL) { - fr_strerror_printf("out of memory"); - return NULL; - } - memcpy(n, vp, sizeof(*n) + name_len); + dsize += sizeof(*vp) - sizeof(vp->data) + name_len; + + n = malloc(dsize); + if (!n) return NULL; + + memcpy(n, vp, dsize); /* * Reset the name field to point to the NEW attribute, @@ -903,15 +936,6 @@ VALUE_PAIR *pairparsevalue(VALUE_PAIR *vp, const char *value) if (!value) return NULL; - /* - * Even for integers, dates and ip addresses we - * keep the original string in vp->vp_strvalue. - */ - if (vp->type != PW_TYPE_TLV) { - strlcpy(vp->vp_strvalue, value, sizeof(vp->vp_strvalue)); - vp->length = strlen(vp->vp_strvalue); - } - switch(vp->type) { case PW_TYPE_STRING: /* @@ -1144,7 +1168,7 @@ VALUE_PAIR *pairparsevalue(VALUE_PAIR *vp, const char *value) } vp->length = size >> 1; - if (size > 2*sizeof(vp->vp_octets)) { + if (size > 2 * sizeof(vp->vp_octets)) { vp->type |= PW_FLAG_LONG; us = vp->vp_tlv = malloc(vp->length); if (!us) { @@ -1158,6 +1182,9 @@ VALUE_PAIR *pairparsevalue(VALUE_PAIR *vp, const char *value) fr_strerror_printf("Invalid hex data"); return NULL; } + } else { + strlcpy(vp->vp_octets, value, sizeof(vp->vp_octets)); + vp->length = strlen(value); } break;