From: Theo Buehler <tb@theobuehler.org>
Subject: Re: bgpd: fix F_ATTR_PARSE_ERR handling for ATTR_ORIGIN
To: tech@openbsd.org
Date: Thu, 7 May 2026 11:37:54 +0200

On Thu, May 07, 2026 at 11:29:55AM +0200, Claudio Jeker wrote:
> rde_attr_parse() is a tricky beast.
> 
> If a prefix triggers the F_ATTR_PARSE_ERR case and therefor a RFC 7606
> treat as withdraw the function must return no error. So remove the
> return -1 in the case of ORIGIN attributes > ORIGIN_INCOMPLETE.

Makes sense.

ok

> I dislike how complex this function is when it comes to error handling.
> There are too many rules to remember, some of which are not very obvious.

Indeed.

> -- 
> :wq Claudio
> 
> Index: rde.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
> diff -u -p -r1.695 rde.c
> --- rde.c	28 Apr 2026 14:06:44 -0000	1.695
> +++ rde.c	5 May 2026 19:18:24 -0000
> @@ -2123,6 +2123,7 @@ rde_attr_parse(struct ibuf *buf, struct 
>  			goto bad_list;
>  		if (ibuf_get_n8(&attrbuf, &a->origin) == -1)
>  			goto bad_len;
> +		a->flags |= F_ATTR_ORIGIN;
>  		if (a->origin > ORIGIN_INCOMPLETE) {
>  			/*
>  			 * mark update as bad and withdraw all routes as per
> @@ -2132,9 +2133,7 @@ rde_attr_parse(struct ibuf *buf, struct 
>  			log_peer_warnx(&peer->conf, "bad ORIGIN %u, "
>  			    "path invalidated and prefix withdrawn",
>  			    a->origin);
> -			return (-1);
>  		}
> -		a->flags |= F_ATTR_ORIGIN;
>  		break;
>  	case ATTR_ASPATH:
>  		if (!CHECK_FLAGS(flags, ATTR_WELL_KNOWN, 0))
>