Skip to content

T7766: FRR: Support multiple labels in BGP-LU - #1066

Merged
sever-sever merged 1 commit into
vyos:currentfrom
hedrok:T7766-bgp-lu-multiple-labels
Nov 27, 2025
Merged

T7766: FRR: Support multiple labels in BGP-LU#1066
sever-sever merged 1 commit into
vyos:currentfrom
hedrok:T7766-bgp-lu-multiple-labels

Conversation

@hedrok

@hedrok hedrok commented Nov 4, 2025

Copy link
Copy Markdown
Contributor

FRR: Support multiple labels in BGP-LU

This is my implementation of FRR support of multiple labels in BGP-LU. It is much easier to review as PR to FRR (see "Related PR(s)" below, I've rebased that branch to stable/10.2 with minimal conflict resolution.

Tested by:

  • Manually on master of FRR
  • Manually on 10.2 FRR (the patch in this PR)
  • All FRR topotest passed for master

For testing ExaBGP was used, see topotest FRRouting/frr@40a900b for configuration.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes)
  • Migration from an old Vyatta component to vyos-1x, please link to related PR inside obsoleted component
  • Other (please describe):

Related Task(s)

Related PR(s)

FRR PR: FRRouting/frr#19961

Checklist:

  • I have read the CONTRIBUTING document
  • I have linked this PR to one or more Phabricator Task(s)
  • My commit headlines contain a valid Task id
  • My change requires a change to the documentation
  • I have updated the documentation accordingly

@github-actions

github-actions Bot commented Nov 4, 2025

Copy link
Copy Markdown

👍
No issues in PR Title / Commit Title

@github-actions github-actions Bot added the current VyOS rolling release label Nov 4, 2025
@sever-sever
sever-sever requested a review from Copilot November 5, 2025 11:30

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds support for multiple labels in BGP-LU (BGP Labeled Unicast) as a patch file for FRRouting (FRR) package builds. The patch extends BGP-LU to handle label stacks of up to 10 labels instead of the previous single label limitation.

Key changes include:

  • Increased BGP_MAX_LABELS from 2 to 10 to accommodate full label stacks in BGP-LU
  • Refactored label handling functions to support multiple labels throughout the codebase
  • Updated display and debugging output to show complete label stacks

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@ritika0313 ritika0313 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review comments have been added to FRR repo PR at:
FRRouting/frr#19961

@dmbaturin dmbaturin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I trust the judgement of @ritika0313 and FRR maintainers, so I think we should merge it.

@ritika0313

Copy link
Copy Markdown
Contributor

A few things that could not be made part of this PR (being re-factoring/optimizing suggestions/future support) and can be considered as a sub-task if needed to:

  1. Refactoring mpls_labels2str :
    Decode the labels and pass them to mpls_label2str() , that way we shall be able to get the string format for the special labels like MPLS_LABEL_IPV4_EXPLICIT_NULL, MPLS_LABEL_ROUTER_ALERT
    or we might want to copy this code/function calling of label2str () to our new function mpls_labels2str()

  2. Set the 4th byte to 0 to avoid garbage data in BGP 3-byte NLRI label

  3. Consider checking the validity of all labels for possible future scenarios like route withdrawals or updates which might lead to an inner invalid label (bgp_route.c subgroup_announce_check(): 2267, bgp_zebra.c bgp_zebra_announce_parse_nexthop : 1339)

@ritika0313 ritika0313 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The code changes look good overall:

  • Both sending and receiving of UPDATE messages with multiple labels is supported
  • Take care of BOS value being appropriately set in a case of single or multiple labels.( Some implicit setting BoS also exists which should have been verified in testing)
  • Correctly calculates the maximum possible number of labels and required size for labels to write in the stream
  • Also, provide backward compatibility for encode_label() used by Mpls VPN.
    Few improvements have been suggested that can be taken up as a sub-task if they need to be incorporated as code optimization.

@hedrok
hedrok marked this pull request as draft November 12, 2025 13:18
@hedrok

hedrok commented Nov 12, 2025

Copy link
Copy Markdown
Contributor Author

Converted to draft till it is under review in FRR. I hope it soon will be merged as there is one approve from maintainer. Then I'll update patch and reopen PR.

Added patch patches/frr/0010-bgp-Support-multiple-labels-in-BGP-LU.patch
It is in FRR upstream master (17a082ee3a5d1295c27323773c523c945af12c3d),
rebased on 10.2
@hedrok
hedrok force-pushed the T7766-bgp-lu-multiple-labels branch from f7e3ce7 to 287bade Compare November 26, 2025 20:07
@hedrok
hedrok marked this pull request as ready for review November 26, 2025 22:13
@hedrok

hedrok commented Nov 26, 2025

Copy link
Copy Markdown
Contributor Author

FRR PR is merged, patch is updated with merged version (rebased on 10.2).
Rebuilt and retested FRR debian package in VyOS - everything works as expected.
Ready to be merged :)

@sever-sever sever-sever left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Merged by upstream, do not see any reason do not merge it.
Nice to ask upstream for backporting it to all stable branches, at least to 10.2

@sever-sever
sever-sever merged commit 88897ba into vyos:current Nov 27, 2025
9 checks passed
@vyosbot vyosbot added mirror-initiated This PR initiated for mirror sync workflow mirror-completed and removed mirror-initiated This PR initiated for mirror sync workflow labels Nov 27, 2025
@hedrok
hedrok deleted the T7766-bgp-lu-multiple-labels branch November 27, 2025 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

current VyOS rolling release mirror-completed

Development

Successfully merging this pull request may close these issues.

7 participants