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

ldpd: Option for disabled LDP hello message during TCP #18417

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

AndriiFullroot
Copy link

@AndriiFullroot AndriiFullroot commented Mar 18, 2025

Currently the LDPD services on TCP session establishment sends additional UDP multicast hello message. Overall, it should speed up label acknowledge.
On the big LAN with a lot of LDPD services(100+) that may reconnect it would lead to a HUGE traffic in the network, when we would lost any performance. So, this PR propose an option to disable "additional hello" during the TCP establishment.

Added option "disable-establish-hello" that disables sending additional LDP hello multicast messages during TCP session establishment. This option enables per interface: "(config-ldp-af-if)"

Usage example:

VyOS# configure
VyOS(config)# mpls ldp
VyOS(config-ldp)# address-family ipv4
VyOS(config-ldp-af)# interface eth0
VyOS(config-ldp-af-if)# disable-establish-hello
VyOS(config-ldp-af-if)# end
VyOS# show run
Building configuration...

Current configuration:
!
frr version 10.4-dev
frr defaults traditional
hostname VyOS
no ipv6 forwarding
!
mpls ldp
 !
 address-family ipv4
  ! Incomplete config, specify a discovery transport-address
  !
  interface eth0
   disable-establish-hello
  exit
  !
 exit-address-family
 !
exit
!
end

@AndriiFullroot AndriiFullroot force-pushed the VyOS_T7226 branch 2 times, most recently from 2537340 to 8c3d2ad Compare March 18, 2025 16:37
@frrbot frrbot bot added the ldp label Mar 18, 2025
@AndriiFullroot AndriiFullroot changed the title T7226 Option for disabled LDP hello message during TCP ldpd: Option for disabled LDP hello message during TCP Mar 18, 2025
@sever-sever
Copy link

some background https://vyos.dev/T7226

@AndriiFullroot AndriiFullroot force-pushed the VyOS_T7226 branch 2 times, most recently from cf40f7e to 945eff4 Compare March 20, 2025 16:16
@donaldsharp
Copy link
Member

please add a topotest that exercises this new functionality. Additionally this command needs to be documented in the ldpd documentation

@github-actions github-actions bot added size/L and removed size/M labels Mar 25, 2025
Andrii Melnychenko added 2 commits March 25, 2025 14:52
Added option "disable-establish-hello" that disableds
sending additional LDP hello multicast messages during
TCP session establishment.
This option enables per interface: "(config-ldp-af-if)".

Signed-off-by: Andrii Melnychenko <[email protected]>
@riw777
Copy link
Member

riw777 commented Mar 25, 2025

I'm a bit confused, I think ... from the external link:

LDP Hello packets are generated to answer incoming Hello before forming neighbor adjacency. According to RFC 5036, Hello packets should be generated periodically at a certain interval. By default, it is 5 sec. But in FRR we see strange behavior. If a router has not formed an LDP neighbor adjacency yet, it answers to all received LDP Hello packets from not neighbors with new Hello packets. That behavior is incorrect.

But then this pr seems to make it an optional change? Is this a problem with the way we implement the spec or is this just an optimization ... ??

@AndriiFullroot
Copy link
Author

I'm a bit confused, I think ... from the external link:

LDP Hello packets are generated to answer incoming Hello before forming neighbor adjacency. According to RFC 5036, Hello packets should be generated periodically at a certain interval. By default, it is 5 sec. But in FRR we see strange behavior. If a router has not formed an LDP neighbor adjacency yet, it answers to all received LDP Hello packets from not neighbors with new Hello packets. That behavior is incorrect.

But then this pr seems to make it an optional change? Is this a problem with the way we implement the spec or is this just an optimization ... ??

Well, sending an additional LDP hello is a valid optimization to speed up the TCP session establishment. The problem appears when there are a lot of ldpd services in a local network. So, this patch adds an option to NOT send the "additional" LDP hello so admins may configure their network if they see the network may be overflowing with multicast traffic, which brings more harm then good.

@AndriiFullroot
Copy link
Author

please add a topotest that exercises this new functionality. Additionally this command needs to be documented in the ldpd documentation

Added please review and provide comments.

…tion

Test runs ldpd on 3 routers with and without option.
Iptables is used to count LDP hello messages.

Signed-off-by: Andrii Melnychenko <[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

@AndriiFullroot
Copy link
Author

AndriiFullroot commented Apr 1, 2025

I've checked on my local docker ubuntu22 image:

docker exec frr-ubuntu22 bash -c 'cd ~/frr/tests/topotests ; sudo pytest -v -s ldp_establish_hello_topo1/test_establish_hello_topo1.py'
....
stdout: /bin/bash: line 1: iptables: command not found
....

Not sure what the correct solution is.
Add iptables to the Dockerfile? Is it suitable?
Any suggestions?

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.

4 participants