Skip to content

Commit

Permalink
Merge branch 'issue83'
Browse files Browse the repository at this point in the history
I was hesitant to put this on main because it seemed like a performance
tank, but truth be told, I'm being a wuss. these table iterations are
nothing compared to the amount of time Fort has to spend downloading.

And it looks like I'm never going to find this bug with the stack trace
alone.

Does not fix #83 nor #89, but prevents the crash at least.
  • Loading branch information
ydahhrk committed Jan 30, 2023
2 parents 178cbd7 + dfc2955 commit f6d3573
Show file tree
Hide file tree
Showing 8 changed files with 43 additions and 20 deletions.
9 changes: 5 additions & 4 deletions src/data_structure/uthash_nonfatal.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@
* so set 'errno' to 0 before this ops are made. The 'obj' won't be freed,
* this is the caller's responsibility.
*
* TODO I think most of the code is not checking this.
*
* Functions that can OOM:
*
* HASH_ADD_TO_TABLE
Expand All @@ -29,13 +27,16 @@
* HASH_ADD_KEYPTR_BYHASHVALUE
* HASH_REPLACE_BYHASHVALUE
* HASH_REPLACE (*)
* HASH_ADD_KEYPTR (**)
* HASH_REPLACE_STR (**)
* HASH_REPLACE_INT
* HASH_REPLACE_PTR
* HASH_ADD_KEYPTR
* HASH_ADD
* HASH_ADD_BYHASHVALUE
* HASH_SELECT
*
* (*) Used by Fort
* (**) Used by Fort, but in its fatal uthash form.
* (**) Used by Fort, but only in its fatal uthash form.
*/
#define HASH_NONFATAL_OOM 1
#define uthash_nonfatal_oom(obj) \
Expand Down
3 changes: 1 addition & 2 deletions src/reqs_errors.c
Original file line number Diff line number Diff line change
Expand Up @@ -177,8 +177,7 @@ reqs_errors_add_uri(char const *uri)
(working_repo_peek_level() <= LOG_REPO_LEVEL);

rwlock_write_lock(&db_lock);
HASH_ADD_KEYPTR(hh, err_uris_db, new_uri->uri, strlen(new_uri->uri),
new_uri);
HASH_ADD_STR(err_uris_db, uri, new_uri);
rwlock_unlock(&db_lock);

return 0;
Expand Down
12 changes: 2 additions & 10 deletions src/rrdp/db/db_rrdp_uris.c
Original file line number Diff line number Diff line change
Expand Up @@ -94,17 +94,9 @@ add_rrdp_uri(struct db_rrdp_uri *uris, struct uris_table *new_uri)
{
struct uris_table *old_uri;

/*
* TODO (fine) this should use HASH_REPLACE instead of HASH_ADD_KEYPTR
*/

old_uri = find_rrdp_uri(uris, new_uri->uri);
if (old_uri != NULL) {
HASH_DELETE(hh, uris->table, old_uri);
HASH_REPLACE_STR(uris->table, uri, new_uri, old_uri);
if (old_uri != NULL)
uris_table_destroy(old_uri);
}
HASH_ADD_KEYPTR(hh, uris->table, new_uri->uri, strlen(new_uri->uri),
new_uri);
}

static int
Expand Down
19 changes: 19 additions & 0 deletions src/rtr/db/db_table.c
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,25 @@ add_roa_deltas(struct hashable_roa *roas1, struct hashable_roa *roas2,
return 0;
}

void
find_bad_vrp(char const *prefix, struct db_table *table)
{
struct hashable_roa *cursor;
struct hashable_roa *tmp;
uint8_t family;

if (table == NULL)
return;

HASH_ITER(hh, table->roas, cursor, tmp) {
family = cursor->data.addr_fam;
if (family != AF_INET && family != AF_INET6) {
pr_op_err("%s: VRP corrupted!", prefix);
return;
}
}
}

static int
add_router_key_delta(struct deltas *deltas, struct hashable_key *key, int op)
{
Expand Down
2 changes: 2 additions & 0 deletions src/rtr/db/db_table.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,6 @@ int rtrhandler_handle_router_key(struct db_table *, unsigned char const *,
uint32_t, unsigned char const *);
int compute_deltas(struct db_table *, struct db_table *, struct deltas **);

void find_bad_vrp(char const *, struct db_table *);

#endif /* SRC_RTR_DB_DB_TABLE_H_ */
9 changes: 8 additions & 1 deletion src/rtr/db/delta.c
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ deltas_add_roa(struct deltas *deltas, struct vrp const *vrp, int op)
struct delta_v4 v4;
struct delta_v6 v6;
} delta;
char buffer[INET6_ADDRSTRLEN];

switch (vrp->addr_fam) {
case AF_INET:
Expand All @@ -139,7 +140,13 @@ deltas_add_roa(struct deltas *deltas, struct vrp const *vrp, int op)
return deltas_v6_add(get_deltas_array6(deltas, op), &delta.v6);
}

pr_crit("Unknown protocol: %d", vrp->addr_fam);
pr_val_err("Unknown protocol: [%u %s/%u-%u %u]",
vrp->asn,
addr2str6(&vrp->prefix.v6, buffer),
vrp->prefix_length,
vrp->max_prefix_length,
vrp->addr_fam);
return 0;
}

int
Expand Down
5 changes: 5 additions & 0 deletions src/rtr/db/vrps.c
Original file line number Diff line number Diff line change
Expand Up @@ -261,16 +261,20 @@ __vrps_update(bool *notify_clients)
if (notify_clients)
*notify_clients = false;
old_base = state.base;
find_bad_vrp("Old base", old_base);
new_base = NULL;

error = __perform_standalone_validation(&new_base);
if (error)
return error;
find_bad_vrp("After standalone", new_base);

error = slurm_apply(new_base, &state.slurm);
if (error) {
db_table_destroy(new_base);
return error;
}
find_bad_vrp("After SLURM", new_base);

/*
* At this point, new_base is completely valid. Even if we error out
Expand All @@ -280,6 +284,7 @@ __vrps_update(bool *notify_clients)
* duplicate ROAs.
*/
output_print_data(new_base);
find_bad_vrp("After CSV", new_base);

error = __compute_deltas(old_base, new_base, notify_clients,
&new_deltas);
Expand Down
4 changes: 1 addition & 3 deletions src/visited_uris.c
Original file line number Diff line number Diff line change
Expand Up @@ -114,9 +114,7 @@ visited_uris_add(struct visited_uris *uris, char const *uri)
if (error)
return error;

HASH_ADD_KEYPTR(hh, uris->table, elem->uri, strlen(elem->uri),
elem);

HASH_ADD_STR(uris->table, uri, elem);
return 0;
}

Expand Down

0 comments on commit f6d3573

Please sign in to comment.