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 | 90 ++++++++++++++++++++++++++ net/ipv6/ip6_output.c | 3 +- 7 files changed, 101 insertions(+), 1 deletion(-)
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.
[0]: https://www.kernel.org/doc/Documentation/networking/ip-sysctl.txt
Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
---
Changelog:
v2 -> v3:
* remove forwarding=0 setting force_forwarding=0 globally.
* add min and max (0 and 1) value to sysctl.
v1 -> v2:
* 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.
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 | 90 ++++++++++++++++++++++++++
net/ipv6/ip6_output.c | 3 +-
7 files changed, 101 insertions(+), 1 deletion(-)
diff --git a/Documentation/networking/ip-sysctl.rst b/Documentation/networking/ip-sysctl.rst
index 0f1251cce314..f709aed44cde 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..cca78f75cf0c 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,77 @@ static int addrconf_sysctl_disable_policy(const struct ctl_table *ctl, int write
return ret;
}
+/* called with RTNL locked */
+static void addrconf_force_forward_change(struct net *net, __s32 newf)
+{
+ 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)
+{
+ int *valp = ctl->data;
+ int ret;
+ int old, new;
+
+ // get extra params from table
+ struct inet6_dev *idev = ctl->extra1;
+ struct net *net = ctl->extra2;
+
+ // copy table and change extra params to min/max so we can use proc_douintvec_minmax
+ struct ctl_table lctl;
+
+ lctl = *ctl;
+ lctl.extra1 = SYSCTL_ZERO;
+ lctl.extra2 = SYSCTL_ONE;
+
+ old = *valp;
+ ret = proc_douintvec_minmax(&lctl, write, buffer, lenp, ppos);
+ new = *valp;
+
+ if (write && old != new) {
+ 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);
+ } else {
+ inet6_netconf_notify_devconf(net, RTM_NEWNETCONF,
+ NETCONFA_FORCE_FORWARDING,
+ idev->dev->ifindex,
+ &idev->cnf);
+ }
+ rtnl_net_unlock(net);
+ }
+
+ return ret;
+}
+
static int minus_one = -1;
static const int two_five_five = 255;
static u32 ioam6_if_id_max = U16_MAX;
@@ -7217,6 +7300,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..c15ed4197416 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) == 0) &&
+ READ_ONCE(net->ipv6.devconf_all->forwarding) == 0)
goto error;
if (skb->pkt_type != PACKET_HOST)
--
2.39.5On Wed, 2 Jul 2025 09:46:18 +0200 Gabriel Goller wrote: > 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. Should we invert the polarity? It appears that the condition below only let's this setting _disable_ forwarding. IMO calling it "force" suggests to the user that it will force it to be enabled. Nicolas, how do you feel about asking for a selftest here? The functionality is fairly trivial from datapath PoV, but feels odd to merge uAPI these days without a selftest..
Le 02/07/2025 à 16:34, Jakub Kicinski a écrit :
> On Wed, 2 Jul 2025 09:46:18 +0200 Gabriel Goller wrote:
>> 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.
>
> Should we invert the polarity? It appears that the condition below only
> let's this setting _disable_ forwarding. IMO calling it "force" suggests
> to the user that it will force it to be enabled.
Not sure to follow you. When force_forwarding is set to 1 the forwarding is
always enabled.
sysctl | all.forwarding | iface.force_forwarding | packet processing from iface
| 0 | 0 | no forward
| 0 | 1 | forward
| 1 | 0 | forward
| 1 | 1 | forward
>
> Nicolas, how do you feel about asking for a selftest here?
> The functionality is fairly trivial from datapath PoV, but feels odd
> to merge uAPI these days without a selftest..
No problem, let's do it right.
On Wed, 2 Jul 2025 17:14:42 +0200 Nicolas Dichtel wrote: > > Should we invert the polarity? It appears that the condition below only > > let's this setting _disable_ forwarding. IMO calling it "force" suggests > > to the user that it will force it to be enabled. > Not sure to follow you. When force_forwarding is set to 1 the forwarding is > always enabled. > > sysctl | all.forwarding | iface.force_forwarding | packet processing from iface > | 0 | 0 | no forward > | 0 | 1 | forward > | 1 | 0 | forward > | 1 | 1 | forward Ugh, I can't read comparisons to zero. Let's switch to more sane logic: if (idev && !READ_ONCE(idev->cnf.force_forwarding) && !READ_ONCE(net->ipv6.devconf_all->forwarding))
On 02.07.2025 09:10, Jakub Kicinski wrote: >On Wed, 2 Jul 2025 17:14:42 +0200 Nicolas Dichtel wrote: >> > Should we invert the polarity? It appears that the condition below only >> > let's this setting _disable_ forwarding. IMO calling it "force" suggests >> > to the user that it will force it to be enabled. >> Not sure to follow you. When force_forwarding is set to 1 the forwarding is >> always enabled. >> >> sysctl | all.forwarding | iface.force_forwarding | packet processing from iface >> | 0 | 0 | no forward >> | 0 | 1 | forward >> | 1 | 0 | forward >> | 1 | 1 | forward > >Ugh, I can't read comparisons to zero. >Let's switch to more sane logic: > > if (idev && !READ_ONCE(idev->cnf.force_forwarding) && > !READ_ONCE(net->ipv6.devconf_all->forwarding)) Agree! Thanks for the review.
Le 02/07/2025 à 18:10, Jakub Kicinski a écrit : > On Wed, 2 Jul 2025 17:14:42 +0200 Nicolas Dichtel wrote: >>> Should we invert the polarity? It appears that the condition below only >>> let's this setting _disable_ forwarding. IMO calling it "force" suggests >>> to the user that it will force it to be enabled. >> Not sure to follow you. When force_forwarding is set to 1 the forwarding is >> always enabled. >> >> sysctl | all.forwarding | iface.force_forwarding | packet processing from iface >> | 0 | 0 | no forward >> | 0 | 1 | forward >> | 1 | 0 | forward >> | 1 | 1 | forward > > Ugh, I can't read comparisons to zero. > Let's switch to more sane logic: > > if (idev && !READ_ONCE(idev->cnf.force_forwarding) && > !READ_ONCE(net->ipv6.devconf_all->forwarding)) +1
Le 02/07/2025 à 09:46, 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.
>
> [0]: https://www.kernel.org/doc/Documentation/networking/ip-sysctl.txt
>
> Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
> ---
>
Please, wait 24 hours before reposting.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/maintainer-netdev.rst#n419
[snip]
> @@ -6747,6 +6759,77 @@ static int addrconf_sysctl_disable_policy(const struct ctl_table *ctl, int write
> return ret;
> }
>
> +/* called with RTNL locked */
Instead of a comment ...
> +static void addrconf_force_forward_change(struct net *net, __s32 newf)
> +{
> + struct net_device *dev;
> + struct inet6_dev *idev;
> +
... put
ASSERT_RTNL();
> + 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)
> +{
> + int *valp = ctl->data;
> + int ret;
> + int old, new;
> +
> + // get extra params from table
/* */ for comment
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-style.rst#n598
> + struct inet6_dev *idev = ctl->extra1;
> + struct net *net = ctl->extra2;
Reverse x-mas tree for the variables declaration
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/maintainer-netdev.rst#n368
> +
> + // copy table and change extra params to min/max so we can use proc_douintvec_minmax
> + struct ctl_table lctl;
> +
> + lctl = *ctl;
> + lctl.extra1 = SYSCTL_ZERO;
> + lctl.extra2 = SYSCTL_ONE;
> +
> + old = *valp;
> + ret = proc_douintvec_minmax(&lctl, write, buffer, lenp, ppos);
> + new = *valp;
I probably missed something. The new value is written in lctl. When is it
written in ctl?
> +
> + if (write && old != new) {
> + 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);
> + } else {
> + inet6_netconf_notify_devconf(net, RTM_NEWNETCONF,
> + NETCONFA_FORCE_FORWARDING,
> + idev->dev->ifindex,
> + &idev->cnf);
> + }
> + rtnl_net_unlock(net);
> + }
> +
> + return ret;
> +}
> +
> static int minus_one = -1;
> static const int two_five_five = 255;
> static u32 ioam6_if_id_max = U16_MAX;
On 02.07.2025 12:05, Nicolas Dichtel wrote:
>Le 02/07/2025 à 09:46, 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.
>>
>> [0]: https://www.kernel.org/doc/Documentation/networking/ip-sysctl.txt
>>
>> Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
>> ---
>>
>Please, wait 24 hours before reposting.
>https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/maintainer-netdev.rst#n419
Ah, my bad, thought I posted v2 in the morning as well :(
>[snip]
>
>> @@ -6747,6 +6759,77 @@ static int addrconf_sysctl_disable_policy(const struct ctl_table *ctl, int write
>> return ret;
>> }
>>
>> +/* called with RTNL locked */
>Instead of a comment ...
>
>> +static void addrconf_force_forward_change(struct net *net, __s32 newf)
>> +{
>> + struct net_device *dev;
>> + struct inet6_dev *idev;
>> +
>... put
>
> ASSERT_RTNL();
>
Agree.
>> + 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)
>> +{
>> + int *valp = ctl->data;
>> + int ret;
>> + int old, new;
>> +
>> + // get extra params from table
>/* */ for comment
>https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-style.rst#n598
NAK
(https://lore.kernel.org/lkml/CA+55aFyQYJerovMsSoSKS7PessZBr4vNp-3QUUwhqk4A4_jcbg@mail.gmail.com/#r)
But I'll capitalize the first word.
>> + struct inet6_dev *idev = ctl->extra1;
>> + struct net *net = ctl->extra2;
>Reverse x-mas tree for the variables declaration
>https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/maintainer-netdev.rst#n368
Done.
>> +
>> + // copy table and change extra params to min/max so we can use proc_douintvec_minmax
>> + struct ctl_table lctl;
>> +
>> + lctl = *ctl;
>> + lctl.extra1 = SYSCTL_ZERO;
>> + lctl.extra2 = SYSCTL_ONE;
>> +
>> + old = *valp;
>> + ret = proc_douintvec_minmax(&lctl, write, buffer, lenp, ppos);
>> + new = *valp;
>I probably missed something. The new value is written in lctl. When is it
>written in ctl?
Ah, sorry there is something missing here.
This is supposed to look like this:
struct inet6_dev *idev = ctl->extra1;
struct net *net = ctl->extra2;
int *valp = ctl->data;
loff_t pos = *ppos;
int new = *valp;
int old = *valp;
int ret;
struct ctl_table lctl;
lctl = *ctl;
lctl.extra1 = SYSCTL_ZERO;
lctl.extra2 = SYSCTL_ONE;
lctl.data = &new;
ret = proc_douintvec_minmax(&lctl, write, buffer, lenp, ppos);
...
if (write)
WRITE_ONCE(*valp, new);
if (ret)
*ppos = pos;
return ret;
>> +
>> + if (write && old != new) {
>> + 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);
>> + } else {
>> + inet6_netconf_notify_devconf(net, RTM_NEWNETCONF,
>> + NETCONFA_FORCE_FORWARDING,
>> + idev->dev->ifindex,
>> + &idev->cnf);
>> + }
>> + rtnl_net_unlock(net);
>> + }
>> +
>> + return ret;
>> +}
>> +
>> static int minus_one = -1;
>> static const int two_five_five = 255;
>> static u32 ioam6_if_id_max = U16_MAX;
Thanks for the review!
Le 03/07/2025 à 13:04, Gabriel Goller a écrit : [snip] >>> + // get extra params from table >> /* */ for comment >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/ >> Documentation/process/coding-style.rst#n598 > > NAK > (https://lore.kernel.org/lkml/ > CA+55aFyQYJerovMsSoSKS7PessZBr4vNp-3QUUwhqk4A4_jcbg@mail.gmail.com/#r) I will follow the netdev maintainers' guidelines. If the doc I pointed to is wrong, please update it. It will be easier to find than a 9-year-old email. Regards, Nicolas
On 03.07.2025 16:04, Nicolas Dichtel wrote: >Le 03/07/2025 à 13:04, Gabriel Goller a écrit : >[snip] >>>> + // get extra params from table >>> /* */ for comment >>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/ >>> Documentation/process/coding-style.rst#n598 >> >> NAK >> (https://lore.kernel.org/lkml/ >> CA+55aFyQYJerovMsSoSKS7PessZBr4vNp-3QUUwhqk4A4_jcbg@mail.gmail.com/#r) > >I will follow the netdev maintainers' guidelines. > >If the doc I pointed to is wrong, please update it. It will be easier to find >than a 9-year-old email. The netdev maintainers guideline doesn't contain anything about comments. Also the coding-style doesn't prohibit single-line comments with the `//` style. The comments are kinda pointless though, I'll remove them in the next version. >Regards, >Nicolas Thanks
On 7/2/25 3:05 AM, Nicolas Dichtel wrote:
> Le 02/07/2025 à 09:46, 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.
>>
>> [0]: https://www.kernel.org/doc/Documentation/networking/ip-sysctl.txt
>>
>> Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
>> ---
[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;
>> +
>> + // get extra params from table
> /* */ for comment
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-style.rst#n598
Hm, lots there from the BK to git transfer in 2005, with a few updates by Mauro, Jakub, and myself.
More recently (2016!), Linus said this:
https://lore.kernel.org/lkml/CA+55aFyQYJerovMsSoSKS7PessZBr4vNp-3QUUwhqk4A4_jcbg@mail.gmail.com/
which seems to allow for "//" style commenting. But yeah, it hasn't been added to
coding-style.rst.
>> + struct inet6_dev *idev = ctl->extra1;
>> + struct net *net = ctl->extra2;
> Reverse x-mas tree for the variables declaration
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/maintainer-netdev.rst#n368
Shouldn't maintainer-netdev.rst contain something about netdev-style comment blocks?
(not that I'm offering since I think it's ugly)
--
~Randy
Le 03/07/2025 à 00:26, Randy Dunlap a écrit :
[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;
>>> +
>>> + // get extra params from table
>> /* */ for comment
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-style.rst#n598
>
> Hm, lots there from the BK to git transfer in 2005, with a few updates by Mauro, Jakub, and myself.
>
>
> More recently (2016!), Linus said this:
> https://lore.kernel.org/lkml/CA+55aFyQYJerovMsSoSKS7PessZBr4vNp-3QUUwhqk4A4_jcbg@mail.gmail.com/
>
> which seems to allow for "//" style commenting. But yeah, it hasn't been added to
> coding-style.rst.
I wasn't aware. I always seen '//' rejected.
>
>>> + struct inet6_dev *idev = ctl->extra1;
>>> + struct net *net = ctl->extra2;
>> Reverse x-mas tree for the variables declaration
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/maintainer-netdev.rst#n368
>
> Shouldn't maintainer-netdev.rst contain something about netdev-style comment blocks?
> (not that I'm offering since I think it's ugly)
>
It has been removed:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=82b8000c28b5
On July 2, 2025 11:58:16 PM PDT, Nicolas Dichtel <nicolas.dichtel@6wind.com> wrote:
>Le 03/07/2025 à 00:26, Randy Dunlap a écrit :
>
>[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;
>>>> +
>>>> + // get extra params from table
>>> /* */ for comment
>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-style.rst#n598
>>
>> Hm, lots there from the BK to git transfer in 2005, with a few updates by Mauro, Jakub, and myself.
>>
>>
>> More recently (2016!), Linus said this:
>> https://lore.kernel.org/lkml/CA+55aFyQYJerovMsSoSKS7PessZBr4vNp-3QUUwhqk4A4_jcbg@mail.gmail.com/
>>
>> which seems to allow for "//" style commenting. But yeah, it hasn't been added to
>> coding-style.rst.
>I wasn't aware. I always seen '//' rejected.
>
>>
>>>> + struct inet6_dev *idev = ctl->extra1;
>>>> + struct net *net = ctl->extra2;
>>> Reverse x-mas tree for the variables declaration
>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/maintainer-netdev.rst#n368
>>
>> Shouldn't maintainer-netdev.rst contain something about netdev-style comment blocks?
>> (not that I'm offering since I think it's ugly)
>>
>It has been removed:
>https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=82b8000c28b5
>
Oh, thanks. Sorry I missed that patch.
~Randy
© 2016 - 2025 Red Hat, Inc.