From 45ca34c680ebc10972c26a99abbad14dcc6572ec Mon Sep 17 00:00:00 2001 From: Mingwei Zhang Date: Tue, 25 Feb 2020 08:15:35 -0800 Subject: [PATCH 1/7] update log_moas function signature remove `moas_signature` parameter as it was not used in the current version of the `log_moas` function --- lib/consumers/bvc_moas.c | 31 ++++--------------------------- 1 file changed, 4 insertions(+), 27 deletions(-) diff --git a/lib/consumers/bvc_moas.c b/lib/consumers/bvc_moas.c index 23064a5..f4cd77f 100644 --- a/lib/consumers/bvc_moas.c +++ b/lib/consumers/bvc_moas.c @@ -326,7 +326,7 @@ static void update_moas_counters(bvc_t *consumer, moas_category_t mc) } static int log_moas(bvc_t *consumer, bgpview_t *view, bgpview_iter_t *it, - bgpstream_pfx_t *pfx, moas_signature_t *ms, + bgpstream_pfx_t *pfx, moas_properties_t *mp, moas_category_t mc, uint32_t ts) { bvc_moas_state_t *state = STATE; @@ -361,13 +361,6 @@ static int log_moas(bvc_t *consumer, bgpview_t *view, bgpview_iter_t *it, * is new or * new-recurring */ - /* - old version: - if (wandio_printf(state->wandio_fh, - "%" PRIu32 "|%s|%s|%" PRIu32 "|%" PRIu32 "|%" PRIu32 "|", - ts, pfx_str, get_category_str(mc), mp->first_seen, - mp->start, mp->end) == -1) { - */ if (wandio_printf(state->wandio_fh, "%" PRIu32 "|%s|%s|", @@ -375,22 +368,6 @@ static int log_moas(bvc_t *consumer, bgpview_t *view, bgpview_iter_t *it, fprintf(stderr, "ERROR: Could not write data to file\n"); return -1; } - /* - int i; - int ret; - for (i = 0; i < ms->n; i++) { - // last origin - if (i == ms->n - 1) { - ret = wandio_printf(state->wandio_fh, "%" PRIu32, ms->origins[i]); - } else { - ret = wandio_printf(state->wandio_fh, "%" PRIu32 " ", ms->origins[i]); - } - if (ret == -1) { - fprintf(stderr, "ERROR: Could not write data to file\n"); - return -1; - } - } - */ if (mc == NEW || mc == NEWREC) { /* mc is either NEW or NEWREC, hence we print the set of AS paths * as observed by all full feed peers */ @@ -487,7 +464,7 @@ static int clean_moas(bvc_t *consumer, uint32_t ts, uint32_t last_valid_ts) if (mpro->end < ts) { // report finished moases if (mpro->start > 0) { - if (log_moas(consumer, NULL, NULL, pfx, ms, + if (log_moas(consumer, NULL, NULL, pfx, mpro, FINISHED, ts) != 0) { return -1; } @@ -566,7 +543,7 @@ static int add_moas(bvc_t *consumer, bgpview_t *view, bgpview_iter_t *it, moas_properties = &kh_value(per_pfx_moases, k); - return log_moas(consumer, view, it, pfx, ms, moas_properties, mc, ts); + return log_moas(consumer, view, it, pfx, moas_properties, mc, ts); } /** Create timeseries metrics */ @@ -811,7 +788,7 @@ int bvc_moas_process_view(bvc_t *consumer, bgpview_t *view) int ipv_idx; // ip version index bgpstream_peer_id_t peerid; bgpstream_as_path_seg_t *origin_seg; - moas_signature_t ms; + moas_signature_t ms; // creating moas signature int i; uint32_t origin_asn; uint32_t last_valid_ts = bgpview_get_time(view) - state->window_size; From ce2e49dda52639c513d4fe96ee22492a057aa919 Mon Sep 17 00:00:00 2001 From: Mingwei Zhang Date: Tue, 25 Feb 2020 09:44:50 -0800 Subject: [PATCH 2/7] extend moas_signature struct for dynamic origins the `moas_signature` struct changes from containing a static allocated int32 array of size 128 and a count integer, to a static array of size 4 and a dynamic array and a origins count. the dynamic array is allocated if there are more than 4 origins in the moas event and is assigned with a static 128 size. this commit is designed to reduce the memory consumption of the moas consumer by reducing the sizes of the moas signatures needed to be stored in memory. --- lib/consumers/bvc_moas.c | 52 +++++++++++++++++++++++++++++++++------- 1 file changed, 44 insertions(+), 8 deletions(-) diff --git a/lib/consumers/bvc_moas.c b/lib/consumers/bvc_moas.c index f4cd77f..406c35b 100644 --- a/lib/consumers/bvc_moas.c +++ b/lib/consumers/bvc_moas.c @@ -51,6 +51,9 @@ /** Maximum size of the str output buffer */ #define MAX_BUFFER_LEN 1024 +/** Maximum number of origin ASns in statically allocated array */ +#define MAX_STATIC_ORIGINS 4 + /** Maximum number of origin ASns */ #define MAX_UNIQUE_ORIGINS 128 @@ -93,8 +96,9 @@ typedef struct moas_properties { /** List of origin ASns in a MOAS */ typedef struct moas_signature { - uint32_t origins[MAX_UNIQUE_ORIGINS]; - uint8_t n; + uint32_t origins[MAX_STATIC_ORIGINS]; // static allocated origins array + uint32_t *origins_dyn; // dynamically allocated origins array + uint8_t n; // total number of origins in the signature } moas_signature_t; /** MOAS signature hash function */ @@ -108,7 +112,12 @@ moasinfo_map_hash(moas_signature_t ms) uint8_t i; uint32_t h = 0; for (i = 0; i < ms.n; i++) { - h += ms.origins[i]; + if(i >= MAX_STATIC_ORIGINS){ + // outside the static origins range + h += ms.origins_dyn[i-MAX_STATIC_ORIGINS]; + } else { + h += ms.origins[i]; + } } return h; } @@ -135,8 +144,15 @@ static int moasinfo_map_equal(moas_signature_t ms1, moas_signature_t ms2) qsort(&ms2.origins, ms2.n, sizeof(uint32_t), uint32_cmp); int i; for (i = 0; i < ms1.n; i++) { - if (ms1.origins[i] != ms2.origins[i]) { - return 0; + if(i >= MAX_STATIC_ORIGINS){ + // outside the static origins range + if (ms1.origins_dyn[i-MAX_STATIC_ORIGINS] != ms2.origins_dyn[i-MAX_STATIC_ORIGINS]) { + return 0; + } + } else { + if (ms1.origins[i] != ms2.origins[i]) { + return 0; + } } } return 1; @@ -838,7 +854,11 @@ int bvc_moas_process_view(bvc_t *consumer, bgpview_t *view) ipv_idx = bgpstream_ipv2idx(pfx->address.version); + // clear moas signature ms.n = 0; + if(ms.origins_dyn != NULL){ + free(ms.origins_dyn); + } for (bgpview_iter_pfx_first_peer(it, BGPVIEW_FIELD_ACTIVE); bgpview_iter_pfx_has_more_peer(it); bgpview_iter_pfx_next_peer(it)) { @@ -866,13 +886,29 @@ int bvc_moas_process_view(bvc_t *consumer, bgpview_t *view) /* check if origin is already in the moas signature struct * this check works when the signature is empty too */ for (i = 0; i < ms.n; i++) { - if (ms.origins[i] == origin_asn) { - break; + if(i >= MAX_STATIC_ORIGINS){ + if (ms.origins_dyn[i-MAX_STATIC_ORIGINS] == origin_asn) { + break; + } + } else { + if (ms.origins[i] == origin_asn) { + break; + } } } /* if the origin was not found, add it */ if (i == ms.n) { - ms.origins[ms.n] = origin_asn; + if(i >= MAX_STATIC_ORIGINS){ + if(ms.origins_dyn == NULL){ + // allocate dyanmic array + if ((ms.origins_dyn = malloc_zero(sizeof(uint32_t)*MAX_UNIQUE_ORIGINS)) == NULL) { + return -1; + } + } + ms.origins_dyn[ms.n-MAX_STATIC_ORIGINS] = origin_asn; + } else { + ms.origins[ms.n] = origin_asn; + } ms.n++; } } From b58b2d00f7e25d76725d439b56e2b4bdaba6cad8 Mon Sep 17 00:00:00 2001 From: Mingwei Zhang Date: Tue, 25 Feb 2020 09:46:01 -0800 Subject: [PATCH 3/7] reset dynamic array pointer after free --- lib/consumers/bvc_moas.c | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/consumers/bvc_moas.c b/lib/consumers/bvc_moas.c index 406c35b..c6190d9 100644 --- a/lib/consumers/bvc_moas.c +++ b/lib/consumers/bvc_moas.c @@ -858,6 +858,7 @@ int bvc_moas_process_view(bvc_t *consumer, bgpview_t *view) ms.n = 0; if(ms.origins_dyn != NULL){ free(ms.origins_dyn); + ms.origins_dyn = NULL; } for (bgpview_iter_pfx_first_peer(it, BGPVIEW_FIELD_ACTIVE); From 62c223de73c0c3cc37344953a9b41280749e3eee Mon Sep 17 00:00:00 2001 From: Mingwei Zhang Date: Tue, 25 Feb 2020 14:52:51 -0800 Subject: [PATCH 4/7] dynamically realloc memory for dynamic array every time we try to write to array location outside the current range, we expand the space by MAX_STATIC_ORIGINS --- lib/consumers/bvc_moas.c | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/lib/consumers/bvc_moas.c b/lib/consumers/bvc_moas.c index c6190d9..a726248 100644 --- a/lib/consumers/bvc_moas.c +++ b/lib/consumers/bvc_moas.c @@ -54,9 +54,6 @@ /** Maximum number of origin ASns in statically allocated array */ #define MAX_STATIC_ORIGINS 4 -/** Maximum number of origin ASns */ -#define MAX_UNIQUE_ORIGINS 128 - /** Default size of window: 1 week (s) */ #define DEFAULT_WINDOW_SIZE (7 * 24 * 3600) @@ -901,11 +898,25 @@ int bvc_moas_process_view(bvc_t *consumer, bgpview_t *view) if (i == ms.n) { if(i >= MAX_STATIC_ORIGINS){ if(ms.origins_dyn == NULL){ - // allocate dyanmic array - if ((ms.origins_dyn = malloc_zero(sizeof(uint32_t)*MAX_UNIQUE_ORIGINS)) == NULL) { + // dynamic array first allocation + if ((ms.origins_dyn = malloc_zero(sizeof(uint32_t)*MAX_STATIC_ORIGINS)) == NULL) { + return -1; + } + } else if (i % MAX_STATIC_ORIGINS == 0){ + // if it needs more space to store origins, realloc + // the size of which should be MAX_STATIC_ORIGINS * (i/MAX_STATIC_ORIGINS) = ms.n + // + // for example, if MAX_STATIC_ORIGINS is 4 and ms.n == 8, we need to realloc the + // dynamic memory space to 8/4 * 4 = 8. We have 4 static origins, and change from + // having 4 uint32 for dynamic origins to 8 uint32 for dynamic origins + if ((ms.origins_dyn = realloc(ms.origins_dyn, sizeof(uint32_t) * ms.n)) == NULL) { return -1; } } + // at this point, we are safe on accessing the memory because: + // 1. we have just allocated the space the first time, or + // 2. we have allocated the space previously and have not used it up yet, or + // 3. we have just reallocated with extra space ms.origins_dyn[ms.n-MAX_STATIC_ORIGINS] = origin_asn; } else { ms.origins[ms.n] = origin_asn; From a4640cf78bfa1b53b6621323c6d913a53cf85a03 Mon Sep 17 00:00:00 2001 From: Mingwei Zhang Date: Wed, 26 Feb 2020 10:10:07 -0800 Subject: [PATCH 5/7] allow realloc on NULL pointer this simplifies the flow for extending the dynamic array size --- lib/consumers/bvc_moas.c | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/lib/consumers/bvc_moas.c b/lib/consumers/bvc_moas.c index a726248..972b5aa 100644 --- a/lib/consumers/bvc_moas.c +++ b/lib/consumers/bvc_moas.c @@ -897,26 +897,28 @@ int bvc_moas_process_view(bvc_t *consumer, bgpview_t *view) /* if the origin was not found, add it */ if (i == ms.n) { if(i >= MAX_STATIC_ORIGINS){ - if(ms.origins_dyn == NULL){ - // dynamic array first allocation - if ((ms.origins_dyn = malloc_zero(sizeof(uint32_t)*MAX_STATIC_ORIGINS)) == NULL) { - return -1; - } - } else if (i % MAX_STATIC_ORIGINS == 0){ + if (i % MAX_STATIC_ORIGINS == 0){ // if it needs more space to store origins, realloc // the size of which should be MAX_STATIC_ORIGINS * (i/MAX_STATIC_ORIGINS) = ms.n // - // for example, if MAX_STATIC_ORIGINS is 4 and ms.n == 8, we need to realloc the - // dynamic memory space to 8/4 * 4 = 8. We have 4 static origins, and change from - // having 4 uint32 for dynamic origins to 8 uint32 for dynamic origins + // example case 1: + // ms.n == 4 + // we need to allocate the space for the first time, + // we do realloc(ms.origins_dyn, sizeof(uint32_t)*4), where ms.origins_dyn==NULL + // + // example case 2: + // ms.n == 8 + // we need to allocate more space for dynamic array, + // we do realloc(ms.origins_dyn, sizeof(uint32_t)*8) + // + // by induction, the future cases will increase the dynamic array space by MAX_STATIC_ORIGINS + // everytime it reaches outside the range of the dynamic array. if ((ms.origins_dyn = realloc(ms.origins_dyn, sizeof(uint32_t) * ms.n)) == NULL) { return -1; } } - // at this point, we are safe on accessing the memory because: - // 1. we have just allocated the space the first time, or - // 2. we have allocated the space previously and have not used it up yet, or - // 3. we have just reallocated with extra space + // at this point, we have allocated memory space for storing + // the origin asn at the dynamic array ms.origins_dyn[ms.n-MAX_STATIC_ORIGINS] = origin_asn; } else { ms.origins[ms.n] = origin_asn; From dc220cb9e17e2234f95d99b77af25f7f03136932 Mon Sep 17 00:00:00 2001 From: Mingwei Zhang Date: Wed, 26 Feb 2020 10:18:09 -0800 Subject: [PATCH 6/7] free dyanmic origins when moas is outdated --- lib/consumers/bvc_moas.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/consumers/bvc_moas.c b/lib/consumers/bvc_moas.c index 972b5aa..d5ee3bd 100644 --- a/lib/consumers/bvc_moas.c +++ b/lib/consumers/bvc_moas.c @@ -472,6 +472,7 @@ static int clean_moas(bvc_t *consumer, uint32_t ts, uint32_t last_valid_ts) /* outdated moas, remove it */ if (mpro->end < last_valid_ts) { + free(ms->origins_dyn); // free dynamically allocated space if any kh_del(moasinfo_map, per_pfx_moases, m); } else { if (mpro->end < ts) { @@ -854,7 +855,6 @@ int bvc_moas_process_view(bvc_t *consumer, bgpview_t *view) // clear moas signature ms.n = 0; if(ms.origins_dyn != NULL){ - free(ms.origins_dyn); ms.origins_dyn = NULL; } From 3e51756ec0400566bef03516429310a08b63fff6 Mon Sep 17 00:00:00 2001 From: Mingwei Zhang Date: Wed, 26 Feb 2020 10:40:43 -0800 Subject: [PATCH 7/7] free dynamic array when destroying moas consumer --- lib/consumers/bvc_moas.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/lib/consumers/bvc_moas.c b/lib/consumers/bvc_moas.c index d5ee3bd..9ecc867 100644 --- a/lib/consumers/bvc_moas.c +++ b/lib/consumers/bvc_moas.c @@ -770,10 +770,20 @@ void bvc_moas_destroy(bvc_t *consumer) if (state != NULL) { if (state->current_moases != NULL) { - khiter_t p; + moasinfo_map_t *per_pfx_moases; + moas_signature_t *ms; + khiter_t p,m; for (p = kh_begin(state->current_moases); p != kh_end(state->current_moases); p++) { if (kh_exist(state->current_moases, p)) { + /* free dynamic origins array for each moas */ + per_pfx_moases = kh_val(state->current_moases, p); + for (m = kh_begin(per_pfx_moases); m != kh_end(per_pfx_moases); m++) { + if (kh_exist(per_pfx_moases, m)) { + ms = &kh_key(per_pfx_moases, m); + free(ms->origins_dyn); + } + } kh_destroy(moasinfo_map, kh_val(state->current_moases, p)); } }