Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove local ASN range limitation in BGPPolicy #6914

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

hongliangl
Copy link
Contributor

@hongliangl hongliangl commented Jan 9, 2025

Previously, for safety and internal using only purposes, the local ASN was restricted to the private range (64512-65535). However, based on user feedback, BGPPolicy may also be used to peer with public BGP ASNs. Therefore, this restriction is no longer justified.

To prevent potential misconfigurations that could disrupt BGP operations, we have added the follow paragraph to documentation to remind users of the implications of using unrestricted ASN ranges.

Private ASNs, which are within the ranges 64512-65534 (16-bit), should be strictly limited to private networks or
environments that do not peer with public ASNs. If public network connectivity is required, coordinate with your upstream
provider to avoid issues caused by private ASN usage.

@hongliangl hongliangl added action/release-note Indicates a PR that should be included in release notes. area/transit/bgp Issues or PRs related to BGP support. labels Jan 9, 2025
@hongliangl hongliangl added this to the Antrea v2.3 release milestone Jan 9, 2025
@hongliangl hongliangl requested review from antoninbas and tnqn January 9, 2025 13:17
Comment on lines 87 to 88
For private ASNs, which are within the ranges 64512-65534 (16-bit) or 4200000000-4294967294 (32-bit), ensure they are
used only within private networks or scenarios where they will not conflict with public ASNs. If public network
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't quite understand how this happens: "private ASNs within the ranges 64512-65534 (16-bit) or 4200000000-4294967294 (32-bit)" conflict with public ASNs, aren't they in different ranges?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, after we extend the bits to 32, I suppose we only support 4200000000-4294967294 (32-bit) as the private ASN? or it's user's decision to define which are the private ASN?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will rephrase this sentence.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@hongliangl hongliangl force-pushed the 20250109-bgppolicy-api-update branch from 782827e to f88e921 Compare January 10, 2025 08:18
@hongliangl hongliangl requested review from luolanzone and tnqn January 10, 2025 08:19

For private ASNs, which are within the ranges 64512-65534 (16-bit) or 4200000000-4294967294 (32-bit), should be strictly
limited to private networks or environments that do not peer with public ASNs. If public network connectivity is required,
coordinate with your upstream provider to avoid issues caused by private ASN usage.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So BGP process on Antrea will advertise the routes regardless of the ASN type? Not sure if it's expected to do so or we should do anything to prevent private ASN routes to be public?

Copy link
Contributor Author

@hongliangl hongliangl Jan 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not our obligation to prevent users from peering private ASN to public BGP. To some extent, we should assume that users have the knowledge about that, similar to RFC 1918 defining the private ranges of IP. To warn users about the private ASNs, we add some explanations here.

minimum: 64512
maximum: 65535
minimum: 1
maximum: 4294967295
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we have a problem here because 4294967295 is MaxUint32 and the field is an int32

Also not sure how we can solve this. One possibility is to restrict the range to [1, 65535] for now, and change the field type from int32 to int64 in the next API version (e.g. v1alpha2). I do not believe we can update the type without also changing the API version.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You just reminded me. We can only restrict the range to [1, 65535] in this PR. For extending the AS number to 32-bit, we can update it in the future with changing the API version.

docs/bgp-policy.md Outdated Show resolved Hide resolved
Previously, for safety and internal using only purposes, the local ASN was
restricted to the private range (64512-65535). However, based on user feedback,
BGPPolicy may also be used to peer with public BGP ASNs. Therefore, this
restriction is no longer justified.

To prevent potential misconfigurations that could disrupt BGP operations,
we have added the follow paragraph to documentation to remind users of the
implications of using unrestricted ASN ranges.

```
Private ASNs, which are within the ranges 64512-65534 (16-bit), should be strictly limited to private networks or
environments that do not peer with public ASNs. If public network connectivity is required, coordinate with your upstream
provider to avoid issues caused by private ASN usage.
```

Signed-off-by: Hongliang Liu <[email protected]>
@hongliangl hongliangl force-pushed the 20250109-bgppolicy-api-update branch from f88e921 to 52e5ea6 Compare January 11, 2025 05:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
action/release-note Indicates a PR that should be included in release notes. area/transit/bgp Issues or PRs related to BGP support.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants