From: Claudio Jeker <cjeker@diehard.n-r-g.com>
Subject: Re: sys/if: egress at non-zero rdomain
To: tech@openbsd.org
Date: Tue, 13 May 2025 06:00:47 +0200

On Mon, May 12, 2025 at 11:35:37PM +0200, Kirill A. Korinsky wrote:
> tech@,
> 
> I'd like to suggest a diff which brings support of egress group for
> interfaces which is in non-zero rdomain.
> 
> Feedback? Ok?

I don't think this is a good idea. egress is used in pf.conf e.g. for
nat-to rules and that will break this usage. Which is one of the main
reasons to have egress in the first place.

Interface groups do not respect rdomain boundaries and I see no good way
how to fix this.

This is why we should only provide egress for rdomain / rtable 0.

> Index: sys/net/if.c
> ===================================================================
> RCS file: /home/cvs/src/sys/net/if.c,v
> diff -u -p -r1.734 if.c
> --- sys/net/if.c	9 May 2025 03:12:36 -0000	1.734
> +++ sys/net/if.c	12 May 2025 19:18:39 -0000
> @@ -3279,6 +3279,7 @@ if_group_routechange(const struct sockad
>  int
>  if_group_egress_build(void)
>  {
> +	u_int			 tid;
>  	struct ifnet		*ifp;
>  	struct ifg_group	*ifg;
>  	struct ifg_member	*ifgm, *next;
> @@ -3296,34 +3297,38 @@ if_group_egress_build(void)
>  		TAILQ_FOREACH_SAFE(ifgm, &ifg->ifg_members, ifgm_next, next)
>  			if_delgroup(ifgm->ifgm_ifp, IFG_EGRESS);
>  
> -	bzero(&sa_in, sizeof(sa_in));
> -	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);
> -	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);
> +	for (tid = 0; tid <= RT_TABLEID_MAX; tid++) {
> +		bzero(&sa_in, sizeof(sa_in));
> +		sa_in.sin_len = sizeof(sa_in);
> +		sa_in.sin_family = AF_INET;
> +		rt = rtable_lookup(tid, 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;
> +			ifp = if_get(rt->rt_ifidx);
> +			if (ifp != NULL) {
> +				if_addgroup(ifp, IFG_EGRESS);
> +				if_put(ifp);
> +			}
>  		}
> -	}
>  
>  #ifdef INET6
> -	bcopy(&sa6_any, &sa_in6, sizeof(sa_in6));
> -	rt = rtable_lookup(0, sin6tosa(&sa_in6), sin6tosa(&sa_in6), NULL,
> -	    RTP_ANY);
> -	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);
> +		bcopy(&sa6_any, &sa_in6, sizeof(sa_in6));
> +		rt = rtable_lookup(tid, sin6tosa(&sa_in6),
> +		    sin6tosa(&sa_in6), NULL, RTP_ANY);
> +		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);
> +			}
>  		}
> -	}
>  #endif /* INET6 */
> +
> +	}
>  
>  	return (0);
>  }
> 
> -- 
> wbr, Kirill
> 

-- 
:wq Claudio