Allow core-dump; refactor NLRI parsing for add-path issue #42
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
when seeing problematic MRT record, if `.enable_core_dump()` is called when creating the parser, the parser then will create a new file `mrt_core_dump` on the current working directory and dump the whole bytes of the current MRT record to file. With #38 merged, we can directly read the raw (uncompressed) dump file in debug session.
This refactor enables parsing of NLRI block with potentially wrong add-path type. In some cases, the BGP message with add-path might be wrapped in a non-add-path message and causing trouble parsing NLRI block in BGP message and in attributes. This patch implements a workaround that can more intelligently "guess" the add-path intention and revert back to regular parsing when guess fails. Here is a brief description of the workaround: - If add-path is not set (i.e. not the add-path BGP msgs) and the first byte of an NLRI entry is 0, it is likely an add-path msg wrapped in non-add-path msg, treat as add-path. This covers both IPv4 and IPv6 cases. - If an error happens when parsing, revert back to the original add-path setting (this only happens for `0.0.0.0/0` or `0::/0` prefixes without add-path, which is pretty rare) and re-parse the whole NLRI block. (needs to go back a few bytes to the beginning). - The unhandled cases are the ones where the first byte of path_id is not zero. In those cases, we will, unfortunately, parse the msg without path-id, and depending on the msg, we might see parsing errors later (e.g. not enough bytes, or extra bytes). Test run show that the parsing of one problematic file is resolved back to normal now. See #31 for more details.
in some cases, attributes marked as partial is not actually partial. we now attempt to parse attributes and log warnings if parsing fails.
Closed
3 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR enables parsing of NLRI block with potentially wrong
add-path type. In some cases, the BGP message with add-path might be
wrapped in a non-add-path message and causing trouble parsing NLRI block
in BGP message and in attributes.
This patch implements a workaround that can more intelligently "guess"
the add-path intention and revert back to regular parsing when guess
fails. Here is a brief description of the workaround:
byte of an NLRI entry is 0, it is likely an add-path msg wrapped in
non-add-path msg, treat as add-path. This covers both IPv4 and IPv6
cases.
setting (this only happens for
0.0.0.0/0or0::/0prefixes withoutadd-path, which is pretty rare) and re-parse the whole NLRI
block. (needs to go back a few bytes to the beginning).
not zero. In those cases, we will, unfortunately, parse the msg without
path-id, and depending on the msg, we might see parsing errors
later (e.g. not enough bytes, or extra bytes).
Test run show that the parsing of one problematic file is resolved back
to normal now. See #31 for
more details.