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

isisd: Fix NHLFE entry memory leaks #16855

Merged
merged 1 commit into from
Sep 24, 2024
Merged

Conversation

zhou-run
Copy link
Contributor

Before adding an NHLFE entry, zebra calls nhlfe_nexthop_active_ipv4()/nhlfe_nexthop_active_ipv6() to check if there is a corresponding directly connected route for the next hop prefix of the NHLFE. If such a route exists, it directly overwrites the output interface of the NHLFE's next hop with the output interface of the directly connected route. If the output interface of the directly connected route is different from the output interface of the NHLFE's next hop, deleting the NHLFE entry later will fail because nhlfe_nhop_match() cannot find the same next hop address and output interface, leading to an NHLFE entry leak.

Of course, the above situation generally does not occur, but ISIS adjacency labels can cause issues.
Establishing neighbors in ISIS only requires the existence of the neighbor's IP address and does not need the neighbor's IP address to be in the same subnet as the local IP address. When zebra creates an adjacency label entry, it uses the neighbor's IP address as the next hop. If the neighbor's IP address is not in the same subnet as the local IP address and there is a local directly connected route with the neighbor's IP address prefix, it will result in an issue where the corresponding adjacency label entry in zebra cannot be deleted.

We cannot enforce that ISIS must establish neighbors using IPv4 and IPv6 addresses in the same subnet, but we can ensure that IPv4 or IPv6 addresses in different subnets do not create adjacency labels.

isisd/isis_sr.c Outdated Show resolved Hide resolved
isisd/isis_sr.c Outdated Show resolved Hide resolved
isisd/isis_sr.c Outdated Show resolved Hide resolved
isisd/isis_sr.c Outdated Show resolved Hide resolved
isisd/isis_sr.c Outdated Show resolved Hide resolved
@github-actions github-actions bot added the rebase PR needs rebase label Sep 19, 2024
@zhou-run zhou-run force-pushed the 202409131731 branch 2 times, most recently from 1a570d4 to 1f77460 Compare September 19, 2024 02:24
Before adding an NHLFE entry, zebra calls nhlfe_nexthop_active_ipv4()/nhlfe_nexthop_active_ipv6() to check if there is a corresponding directly connected route for the next hop prefix of the NHLFE. If such a route exists, it directly overwrites the output interface of the NHLFE's next hop with the output interface of the directly connected route. If the output interface of the directly connected route is different from the output interface of the NHLFE's next hop, deleting the NHLFE entry later will fail because nhlfe_nhop_match() cannot find the same next hop address and output interface, leading to an NHLFE entry leak.

Establishing neighbors in ISIS only requires the existence of the neighbor's IP address and does not need the neighbor's IP address to be in the same subnet as the local IP address. When zebra creates an adjacency label entry, it uses the neighbor's IP address as the next hop. If the neighbor's IP address is not in the same subnet as the local IP address and there is a local directly connected route with the neighbor's IP address prefix, it will result in an issue where the corresponding adjacency label entry in zebra cannot be deleted.

We cannot enforce that ISIS must establish neighbors using IPv4 and IPv6 addresses in the same subnet, but we can ensure that IPv4 or IPv6 addresses in different subnets do not create adjacency labels.

Signed-off-by: zhou-run <[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 Sep 24, 2024

failure is unrelated ... trying just the failed test again

@riw777 riw777 merged commit c12af9f into FRRouting:master Sep 24, 2024
11 checks passed
@zhou-run zhou-run deleted the 202409131731 branch September 25, 2024 02:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants