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 13:17:47 +0200

Download raw body.

Thread
On Thu, May 01, 2025 at 12:16:32PM +0200, Peter Hessler wrote:
> On 2025 May 01 (Thu) at 12:04:00 +0200 (+0200), Claudio Jeker wrote:
> :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.
> : 
> 
> Sure, why not.  as with the previous diff, tested both IPv4 and IPv6.
 
OK claudio@ on this one.
 
> 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 10:07:56 -0000
> @@ -3286,26 +3286,28 @@ if_group_egress_build(void)
>  	sa_in.sin_len = sizeof(sa_in);
>  	sa_in.sin_family = AF_INET;
>  	rt = rtable_lookup(0, sintosa(&sa_in), sintosa(&sa_in), NULL, RTP_ANY);
> -	while (rt != NULL) {
> +	for (; rt != NULL; rt = rtable_iterate(rt)) {
> +		if (ISSET(rt->rt_flags, RTF_REJECT | RTF_BLACKHOLE))
> +			continue;
>  		ifp = if_get(rt->rt_ifidx);
>  		if (ifp != NULL) {
>  			if_addgroup(ifp, IFG_EGRESS);
>  			if_put(ifp);
>  		}
> -		rt = rtable_iterate(rt);
>  	}
>  
>  #ifdef INET6
>  	bcopy(&sa6_any, &sa_in6, sizeof(sa_in6));
>  	rt = rtable_lookup(0, sin6tosa(&sa_in6), sin6tosa(&sa_in6), NULL,
>  	    RTP_ANY);
> -	while (rt != NULL) {
> +	for (; rt != NULL; rt = rtable_iterate(rt)) {
> +		if (ISSET(rt->rt_flags, RTF_REJECT | RTF_BLACKHOLE))
> +			continue;
>  		ifp = if_get(rt->rt_ifidx);
>  		if (ifp != NULL) {
>  			if_addgroup(ifp, IFG_EGRESS);
>  			if_put(ifp);
>  		}
> -		rt = rtable_iterate(rt);
>  	}
>  #endif /* INET6 */
>  
> 
> 
> 
> 
> -- 
> I went into a general store, and they wouldn't sell me anything
> specific.
> 		-- Steven Wright
> 

-- 
:wq Claudio