[VRF]: FRR config templates support VRF #3047
Conversation
Merge Azure/sonic-buildimage
1. BGP_NEIGHBOR key extend to vrf_name|ip_address 2. BGP_PEER_RANGE add an attribute vrf_name 3. Add table STATIC_ROUTE: a. key is prefix or vrf_name|prefix b. fields are nexthop, distance, nexthop-vrf c. nexthop is mandatory, its value can be ip_address or if_alias
|
what is the context of this pr? have we reviewed the design? |
1. Compatible with old interface table, which has no name keys, to deal with config in .xml file 2. Sort all config and then output .conf file 3. Rollback advertise 64 prefix for lo ipv6 addresses 4. Add seperator between default route and static route
|
@lguohan there is the description about frr template change In vrf-hld-1.2 which has been approved. |
|
@tylerlinp , can you resolve the conflict for this? |
|
retest vs please |
|
@tylerlinp , can you please resolve the conflicts |
dockers/docker-fpm-frr/bgpcfgd
Outdated
| @@ -93,12 +93,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() | |||
There was a problem hiding this comment.
vrfs could be a list here
dockers/docker-fpm-frr/bgpcfgd
Outdated
| js_bgp = json.loads(out) | ||
| peers = set(js_bgp.keys()) | ||
| js_vrf = json.loads(out) | ||
| vrfs = set(js_vrf['vrfs'].keys()) |
There was a problem hiding this comment.
you don't need to change it to set
dockers/docker-fpm-frr/bgpcfgd
Outdated
| vrfs = set(js_vrf['vrfs'].keys()) | ||
|
|
||
| peers = set() | ||
| for vrf in vrfs: |
There was a problem hiding this comment.
this require list not set
dockers/docker-fpm-frr/bgpcfgd
Outdated
| 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)) |
There was a problem hiding this comment.
I think this is debug output
dockers/docker-fpm-frr/bgpcfgd
Outdated
| vrf = 'default' | ||
| nbr = key | ||
| cmds.append('router bgp {}'.format(self.bgp_asn)) | ||
| syslog.syslog(syslog.LOG_INFO, 'Enter router bgp {}'.format(self.bgp_asn)) |
| {% endfor %} | ||
| {% set vrf_routers = [] %} | ||
| {% set vrf_bgp_nbr = {} %} | ||
| {% for nbr_key in BGP_NEIGHBOR %} |
There was a problem hiding this comment.
I suggest to put this into bgpd.peer template.
There was a problem hiding this comment.
This can not move out because peer-group PEER_V4/PEER_V6 need config in these vrfs.
|
retest vs please |
|
retest mellanox please |
|
retest vsimage please |
|
retest vsimage please |
1 similar comment
|
retest vsimage please |
| {% endif %} | ||
| {% if vrf_bgp_pr[pr_vrf].update({pr_key:BGP_PEER_RANGE[pr_key]}) %} | ||
| {% endif %} | ||
| {% endfor %} |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
|
retest vsimage please |
1 similar comment
|
retest vsimage please |
|
@prsunny @pavel-shirshov Could you please merge this PR, as vrf pytest need it to load config. |
|
retest vsimage please |
1 similar comment
|
retest vsimage please |
|
@pavel-shirshov , can you please review this and sign-off? |
|
This will be obsoleted. |
|
@prsunny It is obsoleted. Also as I commented before I don't think that having all the logic in j2 makes our templates easy to maintain. |
|
@pavel-shirshov - wondering what's the PR for the FRR config for vrf, would you please point it? |
|
@Rihui we don't have such pr. |
|
@pavel-shirshov , I was referring to the 201911 feature as described in https://github.com/Azure/SONiC/blob/master/doc/vrf/sonic-vrf-hld.md; this PR was associated with that originally. |
[VRF]: FRR config templates support VRF sonic-net#3047.
- What I did
Add VRF support for FRR config templates:
frr.conf.j2
bgpd.conf.j2
zebra.conf.j2
- How I did it
Based the following config schema and j2 templates, sonic-cfggen generate .conf file
"BGP_NEIGHBOR": {
"10.0.0.61": {
"rrclient": "0",
"local_addr": "10.0.0.60",
"asn": "64015",
"name": "ARISTA15T0",
"nhopself": "0"
},
"Vrf-blue|10.0.0.49": {
"rrclient": "0",
"local_addr": "10.0.0.48",
"asn": "64009",
"name": "ARISTA09T0",
"nhopself": "0"
}
}
"BGP_PEER_RANGE": {
"PeerGroup": {
"name": "PeerGroup",
"ip_range": [
"192.168.0.0/27"
]
},
"PeerGroup1": {
"vrf_name": "Vrf-blue",
"name": "PeerGroup1",
"ip_range": [
"192.168.8.0/27"
]
}
}
a. key is prefix or vrf_name|prefix
b. fields are nexthop, distance, nexthop-vrf
c. nexthop is mandatory, its value can be ip_address or if_alias
"STATIC_ROUTE": {
"11.11.11.0/24": {
"distance": "10",
"nexthop": "1.1.1.1"
},
"Vrf-blue|22.11.11.0/24": {
"distance": "10",
"nexthop-vrf": "Vrf-red",
"nexthop": "2.1.1.1"
},
"Vrf-red|11.11.11.0/24": {
"nexthop": "1.1.1.1"
}
}
- How to verify it
Pass ansible test cases and various diffrent configurations generate tests
- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)