[PATCH net-next v4] ipv6: add `force_forwarding` sysctl to enable per-interface forwarding

Gabriel Goller posted 1 patch 3 months ago
There is a newer version of this series
Documentation/networking/ip-sysctl.rst        |   5 +
include/linux/ipv6.h                          |   1 +
include/uapi/linux/ipv6.h                     |   1 +
include/uapi/linux/netconf.h                  |   1 +
include/uapi/linux/sysctl.h                   |   1 +
net/ipv6/addrconf.c                           |  91 +++++++++++++++
net/ipv6/ip6_output.c                         |   3 +-
tools/testing/selftests/net/Makefile          |   1 +
.../selftests/net/ipv6_force_forwarding.sh    | 105 ++++++++++++++++++
9 files changed, 208 insertions(+), 1 deletion(-)
create mode 100644 tools/testing/selftests/net/ipv6_force_forwarding.sh
[PATCH net-next v4] ipv6: add `force_forwarding` sysctl to enable per-interface forwarding
Posted by Gabriel Goller 3 months ago
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 (on all interfaces)
to 0 if the net.ipv6.conf.all.forwarding flag is set to 0.

Add a short selftest that checks if a packet gets forwarded with and
without `force_forwarding`.

[0]: https://www.kernel.org/doc/Documentation/networking/ip-sysctl.txt

Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
---

First time writing a selftest, so LMK if I did something wrong or if
there is something I can improve. Thanks!

v4:
    * actually write the sysctl value to the table
    * use ASSERT_RTNL() when forwarding the sysctl change
    * remove useless comments in function body
    * simplify forwarding and force_forwarding check in ip6_output.c
    * fix code backticks in Documentation (double instead of single)
    * add selftests
v3: https://lore.kernel.org/netdev/20250702074619.139031-1-g.goller@proxmox.com/
    * remove forwarding=0 setting force_forwarding=0 globally.
    * add min and max (0 and 1) value to sysctl.
v2: https://lore.kernel.org/netdev/20250701140423.487411-1-g.goller@proxmox.com/
    * rename from `do_forwarding` to `force_forwarding`.
    * add global `force_forwarding` flag which will enable
      `force_forwarding` on every interface like the
      `ipv4.all.forwarding` flag.
    * `forwarding`=0 will disable global and per-interface
      `force_forwarding`.
    * export option as NETCONFA_FORCE_FORWARDING.
v1: https://lore.kernel.org/netdev/20250702074619.139031-1-g.goller@proxmox.com/

 Documentation/networking/ip-sysctl.rst        |   5 +
 include/linux/ipv6.h                          |   1 +
 include/uapi/linux/ipv6.h                     |   1 +
 include/uapi/linux/netconf.h                  |   1 +
 include/uapi/linux/sysctl.h                   |   1 +
 net/ipv6/addrconf.c                           |  91 +++++++++++++++
 net/ipv6/ip6_output.c                         |   3 +-
 tools/testing/selftests/net/Makefile          |   1 +
 .../selftests/net/ipv6_force_forwarding.sh    | 105 ++++++++++++++++++
 9 files changed, 208 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/net/ipv6_force_forwarding.sh

diff --git a/Documentation/networking/ip-sysctl.rst b/Documentation/networking/ip-sysctl.rst
index 0f1251cce314..ec7fa1e890f1 100644
--- a/Documentation/networking/ip-sysctl.rst
+++ b/Documentation/networking/ip-sysctl.rst
@@ -2292,6 +2292,11 @@ conf/all/forwarding - BOOLEAN
 proxy_ndp - BOOLEAN
 	Do proxy ndp.
 
+force_forwarding - BOOLEAN
+	Enable forwarding on this interface only -- regardless of the setting on
+	``conf/all/forwarding``. When setting ``conf.all.forwarding`` to 0,
+	the ``force_forwarding`` flag will be reset on all interfaces.
+
 fwmark_reflect - BOOLEAN
 	Controls the fwmark of kernel-generated IPv6 reply packets that are not
 	associated with a socket for example, TCP RSTs or ICMPv6 echo replies).
