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

[VRF]: FRR config templates support VRF #3047

Closed
wants to merge 19 commits into from
Closed
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
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
62 changes: 41 additions & 21 deletions dockers/docker-fpm-frr/bgpcfgd
Original file line number Diff line number Diff line change
Expand Up @@ -91,12 +91,22 @@ class BGPConfigManager(object):
daemon.add_manager(swsscommon.CONFIG_DB, swsscommon.CFG_BGP_NEIGHBOR_TABLE_NAME, self.__bgp_handler)

def load_peers(self):
peers = set()
command = ["vtysh", "-c", "show bgp neighbors json"]
vrfs = set()
Copy link
Contributor

Choose a reason for hiding this comment

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

vrfs could be a list here

command = ["vtysh", "-c", "show bgp vrfs json"]
rc, out, err = run_command(command)
if rc == 0:
js_bgp = json.loads(out)
peers = set(js_bgp.keys())
js_vrf = json.loads(out)
vrfs = set(js_vrf['vrfs'].keys())
Copy link
Contributor

Choose a reason for hiding this comment

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

you don't need to change it to set


peers = set()
for vrf in vrfs:
Copy link
Contributor

Choose a reason for hiding this comment

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

this require list not set

command = ["vtysh", "-c", 'show bgp vrf {} neighbors json'.format(vrf)]
rc, out, err = run_command(command)
if rc == 0:
js_bgp = json.loads(out)
for nbr in js_bgp.keys():
peers.add((vrf, nbr))

return peers

def __metadata_handler(self, key, op, data):
Expand All @@ -114,37 +124,48 @@ class BGPConfigManager(object):
def __update_bgp(self):
cmds = []
for key, op, data in self.bgp_messages:
if '|' in key:
vrf, nbr = key.split('|', 1)
cmds.append('router bgp {} vrf {}'.format(self.bgp_asn, vrf))
syslog.syslog(syslog.LOG_INFO, 'Enter router bgp {} vrf {}'.format(self.bgp_asn, vrf))
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is debug output

else:
vrf = 'default'
nbr = key
cmds.append('router bgp {}'.format(self.bgp_asn))
syslog.syslog(syslog.LOG_INFO, 'Enter router bgp {}'.format(self.bgp_asn))
Copy link
Contributor

Choose a reason for hiding this comment

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

debug output

if op == swsscommon.SET_COMMAND:
if key not in self.peers:
if (vrf, nbr) not in self.peers:
try:
txt = self.bgp_peer_add_template.render(DEVICE_METADATA=self.meta, neighbor_addr=key, bgp_session=data)
txt = self.bgp_peer_add_template.render(DEVICE_METADATA=self.meta, neighbor_addr=nbr, bgp_session=data)
cmds.append(txt)
except:
syslog.syslog(syslog.LOG_ERR, 'Peer {}. Error in rendering the template for "SET" command {}'.format(key, data))
syslog.syslog(syslog.LOG_ERR, 'Peer {}. Error in rendering the template for "SET" command {}'.format(nbr, data))
else:
syslog.syslog(syslog.LOG_INFO, 'Peer {} added with attributes {}'.format(key, data))
self.peers.add(key)
syslog.syslog(syslog.LOG_INFO, 'Peer {} added with attributes {}'.format(nbr, data))
self.peers.add((vrf, nbr))
else:
# when the peer is already configured we support "shutdown/no shutdown"
# commands for the peers only
if "admin_status" in data:
if data['admin_status'] == 'up':
cmds.append(self.bgp_peer_no_shutdown.render(neighbor_addr=key))
syslog.syslog(syslog.LOG_INFO, 'Peer {} admin state is set to "up"'.format(key))
cmds.append(self.bgp_peer_no_shutdown.render(neighbor_addr=nbr))
syslog.syslog(syslog.LOG_INFO, 'Peer {} admin state is set to "up"'.format(nbr))
elif data['admin_status'] == 'down':
cmds.append(self.bgp_peer_shutdown.render(neighbor_addr=key))
syslog.syslog(syslog.LOG_INFO, 'Peer {} admin state is set to "down"'.format(key))
cmds.append(self.bgp_peer_shutdown.render(neighbor_addr=nbr))
syslog.syslog(syslog.LOG_INFO, 'Peer {} admin state is set to "down"'.format(nbr))
else:
syslog.syslog(syslog.LOG_ERR, "Peer {}: Can't update the peer. has wrong attribute value attr['admin_status'] = '{}'".format(key, data['admin_status']))
syslog.syslog(syslog.LOG_ERR, "Peer {}: Can't update the peer. has wrong attribute value attr['admin_status'] = '{}'".format(nbr, data['admin_status']))
else:
syslog.syslog(syslog.LOG_INFO, "Peer {}: Can't update the peer. No 'admin_status' attribute in the request".format(key))
syslog.syslog(syslog.LOG_INFO, "Peer {}: Can't update the peer. No 'admin_status' attribute in the request".format(nbr))
elif op == swsscommon.DEL_COMMAND:
if key in self.peers:
cmds.append(self.bgp_peer_del_template.render(neighbor_addr=key))
syslog.syslog(syslog.LOG_INFO, 'Peer {} has been removed'.format(key))
self.peers.remove(key)
if (vrf, nbr) in self.peers:
cmds.append(self.bgp_peer_del_template.render(neighbor_addr=nbr))
syslog.syslog(syslog.LOG_INFO, 'Peer {} has been removed'.format(nbr))
self.peers.remove((vrf, nbr))
else:
syslog.syslog(syslog.LOG_WARNING, 'Peer {} is not found'.format(key))
syslog.syslog(syslog.LOG_WARNING, 'Peer {} is not found'.format(nbr))
cmds.append('!')
syslog.syslog(syslog.LOG_INFO, 'Exit router bgp mode')
self.bgp_messages = []

