* Claudio Jeker <[EMAIL PROTECTED]> [2008-08-25 17:27]:
> 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.
huh? that is exactly wgat my diff does. it doesn't change the way we
handle safis in updates - which means we might have to ignore unknown
safis there too, didn't check wether we do that already.
>
> > 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
yes, v6 not touched here.
> 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.
yup.
--
Henning Brauer, [EMAIL PROTECTED], [EMAIL PROTECTED]
BS Web Services, http://bsws.de
Full-Service ISP - Secure Hosting, Mail and DNS Services
Dedicated Servers, Rootservers, Application Hosting - Hamburg & Amsterdam
