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

bgpd: Set correct TTL for the dynamic neighbor peers #15121

Merged
merged 1 commit into from
Jan 12, 2024

Conversation

routingrocks
Copy link
Contributor

@routingrocks routingrocks commented Jan 9, 2024

In an EBGP multihop configuration with dynamic neighbors, the TTL configured is not being updated for the socket.

Issue:
Assume the following topology:
Host (Dynamic peer to spine - 192.168.1.100) - Leaf - Spine (192.168.1.1)

When the host establishes a BGP multihop session to the spine, the connection uses the MAXTTL value instead of the configured TTL (in this case, 2). This issue is only observed with dynamic peers.

Logs: look at the TTL is still MAXTTL, instead of “2” configured.

18:13:18.872395 48:b0:2d:0c:58:0b > 48:b0:2d:66:64:6b, ethertype IPv4 (0x0800), length 85: (tos 0xc0, ttl 255, id 32078, offset 0, flags [DF], proto TCP (6), length 71)
192.168.1.100.179 > 192.168.1.1.40967: Flags [P.], cksum 0xfe89 (correct), seq 28406:28425, ack 28424, win 255, options [nop,nop,TS val 4192664793 ecr 2814447051], length 19: BGP
Keepalive Message (4), length: 19

Fix:
Whenever a dynamic peer is created, the socket TTL should be updated with the configured TTL, in this case 2.

19:13:24.894890 48:b0:2d:0c:58:0b > 48:b0:2d:66:64:6b, ethertype IPv4 (0x0800), length 85: (tos 0xc0, ttl 2, id 1131, offset 0, flags [DF], proto TCP (6), length 71)
192.168.1.100.179 > 192.168.1.1.41937: Flags [P.], cksum 0x7a67 (correct), seq 2046150759:2046150778, ack 4286110599, win 255, options [nop,nop,TS val 4196270815 ecr 2818051226], length 19: BGP
Keepalive Message (4), length: 19

Testing: UT

UT logs:
2023-12-29T19:13:21.892205+00:00 host bgpd[1591425]: [WWPV7-YSZB5] Dynamic Neighbor 192.168.1.1/32 matches group test listen range 192.168.1.0/30 2023-12-29T19:13:21.892654+00:00 host bgpd[1591425]: [GBPAR-M31QF] 192.168.1.1 Dynamic Neighbor added, group test count 1 2023-12-29T19:13:21.892993+00:00 host bgpd[1591425]: [GPE2H-K9QRE] bgp_set_socket_ttl: set TxTTL on peer (rtrid 0.0.0.0) socket, err = 2, peer ttl 2

Conflicts:
bgpd/bgp_network.c

Ticket: #

Signed-off-by: Rajesh Varatharaj [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 other than @ton31337 's comments

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.

One nit to fix and LGTM

bgpd/bgp_network.c Outdated Show resolved Hide resolved
bgpd/bgp_network.c Outdated Show resolved Hide resolved
@ton31337 ton31337 added this to the 10.0 milestone Jan 11, 2024
In an EBGP multihop configuration with dynamic neighbors, the TTL configured is not being updated for the socket.

Issue:
Assume the following topology:
Host (Dynamic peer to spine - 192.168.1.100) - Leaf - Spine (192.168.1.1)

When the host establishes a BGP multihop session to the spine,
the connection uses the MAXTTL value instead of the configured TTL (in this case, 2).
This issue is only observed with dynamic peers.

Logs: look at the TTL is still MAXTTL, instead of “2” configured.

18:13:18.872395 48:b0:2d:0c:58:0b > 48:b0:2d:66:64:6b, ethertype IPv4 (0x0800), length 85: (tos 0xc0, ttl 255, id 32078, offset 0, flags [DF], proto TCP (6), length 71)
192.168.1.100.179 > 192.168.1.1.40967: Flags [P.], cksum 0xfe89 (correct), seq 28406:28425, ack 28424, win 255, options [nop,nop,TS val 4192664793 ecr 2814447051], length 19: BGP
Keepalive Message (4), length: 19

Fix:
Whenever a dynamic peer is created, the socket TTL should be updated with the configured TTL, in this case 2.

19:13:24.894890 48:b0:2d:0c:58:0b > 48:b0:2d:66:64:6b, ethertype IPv4 (0x0800), length 85: (tos 0xc0, ttl 2, id 1131, offset 0, flags [DF], proto TCP (6), length 71)
    192.168.1.100.179 > 192.168.1.1.41937: Flags [P.], cksum 0x7a67 (correct), seq 2046150759:2046150778, ack 4286110599, win 255, options [nop,nop,TS val 4196270815 ecr 2818051226], length 19: BGP
        Keepalive Message (4), length: 19

Testing: UT

UT logs:
2023-12-29T19:13:21.892205+00:00 host bgpd[1591425]: [WWPV7-YSZB5] Dynamic Neighbor 192.168.1.1/32 matches group test listen range 192.168.1.0/30
2023-12-29T19:13:21.892654+00:00 host bgpd[1591425]: [GBPAR-M31QF] 192.168.1.1 Dynamic Neighbor added, group test count 1
2023-12-29T19:13:21.892993+00:00 host bgpd[1591425]: [GPE2H-K9QRE] bgp_set_socket_ttl:  set TxTTL on peer (rtrid 0.0.0.0) socket, err = 2, peer ttl 2

 Conflicts:
	bgpd/bgp_network.c

Ticket: #
Signed-off-by: Rajesh Varatharaj <[email protected]>
@ton31337 ton31337 merged commit 9453188 into FRRouting:master Jan 12, 2024
9 checks passed
@ton31337
Copy link
Member

@Mergifyio backport stable/9.1 stable/9.0

Copy link

mergify bot commented Jan 12, 2024

backport stable/9.1 stable/9.0

✅ Backports have been created

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