On Mon, Aug 25, 2008 at 03:54:27PM +0200, Henning Brauer wrote: > * Graeme Lee <[EMAIL PROTECTED]> [2008-08-25 03:28]: > > Yes but the safi's are handled during capability negotiation (in function > > parse_capabilities in session.c) > > Do I need to do more than just ignore the unknown safi's? Currently, the > > return (-1) in the mp_safi test never allows the connection to establish. > > Removing this at least allows the bgp session to function, but I'm not sure > > if that's all that's needed, or even if it's safe to do so. > > > I don't remember exactly what the RFCs demanded. IThere is one for > capabilties negotiation and one for the multiprotocol extensions. I > guess the latter is the relevant one. if you could check what it says > about the unknown safi case and it allows us to ingore them I am very > willing to make that change :) >
RFC 2858 Section 7: A speaker that supports multiple <AFI, SAFI> tuples includes them as multiple Capabilities in the Capabilities Optional Parameter. To have a bi-directional exchange of routing information for a particular <AFI, SAFI> between a pair of BGP speakers, each such speaker must advertise to the other (via the Capability Advertisement mechanism) the capability to support that particular <AFI, SAFI> routes. I would say that unknown safi should be accepted in the capabilities but not during a bgp update. That would mean that your diff is not correct. > Index: session.c > =================================================================== > RCS file: /cvs/src/usr.sbin/bgpd/session.c,v > retrieving revision 1.282 > diff -u -p -r1.282 session.c > --- session.c 26 Jun 2008 00:01:51 -0000 1.282 > +++ session.c 25 Aug 2008 13:54:06 -0000 > @@ -2193,13 +2193,12 @@ parse_capabilities(struct peer *peer, u_ > memcpy(&mp_safi, capa_val + 3, sizeof(mp_safi)); > switch (mp_afi) { > case AFI_IPv4: > - if (mp_safi < 1 || mp_safi > 3) { > + if (mp_safi < 1 || mp_safi > 3) > log_peer_warnx(&peer->conf, > "parse_capabilities: AFI IPv4, " > - "mp_safi %u illegal", mp_safi); > - return (-1); > - } > - peer->capa.peer.mp_v4 = mp_safi; > + "mp_safi %u unknown", mp_safi); > + else > + peer->capa.peer.mp_v4 = mp_safi; > break; > case AFI_IPv6: > if (mp_safi < 1 || mp_safi > 3) { > I guess a similar hack should be added to AFI_IPv6. In the end we may need to accept any AFI/SAFI pair and just report them in show neighbor. The if (mp_safi < 1 || mp_safi > 3) will be invalid as soon as we support something like MPLS VPNs. -- :wq Claudio