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

Reply via email to