CAIDA / libbgpstream

Client-side C library and CLI tool of the BGPStream project
https://bgpstream.caida.org
BSD 2-Clause "Simplified" License
44 stars 27 forks source link

Avoid inserting /0s into the prefix patricia trie #219

Closed salcock closed 3 years ago

salcock commented 3 years ago

The main reason to avoid this is because it will run the risk of triggering an assertion:

assert(bgpstream_pfx_equal(&node_it->prefix, pfx));

The problem is that any lookup for the insertion point for a /0 prefix is going to return the tree head (for v6, ::/0) immediately -- but if we are for some reason trying to insert 100::6:6:6/0 then the assertion is going to fail because the two prefixes are not considered equal.

Of course, the real question is why one would try and insert /0 prefixes in the first place -- these bogus prefixes are not coming from our collectors directly, so some bug within the bgpstream consumer (specifically per-geo-visibility in the cases that I have observed) is causing these /0 prefixes to come about...

alistairking commented 3 years ago

Is this specific to /0 prefixes or will it happen for any prefix that isn't masked correctly?

salcock commented 3 years ago

I've only ever seen it happen with /0s -- it might be possible to happen with others. I suspect, though, that the ability to insert glue nodes deeper in the tree probably mitigate the issue for any longer prefix.

Trying to insert a /0 is always going to select the root node as the "insertion point", whereas an insertion anywhere else might just determine that another glue node needs to be inserted first (and therefore take a different code path that would bypass the assertion).

I'm mostly theorizing here, but does that make sense to you?

salcock commented 3 years ago

Problem solved by addressing the root cause in the offending bgpview consumer, closing for now.