On Fri, Mar 22, 2024 at 02:23:00PM +1000, Theo Buehler wrote:
> On Wed, Mar 20, 2024 at 12:19:02PM +0100, Claudio Jeker wrote:
> > While working on capability enforcement I realized that
> > parse_notification() had the handling of ERR_OPEN / ERR_OPEN_CAPA wrong.
> > 
> > RFC5492 is fairly clear about ERR_OPEN_CAPA (Unsupported Capability, 7):
> >    As explained in the "Overview of Operations" section, the Unsupported
> >    Capability NOTIFICATION is a way for a BGP speaker to complain that
> >    its peer does not support a required capability without which the
> >    peering cannot proceed.  It MUST NOT be used when a BGP speaker
> >    receives a capability that it does not understand; such capabilities
> >    MUST be ignored.
> > 
> > So the logic in parse_notification() is reversed since it tries to turn
> > off the capability that the other side complains about. So remove all this
> > humbug.
> > 
> > Now since we are already in there parse the notification exclusivly with
> > the ibuf api. Clear the last_reason when setting last_rcvd_errcode and
> > last_rcvd_suberr. Parse the last_reason before calling log_notification()
> > do this with ibuf functions which simplifes the code a bit (since the full
> > buffer is memset to 0 the string does not need to be terminated anymore).
> > 
> > In log_notification() add handling for last_reason and also for ERR_OPEN /
> > ERR_OPEN_CAPA. For ERR_OPEN_CAPA only look at the first capa_code and
> > report that. I see little reason to dig deeper there (also it seems that
> > bird and zebra both violate the RFC and send no data with the
> > notification).
> 
> I wasn't aware that zebra was still a thing. Does that include quagga?

I think quagga and frr still probably do the same thing as zebra but I did
not look. I only checked the bird source.

> ok tb

-- 
:wq Claudio