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

zebra: skip blackhole nexthop when sending multipath netlink updates #14613

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bhinin
Copy link
Contributor

@bhinin bhinin commented Oct 17, 2023

In case of multipath, when zebra is sending netlink updates to the kernel, if one of the nexthops is a blackhole zebra tags on unnecessary bytes at the end of the message.

This is rejected by kernel resulting in zebra marking the route as rejected. This change fixes that by skipping a blackhole nexthop

In case of multipath, when zebra is sending netlink updates to the
kernel, if one of the nexthops is a blackhole zebra tags on unnecessary
bytes at the end of the message.

This is rejected by kernel resulting in zebra marking the route as
rejected. This change fixes that by skipping a blackhole nexthop

Signed-off-by: Abhishek Naik <[email protected]>
@ton31337
Copy link
Member

Dup #14606?

@bhinin
Copy link
Contributor Author

bhinin commented Oct 17, 2023

Dup #14606?

No, this is a separate bug fix. I raised the pull request now for ease of discussion in #14606.

Copy link
Member

@donaldsharp donaldsharp left a comment

Choose a reason for hiding this comment

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

This is not a rt_netlink.c problem. It's a generic problem that should be handled higher up in zebra before anything reaches the dataplane.

From a routing perspective it makes no sense ( and the linux kernel doesn't allow you ) to have a ecmp path that has 1 blackhole and other paths that can be sent along )

This should be fixed up in the nexthop resolution phase inside of zebra_nhg.c probably nexthop_active_check and the route should be marked as uninstallable.

@bhinin
Copy link
Contributor Author

bhinin commented Oct 17, 2023

This is not a rt_netlink.c problem. It's a generic problem that should be handled higher up in zebra before anything reaches the dataplane.

From a routing perspective it makes no sense ( and the linux kernel doesn't allow you ) to have a ecmp path that has 1 blackhole and other paths that can be sent along )

This should be fixed up in the nexthop resolution phase inside of zebra_nhg.c probably nexthop_active_check and the route should be marked as uninstallable.

Thanks!

rt_netlink is already adding the required space to the message and changing the length. It's just when it's time to add the ifindex that it checks for NEXTHOP_TYPE_BLACKHOLE. and skips. This results in some 0s tagged at the end of the netlink update.

Are you suggesting that this be fixed such that the blackhole nexthop would not even show up when ALL_NEXTHOPS_PTR(dplane_ctx_get_ng(ctx), nexthop) is called?

@donaldsharp
Copy link
Member

yes exactly. This is not a dataplane problem as that the same thing can happen on non-linux or even fully asic platforms. As such it needs to be handled before it hits the data plane code in any way shape form or fashion

Copy link

This PR is stale because it has been open 180 days with no activity. Comment or remove the autoclose label in order to avoid having this PR closed.

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