Vivek, On 1/5/2016 7:43 AM, Vivek Venkatraman wrote: > Hi Lou, > > Thank you for posting these patches. I have had a look at a majority > of them, a few more to go. > > As you have mentioned in this message, this set of patches attempts to > provide the Route Reflector behavior for the VPN SAFIs. I have some > comments (or questions) on a few patches which I'll address to the > corresponding mails; in general, I agree with the changes. >
Excellent. We've been working to address some of the comments received (primarily from David) and have an update to Patch 19 to address his catch of non-standard encoding -- this was a bug. I was planning on posting a v2 of this patch today, but can wait for your comments and address those in the same. I've added a few more to the branch to deal with test failures. > I think there will be quite a few things to discuss and agree upon > when the import/export functionality (i.e., PE behavior) is built on > top of these changes (and the VRF support that is being introduced > into BGP), I certainly agree that there's a lot of discussion to be had WRT import/export functionality, and this is part of the reason why this core set doesn't include any of those changes. The intended scope of this patch set was limited to fixing and completing what was there in bgp_mplsvpn got the VPN_SAFI and adding the equivalent function for ENCAP_SAFI. So I think the PE functionality related discussions should not gate acceptance of these fixes/parallel additions. > such as: > > a) How is the organization of the tables and the flow of control for > import and export. > b) How is nexthop resolution handled (with the current set of patches, > VPN nexthops are assumed to be VALID) > c) What is the structure/organization for L2VPN i.e., how are MACs > imported into and maintained in zebra (or another component, if so > decided), changes to the interface between BGP and zebra for MAC > exchange etc. > d) Interaction with MPLS (or labeled routing) > > There have been a few mails exchanged on some of the above topics by > me and others, those discussions can continue probably after giving > some more time for your changes to be discussed. Agreed. I think all of the above should be discussed in the context of PE import / export. I'd also like to add the vty interface additions to the discussion -- as was mentioned previously on the list. I'd almost like to start with this topic as it drives function/code organization almost as much the specs. We can certainly have these discussions in parallel with the discussion on the patch specific changes. Are they any aspects of the above that you want to cover as part of the patch set (vs PE i/o) review that isn't covered in your patch specific emails? Thanks again, Lou > > Thanks, > Vivek > > On Thu, Dec 24, 2015 at 10:10 AM, Lou Berger <[email protected] > <mailto:[email protected]>> wrote: > > This patch set corrects issues with IPv4 MPLS VPN (RF4364) SAFI > support, > and adds support for IPv6 MPLS VPN (RFC4659), Encap SAFI and related > attribute and types (RFC5512, 5566). These changes are > appropriate for > use of BGPd as a route reflector for these SAFIs. Import/export code > can be built on these core BGP mechanisms. > > The patch set can be viewed in three groups, but has only been fully > tested as a full set. > > Group 1: 01/25 -> 08/25 > This group includes minor/miscellaneous changes made as part > of the work on VPN and Encap SAFIs. > > Group 2: 09/25 -> 20/25 > This is the core VPN and Encap SAFI changes. They also include > configuration handlers, but not full show afi/safi changes. > > Group 3: 21/25 -> 25/25 > 3 files changed, 5728 insertions(+), 1577 deletions(-) > > This patch set adds show commands in the form > show bgp <afi> <safi> ... > > While the motivation for this work was supporting IPv4|6 VPN|Encap > SAFIs, we decided to support a generic afi/safi command form for > existing commands as well for overall command consistency. > Old/existing commands remain available for compatibility. The > patches > are submitted in a way to facilitate removal of the old/non-generic > from of commands if/when the decision is made to remove them. > > G. Paul Ziemba is the primary author of these changes, with me doing > some additional changes. This set replaces the set previously > submitted > by Paul. The patch breakdown is based on (much appreciated) review and > changes done by David Lamparter. > > This patch is available in the patches/master+mplsvpn+encap > branch on https://github.com/LabNConsulting/quagga-vnc . > > Group 1: minor/miscellaneous changes > [PATCH 01/25] lib: write "exit" to config after last route-map > [PATCH 02/25] zebra: wire up "debug zebra packet detail" > [PATCH 03/25] zebra: make RTF_LLINFO optional to fix FreeBSD > [PATCH 04/25] zebra: additional redistribute related logging > [PATCH 05/25] lib: fix bookkeeping for libreadline malloc()s > [PATCH 06/25] lib: add "show commandtree" CLI command > [PATCH 07/25] lib: add facility to log all CLI commands > [PATCH 08/25] lib: add AFI_ETHERNET to lib/prefix code > > lib/command.c | 79 > ++++++++++++++++++++++++++++++++++++++++---------- > lib/command.h | 1 + > lib/prefix.c | 48 ++++++++++++++++++++++++++++++ > lib/prefix.h | 57 ++++++++++++++++++++++++++++++++---- > lib/routemap.c | 2 ++ > lib/vty.c | 41 ++++++++++++++++++++++++-- > lib/zebra.h | 6 ++-- > zebra/debug.c | 5 ++-- > zebra/kernel_socket.c | 2 ++ > zebra/redistribute.c | 18 ++++++++---- > 10 files changed, 224 insertions(+), 35 deletions(-) > > Group 2: core VPN and Encap SAFI changes > [PATCH 09/25] bgpd: add nexthop length to AF macro > [PATCH 10/25] bgpd: kill unused variable in bgp_socket() > [PATCH 11/25] bgpd: make _vpnv4 static handling SAFI-agnostic > [PATCH 12/25] bgpd: handle AS4 and EOI route distinguishers > [PATCH 13/25] bgpd: wire up VPNv6 protocol processing > [PATCH 14/25] *: hook up bgp VPNv6 CLI node > [PATCH 15/25] bgpd: improve cleanup in bgp_delete() > [PATCH 16/25] bgpd: general MP/SAFI improvements > [PATCH 17/25] bgpd: encap: extend extcommunity handling > [PATCH 18/25] bgpd: encap: add attribute handling > [PATCH 19/25] bgpd: encap: add encap SAFI (RFC5512) > [PATCH 20/25] *: hook up bgp ENCAP CLI node > > bgpd/Makefile.am | 6 +- > bgpd/bgp_attr.c | 496 ++++++++++++++++++++++-- > bgpd/bgp_attr.h | 18 + > bgpd/bgp_ecommunity.c | 37 +- > bgpd/bgp_ecommunity.h | 6 +- > bgpd/bgp_encap.c | 991 > ++++++++++++++++++++++++++++++++++++++++++++++++ > bgpd/bgp_encap.h | 34 ++ > bgpd/bgp_encap_tlv.c | 873 > ++++++++++++++++++++++++++++++++++++++++++ > bgpd/bgp_encap_tlv.h | 177 +++++++++ > bgpd/bgp_encap_types.h | 174 +++++++++ > bgpd/bgp_mplsvpn.c | 138 +++++-- > bgpd/bgp_mplsvpn.h | 5 +- > bgpd/bgp_network.c | 2 +- > bgpd/bgp_nexthop.c | 21 + > bgpd/bgp_nexthop.h | 11 + > bgpd/bgp_open.c | 45 ++- > bgpd/bgp_packet.c | 100 ++++- > bgpd/bgp_route.c | 584 ++++++++++++++++++++-------- > bgpd/bgp_route.h | 14 +- > bgpd/bgp_routemap.c | 4 + > bgpd/bgp_vty.c | 582 +++++++++++++++++++++++++++- > bgpd/bgp_vty.h | 6 + > bgpd/bgp_zebra.c | 10 + > bgpd/bgp_zebra.h | 1 + > bgpd/bgpd.c | 72 +++- > bgpd/bgpd.h | 2 + > lib/command.c | 11 +- > lib/command.h | 3 + > lib/memtypes.c | 1 + > lib/vty.c | 3 + > vtysh/extract.pl.in <http://extract.pl.in> | 3 + > vtysh/vtysh.c | 106 +++++- > vtysh/vtysh_config.c | 9 + > 33 files changed, 4284 insertions(+), 261 deletions(-) > > Group 3: cli changes > [PATCH 21/25] CLI: machineparse / random "show" improvements > [PATCH 22/25] CLI: VPNv6 show commands > [PATCH 23/25] CLI: encap show commands > [PATCH 24/25] CLI: Add back old forms of 'show <afi> <safi>' for > [PATCH 25/25] CLI: Add AFI/SAFI show commands to manual > > bgpd/bgp_mplsvpn.c | 408 ++- > bgpd/bgp_route.c | 9734 > ++++++++++++++++++++++++++++++++++----------------- > bgpd/bgp_route.h | 2 +- > bgpd/bgp_vty.c | 789 ++++- > doc/bgpd.texi | 25 + > 5 files changed, 7530 insertions(+), 3428 deletions(-) > > Overall: > bgpd/Makefile.am | 6 +- > bgpd/bgp_attr.c | 496 ++- > bgpd/bgp_attr.h | 18 + > bgpd/bgp_ecommunity.c | 37 +- > bgpd/bgp_ecommunity.h | 6 +- > bgpd/bgp_encap.c | 991 +++++ > bgpd/bgp_encap.h | 34 + > bgpd/bgp_encap_tlv.c | 873 ++++ > bgpd/bgp_encap_tlv.h | 177 + > bgpd/bgp_encap_types.h | 174 + > bgpd/bgp_mplsvpn.c | 544 ++- > bgpd/bgp_mplsvpn.h | 5 +- > bgpd/bgp_network.c | 2 +- > bgpd/bgp_nexthop.c | 21 + > bgpd/bgp_nexthop.h | 11 + > bgpd/bgp_open.c | 45 +- > bgpd/bgp_packet.c | 100 +- > bgpd/bgp_route.c | 10272 > +++++++++++++++++++++++++++++++--------------- > bgpd/bgp_route.h | 16 +- > bgpd/bgp_routemap.c | 4 + > bgpd/bgp_vty.c | 1387 ++++++- > bgpd/bgp_vty.h | 6 + > bgpd/bgp_zebra.c | 10 + > bgpd/bgp_zebra.h | 1 + > bgpd/bgpd.c | 72 +- > bgpd/bgpd.h | 2 + > doc/bgpd.texi | 25 + > lib/command.c | 90 +- > lib/command.h | 4 + > lib/memtypes.c | 1 + > lib/prefix.c | 48 + > lib/prefix.h | 57 +- > lib/routemap.c | 2 + > lib/vty.c | 44 +- > lib/zebra.h | 6 +- > vtysh/extract.pl.in <http://extract.pl.in> | 3 + > vtysh/vtysh.c | 106 +- > vtysh/vtysh_config.c | 9 + > zebra/debug.c | 5 +- > zebra/kernel_socket.c | 2 + > zebra/redistribute.c | 18 +- > 41 files changed, 12022 insertions(+), 3708 deletions(-) > > _______________________________________________ > Quagga-dev mailing list > [email protected] <mailto:[email protected]> > https://lists.quagga.net/mailman/listinfo/quagga-dev > > _______________________________________________ Quagga-dev mailing list [email protected] https://lists.quagga.net/mailman/listinfo/quagga-dev
