Skip to content

T9013: Add FRR patch to fix BMP connect source-interface deletion - #1225

Merged
sever-sever merged 1 commit into
vyos:rollingfrom
natali-rs1985:T9013
Jun 25, 2026
Merged

T9013: Add FRR patch to fix BMP connect source-interface deletion#1225
sever-sever merged 1 commit into
vyos:rollingfrom
natali-rs1985:T9013

Conversation

@natali-rs1985

Copy link
Copy Markdown
Contributor

Change summary

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)

How to test / Smoketest result

Before the fix:

vyos(config-bgp-bmp)# 
vyos(config-bgp-bmp)# bmp connect 192.0.0.1 port 5000 min-retry 1000 max-retry 2000 source-interface eth1
vyos(config-bgp-bmp)# do show running-config 
Building configuration...

Current configuration:
!
frr version 10.5.2
frr defaults traditional
hostname vyos
log syslog notifications
log timestamp precision 3
no log unique-id
service integrated-vtysh-config
!
ip route 0.0.0.0/0 XX.XX.XX.254 eth2 tag 210 210
!
router bgp 65001
 no bgp ebgp-requires-policy
 no bgp reject-as-sets
 no bgp default ipv4-unicast
 no bgp network import-check
 !
 bmp targets test
  bmp connect 192.0.0.1 port 5000 min-retry 1000 max-retry 2000 source-interface eth1
 exit
exit
!
end
vyos(config-bgp-bmp)# no bmp connect 192.0.0.1 port 5000 min-retry 1000 max-retry 2000 source-interface eth1
% No such active connection found
vyos(config-bgp-bmp)# no bmp connect 192.0.0.1 port 5000 min-retry 1000 max-retry 2000 source-interface eth0
vyos(config-bgp-bmp)# do show running-config 
Building configuration...

Current configuration:
!
frr version 10.5.2
frr defaults traditional
hostname vyos
log syslog notifications
log timestamp precision 3
no log unique-id
service integrated-vtysh-config
!
ip route 0.0.0.0/0 XX.XX.XX.254 eth2 tag 210 210
!
router bgp 65001
 no bgp ebgp-requires-policy
 no bgp reject-as-sets
 no bgp default ipv4-unicast
 no bgp network import-check
 !
 bmp targets test
 exit
exit
!
end
vyos(config-bgp-bmp)# 

After the fix:

vyos(config-bgp-bmp)# 
vyos(config-bgp-bmp)# bmp connect 192.0.0.1 port 5000 min-retry 1000 max-retry 2000 source-interface eth1
vyos(config-bgp-bmp)# do show running-config 
Building configuration...

Current configuration:
!
frr version 10.6.1
frr defaults traditional
hostname vyos
log syslog notifications
log timestamp precision 3
no log unique-id
service integrated-vtysh-config
!
ip route 0.0.0.0/0 xx.xx.xx.254 eth2 tag 210 210
!
router bgp 65001
 no bgp ebgp-requires-policy
 no bgp reject-as-sets
 no bgp default ipv4-unicast
 no bgp network import-check
 !
 bmp targets test
  bmp connect 192.0.0.1 port 5000 min-retry 1000 max-retry 2000 source-interface eth1
 exit
exit
!
end
vyos(config-bgp-bmp)# no bmp connect 192.0.0.1 port 5000 min-retry 1000 max-retry 2000 source-interface eth1
vyos(config-bgp-bmp)# do show running-config 
Building configuration...

Current configuration:
!
frr version 10.6.1
frr defaults traditional
hostname vyos
log syslog notifications
log timestamp precision 3
no log unique-id
service integrated-vtysh-config
!
ip route 0.0.0.0/0 xx.xx.xx.254 eth2 tag 210 210
!
router bgp 65001
 no bgp ebgp-requires-policy
 no bgp reject-as-sets
 no bgp default ipv4-unicast
 no bgp network import-check
 !
 bmp targets test
 exit
exit
!
end
vyos(config-bgp-bmp)# 

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

@coderabbitai

coderabbitai Bot commented Jun 24, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: d3d8c035-f7a2-427b-ba63-b4dc6f8902db

📥 Commits

Reviewing files that changed from the base of the PR and between 67f7e54 and a982117.

📒 Files selected for processing (1)
  • scripts/package-build/frr/patches/frr/0025-bgpd-fix-bmp-connect-deletion-with-source-interface.patch
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • ansible/ansible (manual)
📜 Recent review details
⏰ Context from checks skipped due to timeout. (3)
  • GitHub Check: codeql-analysis-call / Analyze (python)
  • GitHub Check: Mergify Merge Protections
  • GitHub Check: Summary
🔇 Additional comments (1)
scripts/package-build/frr/patches/frr/0025-bgpd-fix-bmp-connect-deletion-with-source-interface.patch (1)

21-26: LGTM!


📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes
    • Fixed an issue where deleting a BMP connection could be incorrectly rejected when a source interface was configured.
    • Improved validation so the app now correctly matches the provided source interface before allowing deletion.

Walkthrough

Adds a new patch (0025-bgpd-fix-bmp-connect-deletion-with-source-interface.patch) targeting bgpd/bgp_bmp.c. It removes the ! negation from a strcmp() call in bmp_connect(), correcting the condition that matches a configured source-interface against a supplied interface name during BMP connection deletion.

BMP source-interface deletion fix

Layer / File(s) Summary
Fix inverted strcmp in bmp_connect source-interface check
scripts/package-build/frr/patches/frr/0025-bgpd-fix-bmp-connect-deletion-with-source-interface.patch
Changes !strcmp(ba->ifsrc, srcif) to strcmp(ba->ifsrc, srcif) in bgpd/bgp_bmp.c's bmp_connect(), so deletion is rejected (continues loop) when the interface names differ, and accepted when they match.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed Title clearly summarizes the FRR BMP source-interface deletion fix and matches the patch intent.
Description check ✅ Passed Description is on-topic and describes the bug fix, test evidence, and related task for the BMP connect deletion issue.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
✨ Simplify code
  • Create PR with simplified code

Comment @coderabbitai help to get the list of available commands.

@hedrok hedrok 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.

Approve.
Small valid fix of FRR bug.
I've read code + tested FRR, everything works as expected.
I believe we can safely merge this fix even before upstream review.

@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.

Add patch to fix source-interface deletion (for BMP)

@sever-sever
sever-sever merged commit 0e7145f into vyos:rolling Jun 25, 2026
10 of 11 checks passed
@vyos-bot vyos-bot Bot added mirror-initiated This PR initiated for mirror sync workflow mirror-completed and removed mirror-initiated This PR initiated for mirror sync workflow labels Jun 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

3 participants