Hi, On 12/12/2018 10:02 AM, Anton Degtyarev wrote:
Hi Donald,
Yes, Renato had contacted me about this as well. Thank you for adding the needed tests. I will redo my tests in the lab I had prepared for a customer use case.
please also run bgp_l3vpn_to_bgp_vrf -- this actually would have caught the breakage in the first place, but wasn't being run as part of the original CI.
The fix I had submitted was needed for locally defined subnets in a VRF which needed to be leaked into L3VPN. The issue I had observed was that they weren't -- because zebra thought that these subnets were not reachable. My theory (which worked for me and the customer) was that zebra was treating these prefixes wrong what, I thought, the patch corrected. Apologies for a high-level response -- I do not remember all details as this was a month ago.
I suspect that there are types/sub-types that need to be added to the code if you make the original change...
I will recreate the design I was testing again with the recent FRR master code and will give the new topotests a try as well.
Please also run the old one (the CI problem has been corrected and the test is now being included.)
Best regards, Anton
On Wed, 12 Dec 2018 at 17:30, Donald Sharp <sharpd@cumulusnetworks.com <mailto:sharpd@cumulusnetworks.com>> wrote:
Anton -
Recently you had e23b9ef6d2 committed into FRR. During subsuquent testing it was noticed that this change broke various forms of route-leaking. In order to preserve current functionality, we have backed this commit out. My apologies for not catching these issues earlier on initial submission. In the meantime, we've added new tests to the topotests to catch this problem from happening in the future( see tests/topotests/ bgp-vrf-route-leak-basic and bgp_l3vpn_to_bgp_vrf ).
Lou and I believe that your initial approach was probably the right thing to do but it needs to not break existing functionality. What was the use case you were needing this change for?
donald