bgpd: vpn route-map config should be kept, except if vrf list is on #3162
Conversation
When executing vpn route-map config for importation, the running-config records vrf import route-map instead. Actually, this is a problem when restarting configuring when using vpn route-map. The choice is done to move to vrf format, when at least one import list is created for vrfs. Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
Continuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5612/ This is a comment from an EXPERIMENTAL automated CI system. CLANG Static Analyzer Summary
No Changes in Static Analysis warnings compared to base |
💚 Basic BGPD CI results: SUCCESS, 0 tests failedResults table
For details, please contact louberger |
| vty_out(vty, "%*sroute-map vpn import %s\n", indent, "", | ||
| bgp->vpn_policy[afi] | ||
| .rmap_name[BGP_VPN_POLICY_DIR_FROMVPN]); | ||
| } |
There was a problem hiding this comment.
If I'm reading the code right, these two commands (import vrf route-map and route-map vpn import) are changing the same data structure bgp->vpn_policy[afi].rmap[BGP_VPN_POLICY_DIR_FROMVPN].
Thinking from the perspective of the new northbound API, retrofitting these commands to the new model will be problematic. Is there a reason why we can't combine them into one single command to remove the redundancy?
There was a problem hiding this comment.
good point Renato.
currently, there are some checks that do not permit using vpn command when vrf command is used.
I think we should speak to @paulzlabn
There was a problem hiding this comment.
send paul mail via paulz@labn.net
There was a problem hiding this comment.
To be honest I don't see how this helps in any way. We still have two commands editing the same data structure. Before we would display import vrf route-map in the running configuration even when we entered the route-map vpn import command. Now it's the other way round: we'll display route-map vpn import even if we entered the import vrf route-map command, except if import vrf is also configured. For me it seems like we're changing one problem by another one, frr-reload.py will still have issues given its current limitations.
|
Renato has a point for future work but I think this PR can probably be merged for now. Can some of the BGP people please review & merge it? |
There was a problem hiding this comment.
We should probably open a new PR to think about how to merge these two commands, as per @rwestphal; for now I think it's okay to merge.
When executing vpn route-map config for importation, the running-config
records vrf import route-map instead. Actually, this is a problem when
restarting configuring when using vpn route-map. The choice is done to
move to vrf format, when at least one import list is created for vrfs.
Signed-off-by: Philippe Guibert philippe.guibert@6wind.com