Skip to content

Fixes bug 5362, use local initiator IP for BGP neighbor as update-sou… #5531

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.

Sign up for GitHub

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

Open
n3wtype wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fixes bug 5362, use local initiator IP for BGP neighbor as update-sou… #5531

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
237d2df
Fixes bug #5362, use local initiator IP for BGP neighbor as update-so…
Jun 30, 2026
a763adc
Merge branch 'opnsense:master' into fix_5362_localip
n3wtype Jul 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions net/frr/src/opnsense/mvc/app/controllers/OPNsense/Quagga/forms/dialogEditBGPNeighbor.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@
<label>Local Initiater IP</label>
<type>text</type>
<advanced>true</advanced>
<help>Specify the local IP address used to establish connections with the neighbor. Only relevant for MD5 authentication.</help>
<help>Specify the local IP address (IPv4 or IPv6) used as the source to initiate BGP/BFD sessions, including MD5 authentication. Mutually exclusive with Update-Source Interface.</help>
<grid_view>
<visible>false</visible>
</grid_view>
Expand All @@ -79,7 +79,7 @@
<id>neighbor.updatesource</id>
<label>Update-Source Interface</label>
<type>dropdown</type>
<help>Interface (IPv4) where BGP sessions are sourced from, typically required when using loopback addresses.</help>
<help>Interface (IPv4) where BGP sessions are sourced from, typically required when using loopback addresses. Mutually exclusive with Local Initiator IP.</help>
<grid_view>
<visible>false</visible>
</grid_view>
Expand Down
20 changes: 20 additions & 0 deletions net/frr/src/opnsense/mvc/app/models/OPNsense/Quagga/BGP.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace OPNsense\Quagga;

use OPNsense\Base\Messages\Message;
use OPNsense\Base\BaseModel;

/*
Expand All @@ -28,4 +29,23 @@
*/
class BGP extends BaseModel
{
/**
* {@inheritdoc}
*/
public function performValidation($validateFullModel = false)
{
$messages = parent::performValidation($validateFullModel);
foreach ($this->neighbors->neighbor->iterateItems() as $neighbor) {
if (!$validateFullModel && !$neighbor->isFieldChanged()) {
continue;
}
if (!$neighbor->updatesource->isEmpty() && !$neighbor->localip->isEmpty()) {
$messages->appendMessage(new Message(
gettext("Update-Source Interface and Local Initiator IP are mutually exclusive."),

@Monviech Monviech Jul 1, 2026

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.

https://docs.opnsense.org/development/frontend/models_constraints.html#adding-constraints

Maybe this could be done with a SingleSelectConstraint
https://github.com/opnsense/core/blob/master/src/opnsense/mvc/app/models/OPNsense/Base/Constraints/SingleSelectConstraint.php

CC @fichtner

@fichtner fichtner Jul 1, 2026

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.

yes sure. this beats documenting things that people won't read regarding updating help texts we can easily avoid

@n3wtype n3wtype Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

thanks for pointing out constraints, working on that

$neighbor->updatesource->__reference
));
}
}
return $messages;
}
}
10 changes: 10 additions & 0 deletions net/frr/src/opnsense/mvc/app/views/OPNsense/Quagga/bgp.volt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,16 @@ POSSIBILITY OF SUCH DAMAGE.
}
});

function checkNeighborSourceConflict() {
const $block = $('#help_block_neighbor\\.updatesource');
if ($('#neighbor\\.updatesource').val() && $('#neighbor\\.localip').val()) {
$block.text('{{ lang._("Update-Source Interface and Local Initiator IP are mutually exclusive.") }}');
} else {
$block.text('');
}
}
$(document).on('change', '#neighbor\\.updatesource, #neighbor\\.localip', checkNeighborSourceConflict);

mapDataToFormUI({'frm_bgp_settings':"/api/quagga/bgp/get"}).done(function(data){
formatTokenizersUI();
$('.selectpicker').selectpicker('refresh');
Expand Down
4 changes: 3 additions & 1 deletion net/frr/src/opnsense/service/templates/OPNsense/Quagga/bgpd.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,9 @@ router bgp {{ OPNsense.quagga.bgp.asnumber }}
{% if 'disable_connected_check' in neighbor and neighbor.disable_connected_check == '1' %}
neighbor {{ neighbor.address }} disable-connected-check
{% endif %}
{% if ':' not in neighbor.address and 'updatesource' in neighbor and neighbor.updatesource != '' %}
{% if 'localip' in neighbor and neighbor.localip != '' %}

@AdSchellevis AdSchellevis Jul 1, 2026

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.

at a first glance I would expect we can trust the validation here, in which case we shouldn't have to duplicate the logic in the template.

@n3wtype n3wtype Jul 1, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure if this is duplicates validation. uupdate-source could be both set by both updatesource and localip field. Validation on UI/API makes sure that only of them is set. However still in template we need to check which one is set to use correct variable.

Additionally later on there is additional logic involving updatesource field and IPv6 which I didn't want to change, as I believe may be not fully correct. There could bnothing wrong with setting update-source parameter for ipv6 neighbor and it prevents such setting - next thing to verify.

neighbor {{ neighbor.address }} update-source {{ neighbor.localip }}
{% elif ':' not in neighbor.address and 'updatesource' in neighbor and neighbor.updatesource != '' %}
neighbor {{ neighbor.address }} update-source {{ physical_interface(neighbor.updatesource) }}
{% endif %}
{% if ':' in neighbor.address and 'linklocalinterface' in neighbor and neighbor.linklocalinterface != '' %}
Expand Down