Uniform BGP router ID selection for IPv4 and IPv6

[Atish-iaf]

Fixes #6550

[antoninbas]

I think that for IPv4 / dual-stack we should keep using the Node's IPv4 address, as this is common practice AFAIK.
Issue #6550 was more about making sure that the node.antrea.io/bgp-router-id annotation is always honored, not just for IPv6-only clusters.

[Atish-iaf]

I think that for IPv4 / dual-stack we should keep using the Node's IPv4 address, as this is common practice AFAIK. Issue #6550 was more about making sure that the node.antrea.io/bgp-router-id annotation is always honored, not just for IPv6-only clusters.

So if i understand correctly, it means that incase of IPv4/dual-stack -

• when node.antrea.io/bgp-router-id annotation is provided, use it.

• when not provided, use Node's IPv4 address (no need to generate like we do it in IPv6 only).

[antoninbas]

@Atish-iaf correct.
I actually had written the following for the documentation. We didn't use it originally because it didn't match the implementation, but it should be correct after your change:

The BGP router identifier (ID) is a 4-byte field that is usually represented as an IPv4 address. Antrea uses the following
steps to choose the BGP router ID:

1. If the `node.antrea.io/bgp-router-id` annotation is present on the Node and its value is a valid IPv4 address string,
   we will use the provided value.
2. Otherwise, for an IPv4-only or dual-stack Kubernetes cluster, the Node's IPv4 address (assigned to the transport
   interface) is used.
3. Otherwise, for IPv6-only clusters, a 32-bit integer will be generated by hashing the Node's name, then converted to the
   string representation of an IPv4 address.

After this selection process, the `node.antrea.io/bgp-router-id` annotation is added / updated as necessary to reflect
the selected BGP router ID.

[tnqn]

LGTM

[hongliangl]

LGTM overall

[hongliangl]

LGTM

[antoninbas]

If the Node's IPv4 address is updated, the router ID will stay the same. I believe this is slightly different than the previous behavior, as the router ID would have been updated the next time the BGPPolicy CR was synced. I don't see a simple way to achieve this anymore: once we set the annotation, we will keep using the annotation value indefinitely. Is this an issue @tnqn @hongliangl ?

[rajnkamr]

If the Node's IPv4 address is updated, the router ID will stay the same. I believe this is slightly different than the previous behavior, as the router ID would have been updated the next time the BGPPolicy CR was synced. I don't see a simple way to achieve this anymore: once we set the annotation, we will keep using the annotation value indefinitely. Is this an issue @tnqn @hongliangl ?

Router id must be unique within AS,
current approach may present concerns, as the Router ID could be based on an outdated IPv4 address if the node's IP address has changed. Since the BGP policy sync does not update the Router ID when it is set via annotation, the Router ID will not reflect the node's current IP address.
This discrepancy could potentially lead to confusion as the previous annotated ip could become router-id of some other node !
Since router Id is used to identify each BGP neighbor, If two routers try to establish an adjacency and they have the same router-Id then the adjacency will never come up.
If a router receive a route where the originating router-Id is the local router-Id then the route will be discarded.

However for already established session, if the Router ID changes when a node restarts, can cause Graceful Restart to fail, the annotation allows to keep using the annotation value as router id which will work well for graceful restart case.

[tnqn]

If the Node's IPv4 address is updated, the router ID will stay the same. I believe this is slightly different than the previous behavior, as the router ID would have been updated the next time the BGPPolicy CR was synced. I don't see a simple way to achieve this anymore: once we set the annotation, we will keep using the annotation value indefinitely. Is this an issue @tnqn @hongliangl ?

I'm not sure if other functions can really work if a Node IP changes. We had one connectvity issue caused by it before and had it fixed, but the scenario has never been validated comprehensively and I have never heard of a real use case needing dynamic Node IP. Node IP was usually updated by accident, like DHCP not working properly. And given this can be resolved by just removing or updating the annotation, I feel ok to just document it clearly: If a router ID is not specified via the annotation, Antrea will automatically generate a default value based on the Node's IPv4 address or a hash of the Node's name and set it in the annotation. This default value is assigned once and will not change if the Node's IPv4 address is updated.

[antoninbas]

Thanks @tnqn and @rajnkamr

Node IP was usually updated by accident, like DHCP not working properly

Looking into this a bit more, it seems that even if the IP address changes, it won't be reflected in K8s until kubelet is restarted, so there would be manual steps involved anyway.
Antrea also would not register a Node IP change until a restart, so my earlier comment was not fully accurate, as the BGP controller would keep using the old IP.

I feel ok to just document it clearly: If a router ID is not specified via the annotation, Antrea will automatically generate a default value based on the Node's IPv4 address or a hash of the Node's name and set it in the annotation. This default value is assigned once and will not change if the Node's IPv4 address is updated.

That sounds good to me. @Atish-iaf can you make this explicit in the documentation: the router ID is generated once and will not be updated if the Node configuration changes (e.g., if the Node's IPv4 address is updated).

[tnqn]

LGTM

[antoninbas]

/test-all