On Thu, Dec 24, 2015 at 10:10 AM, Lou Berger <[email protected]> wrote:
> This is part of the core VPN and Encap SAFI changes. > > This fixes some minor mixups particularly in MPLS-related SAFIs, as well > as doing some stylistic changes & adding comments. > I feel it would be good to separate the very specific fixes (related to SAFI values) from the other changes - just an opinion. > > Signed-off-by: Lou Berger <[email protected]> > Reviewed-by: David Lamparter <[email protected]> > --- > @@ -2159,8 +2159,9 @@ bgp_packet_mpattr_start (struct stream *s, afi_t > afi, safi_t safi, > stream_putc (s, BGP_ATTR_MP_REACH_NLRI); > sizep = stream_get_endp (s); > stream_putw (s, 0); /* Marker: Attribute length. */ > - stream_putw (s, afi); /* AFI */ > - stream_putc (s, safi); /* SAFI */ > + > + stream_putw (s, afi); > + stream_putc (s, (safi == SAFI_MPLS_VPN) ? SAFI_MPLS_LABELED_VPN : safi); > > /* Nexthop */ > switch (afi) > @@ -2168,14 +2169,16 @@ bgp_packet_mpattr_start (struct stream *s, afi_t > afi, safi_t safi, > case AFI_IP: > switch (safi) > { > +#if 0 /* LB: doesn't exist */ > case SAFI_UNICAST: > +#endif > Just as a FYI, we have changed this part of the code as part of implementing support for RFC 5549, where IPv4 prefixes can be exchanged as MP NLRI with IPv6 nexthops. One can be reworked on top of the other. > case SAFI_MULTICAST: > stream_putc (s, 4); > stream_put_ipv4 (s, attr->nexthop.s_addr); > break; > > > @@ -2254,7 +2284,6 @@ bgp_packet_attribute (struct bgp *bgp, struct peer > *peer, > int send_as4_path = 0; > int send_as4_aggregator = 0; > int use32bit = (CHECK_FLAG (peer->cap, PEER_CAP_AS4_RCV)) ? 1 : 0; > - size_t mpattrlen_pos = 0; > > if (! bgp) > bgp = bgp_get_default (); > @@ -2262,13 +2291,15 @@ bgp_packet_attribute (struct bgp *bgp, struct peer > *peer, > /* Remember current pointer. */ > cp = stream_get_endp (s); > > +#if 0 /* duplicates mpattr included in > bgp_packet.c */ > if (p && !(afi == AFI_IP && safi == SAFI_UNICAST)) > { > + size_t mpattrlen_pos = 0; > mpattrlen_pos = bgp_packet_mpattr_start(s, afi, safi, attr); > bgp_packet_mpattr_prefix(s, afi, safi, p, prd, tag); > bgp_packet_mpattr_end(s, mpattrlen_pos); > } > - > +#endif > This part of the code (which has been placed under "#if 0") is used by the default originate code. I'm not sure how that will work after this change. Can you please check? > > diff --git a/bgpd/bgp_packet.c b/bgpd/bgp_packet.c > index 35c4719..9d88e11 100644 > --- a/bgpd/bgp_packet.c > +++ b/bgpd/bgp_packet.c > @@ -171,16 +171,24 @@ bgp_update_packet (struct peer *peer, afi_t afi, > safi_t safi) > > /* When remaining space can't include NLRI and it's length. */ > if (STREAM_CONCAT_REMAIN (s, snlri, STREAM_SIZE(s)) <= > - (BGP_NLRI_LENGTH + PSIZE (rn->p.prefixlen))) > + (BGP_NLRI_LENGTH + > bgp_packet_mpattr_prefix_size(afi,safi,&rn->p))) > break; > > /* If packet is empty, set attribute. */ > if (stream_empty (s)) > { > + struct prefix_rd *prd = NULL; > + u_char *tag = NULL; > struct peer *from = NULL; > > + if (rn->prn) > + prd = (struct prefix_rd *) &rn->prn->p; > if (binfo) > - from = binfo->peer; > + { > + from = binfo->peer; > + if (binfo->extra) > + tag = binfo->extra->tag; > + } > > /* 1: Write the BGP message header - 16 bytes marker, 2 bytes > length, > * one byte message type. > @@ -203,8 +211,8 @@ bgp_update_packet (struct peer *peer, afi_t afi, > safi_t safi) > /* 5: Encode all the attributes, except MP_REACH_NLRI attr. */ > total_attr_len = bgp_packet_attribute (NULL, peer, s, > adv->baa->attr, > - NULL, afi, safi, > - from, NULL, NULL); > + &rn->p, afi, safi, > + from, prd, tag); > } > > I don't see the need to pass the prefix in the above call; in fact, since bgp_packet_attribute() is meant to set the non-prefix attributes of the update, I feel the prefix shouldn't be passed. The only case it is needed is in default originate (default_update_send) as I mentioned earlier --- and ideally, that function should be reworked to put the prefix itself.
_______________________________________________ Quagga-dev mailing list [email protected] https://lists.quagga.net/mailman/listinfo/quagga-dev
