From: Claudio Jeker <cjeker@diehard.n-r-g.com>
Subject: Re: bgpd: include NOTIFICATION data in logging if verbose
To: Theo Buehler <tb@theobuehler.org>
Cc: tech@openbsd.org
Date: Tue, 9 Sep 2025 14:01:36 +0200

On Tue, Sep 09, 2025 at 01:55:59PM +0200, Theo Buehler wrote:
> On Tue, Sep 09, 2025 at 01:43:53PM +0200, Claudio Jeker wrote:
> > Sometimes it is helpful to see a hexdump of the additional notification
> > data sent since it may provide the needed info to better understand why
> > the notification was triggered.
> > 
> > This diff adds this extra logging for UPDATE if verbose is on.
> 
> I think the offset in hexdumps is usually printed as hex, but if
> decimal is what helps you here, sure
 
In my usecase I just had to look at the first 3/4 bytes dumped so I did
not look at the offset at all. I just went for decimal (like gdb :))
I think we can fix this bikeshed in the tree if we want :)

> ok tb
> 
> > -- 
> > :wq Claudio
> > 
> > Index: logmsg.c
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/bgpd/logmsg.c,v
> > diff -u -p -r1.15 logmsg.c
> > --- logmsg.c	21 Aug 2025 15:15:25 -0000	1.15
> > +++ logmsg.c	9 Sep 2025 11:33:36 -0000
> > @@ -128,6 +128,27 @@ log_statechange(struct peer *peer, enum 
> >  	    eventnames[event]);
> >  }
> >  
> > +static const char *
> > +tohex(const unsigned char *in, size_t len)
> > +{
> > +	const char hex[] = "0123456789ABCDEF";
> > +	static char out[(16 + 1) * 3];
> > +	size_t i, o = 0;
> > +
> > +	if (len > 16)
> > +		len = 16;
> > +	for (i = 0; i < len; i++) {
> > +		out[o++] = hex[in[i] >> 4];
> > +		out[o++] = hex[in[i] & 0xf];
> > +		out[o++] = ' ';
> > +		if (i == 7)
> > +			out[o++] = ' ';
> > +	}
> > +	out[o - 1] = '\0';
> > +
> > +	return out;
> > +}
> > +
> >  void
> >  log_notification(const struct peer *peer, uint8_t errcode, uint8_t subcode,
> >      const struct ibuf *data, const char *dir)
> > @@ -135,7 +156,7 @@ log_notification(const struct peer *peer
> >  	struct ibuf	 ibuf;
> >  	char		*p;
> >  	const char	*suberrname = NULL;
> > -	int		 uk = 0;
> > +	int		 uk = 0, dump = 0;
> >  
> >  	if (data != NULL)
> >  		ibuf_from_ibuf(&ibuf, data);
> > @@ -176,6 +197,7 @@ log_notification(const struct peer *peer
> >  			uk = 1;
> >  		else
> >  			suberrname = suberr_update_names[subcode];
> > +		dump = 1;
> >  		break;
> >  	case ERR_CEASE:
> >  		if (subcode >= sizeof(suberr_cease_names) / sizeof(char *) ||
> > @@ -238,6 +260,23 @@ log_notification(const struct peer *peer
> >  			logit(LOG_ERR, "%s: %s notification: %s, %s",
> >  			    p, dir, errnames[errcode], suberrname);
> >  	}
> > +
> > +	if (dump && log_getverbose() && ibuf_size(&ibuf) > 0) {
> > +		size_t off = 0;
> > +		logit(LOG_INFO, "%s: notification data", p);
> > +		while (ibuf_size(&ibuf) > 0) {
> > +			unsigned char buf[16];
> > +			size_t len = sizeof(buf);
> > +			if (ibuf_size(&ibuf) < len)
> > +				len = ibuf_size(&ibuf);
> > +			if (ibuf_get(&ibuf, buf, len) == -1) {
> > +				break;
> > +			}
> > +			logit(LOG_INFO, "   %5zu: %s", off, tohex(buf, len));
> > +			off += len;
> > +		}
> > +	}
> > +
> >  	free(p);
> >  }
> >  
> > 
> 

-- 
:wq Claudio