Add BGP confederation support in BGPPolicy #6905
Conversation
hongliangl
added
action/release-note
Signed-off-by: Hongliang Liu <[email protected]>
hongliangl
force-pushed
the
20241205-bgp-confederation
branch
from
a61659d to
dc54219
Compare
hongliangl
changed the title
| Identifier int32 `json:"identifier,omitempty"` | ||
|
|
||
| // Peers lists the ASNs of other members in the confederation. | ||
| Peers []int32 `json:"peers,omitempty"` |
There was a problem hiding this comment.
I think the term "members" is typically used for BGP confederation. Should we name this field MemberASNs?
There was a problem hiding this comment.
I selected the term "peers" because it is used by the commands of some well-known vendors like Cisco. For example:
router bgp 65000
bgp confederation identifier 65000
bgp confederation peers 65001 65002
neighbor 192.168.1.1 remote-as 65001
neighbor 192.168.2.1 remote-as 65002
The term "peers" is not very readable. I'm fine with MemberASN, and it is more readable.
| identifier: | ||
| type: integer | ||
| format: int32 |
There was a problem hiding this comment.
what is the range of valid values?
There was a problem hiding this comment.
The range should be 1-4294967295 if we can merge #6914
| peers: | ||
| type: array | ||
| items: | ||
| type: integer |
There was a problem hiding this comment.
should there be a minimum and maximum value here like for asn (and also we are missing format)?
There was a problem hiding this comment.
My bad, will update.
| // Enabled indicates whether BGP confederation is enabled. | ||
| Enabled bool `json:"enabled,omitempty"` |
There was a problem hiding this comment.
Do we need an "enabled" field or should the confederation field be a pointer / optional in BGPPolicySpec?
For #6567
Depends on #6914