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: fix prefix same as nexthop in label per nexthop #16990

Merged
merged 2 commits into from
Oct 29, 2024

Conversation

lsang6WIND
Copy link

When a prefix is imported using the "network" command under a vrf, which
is a connected prefix, and in the context of label allocation per
nexthop:

..

router bgp 1 vrf vrf1
address-family ipv4 unicast
redistribute static
network 172.16.0.1/32 <--- connected network
network 192.168.106.0/29
label vpn export auto
label vpn export allocation-mode per-nexthop
..

We encounter an MPLS entry where the nexthop is the prefix itself:

18 BGP 172.16.0.1 -

Actually, when using the "network" command, a bnc context is used, but
it is filled by using the prefix itself instead of the nexthop for other
BGP updates. Consequently, when picking up the original nexthop for
label allocation, the function behaves incorrectly. Instead ensure that
the nexthop type of bnc->nexthop is not a nexthop_ifindex; otherwise
fallback to the per vrf label.

Update topotests.

bgpd/bgp_mplsvpn.c Outdated Show resolved Hide resolved
@ton31337 ton31337 added this to the 10.2 milestone Oct 3, 2024
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 ... waiting on the topo test corrections

@lsang6WIND
Copy link
Author

ci:rerun

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.

LGTM

@ton31337 ton31337 requested a review from riw777 October 15, 2024 06:10
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 Oct 22, 2024

it looks like the lint errors need to be fixed up before we can push

@ton31337
Copy link
Member

@lsang6WIND could you fix lint issues?

Loïc Sang added 2 commits October 25, 2024 09:30
When a prefix is imported using the "network" command under a vrf, which
is a connected prefix, and in the context of label allocation per
nexthop:

..
>router bgp 1 vrf vrf1
> address-family ipv4 unicast
>  redistribute static
>  network 172.16.0.1/32  <--- connected network
>  network 192.168.106.0/29
>  label vpn export auto
>  label vpn export allocation-mode per-nexthop
..

We encounter an MPLS entry where the nexthop is the prefix itself:

> 18             BGP   172.16.0.1     -

Actually, when using the "network" command, a bnc context is used, but
it is filled by using the prefix itself instead of the nexthop for other
BGP updates. Consequently, when picking up the original nexthop for
label allocation, the function behaves incorrectly. Instead ensure that
the nexthop type of bnc->nexthop is not a nexthop_ifindex; otherwise
fallback to the per vrf label.

Update topotests.

Signed-off-by: Loïc Sang <loic.sang@6wind.com>
WARNING: topo: Waiting time is too small
(count=10, wait=0.5), using default values (count=20, wait=3)

Supress warning by inscreasing wait time.

Signed-off-by: Loïc Sang <loic.sang@6wind.com>
@github-actions github-actions bot added the rebase PR needs rebase label Oct 25, 2024
@riw777 riw777 merged commit 4a6e1c0 into FRRouting:master Oct 29, 2024
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