Index | Thread | Search

From:
Claudio Jeker <cjeker@diehard.n-r-g.com>
Subject:
Re: do not add default routes with blackhole or reject to the egress group
To:
Peter Hessler <phessler@theapt.org>
Cc:
tech@openbsd.org
Date:
Thu, 1 May 2025 12:04:00 +0200

Download raw body.

Thread
On Thu, May 01, 2025 at 11:52:48AM +0200, Peter Hessler wrote:
> The egress group is automatically added to any interface that has a
> default route pointing to it.  However, not all default routes are
> usable.  Both blackhole and reject routes are unreachable, so do not add
> those to the egress group.
> 
> Found by tb@, debugged by myself and claudio@
> 
> OK?
 
It would probably be nicer to write those while loops as for () loops
and then use a simple continue. Like:

	rt = rtable_lookup(0, sintosa(&sa_in), sintosa(&sa_in), NULL, RTP_ANY);
	for (; rt != NULL; rt = rtable_iterate(rt)) {
		if (ISSET(rt->rt_flags, RTF_REJECT | RTF_BLACKHOLE))
			continue;
		...
	}

I did not fold the rtable_lookup() line into the for loop since it is tool
long.
 
> Index: net/if.c
> ===================================================================
> RCS file: /cvs/openbsd/src/sys/net/if.c,v
> diff -u -p -u -p -r1.730 if.c
> --- net/if.c	17 Apr 2025 17:23:17 -0000	1.730
> +++ net/if.c	1 May 2025 09:42:33 -0000
> @@ -3287,10 +3287,12 @@ if_group_egress_build(void)
>  	sa_in.sin_family = AF_INET;
>  	rt = rtable_lookup(0, sintosa(&sa_in), sintosa(&sa_in), NULL, RTP_ANY);
>  	while (rt != NULL) {
> -		ifp = if_get(rt->rt_ifidx);
> -		if (ifp != NULL) {
> -			if_addgroup(ifp, IFG_EGRESS);
> -			if_put(ifp);
> +		if (!ISSET(rt->rt_flags, RTF_REJECT | RTF_BLACKHOLE)) {
> +			ifp = if_get(rt->rt_ifidx);
> +			if (ifp != NULL) {
> +				if_addgroup(ifp, IFG_EGRESS);
> +				if_put(ifp);
> +			}
>  		}
>  		rt = rtable_iterate(rt);
>  	}
> @@ -3300,10 +3302,12 @@ if_group_egress_build(void)
>  	rt = rtable_lookup(0, sin6tosa(&sa_in6), sin6tosa(&sa_in6), NULL,
>  	    RTP_ANY);
>  	while (rt != NULL) {
> -		ifp = if_get(rt->rt_ifidx);
> -		if (ifp != NULL) {
> -			if_addgroup(ifp, IFG_EGRESS);
> -			if_put(ifp);
> +		if (!ISSET(rt->rt_flags, RTF_REJECT | RTF_BLACKHOLE)) {
> +			ifp = if_get(rt->rt_ifidx);
> +			if (ifp != NULL) {
> +				if_addgroup(ifp, IFG_EGRESS);
> +				if_put(ifp);
> +			}
>  		}
>  		rt = rtable_iterate(rt);
>  	}
> 
> 
> 
> -- 
> Hand, n.:
> 	A singular instrument worn at the end of a human arm and
> commonly thrust into somebody's pocket.
> 		-- Ambrose Bierce, "The Devil's Dictionary"
> 

-- 
:wq Claudio