Re: [PATCH v2] ipv6: add `force_forwarding` sysctl to enable per-interface forwarding

[Gabriel Goller]

On 01.07.2025 17:58, Nicolas Dichtel wrote:
> Le 01/07/2025 à 16:04, Gabriel Goller a écrit :
> > It is currently impossible to enable ipv6 forwarding on a per-interface
> > basis like in ipv4. To enable forwarding on an ipv6 interface we need to
> > enable it on all interfaces and disable it on the other interfaces using
> > a netfilter rule. This is especially cumbersome if you have lots of
> > interface and only want to enable forwarding on a few. According to the
> > sysctl docs [0] the `net.ipv6.conf.all.forwarding` enables forwarding
> > for all interfaces, while the interface-specific
> > `net.ipv6.conf.<interface>.forwarding` configures the interface
> > Host/Router configuration.
> >
> > Introduce a new sysctl flag `force_forwarding`, which can be set on every
> > interface. The ip6_forwarding function will then check if the global
> > forwarding flag OR the force_forwarding flag is active and forward the
> > packet.
> >
> > To preserver backwards-compatibility reset the flag (global and on all
> > interfaces) to 0 if the net.ipv6.conf.all.forwarding flag is set to 0.
> >
> > [0]: https://www.kernel.org/doc/Documentation/networking/ip-sysctl.txt
> >
> > Signed-off-by: Gabriel Goller <g.goller@xxxxxxxxxxx>
> > ---
>
> [snip]
>
> > @@ -896,6 +907,16 @@ static int addrconf_fixup_forwarding(const struct ctl_table *table, int *p, int
> >  						     NETCONFA_IFINDEX_DEFAULT,
> >  						     net->ipv6.devconf_dflt);
> >
> > +		/*
> > +		 * With the introduction of force_forwarding, we need to be backwards
> > +		 * compatible, so that means we need to set the force_forwarding global
> > +		 * flag to 0 if the global forwarding flag is set to 0. Below in
> > +		 * addrconf_forward_change(), we also set the force_forwarding flag on every
> > +		 * interface to 0 if the global forwarding flag is set to 0.
> > +		 */
> > +		if (newf == 0)
> > +			WRITE_ONCE(net->ipv6.devconf_all->force_forwarding, newf);
> Hmm, is this true? Configuring the default value only impacts new interfaces.
> And before your patch, only the 'all' entry is took into account. In other
> words, configuring the default entry today doesn't change the current behavior,
> so I don't see the backward compat point.

Yes, you're right I didn't think this through.
This only affects new interfaces.
I'll remove it.

> > +
> >  		addrconf_forward_change(net, newf);
> >  		if ((!newf) ^ (!old))
> >  			inet6_netconf_notify_devconf(net, RTM_NEWNETCONF,
>
> [snip]
>
> > +static int addrconf_sysctl_force_forwarding(const struct ctl_table *ctl, int write,
> > +					    void *buffer, size_t *lenp, loff_t *ppos)
> > +{
> > +	int *valp = ctl->data;
> > +	int ret;
> > +	int old, new;
> > +
> > +	old = *valp;
> > +	ret = proc_douintvec(ctl, write, buffer, lenp, ppos);
> > +	new = *valp;
> Maybe you can limit values to 0 and 1, like it was done in the v1.

We use the extra1 and extra2 params for the interfaces when setting
interface-specific options, but I can just copy them out and then add
the min/max options to the table like in `addrconf_sysctl_mtu`.

I'll send a patch soon!

> Regards,
> Nicolas

Thanks for the review!