Index | Thread | Search

From:
Claudio Jeker <cjeker@diehard.n-r-g.com>
Subject:
Re: bgpd: use unsigned int instead of uint8_t as loop iterator var
To:
Theo Buehler <tb@theobuehler.org>
Cc:
tech@openbsd.org
Date:
Fri, 8 May 2026 13:11:27 +0200

Download raw body.

Thread
On Fri, May 08, 2026 at 08:35:59AM +0200, Theo Buehler wrote:
> On Fri, May 08, 2026 at 06:59:56AM +0200, Claudio Jeker wrote:
> > Yes, most certainly. Time to grep for AID_MAX.
> 
> That results in this diff. The only slight complication is in the second
> and third hunk of parse.y where I had to split the uses of aid. Where I
> changed the type, I checked that the uses of aid and i are only as loop
> indexing variables.

OK claudio@. I wonder if afi2aid should use u_int instead but that is a
followup diff.

We want the uint8_t in some structs for AID to preserve space but apart
from that I think u_int makes more sense.
 
> Index: parse.y
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/parse.y,v
> diff -u -p -r1.491 parse.y
> --- parse.y	8 May 2026 05:27:25 -0000	1.491
> +++ parse.y	8 May 2026 06:25:28 -0000
> @@ -1720,7 +1720,7 @@ neighbor	: { curpeer = new_peer(); }
>  			}
>  		}
>  		    peeropts_h {
> -			uint8_t		aid;
> +			u_int		aid;
>  
>  			if (curpeer_filter[0] != NULL)
>  				TAILQ_INSERT_TAIL(peerfilter_l,
> @@ -1945,10 +1945,11 @@ peeropts	: REMOTEAS as4number	{
>  			curpeer->conf.staletime = $2;
>  		}
>  		| ANNOUNCE af safi enforce {
> -			uint8_t		aid, safi;
> -			uint16_t	afi;
> -
>  			if ($3 == SAFI_NONE) {
> +				u_int		aid;
> +				uint8_t		safi;
> +				uint16_t	afi;
> +
>  				for (aid = AID_MIN; aid < AID_MAX; aid++) {
>  					if (aid2afi(aid, &afi, &safi) == -1 ||
>  					    afi != $2)
> @@ -1956,6 +1957,8 @@ peeropts	: REMOTEAS as4number	{
>  					curpeer->conf.capabilities.mp[aid] = -1;
>  				}
>  			} else {
> +				uint8_t aid;
> +
>  				if (afi2aid($2, $3, &aid) == -1) {
>  					yyerror("unknown AFI/SAFI pair");
>  					YYERROR;
> Index: printconf.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/printconf.c,v
> diff -u -p -r1.185 printconf.c
> --- printconf.c	2 Mar 2026 09:51:48 -0000	1.185
> +++ printconf.c	8 May 2026 06:25:28 -0000
> @@ -918,7 +918,7 @@ print_addpath_mode(enum addpath_mode mod
>  void
>  print_announce(struct peer_config *p, const char *c)
>  {
> -	uint8_t	aid;
> +	u_int	aid;
>  	int match = 0;
>  
>  	for (aid = AID_MIN; aid < AID_MAX; aid++)
> Index: rde.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
> diff -u -p -r1.696 rde.c
> --- rde.c	7 May 2026 09:42:26 -0000	1.696
> +++ rde.c	8 May 2026 06:25:28 -0000
> @@ -168,7 +168,7 @@ rde_main(int debug, int verbose)
>  	void			*newp;
>  	u_int			 pfd_elms = 0, i, j;
>  	int			 timeout;
> -	uint8_t			 aid;
> +	u_int			 aid;
>  
>  	log_init(debug, LOG_DAEMON);
>  	log_setverbose(verbose);
> @@ -468,7 +468,7 @@ rde_dispatch_imsg_session(struct imsgbuf
>  	uint32_t		 peerid;
>  	pid_t			 pid;
>  	int			 verbose;
> -	uint8_t			 aid;
> +	u_int			 aid;
>  
>  	while (imsgbuf) {
>  		if ((n = imsg_get(imsgbuf, &imsg)) == -1)
> @@ -3636,7 +3636,7 @@ int
>  rde_update_queue_pending(void)
>  {
>  	struct rde_peer *peer;
> -	uint8_t aid;
> +	u_int aid;
>  
>  	if (ibuf_se && imsgbuf_queuelen(ibuf_se) >= SESS_MSG_HIGH_MARK)
>  		return 0;
> @@ -4209,7 +4209,7 @@ rde_softreconfig_in_done(void *arg, uint
>  	}
>  
>  	RB_FOREACH(peer, peer_tree, &peertable) {
> -		uint8_t aid;
> +		u_int aid;
>  
>  		if (peer->reconf_rib) {
>  			/* dump the full table to neighbors that changed rib */
> Index: rde_adjout.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/rde_adjout.c,v
> diff -u -p -r1.17 rde_adjout.c
> --- rde_adjout.c	7 May 2026 12:33:12 -0000	1.17
> +++ rde_adjout.c	8 May 2026 06:25:28 -0000
> @@ -861,7 +861,7 @@ adjout_peer_flush_pending(struct rde_pee
>  {
>  	struct pend_attr *pa, *npa;
>  	struct pend_prefix *pp, *npp;
> -	uint8_t aid;
> +	u_int aid;
>  
>  	for (aid = AID_MIN; aid < AID_MAX; aid++) {
>  		TAILQ_FOREACH_SAFE(pp, &peer->withdraws[aid], entry, npp) {
> Index: rde_peer.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/rde_peer.c,v
> diff -u -p -r1.71 rde_peer.c
> --- rde_peer.c	7 May 2026 09:19:48 -0000	1.71
> +++ rde_peer.c	8 May 2026 06:25:28 -0000
> @@ -428,7 +428,7 @@ peer_flush_upcall(struct rib_entry *re, 
>  void
>  peer_up(struct rde_peer *peer, struct session_up *sup)
>  {
> -	uint8_t	 i;
> +	u_int	 i;
>  	int force_sync = 1;
>  
>  	if (peer->state == PEER_ERR) {
> @@ -568,7 +568,7 @@ peer_flush(struct rde_peer *peer, uint8_
>  
>  	/* every route is gone so reset staletime */
>  	if (aid == AID_UNSPEC) {
> -		uint8_t i;
> +		u_int i;
>  		for (i = AID_MIN; i < AID_MAX; i++)
>  			peer->staletime[i] = monotime_clear();
>  	} else {
> Index: session.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/session.c,v
> diff -u -p -r1.533 session.c
> --- session.c	7 May 2026 17:59:15 -0000	1.533
> +++ session.c	8 May 2026 06:25:28 -0000
> @@ -1047,7 +1047,7 @@ session_handle_rrefresh(struct peer *pee
>  void
>  session_graceful_restart(struct peer *p)
>  {
> -	uint8_t	i;
> +	u_int i;
>  	u_int staletime = conf->staletime;
>  
>  	if (p->conf.staletime)
> @@ -1087,7 +1087,7 @@ session_graceful_restart(struct peer *p)
>  void
>  session_graceful_stop(struct peer *p)
>  {
> -	uint8_t	i;
> +	u_int	i;
>  
>  	for (i = AID_MIN; i < AID_MAX; i++) {
>  		/*
> Index: session_bgp.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/session_bgp.c,v
> diff -u -p -r1.8 session_bgp.c
> --- session_bgp.c	7 May 2026 12:20:42 -0000	1.8
> +++ session_bgp.c	8 May 2026 06:25:28 -0000
> @@ -182,7 +182,7 @@ session_open(struct peer *p)
>  {
>  	struct ibuf		*buf, *opb;
>  	size_t			 len, optparamlen;
> -	uint8_t			 i;
> +	u_int			 i;
>  	int			 errs = 0, extlen = 0;
>  	int			 mpcapa = 0;
>  
> @@ -519,7 +519,7 @@ session_notification(struct peer *p, uin
>  int
>  session_neighbor_rrefresh(struct peer *p)
>  {
> -	uint8_t	i;
> +	u_int	i;
>  
>  	if (!(p->capa.neg.refresh || p->capa.neg.enhanced_rr))
>  		return (-1);
> @@ -1280,7 +1280,8 @@ static int
>  capa_neg_calc(struct peer *p)
>  {
>  	struct ibuf *ebuf;
> -	uint8_t	i, hasmp = 0, capa_code, capa_len, capa_aid = 0;
> +	u_int	i;
> +	uint8_t	hasmp = 0, capa_code, capa_len, capa_aid = 0;
>  
>  	/* a capability is accepted only if both sides announced it */
>  
> Index: util.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/util.c,v
> diff -u -p -r1.100 util.c
> --- util.c	5 May 2026 09:12:04 -0000	1.100
> +++ util.c	8 May 2026 06:25:28 -0000
> @@ -1140,7 +1140,7 @@ aid2afi(uint8_t aid, uint16_t *afi, uint
>  int
>  afi2aid(uint16_t afi, uint8_t safi, uint8_t *aid)
>  {
> -	uint8_t i;
> +	u_int i;
>  
>  	for (i = AID_MIN; i < AID_MAX; i++)
>  		if (aid_vals[i].afi == afi && aid_vals[i].safi == safi) {
> @@ -1162,7 +1162,7 @@ aid2af(uint8_t aid)
>  int
>  af2aid(sa_family_t af, uint8_t safi, uint8_t *aid)
>  {
> -	uint8_t i;
> +	u_int i;
>  
>  	if (safi == 0) /* default to unicast subclass */
>  		safi = SAFI_UNICAST;

-- 
:wq Claudio