Skip to content

bgp: add local_as support to BGP_NEIGHBOR template #26556

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
briantopping wants to merge 1 commit into
base: master
Choose a base branch
from
Open

bgp: add local_as support to BGP_NEIGHBOR template #26556

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 3 additions & 0 deletions dockers/docker-fpm-frr/frr/bgpd/templates/general/instance.conf.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
!
neighbor {{ neighbor_addr }} remote-as {{ bgp_session['asn'] }}
neighbor {{ neighbor_addr }} description {{ bgp_session['name'] }}
{% if bgp_session['local_as'] is defined %}
Copy link

Copilot AI Apr 4, 2026

Choose a reason for hiding this comment

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

The template checks/uses bgp_session['local_as'], but the upstream CONFIG_DB/YANG schema for BGP neighbors uses local_asn (see src/sonic-yang-models/yang-models/sonic-bgp-common.yang and src/sonic-yang-models/tests/files/sample_config_db.json). As written, setting local_asn on BGP_NEIGHBOR will not render any neighbor ... local-as ... line. Consider switching to local_asn (and optionally supporting both keys for backward compatibility).

Suggested change
{% if bgp_session['local_as'] is defined %}
{% if bgp_session['local_asn'] is defined %}
neighbor {{ neighbor_addr }} local-as {{ bgp_session['local_asn'] }} no-prepend replace-as
{% elif bgp_session['local_as'] is defined %}

Copilot uses AI. Check for mistakes.
neighbor {{ neighbor_addr }} local-as {{ bgp_session['local_as'] }} no-prepend replace-as
Copy link

Copilot AI Apr 4, 2026

Choose a reason for hiding this comment

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

neighbor ... local-as ... no-prepend replace-as is rendered unconditionally whenever local_as is set. CONFIG_DB/YANG also defines local_as_no_prepend and local_as_replace_as flags, and hard-coding both options changes semantics vs plain local-as for users who only want one (or neither). Consider rendering no-prepend/replace-as conditionally based on those attributes (defaulting to FRR defaults when unspecified).

Suggested change
neighbor {{ neighbor_addr }} local-as {{ bgp_session['local_as'] }} no-prepend replace-as
neighbor {{ neighbor_addr }} local-as {{ bgp_session['local_as'] }}{% if bgp_session['local_as_no_prepend'] is defined and bgp_session['local_as_no_prepend'] | int != 0 %} no-prepend{% endif %}{% if bgp_session['local_as_replace_as'] is defined and bgp_session['local_as_replace_as'] | int != 0 %} replace-as{% endif %}

Copilot uses AI. Check for mistakes.
Comment on lines +6 to +7
Copy link

Copilot AI Apr 4, 2026

Choose a reason for hiding this comment

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

This new template behavior isn’t covered by existing bgpcfgd unit tests (e.g., src/sonic-bgpcfgd/tests/test_bgp.py adds peers but doesn’t assert the rendered config for new attributes). Adding a test that sets local_asn (and the optional local_as_no_prepend / local_as_replace_as flags if supported) and asserts the generated neighbor ... local-as ... line would prevent regressions.

Copilot uses AI. Check for mistakes.
{% endif %}
{# set the bgp neighbor timers if they have not default values #}
{% if (bgp_session['keepalive'] is defined and bgp_session['keepalive'] | int != 60)
or (bgp_session['holdtime'] is defined and bgp_session['holdtime'] | int != 180) %}
Expand Down
Loading