On Tue, Jan 5, 2016 at 5:56 AM, Lou Berger <lber...@labn.net> wrote: > Vivek, > > On 1/5/2016 7:55 AM, Vivek Venkatraman wrote: > > I think it'd be really good if the new functions introduced for VPN > > static routes (which take SAFI as the parameter) subsume the non-VPN > > (i.e., IP unicast) static routes too, so we don't have duplicate code. > > > > The genesis of these changes were the *existing* commands: > bgp_mplsvpn.c:DEFUN (vpnv4_network, > bgp_mplsvpn.c:DEFUN (no_vpnv4_network, > bgp_routemap.c:DEFUN (set_vpnv4_nexthop, > bgp_routemap.c:DEFUN (no_set_vpnv4_nexthop, > bgp_routemap.c:ALIAS (no_set_vpnv4_nexthop, > bgp_vty.c:DEFUN (address_family_vpnv4, > bgp_vty.c:ALIAS (address_family_vpnv4, > > Understood. But I did see a comment in this patch itself about the potential to combine these (i.e., a single bgp_static_set() or bgp_static_update() handles all the different AFI/SAFI) and I was stating my preference for this. Of course, this could be incorporated as a subsequent patch.
> > bgp_static_update_safi() is handling change to attribute (of an > > existing VPN route) but its calling function bgp_static_set_safi() > > right, which replaces the old bgp_static_set_vpnv4 function > > doesn't seem to be; compare with bgp_static_set(). > > > > Can you elaborate on what you mean. Our code was a modification of > bgp_static_set_vpnv4 rather than trying to modify / extend bgp_static_set. > I'll get back on this tomorrow as I'm on a different system right now. But basically, I was saying that while the call chain for IPv4/IPv6 networks (bgp_static_set() and bgp_static_update()) handles changes to attributes, the similar chain for VPNv4 doesn't appear to be. > > > Overall, I'm unsure about the use cases for injecting (or removing) > > VPN static routes (networks) directly. The common thing on a PE would > > be to inject networks into appropriate VRFs and these would then get > > exported by BGP. The original quagga code says these functions are for > > "test purposes". Possibly, there are scenarios where a controller > > would directly inject VPN routes into BGP? > > Not sure, but we were aiming to generalize/fix what was there. I think > it's at least useful for testing and perhaps other uses that may come up > in actual operation. We also weren't comfortable suggesting removing > existing function without a good reason. > Sure. This question was just to know if this interface has other well-known uses. > > Thanks again, > Lou > > > > > On Thu, Dec 24, 2015 at 10:10 AM, Lou Berger <lber...@labn.net > > <mailto:lber...@labn.net>> wrote: > > > > This is part of the core VPN and Encap SAFI changes. > > > > This changes the existing _vpnv4 functions for MPLS-VPN into > > SAFI-agnostic functions, renaming them from *_vpnv4 to *_safi. > > > > Also adds route-map support while at it. > > > > Signed-off-by: Lou Berger <lber...@labn.net <mailto:lber...@labn.net > >> > > Reviewed-by: David Lamparter <equi...@opensourcerouting.org > > <mailto:equi...@opensourcerouting.org>> > > --- > > bgpd/bgp_mplsvpn.c | 20 ++++- > > bgpd/bgp_route.c | 212 > > ++++++++++++++++++++++++++++++++++++++++------------- > > bgpd/bgp_route.h | 6 +- > > 3 files changed, 184 insertions(+), 54 deletions(-) > > > > diff --git a/bgpd/bgp_mplsvpn.c b/bgpd/bgp_mplsvpn.c > > index a72d5ed..18627ad 100644 > > --- a/bgpd/bgp_mplsvpn.c > > +++ b/bgpd/bgp_mplsvpn.c > > @@ -294,7 +294,22 @@ DEFUN (vpnv4_network, > > "BGP tag\n" > > "tag value\n") > > { > > - return bgp_static_set_vpnv4 (vty, argv[0], argv[1], argv[2]); > > + return bgp_static_set_safi (SAFI_MPLS_VPN, vty, argv[0], > > argv[1], argv[2], NULL); > > +} > > + > > +DEFUN (vpnv4_network_route_map, > > + vpnv4_network_route_map_cmd, > > + "network A.B.C.D/M rd ASN:nn_or_IP-address:nn tag WORD > > route-map WORD", > > + "Specify a network to announce via BGP\n" > > + "IP prefix <network>/<length>, e.g., 35.0.0.0/8\n > > <http://35.0.0.0/8%5Cn>" > > + "Specify Route Distinguisher\n" > > + "VPN Route Distinguisher\n" > > + "BGP tag\n" > > + "tag value\n" > > + "route map\n" > > + "route map name\n") > > +{ > > + return bgp_static_set_safi (SAFI_MPLS_VPN, vty, argv[0], > > argv[1], argv[2], argv[3]); > > } > > > > /* For testing purpose, static route of MPLS-VPN. */ > > @@ -309,7 +324,7 @@ DEFUN (no_vpnv4_network, > > "BGP tag\n" > > "tag value\n") > > { > > - return bgp_static_unset_vpnv4 (vty, argv[0], argv[1], argv[2]); > > + return bgp_static_unset_safi (SAFI_MPLS_VPN, vty, argv[0], > > argv[1], argv[2]); > > } > > > > static int > > @@ -722,6 +737,7 @@ void > > bgp_mplsvpn_init (void) > > { > > install_element (BGP_VPNV4_NODE, &vpnv4_network_cmd); > > + install_element (BGP_VPNV4_NODE, &vpnv4_network_route_map_cmd); > > install_element (BGP_VPNV4_NODE, &no_vpnv4_network_cmd); > > > > > > diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c > > index 316fa5a..9866e37 100644 > > --- a/bgpd/bgp_route.c > > +++ b/bgpd/bgp_route.c > > @@ -3614,39 +3614,6 @@ bgp_static_update (struct bgp *bgp, struct > > prefix *p, > > } > > } > > > > -static void > > -bgp_static_update_vpnv4 (struct bgp *bgp, struct prefix *p, afi_t > > afi, > > - safi_t safi, struct prefix_rd *prd, > > u_char *tag) > > -{ > > - struct bgp_node *rn; > > - struct bgp_info *new; > > - > > - rn = bgp_afi_node_get (bgp->rib[afi][safi], afi, safi, p, prd); > > - > > - /* Make new BGP info. */ > > - new = bgp_info_new (); > > - new->type = ZEBRA_ROUTE_BGP; > > - new->sub_type = BGP_ROUTE_STATIC; > > - new->peer = bgp->peer_self; > > - new->attr = bgp_attr_default_intern (BGP_ORIGIN_IGP); > > - SET_FLAG (new->flags, BGP_INFO_VALID); > > - new->uptime = bgp_clock (); > > - new->extra = bgp_info_extra_new(); > > - memcpy (new->extra->tag, tag, 3); > > - > > - /* Aggregate address increment. */ > > - bgp_aggregate_increment (bgp, p, new, afi, safi); > > - > > - /* Register new BGP information. */ > > - bgp_info_add (rn, new); > > - > > - /* route_node_get lock */ > > - bgp_unlock_node (rn); > > - > > - /* Process change. */ > > - bgp_process (bgp, rn, afi, safi); > > -} > > - > > void > > bgp_static_withdraw (struct bgp *bgp, struct prefix *p, afi_t afi, > > safi_t safi) > > @@ -3695,9 +3662,12 @@ bgp_check_local_routes_rsclient (struct > > peer *rsclient, afi_t afi, safi_t safi) > > } > > } > > > > +/* > > + * Used for SAFI_MPLS_VPN and SAFI_ENCAP > > + */ > > static void > > -bgp_static_withdraw_vpnv4 (struct bgp *bgp, struct prefix *p, > > afi_t afi, > > - safi_t safi, struct prefix_rd *prd, > > u_char *tag) > > +bgp_static_withdraw_safi (struct bgp *bgp, struct prefix *p, > > afi_t afi, > > + safi_t safi, struct prefix_rd *prd, > > u_char *tag) > > { > > struct bgp_node *rn; > > struct bgp_info *ri; > > @@ -3723,6 +3693,131 @@ bgp_static_withdraw_vpnv4 (struct bgp > > *bgp, struct prefix *p, afi_t afi, > > bgp_unlock_node (rn); > > } > > > > +static void > > +bgp_static_update_safi (struct bgp *bgp, struct prefix *p, > > + struct bgp_static *bgp_static, afi_t afi, > > safi_t safi) > > +{ > > + struct bgp_node *rn; > > + struct bgp_info *new; > > + struct attr *attr_new; > > + struct attr attr = { 0 }; > > + struct bgp_info *ri; > > + > > + assert (bgp_static); > > + > > + rn = bgp_afi_node_get (bgp->rib[afi][safi], afi, safi, p, > > &bgp_static->prd); > > + > > + bgp_attr_default_set (&attr, BGP_ORIGIN_IGP); > > + > > + attr.nexthop = bgp_static->igpnexthop; > > + attr.med = bgp_static->igpmetric; > > + attr.flag |= ATTR_FLAG_BIT (BGP_ATTR_MULTI_EXIT_DISC); > > + > > + /* Apply route-map. */ > > + if (bgp_static->rmap.name <http://rmap.name>) > > + { > > + struct attr attr_tmp = attr; > > + struct bgp_info info; > > + int ret; > > + > > + info.peer = bgp->peer_self; > > + info.attr = &attr_tmp; > > + > > + SET_FLAG (bgp->peer_self->rmap_type, PEER_RMAP_TYPE_NETWORK); > > + > > + ret = route_map_apply (bgp_static->rmap.map, p, RMAP_BGP, > > &info); > > + > > + bgp->peer_self->rmap_type = 0; > > + > > + if (ret == RMAP_DENYMATCH) > > + { > > + /* Free uninterned attribute. */ > > + bgp_attr_flush (&attr_tmp); > > + > > + /* Unintern original. */ > > + aspath_unintern (&attr.aspath); > > + bgp_attr_extra_free (&attr); > > + bgp_static_withdraw_safi (bgp, p, afi, safi, > > &bgp_static->prd, > > + bgp_static->tag); > > + return; > > + } > > + > > + attr_new = bgp_attr_intern (&attr_tmp); > > + } > > + else > > + { > > + attr_new = bgp_attr_intern (&attr); > > + } > > + > > + for (ri = rn->info; ri; ri = ri->next) > > + if (ri->peer == bgp->peer_self && ri->type == ZEBRA_ROUTE_BGP > > + && ri->sub_type == BGP_ROUTE_STATIC) > > + break; > > + > > + if (ri) > > + { > > + if (attrhash_cmp (ri->attr, attr_new) && > > + !CHECK_FLAG(ri->flags, BGP_INFO_REMOVED)) > > + { > > + bgp_unlock_node (rn); > > + bgp_attr_unintern (&attr_new); > > + aspath_unintern (&attr.aspath); > > + bgp_attr_extra_free (&attr); > > + return; > > + } > > + else > > + { > > + /* The attribute is changed. */ > > + bgp_info_set_flag (rn, ri, BGP_INFO_ATTR_CHANGED); > > + > > + /* Rewrite BGP route information. */ > > + if (CHECK_FLAG(ri->flags, BGP_INFO_REMOVED)) > > + bgp_info_restore(rn, ri); > > + else > > + bgp_aggregate_decrement (bgp, p, ri, afi, safi); > > + bgp_attr_unintern (&ri->attr); > > + ri->attr = attr_new; > > + ri->uptime = bgp_clock (); > > + > > + /* Process change. */ > > + bgp_aggregate_increment (bgp, p, ri, afi, safi); > > + bgp_process (bgp, rn, afi, safi); > > + bgp_unlock_node (rn); > > + aspath_unintern (&attr.aspath); > > + bgp_attr_extra_free (&attr); > > + return; > > + } > > + } > > + > > + > > + /* Make new BGP info. */ > > + new = bgp_info_new (); > > + new->type = ZEBRA_ROUTE_BGP; > > + new->sub_type = BGP_ROUTE_STATIC; > > + new->peer = bgp->peer_self; > > + new->attr = attr_new; > > + SET_FLAG (new->flags, BGP_INFO_VALID); > > + new->uptime = bgp_clock (); > > + new->extra = bgp_info_extra_new(); > > + memcpy (new->extra->tag, bgp_static->tag, 3); > > + > > + /* Aggregate address increment. */ > > + bgp_aggregate_increment (bgp, p, new, afi, safi); > > + > > + /* Register new BGP information. */ > > + bgp_info_add (rn, new); > > + > > + /* route_node_get lock */ > > + bgp_unlock_node (rn); > > + > > + /* Process change. */ > > + bgp_process (bgp, rn, afi, safi); > > + > > + /* Unintern original. */ > > + aspath_unintern (&attr.aspath); > > + bgp_attr_extra_free (&attr); > > +} > > + > > /* Configure static BGP network. When user don't run zebra, static > > route should be installed as valid. */ > > static int > > @@ -3893,8 +3988,8 @@ bgp_static_delete (struct bgp *bgp) > > for (rm = bgp_table_top (table); rm; rm = > > bgp_route_next (rm)) > > { > > bgp_static = rn->info; > > - bgp_static_withdraw_vpnv4 (bgp, &rm->p, > > - AFI_IP, SAFI_MPLS_VPN, > > + bgp_static_withdraw_safi (bgp, &rm->p, > > + AFI_IP, safi, > > (struct prefix_rd > > *)&rn->p, > > bgp_static->tag); > > bgp_static_free (bgp_static); > > @@ -3913,9 +4008,15 @@ bgp_static_delete (struct bgp *bgp) > > } > > } > > > > +/* > > + * gpz 110624 > > + * Currently this is used to set static routes for VPN and ENCAP. > > + * I think it can probably be factored with bgp_static_set. > > + */ > > int > > -bgp_static_set_vpnv4 (struct vty *vty, const char *ip_str, const > > char *rd_str, > > - const char *tag_str) > > +bgp_static_set_safi (safi_t safi, struct vty *vty, const char > > *ip_str, > > + const char *rd_str, const char *tag_str, > > + const char *rmap_str) > > { > > int ret; > > struct prefix p; > > @@ -3951,10 +4052,10 @@ bgp_static_set_vpnv4 (struct vty *vty, > > const char *ip_str, const char *rd_str, > > return CMD_WARNING; > > } > > > > - prn = bgp_node_get (bgp->route[AFI_IP][SAFI_MPLS_VPN], > > + prn = bgp_node_get (bgp->route[AFI_IP][safi], > > (struct prefix *)&prd); > > if (prn->info == NULL) > > - prn->info = bgp_table_init (AFI_IP, SAFI_MPLS_VPN); > > + prn->info = bgp_table_init (AFI_IP, safi); > > else > > bgp_unlock_node (prn); > > table = prn->info; > > @@ -3970,11 +4071,24 @@ bgp_static_set_vpnv4 (struct vty *vty, > > const char *ip_str, const char *rd_str, > > { > > /* New configuration. */ > > bgp_static = bgp_static_new (); > > - bgp_static->valid = 1; > > - memcpy (bgp_static->tag, tag, 3); > > + bgp_static->backdoor = 0; > > + bgp_static->valid = 0; > > + bgp_static->igpmetric = 0; > > + bgp_static->igpnexthop.s_addr = 0; > > + memcpy(bgp_static->tag, tag, 3); > > + bgp_static->prd = prd; > > + > > + if (rmap_str) > > + { > > + if (bgp_static->rmap.name <http://rmap.name>) > > + free (bgp_static->rmap.name <http://rmap.name>); > > + bgp_static->rmap.name <http://rmap.name> = strdup > > (rmap_str); > > + bgp_static->rmap.map = route_map_lookup_by_name (rmap_str); > > + } > > rn->info = bgp_static; > > > > - bgp_static_update_vpnv4 (bgp, &p, AFI_IP, SAFI_MPLS_VPN, > > &prd, tag); > > + bgp_static->valid = 1; > > + bgp_static_update_safi (bgp, &p, bgp_static, AFI_IP, safi); > > } > > > > return CMD_SUCCESS; > > @@ -3982,8 +4096,8 @@ bgp_static_set_vpnv4 (struct vty *vty, const > > char *ip_str, const char *rd_str, > > > > /* Configure static BGP network. */ > > int > > -bgp_static_unset_vpnv4 (struct vty *vty, const char *ip_str, > > - const char *rd_str, const char *tag_str) > > +bgp_static_unset_safi(safi_t safi, struct vty *vty, const char > > *ip_str, > > + const char *rd_str, const char *tag_str) > > { > > int ret; > > struct bgp *bgp; > > @@ -4020,10 +4134,10 @@ bgp_static_unset_vpnv4 (struct vty *vty, > > const char *ip_str, > > return CMD_WARNING; > > } > > > > - prn = bgp_node_get (bgp->route[AFI_IP][SAFI_MPLS_VPN], > > + prn = bgp_node_get (bgp->route[AFI_IP][safi], > > (struct prefix *)&prd); > > if (prn->info == NULL) > > - prn->info = bgp_table_init (AFI_IP, SAFI_MPLS_VPN); > > + prn->info = bgp_table_init (AFI_IP, safi); > > else > > bgp_unlock_node (prn); > > table = prn->info; > > @@ -4032,7 +4146,7 @@ bgp_static_unset_vpnv4 (struct vty *vty, > > const char *ip_str, > > > > if (rn) > > { > > - bgp_static_withdraw_vpnv4 (bgp, &p, AFI_IP, SAFI_MPLS_VPN, > > &prd, tag); > > + bgp_static_withdraw_safi (bgp, &p, AFI_IP, safi, &prd, tag); > > > > bgp_static = rn->info; > > bgp_static_free (bgp_static); > > diff --git a/bgpd/bgp_route.h b/bgpd/bgp_route.h > > index 3d2eea5..3e7f6f5 100644 > > --- a/bgpd/bgp_route.h > > +++ b/bgpd/bgp_route.h > > @@ -209,10 +209,10 @@ extern void bgp_static_update (struct bgp *, > > struct prefix *, struct bgp_static > > afi_t, safi_t); > > extern void bgp_static_withdraw (struct bgp *, struct prefix *, > > afi_t, safi_t); > > > > -extern int bgp_static_set_vpnv4 (struct vty *vty, const char *, > > - const char *, const char *); > > +extern int bgp_static_set_safi (safi_t safi, struct vty *vty, > > const char *, > > + const char *, const char *, const char *); > > > > -extern int bgp_static_unset_vpnv4 (struct vty *, const char *, > > +extern int bgp_static_unset_safi (safi_t safi, struct vty *, > > const char *, > > const char *, const char *); > > > > /* this is primarily for MPLS-VPN */ > > -- > > 2.1.3 > > > > > > _______________________________________________ > > Quagga-dev mailing list > > Quagga-dev@lists.quagga.net <mailto:Quagga-dev@lists.quagga.net> > > https://lists.quagga.net/mailman/listinfo/quagga-dev > > > > > > >
_______________________________________________ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