diff --git a/include/linux/ipv6.h b/include/linux/ipv6.h
index 5aeeed22f35b..5380107e466c 100644
--- a/include/linux/ipv6.h
+++ b/include/linux/ipv6.h
@@ -19,6 +19,7 @@ struct ipv6_devconf {
 	__s32		forwarding;
 	__s32		disable_policy;
 	__s32		proxy_ndp;
+	__s32		force_forwarding;
 	__cacheline_group_end(ipv6_devconf_read_txrx);
 
 	__s32		accept_ra;
diff --git a/include/uapi/linux/ipv6.h b/include/uapi/linux/ipv6.h
index cf592d7b630f..d4d3ae774b26 100644
--- a/include/uapi/linux/ipv6.h
+++ b/include/uapi/linux/ipv6.h
@@ -199,6 +199,7 @@ enum {
 	DEVCONF_NDISC_EVICT_NOCARRIER,
 	DEVCONF_ACCEPT_UNTRACKED_NA,
 	DEVCONF_ACCEPT_RA_MIN_LFT,
+	DEVCONF_FORCE_FORWARDING,
 	DEVCONF_MAX
 };
 
diff --git a/include/uapi/linux/netconf.h b/include/uapi/linux/netconf.h
index fac4edd55379..1c8c84d65ae3 100644
--- a/include/uapi/linux/netconf.h
+++ b/include/uapi/linux/netconf.h
@@ -19,6 +19,7 @@ enum {
 	NETCONFA_IGNORE_ROUTES_WITH_LINKDOWN,
 	NETCONFA_INPUT,
 	NETCONFA_BC_FORWARDING,
+	NETCONFA_FORCE_FORWARDING,
 	__NETCONFA_MAX
 };
 #define NETCONFA_MAX	(__NETCONFA_MAX - 1)
diff --git a/include/uapi/linux/sysctl.h b/include/uapi/linux/sysctl.h
index 8981f00204db..63d1464cb71c 100644
--- a/include/uapi/linux/sysctl.h
+++ b/include/uapi/linux/sysctl.h
@@ -573,6 +573,7 @@ enum {
 	NET_IPV6_ACCEPT_RA_FROM_LOCAL=26,
 	NET_IPV6_ACCEPT_RA_RT_INFO_MIN_PLEN=27,
 	NET_IPV6_RA_DEFRTR_METRIC=28,
+	NET_IPV6_FORCE_FORWARDING=29,
 	__NET_IPV6_MAX
 };
 
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index ba2ec7c870cc..dcf4e8bf8cf8 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -239,6 +239,7 @@ static struct ipv6_devconf ipv6_devconf __read_mostly = {
 	.ndisc_evict_nocarrier	= 1,
 	.ra_honor_pio_life	= 0,
 	.ra_honor_pio_pflag	= 0,
+	.force_forwarding	= 0,
 };
 
 static struct ipv6_devconf ipv6_devconf_dflt __read_mostly = {
@@ -303,6 +304,7 @@ static struct ipv6_devconf ipv6_devconf_dflt __read_mostly = {
 	.ndisc_evict_nocarrier	= 1,
 	.ra_honor_pio_life	= 0,
 	.ra_honor_pio_pflag	= 0,
+	.force_forwarding	= 0,
 };
 
 /* Check if link is ready: is it up and is a valid qdisc available */
@@ -857,6 +859,15 @@ static void addrconf_forward_change(struct net *net, __s32 newf)
 		idev = __in6_dev_get_rtnl_net(dev);
 		if (idev) {
 			int changed = (!idev->cnf.forwarding) ^ (!newf);
+			/*
+			 * With the introduction of force_forwarding, we need to be backwards
+			 * compatible, so that means we need to set the force_forwarding flag
+			 * on every interface to 0 if net.ipv6.conf.all.forwarding is set to 0.
+			 * This allows the global forwarding flag to disable forwarding for
+			 * all interfaces.
+			 */
+			if (newf == 0)
+				WRITE_ONCE(idev->cnf.force_forwarding, newf);
 
 			WRITE_ONCE(idev->cnf.forwarding, newf);
 			if (changed)
@@ -5719,6 +5730,7 @@ static void ipv6_store_devconf(const struct ipv6_devconf *cnf,
 	array[DEVCONF_ACCEPT_UNTRACKED_NA] =
 		READ_ONCE(cnf->accept_untracked_na);
 	array[DEVCONF_ACCEPT_RA_MIN_LFT] = READ_ONCE(cnf->accept_ra_min_lft);
+	array[DEVCONF_FORCE_FORWARDING] = READ_ONCE(cnf->force_forwarding);
 }
 
 static inline size_t inet6_ifla6_size(void)
@@ -6747,6 +6759,78 @@ static int addrconf_sysctl_disable_policy(const struct ctl_table *ctl, int write
 	return ret;
 }
 
+static void addrconf_force_forward_change(struct net *net, __s32 newf)
+{
+	ASSERT_RTNL();
+	struct net_device *dev;
+	struct inet6_dev *idev;
+
+	for_each_netdev(net, dev) {
+		idev = __in6_dev_get_rtnl_net(dev);
+		if (idev) {
+			int changed = (!idev->cnf.force_forwarding) ^ (!newf);
+
+			WRITE_ONCE(idev->cnf.force_forwarding, newf);
+			if (changed) {
+				inet6_netconf_notify_devconf(dev_net(dev), RTM_NEWNETCONF,
+							     NETCONFA_FORCE_FORWARDING,
+							     dev->ifindex, &idev->cnf);
+			}
+		}
+	}
+}
+
+static int addrconf_sysctl_force_forwarding(const struct ctl_table *ctl, int write,
+					    void *buffer, size_t *lenp, loff_t *ppos)
+{
+	struct inet6_dev *idev = ctl->extra1;
+	struct net *net = ctl->extra2;
+	int *valp = ctl->data;
+	loff_t pos = *ppos;
+	int new_val = *valp;
+	int old_val = *valp;
+	int ret;
+
+	struct ctl_table tmp_ctl = *ctl;
+
+	tmp_ctl.extra1 = SYSCTL_ZERO;
+	tmp_ctl.extra2 = SYSCTL_ONE;
+	tmp_ctl.data = &new_val;
+
+	ret = proc_douintvec_minmax(&tmp_ctl, write, buffer, lenp, ppos);
+
+	if (write && old_val != new_val) {
+		if (!rtnl_net_trylock(net))
+			return restart_syscall();
+
+		if (valp == &net->ipv6.devconf_dflt->force_forwarding) {
+			inet6_netconf_notify_devconf(net, RTM_NEWNETCONF,
+						     NETCONFA_FORCE_FORWARDING,
+						     NETCONFA_IFINDEX_DEFAULT,
+						     net->ipv6.devconf_dflt);
+		} else if (valp == &net->ipv6.devconf_all->force_forwarding) {
+			inet6_netconf_notify_devconf(net, RTM_NEWNETCONF,
+						     NETCONFA_FORCE_FORWARDING,
+						     NETCONFA_IFINDEX_ALL,
+						     net->ipv6.devconf_all);
+
+			addrconf_force_forward_change(net, new_val);
+		} else {
+			inet6_netconf_notify_devconf(net, RTM_NEWNETCONF,
+						     NETCONFA_FORCE_FORWARDING,
+						     idev->dev->ifindex,
+						     &idev->cnf);
+		}
+		rtnl_net_unlock(net);
+	}
+
+	if (write)
+		WRITE_ONCE(*valp, new_val);
+	if (ret)
+		*ppos = pos;
+	return ret;
+}
+
 static int minus_one = -1;
 static const int two_five_five = 255;
 static u32 ioam6_if_id_max = U16_MAX;
@@ -7217,6 +7301,13 @@ static const struct ctl_table addrconf_sysctl[] = {
 		.extra1		= SYSCTL_ZERO,
 		.extra2		= SYSCTL_TWO,
 	},
+	{
+		.procname	= "force_forwarding",
+		.data		= &ipv6_devconf.force_forwarding,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= addrconf_sysctl_force_forwarding,
+	},
 };
 
 static int __addrconf_sysctl_register(struct net *net, char *dev_name,
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index 7bd29a9ff0db..440b9efced72 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -509,7 +509,8 @@ int ip6_forward(struct sk_buff *skb)
 	u32 mtu;
 
 	idev = __in6_dev_get_safely(dev_get_by_index_rcu(net, IP6CB(skb)->iif));
-	if (READ_ONCE(net->ipv6.devconf_all->forwarding) == 0)
+	if (idev && !READ_ONCE(idev->cnf.force_forwarding) &&
+	    !READ_ONCE(net->ipv6.devconf_all->forwarding))
 		goto error;
 
 	if (skb->pkt_type != PACKET_HOST)
diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
index 332f387615d7..f64ec8a15a77 100644
--- a/tools/testing/selftests/net/Makefile
+++ b/tools/testing/selftests/net/Makefile
@@ -112,6 +112,7 @@ TEST_PROGS += skf_net_off.sh
 TEST_GEN_FILES += skf_net_off
 TEST_GEN_FILES += tfo
 TEST_PROGS += tfo_passive.sh
+TEST_PROGS += ipv6_force_forwarding.sh
 
 # YNL files, must be before "include ..lib.mk"
 YNL_GEN_FILES := busy_poller netlink-dumps
diff --git a/tools/testing/selftests/net/ipv6_force_forwarding.sh b/tools/testing/selftests/net/ipv6_force_forwarding.sh
new file mode 100644
index 000000000000..62adc9d4afc9
--- /dev/null
+++ b/tools/testing/selftests/net/ipv6_force_forwarding.sh
@@ -0,0 +1,105 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+#
+# Test IPv6 force_forwarding interface property
+#
+# This test verifies that the force_forwarding property works correctly:
+# - When global forwarding is disabled, packets are not forwarded normally
+# - When force_forwarding is enabled on an interface, packets are forwarded
+#   regardless of the global forwarding setting
+
+source lib.sh
+
+cleanup() {
+    cleanup_ns $ns1 $ns2 $ns3
+}
+
+trap cleanup EXIT
+
+setup_test() {
+    # Create three namespaces: sender, router, receiver
+    setup_ns ns1 ns2 ns3
+
+    # Create veth pairs: ns1 <-> ns2 <-> ns3
+    ip link add name veth12 type veth peer name veth21
+    ip link add name veth23 type veth peer name veth32
+
+    # Move interfaces to namespaces
+    ip link set veth12 netns $ns1
+    ip link set veth21 netns $ns2
+    ip link set veth23 netns $ns2
+    ip link set veth32 netns $ns3
+
+    # Configure interfaces
+    ip -n $ns1 addr add 2001:db8:1::1/64 dev veth12
+    ip -n $ns2 addr add 2001:db8:1::2/64 dev veth21
+    ip -n $ns2 addr add 2001:db8:2::1/64 dev veth23
+    ip -n $ns3 addr add 2001:db8:2::2/64 dev veth32
+
+    # Bring up interfaces
+    ip -n $ns1 link set veth12 up
+    ip -n $ns2 link set veth21 up
+    ip -n $ns2 link set veth23 up
+    ip -n $ns3 link set veth32 up
+
+    # Add routes
+    ip -n $ns1 route add 2001:db8:2::/64 via 2001:db8:1::2
+    ip -n $ns3 route add 2001:db8:1::/64 via 2001:db8:2::1
+
+    # Disable global forwarding
+    ip netns exec $ns2 sysctl -qw net.ipv6.conf.all.forwarding=0
+}
+
+test_force_forwarding() {
+    local ret=0
+
+    echo "TEST: force_forwarding functionality"
+
+    # Check if force_forwarding sysctl exists
+    if ! ip netns exec $ns2 test -f /proc/sys/net/ipv6/conf/veth21/force_forwarding; then
+        echo "SKIP: force_forwarding not available"
+        return $ksft_skip
+    fi
+
+    # Test 1: Without force_forwarding, ping should fail
+    ip netns exec $ns2 sysctl -qw net.ipv6.conf.veth21.force_forwarding=0
+    ip netns exec $ns2 sysctl -qw net.ipv6.conf.veth23.force_forwarding=0
+
+    if ip netns exec $ns1 ping -6 -c 1 -W 2 2001:db8:2::2 &>/dev/null; then
+        echo "FAIL: ping succeeded when forwarding disabled"
+        ret=1
+    else
+        echo "PASS: forwarding disabled correctly"
+    fi
+
+    # Test 2: With force_forwarding enabled, ping should succeed
+    ip netns exec $ns2 sysctl -qw net.ipv6.conf.veth21.force_forwarding=1
+    ip netns exec $ns2 sysctl -qw net.ipv6.conf.veth23.force_forwarding=1
+
+    if ip netns exec $ns1 ping -6 -c 1 -W 2 2001:db8:2::2 &>/dev/null; then
+        echo "PASS: force_forwarding enabled forwarding"
+    else
+        echo "FAIL: ping failed with force_forwarding enabled"
+        ret=1
+    fi
+
+    return $ret
+}
+
+echo "IPv6 force_forwarding test"
+echo "=========================="
+
+setup_test
+test_force_forwarding
+ret=$?
+
+if [ $ret -eq 0 ]; then
+    echo "OK"
+    exit 0
+elif [ $ret -eq $ksft_skip ]; then
+    echo "SKIP"
+    exit $ksft_skip
+else
+    echo "FAIL"
+    exit 1
+fi
-- 
2.39.5
Re: [PATCH net-next v4] ipv6: add `force_forwarding` sysctl to enable per-interface forwarding
Posted by Nicolas Dichtel 3 months ago
Le 03/07/2025 à 18:01, 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 (on all interfaces)
> to 0 if the net.ipv6.conf.all.forwarding flag is set to 0.
> 
> Add a short selftest that checks if a packet gets forwarded with and
> without `force_forwarding`.
> 
> [0]: https://www.kernel.org/doc/Documentation/networking/ip-sysctl.txt
> 
> Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
> ---
> 

[snip]

> @@ -6747,6 +6759,78 @@ static int addrconf_sysctl_disable_policy(const struct ctl_table *ctl, int write
>  	return ret;
>  }
>  
> +static void addrconf_force_forward_change(struct net *net, __s32 newf)
> +{
> +	ASSERT_RTNL();
> +	struct net_device *dev;
> +	struct inet6_dev *idev;
> +

ASSERT_RTNL() is always put after variables declaration.


> +	for_each_netdev(net, dev) {
> +		idev = __in6_dev_get_rtnl_net(dev);
> +		if (idev) {
> +			int changed = (!idev->cnf.force_forwarding) ^ (!newf);
> +
> +			WRITE_ONCE(idev->cnf.force_forwarding, newf);
> +			if (changed) {
> +				inet6_netconf_notify_devconf(dev_net(dev), RTM_NEWNETCONF,
> +							     NETCONFA_FORCE_FORWARDING,
> +							     dev->ifindex, &idev->cnf);
> +			}
> +		}
> +	}
> +}
> +
> +static int addrconf_sysctl_force_forwarding(const struct ctl_table *ctl, int write,
> +					    void *buffer, size_t *lenp, loff_t *ppos)
> +{
> +	struct inet6_dev *idev = ctl->extra1;
> +	struct net *net = ctl->extra2;
> +	int *valp = ctl->data;
> +	loff_t pos = *ppos;
> +	int new_val = *valp;
> +	int old_val = *valp;
> +	int ret;
> +
> +	struct ctl_table tmp_ctl = *ctl;
This declaration should be put with other declarations.

> +
> +	tmp_ctl.extra1 = SYSCTL_ZERO;
> +	tmp_ctl.extra2 = SYSCTL_ONE;
> +	tmp_ctl.data = &new_val;
> +
> +	ret = proc_douintvec_minmax(&tmp_ctl, write, buffer, lenp, ppos);
> +
> +	if (write && old_val != new_val) {
> +		if (!rtnl_net_trylock(net))
> +			return restart_syscall();
> +
> +		if (valp == &net->ipv6.devconf_dflt->force_forwarding) {
> +			inet6_netconf_notify_devconf(net, RTM_NEWNETCONF,
> +						     NETCONFA_FORCE_FORWARDING,
> +						     NETCONFA_IFINDEX_DEFAULT,
> +						     net->ipv6.devconf_dflt);
> +		} else if (valp == &net->ipv6.devconf_all->force_forwarding) {
> +			inet6_netconf_notify_devconf(net, RTM_NEWNETCONF,
> +						     NETCONFA_FORCE_FORWARDING,
> +						     NETCONFA_IFINDEX_ALL,
> +						     net->ipv6.devconf_all);
> +
> +			addrconf_force_forward_change(net, new_val);
> +		} else {
> +			inet6_netconf_notify_devconf(net, RTM_NEWNETCONF,
> +						     NETCONFA_FORCE_FORWARDING,
> +						     idev->dev->ifindex,
> +						     &idev->cnf);
> +		}
> +		rtnl_net_unlock(net);
> +	}
> +
> +	if (write)
> +		WRITE_ONCE(*valp, new_val);
Why not putting this in the above block?
And maybe under the rtnl_lock to avoid race if two users change the value at the
same time.

Nicolas
Re: [PATCH net-next v4] ipv6: add `force_forwarding` sysctl to enable per-interface forwarding
Posted by Gabriel Goller 3 months ago
On 04.07.2025 10:07, Nicolas Dichtel wrote:
>Le 03/07/2025 à 18:01, 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 (on all interfaces)
>> to 0 if the net.ipv6.conf.all.forwarding flag is set to 0.
>>
>> Add a short selftest that checks if a packet gets forwarded with and
>> without `force_forwarding`.
>>
>> [0]: https://www.kernel.org/doc/Documentation/networking/ip-sysctl.txt
>>
>> Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
>> ---
>>
>
>[snip]
>
>> @@ -6747,6 +6759,78 @@ static int addrconf_sysctl_disable_policy(const struct ctl_table *ctl, int write
>>  	return ret;
>>  }
>>
>> +static void addrconf_force_forward_change(struct net *net, __s32 newf)
>> +{
>> +	ASSERT_RTNL();
>> +	struct net_device *dev;
>> +	struct inet6_dev *idev;
>> +
>
>ASSERT_RTNL() is always put after variables declaration.

I removed ASSERT_RTNL completely, this is already checked by __in6_dev_get_rtnl_net.

>> +	for_each_netdev(net, dev) {
>> +		idev = __in6_dev_get_rtnl_net(dev);
>> +		if (idev) {
>> +			int changed = (!idev->cnf.force_forwarding) ^ (!newf);
>> +
>> +			WRITE_ONCE(idev->cnf.force_forwarding, newf);
>> +			if (changed) {
>> +				inet6_netconf_notify_devconf(dev_net(dev), RTM_NEWNETCONF,
>> +							     NETCONFA_FORCE_FORWARDING,
>> +							     dev->ifindex, &idev->cnf);
>> +			}
>> +		}
>> +	}
>> +}
>> +
>> +static int addrconf_sysctl_force_forwarding(const struct ctl_table *ctl, int write,
>> +					    void *buffer, size_t *lenp, loff_t *ppos)
>> +{
>> +	struct inet6_dev *idev = ctl->extra1;
>> +	struct net *net = ctl->extra2;
>> +	int *valp = ctl->data;
>> +	loff_t pos = *ppos;
>> +	int new_val = *valp;
>> +	int old_val = *valp;
>> +	int ret;
>> +
>> +	struct ctl_table tmp_ctl = *ctl;
>This declaration should be put with other declarations.

Agree.

>> +
>> +	tmp_ctl.extra1 = SYSCTL_ZERO;
>> +	tmp_ctl.extra2 = SYSCTL_ONE;
>> +	tmp_ctl.data = &new_val;
>> +
>> +	ret = proc_douintvec_minmax(&tmp_ctl, write, buffer, lenp, ppos);
>> +
>> +	if (write && old_val != new_val) {
>> +		if (!rtnl_net_trylock(net))
>> +			return restart_syscall();
>> +
>> +		if (valp == &net->ipv6.devconf_dflt->force_forwarding) {
>> +			inet6_netconf_notify_devconf(net, RTM_NEWNETCONF,
>> +						     NETCONFA_FORCE_FORWARDING,
>> +						     NETCONFA_IFINDEX_DEFAULT,
>> +						     net->ipv6.devconf_dflt);
>> +		} else if (valp == &net->ipv6.devconf_all->force_forwarding) {
>> +			inet6_netconf_notify_devconf(net, RTM_NEWNETCONF,
>> +						     NETCONFA_FORCE_FORWARDING,
>> +						     NETCONFA_IFINDEX_ALL,
>> +						     net->ipv6.devconf_all);
>> +
>> +			addrconf_force_forward_change(net, new_val);
>> +		} else {
>> +			inet6_netconf_notify_devconf(net, RTM_NEWNETCONF,
>> +						     NETCONFA_FORCE_FORWARDING,
>> +						     idev->dev->ifindex,
>> +						     &idev->cnf);
>> +		}
>> +		rtnl_net_unlock(net);
>> +	}
>> +
>> +	if (write)
>> +		WRITE_ONCE(*valp, new_val);
>Why not putting this in the above block?
>And maybe under the rtnl_lock to avoid race if two users change the value at the
>same time.

Yep, you're right.

>Nicolas

Thanks for the review!

Re: [PATCH net-next v4] ipv6: add `force_forwarding` sysctl to enable per-interface forwarding
Posted by Kuniyuki Iwashima 3 months ago
From: Gabriel Goller <g.goller@proxmox.com>
Date: Thu,  3 Jul 2025 18:01:53 +0200
> diff --git a/Documentation/networking/ip-sysctl.rst b/Documentation/networking/ip-sysctl.rst
> index 0f1251cce314..ec7fa1e890f1 100644
> --- a/Documentation/networking/ip-sysctl.rst
> +++ b/Documentation/networking/ip-sysctl.rst
> @@ -2292,6 +2292,11 @@ conf/all/forwarding - BOOLEAN
>  proxy_ndp - BOOLEAN
>  	Do proxy ndp.
>  
> +force_forwarding - BOOLEAN
> +	Enable forwarding on this interface only -- regardless of the setting on
> +	``conf/all/forwarding``. When setting ``conf.all.forwarding`` to 0,
> +	the ``force_forwarding`` flag will be reset on all interfaces.

Please update conf/all/forwarding too as it will be stale after
this patch.


> +
>  fwmark_reflect - BOOLEAN
>  	Controls the fwmark of kernel-generated IPv6 reply packets that are not
>  	associated with a socket for example, TCP RSTs or ICMPv6 echo replies).
> diff --git a/include/linux/ipv6.h b/include/linux/ipv6.h
> index 5aeeed22f35b..5380107e466c 100644
> --- a/include/linux/ipv6.h
> +++ b/include/linux/ipv6.h
> @@ -19,6 +19,7 @@ struct ipv6_devconf {
>  	__s32		forwarding;
>  	__s32		disable_policy;
>  	__s32		proxy_ndp;
> +	__s32		force_forwarding;

nit: place force_forwarding just after forwarding.


>  	__cacheline_group_end(ipv6_devconf_read_txrx);
>  
>  	__s32		accept_ra;
> @@ -857,6 +859,15 @@ static void addrconf_forward_change(struct net *net, __s32 newf)
>  		idev = __in6_dev_get_rtnl_net(dev);
>  		if (idev) {
>  			int changed = (!idev->cnf.forwarding) ^ (!newf);
> +			/*
> +			 * With the introduction of force_forwarding, we need to be backwards
> +			 * compatible, so that means we need to set the force_forwarding flag

Strictly backward compatibility is not related I think because
the per iface conf is disabled by default, and this is a new
behaviour.  Maybe simply say

/* Disabling all.forwarding sets 0 to force_forwarding for all interfaces */

> +			 * on every interface to 0 if net.ipv6.conf.all.forwarding is set to 0.
> +			 * This allows the global forwarding flag to disable forwarding for
> +			 * all interfaces.
> +			 */
> +			if (newf == 0)
> +				WRITE_ONCE(idev->cnf.force_forwarding, newf);
>  
>  			WRITE_ONCE(idev->cnf.forwarding, newf);
>  			if (changed)
> @@ -5719,6 +5730,7 @@ static void ipv6_store_devconf(const struct ipv6_devconf *cnf,
>  	array[DEVCONF_ACCEPT_UNTRACKED_NA] =
>  		READ_ONCE(cnf->accept_untracked_na);
>  	array[DEVCONF_ACCEPT_RA_MIN_LFT] = READ_ONCE(cnf->accept_ra_min_lft);
> +	array[DEVCONF_FORCE_FORWARDING] = READ_ONCE(cnf->force_forwarding);
>  }
>  
[...]
>  static inline size_t inet6_ifla6_size(void)
> @@ -6747,6 +6759,78 @@ static int addrconf_sysctl_disable_policy(const struct ctl_table *ctl, int write
>  	return ret;
>  }
>  
> +static void addrconf_force_forward_change(struct net *net, __s32 newf)
> +{
> +	ASSERT_RTNL();

__in6_dev_get_rtnl_net() has the same check so this is not needed.


> +	struct net_device *dev;
> +	struct inet6_dev *idev;
> +
> +	for_each_netdev(net, dev) {
> +		idev = __in6_dev_get_rtnl_net(dev);
> +		if (idev) {
> +			int changed = (!idev->cnf.force_forwarding) ^ (!newf);
> +
> +			WRITE_ONCE(idev->cnf.force_forwarding, newf);
> +			if (changed) {
> +				inet6_netconf_notify_devconf(dev_net(dev), RTM_NEWNETCONF,
> +							     NETCONFA_FORCE_FORWARDING,
> +							     dev->ifindex, &idev->cnf);
> +			}
> +		}
> +	}
> +}
> +
> +static int addrconf_sysctl_force_forwarding(const struct ctl_table *ctl, int write,
> +					    void *buffer, size_t *lenp, loff_t *ppos)
> +{
> +	struct inet6_dev *idev = ctl->extra1;
> +	struct net *net = ctl->extra2;
> +	int *valp = ctl->data;
> +	loff_t pos = *ppos;

nit: Please keep the reverse xmas tree order.
https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html#local-variable-ordering-reverse-xmas-tree-rcs

> +	int new_val = *valp;
> +	int old_val = *valp;
> +	int ret;
> +
> +	struct ctl_table tmp_ctl = *ctl;

same here.

> +
> +	tmp_ctl.extra1 = SYSCTL_ZERO;
> +	tmp_ctl.extra2 = SYSCTL_ONE;

As you are copying *ctl, please specify this in addrconf_sysctl[].


> +	tmp_ctl.data = &new_val;
> +
> +	ret = proc_douintvec_minmax(&tmp_ctl, write, buffer, lenp, ppos);
> +
> +	if (write && old_val != new_val) {
> +		if (!rtnl_net_trylock(net))
> +			return restart_syscall();
> +
> +		if (valp == &net->ipv6.devconf_dflt->force_forwarding) {
> +			inet6_netconf_notify_devconf(net, RTM_NEWNETCONF,
> +						     NETCONFA_FORCE_FORWARDING,
> +						     NETCONFA_IFINDEX_DEFAULT,
> +						     net->ipv6.devconf_dflt);
> +		} else if (valp == &net->ipv6.devconf_all->force_forwarding) {
> +			inet6_netconf_notify_devconf(net, RTM_NEWNETCONF,
> +						     NETCONFA_FORCE_FORWARDING,
> +						     NETCONFA_IFINDEX_ALL,
> +						     net->ipv6.devconf_all);
> +
> +			addrconf_force_forward_change(net, new_val);
> +		} else {
> +			inet6_netconf_notify_devconf(net, RTM_NEWNETCONF,
> +						     NETCONFA_FORCE_FORWARDING,
> +						     idev->dev->ifindex,
> +						     &idev->cnf);
> +		}
> +		rtnl_net_unlock(net);
> +	}
> +
> +	if (write)
> +		WRITE_ONCE(*valp, new_val);
> +	if (ret)
> +		*ppos = pos;
> +	return ret;
> +}
> +
>  static int minus_one = -1;
>  static const int two_five_five = 255;
>  static u32 ioam6_if_id_max = U16_MAX;
> @@ -7217,6 +7301,13 @@ static const struct ctl_table addrconf_sysctl[] = {
>  		.extra1		= SYSCTL_ZERO,
>  		.extra2		= SYSCTL_TWO,
>  	},
> +	{
> +		.procname	= "force_forwarding",
> +		.data		= &ipv6_devconf.force_forwarding,
> +		.maxlen		= sizeof(int),
> +		.mode		= 0644,
> +		.proc_handler	= addrconf_sysctl_force_forwarding,

Here for extra{1,2}.


> +	},
>  };
>  
>  static int __addrconf_sysctl_register(struct net *net, char *dev_name,
> diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
> index 7bd29a9ff0db..440b9efced72 100644
> --- a/net/ipv6/ip6_output.c
> +++ b/net/ipv6/ip6_output.c
> @@ -509,7 +509,8 @@ int ip6_forward(struct sk_buff *skb)
>  	u32 mtu;
>  
>  	idev = __in6_dev_get_safely(dev_get_by_index_rcu(net, IP6CB(skb)->iif));
> -	if (READ_ONCE(net->ipv6.devconf_all->forwarding) == 0)
> +	if (idev && !READ_ONCE(idev->cnf.force_forwarding) &&
> +	    !READ_ONCE(net->ipv6.devconf_all->forwarding))

Now this ignores devconf_all when !idev whose dev was not
found or has not had a valid mtu.

if (!READ_ONCE(net->ipv6.devconf_all->forwarding) &&
    (!idev || !READ_ONCE(idev->cnf.force_forwarding)))
Re: [PATCH net-next v4] ipv6: add `force_forwarding` sysctl to enable per-interface forwarding
Posted by Gabriel Goller 3 months ago
On 04.07.2025 08:00, Kuniyuki Iwashima wrote:
>From: Gabriel Goller <g.goller@proxmox.com>
>Date: Thu,  3 Jul 2025 18:01:53 +0200
>> diff --git a/Documentation/networking/ip-sysctl.rst b/Documentation/networking/ip-sysctl.rst
>> index 0f1251cce314..ec7fa1e890f1 100644
>> --- a/Documentation/networking/ip-sysctl.rst
>> +++ b/Documentation/networking/ip-sysctl.rst
>> @@ -2292,6 +2292,11 @@ conf/all/forwarding - BOOLEAN
>>  proxy_ndp - BOOLEAN
>>  	Do proxy ndp.
>>
>> +force_forwarding - BOOLEAN
>> +	Enable forwarding on this interface only -- regardless of the setting on
>> +	``conf/all/forwarding``. When setting ``conf.all.forwarding`` to 0,
>> +	the ``force_forwarding`` flag will be reset on all interfaces.
>
>Please update conf/all/forwarding too as it will be stale after
>this patch.

I'll change the conf/all/forwarding section from:

	IPv4 and IPv6 work differently here; e.g. netfilter must be used
	to control which interfaces may forward packets and which not.

to:

	IPv4 and IPv6 work differently here; the ``force_forwarding`` flag must
	be used to control which interfaces may forward packets.

I hope that's all right.

>> +
>>  fwmark_reflect - BOOLEAN
>>  	Controls the fwmark of kernel-generated IPv6 reply packets that are not
>>  	associated with a socket for example, TCP RSTs or ICMPv6 echo replies).
>> diff --git a/include/linux/ipv6.h b/include/linux/ipv6.h
>> index 5aeeed22f35b..5380107e466c 100644
>> --- a/include/linux/ipv6.h
>> +++ b/include/linux/ipv6.h
>> @@ -19,6 +19,7 @@ struct ipv6_devconf {
>>  	__s32		forwarding;
>>  	__s32		disable_policy;
>>  	__s32		proxy_ndp;
>> +	__s32		force_forwarding;
>
>nit: place force_forwarding just after forwarding.

Agree.

>>  	__cacheline_group_end(ipv6_devconf_read_txrx);
>>
>>  	__s32		accept_ra;
>> @@ -857,6 +859,15 @@ static void addrconf_forward_change(struct net *net, __s32 newf)
>>  		idev = __in6_dev_get_rtnl_net(dev);
>>  		if (idev) {
>>  			int changed = (!idev->cnf.forwarding) ^ (!newf);
>> +			/*
>> +			 * With the introduction of force_forwarding, we need to be backwards
>> +			 * compatible, so that means we need to set the force_forwarding flag
>
>Strictly backward compatibility is not related I think because
>the per iface conf is disabled by default, and this is a new
>behaviour.  Maybe simply say
>
>/* Disabling all.forwarding sets 0 to force_forwarding for all interfaces */

Agree.

>> +			 * on every interface to 0 if net.ipv6.conf.all.forwarding is set to 0.
>> +			 * This allows the global forwarding flag to disable forwarding for
>> +			 * all interfaces.
>> +			 */
>> +			if (newf == 0)
>> +				WRITE_ONCE(idev->cnf.force_forwarding, newf);
>>
>>  			WRITE_ONCE(idev->cnf.forwarding, newf);
>>  			if (changed)
>> @@ -5719,6 +5730,7 @@ static void ipv6_store_devconf(const struct ipv6_devconf *cnf,
>>  	array[DEVCONF_ACCEPT_UNTRACKED_NA] =
>>  		READ_ONCE(cnf->accept_untracked_na);
>>  	array[DEVCONF_ACCEPT_RA_MIN_LFT] = READ_ONCE(cnf->accept_ra_min_lft);
>> +	array[DEVCONF_FORCE_FORWARDING] = READ_ONCE(cnf->force_forwarding);
>>  }
>>
>[...]
>>  static inline size_t inet6_ifla6_size(void)
>> @@ -6747,6 +6759,78 @@ static int addrconf_sysctl_disable_policy(const struct ctl_table *ctl, int write
>>  	return ret;
>>  }
>>
>> +static void addrconf_force_forward_change(struct net *net, __s32 newf)
>> +{
>> +	ASSERT_RTNL();
>
>__in6_dev_get_rtnl_net() has the same check so this is not needed.

Agree.

>> +	struct net_device *dev;
>> +	struct inet6_dev *idev;
>> +
>> +	for_each_netdev(net, dev) {
>> +		idev = __in6_dev_get_rtnl_net(dev);
>> +		if (idev) {
>> +			int changed = (!idev->cnf.force_forwarding) ^ (!newf);
>> +
>> +			WRITE_ONCE(idev->cnf.force_forwarding, newf);
>> +			if (changed) {
>> +				inet6_netconf_notify_devconf(dev_net(dev), RTM_NEWNETCONF,
>> +							     NETCONFA_FORCE_FORWARDING,
>> +							     dev->ifindex, &idev->cnf);
>> +			}
>> +		}
>> +	}
>> +}
>> +
>> +static int addrconf_sysctl_force_forwarding(const struct ctl_table *ctl, int write,
>> +					    void *buffer, size_t *lenp, loff_t *ppos)
>> +{
>> +	struct inet6_dev *idev = ctl->extra1;
>> +	struct net *net = ctl->extra2;
>> +	int *valp = ctl->data;
>> +	loff_t pos = *ppos;
>
>nit: Please keep the reverse xmas tree order.
>https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html#local-variable-ordering-reverse-xmas-tree-rcs
>
>> +	int new_val = *valp;
>> +	int old_val = *valp;
>> +	int ret;
>> +
>> +	struct ctl_table tmp_ctl = *ctl;
>
>same here.

Will do; moved this up.

>> +
>> +	tmp_ctl.extra1 = SYSCTL_ZERO;
>> +	tmp_ctl.extra2 = SYSCTL_ONE;
>
>As you are copying *ctl, please specify this in addrconf_sysctl[].

Umm how would I do that? Do you want me to add a comment explaining it?
I need extra1 and extra2 to be the network device so that I can set
NETCONFA_FORCE_FORWARDING but I also want to use proc_douintvec_minmax.

>> +	tmp_ctl.data = &new_val;
>> +
>> +	ret = proc_douintvec_minmax(&tmp_ctl, write, buffer, lenp, ppos);
>> +
>> +	if (write && old_val != new_val) {
>> +		if (!rtnl_net_trylock(net))
>> +			return restart_syscall();
>> +
>> +		if (valp == &net->ipv6.devconf_dflt->force_forwarding) {
>> +			inet6_netconf_notify_devconf(net, RTM_NEWNETCONF,
>> +						     NETCONFA_FORCE_FORWARDING,
>> +						     NETCONFA_IFINDEX_DEFAULT,
>> +						     net->ipv6.devconf_dflt);
>> +		} else if (valp == &net->ipv6.devconf_all->force_forwarding) {
>> +			inet6_netconf_notify_devconf(net, RTM_NEWNETCONF,
>> +						     NETCONFA_FORCE_FORWARDING,
>> +						     NETCONFA_IFINDEX_ALL,
>> +						     net->ipv6.devconf_all);
>> +
>> +			addrconf_force_forward_change(net, new_val);
>> +		} else {
>> +			inet6_netconf_notify_devconf(net, RTM_NEWNETCONF,
>> +						     NETCONFA_FORCE_FORWARDING,
>> +						     idev->dev->ifindex,
>> +						     &idev->cnf);
>> +		}
>> +		rtnl_net_unlock(net);
>> +	}
>> +
>> +	if (write)
>> +		WRITE_ONCE(*valp, new_val);
>> +	if (ret)
>> +		*ppos = pos;
>> +	return ret;
>> +}
>> +
>>  static int minus_one = -1;
>>  static const int two_five_five = 255;
>>  static u32 ioam6_if_id_max = U16_MAX;
>> @@ -7217,6 +7301,13 @@ static const struct ctl_table addrconf_sysctl[] = {
>>  		.extra1		= SYSCTL_ZERO,
>>  		.extra2		= SYSCTL_TWO,
>>  	},
>> +	{
>> +		.procname	= "force_forwarding",
>> +		.data		= &ipv6_devconf.force_forwarding,
>> +		.maxlen		= sizeof(int),
>> +		.mode		= 0644,
>> +		.proc_handler	= addrconf_sysctl_force_forwarding,
>
>Here for extra{1,2}.

See above.

>> +	},
>>  };
>>  static int __addrconf_sysctl_register(struct net *net, char *dev_name,
>> diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
>> index 7bd29a9ff0db..440b9efced72 100644
>> --- a/net/ipv6/ip6_output.c
>> +++ b/net/ipv6/ip6_output.c
>> @@ -509,7 +509,8 @@ int ip6_forward(struct sk_buff *skb)
>>  	u32 mtu;
>>
>>  	idev = __in6_dev_get_safely(dev_get_by_index_rcu(net, IP6CB(skb)->iif));
>> -	if (READ_ONCE(net->ipv6.devconf_all->forwarding) == 0)
>> +	if (idev && !READ_ONCE(idev->cnf.force_forwarding) &&
>> +	    !READ_ONCE(net->ipv6.devconf_all->forwarding))
>
>Now this ignores devconf_all when !idev whose dev was not
>found or has not had a valid mtu.
>
>if (!READ_ONCE(net->ipv6.devconf_all->forwarding) &&
>    (!idev || !READ_ONCE(idev->cnf.force_forwarding)))

Oof yeah, you're right this is my mistake. I'll use your solution.

Thanks for the review!
Re: [PATCH net-next v4] ipv6: add `force_forwarding` sysctl to enable per-interface forwarding
Posted by Kuniyuki Iwashima 3 months ago
On Fri, Jul 4, 2025 at 2:37 AM Gabriel Goller <g.goller@proxmox.com> wrote:
[...]
> >> +
> >> +    tmp_ctl.extra1 = SYSCTL_ZERO;
> >> +    tmp_ctl.extra2 = SYSCTL_ONE;
> >
> >As you are copying *ctl, please specify this in addrconf_sysctl[].
>
> Umm how would I do that? Do you want me to add a comment explaining it?
> I need extra1 and extra2 to be the network device so that I can set
> NETCONFA_FORCE_FORWARDING but I also want to use proc_douintvec_minmax.

Ah, I simply missed the net/idev use, please ignore my comment here.