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

Bgp connect refactor #17810

Merged
merged 3 commits into from
Jan 15, 2025
Merged

Conversation

donaldsharp
Copy link
Member

No description provided.

Copy link
Member

@ton31337 ton31337 left a comment

Choose a reason for hiding this comment

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

Overall LGTM.

@@ -419,6 +419,7 @@ static void bgp_accept(struct event *thread)

/* Accept client connection. */
bgp_sock = sockunion_accept(accept_sock, &su);
zlog_debug("DBS: accepted: %pSUp", &su);
Copy link
Member

Choose a reason for hiding this comment

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

Leftovers?

bgpd/bgpd.h Outdated
@@ -1350,8 +1353,8 @@ struct peer {
char *update_if;
union sockunion *update_source;

union sockunion *su_local; /* Sockunion of local address. */
union sockunion *su_remote; /* Sockunion of remote address. */
// union sockunion *su_local; /* Sockunion of local address. */
Copy link
Member

Choose a reason for hiding this comment

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

Can we drop them actually?

Let's make it so.

Signed-off-by: Donald Sharp <[email protected]>
su_local and su_remote in the peer can change based upon
if we are initiating the remote connection or receiving it.
As such we need to treat it as a property of the connection.

Signed-off-by: Donald Sharp <[email protected]>
Currently bgp is repeatedly grabbing peer connection information.
This is a bit overkill.  There are two situations:

a) Opening a connection to the peer
   In this case, we know the remote port/address a priori and can get
   the local information by just asking the OS.
b) Peer opening a connection to us.
   In this case, we know the local port/address a priori and can get
   the remote information by just asking the OS.

Modify the code to just grab this data at the appropriate time.

Signed-off-by: Donald Sharp <[email protected]>
Copy link
Member

@riw777 riw777 left a comment

Choose a reason for hiding this comment

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

looks good

@riw777
Copy link
Member

riw777 commented Jan 15, 2025

Build FRR-PULLREQ3-STATICANALYZER-6919 had to be cancelled: it was marked as in progress in DB but no agents were assigned to it. (13 Jan 2025, 8:30:57 AM)

looks like CI is stuck

@riw777
Copy link
Member

riw777 commented Jan 15, 2025

ci:rerun

@riw777 riw777 merged commit 66a5d76 into FRRouting:master Jan 15, 2025
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants