[do not review yet] Add ability to disallow transit traffic#4911
Draft
katyatitkova wants to merge 84 commits into
Draft
[do not review yet] Add ability to disallow transit traffic#4911katyatitkova wants to merge 84 commits into
katyatitkova wants to merge 84 commits into
Conversation
# Conflicts: # control/beaconing/writer.go
# Conflicts: # control/cmd/control/main.go # topology/BUILD.bazel
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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 contains changes for two distinct things: option for disallow transit traffic, and peering links bug fixes. Peering links bugs didn't let me test transit traffic properly. Therefore, the code from #4905 is merged into this branch. Before #4905 (or #4890, depending on which approach the reviewers like most) is merged, there's not much sense in trying to review this PR since its intended to be about transit traffic.
Beaconing policy that disallows transit traffic comes with limitations related to the peering links. Because peering links are just a part of the beacon and are not announced separately, blocking the beacon containing the info on peering links will also block the propagation of such info. Examples of it can be seen in the test_isd_3 test file. It renders such peering links useless in certain scenarios, but should be acceptable in real-life situation where ASes are usually well interconnected (as there should be other paths where the beacons with peering link info are flowing unrestricted).
This limitation is hard, if even possible, to overcome without significant changes to the protocol itself. If we don't block propagation, other ASes still get the beacon, and they have no idea that they are only allowed to use peering links portion of the beacon. Same thing if the AS that blocks transit traffic lets the beacons through, but refuses serving certain segments requests. Extending the beacon by adding some metadata is also a bad decision, since it shouldn't be a problem of other ASes that someone else wants to implement certain policies for themselves. And, due to cryptography, we can't just extact the peering links info from the incoming beacon, don't propagate it, and then originate a new beacon with the extracted peerling links info.
Please tell me if I'm wrong in such assessment and there is actually a way to propagate peering links info while blocking the transit traffic.
Fixes #4699