if len(cmds) == 0:
Expand All @@ -153,7 +174,6 @@ class BGPConfigManager(object):
fd, tmp_filename = tempfile.mkstemp(dir='/tmp')
os.close(fd)
with open (tmp_filename, 'w') as fp:
fp.write('router bgp %s\n' % self.bgp_asn)
for cmd in cmds:
fp.write("%s\n" % cmd)

Expand Down
103 changes: 84 additions & 19 deletions dockers/docker-fpm-frr/bgpd.conf.default.j2
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
!
{% block bgp %}
{% if DEVICE_METADATA['localhost'].has_key('bgp_asn') %}
{% block bgp_init %}
{# block bgp_init #}
pavel-shirshov marked this conversation as resolved.
Show resolved Hide resolved
!
! bgp multiple-instance
!
Expand Down Expand Up @@ -36,7 +37,53 @@ route-map ISOLATE permit 10
route-map set-next-hop-global-v6 permit 10
set ipv6 next-hop prefer-global
!
{% set vrf_lo_intf = {} %}
{% for (name, prefix) in LOOPBACK_INTERFACE|pfx_filter %}
{% set intf_vrf = 'default' %}
{% if LOOPBACK_INTERFACE.has_key(name) and LOOPBACK_INTERFACE[name].has_key('vrf_name') %}
{% set intf_vrf = LOOPBACK_INTERFACE[name]['vrf_name'] %}
{% endif %}
{% if vrf_lo_intf.has_key(intf_vrf) or vrf_lo_intf.update({intf_vrf:[]}) %}
{% endif %}
{% if name not in vrf_lo_intf[intf_vrf] and vrf_lo_intf[intf_vrf].append(name) %}
{% endif %}
{% endfor %}
{% set vrf_routers = [] %}
{% set vrf_bgp_nbr = {} %}
{% for nbr_key in BGP_NEIGHBOR %}
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest to put this into bgpd.peer template.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This can not move out because peer-group PEER_V4/PEER_V6 need config in these vrfs.

{% if nbr_key | length == 2 %}
{% set nbr_vrf = nbr_key[0] %}
{% set nbr_addr = nbr_key[1] %}
{% else %}
{% set nbr_vrf = 'default' %}
{% set nbr_addr = nbr_key %}
{% endif %}
{% if nbr_vrf in vrf_routers or vrf_routers.append(nbr_vrf) %}
{% endif %}
{% if vrf_bgp_nbr.has_key(nbr_vrf) or vrf_bgp_nbr.update({nbr_vrf:{}}) %}
{% endif %}
{% if vrf_bgp_nbr[nbr_vrf].update({nbr_addr:BGP_NEIGHBOR[nbr_key]}) %}
{% endif %}
{% endfor %}
{% set vrf_bgp_pr = {} %}
{% for pr_key in BGP_PEER_RANGE %}
{% set pr_vrf = 'default' %}
{% if BGP_PEER_RANGE[pr_key].has_key('vrf_name') %}
{% set pr_vrf = BGP_PEER_RANGE[pr_key]['vrf_name'] %}
{% endif %}
{% if pr_vrf not in vrf_routers and vrf_routers.append(pr_vrf) %}
{% endif %}
{% if vrf_bgp_pr.has_key(pr_vrf) or vrf_bgp_pr.update({pr_vrf:{}}) %}
{% endif %}
{% if vrf_bgp_pr[pr_vrf].update({pr_key:BGP_PEER_RANGE[pr_key]}) %}
{% endif %}
{% endfor %}
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to put all the code inside of bgpcfgd?
Currently this change put a huge amount of logic inside of the jinja2.
Can we put all the logic into bgpcfgd?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not just peer_range, there are configs even peer. If to aware vrf in conf, we have to loop all vrfs. Now vrfs get from BGP_NEIGHBOR and BGP_PEER_RANGE. Theoretically speaking, all can be put into bgpcfgd, then no j2 template left. But that would be a huge work.

{% for vrf in vrf_routers|sort %}
{% if vrf == 'default' %}
router bgp {{ DEVICE_METADATA['localhost']['bgp_asn'] }}
{% else %}
router bgp {{ DEVICE_METADATA['localhost']['bgp_asn'] }} vrf {{ vrf }}
{% endif %}
bgp log-neighbor-changes
bgp bestpath as-path multipath-relax
no bgp default ipv4-unicast
Expand All @@ -45,43 +92,52 @@ router bgp {{ DEVICE_METADATA['localhost']['bgp_asn'] }}
{% if DEVICE_METADATA['localhost']['type'] == 'ToRRouter' %}
bgp graceful-restart preserve-fw-state
{% endif %}
{% set vrf_lo0 = "" %}
{% if vrf_lo_intf[vrf] | length > 0 %}
{% set vrf_lo0 = vrf_lo_intf[vrf][0] %}
{% endif %}
{% for (name, prefix) in LOOPBACK_INTERFACE|pfx_filter %}
{% if prefix | ipv4 and name == 'Loopback0' %}
{% if prefix | ipv4 and name == vrf_lo0 %}
bgp router-id {{ prefix | ip }}
{% endif %}
{% endfor %}
{# advertise loopback #}
{% for (name, prefix) in LOOPBACK_INTERFACE|pfx_filter %}
{% if prefix | ipv4 and name == 'Loopback0' %}
{% if prefix | ipv4 and name == vrf_lo0 %}
network {{ prefix | ip }}/32
{% elif prefix | ipv6 and name == 'Loopback0' %}
{% elif prefix | ipv6 and name == vrf_lo0 %}
address-family ipv6
network {{ prefix | ip }}/64
exit-address-family
{% endif %}
{% endfor %}
{% endblock bgp_init %}
{% endif %}
{% block vlan_advertisement %}
{# endblock bgp_init #}
{# block vlan_advertisement #}
{% for (name, prefix) in VLAN_INTERFACE|pfx_filter %}
{% set vlan_intf_vrf = 'default' %}
{% if VLAN_INTERFACE.has_key(name) and VLAN_INTERFACE[name].has_key('vrf_name') %}
{% set vlan_intf_vrf = VLAN_INTERFACE[name]['vrf_name'] %}
{% endif %}
{% if vlan_intf_vrf == vrf %}
{% if prefix | ipv4 %}
network {{ prefix }}
{% elif prefix | ipv6 %}
address-family ipv6
network {{ prefix }}
exit-address-family
{% endif %}
{% endif %}
{% endfor %}
{% endblock vlan_advertisement %}
{% block maximum_paths %}
{# endblock vlan_advertisement #}
{# block maximum_paths #}
address-family ipv4
maximum-paths 64
exit-address-family
address-family ipv6
maximum-paths 64
exit-address-family
{% endblock maximum_paths %}
{% block peers_peer_group %}
{# endblock maximum_paths #}
{# block peers_peer_group #}
neighbor PEER_V4 peer-group
neighbor PEER_V6 peer-group
address-family ipv4
Expand All @@ -98,10 +154,10 @@ router bgp {{ DEVICE_METADATA['localhost']['bgp_asn'] }}
neighbor PEER_V6 soft-reconfiguration inbound
neighbor PEER_V6 route-map TO_BGP_PEER_V6 out
exit-address-family
{% endblock peers_peer_group %}
{% block bgp_peers_with_range %}
{% if BGP_PEER_RANGE %}
{% for bgp_peer in BGP_PEER_RANGE.values() %}
{# endblock peers_peer_group #}
{# block bgp_peers_with_range #}
pavel-shirshov marked this conversation as resolved.
Show resolved Hide resolved
{% if vrf_bgp_pr.has_key(vrf) %}
{% for peer_key, bgp_peer in vrf_bgp_pr[vrf]|dictsort %}
neighbor {{ bgp_peer['name'] }} peer-group
neighbor {{ bgp_peer['name'] }} passive
{% if bgp_peer['peer_asn'] is defined %}
Expand All @@ -114,8 +170,12 @@ router bgp {{ DEVICE_METADATA['localhost']['bgp_asn'] }}
{% if bgp_peer['src_address'] is defined %}
neighbor {{ bgp_peer['name'] }} update-source {{ bgp_peer['src_address'] | ip }}
{% else %}
{% set vrf_lo1 = "" %}
{% if vrf_lo_intf[vrf] | length > 1 %}
{% set vrf_lo1 = vrf_lo_intf[vrf][1] %}
{% endif %}
{% for (name, prefix) in LOOPBACK_INTERFACE|pfx_filter %}
{% if name == 'Loopback1' %}
{% if name == vrf_lo1 %}
neighbor {{ bgp_peer['name'] }} update-source {{ prefix | ip }}
{% endif %}
{% endfor %}
Expand All @@ -133,8 +193,9 @@ router bgp {{ DEVICE_METADATA['localhost']['bgp_asn'] }}
exit-address-family
{% endfor %}
{% endif %}
{% endblock bgp_peers_with_range %}
{% block bgp_monitors %}
{# endblock bgp_peers_with_range #}
{# block bgp_monitors #}
{% if vrf == 'default' %}
{% if BGP_MONITORS is defined and BGP_MONITORS|length > 0 %}
neighbor BGPMON_V4 peer-group
{% for (name, prefix) in LOOPBACK_INTERFACE|pfx_filter %}
Expand All @@ -153,5 +214,9 @@ router bgp {{ DEVICE_METADATA['localhost']['bgp_asn'] }}
neighbor {{ neighbor_addr }} activate
{% endfor %}
{% endif %}
{% endblock bgp_monitors %}
{% endif %}
{# endblock bgp_monitors #}
!
{% endfor %}
{% endif %}
{% endblock bgp %}
1 change: 1 addition & 0 deletions dockers/docker-fpm-frr/frr.conf.j2
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ agentx
{% include "zebra.interfaces.conf.j2" %}
!
{% include "staticd.default_route.conf.j2" %}
{% include "staticd.static_route.conf.j2" %}
!
{% include "bgpd.conf.default.j2" %}
!
1 change: 1 addition & 0 deletions dockers/docker-fpm-frr/staticd.conf.j2
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,5 @@
{% include "daemons.common.conf.j2" %}
!
{% include "staticd.default_route.conf.j2" %}
{% include "staticd.static_route.conf.j2" %}
!
52 changes: 52 additions & 0 deletions dockers/docker-fpm-frr/staticd.static_route.conf.j2
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
{% block static_route %}
{% if STATIC_ROUTE %}
{% set glb_rt = {} %}
{% set vrf_rt = {} %}
{% for rt_key in STATIC_ROUTE %}
{% if rt_key is not string() %}
{% if vrf_rt.has_key(rt_key[0]) or vrf_rt.update({rt_key[0]:{}}) %}
{% endif %}
pavel-shirshov marked this conversation as resolved.
Show resolved Hide resolved
{% if vrf_rt[rt_key[0]].update({rt_key[1]:STATIC_ROUTE[rt_key]}) %}
{% endif %}
{% else %}
{% if glb_rt.update({rt_key:STATIC_ROUTE[rt_key]}) %}
{% endif %}
{% endif %}
{% endfor %}
{% for rt_key, rt_attr in glb_rt|dictsort %}
{% if rt_attr.has_key('distance') and rt_attr['distance'] | int != 1 %}
{% if rt_attr.has_key('nexthop-vrf') %}
ip route {{ rt_key }} {{ rt_attr['nexthop'] }} {{ rt_attr['distance'] }} nexthop-vrf {{ rt_attr['nexthop-vrf'] }}
{% else %}
ip route {{ rt_key }} {{ rt_attr['nexthop'] }} {{ rt_attr['distance'] }}
{% endif %}
{% else %}
{% if rt_attr.has_key('nexthop-vrf') %}
ip route {{ rt_key }} {{ rt_attr['nexthop'] }} nexthop-vrf {{ rt_attr['nexthop-vrf'] }}
{% else %}
ip route {{ rt_key }} {{ rt_attr['nexthop'] }}
{% endif %}
{% endif %}
{% endfor %}
!
{% for vrf, rt in vrf_rt|dictsort %}
vrf {{ vrf }}
{% for rt_key, rt_attr in rt|dictsort %}
{% if rt_attr.has_key('distance') and rt_attr['distance'] | int != 1 %}
{% if rt_attr.has_key('nexthop-vrf') %}
ip route {{ rt_key }} {{ rt_attr['nexthop'] }} {{ rt_attr['distance'] }} nexthop-vrf {{ rt_attr['nexthop-vrf'] }}
{% else %}
ip route {{ rt_key }} {{ rt_attr['nexthop'] }} {{ rt_attr['distance'] }}
{% endif %}
{% else %}
{% if rt_attr.has_key('nexthop-vrf') %}
ip route {{ rt_key }} {{ rt_attr['nexthop'] }} nexthop-vrf {{ rt_attr['nexthop-vrf'] }}
{% else %}
ip route {{ rt_key }} {{ rt_attr['nexthop'] }}
{% endif %}
{% endif %}
{% endfor %}
!
{% endfor %}
{% endif %}
{% endblock static_route %}