check prefixlen in check_inclusion#145
Conversation
Huh, this confuses me. I thought we would be getting more assignments with this since we are accepting assignments that were previously rejected? |
We said we don't really do versions anymore but we still have them so I would say we should bump it to v0.5.0 with this change. |
Description nit: It's "greater than or equal", right? |
|
Concept ACK obviously, thanks for extracting this separate PR. |
when merging in a map, a more specific prefix should not supersede a less specific network. This adds a check on the prefix length of the network, which returns check_inclusion = True only if the networks match and the prefix length is greater than the existing one.
jurraca
force-pushed
the
fix-inclusion-check
branch
from
4b696ef to
8e70dd2
Compare
Sorry, yes, the diffs shows mostly previously unassigned networks (16076 out of 16856), so there's more assignments in the new file. Though it may not always be the case, depending on what IRR and RV provide. For example, in the test, new IRR networks are added but the more specific RV networks are rejected, even though they would be included previously. diff file: fix-diff.txt |
ACK on 0.5.0, makes sense. imo we should still do versions in general, any reason not to? |
2 participants
When merging in a map (such as IRR into RPKI), a more specific prefix should not supersede a less specific network. This adds a check on the prefix length of the network, which returns check_inclusion = True only if the networks match and the prefix length is greater (edit: or equal to)
thanthe existing one.The diff from the 1780588800 run with the same data sources yields 16856 lines, the majority of which are un-assignments, i.e. many networks that were previously merged in due to the current check_inclusion are no longer assigned, as expected.
This is backwards incompatible, in the sense that given a same input, the output ASmap will be different if this is merged in.
Closes #144