[PATCH 1/3 net-next v6] ipv4: centralize devconf sysctl handling

[PATCH 1/3 net-next v6] ipv4: centralize devconf sysctl handling

[Fernando Fernandez Mancera]

The logic for handling IPv4 devconf sysctls is scattered. Notification
and cache flushes are managed in devinet_conf_proc(), while a separate
ipv4_doint_and_flush() function and DEVINET_SYSCTL_FLUSHING_ENTRY macro
is used for properties that solely require a cache flush.

This patch refactors the sysctl handling by introducing a centralized
helper, devinet_conf_post_set(). This new function evaluates the changed
attribute and handles all necessary operations like triggering netlink
notifications. It returns a boolean indicating whether a routing cache
flush is required.

Note that the boolean is necessary as this function will be re-used for
netlink IPv4 devconf handling where the cache flushing must wait until
all the attributes have been processed.

Finally, this is introducing a small change in behavior for
IPV4_DEVCONF_ROUTE_LOCALNET. As commit d0daebc3d622 ("ipv4: Add
interface option to enable routing of 127.0.0.0/8") intended, the cache
flush should only be performed when ROUTE_LOCALNET changes from 1 to 0.
Unfortunately, this was not true because while implementing it the
DEVINET_SYSCTL_FLUSHING_ENTRY was used for the attribute, making the
code related to it on devinet_conf_proc() dead.

IPV4_DEVCONF_FORWARDING is still being handled separately as it requires
more operations.

Reviewed-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Signed-off-by: Fernando Fernandez Mancera <fmancera@suse.de>
---
v2: no changes
v3: no changes
v4: no changes
v5: no changes
v6: no changes
---
 net/ipv4/devinet.c | 127 ++++++++++++++++++++++++---------------------
 1 file changed, 68 insertions(+), 59 deletions(-)

diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index 58fe7cb69545..8300516fb38f 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -2128,6 +2128,46 @@ static int inet_validate_link_af(const struct net_device *dev,
 	return 0;
 }
 
+static bool devinet_conf_post_set(struct net *net, struct ipv4_devconf *cnf,
+				  int attr, int new, int old, int ifindex)
+{
+	if (new == old)
+		return false;
+
+	switch (attr) {
+	case IPV4_DEVCONF_ROUTE_LOCALNET:
+	case IPV4_DEVCONF_ACCEPT_LOCAL:
+		if (new == 0)
+			return true;
+		break;
+	case IPV4_DEVCONF_NOXFRM:
+	case IPV4_DEVCONF_NOPOLICY:
+	case IPV4_DEVCONF_PROMOTE_SECONDARIES:
+	case IPV4_DEVCONF_DROP_UNICAST_IN_L2_MULTICAST:
+	case IPV4_DEVCONF_BC_FORWARDING:
+		return true;
+	case IPV4_DEVCONF_RP_FILTER:
+		inet_netconf_notify_devconf(net, RTM_NEWNETCONF,
+					    NETCONFA_RP_FILTER,
+					    ifindex, cnf);
+		break;
+	case IPV4_DEVCONF_PROXY_ARP:
+		inet_netconf_notify_devconf(net, RTM_NEWNETCONF,
+					    NETCONFA_PROXY_NEIGH,
+					    ifindex, cnf);
+		break;
+	case IPV4_DEVCONF_IGNORE_ROUTES_WITH_LINKDOWN:
+		inet_netconf_notify_devconf(net, RTM_NEWNETCONF,
+					    NETCONFA_IGNORE_ROUTES_WITH_LINKDOWN,
+					    ifindex, cnf);
+		break;
+	default:
+		break;
+	}
+
+	return false;
+}
+
 static int inet_set_link_af(struct net_device *dev, const struct nlattr *nla,
 			    struct netlink_ext_ack *extack)
 {
@@ -2509,44 +2549,31 @@ static int devinet_conf_proc(const struct ctl_table *ctl, int write,
 
 	if (write) {
 		struct ipv4_devconf *cnf = ctl->extra1;
-		struct net *net = ctl->extra2;
 		int i = (int *)ctl->data - cnf->data;
+		struct net *net = ctl->extra2;
 		int ifindex;
 
-		set_bit(i, cnf->state);
-
-		if (cnf == net->ipv4.devconf_dflt)
-			devinet_copy_dflt_conf(net, i);
-		if (i == IPV4_DEVCONF_ACCEPT_LOCAL - 1 ||
-		    i == IPV4_DEVCONF_ROUTE_LOCALNET - 1)
-			if ((new_value == 0) && (old_value != 0))
-				rt_cache_flush(net);
+		/* These attributes are bypassing the tracking state,
+		 * for the rest track the state and propagate the changes
+		 * to default config
+		 */
+		switch (i + 1) {
+		case IPV4_DEVCONF_NOXFRM:
+		case IPV4_DEVCONF_NOPOLICY:
+		case IPV4_DEVCONF_PROMOTE_SECONDARIES:
+		case IPV4_DEVCONF_DROP_UNICAST_IN_L2_MULTICAST:
+			break;
+		default:
+			set_bit(i, cnf->state);
+			if (cnf == net->ipv4.devconf_dflt)
+				devinet_copy_dflt_conf(net, i);
+			break;
+		}
 
-		if (i == IPV4_DEVCONF_BC_FORWARDING - 1 &&
-		    new_value != old_value)
+		ifindex = devinet_conf_ifindex(net, cnf);
+		if (devinet_conf_post_set(net, cnf, i + 1, new_value,
+					  old_value, ifindex))
 			rt_cache_flush(net);
-
-		if (i == IPV4_DEVCONF_RP_FILTER - 1 &&
-		    new_value != old_value) {
-			ifindex = devinet_conf_ifindex(net, cnf);
-			inet_netconf_notify_devconf(net, RTM_NEWNETCONF,
-						    NETCONFA_RP_FILTER,
-						    ifindex, cnf);
-		}
-		if (i == IPV4_DEVCONF_PROXY_ARP - 1 &&
-		    new_value != old_value) {
-			ifindex = devinet_conf_ifindex(net, cnf);
-			inet_netconf_notify_devconf(net, RTM_NEWNETCONF,
-						    NETCONFA_PROXY_NEIGH,
-						    ifindex, cnf);
-		}
-		if (i == IPV4_DEVCONF_IGNORE_ROUTES_WITH_LINKDOWN - 1 &&
-		    new_value != old_value) {
-			ifindex = devinet_conf_ifindex(net, cnf);
-			inet_netconf_notify_devconf(net, RTM_NEWNETCONF,
-						    NETCONFA_IGNORE_ROUTES_WITH_LINKDOWN,
-						    ifindex, cnf);
-		}
 	}
 
 	return ret;
@@ -2599,20 +2626,6 @@ static int devinet_sysctl_forward(const struct ctl_table *ctl, int write,
 	return ret;
 }
 
-static int ipv4_doint_and_flush(const struct ctl_table *ctl, int write,
-				void *buffer, size_t *lenp, loff_t *ppos)
-{
-	int *valp = ctl->data;
-	int val = *valp;
-	int ret = proc_dointvec(ctl, write, buffer, lenp, ppos);
-	struct net *net = ctl->extra2;
-
-	if (write && *valp != val)
-		rt_cache_flush(net);
-
-	return ret;
-}
-
 #define DEVINET_SYSCTL_ENTRY(attr, name, mval, proc) \
 	{ \
 		.procname	= name, \
@@ -2633,9 +2646,6 @@ static int ipv4_doint_and_flush(const struct ctl_table *ctl, int write,
 #define DEVINET_SYSCTL_COMPLEX_ENTRY(attr, name, proc) \
 	DEVINET_SYSCTL_ENTRY(attr, name, 0644, proc)
 
-#define DEVINET_SYSCTL_FLUSHING_ENTRY(attr, name) \
-	DEVINET_SYSCTL_COMPLEX_ENTRY(attr, name, ipv4_doint_and_flush)
-
 static struct devinet_sysctl_table {
 	struct ctl_table_header *sysctl_header;
 	struct ctl_table devinet_vars[IPV4_DEVCONF_MAX];
@@ -2678,15 +2688,14 @@ static struct devinet_sysctl_table {
 					"ignore_routes_with_linkdown"),
 		DEVINET_SYSCTL_RW_ENTRY(DROP_GRATUITOUS_ARP,
 					"drop_gratuitous_arp"),
-
-		DEVINET_SYSCTL_FLUSHING_ENTRY(NOXFRM, "disable_xfrm"),
-		DEVINET_SYSCTL_FLUSHING_ENTRY(NOPOLICY, "disable_policy"),
-		DEVINET_SYSCTL_FLUSHING_ENTRY(PROMOTE_SECONDARIES,
-					      "promote_secondaries"),
-		DEVINET_SYSCTL_FLUSHING_ENTRY(ROUTE_LOCALNET,
-					      "route_localnet"),
-		DEVINET_SYSCTL_FLUSHING_ENTRY(DROP_UNICAST_IN_L2_MULTICAST,
-					      "drop_unicast_in_l2_multicast"),
+		DEVINET_SYSCTL_RW_ENTRY(NOXFRM, "disable_xfrm"),
+		DEVINET_SYSCTL_RW_ENTRY(NOPOLICY, "disable_policy"),
+		DEVINET_SYSCTL_RW_ENTRY(PROMOTE_SECONDARIES,
+					"promote_secondaries"),
+		DEVINET_SYSCTL_RW_ENTRY(ROUTE_LOCALNET,
+					"route_localnet"),
+		DEVINET_SYSCTL_RW_ENTRY(DROP_UNICAST_IN_L2_MULTICAST,
+					"drop_unicast_in_l2_multicast"),
 	},
 };
 
-- 
2.54.0

[PATCH 2/3 net-next v6] ipv4: handle devconf post-set actions on netlink updates

[Fernando Fernandez Mancera]

When IPv4 device configuration parameters are updated via netlink, the
kernel currently only updates the value. This bypasses several
post-modification actions that occur when these same parameters are
updated via sysctl, such as flushing the routing cache or emitting
RTM_NEWNETCONF notifications.

This patch addresses the inconsistency by calling the
devinet_conf_post_set() helper inside inet_set_link_af(). If a flush is
required, we defer it until the netlink attribute parsing loop
completes.

This ensures consistent behavior and side-effects for devconf changes,
regardless of whether they are initiated via sysctl or netlink.

Reviewed-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Signed-off-by: Fernando Fernandez Mancera <fmancera@suse.de>
---
v2: handled forwarding notification and disabling LRO
v3: no changes
v4: no changes
v5: no changes
v6: no changes
---
 net/ipv4/devinet.c | 29 +++++++++++++++++++++++++++--
 1 file changed, 27 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index 8300516fb38f..a35b72662e43 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -2161,6 +2161,20 @@ static bool devinet_conf_post_set(struct net *net, struct ipv4_devconf *cnf,
 					    NETCONFA_IGNORE_ROUTES_WITH_LINKDOWN,
 					    ifindex, cnf);
 		break;
+	case IPV4_DEVCONF_FORWARDING:
+		if (new == 1) {
+			/* it is safe to use container_of() because forwarding case
+			 * is only used by the netlink path
+			 */
+			struct in_device *idev = container_of(cnf, struct in_device, cnf);
+
+			netif_disable_lro(idev->dev);
+		}
+
+		inet_netconf_notify_devconf(net, RTM_NEWNETCONF,
+					    NETCONFA_FORWARDING,
+					    ifindex, cnf);
+		return true;
 	default:
 		break;
 	}
@@ -2173,6 +2187,8 @@ static int inet_set_link_af(struct net_device *dev, const struct nlattr *nla,
 {
 	struct in_device *in_dev = __in_dev_get_rtnl(dev);
 	struct nlattr *a, *tb[IFLA_INET_MAX+1];
+	struct net *net = dev_net(dev);
+	bool flush_cache = false;
 	int rem;
 
 	if (!in_dev)
@@ -2182,8 +2198,17 @@ static int inet_set_link_af(struct net_device *dev, const struct nlattr *nla,
 		return -EINVAL;
 
 	if (tb[IFLA_INET_CONF]) {
-		nla_for_each_nested(a, tb[IFLA_INET_CONF], rem)
-			ipv4_devconf_set(in_dev, nla_type(a), nla_get_u32(a));
+		nla_for_each_nested(a, tb[IFLA_INET_CONF], rem) {
+			int old_value = ipv4_devconf_get(in_dev, nla_type(a));
+			int new_value = nla_get_u32(a);
+
+			ipv4_devconf_set(in_dev, nla_type(a), new_value);
+			if (devinet_conf_post_set(net, &in_dev->cnf, nla_type(a), new_value,
+						  old_value, dev->ifindex))
+				flush_cache = true;
+		}
+		if (flush_cache)
+			rt_cache_flush(net);
 	}
 
 	return 0;
-- 
2.54.0

[PATCH 3/3 net-next v6] selftests: net: add test for IPv4 devconf netlink notifications

[Fernando Fernandez Mancera]

Introduce a new test, `ipv4_devconf_notify`, to verify that the kernel
sends the appropriate netlink notifications when IPv4 devconf parameters
are modified.

The test depends on the newly introduced iproute2 command:

`ip link set dev <ifname> inet`

Signed-off-by: Fernando Fernandez Mancera <fmancera@suse.de>
---
v3: added this patch to the series as requested by Paolo.
v4: fixed lint warnings, some of them cannot be fixed as they are
related to the YNL hack.
v5: use the newly introduce iproute2 inet command, drop the YNL hack
v6: fix ruff warning about plain "except:", instead catch CmdExitFailure
---
 tools/testing/selftests/net/rtnetlink.py | 41 +++++++++++++++++++++---
 1 file changed, 37 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/net/rtnetlink.py b/tools/testing/selftests/net/rtnetlink.py
index e9ad5e88da97..3622413d793d 100755
--- a/tools/testing/selftests/net/rtnetlink.py
+++ b/tools/testing/selftests/net/rtnetlink.py
@@ -1,17 +1,20 @@
 #!/usr/bin/env python3
 # SPDX-License-Identifier: GPL-2.0
 
-from lib.py import ksft_exit, ksft_run, ksft_ge, RtnlAddrFamily
 import socket
+import time
+from lib.py import bkg, ip, ksft_exit, ksft_run, ksft_ge, ksft_true, KsftSkipEx
+from lib.py import CmdExitFailure, NetNS, NetNSEnter, RtnlAddrFamily
 
 IPV4_ALL_HOSTS_MULTICAST = b'\xe0\x00\x00\x01'
 
-def dump_mcaddr_check(rtnl: RtnlAddrFamily) -> None:
+def dump_mcaddr_check() -> None:
     """
     Verify that at least one interface has the IPv4 all-hosts multicast address.
     At least the loopback interface should have this address.
     """
 
+    rtnl = RtnlAddrFamily()
     addresses = rtnl.getmulticast({"ifa-family": socket.AF_INET}, dump=True)
 
     all_host_multicasts = [
@@ -21,9 +24,39 @@ def dump_mcaddr_check(rtnl: RtnlAddrFamily) -> None:
     ksft_ge(len(all_host_multicasts), 1,
             "No interface found with the IPv4 all-hosts multicast address")
 
+def ipv4_devconf_notify() -> None:
+    """
+    Configure an interface and set ipv4-devconf values through netlink
+    to verify that the appropriate netlink notifications are being sent.
+    """
+
+    with NetNS() as ns:
+        with NetNSEnter(str(ns)):
+            ifname = "dummy1"
+            ip(f"link add name {ifname} type dummy", ns=str(ns))
+
+            with bkg("ip monitor", ns=str(ns)) as cmd_obj:
+                time.sleep(1)
+                try:
+                    ip(f"link set dev {ifname} inet forwarding on")
+                    ip(f"link set dev {ifname} inet proxy_arp on")
+                    ip(f"link set dev {ifname} inet rp_filter 1")
+                    ip(f"link set dev {ifname} inet ignore_routes_with_linkdown on")
+                except CmdExitFailure:
+                    raise KsftSkipEx("iproute2 does not support IPv4 devconf attributes")
+                time.sleep(1)
+
+    ksft_true(f"inet {ifname} ignore_routes_with_linkdown on" in cmd_obj.stdout,
+              f"No 'ignore_routes_with_linkdown on' notificiation found for interface {ifname}")
+    ksft_true(f"inet {ifname} rp_filter strict" in cmd_obj.stdout,
+              f"No 'rp_filter strict' notificiation found for interface {ifname}")
+    ksft_true(f"inet {ifname} proxy_neigh on" in cmd_obj.stdout,
+              f"No 'proxy_neigh on' notificiation found for interface {ifname}")
+    ksft_true(f"inet {ifname} forwarding on" in cmd_obj.stdout,
+              f"No 'forwarding on' notificiation found for interface {ifname}")
+
 def main() -> None:
-    rtnl = RtnlAddrFamily()
-    ksft_run([dump_mcaddr_check], args=(rtnl, ))
+    ksft_run([dump_mcaddr_check, ipv4_devconf_notify])
     ksft_exit()
 
 if __name__ == "__main__":
-- 
2.54.0

Re: [PATCH 2/3 net-next v6] ipv4: handle devconf post-set actions on netlink updates

[Simon Horman]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev
---
ipv4: handle devconf post-set actions on netlink updates

This commit ensures that IPv4 device configuration parameters updated
via netlink trigger the same post-modification actions as sysctl updates.
It addresses the inconsistency by calling devinet_conf_post_set() inside
inet_set_link_af() and deferring cache flushing.

> diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
> index 8300516fb38f4..a35b72662e431 100644
> --- a/net/ipv4/devinet.c
> +++ b/net/ipv4/devinet.c
> @@ -2161,6 +2161,20 @@ static bool devinet_conf_post_set(struct net *net, struct ipv4_devconf *cnf,
[ ... ]
> +	case IPV4_DEVCONF_FORWARDING:
> +		if (new == 1) {
> +			/* it is safe to use container_of() because forwarding case
> +			 * is only used by the netlink path
> +			 */
> +			struct in_device *idev = container_of(cnf, struct in_device, cnf);
> +
> +			netif_disable_lro(idev->dev);

[Severity: High]
Does calling netif_disable_lro() directly here without acquiring the
netdev_lock_ops mutex cause lockdep assertion failures?

Since inet_set_link_af() runs only under rtnl_lock() and does not acquire
the device ops lock, this direct invocation appears to bypass the locking
requirements for netif_disable_lro(), which modifies dev->wanted_features
and invokes netdev_update_features(dev).

On drivers that opt-in to netdev_lock_ops, does this create a concurrent
data race on the device features mask? Should this use dev_disable_lro()
instead, which correctly acquires and releases netdev_lock_ops(dev) around
the netif_disable_lro() call?

> +		}
> +
> +		inet_netconf_notify_devconf(net, RTM_NEWNETCONF,
> +					    NETCONFA_FORWARDING,
> +					    ifindex, cnf);
> +		return true;

Re: [PATCH 2/3 net-next v6] ipv4: handle devconf post-set actions on netlink updates

[Fernando Fernandez Mancera]

On 6/12/26 2:38 PM, Simon Horman wrote:
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
> Full review at: https://sashiko.dev
> ---
> ipv4: handle devconf post-set actions on netlink updates
> 
> This commit ensures that IPv4 device configuration parameters updated
> via netlink trigger the same post-modification actions as sysctl updates.
> It addresses the inconsistency by calling devinet_conf_post_set() inside
> inet_set_link_af() and deferring cache flushing.
> 
>> diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
>> index 8300516fb38f4..a35b72662e431 100644
>> --- a/net/ipv4/devinet.c
>> +++ b/net/ipv4/devinet.c
>> @@ -2161,6 +2161,20 @@ static bool devinet_conf_post_set(struct net *net, struct ipv4_devconf *cnf,
> [ ... ]
>> +	case IPV4_DEVCONF_FORWARDING:
>> +		if (new == 1) {
>> +			/* it is safe to use container_of() because forwarding case
>> +			 * is only used by the netlink path
>> +			 */
>> +			struct in_device *idev = container_of(cnf, struct in_device, cnf);
>> +
>> +			netif_disable_lro(idev->dev);
> 
> [Severity: High]
> Does calling netif_disable_lro() directly here without acquiring the
> netdev_lock_ops mutex cause lockdep assertion failures?
> 
> Since inet_set_link_af() runs only under rtnl_lock() and does not acquire
> the device ops lock, this direct invocation appears to bypass the locking
> requirements for netif_disable_lro(), which modifies dev->wanted_features
> and invokes netdev_update_features(dev).
> 
> On drivers that opt-in to netdev_lock_ops, does this create a concurrent
> data race on the device features mask? Should this use dev_disable_lro()
> instead, which correctly acquires and releases netdev_lock_ops(dev) around
> the netif_disable_lro() call?
> 

The device operations lock is held when handling IFLA_AF_SPEC at
net/core/rtnetlink.c. Calling netif_disable_lro() is the right thing to
do IMHO. Keep on mind that FORWARDING case here is only triggered by
netlink path and not sysctl.

AI keeps complaining about this but it isn't right AFAICS.

Thanks,
Fernando.

>> +		}
>> +
>> +		inet_netconf_notify_devconf(net, RTM_NEWNETCONF,
>> +					    NETCONFA_FORWARDING,
>> +					    ifindex, cnf);
>> +		return true;
> 

Re: [PATCH 1/3 net-next v6] ipv4: centralize devconf sysctl handling

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Tue,  9 Jun 2026 22:45:18 +0200 you wrote:
> The logic for handling IPv4 devconf sysctls is scattered. Notification
> and cache flushes are managed in devinet_conf_proc(), while a separate
> ipv4_doint_and_flush() function and DEVINET_SYSCTL_FLUSHING_ENTRY macro
> is used for properties that solely require a cache flush.
> 
> This patch refactors the sysctl handling by introducing a centralized
> helper, devinet_conf_post_set(). This new function evaluates the changed
> attribute and handles all necessary operations like triggering netlink
> notifications. It returns a boolean indicating whether a routing cache
> flush is required.
> 
> [...]

Here is the summary with links:
  - [1/3,net-next,v6] ipv4: centralize devconf sysctl handling
    https://git.kernel.org/netdev/net-next/c/3a29b55505f3
  - [2/3,net-next,v6] ipv4: handle devconf post-set actions on netlink updates
    https://git.kernel.org/netdev/net-next/c/489730ec2a73
  - [3/3,net-next,v6] selftests: net: add test for IPv4 devconf netlink notifications
    https://git.kernel.org/netdev/net-next/c/32229484e381

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html